On Tue, Oct 22, 2024 at 01:57:20PM +0200, Borislav Petkov wrote:
> On Mon, Oct 21, 2024 at 10:46:07PM -0500, Mario Limonciello wrote:
> > * Take this patch from Pawan's series and fix up for feedback left on it.
> > * Add in AMD case as well
>
> Yah, this one is adding way more boilerplate code than it should. IOW, I'm
> thinking of something simpler like this:
>
> ---
>
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index 98eced5084ca..44122b077932 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -53,6 +53,7 @@ static inline void bus_lock_init(void) {}
> #ifdef CONFIG_CPU_SUP_INTEL
> u8 get_this_hybrid_cpu_type(void);
> u32 get_this_hybrid_cpu_native_id(void);
> +enum x86_topology_cpu_type get_intel_cpu_type(struct cpuinfo_x86 *c);
> #else
> static inline u8 get_this_hybrid_cpu_type(void)
> {
> @@ -63,6 +64,18 @@ static inline u32 get_this_hybrid_cpu_native_id(void)
> {
> return 0;
> }
> +static enum x86_topology_cpu_type get_intel_cpu_type(struct cpuinfo_x86 *c)
> +{
> + return TOPO_CPU_TYPE_UNKNOWN;
> +}
> +#endif
> +#ifdef CONFIG_CPU_SUP_AMD
> +enum x86_topology_cpu_type get_amd_cpu_type(struct cpuinfo_x86 *c);
> +#else
> +static inline enum x86_topology_cpu_type get_amd_cpu_type(struct cpuinfo_x86 *c)
> +{
> + return TOPO_CPU_TYPE_UNKNOWN;
> +}
> #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..c0975815980c 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -105,6 +105,24 @@ struct cpuinfo_topology {
> // Cache level topology IDs
> u32 llc_id;
> u32 l2c_id;
> +
> + // Hardware defined CPU-type
> + union {
> + u32 cpu_type;
> + struct {
> + // CPUID.1A.EAX[23-0]
> + u32 intel_native_model_id :24;
> + // CPUID.1A.EAX[31-24]
> + u32 intel_type :8;
> + };
> + struct {
> + // CPUID 0x80000026.EBX
> + u32 amd_num_processors :16,
> + amd_power_eff_ranking :8,
> + amd_native_model_id :4,
> + amd_type :4;
> + };
> + };
> };
>
> struct cpuinfo_x86 {
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index aef70336d624..c43d4b3324bc 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_cpu_type {
> + TOPO_CPU_TYPE_PERFORMANCE,
> + TOPO_CPU_TYPE_EFFICIENCY,
> + TOPO_CPU_TYPE_UNKNOWN,
> +};
> +
> 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;
>
> +const char *get_topology_cpu_type_name(struct cpuinfo_x86 *c);
> +
> static inline unsigned int topology_max_packages(void)
> {
> return __max_logical_packages;
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index fab5caec0b72..7d8d62fdc112 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1205,3 +1205,12 @@ void amd_check_microcode(void)
> if (cpu_feature_enabled(X86_FEATURE_ZEN2))
> on_each_cpu(zenbleed_check_cpu, NULL, 1);
> }
> +
> +enum x86_topology_cpu_type get_amd_cpu_type(struct cpuinfo_x86 *c)
> +{
> + switch (c->topo.amd_type) {
> + case 0: return TOPO_CPU_TYPE_PERFORMANCE;
> + case 1: return TOPO_CPU_TYPE_EFFICIENCY;
> + }
> + return TOPO_CPU_TYPE_UNKNOWN;
> +}
> diff --git a/arch/x86/kernel/cpu/debugfs.c b/arch/x86/kernel/cpu/debugfs.c
> index 3baf3e435834..10719aba6276 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, "cpu_type: %s\n", get_topology_cpu_type_name(c));
> 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 d1de300af173..7b1011d0b369 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -907,3 +907,12 @@ u32 get_this_hybrid_cpu_native_id(void)
> return cpuid_eax(0x0000001a) &
> (BIT_ULL(X86_HYBRID_CPU_TYPE_ID_SHIFT) - 1);
> }
> +
> +enum x86_topology_cpu_type get_intel_cpu_type(struct cpuinfo_x86 *c)
> +{
> + switch (c->topo.intel_type) {
> + case 0x20: return TOPO_CPU_TYPE_EFFICIENCY;
> + case 0x40: return TOPO_CPU_TYPE_PERFORMANCE;
> + }
> + return TOPO_CPU_TYPE_UNKNOWN;
> +}
The name of the function is a bit misleading, as it suggests that it
returns the Intel specific CPU type(ATOM/CORE), but it actually returns
the performance/efficiency generic types.
I would suggest to name the function based on what it returns, like:
get_generic_cpu_type(struct cpuinfo_x86 *c)
{
enum x86_topology_cpu_type type;
if (c->x86_vendor == X86_VENDOR_INTEL) {
switch (c->topo.intel_type) {
case 0x20: return TOPO_CPU_TYPE_EFFICIENCY;
case 0x40: return TOPO_CPU_TYPE_PERFORMANCE;
}
if (c->x86_vendor == X86_VENDOR_AMD) {
switch (c->topo.amd_type) {
case 0: return TOPO_CPU_TYPE_PERFORMANCE;
case 1: return TOPO_CPU_TYPE_EFFICIENCY;
}
return TOPO_CPU_TYPE_UNKNOWN;
}
get_intel_cpu_type(struct cpuinfo_x86 *c)
{
return c->topo.intel_type;
}
get_amd_cpu_type(struct cpuinfo_x86 *c)
{
return c->topo.amd_type;
}
