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@xxxxxxxxxxxxxxx
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
>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
