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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: 
 <CAGwozwFGx4n8NccKOJf1Bf1k7P8t_Xd3R4SHeij+s6X=NqBz9Q@mail.gmail.com>
Date: Fri, 28 Feb 2025 20:53:54 +0100
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Mario Limonciello <superm1@...nel.org>
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 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].

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