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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.08.2023 11:38, 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.

I'm afraid it doesn't become clear to me what problem you're trying to
solve.

> --- 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 */
> +     if (processor->local_apic_id == 0xffffffff)
> +             return 0;
> +
>       /* Don't register processors that cannot be onlined. */
>       if (madt_revision >= 5 &&
>           !(processor->lapic_flags & ACPI_MADT_ENABLED) &&
>           !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
>               return 0;
>  
> -     if ((processor->lapic_flags & ACPI_MADT_ENABLED) ||
> -         processor->local_apic_id != 0xffffffff || opt_cpu_info) {
> +     if ((processor->lapic_flags & ACPI_MADT_ENABLED) || opt_cpu_info) {
>               acpi_table_print_madt_entry(header);
>               log = true;
>       }

In particular you're now suppressing log messages which may be relevant.

The one issue that I'm aware of (and that I use a local hack to deal
with; see bottom) is excess verbosity.

Jan

--- unstable.orig/xen/arch/x86/mpparse.c
+++ unstable/xen/arch/x86/mpparse.c
@@ -809,8 +809,13 @@ int mp_register_lapic(u32 id, bool enabl
        };
        
        if (MAX_APICS <= id) {
-               printk(KERN_WARNING "Processor #%d invalid (max %d)\n",
-                       id, MAX_APICS);
+               static u32 max_warn = -1;
+
+               if (id <= max_warn) {
+                       printk(KERN_WARNING "Processor #%d invalid (max %d)\n",
+                              id, MAX_APICS);
+                       max_warn = id - 1;
+               }
                return -EINVAL;
        }