linux-kernel - Re: [PATCH 06/15] cpufreq: cppc: Set policy->boost

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: <20250206052754.3p5axkd4tsz5knhe@vireshk-i7>
Date: Thu, 6 Feb 2025 10:57:54 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org,
	Vincent Guittot <vincent.guittot@...aro.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/15] cpufreq: cppc: Set policy->boost_supported

On 06-02-25, 11:58, zhenglifeng (A) wrote:
> On 2025/1/24 16:58, Viresh Kumar wrote:
> > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > index 7fa89b601d2a..08117fb9c1eb 100644
> > --- a/drivers/cpufreq/cppc_cpufreq.c
> > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > @@ -34,8 +34,6 @@
> >   */
> >  static LIST_HEAD(cpu_data_list);
> >  
> > -static bool boost_supported;
> > -
> >  static struct cpufreq_driver cppc_cpufreq_driver;
> >  
> >  #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
> > @@ -653,7 +651,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
> >  	 * is supported.
> >  	 */
> >  	if (caps->highest_perf > caps->nominal_perf)
> > -		boost_supported = true;
> > +		policy->boost_supported = true;
> >  
> >  	/* Set policy->cur to max now. The governors will adjust later. */
> >  	policy->cur = cppc_perf_to_khz(caps, caps->highest_perf);
> > @@ -791,11 +789,6 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
> >  	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> >  	int ret;
> >  
> > -	if (!boost_supported) {
> > -		pr_err("BOOST not supported by CPU or firmware\n");
> > -		return -EINVAL;
> > -	}
> > -
> >  	if (state)
> >  		policy->max = cppc_perf_to_khz(caps, caps->highest_perf);
> >  	else
> 
> I have a little question. With the old boost_supported flag as false, it
> will fail when you operate the global boost flag. But if you replace it
> with the per-policy boost_supported flag, the global boost_enabled flag can
> be set to true without any error, even no policy's boost_enabled flag
> changed. Is this interface behavior change OK?

Right, so earlier even if a single policy didn't support boost, the code disabled
boost for all of them. Or it was rather racy, as the last policy to be
initialized will decide if boost will be supported or not. This is surely
incorrect.

The global boost flag can be enabled disabled without worrying about any
individual policy. If the policy supports boost, it will follow the global boost
here, else it shouldn't take part in the change.

So yes, the new interface does the right thing here.

-- 
viresh