linux-kernel - Re: [PATCH v2 0/4] platform/x86: Add Lenovo Gaming Series WMI Drivers

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: <CAFqHKTnMAcbtgkD-KncO0H-qvXfnBLwdnhSoH+rM_LCV85H_WA@mail.gmail.com>
Date: Sat, 11 Jan 2025 09:13:02 -0800
From: Derek John Clark <derekjohn.clark@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: Mario Limonciello <superm1@...nel.org>, Hans de Goede <hdegoede@...hat.com>, 
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	Jonathan Corbet <corbet@....net>, Luke Jones <luke@...nes.dev>, Xino Ni <nijs1@...ovo.com>, 
	Zhixin Zhang <zhangzx36@...ovo.com>, Mia Shao <shaohz1@...ovo.com>, 
	Mark Pearson <mpearson-lenovo@...ebb.ca>, 
	"Pierre-Loup A . Griffais" <pgriffais@...vesoftware.com>, "Cody T . -H . Chiu" <codyit@...il.com>, 
	John Martens <johnfanv2@...il.com>, platform-driver-x86@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/4] platform/x86: Add Lenovo Gaming Series WMI Drivers

On Fri, Jan 10, 2025 at 4:25 PM Armin Wolf <W_Armin@....de> wrote:
> >> I attached a prototype WMI driver for another device which had a similar problem.
> >> The solution was to provide a notifier so other event consumers can be notified
> >> when an WMI event was received.
> >>
> >> Example event consumer callback code:
> >>
> >>          static int uniwill_wmi_notify_call(struct notifier_block *nb, unsigned long action, void *data)
> >>          {
> >>                  if (action != UNIWILL_OSD_PERF_MODE_CHANGED)
> >>                          return NOTIFY_DONE;
> >>
> >>                  platform_profile_cycle();
> >>
> >>                  return NOTIFY_OK;
> >>          }
> >>
> >> I would also suggest that you use a notifier for communicating with the gamezone
> >> interface. Then you just have to submit commands (as action values) in the form of events
> >> which will then be processed by the available gamezone drivers (the result can be stored in *data).
> >>
> >> Those gamezone drivers can then return NOTIFY_STOP which will ensure that only a single gamezone
> >> driver can successfully process a given command.
> >>
> >> All in all the patch series seems to progress nicely. I am confident that we will solve the remaining issues.
> >>
> >> Thanks,
> >> Armin Wolf
> >>
> > That's a novel approach. There are some EVENT GUID's for the gamezone
> > interface I'll need to incorporate to keep everything in sync. These
> > devices have physical buttons (Fn+Q on laptops, Legion +Y button on
> > handhelds) to cycle the profiles. I didn't add this previously because
> > we were always updating it when called. I presume that each GUID will
> > need a separate driver for this. Any advice or examples on how to use
> > this to update the pprof in GameZone would be appreciated as I've
> > never used .notify before.
>
> The WMI driver inside the attachment should be a suitable starting point.
> You can also reuse the same driver for many different GUIDs and do the following:
>
> - use the context inside the wmi_device_id to find out which GUID is being probed.
> You can use drivers/platform/x86/xiaomi-wmi.c as an example.
>
> - inside the .notify callback parse the event data and the call the notifier.
> You can use the action parameter to signal which kind of WMI event was received (SMART_FAN_MODE_EVENT, ...)
> and the data parameter to pass the event data.
>
> With this you only need to provide a single WMI driver.
>
> The lenovo-wmi-gamezone driver can then register with this notifier and listen for
> platform profile changes:
>
>         static int lenovo_gz_notify_call(struct notifier_block *nb, unsigned long action, void *data)
>          {
>                  if (action != SMART_FAN_MODE_EVENT)    // Filter events
>                          return NOTIFY_DONE;
>
>                 <check *data if necessary>
>
>                  platform_profile_cycle();      // Cycle platform profile if necessary
>
>                  return NOTIFY_OK;
>          }
>

Makes sense, I'll get to work on it and if I have more questions I'll
reach out again.

Thanks Armin,
Derek

> >
> > My expected information flow will be these paths:
> > Physical Button press -> WMI event GUID notifier driver -> Gamezone
> > driver update & notify_call -> Other Mode save data to priv for lookup
> > when current_value is checked and return STOP .
> > or
> > platform-profile class write from sysfs -> Gamezone driver update &
> > notify_call ->Other Mode save data to priv for lookup when
> > current_value is checked and return STOP .
> >
> > Thanks,
> > Derek
>
> Your approach would have a problem: how to communicate the initial platform profile state
> when lenovo-wmi-other probes?
>
> I suggest that lenovo-wmi-gamezone stores the current platform profile. This value can then
> be retrieved by lenovo-wmi-other by using the special gamezone notifier. This would also allow
> lenovo-wmi-other to detect when lenovo-wmi-gamezone is not ready and can thus not provide
> platform profile data.
>
> Thanks,
> Armin Wolf
>