| Message ID | 20241003213759.3038862-3-superm1@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Detect max performance values for heterogeneous AMD designs | expand |
Hello Mario, On Thu, Oct 03, 2024 at 04:37:59PM -0500, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > AMD heterogeneous designs include two types of cores: > * Performance > * Efficiency > > Each core type has different highest performance values configured by the > platform. Drivers such as `amd_pstate` need to identify the type of > core to correctly set an appropriate boost numerator to calculate the > maximum frequency. > > X86_FEATURE_HETERO_CORE_TOPOLOGY is used to identify whether the SoC > supports heterogeneous core type by reading CPUID leaf Fn_0x80000026. > > On performance cores the scaling factor of 196 is used. On efficiency > cores the scaling factor is the value reported as the highest perf. > Efficiency cores have the same preferred core rankings. > > Suggested-by: Perry Yuan <perry.yuan@amd.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > arch/x86/include/asm/processor.h | 13 +++++++++++++ > arch/x86/kernel/acpi/cppc.c | 30 ++++++++++++++++++++++++++++++ > arch/x86/kernel/cpu/amd.c | 29 +++++++++++++++++++++++++++++ > 3 files changed, 72 insertions(+) > > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index 4a686f0e5dbf..d81a6efa81bb 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -691,6 +691,14 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu) > return per_cpu(cpu_info.topo.l2c_id, cpu); > } > > +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ > +enum amd_core_type { > + CPU_CORE_TYPE_NO_HETERO_SUP = -1, > + CPU_CORE_TYPE_PERFORMANCE = 0, > + CPU_CORE_TYPE_EFFICIENCY = 1, > + CPU_CORE_TYPE_UNDEFINED = 2, > +}; > + > #ifdef CONFIG_CPU_SUP_AMD > /* > * Issue a DIV 0/1 insn to clear any division data from previous DIV > @@ -703,9 +711,14 @@ static __always_inline void amd_clear_divider(void) > } > > extern void amd_check_microcode(void); > +extern enum amd_core_type amd_get_core_type(void); > #else > static inline void amd_clear_divider(void) { } > static inline void amd_check_microcode(void) { } > +static inline enum amd_core_type amd_get_core_type(void) > +{ > + return CPU_CORE_TYPE_NO_HETERO_SUP; > +} > #endif > > extern unsigned long arch_align_stack(unsigned long sp); > diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c > index 956984054bf3..ca289e6ec82c 100644 > --- a/arch/x86/kernel/acpi/cppc.c > +++ b/arch/x86/kernel/acpi/cppc.c > @@ -217,6 +217,12 @@ int amd_detect_prefcore(bool *detected) > } > EXPORT_SYMBOL_GPL(amd_detect_prefcore); > > +static void amd_do_get_core_type(void *data) > +{ > + enum amd_core_type *core_type = data; > + *core_type = amd_get_core_type(); > +} > + > /** > * amd_get_boost_ratio_numerator: Get the numerator to use for boost ratio calculation > * @cpu: CPU to get numerator for. > @@ -234,7 +240,9 @@ EXPORT_SYMBOL_GPL(amd_detect_prefcore); > */ > int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) > { > + enum amd_core_type core_type; > bool prefcore; > + u32 tmp; > int ret; > > ret = amd_detect_prefcore(&prefcore); > @@ -261,6 +269,28 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) > break; > } > } > + > + /* detect if running on heterogeneous design */ > + smp_call_function_single(cpu, amd_do_get_core_type, &core_type, 1); > + switch (core_type) { > + case CPU_CORE_TYPE_NO_HETERO_SUP: > + break; > + case CPU_CORE_TYPE_PERFORMANCE: > + /* use the max scale for performance cores */ > + *numerator = CPPC_HIGHEST_PERF_PERFORMANCE; > + return 0; > + case CPU_CORE_TYPE_EFFICIENCY: > + /* use the highest perf value for efficiency cores */ > + ret = amd_get_highest_perf(cpu, &tmp); > + if (ret) > + return ret; > + *numerator = tmp; > + return 0; > + default: > + pr_warn("WARNING: Undefined core type %d found\n", core_type); > + break; > + } > + > *numerator = CPPC_HIGHEST_PERF_PREFCORE; > > return 0; > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c > index 015971adadfc..8ad5f1385f0e 100644 > --- a/arch/x86/kernel/cpu/amd.c > +++ b/arch/x86/kernel/cpu/amd.c > @@ -1204,3 +1204,32 @@ void amd_check_microcode(void) > > on_each_cpu(zenbleed_check_cpu, NULL, 1); > } > + > +/** > + * amd_get_core_type - Heterogeneous core type identification > + * > + * Returns the CPU type [31:28] (i.e., performance or efficient) of > + * a CPU in the processor. > + * > + * If the processor has no core type support, returns > + * CPU_CORE_TYPE_NO_HETERO_SUP. > + */ > +enum amd_core_type amd_get_core_type(void) > +{ > + struct { > + u32 num_processors :16, > + power_efficiency_ranking :8, > + native_model_id :4, > + core_type :4; > + } props; > + > + if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY)) > + return CPU_CORE_TYPE_NO_HETERO_SUP; > + > + cpuid_leaf_reg(0x80000026, CPUID_EBX, &props); > + if (props.core_type >= CPU_CORE_TYPE_UNDEFINED) > + return CPU_CORE_TYPE_UNDEFINED; > + > + return props.core_type; > +} > +EXPORT_SYMBOL_GPL(amd_get_core_type); LGTM, Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham.
On Thu, Oct 03, 2024 at 04:37:59PM -0500, Mario Limonciello wrote: > Subject: Re: [PATCH 2/2] CPPC: Use heterogeneous core topology for identifying boost numerator The tip tree preferred format for patch subject prefixes is 'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:', 'genirq/core:'. Please do not use file names or complete file paths as prefix. 'git log path/to/file' should give you a reasonable hint in most cases. The condensed patch description in the subject line should start with a uppercase letter and should be written in imperative tone. > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index 4a686f0e5dbf..d81a6efa81bb 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -691,6 +691,14 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu) > return per_cpu(cpu_info.topo.l2c_id, cpu); > } > > +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ > +enum amd_core_type { > + CPU_CORE_TYPE_NO_HETERO_SUP = -1, Why? Why isn't this the last value in the enum without explicitly enumerating them? > + CPU_CORE_TYPE_PERFORMANCE = 0, > + CPU_CORE_TYPE_EFFICIENCY = 1, > + CPU_CORE_TYPE_UNDEFINED = 2, > +}; That thing goes... > + > #ifdef CONFIG_CPU_SUP_AMD ... in here. > /* > * Issue a DIV 0/1 insn to clear any division data from previous DIV ... > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c > index 015971adadfc..8ad5f1385f0e 100644 > --- a/arch/x86/kernel/cpu/amd.c > +++ b/arch/x86/kernel/cpu/amd.c > @@ -1204,3 +1204,32 @@ void amd_check_microcode(void) > > on_each_cpu(zenbleed_check_cpu, NULL, 1); > } > + > +/** > + * amd_get_core_type - Heterogeneous core type identification > + * > + * Returns the CPU type [31:28] (i.e., performance or efficient) of > + * a CPU in the processor. > + * > + * If the processor has no core type support, returns > + * CPU_CORE_TYPE_NO_HETERO_SUP. > + */ > +enum amd_core_type amd_get_core_type(void) > +{ > + struct { > + u32 num_processors :16, > + power_efficiency_ranking :8, > + native_model_id :4, > + core_type :4; > + } props; So this thing wants to use this stuff here: https://lore.kernel.org/r/20240930-add-cpu-type-v4-2-104892b7ab5f@linux.intel.com and add the AMD part to the union. Instead of calling CPUID each time and adding an ugly export... > + > + if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY)) > + return CPU_CORE_TYPE_NO_HETERO_SUP; > + > + cpuid_leaf_reg(0x80000026, CPUID_EBX, &props); > + if (props.core_type >= CPU_CORE_TYPE_UNDEFINED) > + return CPU_CORE_TYPE_UNDEFINED; > + > + return props.core_type; > +} > +EXPORT_SYMBOL_GPL(amd_get_core_type); > -- > 2.43.0 >
On 10/21/2024 05:58, Borislav Petkov wrote: > On Thu, Oct 03, 2024 at 04:37:59PM -0500, Mario Limonciello wrote: >> Subject: Re: [PATCH 2/2] CPPC: Use heterogeneous core topology for identifying boost numerator > > The tip tree preferred format for patch subject prefixes is > 'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:', > 'genirq/core:'. Please do not use file names or complete file paths as > prefix. 'git log path/to/file' should give you a reasonable hint in most > cases. > > The condensed patch description in the subject line should start with a > uppercase letter and should be written in imperative tone. > >> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h >> index 4a686f0e5dbf..d81a6efa81bb 100644 >> --- a/arch/x86/include/asm/processor.h >> +++ b/arch/x86/include/asm/processor.h >> @@ -691,6 +691,14 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu) >> return per_cpu(cpu_info.topo.l2c_id, cpu); >> } >> >> +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ >> +enum amd_core_type { >> + CPU_CORE_TYPE_NO_HETERO_SUP = -1, > > Why? > > Why isn't this the last value in the enum without explicitly enumerating them? > >> + CPU_CORE_TYPE_PERFORMANCE = 0, >> + CPU_CORE_TYPE_EFFICIENCY = 1, >> + CPU_CORE_TYPE_UNDEFINED = 2, >> +}; > > That thing goes... > >> + >> #ifdef CONFIG_CPU_SUP_AMD > > ... in here. > >> /* >> * Issue a DIV 0/1 insn to clear any division data from previous DIV > > ... > Ack; will drop a series momentarily fixing this and your other above feedback. Thx. >> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c >> index 015971adadfc..8ad5f1385f0e 100644 >> --- a/arch/x86/kernel/cpu/amd.c >> +++ b/arch/x86/kernel/cpu/amd.c >> @@ -1204,3 +1204,32 @@ void amd_check_microcode(void) >> >> on_each_cpu(zenbleed_check_cpu, NULL, 1); >> } >> + >> +/** >> + * amd_get_core_type - Heterogeneous core type identification >> + * >> + * Returns the CPU type [31:28] (i.e., performance or efficient) of >> + * a CPU in the processor. >> + * >> + * If the processor has no core type support, returns >> + * CPU_CORE_TYPE_NO_HETERO_SUP. >> + */ >> +enum amd_core_type amd_get_core_type(void) >> +{ >> + struct { >> + u32 num_processors :16, >> + power_efficiency_ranking :8, >> + native_model_id :4, >> + core_type :4; >> + } props; > > So this thing wants to use this stuff here: > > https://lore.kernel.org/r/20240930-add-cpu-type-v4-2-104892b7ab5f@linux.intel.com > > and add the AMD part to the union. Instead of calling CPUID each time and > adding an ugly export... Due to the cross tree landing complexity I'll rip this and the symbol out after that one lands (probably next cycle). > >> + >> + if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY)) >> + return CPU_CORE_TYPE_NO_HETERO_SUP; >> + >> + cpuid_leaf_reg(0x80000026, CPUID_EBX, &props); >> + if (props.core_type >= CPU_CORE_TYPE_UNDEFINED) >> + return CPU_CORE_TYPE_UNDEFINED; >> + >> + return props.core_type; >> +} >> +EXPORT_SYMBOL_GPL(amd_get_core_type); >> -- >> 2.43.0 >> >
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 4a686f0e5dbf..d81a6efa81bb 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -691,6 +691,14 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu) return per_cpu(cpu_info.topo.l2c_id, cpu); } +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ +enum amd_core_type { + CPU_CORE_TYPE_NO_HETERO_SUP = -1, + CPU_CORE_TYPE_PERFORMANCE = 0, + CPU_CORE_TYPE_EFFICIENCY = 1, + CPU_CORE_TYPE_UNDEFINED = 2, +}; + #ifdef CONFIG_CPU_SUP_AMD /* * Issue a DIV 0/1 insn to clear any division data from previous DIV @@ -703,9 +711,14 @@ static __always_inline void amd_clear_divider(void) } extern void amd_check_microcode(void); +extern enum amd_core_type amd_get_core_type(void); #else static inline void amd_clear_divider(void) { } static inline void amd_check_microcode(void) { } +static inline enum amd_core_type amd_get_core_type(void) +{ + return CPU_CORE_TYPE_NO_HETERO_SUP; +} #endif extern unsigned long arch_align_stack(unsigned long sp); diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c index 956984054bf3..ca289e6ec82c 100644 --- a/arch/x86/kernel/acpi/cppc.c +++ b/arch/x86/kernel/acpi/cppc.c @@ -217,6 +217,12 @@ int amd_detect_prefcore(bool *detected) } EXPORT_SYMBOL_GPL(amd_detect_prefcore); +static void amd_do_get_core_type(void *data) +{ + enum amd_core_type *core_type = data; + *core_type = amd_get_core_type(); +} + /** * amd_get_boost_ratio_numerator: Get the numerator to use for boost ratio calculation * @cpu: CPU to get numerator for. @@ -234,7 +240,9 @@ EXPORT_SYMBOL_GPL(amd_detect_prefcore); */ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) { + enum amd_core_type core_type; bool prefcore; + u32 tmp; int ret; ret = amd_detect_prefcore(&prefcore); @@ -261,6 +269,28 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) break; } } + + /* detect if running on heterogeneous design */ + smp_call_function_single(cpu, amd_do_get_core_type, &core_type, 1); + switch (core_type) { + case CPU_CORE_TYPE_NO_HETERO_SUP: + break; + case CPU_CORE_TYPE_PERFORMANCE: + /* use the max scale for performance cores */ + *numerator = CPPC_HIGHEST_PERF_PERFORMANCE; + return 0; + case CPU_CORE_TYPE_EFFICIENCY: + /* use the highest perf value for efficiency cores */ + ret = amd_get_highest_perf(cpu, &tmp); + if (ret) + return ret; + *numerator = tmp; + return 0; + default: + pr_warn("WARNING: Undefined core type %d found\n", core_type); + break; + } + *numerator = CPPC_HIGHEST_PERF_PREFCORE; return 0; diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 015971adadfc..8ad5f1385f0e 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -1204,3 +1204,32 @@ void amd_check_microcode(void) on_each_cpu(zenbleed_check_cpu, NULL, 1); } + +/** + * amd_get_core_type - Heterogeneous core type identification + * + * Returns the CPU type [31:28] (i.e., performance or efficient) of + * a CPU in the processor. + * + * If the processor has no core type support, returns + * CPU_CORE_TYPE_NO_HETERO_SUP. + */ +enum amd_core_type amd_get_core_type(void) +{ + struct { + u32 num_processors :16, + power_efficiency_ranking :8, + native_model_id :4, + core_type :4; + } props; + + if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY)) + return CPU_CORE_TYPE_NO_HETERO_SUP; + + cpuid_leaf_reg(0x80000026, CPUID_EBX, &props); + if (props.core_type >= CPU_CORE_TYPE_UNDEFINED) + return CPU_CORE_TYPE_UNDEFINED; + + return props.core_type; +} +EXPORT_SYMBOL_GPL(amd_get_core_type);