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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Regards, Roger.