linux-kernel - Re: [PATCH RESEND v4] x86/acpi: fix panic while AP online later with kernel parameter maxcpus=1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJZ5v0j-rEioP-BMYJwDYKeT_M_5FBP25UA6GHvRpzxd6oiqTg@mail.gmail.com>
Date: Mon, 26 Aug 2024 18:49:58 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Zhiquan Li <zhiquan1.li@...el.com>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, kirill.shutemov@...ux.intel.com, x86@...nel.org, 
	rafael@...nel.org, hpa@...or.com, kai.huang@...el.com, 
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v4] x86/acpi: fix panic while AP online later with
 kernel parameter maxcpus=1

On Mon, Aug 5, 2024 at 12:03 PM Zhiquan Li <zhiquan1.li@...el.com> 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.  Before waking up the APs, BSP should memremap() the
> physical address of the MP Wakeup Structure mailbox to the variable
> acpi_mp_wake_mailbox, which holds the virtual address of mailbox.  When
> BSP needs to wake up the APs, it writes the APIC ID of APs, wakeup
> vector, and the ACPI_MP_WAKE_COMMAND_WAKEUP command into the mailbox.
>
> Current implementation doesn't consider the case that restricts boot
> time CPU to 1 with the kernel parameter "maxcpus=1" and brings other
> CPUs online later, the variable acpi_mp_wake_mailbox will be set as
> read-only after init.  So when the first AP gets online 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.  Moreover, the APs might never be brought up, keep
> the memremap() call in acpi_wakeup_cpu() so that the operation only
> takes place on demand.
>
> [1] Details about the MP Wakeup structure can be found in ACPI v6.4, in
>     the "Multiprocessor Wakeup Structure" section.
>
> Fixes: 24dd05da8c79 ("x86/apic: Mark acpi_mp_wake_* variables as __ro_after_init")
> Signed-off-by: Zhiquan Li <zhiquan1.li@...el.com>
> Reviewed-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>

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

> ---
>
> V4 RESEND note:
> - No changes on this, just rebasing as v6.11-rc1 is out.
>
> V3: https://lore.kernel.org/all/20240702005800.622910-1-zhiquan1.li@intel.com/
>
> Changes since V3:
> - Add Fixes tag for the commit found by Kai.
> - Extend the commit message for the root cause and solution.
>
> V2: https://lore.kernel.org/all/20240628082119.357735-1-zhiquan1.li@intel.com/
>
> Changes since V2:
> - Modify the commit log as suggested by Kirill.
> - Add Kirill's Reviewed-by tag.
>
> V1: https://lore.kernel.org/all/20240626073920.176471-1-zhiquan1.li@intel.com/
>
> Changes since V1:
> - Amend the commit message as per Kirill's comments.
> - Remove the oops message.
> ---
>  arch/x86/kernel/acpi/madt_wakeup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
> index 6cfe762be28b..d5ef6215583b 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -19,7 +19,7 @@
>  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 __ro_after_init;
> +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;
>
> base-commit: 435dfff07e5b71ceecc35954645740ab91090fbb
> --
> 2.25.1
>