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: <435855f8-7c92-4fc3-be07-8856772e729f@huawei.com>
Date: Thu, 24 Apr 2025 21:15:15 +0800
From: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>, 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 1/6] cpufreq: acpi: Don't enable boost on policy exit

On 2025/4/24 19:26, Rafael J. Wysocki wrote:

> 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.

IIUC, this means that if one sets unboost to policy A, another core in
policy B (but sharing the same VR with core in policy A) will not be able
to achieve boost freq too. Then if policy A goes exit, the core in policy B
will get back to boost freq (without patch 1). And then core in B will be
unboosted again after core in A goes online/resume (because of patch 2).
But in the entire process, the boost flag in policy B is always enabled.

Please tell me I misunderstood because it looks really weird.😥

> 
> 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.
>