On Tue, 2024-07-02 at 08:58 +0800, Zhiquan Li wrote:
> The issue was found on the platform that using "Multiprocessor Wakeup
> Structure"[1] to startup secondary CPU, which is usually used by
> encrypted guest. When restrict boot time CPU to 1 with the kernel
> parameter "maxcpus=1" and bring other CPUs online later, there will be
> a kernel panic.
>
> The variable acpi_mp_wake_mailbox, which holds the virtual address of
> the MP Wakeup Structure mailbox, will be set as read-only after init.
> If the first AP gets online later, after init, the attempt to update
> the variable results in panic.
>
> The memremap() call that initializes the variable cannot be moved into
> acpi_parse_mp_wake() because memremap() is not functional at that point
> in the boot process.
>
> [1] Details about the MP Wakeup structure can be found in ACPI v6.4, in
> the "Multiprocessor Wakeup Structure" section.
>
> Signed-off-by: Zhiquan Li <zhiquan1.li@xxxxxxxxx>
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Seems this changelog only mentions the problem, but doesn't say how to fix:
Remove the __ro_after_init annotation of acpi_mp_wake_mailbox to fix.
[...]
>
> /* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> -static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox __ro_after_init;
> +static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
>
>
But if memremap() isn't available in acpi_parse_mp_wake() is the concern, could
we change to do it before smp_init() but after memremap() is available?
E.g, we should be able to use early_initcall() (only compile tested):
diff --git a/arch/x86/kernel/acpi/madt_wakeup.c
b/arch/x86/kernel/acpi/madt_wakeup.c
index 6cfe762be28b..ca0aea0832ac 100644
--- a/arch/x86/kernel/acpi/madt_wakeup.c
+++ b/arch/x86/kernel/acpi/madt_wakeup.c
@@ -176,18 +176,6 @@ static int acpi_wakeup_cpu(u32 apicid, unsigned long
start_ip)
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
@@ -290,3 +278,29 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers
*header,
return 0;
}
+
+static int __init acpi_mp_wake_map_mailbox(void)
+{
+ /*
+ * Nothing to do if the "Multiprocessor Wakeup Structure" is
+ * not present. acpi_mp_wake_mailbox_paddr could also be 0
+ * if the kernel is from kexec.
+ */
+ if (!acpi_mp_wake_mailbox_paddr)
+ return 0;
+
+ /* memremap() isn't available in acpi_parse_mp_wake() */
+ acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
+ sizeof(*acpi_mp_wake_mailbox),
+ MEMREMAP_WB);
+ /*
+ * When memremap() fails, clear acpi_mp_wake_mailbox_paddr
+ * to prevent NULL dereference of acpi_mp_wake_mailbox
+ * when bringing up secondary CPUs.
+ */
+ if (WARN_ON(!acpi_mp_wake_mailbox))
+ acpi_mp_wake_mailbox_paddr = 0;
+
+ return 0;
+}
+early_initcall(acpi_mp_wake_map_mailbox);
