On 07/08/2023 3:45 pm, Simon Gaiser wrote: > Andrew Cooper: >> 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. > So I noticed that the check in Linux is actually slightly different than > I thought. Since [3] it always considers the CPU usable if > ACPI_MADT_ENABLED is set. Otherwise it consider it only usable if > MADT revision >= 5 and ACPI_MADT_ONLINE_CAPABLE is set. > > So I checked what the ACPI spec says: > > Up to 6.2 Errata B [6] it only defines ACPI_MADT_ENABLE as: > > If zero, this processor is unusable, and the operating system > support will not attempt to use it. > > And the bit that later will be ACPI_MADT_ONLINE_CAPABLE is reserved with > "Must be zero". > > 6.3 [7] 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 confirming firmwares it should be safe change the simply ignore > the entry if !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE > > We can also do it like Linux and ignore ACPI_MADT_ONLINE_CAPABLE > completely if revision < 5. > > Note that the revision was already increased to 5 before 6.3. > > ACPI spec version MADT revision > > 6.2 [4] 4 > 6.2 Errata A [5] 45 (typo I guess) > 6.2 Errata B 5 > 6.3 5 > > [3]: > https://git.kernel.org/torvalds/c/e2869bd7af608c343988429ceb1c2fe99644a01f > [4]: http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf > [5]: http://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf > [6]: https://uefi.org/sites/default/files/resources/ACPI_6_2_B_final_Jan30.pdf > [7]: https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf
Honestly, the reserved must be zero means there's no need for a version check at all. That bit will not be set even in older MADT revisions. That said, it's likely easier to retain the version check than to set up a quirks infrastructure for buggy older MADTs. ~Andrew
