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: 
 <CAGwozwG8rGwwcNVwxC7zP+-pg2x=7ZA2VMTGKY6XF1arEAZhBA@mail.gmail.com>
Date: Fri, 30 May 2025 23:28:24 +0200
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Armin Wolf <W_Armin@....de>
Cc: platform-driver-x86@...r.kernel.org, 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>,
 Kurt Borja <kuurtb@...il.com>,
	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 Fri, 30 May 2025 at 23:16, Armin Wolf <W_Armin@....de> wrote:
>
> Am 30.05.25 um 22:50 schrieb Antheas Kapenekakis:
>
> > On Mon, 19 May 2025 at 04:38, Armin Wolf <W_Armin@....de> wrote:
> >> Am 11.05.25 um 22:44 schrieb Antheas Kapenekakis:
> >>
> >>> 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.
> >>>     - 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.
> >>>
> >>> 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.
> >> Overall the patch series looks promising, however the suspend/resume handling
> >> and the quirk system still needs some work.
> >>
> >> If you wish i can provide you with a patch for the EC-based quirk system. You
> >> can then structure your exiting patches around that.
> > Hi,
> > Sorry I have been busy with personal life. I will try to get back to
> > this in 1-2 weeks.
> >
> > I have three minor concerns that mirror each other with using an EC based check.
> >
> > 1) First is that we use boardname on the userspace side to check for
> > the Claw. Therefore, using the EC ID kernel side introduces a failure
> > point I am not very fond of. 2) Second is that collecting the IDs from
> > users might prove more difficult 3) userspace software from MSI uses
> > boardname as well.
>
> Actually the EC ID contains the board name (among other data). I envisioned that we
> rely on the board name reported by the EC instead of the board name reported over SMBIOS.
> This would allow us to better support model variations that share a common board name.
>
> Maybe we can still expose some data (EC ID, debugfs interface) even if a given board is
> not whitelisted. This way users can easily retrieve the EC ID with the board name even
> on unknown boards.

Would a hybrid approach be an option perhaps?

In my mind, Id say an info message in dmesg if the board is not
supported should be enough. That's what MSI-EC does. Are there any
other platform drivers that bind to EC ID?

Antheas

> Thanks,
> Armin Wolf
>
> > Could we use a hybrid approach perhaps? What do you think?
> >
> > Antheas
> >
> >> Thanks,
> >> Armin Wolf
> >>
> >>> 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