lkml.org 
[lkml]   [2021]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support
From
Date


On 5/21/21 7:45 AM, Peter Zijlstra wrote:
> On Thu, May 13, 2021 at 02:37:32PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip)
>> +{
>> + u8 timeout = 0xFF;
>> +
>> + /* Remap mailbox memory only for the first call to acpi_wakeup_cpu() */
>> + if (physids_empty(apic_id_wakemap))
>> + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
>> + sizeof(*acpi_mp_wake_mailbox),
>> + MEMREMAP_WB);
>
> { } for being multi-line

Yes. I will fix it.

>
>> + /*
>> + * According to the ACPI specification r6.4, sec 5.2.12.19, the
>> + * mailbox-based wakeup mechanism cannot be used more than once
>> + * for the same CPU, so skip sending wake commands to already
>> + * awake CPU.
>> + */
>> + if (physid_isset(apicid, apic_id_wakemap)) {
>> + pr_err("CPU already awake (APIC ID %x), skipping wakeup\n",
>> + apicid);
>> + return -EINVAL;
>> + }
>> +
>> +
>> + /*
>> + * Mailbox memory is shared between firmware and OS. Firmware will
>> + * listen on mailbox command address, and once it receives the wakeup
>> + * command, CPU associated with the given apicid will be booted. So,
>> + * the value of apic_id and wakeup_vector has to be set before updating
>> + * the wakeup command. So use WRITE_ONCE to let the compiler know about
>> + * it and preserve the order of writes.
>> + */
>> + WRITE_ONCE(acpi_mp_wake_mailbox->apic_id, apicid);
>> + WRITE_ONCE(acpi_mp_wake_mailbox->wakeup_vector, start_ip);
>> + WRITE_ONCE(acpi_mp_wake_mailbox->command, ACPI_MP_WAKE_COMMAND_WAKEUP);
>
> Do those want to be smp_store_release(), in addition to being a volatile
> write, those also include compiler barriers to make sure the compiler
> doesn't lift stuff around.

I think we can use smp_store_release(). Let me test and add it in next
version.

>
>> +
>> + /*
>> + * After writing wakeup command, wait for maximum timeout of 0xFF
>> + * for firmware to reset the command address back zero to indicate
>> + * the successful reception of command.
>> + * NOTE: 255 as timeout value is decided based on our experiments.
>> + *
>> + * XXX: Change the timeout once ACPI specification comes up with
>> + * standard maximum timeout value.
>> + */
>> + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--)
>> + cpu_relax();
>
> What's the unit of the timeout? The mailbox reads, the PAUSE
> instruction?

Read mailbox memory, timeout dec and then pause. Its more like busy wait loop.

And timeout count is decided based on our experiments. Once spec defines a
standard, we can modify it.

>
>> +
>> + if (timeout) {
>> + /*
>> + * If the CPU wakeup process is successful, store the
>> + * status in apic_id_wakemap to prevent re-wakeup
>> + * requests.
>> + */
>> + physid_set(apicid, apic_id_wakemap);
>> + return 0;
>> + }
>> +
>> + /* If timed out (timeout == 0), return error */
>> + return -EIO;
>> +}
>> +
>> #endif /*CONFIG_X86_LOCAL_APIC */

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

\
 
 \ /
  Last update: 2021-05-21 17:17    [W:0.121 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site