Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Fri, 1 Dec 2023 12:23:04 -0800
From:   Ashok Raj <ashok.raj@intel.com>
To:     "Zhang, Rui" <rui.zhang@intel.com>
CC:     "ashok_raj@linux.intel.com" <ashok_raj@linux.intel.com>,
        "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>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tip-bot2@linutronix.de" <tip-bot2@linutronix.de>,
        Ashok Raj <ashok.raj@intel.com>
Subject: Re: [tip: x86/urgent] x86/acpi: Ignore invalid x2APIC entries
Message-ID: <ZWpAqAumIajbw4iK@a4bf019067fa.jf.intel.com>
References: <169953729188.3135.6804572126118798018.tip-bot2@tip-bot2>
 <20231122221947.781812-1-jsperbeck@google.com>
 <1e565bb08ebdd03897580a5905d1d2de01e15add.camel@intel.com>
 <ZWlSRE6KNNFwIYyq@araj-dh-work.jf.intel.com>
 <b561524f3378fb1d4c69451fdbf70bb5cbbb5ec9.camel@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <b561524f3378fb1d4c69451fdbf70bb5cbbb5ec9.camel@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?EnnPFJzk4i17FYvWKADUVXINApeUqmKqkCZMJwG84cHV0LIDY42+XjLt4e?=
 =?iso-8859-1?Q?RBUV/aCywm35BArJiis/wwsFR096TifiOSMYTz+wJihI+UzttLTGxBsX5f?=
 =?iso-8859-1?Q?SgUKpP0ZEShn/jVsQkR4YzjMJinpFI29oN2WELde+3VY+ivvlpisfsmwg+?=
 =?iso-8859-1?Q?c/FjRLmHhHXrliFQ9Pi87VBlNxy91k7wyQ7Ud4gp4ZvXv+h9iE7gnHi4VV?=
 =?iso-8859-1?Q?8GWhSRe5oa+I9akDbpzfZvUtnzJyZwmkUiArJZ20XTU9brSrZe6xAFViFk?=
 =?iso-8859-1?Q?Bi9eNk2xXr/vJMrjGskMmvY9ZLDq97MFRjnIGNsueoQegVHQ7AMpb3M6cR?=
 =?iso-8859-1?Q?6ZPZISz12LU70hJeDVADzFXmKVwha+R9hDAKrQSRH/eQG3HVjuEMcKcl8h?=
 =?iso-8859-1?Q?v8jLecnfZrdFFv8ii5BlZNq1V2Y850mspUcOMnIeNE8l4iHyNLew/kbIdU?=
 =?iso-8859-1?Q?+oNxpHuRtGHonRkZ6lHeGq1SmXAfKp5dNCJdEICDlWob7oHWS0Q1rxjr5d?=
 =?iso-8859-1?Q?awUqjsjxy5GfmcsZ+8cPWh/LaCEvwHC0BAHcPqOS7py1tf7JvBi4OhWmsa?=
 =?iso-8859-1?Q?dW1nSFcgsp9I15g/5X/gqcM/q3AhG7Mo37WRI7mz3tUq6/qdYepW2lwuPS?=
 =?iso-8859-1?Q?ImYDjSUv2oe8QOsPEJ3jEALZ12Yy1DEuhdPQnosnpo3nFpQKt/nLf2Je3m?=
 =?iso-8859-1?Q?YhztRrYWU7p4gFgQcdRIFjlAOhIYrAo4zHcsvBaDl44c1YUNFWJ75O6Mml?=
 =?iso-8859-1?Q?YQ6QSNsLl4P0Hcfaelvzwb7jiW9MT92n7jyejkduhe6X2mipx+Mo8Gsn6E?=
 =?iso-8859-1?Q?CC9hxw7KjGEAf4p6vUR1qgRnqQThZoSA0bkUNOj3e29QXCZrxSaTlMV7zI?=
 =?iso-8859-1?Q?ZaU3G6CwzGLqDxhFuBoHDKz7+5LCmM4oZv21ktpJfA0wfXDKSgK0qnknF/?=
 =?iso-8859-1?Q?KKZsEriN5f84/3RnFk00lgkS2tD/iNWdAT0oaCu2/hl9GdzW31JLBFEcBZ?=
 =?iso-8859-1?Q?6pOZ8m8piLkBS5fYzXJEzdED+qEVspaOhYqzlwKwEFuPZxgcc2+bWEP/Y+?=
 =?iso-8859-1?Q?KhMmDPHd4wxqgkUu3tjXNIa6sWL9rrrr4XZF2iyH2P+dTG2/z6k+d7WnwW?=
 =?iso-8859-1?Q?TKui6NbeO3bDVRMRS+wHp2LS0pPwP5vFNhwsqHXi4O4SiErcoKvWmKiNUh?=
 =?iso-8859-1?Q?0oG9Ce+9UXf3sFdv9j0jpf3pTzuVP9jv43EPBNdrngOayDQsjb0Hc5b83v?=
 =?iso-8859-1?Q?cRQ/96h2zbI3yk6iffNmofKWoRD2OcexgUhp5qNowTrlMtAwsWueao8MiE?=
 =?iso-8859-1?Q?TvpiQulVV/yJAYHAuXAJWWULyyPm2eofQ7mY8Rjb5C9MR5/gl+Hp8wsKte?=
 =?iso-8859-1?Q?o/7Z+NkpQeYiuFJ5KwKU3/6NLR8jefK+T21e/x+Y00rb7sqU0w2BuuwOAL?=
 =?iso-8859-1?Q?iqPuTwbhXJd8Ew3AbF14gktjSPXQ2vlcg2ymRyXLorXj5DURCX70Pk5z+Y?=
 =?iso-8859-1?Q?PDClemMDq6WoKLn7YBxlLIWhvQyWyrh0kTF0jnro86Dn3OhABe6pbdkmOc?=
 =?iso-8859-1?Q?YhX27O8SFp+gdpeqPmJJKOsCNYIpUI1UL9hkugmH09XIVQDfQ6jSBYWAZP?=
 =?iso-8859-1?Q?tlb05bkJCaNxXTgrzhlB98++txSa/Q08ze?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bb960f4f-3f09-47cd-700a-08dbf2ab5493
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Dec 2023 20:23:07.7004
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: M4680rxTQxBmAhEWofNt7NKmGGpWbPhhXd/7L0s9fvDpgVBs3IvnCt47uq9K+JPQoI+gQ+Vp3o36Wg/tyTaSzA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB7980
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Fri, 01 Dec 2023 12:23:17 -0800 (PST)

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?