On Fri, 2023-12-08 at 05:06 -0500, Sasha Levin wrote: > This is a note to let you know that I've just added the patch titled > > x86/acpi: Ignore invalid x2APIC entries > > to the 6.1-stable tree which can be found at: > > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > The filename of the patch is: > x86-acpi-ignore-invalid-x2apic-entries.patch > and it can be found in the queue-6.1 subdirectory. > > If you, or anyone else, feels it should not be added to the stable > tree, > please let <stable@xxxxxxxxxxxxxxx> know about it. > > Hi, This patch causes regressions on two platforms, and a fix has been posted at https://lore.kernel.org/all/20231210143925.38722-1-rui.zhang@xxxxxxxxx/ And I agree with Andres that this should not be a -stable material. https://lore.kernel.org/all/20231206070423.wp7cxxnwfe3lidm3@xxxxxxxxxxxxxxxxxx/ thanks, rui > > commit 4c7717939f49024466540344aa3463a934461ec2 > Author: Zhang Rui <rui.zhang@xxxxxxxxx> > Date: Mon Jul 3 00:28:02 2023 +0800 > > x86/acpi: Ignore invalid x2APIC entries > > [ Upstream commit ec9aedb2aa1ab7ac420c00b31f5edc5be15ec167 ] > > Currently, the kernel enumerates the possible CPUs by parsing > both ACPI > MADT Local APIC entries and x2APIC entries. So CPUs with "valid" > APIC IDs, > even if they have duplicated APIC IDs in Local APIC and x2APIC, > are always > enumerated. > > Below is what ACPI MADT Local APIC and x2APIC describes on an > Ivebridge-EP system, > > [02Ch 0044 1] Subtable Type : 00 [Processor > Local APIC] > [02Fh 0047 1] Local Apic ID : 00 > ... > [164h 0356 1] Subtable Type : 00 [Processor > Local APIC] > [167h 0359 1] Local Apic ID : 39 > [16Ch 0364 1] Subtable Type : 00 [Processor > Local APIC] > [16Fh 0367 1] Local Apic ID : FF > ... > [3ECh 1004 1] Subtable Type : 09 [Processor > Local x2APIC] > [3F0h 1008 4] Processor x2Apic ID : 00000000 > ... > [B5Ch 2908 1] Subtable Type : 09 [Processor > Local x2APIC] > [B60h 2912 4] Processor x2Apic ID : 00000077 > > As a result, kernel shows "smpboot: Allowing 168 CPUs, 120 > hotplug CPUs". > And this wastes significant amount of memory for the per-cpu > data. > Plus this also breaks > https://lore.kernel.org/all/87edm36qqb.ffs@tglx/, > because __max_logical_packages is over-estimated by the APIC IDs > in > the x2APIC entries. > > According to > https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#processor-local-x2apic-structure > : > > "[Compatibility note] On some legacy OSes, Logical processors > with APIC > ID values less than 255 (whether in XAPIC or X2APIC mode) must > use the > Processor Local APIC structure to convey their APIC > information to OSPM, > and those processors must be declared in the DSDT using the > Processor() > keyword. Logical processors with APIC ID values 255 and > greater must use > the Processor Local x2APIC structure and be declared using the > Device() > keyword." > > Therefore prevent the registration of x2APIC entries with an APIC > ID less > than 255 if the local APIC table enumerates valid APIC IDs. > > [ tglx: Simplify the logic ] > > Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx> > Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Tested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Link: > https://lore.kernel.org/r/20230702162802.344176-1-rui.zhang@xxxxxxxxx > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> > > diff --git a/arch/x86/kernel/acpi/boot.c > b/arch/x86/kernel/acpi/boot.c > index 2252340b2133e..14af7fbdc6b5e 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -62,6 +62,7 @@ int acpi_fix_pin2_polarity __initdata; > > #ifdef CONFIG_X86_LOCAL_APIC > static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE; > +static bool has_lapic_cpus __initdata; > static bool acpi_support_online_capable; > #endif > > @@ -235,6 +236,14 @@ acpi_parse_x2apic(union acpi_subtable_headers > *header, const unsigned long end) > if (!acpi_is_processor_usable(processor->lapic_flags)) > return 0; > > + /* > + * According to > https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#processor-local-x2apic-structure > + * when MADT provides both valid LAPIC and x2APIC entries, > the APIC ID > + * in x2APIC must be equal or greater than 0xff. > + */ > + if (has_lapic_cpus && apic_id < 0xff) > + return 0; > + > /* > * We need to register disabled CPU as well to permit > * counting disabled CPUs. This allows us to size > @@ -1114,10 +1123,7 @@ static int __init > early_acpi_parse_madt_lapic_addr_ovr(void) > > static int __init acpi_parse_madt_lapic_entries(void) > { > - int count; > - int x2count = 0; > - int ret; > - struct acpi_subtable_proc madt_proc[2]; > + int count, x2count = 0; > > if (!boot_cpu_has(X86_FEATURE_APIC)) > return -ENODEV; > @@ -1126,21 +1132,11 @@ static int __init > acpi_parse_madt_lapic_entries(void) > acpi_parse_sapic, > MAX_LOCAL_APIC); > > if (!count) { > - memset(madt_proc, 0, sizeof(madt_proc)); > - madt_proc[0].id = ACPI_MADT_TYPE_LOCAL_APIC; > - madt_proc[0].handler = acpi_parse_lapic; > - madt_proc[1].id = ACPI_MADT_TYPE_LOCAL_X2APIC; > - madt_proc[1].handler = acpi_parse_x2apic; > - ret = acpi_table_parse_entries_array(ACPI_SIG_MADT, > - sizeof(struct acpi_table_madt), > - madt_proc, ARRAY_SIZE(madt_proc), > MAX_LOCAL_APIC); > - if (ret < 0) { > - pr_err("Error parsing LAPIC/X2APIC > entries\n"); > - return ret; > - } > - > - count = madt_proc[0].count; > - x2count = madt_proc[1].count; > + count = > acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_APIC, > + acpi_parse_lapic, > MAX_LOCAL_APIC); > + has_lapic_cpus = count > 0; > + x2count = > acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_X2APIC, > + acpi_parse_x2apic, > MAX_LOCAL_APIC); > } > if (!count && !x2count) { > pr_err("No LAPIC entries present\n");
