On Sat, 2023-07-29 at 09:07 +0200, Thomas Gleixner wrote: > On Fri, Jul 28 2023 at 16:47, Rui Zhang wrote: > > On Fri, 2023-07-28 at 14:51 +0200, Thomas Gleixner wrote: > > > As the call sites during MADT parsing ignore the return value > > > anyway, > > > there is no harm and this is a proper defense against broken > > > tables > > > which enumerate an APIC twice. > > > > Yeah, this can fix the duplicate APIC ID issue. > > We want it independent of the below. > > > But for x2APIC CPUs with unique APIC ID, but smaller than 255, > > should > > we still enumerate them when we already have valid LAPIC entries? > > > > For the Ivebridge-EP 2-socket system, > > > > LAPIC: APIC ID from 0x0 - 0xB, 0x10 - 0x1B, 0x20 - 0x2B, 0x30 - > > 0x3B > > x2APIC: APIC ID from 0x0 - 0x77 > > > > # cpuid -1 -l 0xb -s 1 > > CPU: > > --- level 1 (core) --- > > bits to shift APIC ID to get next = 0x5 (5) > > logical processors at this level = 0x18 (24) > > level number = 0x1 (1) > > level type = core (2) > > extended APIC ID = 0 > > > > If we still enumerates all the x2APIC entries, > > 1. we got 72 extra possible CPUs from x2APIC > > 2. with the patch at > > https://lore.kernel.org/all/87edm36qqb.ffs@tglx/ ;, > > _max_logical_packages is set to 4 instead of 2. > > > > this is still a problem, right? > > Yes, you are right. > > But I still don't like the indirection of the returned CPU number. > It's > an ACPI selfcontained issue, no? > > So something like this should do the trick: > > + count = > acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_APIC, > + acpi_parse_lapic, > MAX_LOCAL_APIC); > + if (count) > + has_lapic_cpus = true; > + 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"); Agreed, thanks for the advice. Let me try to do this in v2 patch series. thanks, rui
