linux-kernel - Re: [PATCH v3] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <9774DEAE-8E9E-40CD-B5A5-0A19243BB432@alien8.de>
Date: Thu, 04 Jul 2024 13:31:00 +0200
From: Borislav Petkov <bp@...en8.de>
To: Zhiquan Li <zhiquan1.li@...el.com>, "Huang, Kai" <kai.huang@...el.com>
CC: "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
 "rafael@...nel.org" <rafael@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
 "mingo@...hat.com" <mingo@...hat.com>,
 "tglx@...utronix.de" <tglx@...utronix.de>,
 "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "x86@...nel.org" <x86@...nel.org>,
 "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v3] x86/acpi: fix panic while AP online later with kernel parameter maxcpus=1

On July 3, 2024 4:39:43 AM GMT+02:00, Zhiquan Li <zhiquan1.li@...el.com> wrote:
>
>On 2024/7/3 07:55, Huang, Kai wrote:
>>> This happens to us maintainers at least once a week. Well, I don't want
>>> that to happen in my tree anymore.
>>>
>>> So none of this text above still doesn't explain to me *why* this is
>>> happening.
>>>
>>> Why do APs need to update acpi_mp_wake_mailbox?
>
>Not AP needs to update acpi_mp_wake_mailbox, but BSP might need to
>update it after the init stage.  In the encrypted guest CPU hot-plug
>scenario, BSP memremap() the acpi_mp_wake_mailbox_paddr, and writes APIC
>ID of APs, wakeup vector and the ACPI_MP_WAKE_COMMAND_WAKEUP command
>into mailbox.  Firmware will listen on mailbox command address, and once
>it receives the wakeup command, the CPU associated with the given apicid
>will be booted.
>
>We cannot assume that all APs will be brought up in the init stage.
>
>> They don't need to if acpi_mp_wake_mailbox can be setup before smp_init()
>> once for all.
>> 
>> But currently the setup of acpi_mp_wake_mailbox is done when the first AP is
>> brought up because memremap() doesn't work in acpi_parse_mp_wake(), as
>> mentioned in the changelog of this patch.
>> 
>> I also feel it's not ideal to setup acpi_mp_wake_mailbox when bringing up
>> the first AP, so I provided my diff.  IIUC, if memremap() works for
>> acpi_mp_wake_mailbox when bringing up the first AP, then it should also work
>> in
>> the early_initcall().
>
>Besides the factor that whether memremap() is functional at the point in
>the boot process, another reason I can think of is, if the intention is
>just to work with BSP, then the remapping is a redundant step.
>Especially in the kexec & kdump case, the capture kernel only needs
>single CPU to work usually with the "maxcpus=1" option.
>
>IMHO, the solution that postpone the remapping while really needs to
>bring up APs is reasonable, just don't make acpi_mp_wake_mailbox
>read-only.  The APs might be brought up later, might be never.
>
>
>> 
>>> Which patch is this fixing?
>> It fiexes below commit AFAICT:
>> 
>>   24dd05da8c79 ("x86/apic: Mark acpi_mp_wake_* variables as
>> __ro_after_init")
>> 
>> Which didn't consider 'maxvcpus=xx' case.
>> 
>
>Thanks a lot for checking this, Kai.
>
>> 
>> But I will leave to Kirill to confirm.
>
>Best Regards,
>Zhiquan

Then please extend the commit message with the "why", add the Fixes tag and resend.

Thx.
-- 
Sent from a small device: formatting sucks and brevity is inevitable.