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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20250417015424.36487-1-nic.c3.14@gmail.com>
Date: Wed, 16 Apr 2025 19:54:17 -0600
From: Nicholas Chin <nic.c3.14@...il.com>
To: viresh.kumar@...aro.org
Cc: linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org,
	nic.c3.14@...il.com,
	rafael.j.wysocki@...el.com,
	rafael@...nel.org,
	vincent.guittot@...aro.org,
	zhenglifeng1@...wei.com
Subject: Re: [PATCH] cpufreq: acpi: Don't enable boost on policy exit

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

Unfortunately the issue I reported still seems to be present after applying this patch. Upon resuming from suspend, the system is still entering boost states descpite the boost flag being set to 0.