linux-kernel - Re: [tip: x86/urgent] x86/acpi: Ignore invalid x2APIC entries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZWpAqAumIajbw4iK@a4bf019067fa.jf.intel.com>
Date:   Fri, 1 Dec 2023 12:23:04 -0800
From:   Ashok Raj <ashok.raj@...el.com>
To:     "Zhang, Rui" <rui.zhang@...el.com>
CC:     "ashok_raj@...ux.intel.com" <ashok_raj@...ux.intel.com>,
        "linux-tip-commits@...r.kernel.org" 
        <linux-tip-commits@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "jsperbeck@...gle.com" <jsperbeck@...gle.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tip-bot2@...utronix.de" <tip-bot2@...utronix.de>,
        Ashok Raj <ashok.raj@...el.com>
Subject: Re: [tip: x86/urgent] x86/acpi: Ignore invalid x2APIC entries

On Fri, Dec 01, 2023 at 10:08:55AM -0800, Zhang, Rui wrote:
> On Thu, 2023-11-30 at 19:25 -0800, Ashok Raj wrote:
> > On Thu, Nov 23, 2023 at 12:50:47PM +0000, Zhang Rui wrote:
> > > Hi, John,
> > > 
> > > Thanks for catching this issue.
> > > 
> > > On Wed, 2023-11-22 at 22:19 +0000, John Sperbeck wrote:
> > > > I have a platform with both LOCAL_APIC and LOCAL_X2APIC entries
> > > > for
> > > > each CPU.  However, the ids for the LOCAL_APIC entries are all
> > > > invalid ids of 255, so they have always been skipped in
> > > > acpi_parse_lapic()
> > > > by this code from f3bf1dbe64b6 ("x86/acpi: Prevent LAPIC id 0xff
> > > > from
> > > > being
> > > > accounted"):
> > > > 
> > > >     /* Ignore invalid ID */
> > > >     if (processor->id == 0xff)
> > > >             return 0;
> > > > 
> > > > With the change in this thread, the return value of 0 means that
> > > > the
> > > > 'count' variable in acpi_parse_entries_array() is incremented. 
> > > > The
> > > > positive return value means that 'has_lapic_cpus' is set, even
> > > > though
> > > > no entries were actually matched.
> > > 
> > > So in acpi_parse_madt_lapic_entries, without this patch,
> > > madt_proc[0].count is a positive value on this platform, right?
> > > 
> > > This sounds like a potential issue because the following checks to
> > > fall
> > > back to MPS mode can also break. (If all LOCAL_APIC entries have
> > > apic_id 0xff and all LOCAL_X2APIC entries have apic_id 0xffffffff)
> > > 
> > > >   Then, when the MADT is iterated
> > > > with acpi_parse_x2apic(), the x2apic entries with ids less than
> > > > 255
> > > > are skipped and most of my CPUs aren't recognized.
> > 
> > This smells wrong. If a BIOS is placing some in lapic and some in
> > x2apic
> > table, its really messed up. 
> > 
> > Shouldn't the kernel scan them in some priority and only consider one
> > set of
> > tables?
> > 
> > Shouldn't the code stop looking once something once a type is found?
> > 
> 
> I also want to get this clarified but there is no spec saying this. And
> instead, as mentioned in the comment, we do have something in
> 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."
> 
> so it is possible to enumerate CPUs from both LAPIC and X2APIC.
> 

Ah, so this looks like the legacy case, old OS can atleast boot the APIC
entries and not process the x2apic ones. 

So you can potentially have duplicates

APIC = has all APIC id's < 255
X2apic has all entries > 255 OR 
	It can contain everything, so you might need to weed out
	duplicates?