Re: [PATCH] x86/topo: Map vendor CPU types to generic Linux such types
Pawan Gupta <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.acpi.devel |
|---|---|
| Message-ID | <6ff5asua7jywnn7ge4k37bu22artrkfgqybfdt747cm5ieiybo@bwkdi2bqlsm6> |
On Mon, Jul 20, 2026 at 08:18:58PM -0700, Borislav Petkov wrote: > On Wed, Jul 08, 2026 at 01:24:05AM +0200, Thomas Gleixner wrote: > > If that's the only case to handle, then sure that open coded check is > > fine. > > This is ontop of tip:x86/cpu which already has Pawan's change that drops the > one Intel-specific CPU type use. > > It boots here so... :-P > > --- > From: "Borislav Petkov (AMD)" <[email protected]> > Date: Thu, 2 Jul 2026 17:00:28 -0700 > Subject: [PATCH] x86/topo: Map vendor CPU types to generic Linux such types > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Sashiko reported¹ that a confusion could ensue if a vendor-specific CPU > type 0 (performance) gets attempted to be used in a x86_cpu_id match > table. The current logic treats 0 as the wildcard X86_CPU_TYPE_ANY and > such a thing would end up matching the wrong CPUs. > > This is all backwards because we started using the vendor-specific CPU > type number instead of using a Linux-defined, generic CPU type which > is agnostic. > > Convert the current logic to it before users start appearing. > > ¹https://sashiko.dev/#/patchset/20260629094349.533301-1-Vishal.Badole%40amd.com > > Signed-off-by: Borislav Petkov (AMD) <[email protected]> > Link: https://lore.kernel.org/r/[email protected] > --- > arch/x86/include/asm/processor.h | 18 ++++++++++++---- > arch/x86/include/asm/topology.h | 7 ------- > arch/x86/kernel/acpi/cppc.c | 4 ++-- > arch/x86/kernel/cpu/match.c | 30 +-------------------------- > arch/x86/kernel/cpu/topology.h | 1 + > arch/x86/kernel/cpu/topology_amd.c | 6 ++++-- > arch/x86/kernel/cpu/topology_common.c | 8 +++++-- > 7 files changed, 28 insertions(+), 46 deletions(-) > > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index 87b1d4c0727e..fc0350fe8f5e 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -68,9 +68,13 @@ extern u16 __read_mostly tlb_lld_2m; > extern u16 __read_mostly tlb_lld_4m; > extern u16 __read_mostly tlb_lld_1g; > > -/* > - * CPU type and hardware bug flags. Kept separately for each CPU. > - */ > +enum x86_topology_cpu_type { > + /* X86_CPU_TYPE_ANY */ > + TOPO_CPU_TYPE_ANY = 0, I don't really understand the purpose of TOPO_CPU_TYPE_ANY here. In the cpu matching infrastructure, X86_CPU_TYPE_ANY makes sense. Here I fail to understand how is TOPO_CPU_TYPE_ANY different from UNKNOWN? A table passed to x86_match_cpu() with X86_CPU_TYPE_ANY entry will match any CPU type including TOPO_CPU_TYPE_UNKNOWN. > + TOPO_CPU_TYPE_PERFORMANCE, > + TOPO_CPU_TYPE_EFFICIENCY, > + TOPO_CPU_TYPE_UNKNOWN, > +}; > > struct cpuinfo_topology { > // Real APIC ID read from the local APIC > @@ -104,7 +108,7 @@ struct cpuinfo_topology { > > // Hardware defined CPU-type > union { > - u32 cpu_type; > + u32 hw_cpu_type; > struct { > // CPUID.1A.EAX[23-0] > u32 intel_native_model_id :24; > @@ -119,8 +123,14 @@ struct cpuinfo_topology { > amd_type :4; > }; > }; > + > + // Linux vendor-agnostic CPU type > + enum x86_topology_cpu_type cpu_type; > }; > > +/* > + * CPU type and hardware bug flags. Kept separately for each CPU. > + */ > struct cpuinfo_x86 { > union { > /* > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h > index 8fb61d2465eb..ef76ba674f1b 100644 > --- a/arch/x86/include/asm/topology.h > +++ b/arch/x86/include/asm/topology.h > @@ -114,12 +114,6 @@ enum x86_topology_domains { > TOPO_MAX_DOMAIN, > }; > > -enum x86_topology_cpu_type { > - TOPO_CPU_TYPE_PERFORMANCE, > - TOPO_CPU_TYPE_EFFICIENCY, > - TOPO_CPU_TYPE_UNKNOWN, > -}; ... > /** > * x86_match_cpu - match current CPU against an array of x86_cpu_ids > * @match: Pointer to array of x86_cpu_ids. Last entry terminated with > @@ -81,7 +53,7 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match) > continue; > if (m->feature != X86_FEATURE_ANY && !cpu_has(c, m->feature)) > continue; > - if (!x86_match_vendor_cpu_type(c, m)) > + if (m->type != X86_CPU_TYPE_ANY && c->topo.cpu_type != m->type) In line with what Thomas pointed that cpu_type is not a system property, I suggest matching cpu_type should be dropped from x86_match_cpu(). More so because it only uses boot_cpu_data and misses the other types of secondary CPUs. I feel it is not a reliable way to match CPU types. Thoughts? > continue; > return m; > } > diff --git a/arch/x86/kernel/cpu/topology.h b/arch/x86/kernel/cpu/topology.h > index 37326297f80c..74e02bacd854 100644 > --- a/arch/x86/kernel/cpu/topology.h > +++ b/arch/x86/kernel/cpu/topology.h > @@ -22,6 +22,7 @@ void topology_set_dom(struct topo_scan *tscan, enum x86_topology_domains dom, > bool cpu_parse_topology_ext(struct topo_scan *tscan); > void cpu_parse_topology_amd(struct topo_scan *tscan); > void cpu_topology_fixup_amd(struct topo_scan *tscan); > +enum x86_topology_cpu_type get_topology_cpu_type(struct cpuinfo_x86 *c); > > static inline u32 topo_shift_apicid(u32 apicid, enum x86_topology_domains dom) > { > diff --git a/arch/x86/kernel/cpu/topology_amd.c b/arch/x86/kernel/cpu/topology_amd.c > index da080d732e10..c5a6944df86a 100644 > --- a/arch/x86/kernel/cpu/topology_amd.c > +++ b/arch/x86/kernel/cpu/topology_amd.c > @@ -177,8 +177,10 @@ static void topoext_fixup(struct topo_scan *tscan) > > static void parse_topology_amd(struct topo_scan *tscan) > { > - if (cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES)) > - tscan->c->topo.cpu_type = cpuid_ebx(0x80000026); > + if (cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES)) { Slightly unrelated to this patch, on AMD is it that hw_cpu_type only available on hybrid systems? On Intel ... > + tscan->c->topo.hw_cpu_type = cpuid_ebx(0x80000026); > + tscan->c->topo.cpu_type = get_topology_cpu_type(tscan->c); > + } > > /* > * Try to get SMT, CORE, TILE, and DIE shifts from extended > diff --git a/arch/x86/kernel/cpu/topology_common.c b/arch/x86/kernel/cpu/topology_common.c > index cf7513416b70..b9d025f3373a 100644 > --- a/arch/x86/kernel/cpu/topology_common.c > +++ b/arch/x86/kernel/cpu/topology_common.c > @@ -168,8 +168,12 @@ static void parse_topology(struct topo_scan *tscan, bool early) > case X86_VENDOR_INTEL: > if (!IS_ENABLED(CONFIG_CPU_SUP_INTEL) || !cpu_parse_topology_ext(tscan)) > parse_legacy(tscan); > - if (c->cpuid_level >= 0x1a) > - c->topo.cpu_type = cpuid_eax(0x1a); > + > + if (c->cpuid_level >= 0x1a) { > + c->topo.hw_cpu_type = cpuid_eax(0x1a); > + c->topo.cpu_type = get_topology_cpu_type(c); ... hw_cpu_type could be enumerated on non-hybrid parts. > + } > + > break; > } > }