mbox series

[v2,0/5] x86 Heterogeneous design identification

Message ID 20241022034608.32396-1-mario.limonciello@amd.com
Headers show
Series x86 Heterogeneous design identification | expand

Message

Mario Limonciello Oct. 22, 2024, 3:46 a.m. UTC
This series adds topology identification for Intel and AMD processors and
uses this identification in the AMD CPPC code to identify the boost
numerator.

This series was previously submitted as [1], but this was based on some
patches in linux-pm/linux-next that will be dropped.

Instead the series is now based on tip/master.

This also pulls one patch from Pawan's series [2] and adjusts it for all
feedback while adding AMD support at the same time.

[1] https://lore.kernel.org/all/20241021175509.2079-5-mario.limonciello@amd.com/T/
[2] https://lore.kernel.org/all/20240930-add-cpu-type-v4-0-104892b7ab5f@linux.intel.com/

Mario Limonciello (2):
  x86/cpufeatures: Rename X86_FEATURE_FAST_CPPC to have AMD prefix
  x86/amd: Use heterogeneous core topology for identifying boost
    numerator

Pawan Gupta (1):
  x86/cpu: Add CPU type to struct cpuinfo_topology

Perry Yuan (2):
  x86/cpufeatures: Add feature bits for AMD heterogeneous processor
  x86/cpu: Enable SD_ASYM_PACKING for PKG Domain on AMD Processors

 arch/x86/include/asm/cpu.h               | 19 +++++++++++++++++++
 arch/x86/include/asm/cpufeatures.h       |  3 ++-
 arch/x86/include/asm/processor.h         | 18 ++++++++++++++++++
 arch/x86/include/asm/topology.h          |  8 ++++++++
 arch/x86/kernel/acpi/cppc.c              | 23 +++++++++++++++++++++++
 arch/x86/kernel/cpu/amd.c                | 14 ++++++++++++++
 arch/x86/kernel/cpu/debugfs.c            |  1 +
 arch/x86/kernel/cpu/intel.c              | 18 ++++++++++++++++++
 arch/x86/kernel/cpu/scattered.c          |  3 ++-
 arch/x86/kernel/cpu/topology_amd.c       |  3 +++
 arch/x86/kernel/cpu/topology_common.c    | 13 +++++++++++++
 arch/x86/kernel/smpboot.c                |  5 +++--
 drivers/cpufreq/amd-pstate.c             |  2 +-
 tools/arch/x86/include/asm/cpufeatures.h |  2 +-
 14 files changed, 126 insertions(+), 6 deletions(-)


base-commit: 21f0d4005e7eb71b95cf6b55041fd525bdb11c1f

Comments

Pawan Gupta Oct. 23, 2024, 4:40 a.m. UTC | #1
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;
}
Borislav Petkov Oct. 23, 2024, 3:59 p.m. UTC | #2
On Tue, Oct 22, 2024 at 09:40:15PM -0700, Pawan Gupta wrote:
> 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;
> }

Ok...

> 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;

... except those silly wrappers can go. There's a reason cpuinfo_x86 has
well-defined members which can be used directly.