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

Ricardo Neri posted 10 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Ricardo Neri 3 months, 3 weeks 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 the 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 can be used by both ACPI
and DeviceTree.

No functional changes are intended.

Reviewed-by: Dexuan Cui <decui@microsoft.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
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 v5:
 - Minor change to the changelog. (Rafael)
 - Added Acked-by tag from Rafael. Thanks!
 - Added Reviewed-by tag from Dexuan. Thanks!

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 fa3b616af03a..f57192a34a87 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1108,6 +1108,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 bc184dd38d99..a9a1fbc798fb 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 v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Borislav Petkov 3 months, 2 weeks ago
On Thu, Oct 16, 2025 at 07:57:24PM -0700, Ricardo Neri wrote:
>  arch/x86/kernel/acpi/madt_wakeup.c | 76 ----------------------------------
>  arch/x86/kernel/smpwakeup.c        | 83 ++++++++++++++++++++++++++++++++++++++

How does ACPI-related code belong in the arch/x86/kernel/ hierarchy?

Not to mention that arch/x86/kernel/ is a dumping ground for everything *and*
the kitchen sink so we should not put more crap in there...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Ricardo Neri 3 months, 2 weeks ago
On Mon, Oct 27, 2025 at 03:18:35PM +0100, Borislav Petkov wrote:
> On Thu, Oct 16, 2025 at 07:57:24PM -0700, Ricardo Neri wrote:
> >  arch/x86/kernel/acpi/madt_wakeup.c | 76 ----------------------------------
> >  arch/x86/kernel/smpwakeup.c        | 83 ++++++++++++++++++++++++++++++++++++++
> 
> How does ACPI-related code belong in the arch/x86/kernel/ hierarchy?
> 
> Not to mention that arch/x86/kernel/ is a dumping ground for everything *and*
> the kitchen sink so we should not put more crap in there...

Right. All the functions in the file start with the acpi_ prefix. It could
be kept under arch/x86/kernel/acpi/. The Kconfig symbol X86_MAILBOX_WAKEUP
would have to live in arch/x86/Kconfig as there is no Kconfig file under
arch/x86/kernel/acpi. ACPI_MADT_WAKEUP is arch/x86/Kconfig.

Does that sound acceptable?

Thank you for your feedback, Boris,

Best,
Ricardo
Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Borislav Petkov 3 months, 1 week ago
On Mon, Oct 27, 2025 at 01:58:16PM -0700, Ricardo Neri wrote:
> Right. All the functions in the file start with the acpi_ prefix. It could
> be kept under arch/x86/kernel/acpi/. The Kconfig symbol X86_MAILBOX_WAKEUP
> would have to live in arch/x86/Kconfig as there is no Kconfig file under
> arch/x86/kernel/acpi. ACPI_MADT_WAKEUP is arch/x86/Kconfig.
> 
> Does that sound acceptable?

Right, this looks kinda weird. You have devicetree thing using ACPI code,
you're trying to carve it out but then it is ACPI code anyway. So why even do
that?

You can simply leave ACPI enabled on that configuration. I don't see yet what
the point for the split is - saving memory, or...?

> Thank you for your feedback, Boris,

Sure, np. Trying my best. :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Ricardo Neri 3 months, 1 week ago
On Wed, Oct 29, 2025 at 12:13:58PM +0100, Borislav Petkov wrote:
> On Mon, Oct 27, 2025 at 01:58:16PM -0700, Ricardo Neri wrote:
> > Right. All the functions in the file start with the acpi_ prefix. It could
> > be kept under arch/x86/kernel/acpi/. The Kconfig symbol X86_MAILBOX_WAKEUP
> > would have to live in arch/x86/Kconfig as there is no Kconfig file under
> > arch/x86/kernel/acpi. ACPI_MADT_WAKEUP is arch/x86/Kconfig.
> > 
> > Does that sound acceptable?
> 
> Right, this looks kinda weird. You have devicetree thing using ACPI code,
> you're trying to carve it out but then it is ACPI code anyway. So why even do
> that?
> 
> You can simply leave ACPI enabled on that configuration. I don't see yet what
> the point for the split is - saving memory, or...?

I did not want to enable the whole of ACPI code as I need a tiny portion of it.
Then yes, saving memory and having a smaller binary were considerations.

The only dependency that ACPI_MADT_WAKEUP has on ACPI is the code to read and
parse the ACPI table that enumerates the mailbox. (There are a couple of
declarations for CPU offlining that need tweaking if I want ACPI_MADT_WAKEUP to
not depend on ACPI at all).

The DeviceTree firmware only needs the code to wake CPUs up. That is the code
I am carving out.

Having said that, vmlinux and bzImage increase by 4% if I enable ACPI.

Thanks and BR,
Ricardo
Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Borislav Petkov 3 months, 1 week ago
On Wed, Oct 29, 2025 at 10:43:50PM -0700, Ricardo Neri wrote:
> I did not want to enable the whole of ACPI code as I need a tiny portion of it.
> Then yes, saving memory and having a smaller binary were considerations.
> 
> The only dependency that ACPI_MADT_WAKEUP has on ACPI is the code to read and
> parse the ACPI table that enumerates the mailbox. (There are a couple of
> declarations for CPU offlining that need tweaking if I want ACPI_MADT_WAKEUP to
> not depend on ACPI at all).
> 
> The DeviceTree firmware only needs the code to wake CPUs up. That is the code
> I am carving out.
> 
> Having said that, vmlinux and bzImage increase by 4% if I enable ACPI.

So, is it a concern or not? I cannot understand from the above whether you
care about 4% or not.

If you do, I guess you can make a piece of ACPI code available through another
Kconfig option but keep it in the ACPI hierarchy.

Because no matter how you look at it, it is ACPI code which is trying to be
generic and failing at that.

Unless you scrub it completely and make it a generic thing which is used by
ACPI too.

Which would be a separate patchset.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Ricardo Neri 3 months ago
On Mon, Nov 03, 2025 at 02:40:37PM +0100, Borislav Petkov wrote:
> On Wed, Oct 29, 2025 at 10:43:50PM -0700, Ricardo Neri wrote:
> > I did not want to enable the whole of ACPI code as I need a tiny portion of it.
> > Then yes, saving memory and having a smaller binary were considerations.
> > 
> > The only dependency that ACPI_MADT_WAKEUP has on ACPI is the code to read and
> > parse the ACPI table that enumerates the mailbox. (There are a couple of
> > declarations for CPU offlining that need tweaking if I want ACPI_MADT_WAKEUP to
> > not depend on ACPI at all).
> > 
> > The DeviceTree firmware only needs the code to wake CPUs up. That is the code
> > I am carving out.
> > 
> > Having said that, vmlinux and bzImage increase by 4% if I enable ACPI.
> 
> So, is it a concern or not? I cannot understand from the above whether you
> care about 4% or not.

I apologize for my late reply. Also, I am sorry I was not clear. I needed to
consult with a few stakeholders whether they could live with the increase in
size resulting from having CONFIG_ACPI=y. They can.

If it is OK with Rafael, I plan to post a new version that drops this patch and
adds the necessary function stubs for the !CONFIG_ACPI case.

Thanks and BR,
Ricardo
Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Borislav Petkov 3 months ago
On Mon, Nov 10, 2025 at 09:49:38AM -0800, Ricardo Neri wrote:
> I apologize for my late reply. Also, I am sorry I was not clear. I needed to
> consult with a few stakeholders whether they could live with the increase in
> size resulting from having CONFIG_ACPI=y. They can.
> 
> If it is OK with Rafael, I plan to post a new version that drops this patch and
> adds the necessary function stubs for the !CONFIG_ACPI case.

Sounds good to me.

It is the simplest thing to do. If the size increase bothers someone, we can
always do the more involved refactoring later.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
Posted by Rafael J. Wysocki 3 months ago
On Mon, Nov 10, 2025 at 8:47 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Nov 10, 2025 at 09:49:38AM -0800, Ricardo Neri wrote:
> > I apologize for my late reply. Also, I am sorry I was not clear. I needed to
> > consult with a few stakeholders whether they could live with the increase in
> > size resulting from having CONFIG_ACPI=y. They can.
> >
> > If it is OK with Rafael, I plan to post a new version that drops this patch and
> > adds the necessary function stubs for the !CONFIG_ACPI case.
>
> Sounds good to me.

Yeah, sounds good.

> It is the simplest thing to do. If the size increase bothers someone, we can
> always do the more involved refactoring later.

So long as they have a good enough justification for it that is.