linux-kernel - Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency

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]

Date: Tue, 25 Jun 2024 10:11:29 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
 rafael@...nel.org, viresh.kumar@...aro.org, gautham.shenoy@....com,
 perry.yuan@....com, skhan@...uxfoundation.org, li.meng@....com,
 ray.huang@....com
Subject: Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency

On 6/25/2024 08:51, Dhananjay Ugwekar wrote:
> Minor modification, the commit subject is supposed to be
> "cpufreq/amd-pstate-ut: Handle the inconsistency between nominal_freq and other *_freq units"
> 
> The second half disappeared due to the word wrapping I guess.

I had some other feedback on the series, so when you submit a v2 can you 
try to fix the title on the first patch?

> 
> Regards,
> Dhananjay
> 
> On 6/25/2024 7:11 PM, Dhananjay Ugwekar wrote:
>> cpudata->nominal_freq being in MHz whereas other frequencies being in
>> KHz breaks the amd-pstate-ut frequency sanity check. This fixes it.
>>
>> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
>> Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
>> ---
>>   drivers/cpufreq/amd-pstate-ut.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
>> index fc275d41d51e..66b73c308ce6 100644
>> --- a/drivers/cpufreq/amd-pstate-ut.c
>> +++ b/drivers/cpufreq/amd-pstate-ut.c
>> @@ -202,6 +202,7 @@ static void amd_pstate_ut_check_freq(u32 index)
>>   	int cpu = 0;
>>   	struct cpufreq_policy *policy = NULL;
>>   	struct amd_cpudata *cpudata = NULL;
>> +	u32 nominal_freq_khz;
>>   
>>   	for_each_possible_cpu(cpu) {
>>   		policy = cpufreq_cpu_get(cpu);
>> @@ -209,13 +210,14 @@ static void amd_pstate_ut_check_freq(u32 index)
>>   			break;
>>   		cpudata = policy->driver_data;
>>   
>> -		if (!((cpudata->max_freq >= cpudata->nominal_freq) &&
>> -			(cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
>> +		nominal_freq_khz = cpudata->nominal_freq*1000;
>> +		if (!((cpudata->max_freq >= nominal_freq_khz) &&
>> +			(nominal_freq_khz > cpudata->lowest_nonlinear_freq) &&
>>   			(cpudata->lowest_nonlinear_freq > cpudata->min_freq) &&
>>   			(cpudata->min_freq > 0))) {
>>   			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>>   			pr_err("%s cpu%d max=%d >= nominal=%d > lowest_nonlinear=%d > min=%d > 0, the formula is incorrect!\n",
>> -				__func__, cpu, cpudata->max_freq, cpudata->nominal_freq,
>> +				__func__, cpu, cpudata->max_freq, nominal_freq_khz,
>>   				cpudata->lowest_nonlinear_freq, cpudata->min_freq);
>>   			goto skip_test;
>>   		}
>> @@ -229,13 +231,13 @@ static void amd_pstate_ut_check_freq(u32 index)
>>   
>>   		if (cpudata->boost_supported) {
>>   			if ((policy->max == cpudata->max_freq) ||
>> -					(policy->max == cpudata->nominal_freq))
>> +					(policy->max == nominal_freq_khz))
>>   				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
>>   			else {
>>   				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>>   				pr_err("%s cpu%d policy_max=%d should be equal cpu_max=%d or cpu_nominal=%d !\n",
>>   					__func__, cpu, policy->max, cpudata->max_freq,
>> -					cpudata->nominal_freq);
>> +					nominal_freq_khz);
>>   				goto skip_test;
>>   			}
>>   		} else {