linux-kernel - Re: [PATCH v13 3/9] cpufreq: introduce init_boost callback to initialize boost state for pstate drivers

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] [day] [month] [year] [list]

Message-ID: <b584f631-b948-4cc5-89af-724b892341ac@amd.com>
Date: Mon, 24 Jun 2024 16:47:21 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Perry Yuan <perry.yuan@....com>, rafael.j.wysocki@...el.com,
 viresh.kumar@...aro.org, gautham.shenoy@....com
Cc: 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 v13 3/9] cpufreq: introduce init_boost callback to
 initialize boost state for pstate drivers

On 6/21/2024 11:50, Perry Yuan wrote:
> Introduce a new init_boost callback in cpufreq to initialize the boost
> state for specific pstate drivers. This initialization is required before
> calling the set_boost interface for each CPU.
> 
> The init_boost callback will set up and synchronize each CPU's current
> boost state before invoking the set_boost function. Without this step,
> the boost state may be inconsistent across CPUs.
> 
> Signed-off-by: Perry Yuan <perry.yuan@....com>

As a part of the testing on the v13 series the past few days, we 
realized that this patch is not necessary.  It's really a workaround for 
a change in behavior introduced in

f37a4d6b4a2c ("cpufreq: Fix per-policy boost behavior on SoCs using 
cpufreq_boost_set_sw()")

I've adjusted the series (as v14) to drop this patch and instead send a 
fix for that one.

> ---
>   drivers/cpufreq/cpufreq.c | 14 ++++++++++++--
>   include/linux/cpufreq.h   |  2 ++
>   2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 1fdabb660231..0c99d2dfdb53 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1429,8 +1429,18 @@ static int cpufreq_online(unsigned int cpu)
>   			goto out_free_policy;
>   		}
>   
> -		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> -		policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> +		/* init boost state to prepare set_boost callback for each CPU */
> +		if (cpufreq_driver->init_boost) {
> +			ret = cpufreq_driver->init_boost(policy);
> +			if (ret) {
> +				pr_debug("%s: %d: boost initialization failed\n", __func__,
> +					__LINE__);
> +				goto out_offline_policy;
> +			}
> +		} else {
> +			/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> +			policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> +		}
>   
>   		/*
>   		 * The initialization has succeeded and the policy is online.
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 20f7e98ee8af..0698c0292d8f 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -409,6 +409,8 @@ struct cpufreq_driver {
>   	bool		boost_enabled;
>   	int		(*set_boost)(struct cpufreq_policy *policy, int state);
>   
> +	/* initialize boost state to be consistent before calling set_boost */
> +	int		(*init_boost)(struct cpufreq_policy *policy);
>   	/*
>   	 * Set by drivers that want to register with the energy model after the
>   	 * policy is properly initialized, but before the governor is started.