[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <613df280116378115585d0c483f7e186cffaeb58.camel@intel.com>
Date: Mon, 31 Jul 2023 13:04:00 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "tglx@...utronix.de" <tglx@...utronix.de>,
"peterz@...radead.org" <peterz@...radead.org>,
"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
"bp@...en8.de" <bp@...en8.de>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Tang, Feng" <feng.tang@...el.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"x86@...nel.org" <x86@...nel.org>
Subject: Re: [RFC PATCH] x86/acpi: Ignore invalid x2APIC entries
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
Powered by blists - more mailing lists