linux-kernel - Re: [PATCH v1 00/10] platform/x86: msi-wmi-platform: Add fan curves/platform profile/tdp/battery limiting

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: 
 <CAGwozwFJnR2aMhj6LJKU8aF+MDzF9FR21fXPPd7_=44M+KUJGg@mail.gmail.com>
Date: Mon, 12 May 2025 12:16:14 +0200
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Kurt Borja <kuurtb@...il.com>
Cc: platform-driver-x86@...r.kernel.org, Armin Wolf <W_Armin@....de>,
	Jonathan Corbet <corbet@....net>, Hans de Goede <hdegoede@...hat.com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Jean Delvare <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
 linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v1 00/10] platform/x86: msi-wmi-platform: Add fan
 curves/platform profile/tdp/battery limiting

On Mon, 12 May 2025 at 01:30, Kurt Borja <kuurtb@...il.com> wrote:
>
> Hi Antheas,
>
> On Sun May 11, 2025 at 5:44 PM -03, Antheas Kapenekakis wrote:
> > This draft patch series brings into parity the msi-wmi-platform driver with
> > the MSI Center M Windows application for the MSI Claw (all models).
> > Unfortunately, MSI Center M and this interface do not have a discovery API,
> > necessitating the introduction of a quirk system.
> >
> > While this patch series is fully functional and tested, there are still
> > some issues that need to be addressed:
> >   - Armin notes we need to disable fan curve support by default and quirk
> >     it as well, as it is not supported on all models. However, the way
> >     PWM enable ops work, this makes it a bit difficult, so I would like
> >     some suggestions on how to rework this.
>
> If I understood the question correctly, then you should control the
> visibility of all "curve" related attributes with the quirk.

Yep, this is what I was wondering. I will investigate this. It would
be good to get some comments on the quirk naming as well.

> The custom hwmon attribute_group has an is_visible callback, and so do
> the hwmon_ops.
>
> >   - It turns out that to fully disable the fan curve, we have to restore
> >     the default fan values. This is also what is done on the OEM software.
> >     For this, the last patch in the series is used, which is a bit dirty.
>
> I have a couple questions about this.
>
> * What are the default fan curves? Can these be statically defined?
> * Are user-defined fan curves persistent between reboots?
>
> I have some doubts about the approach you took on the last patch, but I
> want to understand how the platform works first.

So do I. Essentially here is how the Windows software works: when it
first opens, it saves the current curve in Windows registry. Then,
when the user sets a fan curve, it applies it in the same way we do
here and sets a bit in AP. When the custom curve is removed, it unsets
that bit and restores the original curve in WMI.

The logical reasoning would be that that bit controls the fan curve.
This is how it is named in the software. However, when setting that
bit on its own, it seems to only partially affect the fan curve. E.g.,
when the fan curve is 100% in all points, unsetting that bit makes it
go down to 50% when no load occurs. When using the default fan curve,
it goes to 0%. Therefore, it seems like that bit makes the fan curve
semi-autonomous?

The fan curve seems to be hardware specific and resets after reboots.
So a straightforward way to get it is to grab it on a fresh boot.

Antheas

> >
> > Sleep was tested with all values being preserved during S0iX (platform
> > profile, fan curve, PL1/PL2), so we do not need suspend/resume hooks, at
> > least for the Claw devices.
> >
> > For PL1/PL2, we use firmware-attributes. So for that I +cc Kurt since if
> > his new high level interface is merged beforehand, we can use that instead.
>
> Hopefully!
>
> --
>  ~ Kurt
>
> >
> > Antheas Kapenekakis (8):
> >   platform/x86: msi-wmi-platform: Add unlocked msi_wmi_platform_query
> >   platform/x86: msi-wmi-platform: Add quirk system
> >   platform/x86: msi-wmi-platform: Add platform profile through shift
> >     mode
> >   platform/x86: msi-wmi-platform: Add PL1/PL2 support via firmware
> >     attributes
> >   platform/x86: msi-wmi-platform: Add charge_threshold support
> >   platform/x86: msi-wmi-platform: Drop excess fans in dual fan devices
> >   platform/x86: msi-wmi-platform: Update header text
> >   platform/x86: msi-wmi-platform: Restore fan curves on PWM disable and
> >     unload
> >
> > Armin Wolf (2):
> >   platform/x86: msi-wmi-platform: Use input buffer for returning result
> >   platform/x86: msi-wmi-platform: Add support for fan control
> >
> >  .../wmi/devices/msi-wmi-platform.rst          |   26 +
> >  drivers/platform/x86/Kconfig                  |    3 +
> >  drivers/platform/x86/msi-wmi-platform.c       | 1181 ++++++++++++++++-
> >  3 files changed, 1156 insertions(+), 54 deletions(-)
> >
> >
> > base-commit: 62b1dcf2e7af3dc2879d1a39bf6823c99486a8c2
>