Re: [XEN PATCH] x86/ACPI: Ignore entries marked as unusable when parsing MADT

[Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>]

Roger Pau Monné:
> On Mon, Sep 11, 2023 at 12:12:38PM +0200, Simon Gaiser wrote:
>> Up to version 6.2 Errata B [2] the ACPI spec only defines
>> ACPI_MADT_ENABLE as:
>>
>>     If zero, this processor is unusable, and the operating system
>>     support will not attempt to use it.
>>
>> The bit that later will be ACPI_MADT_ONLINE_CAPABLE is reserved with
>> "Must be zero".
>>
>> Version 6.3 [3] then adds ACPI_MADT_ONLINE_CAPABLE and changes the
>> meaning of ACPI_MADT_ENABLE:
>>
>>     Enabled
>>         If this bit is set the processor is ready for use. If this bit
>>         is clear and the Online Capable bit is set, system hardware
>>         supports enabling this processor during OS runtime. If this bit
>>         is clear and the Online Capable bit is also clear, this
>>         processor is unusable, and OSPM shall ignore the contents of the
>>         Processor Local APIC Structure.
>>
>>     Online Capbable
>>         The information conveyed by this bit depends on the value of the
>>         Enabled bit. If the Enabled bit is set, this bit is reserved and
>>         must be zero. Otherwise, if this this bit is set, system
>>         hardware supports enabling this processor during OS runtime.
>>
>> So with conforming firmwares it should be safe to simply ignore the
>> entry if !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE
>>
>> As a precaution against buggy firmwares this change, like Linux [4],
>> ignores ACPI_MADT_ONLINE_CAPABLE completely if MADT revision < 5. Note
>> that the MADT revision was already increased to 5 with spec version 6.2
>> Errata A [1], so before introducing the online capable flag. But it
>> wasn't changed for the new flag, so this is the best we can do here.
>>
>> For previous discussion see thread [5].
>>
>> Link: 
>> http://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf # 
>> [1]
>> Link: 
>> https://uefi.org/sites/default/files/resources/ACPI_6_2_B_final_Jan30.pdf # 
>> [2]
>> Link: https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf # [3]
>> Link: 
>> https://git.kernel.org/torvalds/c/e2869bd7af608c343988429ceb1c2fe99644a01f # 
>> [4]
>> Link: 
>> https://lore.kernel.org/xen-devel/80bae614-052e-0f90-cf13-0e5e4ed1a5cd@xxxxxxxxxxxxxxxxxxxxxx/
>>  # [5]
>> Signed-off-by: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
>> ---
>>  xen/arch/x86/acpi/boot.c | 19 +++++++++++++------
>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
>> index 4a62822fa9..2d0b8a9afc 100644
>> --- a/xen/arch/x86/acpi/boot.c
>> +++ b/xen/arch/x86/acpi/boot.c
>> @@ -77,6 +77,17 @@ static int __init cf_check acpi_parse_madt(struct 
>> acpi_table_header *table)
>>      return 0;
>>  }
>>  
>> +static bool __init acpi_is_processor_usable(uint32_t lapic_flags)
>> +{
>> +    if (lapic_flags & ACPI_MADT_ENABLED)
>> +            return true;
>> +
>> +    if (madt_revision >= 5 && (lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
>> +            return true;
>> +
>> +    return false;
> 
> So this means that Xen would only support ACPI CPU Hotplug with
> versions of the MADT >= 5?  Because with the proposed code non enabled
> entries on MADT versions < 5 will be reported as unusable.
> 
> Will this work with QEMU?  (ie: does QEMU expose a MADT table with
> version >= 5)  Otherwise we will loose all possible ways of testing
> ACPI CPU Hotplug.

That's a good question. I just had a look and it looks like QEMU doesn't
use the new revision (and the new flag) (yet?). (But QEMU assigns a
valid APIC ID from the start.)

So why does hotplugging works in Linux then? Turns out that I missed a
later change:

https://git.kernel.org/torvalds/c/fed8d8773b8ea68ad99d9eee8c8343bef9da2c2c

So they went back to (mostly) the old logic. So then the above change
isn't a good idea. It's only "mostly" the old logic because in the
meantime Linux also improved the version check, to accurately match the
version that introduced the new flag:

https://git.kernel.org/torvalds/c/a74fabfbd1b7013045afc8cc541e6cab3360ccb5

Simon

Attachment: OpenPGP_signature
Description: OpenPGP digital signature