On 07/08/2023 11:01 am, Andrew Cooper wrote: > 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 <[email protected]> >> --- >> 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.
Oh, and I forgot to say. I'm happy to fix all of this up on commit if you're happy. ~Andrew
