RE: [PATCH 02/12] cpufreq: amd-pstate: enable AMD Precision Boost mode switch

[Yuan, Perry]

[AMD Official Use Only - General]

Hi Nathan:

> -----Original Message-----
> From: Fontenot, Nathan <Nathan.Fontenot@xxxxxxx>
> Sent: Friday, July 8, 2022 3:32 AM
> To: Yuan, Perry <Perry.Yuan@xxxxxxx>; rafael.j.wysocki@xxxxxxxxx;
> viresh.kumar@xxxxxxxxxx; Huang, Ray <Ray.Huang@xxxxxxx>; Thomas
> Gleixner <tglx@xxxxxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>; Borislav
> Petkov <bp@xxxxxxxxx>; Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>;
> x86@xxxxxxxxxx; H. Peter Anvin <hpa@xxxxxxxxx>; Rafael J. Wysocki
> <rafael@xxxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Adrian Hunter
> <adrian.hunter@xxxxxxxxx>; Pawan Gupta
> <pawan.kumar.gupta@xxxxxxxxxxxxxxx>; Alexander Shishkin
> <alexander.shishkin@xxxxxxxxxxxxxxx>; Tony Luck <tony.luck@xxxxxxxxx>;
> Stephane Eranian <eranian@xxxxxxxxxx>; Ricardo Neri <ricardo.neri-
> calderon@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; linux-
> pm@xxxxxxxxxxxxxxx
> Cc: Sharma, Deepak <Deepak.Sharma@xxxxxxx>; Limonciello, Mario
> <Mario.Limonciello@xxxxxxx>; Fontenot, Nathan
> <Nathan.Fontenot@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Su, Jinzhou (Joe) <Jinzhou.Su@xxxxxxx>;
> Huang, Shimmer <Shimmer.Huang@xxxxxxx>; Du, Xiaojian
> <Xiaojian.Du@xxxxxxx>; Meng, Li (Jassmine) <Li.Meng@xxxxxxx>
> Subject: Re: [PATCH 02/12] cpufreq: amd-pstate: enable AMD Precision
> Boost mode switch
> 
> On 7/7/22 11:55, Perry Yuan wrote:
> > Add support to switch AMD precision boost state to scale cpu max
> > frequency that will help to improve the processor throughput.
> >
> > when set boost state to be enabled, user will need to execute below
> > commands, the CPU will reach absolute maximum performance level or
> the
> > highest perf which CPU physical support. This performance level may
> > not be sustainable for long durations, it will help to improve the IO
> workload tasks.
> >
> > * turn on CPU boost state under root
> >   echo 1 > /sys/devices/system/cpu/cpufreq/boost
> >
> > If user set boost off,the CPU can reach to the maximum sustained
> > performance level of the process, that level is the process can
> > maintain continously working and definitely it can save some power
> > compared to boost on mode.
> >
> > * turn off CPU boost state under root
> >   echo 0 > /sys/devices/system/cpu/cpufreq/boost
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@xxxxxxx>
> > ---
> >  arch/x86/include/asm/msr-index.h |  2 ++
> >  drivers/cpufreq/amd-pstate.c     | 22 +++++++++++++++++++---
> >  2 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/msr-index.h
> > b/arch/x86/include/asm/msr-index.h
> > index 869508de8269..b952fd6d6916 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -559,6 +559,8 @@
> >  #define AMD_CPPC_MIN_PERF(x)		(((x) & 0xff) << 8)
> >  #define AMD_CPPC_DES_PERF(x)		(((x) & 0xff) << 16)
> >  #define AMD_CPPC_ENERGY_PERF_PREF(x)	(((x) & 0xff) << 24)
> > +#define AMD_CPPC_PRECISION_BOOST_BIT	25
> > +#define AMD_CPPC_PRECISION_BOOST_ENABLED
> 	BIT_ULL(AMD_CPPC_PRECISION_BOOST_BIT)
> >
> >  /* AMD Performance Counter Global Status and Control MSRs */
> >  #define MSR_AMD64_PERF_CNTR_GLOBAL_STATUS	0xc0000300
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 9ac75c1cde9c..188e055e24a2
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -122,6 +122,7 @@ struct amd_cpudata {
> >
> >  	u64 freq;
> >  	bool	boost_supported;
> > +	u64 	cppc_hw_conf_cached;
> 
> The MSR value is cached but I don't see that the cached value is used
> anywhere. Perhaps I missed it in one of the other patches. Does this need
> to be cached?
> 
> -Nathan

The bit value will be used in the coming CPPC EPP patches to check if the CPU precision boost bit is enabled.
I early added this prefetching code as a preparing code.  

Perry.

> 
> >  };
> >
> >  static inline int pstate_enable(bool enable) @@ -438,18 +439,27 @@
> > static int amd_pstate_set_boost(struct cpufreq_policy *policy, int
> > state)  {
> >  	struct amd_cpudata *cpudata = policy->driver_data;
> >  	int ret;
> > +	u64 value;
> >
> >  	if (!cpudata->boost_supported) {
> >  		pr_err("Boost mode is not supported by this processor or
> SBIOS\n");
> >  		return -EINVAL;
> >  	}
> >
> > -	if (state)
> > +	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_HW_CTL,
> &value);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (state) {
> > +		value |= AMD_CPPC_PRECISION_BOOST_ENABLED;
> >  		policy->cpuinfo.max_freq = cpudata->max_freq;
> > -	else
> > +	} else {
> > +		value &= ~AMD_CPPC_PRECISION_BOOST_ENABLED;
> >  		policy->cpuinfo.max_freq = cpudata->nominal_freq;
> > -
> > +	}
> >  	policy->max = policy->cpuinfo.max_freq;
> > +	WRITE_ONCE(cpudata->cppc_hw_conf_cached, value);
> > +	wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_HW_CTL, value);
> >
> >  	ret = freq_qos_update_request(&cpudata->req[1],
> >  				      policy->cpuinfo.max_freq);
> > @@ -478,6 +488,7 @@ static int amd_pstate_cpu_init(struct
> cpufreq_policy *policy)
> >  	int min_freq, max_freq, nominal_freq, lowest_nonlinear_freq, ret;
> >  	struct device *dev;
> >  	struct amd_cpudata *cpudata;
> > +	u64 value;
> >
> >  	dev = get_cpu_device(policy->cpu);
> >  	if (!dev)
> > @@ -542,6 +553,11 @@ static int amd_pstate_cpu_init(struct
> > cpufreq_policy *policy)
> >
> >  	policy->driver_data = cpudata;
> >
> > +	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_HW_CTL,
> &value);
> > +	if (ret)
> > +		return ret;
> > +	WRITE_ONCE(cpudata->cppc_hw_conf_cached, value);
> > +
> >  	amd_pstate_boost_init(cpudata);
> >
> >  	return 0;