Re: [PATCH 1/2] x86/cpufeatures: Add feature bits for AMD heterogeneous processor

[Mario Limonciello <superm1@kernel.org>]

On 10/18/2024 13:24, Borislav Petkov wrote:
> On Thu, Oct 03, 2024 at 04:37:58PM -0500, Mario Limonciello wrote:
>> From: Perry Yuan <perry.yuan@amd.com>
>>
>> CPUID leaf 0x80000026 advertises core types with different efficiency
>> rankings.
>>
>> Bit 30 indicates the heterogeneous core topology feature, if the bit
>> set, it means not all instances at the current hierarchical level have
>> the same core topology.
>>
>> This is described in the AMD64 Architecture Programmers Manual Volume
>> 2 and 3, doc ID #25493 and #25494.
>>
>> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   arch/x86/include/asm/cpufeatures.h | 1 +
>>   arch/x86/kernel/cpu/scattered.c    | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index dd4682857c12..cea1ed82aeb4 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -473,6 +473,7 @@
>>   #define X86_FEATURE_CLEAR_BHB_HW	(21*32+ 3) /* BHI_DIS_S HW control enabled */
>>   #define X86_FEATURE_CLEAR_BHB_LOOP_ON_VMEXIT (21*32+ 4) /* Clear branch history at vmexit using SW loop */
>>   #define X86_FEATURE_FAST_CPPC		(21*32 + 5) /* AMD Fast CPPC */
>> +#define X86_FEATURE_HETERO_CORE_TOPOLOGY	(21*32 + 6) /* Heterogeneous Core Topology */
> 
> So this is an AMD-specific feature bit and so it should have "AMD" in the
> name:
> 
> #define X86_FEATURE_AMD_HETERO_CORE_TOPOLOGY (21*32 + 6) /* Heterogeneous Core Topology */
> 
> Also, as clarified offlist, please do not take x86 patches without an Ack at
> least through some other tree.

Yes; sorry about this, the R-b was not sufficient and should have 
explicitly pinged for an A-b.

> 
>> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
>> index c84c30188fdf..3bba55323163 100644
>> --- a/arch/x86/kernel/cpu/scattered.c
>> +++ b/arch/x86/kernel/cpu/scattered.c
>> @@ -52,6 +52,7 @@ static const struct cpuid_bit cpuid_bits[] = {
>>   	{ X86_FEATURE_PERFMON_V2,	CPUID_EAX,  0, 0x80000022, 0 },
>>   	{ X86_FEATURE_AMD_LBR_V2,	CPUID_EAX,  1, 0x80000022, 0 },
>>   	{ X86_FEATURE_AMD_LBR_PMC_FREEZE,	CPUID_EAX,  2, 0x80000022, 0 },
>> +	{ X86_FEATURE_HETERO_CORE_TOPOLOGY,	CPUID_EAX,  30, 0x80000026, 0 },
> 
> So my APM calls this bit: "HeterogeneousCores".
> 
> Why aren't you calling it this?
> 
> IOW: X86_FEATURE_AMD_HETEROGENEOUS_CORES
> 
> Thx.
> 

OK - I'll adjust accordingly.

There is other content in linux-pm/linux-next that uses this.

As this patch is already in linux-pm/linux-next, I see 3 options:

1) I can bring a revert through superm1/linux.git to PR to 
linux-pm/linux-next and resubmit with fixes for you to take through tip.

2) Rafael can drop this and the follow on and I'll resubmit with your 
feedback and we can bring through tip

3) I can amend with the fixes we take through linux-pm/linux-next to 
avoid the acrobatics of 1 or 2.

Please let me know your preference.