Re: [PATCH v4 02/10] x86/cpu/topology: Add CPU type to struct cpuinfo_topology

[Mario Limonciello]

On 9/30/2024 09:47, Pawan Gupta wrote:
> Sometimes it is required to take actions based on if a CPU is a performance
> or efficiency core. As an example, intel_pstate driver uses the Intel
> core-type to determine CPU scaling. Also, some CPU vulnerabilities only
> affect a specific CPU type, like RFDS only affects Intel Atom. Hybrid
> systems that have variants P+E, P-only(Core) and E-only(Atom), it is not
> straightforward to identify which variant is affected by a type specific
> vulnerability.
>
> Such processors do have CPUID field that can uniquely identify them. Like,
> P+E, P-only and E-only enumerates CPUID.1A.CORE_TYPE identification, while
> P+E additionally enumerates CPUID.7.HYBRID. Based on this information, it
> is possible for boot CPU to identify if a system has mixed CPU types.
>
> Add a new field hw_cpu_type to struct cpuinfo_topology that stores the
> hardware specific CPU type. This saves the overhead of IPIs to get the CPU
> type of a different CPU. CPU type is populated early in the boot process,
> before vulnerabilities are enumerated.
>
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>
> ---
>   arch/x86/include/asm/cpu.h            |  6 ++++++
>   arch/x86/include/asm/processor.h      | 11 +++++++++++
>   arch/x86/include/asm/topology.h       |  8 ++++++++
>   arch/x86/kernel/cpu/debugfs.c         |  1 +
>   arch/x86/kernel/cpu/intel.c           |  5 +++++
>   arch/x86/kernel/cpu/topology_common.c | 11 +++++++++++
>   6 files changed, 42 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index aa30fd8cad7f..2244dd86066a 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -32,6 +32,7 @@ extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
>   extern bool handle_guest_split_lock(unsigned long ip);
>   extern void handle_bus_lock(struct pt_regs *regs);
>   u8 get_this_hybrid_cpu_type(void);
> +u32 intel_native_model_id(struct cpuinfo_x86 *c);
>   #else
>   static inline void __init sld_setup(struct cpuinfo_x86 *c) {}
>   static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code)
> @@ -50,6 +51,11 @@ static inline u8 get_this_hybrid_cpu_type(void)
>   {
>   	return 0;
>   }
> +
> +static u32 intel_native_model_id(struct cpuinfo_x86 *c)
> +{
> +	return 0;
> +}
>   #endif
>   #ifdef CONFIG_IA32_FEAT_CTL
>   void init_ia32_feat_ctl(struct cpuinfo_x86 *c);
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 4a686f0e5dbf..61c8336bc99b 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -105,6 +105,17 @@ struct cpuinfo_topology {
>   	// Cache level topology IDs
>   	u32			llc_id;
>   	u32			l2c_id;
> +
> +	// Hardware defined CPU-type
> +	union {
> +		u32		hw_cpu_type;
> +		struct {
> +			/* CPUID.1A.EAX[23-0] */
> +			u32	intel_core_native_model_id:24;
> +			/* CPUID.1A.EAX[31-24] */
> +			u32	intel_core_type:8;
> +		};
> +	};
>   };
>   
>   struct cpuinfo_x86 {
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index aef70336d624..faf7cb7f7d7e 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -114,6 +114,12 @@ enum x86_topology_domains {
>   	TOPO_MAX_DOMAIN,
>   };
>   
> +enum x86_topology_hw_cpu_type {
> +	TOPO_HW_CPU_TYPE_UNKNOWN	= 0,
> +	TOPO_HW_CPU_TYPE_INTEL_ATOM	= 0x20,
> +	TOPO_HW_CPU_TYPE_INTEL_CORE	= 0x40,
> +};

This isn't exactly generic.  Unless you have a strong need to know 
"Atom" instead of "Efficient" or "Core" instead of "Performance" I think 
it would be better to do this as:

enum x86_topology_hw_core_type {
	TOPO_HW_CORE_TYPE_UNKNOWN	= 0,
	TOPO_HW_CORE_TYPE_PERFORMANT,
	TOPO_HW_CORE_TYPE_EFFICIENT,
};

Then you can do the mapping of 0x20 = Efficient and 0x40 = performant in 
the Intel topology lookup function.

After you land the series we can do something similar to move AMD code 
around and map it out to the right generic mapping.

> +
>   struct x86_topology_system {
>   	unsigned int	dom_shifts[TOPO_MAX_DOMAIN];
>   	unsigned int	dom_size[TOPO_MAX_DOMAIN];
> @@ -149,6 +155,8 @@ extern unsigned int __max_threads_per_core;
>   extern unsigned int __num_threads_per_package;
>   extern unsigned int __num_cores_per_package;
>   
> +enum x86_topology_hw_cpu_type topology_hw_cpu_type(struct cpuinfo_x86 *c);
> +
>   static inline unsigned int topology_max_packages(void)
>   {
>   	return __max_logical_packages;
> diff --git a/arch/x86/kernel/cpu/debugfs.c b/arch/x86/kernel/cpu/debugfs.c
> index ca373b990c47..d1731e0e36b0 100644
> --- a/arch/x86/kernel/cpu/debugfs.c
> +++ b/arch/x86/kernel/cpu/debugfs.c
> @@ -22,6 +22,7 @@ static int cpu_debug_show(struct seq_file *m, void *p)
>   	seq_printf(m, "die_id:              %u\n", c->topo.die_id);
>   	seq_printf(m, "cu_id:               %u\n", c->topo.cu_id);
>   	seq_printf(m, "core_id:             %u\n", c->topo.core_id);
> +	seq_printf(m, "hw_cpu_type:       0x%x\n", c->topo.hw_cpu_type);
>   	seq_printf(m, "logical_pkg_id:      %u\n", c->topo.logical_pkg_id);
>   	seq_printf(m, "logical_die_id:      %u\n", c->topo.logical_die_id);
>   	seq_printf(m, "llc_id:              %u\n", c->topo.llc_id);
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index e7656cbef68d..e56401c5c050 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -1299,3 +1299,8 @@ u8 get_this_hybrid_cpu_type(void)
>   
>   	return cpuid_eax(0x0000001a) >> X86_HYBRID_CPU_TYPE_ID_SHIFT;
>   }
> +
> +u32 intel_native_model_id(struct cpuinfo_x86 *c)
> +{
> +	return c->topo.intel_core_native_model_id;
> +}
> diff --git a/arch/x86/kernel/cpu/topology_common.c b/arch/x86/kernel/cpu/topology_common.c
> index 9a6069e7133c..e4814cd3d8ae 100644
> --- a/arch/x86/kernel/cpu/topology_common.c
> +++ b/arch/x86/kernel/cpu/topology_common.c
> @@ -27,6 +27,14 @@ void topology_set_dom(struct topo_scan *tscan, enum x86_topology_domains dom,
>   	}
>   }
>   
> +enum x86_topology_hw_cpu_type topology_hw_cpu_type(struct cpuinfo_x86 *c)
> +{
> +	if (c->x86_vendor == X86_VENDOR_INTEL)
> +		return c->topo.intel_core_type;
> +
> +	return c->topo.hw_cpu_type;
> +}
> +
>   static unsigned int __maybe_unused parse_num_cores_legacy(struct cpuinfo_x86 *c)
>   {
>   	struct {
> @@ -87,6 +95,7 @@ static void parse_topology(struct topo_scan *tscan, bool early)
>   		.cu_id			= 0xff,
>   		.llc_id			= BAD_APICID,
>   		.l2c_id			= BAD_APICID,
> +		.hw_cpu_type		= TOPO_HW_CPU_TYPE_UNKNOWN,
>   	};
>   	struct cpuinfo_x86 *c = tscan->c;
>   	struct {
> @@ -132,6 +141,8 @@ 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.hw_cpu_type = cpuid_eax(0x1a);
>   		break;
>   	case X86_VENDOR_HYGON:
>   		if (IS_ENABLED(CONFIG_CPU_SUP_HYGON))