[PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c

Ricardo Neri posted 10 patches 3 months, 1 week ago
[PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Ricardo Neri 3 months, 1 week ago
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@linux.intel.com>
Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
 - Removed dependency on CONFIG_OF. It will be added in a later patch.
   (Rafael)
 - Rebased on v6.16-rc3.

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 71019b3b54ea..e3009cb59928 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1114,6 +1114,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 ACPI_MADT_WAKEUP
+	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 0d2a6d953be9..1fce3d20cf2d 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 c3ac5ecf3e7d..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, unsigned int cpu)
-{
-	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..5089bcda615d
--- /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, unsigned int cpu)
+{
+	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
Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Rafael J. Wysocki 3 months, 1 week ago
On Sat, Jun 28, 2025 at 5:35 AM Ricardo Neri
<ricardo.neri-calderon@linux.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.

IMV "that can be used by both ACPI and DeviceTree" would sound better.

> No functional changes are intended.
>
> Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

With the above addressed

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Changes since v4:
>  - Removed dependency on CONFIG_OF. It will be added in a later patch.
>    (Rafael)
>  - Rebased on v6.16-rc3.
>
> 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 71019b3b54ea..e3009cb59928 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1114,6 +1114,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 ACPI_MADT_WAKEUP
> +       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 0d2a6d953be9..1fce3d20cf2d 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 c3ac5ecf3e7d..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, unsigned int cpu)
> -{
> -       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..5089bcda615d
> --- /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, unsigned int cpu)
> +{
> +       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
>
Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Ricardo Neri 3 months, 1 week ago
On Mon, Jun 30, 2025 at 08:58:56PM +0200, Rafael J. Wysocki wrote:
> On Sat, Jun 28, 2025 at 5:35 AM Ricardo Neri
> <ricardo.neri-calderon@linux.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.
> 
> IMV "that can be used by both ACPI and DeviceTree" would sound better.

I have taken your rewording suggestion.

> 
> > No functional changes are intended.
> >
> > Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> 
> With the above addressed
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thank you!
Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Peter Zijlstra 3 months, 1 week ago
On Fri, Jun 27, 2025 at 08:35:08PM -0700, Ricardo Neri wrote:

> -	/*
> -	 * 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;
> -}

> +	while (READ_ONCE(acpi_mp_wake_mailbox->command))
> +		cpu_relax();
> +
> +	return 0;
> +}

So I realize this is just code movement at this point, but this will
hard lockup the machine if the AP doesn't come up, right?

IIRC we have some timeout in the regular SIPI bringup if the AP doesn't
respond.
Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Kirill A. Shutemov 3 months, 1 week ago
On Mon, Jun 30, 2025 at 01:03:16PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 27, 2025 at 08:35:08PM -0700, Ricardo Neri wrote:
> 
> > -	/*
> > -	 * 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;
> > -}
> 
> > +	while (READ_ONCE(acpi_mp_wake_mailbox->command))
> > +		cpu_relax();
> > +
> > +	return 0;
> > +}
> 
> So I realize this is just code movement at this point, but this will
> hard lockup the machine if the AP doesn't come up, right?

Correct.

> IIRC we have some timeout in the regular SIPI bringup if the AP doesn't
> respond.

See the comment.

In TDX guest case, we need to consider malicious VMM that can postpone
scheduling the target vCPU indefinitely. It can give VMM indirect control
of what the target would run upon wakeup. Like, it can wait until the
guest do kexec and the same start RIP would point non-startup code.

I hope we will get SIPI-based CPU bringup in TDX guest eventually. It will
be more reliable.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Peter Zijlstra 3 months, 1 week ago
On Mon, Jun 30, 2025 at 03:07:08PM +0300, Kirill A. Shutemov wrote:
> On Mon, Jun 30, 2025 at 01:03:16PM +0200, Peter Zijlstra wrote:
> > On Fri, Jun 27, 2025 at 08:35:08PM -0700, Ricardo Neri wrote:
> > 
> > > -	/*
> > > -	 * 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;
> > > -}
> > 
> > > +	while (READ_ONCE(acpi_mp_wake_mailbox->command))
> > > +		cpu_relax();
> > > +
> > > +	return 0;
> > > +}
> > 
> > So I realize this is just code movement at this point, but this will
> > hard lockup the machine if the AP doesn't come up, right?
> 
> Correct.
> 
> > IIRC we have some timeout in the regular SIPI bringup if the AP doesn't
> > respond.
> 
> See the comment.

Doh, reading hard ;-) Thanks!