linux-kernel - Re: [PATCH 0/3] Add support for hidden choices to platform

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: <07a94bc8-ee1a-41ea-bfd9-44c7f7188680@app.fastmail.com>
Date: Fri, 28 Feb 2025 15:03:29 -0500
From: "Mark Pearson" <mpearson-lenovo@...ebb.ca>
To: "Mario Limonciello" <superm1@...nel.org>,
 "Shyam Sundar S K" <Shyam-sundar.S-k@....com>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 "Hans de Goede" <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 "Luke D . Jones" <luke@...nes.dev>
Cc: 
 "platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
 "open list" <linux-kernel@...r.kernel.org>,
 "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
 "Derek J . Clark" <derekjohn.clark@...il.com>,
 "Antheas Kapenekakis" <lkml@...heas.dev>, me@...egospodneti.ch,
 "Denis Benato" <benato.denis96@...il.com>,
 "Limonciello, Mario" <mario.limonciello@....com>
Subject: Re: [PATCH 0/3] Add support for hidden choices to platform_profile



On Fri, Feb 28, 2025, at 2:44 PM, Mario Limonciello wrote:
> On 2/28/2025 13:39, Mark Pearson wrote:
>> Hi Mario,
>> 
>> On Fri, Feb 28, 2025, at 12:01 PM, Mario Limonciello wrote:
>>> From: Mario Limonciello <mario.limonciello@....com>
>>>
>>> When two drivers provide platform profile handlers but use different
>>> strings to mean (essentially) the same thing the legacy interface won't
>>> export them because it only shows profiles common to multiple drivers.
>>>
>>> This causes an unexpected behavior to people who have upgraded from an
>>> earlier kernel because if multiple drivers have bound platform profile
>>> handlers they might not be able to access profiles they were expecting.
>>>
>>> Introduce a concept of a "hidden choice" that drivers can register and
>>> the platform profile handler code will utilize when using the legacy
>>> interface.
>>>
>>> There have been some other attempts at solving this issue in other ways.
>>> This serves as an alternative to those attempts.
>>>
>>> Link:
>>> https://lore.kernel.org/platform-driver-x86/e64b771e-3255-42ad-9257-5b8fc6c24ac9@gmx.de/T/#t
>>> Link:
>>> https://lore.kernel.org/platform-driver-x86/CAGwozwF-WVEgiAbWbRCiUaXf=BVa3KqmMJfs06trdMQHpTGmjQ@mail.gmail.com/T/#m2f3929e2d4f73cc0eedd14738170dad45232fd18
>>> Cc: Antheas Kapenekakis <lkml@...heas.dev>
>>> Cc: "Luke D. Jones" <luke@...nes.dev>
>>>
>>> Mario Limonciello (3):
>>>    ACPI: platform_profile: Add support for hidden choices
>>>    platform/x86/amd: pmf: Add 'quiet' to hidden choices
>>>    platform/x86/amd: pmf: Add balanced-performance to hidden choices
>>>
>>>   drivers/acpi/platform_profile.c    | 94 +++++++++++++++++++++++-------
>>>   drivers/platform/x86/amd/pmf/sps.c | 11 ++++
>>>   include/linux/platform_profile.h   |  3 +
>>>   3 files changed, 87 insertions(+), 21 deletions(-)
>>>
>>> -- 
>>> 2.43.0
>> 
>> The patches are all good - but my question is do we really need the whole hidden implementation bit?
>> 
>> If the options are not hidden, and someone chooses quiet or balanced-performance for the amd-pmf driver - does it really matter that it's going to do the same as low-power or performance?
>> 
>> So, same feedback as I had for Antheas's patches. I understand why this is being proposed but for me it is making things unnecessarily complicated.
>> 
>> My personal vote remains that the amd_pmf driver carries the superset to keep everyone happy (sorry - it sucks to be the CPU vendor that has to play nice with everyone).
>> 
>> Mark
>
> Well so the problem with having all of them is specifically what happens 
> when "only" amd-pmf is bound?
>
> If you advertise both "low power" and "quiet" it's really confusing to 
> userspace what the difference is.

Ah - I guess you get platforms without profile support where amd-pmf is the only thing. I hadn't considered that.

FWIW, I believe we (Lenovo) have both low power and quiet on Windows...and they really don't make much difference (which is why the thermal team didn't do them both on Linux). 
I don't know if Windows users are more or less confused (or maybe they've just abandoned all hope and are migrating to Linux...)

You have a better feeling as to how many issues you'll get raised if they behave the same, and have to support a wider ecosystem, so I'm happy to be over-ruled. I just wanted to wave my flag that I think the driver is getting too complicated. I'm slightly dreading having to debug customer issues at this point.

>
> The fact that it's actually 100% the same brings me to my personal 
> opinion on all of this.  Although I spent time writing up this series to 
> do it this way my "preference" is that we permanently alias "low power" 
> and "quiet" to one another and update all drivers to use "low power" 
> instead.
>

Guaranteed if you do that some vendor will have something that differentiates.

I can see having a 'use as much power as possible without needing fans' for quiet, and 'make the battery last as long as humanely possible whilst keeping the system usable' mode for low-power. Don't think anyone has done it...but they could.

> Granted that doesn't help the case of balance-performance being hidden 
> that Antheas mentioned for acer-wmi and legion-wmi but I don't know 
> serious of a problem that actually is.

I really have to go play with this on the Legion-Go. I need a time-machine.....

Mark