Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     "Zhang, Rui" <rui.zhang@intel.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
        "bp@alien8.de" <bp@alien8.de>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Tang, Feng" <feng.tang@intel.com>,
        "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>
Subject: Re: [RFC PATCH] x86/acpi: Ignore invalid x2APIC entries
In-Reply-To: <84216c743c6368691bc3fae924c6cbd33805ca9b.camel@intel.com>
References: <20230702162802.344176-1-rui.zhang@intel.com>
 <87jzukqjvf.ffs@tglx>
 <84216c743c6368691bc3fae924c6cbd33805ca9b.camel@intel.com>
Date:   Sat, 29 Jul 2023 09:07:40 +0200
Message-ID: <87pm4bp54z.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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 =3D 0x5 (5)
>       logical processors at this level  =3D 0x18 (24)
>       level number                      =3D 0x1 (1)
>       level type                        =3D core (2)
>       extended APIC ID                  =3D 0
>
> If we still enumerates all the x2APIC entries,
> 1. we got 72 extra possible CPUs from x2APIC
> 2.=C2=A0with 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 =3D acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_APIC,
+					acpi_parse_lapic, MAX_LOCAL_APIC);
+		if (count)
+			has_lapic_cpus =3D true;
+		x2count =3D 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");