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: <CAJZ5v0g2+Eu9Nv0gha-806_wOiPQCrtLJikNmMoCo48tt4QvsQ@mail.gmail.com>
Date: Wed, 16 Apr 2025 14:38:14 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: "zhenglifeng (A)" <zhenglifeng1@...wei.com>, Viresh Kumar <viresh.kumar@...aro.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 Wed, Apr 16, 2025 at 10:47 AM zhenglifeng (A)
<zhenglifeng1@...wei.com> wrote:
>
> 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>

Applied as 6.15-rc material, thanks!