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;
>  	}
>  }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.