linux-kernel - Re: [PATCH] cpufreq: acpi: Don't enable boost on policy exit

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87665307-1be3-465b-ab7c-3bf908b45217@huawei.com>
Date: Wed, 16 Apr 2025 16:47:10 +0800
From: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
To: Viresh Kumar <viresh.kumar@...aro.org>, "Rafael J. Wysocki"
	<rafael@...nel.org>
CC: <linux-pm@...r.kernel.org>, Vincent Guittot <vincent.guittot@...aro.org>,
	Nicholas Chin <nic.c3.14@...il.com>, "Rafael J. Wysocki"
	<rafael.j.wysocki@...el.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cpufreq: acpi: Don't enable boost on policy exit

On 2025/4/16 13:29, Viresh Kumar wrote:

> The boost-related code in cpufreq has undergone several changes over the
> years, but this particular piece remained unchanged and is now outdated.
> 
> The cpufreq core currently manages boost settings during initialization,
> and only when necessary. As such, there's no longer a need to enable
> boost explicitly when entering system suspend.
> 
> Previously, this wasn’t causing issues because boost settings were
> force-updated during policy initialization. However, commit 2b16c631832d
> ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") changed
> that behavior—correctly—by avoiding unnecessary updates.
> 
> As a result of this change, if boost was disabled prior to suspend, it
> remains disabled on resume—as expected. But due to the current code
> forcibly enabling boost at suspend time, the system ends up with boost
> frequencies enabled after resume, even if the global boost flag was
> disabled. This contradicts the intended behavior.
> 
> Fix this by not enabling boost on policy exit.
> 
> Fixes: 2b16c631832d ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220013
> Reported-by: Nicholas Chin <nic.c3.14@...il.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 924314cdeebc..85b5a88f723f 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -89,8 +89,9 @@ static bool boost_state(unsigned int cpu)
>  	return false;
>  }
>  
> -static int boost_set_msr(bool enable)
> +static void boost_set_msr_each(void *p_en)
>  {
> +	bool enable = (bool) p_en;
>  	u32 msr_addr;
>  	u64 msr_mask, val;
>  
> @@ -107,7 +108,7 @@ static int boost_set_msr(bool enable)
>  		msr_mask = MSR_K7_HWCR_CPB_DIS;
>  		break;
>  	default:
> -		return -EINVAL;
> +		return;
>  	}
>  
>  	rdmsrl(msr_addr, val);
> @@ -118,14 +119,6 @@ static int boost_set_msr(bool enable)
>  		val |= msr_mask;
>  
>  	wrmsrl(msr_addr, val);
> -	return 0;
> -}
> -
> -static void boost_set_msr_each(void *p_en)
> -{
> -	bool enable = (bool) p_en;
> -
> -	boost_set_msr(enable);
>  }
>  
>  static int set_boost(struct cpufreq_policy *policy, int val)
> @@ -532,15 +525,6 @@ static void free_acpi_perf_data(void)
>  	free_percpu(acpi_perf_data);
>  }
>  
> -static int cpufreq_boost_down_prep(unsigned int cpu)
> -{
> -	/*
> -	 * Clear the boost-disable bit on the CPU_DOWN path so that
> -	 * this cpu cannot block the remaining ones from boosting.
> -	 */
> -	return boost_set_msr(1);
> -}
> -
>  /*
>   * acpi_cpufreq_early_init - initialize ACPI P-States library
>   *
> @@ -931,7 +915,6 @@ static void acpi_cpufreq_cpu_exit(struct cpufreq_policy *policy)
>  
>  	pr_debug("%s\n", __func__);
>  
> -	cpufreq_boost_down_prep(policy->cpu);
>  	policy->fast_switch_possible = false;
>  	policy->driver_data = NULL;
>  	acpi_processor_unregister_performance(data->acpi_perf_cpu);

Nice!

I wonder why this cpufreq_boost_down_prep() was needed at the beginning.

Reviewed-by: Lifeng Zheng <zhenglifeng1@...wei.com>