linux-kernel - Re: [PATCH v6 2/6] cpufreq:amd-pstate: fix the nominal freq value set

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: <ZcRvoYZKdUEjBUHp@BLR-5CG11610CF.amd.com>
Date: Thu, 8 Feb 2024 11:37:29 +0530
From: "Gautham R. Shenoy" <gautham.shenoy@....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
Subject: Re: [PATCH v6 2/6] cpufreq:amd-pstate: fix the nominal freq value set

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.

 
> 
> 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
>