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: <d1a29343-fe37-4564-a48d-2ba4890100fe@kernel.org>
Date: Fri, 28 Feb 2025 13:56:21 -0600
From: Mario Limonciello <superm1@...nel.org>
To: Antheas Kapenekakis <lkml@...heas.dev>
Cc: Mark Pearson <mpearson-lenovo@...ebb.ca>,
 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>,
 "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>, 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 2/28/2025 13:53, Antheas Kapenekakis wrote:
> On Fri, 28 Feb 2025 at 20:45, Mario Limonciello <superm1@...nel.org> 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.
>>
>> 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.
>>
>> 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.
> 
> Hi Mark,
> So I agree with Mario here on that. I actually made the patch you
> suggested last Sunday [1].

My suggestion is actually more drastic than what you linked.  It's that 
we make a change in the core to special case the aliased strings, and 
then adjust all in-tree drivers to pick one or the other.

> 
> But then I looked at it and thought to myself that I can't ship this,
> so I did a v2, which is what I sent on Tuesday.
> 
> My priority here is to not disrupt the existing ABI with 6.14. This
> would allow extending this discussion by a couple of weeks, so a more
> permanent solution can be found.
> 
> If that would be the case, perhaps my patch series is more preferable
> as, since it is smaller, it would be cleaner to revert.
> 
> Antheas
> 
> [1] https://github.com/hhd-dev/patchwork/commit/aae724e8c90da3605bd131672fea07cd866af55f