linux-kernel - Re: [PATCH 1/6] 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: <CAJZ5v0hLBE0vLvpw6k8E7KxiUGqXbH7wEZwFhEziJNYqfxJbyA@mail.gmail.com>
Date: Thu, 24 Apr 2025 13:26:10 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Lifeng Zheng <zhenglifeng1@...wei.com>, 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 1/6] cpufreq: acpi: Don't enable boost on policy exit

On Thu, Apr 24, 2025 at 9:15 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> On 23-04-25, 16:14, Rafael J. Wysocki wrote:
> > Even after commit 2b16c631832d, the code removed by this patch does a
> > useful thing.  Namely, it clears the boost-disable bit in the MSR so
> > that the offline CPU doesn't prevent online CPUs from getting the
> > boost (in case the boost settings change after it has been taken
> > offline).
>
> I didn't understand this part earlier (and even now). How does a CPU
> with boost-disabled, prevents others from boosting ? I have tried
> looking at git logs, and still don't understand it :(

At the HW level, this is certainly possible.

Say two (or more) cores are driven by the same VR.  Boost typically
(always?) requires a separate OPP with a higher voltage and this
applies to all cores sharing the VR, so if one of them says it doesn't
want that (which is what the bit in the boost-disable MSR effectively
means), they all won't get it.

They arguably should belong to the same cpufreq policy, but this
information is often missing from the ACPI tables, sometimes on
purpose (for instance, the firmware may want to be in charge of the
frequency coordination between the cores).

> Also, IIUC this and the boost-enabling at init() only happens for one
> CPU in a policy, as init() and exit() are only called for the first
> and last CPU of a policy. So if a policy has multiple CPUs, we aren't
> touching boost states of other CPUs at init/exit.

But there may be a policy per CPU.

> And yes, this patch isn't mandatory at all for the
>
> > Moreover, without the $subject patch, the change made by the next one
> > will cause the boost setting in the MSR to get back in sync with
> > policy->boost_enabled during online AFAICS, so why exactly is the
> > $subject patch needed?
>
> Right, this is merely a cleanup patch and isn't really required for
> the next patch to make it work.

So I'd rather not make this change.

Evidently, someone made the effort to put in a comment explaining the
purpose of the code in question, so it looks like they had a reason
for adding it.