From: "Zhang, Rui" <rui.zhang@intel.com>
To: "Raj, Ashok" <ashok.raj@intel.com>
Cc: "linux-tip-commits@vger.kernel.org"
<linux-tip-commits@vger.kernel.org>,
"x86@kernel.org" <x86@kernel.org>,
"peterz@infradead.org" <peterz@infradead.org>,
"jsperbeck@google.com" <jsperbeck@google.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"ashok_raj@linux.intel.com" <ashok_raj@linux.intel.com>,
"tip-bot2@linutronix.de" <tip-bot2@linutronix.de>
Subject: Re: [tip: x86/urgent] x86/acpi: Ignore invalid x2APIC entries
Date: Sat, 2 Dec 2023 02:53:36 +0000 [thread overview]
Message-ID: <b5070898c96232f83ba738f6516cfc8102cb7ca8.camel@intel.com> (raw)
In-Reply-To: <ZWpAqAumIajbw4iK@a4bf019067fa.jf.intel.com>
On Fri, 2023-12-01 at 12:23 -0800, Ashok Raj wrote:
> 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?
>
That is what this patch tries to do.
Say, if we have valid CPUs in LAPIC, probe X2APIC CPUs with ID >= 255
only.
thanks,
rui
next prev parent reply other threads:[~2023-12-02 2:53 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-02 16:28 [RFC PATCH] x86/acpi: Ignore invalid x2APIC entries Zhang Rui
2023-07-28 12:51 ` Thomas Gleixner
2023-07-28 12:55 ` Thomas Gleixner
2023-07-28 16:47 ` Zhang, Rui
2023-07-29 7:07 ` Thomas Gleixner
2023-07-31 13:04 ` Zhang, Rui
2023-11-09 13:41 ` [tip: x86/urgent] " tip-bot2 for Zhang Rui
2023-11-22 22:17 ` [PATCH] platform/x86: intel_telemetry: Fix kernel doc descriptions John Sperbeck
2023-11-22 22:18 ` [tip: x86/urgent] x86/acpi: Ignore invalid x2APIC entries John Sperbeck
2023-11-22 22:19 ` John Sperbeck
2023-11-23 12:50 ` Zhang, Rui
2023-12-01 3:25 ` Ashok Raj
2023-12-01 18:08 ` Zhang, Rui
2023-12-01 20:23 ` Ashok Raj
2023-12-02 2:53 ` Zhang, Rui [this message]
2023-12-01 8:31 ` Zhang, Rui
2023-12-01 23:32 ` John Sperbeck
2023-12-06 6:58 ` Andres Freund
2023-12-07 2:41 ` Zhang, Rui
2023-12-07 5:10 ` Andres Freund
2023-12-12 17:34 ` Thomas Gleixner
2023-12-13 7:39 ` Zhang, Rui
2023-12-13 14:51 ` Thomas Gleixner
2023-12-14 15:00 ` Zhang, Rui
2023-12-14 21:11 ` Thomas Gleixner
2023-12-15 14:19 ` [PATCH] x86/acpi: Handle bogus MADT APIC tables gracefully Thomas Gleixner
2023-12-15 14:20 ` [tip: x86/urgent] x86/acpi: Ignore invalid x2APIC entries Andres Freund
2023-12-10 11:47 ` Linux regression tracking (Thorsten Leemhuis)
2023-12-18 13:57 ` [tip: x86/urgent] x86/acpi: Handle bogus MADT APIC tables gracefully tip-bot2 for Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b5070898c96232f83ba738f6516cfc8102cb7ca8.camel@intel.com \
--to=rui.zhang@intel.com \
--cc=ashok.raj@intel.com \
--cc=ashok_raj@linux.intel.com \
--cc=jsperbeck@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=tip-bot2@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).