[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0geZAnLRkeunW06JKE1gyDcd15EGzqJ_A-cZHO_koJVAw@mail.gmail.com>
Date: Wed, 4 Jun 2025 11:12:53 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc: x86@...nel.org, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Rob Herring <robh@...nel.org>, "K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
Dexuan Cui <decui@...rosoft.com>, Michael Kelley <mhklinux@...look.com>,
"Rafael J. Wysocki" <rafael@...nel.org>, Saurabh Sengar <ssengar@...ux.microsoft.com>,
Chris Oo <cho@...rosoft.com>, "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
linux-hyperv@...r.kernel.org, devicetree@...r.kernel.org,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
"Ravi V. Shankar" <ravi.v.shankar@...el.com>, Ricardo Neri <ricardo.neri@...el.com>,
Yunhong Jiang <yunhong.jiang@...ux.intel.com>
Subject: Re: [PATCH v4 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
On Wed, Jun 4, 2025 at 2:18 AM Ricardo Neri
<ricardo.neri-calderon@...ux.intel.com> wrote:
>
> The bootstrap processor uses acpi_wakeup_cpu() to indicate to firmware that
> it wants to boot a secondary CPU using a mailbox as described in the
> Multiprocessor Wakeup Structure of the ACPI specification.
>
> The platform firmware may implement the mailbox as described in the ACPI
> specification but enumerate it using a DeviceTree graph. An example of
> this is OpenHCL paravisor.
>
> Move the code used to setup and use the mailbox for CPU wakeup out of the
> ACPI directory into a new smpwakeup.c file that both ACPI and DeviceTree
> can use.
>
> No functional changes are intended.
>
> Co-developed-by: Yunhong Jiang <yunhong.jiang@...ux.intel.com>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@...ux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
> ---
> Changes since v3:
> - Create a new file smpwakeup.c instead of relocating it to smpboot.c.
> (Rafael)
>
> Changes since v2:
> - Only move to smpboot.c the portions of the code that configure and
> use the mailbox. This also resolved the compile warnings about unused
> functions that Michael Kelley reported.
> - Edited the commit message for clarity.
>
> Changes since v1:
> - None.
> ---
> arch/x86/Kconfig | 7 ++++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/acpi/madt_wakeup.c | 76 ----------------------------------
> arch/x86/kernel/smpwakeup.c | 83 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 91 insertions(+), 76 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cb0f4af31789..82147edb355a 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1113,6 +1113,13 @@ config X86_LOCAL_APIC
> depends on X86_64 || SMP || X86_UP_APIC || PCI_MSI
> select IRQ_DOMAIN_HIERARCHY
>
> +config X86_MAILBOX_WAKEUP
> + def_bool y
> + depends on OF || ACPI_MADT_WAKEUP
At this point the dependency on OF is premature. IMV it should be
added in a later patch.
> + depends on X86_64
> + depends on SMP
> + depends on X86_LOCAL_APIC
> +
> config ACPI_MADT_WAKEUP
> def_bool y
> depends on X86_64
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 99a783fd4691..8f078af42a71 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -94,6 +94,7 @@ apm-y := apm_32.o
> obj-$(CONFIG_APM) += apm.o
> obj-$(CONFIG_SMP) += smp.o
> obj-$(CONFIG_SMP) += smpboot.o
> +obj-$(CONFIG_X86_MAILBOX_WAKEUP) += smpwakeup.o
> obj-$(CONFIG_X86_TSC) += tsc_sync.o
> obj-$(CONFIG_SMP) += setup_percpu.o
> obj-$(CONFIG_X86_MPPARSE) += mpparse.o
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
> index 4033c804307a..a7e0158269b0 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -2,12 +2,10 @@
> #include <linux/acpi.h>
> #include <linux/cpu.h>
> #include <linux/delay.h>
> -#include <linux/io.h>
> #include <linux/kexec.h>
> #include <linux/memblock.h>
> #include <linux/pgtable.h>
> #include <linux/sched/hotplug.h>
> -#include <asm/apic.h>
> #include <asm/barrier.h>
> #include <asm/init.h>
> #include <asm/intel_pt.h>
> @@ -15,12 +13,6 @@
> #include <asm/processor.h>
> #include <asm/reboot.h>
>
> -/* Physical address of the Multiprocessor Wakeup Structure mailbox */
> -static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> -
> -/* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> -static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> -
> static u64 acpi_mp_pgd __ro_after_init;
> static u64 acpi_mp_reset_vector_paddr __ro_after_init;
>
> @@ -127,63 +119,6 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
> return 0;
> }
>
> -static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> -{
> - if (!acpi_mp_wake_mailbox_paddr) {
> - pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs. Booting with kexec?\n");
> - return -EOPNOTSUPP;
> - }
> -
> - /*
> - * Remap mailbox memory only for the first call to acpi_wakeup_cpu().
> - *
> - * Wakeup of secondary CPUs is fully serialized in the core code.
> - * No need to protect acpi_mp_wake_mailbox from concurrent accesses.
> - */
> - if (!acpi_mp_wake_mailbox) {
> - acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> - sizeof(*acpi_mp_wake_mailbox),
> - MEMREMAP_WB);
> - }
> -
> - /*
> - * Mailbox memory is shared between the firmware and OS. Firmware will
> - * listen on mailbox command address, and once it receives the wakeup
> - * command, the CPU associated with the given apicid will be booted.
> - *
> - * The value of 'apic_id' and 'wakeup_vector' must be visible to the
> - * firmware before the wakeup command is visible. smp_store_release()
> - * ensures ordering and visibility.
> - */
> - acpi_mp_wake_mailbox->apic_id = apicid;
> - acpi_mp_wake_mailbox->wakeup_vector = start_ip;
> - smp_store_release(&acpi_mp_wake_mailbox->command,
> - ACPI_MP_WAKE_COMMAND_WAKEUP);
> -
> - /*
> - * Wait for the CPU to wake up.
> - *
> - * The CPU being woken up is essentially in a spin loop waiting to be
> - * woken up. It should not take long for it wake up and acknowledge by
> - * zeroing out ->command.
> - *
> - * ACPI specification doesn't provide any guidance on how long kernel
> - * has to wait for a wake up acknowledgment. It also doesn't provide
> - * a way to cancel a wake up request if it takes too long.
> - *
> - * In TDX environment, the VMM has control over how long it takes to
> - * wake up secondary. It can postpone scheduling secondary vCPU
> - * indefinitely. Giving up on wake up request and reporting error opens
> - * possible attack vector for VMM: it can wake up a secondary CPU when
> - * kernel doesn't expect it. Wait until positive result of the wake up
> - * request.
> - */
> - while (READ_ONCE(acpi_mp_wake_mailbox->command))
> - cpu_relax();
> -
> - return 0;
> -}
> -
> static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake)
> {
> cpu_hotplug_disable_offlining();
> @@ -246,14 +181,3 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
>
> return 0;
> }
> -
> -void __init acpi_setup_mp_wakeup_mailbox(u64 mailbox_paddr)
> -{
> - acpi_mp_wake_mailbox_paddr = mailbox_paddr;
> - apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> -}
> -
> -struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
> -{
> - return acpi_mp_wake_mailbox;
> -}
> diff --git a/arch/x86/kernel/smpwakeup.c b/arch/x86/kernel/smpwakeup.c
> new file mode 100644
> index 000000000000..e34ffbfffaf5
> --- /dev/null
> +++ b/arch/x86/kernel/smpwakeup.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/acpi.h>
> +#include <linux/io.h>
> +#include <linux/printk.h>
> +#include <linux/types.h>
> +#include <asm/apic.h>
> +#include <asm/barrier.h>
> +#include <asm/processor.h>
> +
> +/* Physical address of the Multiprocessor Wakeup Structure mailbox */
> +static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> +
> +/* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> +static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> +
> +static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> +{
> + if (!acpi_mp_wake_mailbox_paddr) {
> + pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs. Booting with kexec?\n");
> + return -EOPNOTSUPP;
> + }
> +
> + /*
> + * Remap mailbox memory only for the first call to acpi_wakeup_cpu().
> + *
> + * Wakeup of secondary CPUs is fully serialized in the core code.
> + * No need to protect acpi_mp_wake_mailbox from concurrent accesses.
> + */
> + if (!acpi_mp_wake_mailbox) {
> + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> + sizeof(*acpi_mp_wake_mailbox),
> + MEMREMAP_WB);
> + }
> +
> + /*
> + * Mailbox memory is shared between the firmware and OS. Firmware will
> + * listen on mailbox command address, and once it receives the wakeup
> + * command, the CPU associated with the given apicid will be booted.
> + *
> + * The value of 'apic_id' and 'wakeup_vector' must be visible to the
> + * firmware before the wakeup command is visible. smp_store_release()
> + * ensures ordering and visibility.
> + */
> + acpi_mp_wake_mailbox->apic_id = apicid;
> + acpi_mp_wake_mailbox->wakeup_vector = start_ip;
> + smp_store_release(&acpi_mp_wake_mailbox->command,
> + ACPI_MP_WAKE_COMMAND_WAKEUP);
> +
> + /*
> + * Wait for the CPU to wake up.
> + *
> + * The CPU being woken up is essentially in a spin loop waiting to be
> + * woken up. It should not take long for it wake up and acknowledge by
> + * zeroing out ->command.
> + *
> + * ACPI specification doesn't provide any guidance on how long kernel
> + * has to wait for a wake up acknowledgment. It also doesn't provide
> + * a way to cancel a wake up request if it takes too long.
> + *
> + * In TDX environment, the VMM has control over how long it takes to
> + * wake up secondary. It can postpone scheduling secondary vCPU
> + * indefinitely. Giving up on wake up request and reporting error opens
> + * possible attack vector for VMM: it can wake up a secondary CPU when
> + * kernel doesn't expect it. Wait until positive result of the wake up
> + * request.
> + */
> + while (READ_ONCE(acpi_mp_wake_mailbox->command))
> + cpu_relax();
> +
> + return 0;
> +}
> +
> +void __init acpi_setup_mp_wakeup_mailbox(u64 mailbox_paddr)
> +{
> + acpi_mp_wake_mailbox_paddr = mailbox_paddr;
> + apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> +}
> +
> +struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
> +{
> + return acpi_mp_wake_mailbox;
> +}
>
> --
> 2.43.0
>
Powered by blists - more mailing lists