| 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
| ||
|
Message-ID: <b3a7ece2-c49c-5c5a-c53b-99acc10f68fc@linux.intel.com>
Date: Thu, 5 Dec 2024 17:22:55 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Mario Limonciello <mario.limonciello@....com>
cc: Hans de Goede <hdegoede@...hat.com>,
"Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Maximilian Luz <luzmaximilian@...il.com>, Lee Chun-Yi <jlee@...e.com>,
Shyam Sundar S K <Shyam-sundar.S-k@....com>,
Corentin Chary <corentin.chary@...il.com>,
"Luke D . Jones" <luke@...nes.dev>, Ike Panhc <ike.pan@...onical.com>,
Henrique de Moraes Holschuh <hmh@....eng.br>,
Alexis Belmonte <alexbelm48@...il.com>,
Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
Ai Chao <aichao@...inos.cn>, Gergo Koteles <soyer@....hu>,
open list <linux-kernel@...r.kernel.org>,
"open list:ACPI" <linux-acpi@...r.kernel.org>,
"open list:MICROSOFT SURFACE PLATFORM PROFILE DRIVER" <platform-driver-x86@...r.kernel.org>,
"open list:THINKPAD ACPI EXTRAS DRIVER" <ibm-acpi-devel@...ts.sourceforge.net>,
Mark Pearson <mpearson-lenovo@...ebb.ca>,
Matthew Schwartz <matthew.schwartz@...ux.dev>, Armin Wolf <W_Armin@....de>
Subject: Re: [PATCH v9 18/22] ACPI: platform_profile: Check all profile
handler to calculate next
On Thu, 5 Dec 2024, Mario Limonciello wrote:
> On 12/5/2024 08:22, Ilpo Järvinen wrote:
> > On Sun, 1 Dec 2024, Mario Limonciello wrote:
> >
> > > As multiple platform profile handlers might not all support the same
> > > profile, cycling to the next profile could have a different result
> > > depending on what handler are registered.
> > >
> > > Check what is active and supported by all handlers to decide what
> > > to do.
> > >
> > > Reviewed-by: Armin Wolf <W_Armin@....de>
> > > Tested-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
> > > Reviewed-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
> > > Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> > > ---
> > > drivers/acpi/platform_profile.c | 30 +++++++++++++++++++++---------
> > > 1 file changed, 21 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/acpi/platform_profile.c
> > > b/drivers/acpi/platform_profile.c
> > > index d5f0679d59d50..16746d9b9aa7c 100644
> > > --- a/drivers/acpi/platform_profile.c
> > > +++ b/drivers/acpi/platform_profile.c
> > > @@ -407,25 +407,37 @@ EXPORT_SYMBOL_GPL(platform_profile_notify);
> > > int platform_profile_cycle(void)
> > > {
> > > - enum platform_profile_option profile;
> > > - enum platform_profile_option next;
> > > + enum platform_profile_option next = PLATFORM_PROFILE_LAST;
> > > + enum platform_profile_option profile = PLATFORM_PROFILE_LAST;
> > > + unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
> > > int err;
> > > + set_bit(PLATFORM_PROFILE_LAST, choices);
> > > scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) {
> > > - if (!cur_profile)
> > > - return -ENODEV;
> > > + err = class_for_each_device(&platform_profile_class, NULL,
> > > + &profile, _aggregate_profiles);
> > > + if (err)
> > > + return err;
> > > - err = cur_profile->profile_get(cur_profile, &profile);
> > > + if (profile == PLATFORM_PROFILE_CUSTOM ||
> > > + profile == PLATFORM_PROFILE_LAST)
> > > + return -EINVAL;
> > > +
> > > + err = class_for_each_device(&platform_profile_class, NULL,
> > > + choices, _aggregate_choices);
> > > if (err)
> > > return err;
> > > - next = find_next_bit_wrap(cur_profile->choices,
> > > PLATFORM_PROFILE_LAST,
> > > + /* never iterate into a custom if all drivers supported it */
> > > + clear_bit(PLATFORM_PROFILE_CUSTOM, choices);
> >
> > I'm confused by the comment. I was under impression the custom "profile"
> > is just a framework construct when the _framework_ couldn't find a
> > consistent profile? How can a driver decide to "support it"? It sounds
> > like a driver overstepping its intended domain of operation.
> >
> > If the intention really is for the driver to "support" or "not support"
> > custom profile, then you should adjust the commit message of the patch
> > which introduced it.
> >
>
> Yes I had envisioned that a driver could potentially set custom as well.
>
> This idea was introduced by my RFC series that precluded doing the
> multiple driver handlers.
>
> The basic idea is that some drivers (for example asus-wmi and asus-armoury)
> have the ability for the user to change a sysfs file that represents sPPT or
> fPPT directly.
I recall that series.
> If this has been done they're "off the beating path" of a predfined
> profile because they're picking and choosing individual knobs.
The user would still not set it to "custom" nor driver "support" it,
right? But it's a consequence of tuning those other knobs? Or do you mean
user would first have to set "custom" and tuning the knobs is blocked
otherwise?
> So if a user touches those a driver could set profile as "custom" and if a
> user chooses the platform profile the driver will override all of those and
> report a pre-defined profile.
>
> So, yes I had that all in my mind but as you point out I definitely forgot to
> mention it in the commit messages.
>
> Do you agree with it? If so, I'll amend the next version where applicable
> (probably the patch that introduces custom and the documentation patch).
I'm a little worried about overloading the meaning from mere profile
disagreement to truly off the charted waters travel. Albeit, I suppose
that overloading is just between global "custom" vs per-driver "custom",
the latter would never be "custom" in case of mere profile disagreement,
if I've understood everything correctly?
--
i.
Powered by blists - more mailing lists