linux-kernel - Re: [PATCH v3 5/5] x86/amd: Use heterogeneous core topology for identifying boost numerator

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f1f8c20d-ec13-49d3-ac47-a2fcd4433baf@amd.com>
Date: Fri, 25 Oct 2024 08:57:46 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Dave Hansen <dave.hansen@...ux.intel.com>,
 "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
 "H . Peter Anvin" <hpa@...or.com>, "Rafael J . Wysocki" <rafael@...nel.org>,
 "Gautham R . Shenoy" <gautham.shenoy@....com>,
 Perry Yuan <perry.yuan@....com>, Brijesh Singh <brijesh.singh@....com>,
 Peter Zijlstra <peterz@...radead.org>, Li RongQing <lirongqing@...du.com>,
 "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
 <linux-kernel@...r.kernel.org>, "open list:ACPI"
 <linux-acpi@...r.kernel.org>,
 "open list:AMD PSTATE DRIVER" <linux-pm@...r.kernel.org>,
 Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
Subject: Re: [PATCH v3 5/5] x86/amd: Use heterogeneous core topology for
 identifying boost numerator

On 10/25/2024 08:51, Borislav Petkov wrote:
> On Wed, Oct 23, 2024 at 12:43:57PM -0500, Mario Limonciello wrote:
>>   int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
>>   {
>> +	enum x86_topology_cpu_type core_type = get_topology_generic_cpu_type(&cpu_data(cpu));
>>   	bool prefcore;
>>   	int ret;
>> +	u32 tmp;
>>   
>>   	ret = amd_detect_prefcore(&prefcore);
>>   	if (ret)
>> @@ -261,6 +263,27 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
>>   			break;
>>   		}
>>   	}
>> +
> 
> What's the difference between this case:
> 
>> +	/* detect if running on heterogeneous design */
>> +	switch (core_type) {
>> +	case TOPO_CPU_TYPE_UNKNOWN:
> 	     ^^^^^^^^^^^^^^^^^^^^^^^
> 
>> +		break;
>> +	case TOPO_CPU_TYPE_PERFORMANCE:
>> +		/* use the max scale for performance cores */
>> +		*numerator = CPPC_HIGHEST_PERF_PERFORMANCE;
>> +		return 0;
>> +	case TOPO_CPU_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:
> 
> ... and this case and why aren't you warning if TOPO_CPU_TYPE_UNKNOWN?
> 
> I think for that you need to check X86_FEATURE_AMD_HETEROGENEOUS_CORES and
> warn if set but still CPU type unknown or?

Yeah; you're right.  An earlier version of this behaved differently and 
I missed updating this switch/case when using Pawan's updated patch.

After we get Intel feedback on the previous patch I'll drop the 
'default' case in the next version and switch it to this:

case TOPO_CPU_TYPE_UNKNOWN:
	if (cpu_feature_enabled(X86_FEATURE_AMD_HETEROGENEOUS_CORES))
		pr_warn("Undefined core type found for cpu %d\n", cpu);
	break;

> 
>> +		pr_warn("WARNING: Undefined core type %d found\n", core_type);
>> +		break;
>> +	}
>