Re: [XEN PATCH] x86/ACPI: Ignore entries with invalid APIC IDs when parsing MADT

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 07/08/2023 10:38 am, Simon Gaiser wrote:
> It seems some firmwares put dummy entries in the ACPI MADT table for non
> existing processors. On my NUC11TNHi5 those have the invalid APIC ID
> 0xff. Linux already has code to handle those cases both in
> acpi_parse_lapic [1] as well as in acpi_parse_x2apic [2]. So add the
> same check to Xen.
>
> Note that on some older (2nd gen Core i) laptop of mine I also saw dummy
> entries with a valid APIC ID. Linux would still ignore those because
> they have !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE. But in Xen
> this check is only active for madt_revision >= 5. But since this version
> check seems to be intentionally I leave that alone.

I recall there being a discussion over this, ultimately with the version
check being removed.  IIRC it was a defacto standard used for a long
time prior to actually getting written into the ACPI spec, so does exist
in practice in older MADTs.

Otherwise LGTM.  I'd suggest dropping this paragraph as it's not related
to the change.  It will also help if we do decide to follow up and drop
the MADT version check.

>
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3bf1dbe64b62a2058dd1944c00990df203e8e7a # [1]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=10daf10ab154e31237a8c07242be3063fb6a9bf4 # [2]

https://git.kernel.org/torvalds/c/$SHA

Somewhat less verbose. https://korg.docs.kernel.org/git-url-shorteners.html

> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
> ---
>  xen/arch/x86/acpi/boot.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
> index 54b72d716b..4a62822fa9 100644
> --- a/xen/arch/x86/acpi/boot.c
> +++ b/xen/arch/x86/acpi/boot.c
> @@ -87,14 +87,17 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
>  	if (BAD_MADT_ENTRY(processor, end))
>  		return -EINVAL;
>  
> +	/* Ignore entries with invalid apicid */

x2apic ID.

~Andrew