| 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
| ||
|
Date: Mon, 11 Mar 2024 15:28:40 +0530
From: "Ugwekar, Dhananjay" <Dhananjay.Ugwekar@....com>
To: Perry Yuan <perry.yuan@....com>
Cc: rafael.j.wysocki@...el.com, Mario.Limonciello@....com,
viresh.kumar@...aro.org, Ray.Huang@....com, Borislav.Petkov@....com,
Alexander.Deucher@....com, Xinmei.Huang@....com, Xiaojian.Du@....com,
Li.Meng@....com, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
"Gautham R. Shenoy" <gautham.shenoy@....com>
Subject: Re: [PATCH v6 2/6] cpufreq:amd-pstate: fix the nominal freq value set
On 2/8/2024 11:37 AM, Gautham R. Shenoy wrote:
> On Thu, Feb 08, 2024 at 11:46:29AM +0800, Perry Yuan wrote:
>> Address an untested error where the nominal_freq was returned in KHz
>> instead of the correct MHz units, this oversight led to a wrong
>> nominal_freq set and resued, it will cause the max frequency of core to
>> be initialized with a wrong frequency value.
>
> The _CPC object on my Zen2 EPYC server which has CPPC v3, I see :
> 0x00000190, //LowestFrequency
> 0x000007D0 //NominalFrequency
> })
> }
>
> where 0x7D0 is 2000 Mhz. Thus the value returend by cppc_perf.nominal_freq is in Mhz.
>
> In this patch, we are changing making amd_get_nominal_freq() return
> 2000 (Mhz) instead of 2000000 (Khz) as it was doing previously.
>
> amd_get_min_freq(), amd_get_max_freq(),
> amd_get_lowest_nonlinear_perf() -all these functions return the
> frequency in Khz units.
>
> These functions are used the initialize the value of
> cpudata->max_freq, cpu_data->min_freq, .... all of which are in Khz
> units.
>
> In, amd_pstate_set_boost(), we have:
>
>
> if (state)
> policy->cpuinfo.max_freq = cpudata->max_freq; <---- In Khz
> else
> policy->cpuinfo.max_freq = cpudata->nominal_freq; <--- Now in Mhz
>
> Since policy->cpuinfo.max_freq is expected to be in Khz, isn't this
> patch introducing an error ? Or perhaps this patch series is based on
> another patchset ?
>
> --
> Thanks and Regards
> gautham.
>
>
I can confirm on Gautham's comment, when boost is disabled with the scaling
driver set as amd-pstate, the max frequency reported is incorrect, as it
shows the frequency in MHz.
I tested this patch on Zen4 Genoa server platform, below are the
inconsistent min and max frequencies I observed.
[cpufreq]# grep . *freq
amd_pstate_lowest_nonlinear_freq:1804000
amd_pstate_max_freq:3514000
cpuinfo_max_freq:2151
cpuinfo_min_freq:400000
scaling_cur_freq:2151
scaling_max_freq:2151
scaling_min_freq:2151
[cpufreq]# pwd
/sys/devices/system/cpu/cpu0/cpufreq
According to Documentation/admin-guide/pm/cpufreq.rst, all the frequency
values need to be in KHz.
Thanks,
Dhananjay
>>
>> Cc: stable@...r.kernel.org
>> Fixes: ec437d71db7 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
>> Reviewed-by: Mario Limonciello <mario.limonciello@....com>
>> Signed-off-by: Perry Yuan <perry.yuan@....com>
>> ---
>> drivers/cpufreq/amd-pstate.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index 08e112444c27..ac7faa98a450 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -640,8 +640,7 @@ static int amd_get_nominal_freq(struct amd_cpudata *cpudata)
>> if (ret)
>> return ret;
>>
>> - /* Switch to khz */
>> - return cppc_perf.nominal_freq * 1000;
>> + return cppc_perf.nominal_freq;
>> }
>>
>> static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists