linux-kernel - Re: [UPDATE][PATCH] thermal: int340x: Mode setting with new OS handshake

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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0hT8UBDn3Lo5znFTao1bbjVUOgk8fPxkW9x4wC7XME1-A@mail.gmail.com>
Date:   Thu, 12 May 2022 12:30:19 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Amit Kucheria <amitk@...nel.org>,
        "Zhang, Rui" <rui.zhang@...el.com>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Stable <stable@...r.kernel.org>
Subject: Re: [UPDATE][PATCH] thermal: int340x: Mode setting with new OS handshake

On Wed, May 11, 2022 at 9:56 PM srinivas pandruvada
<srinivas.pandruvada@...ux.intel.com> wrote:
>
> On Wed, 2022-05-11 at 20:14 +0200, Rafael J. Wysocki wrote:
> > On Tue, May 10, 2022 at 8:22 PM Srinivas Pandruvada
> > <srinivas.pandruvada@...ux.intel.com> wrote:
> > >
> > > With the new OS handshake introduced with the commit: "c7ff29763989
> > > ("thermal: int340x: Update OS policy capability handshake")",
> > > thermal zone mode "enabled" doesn't work in the same way as the
> > > legacy
> > > handshake. The mode "enabled" fails with -EINVAL using new
> > > handshake.
> > >
> > > To address this issue, when the new OS UUID mask is set:
> > > - When mode is "enabled", return 0 as the firmware already has the
> > > latest policy mask.
> > > - When mode is "disabled", update the firmware with UUID mask of
> > > zero.
> > > In this way firmware can take control of the thermal control. Also
> > > reset the OS UUID mask. This allows user space to update with new
> > > set of policies.
> > >
> > > Fixes: c7ff29763989 ("thermal: int340x: Update OS policy capability
> > > handshake")
> > > Signed-off-by: Srinivas Pandruvada
> > > <srinivas.pandruvada@...ux.intel.com>
> > > Cc: stable@...r.kernel.org
> >
> > This is not -stable material yet.
> I thought it will wait for 5.19 merge window.

It is better to avoid making a major release with a known-broken
interface if possible.

> >
> > > ---
> > > update:
> > > Added Fixes tag
> > >
> > >
>
> [...]
>
> > > +               if (priv->os_uuid_mask) {
> > > +                       if (!enabled) {
> > > +                               priv->os_uuid_mask = 0;
> > > +                               result = set_os_uuid_mask(priv,
> > > priv->os_uuid_mask);
> >
> > This change worries me a bit, because it means replaying an already
> > established _OSC handshake which shouldn't be done by the spec.
> >
> I checked with the firmware team. The _OSC changes dynamically is
> validated and is recommended when enable/disable user space thermal
> control.

OK

> Looking at ACPI Spec
> "OSPM may evaluate _OSC multiple times to indicate changes in OSPM
> capability to the device but this may be precluded by specific device
> requirements"

Well, different paragraphs say different things, but it is fine as
long as the firmware and the OS are on the same page in the given use
case.

> > But I suppose you have tested this?
> I tested on TigerLake system.

OK

> >
> > > +                       }
> > > +
>
> [...]
>
> >
> > Patch applied as 5.18-rc material, but I've removed some unneeded
> > parens from the new code, so please double check the result in
> > bleeding-edge.
> I tested the patch from bleeding edge.
> Works fine.

Good.

I'll queue it up for -rc7 then (I will be offline for the next few
days, most likely, and I'm not ready to send a pull request today).

Thanks!