[RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[Armin Wolf]

Currently, the hp-wmi-sensors driver needs to be loaded manually
on supported machines. This however is unnecessary since the WMI
id table can be used to support autoloading.

However the driver might conflict with the hp-wmi driver since both
seem to use the same WMI GUID for registering notify handler.

I am thus submitting this patch as an RFC for now.

Armin Wolf (1):
  hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()

 drivers/hwmon/hp-wmi-sensors.c | 2 ++
 1 file changed, 2 insertions(+)

--
2.39.2

[RFC PATCH 1/1] hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()

[Armin Wolf]

Register the WMI id table with MODULE_DEVICE_TABLE() so that
the driver can automatically be leaded on supported machines.

Compile-tested only.

Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/hp-wmi-sensors.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
index b5325d0e72b9..493a3b3e86f3 100644
--- a/drivers/hwmon/hp-wmi-sensors.c
+++ b/drivers/hwmon/hp-wmi-sensors.c
@@ -25,6 +25,7 @@
 #include <linux/debugfs.h>
 #include <linux/hwmon.h>
 #include <linux/jiffies.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/nls.h>
 #include <linux/units.h>
@@ -2072,6 +2073,7 @@ static const struct wmi_device_id hp_wmi_sensors_id_table[] = {
 	{ HP_WMI_NUMERIC_SENSOR_GUID, NULL },
 	{},
 };
+MODULE_DEVICE_TABLE(wmi, hp_wmi_sensors_id_table);

 static struct wmi_driver hp_wmi_sensors_driver = {
 	.driver   = { .name = "hp-wmi-sensors" },
--
2.39.2

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[Guenter Roeck]

On 3/18/24 14:57, Armin Wolf wrote:
> Currently, the hp-wmi-sensors driver needs to be loaded manually
> on supported machines. This however is unnecessary since the WMI
> id table can be used to support autoloading.
> 
> However the driver might conflict with the hp-wmi driver since both
> seem to use the same WMI GUID for registering notify handler.
> 

I have no idea how this is supposed to be handled.
wmi_install_notify_handler() is marked as deprecated,
suggesting that the entire driver structure may be outdated.

> I am thus submitting this patch as an RFC for now.
> 

.... and, given the above, I have no idea what to do with it, sorry.

Guenter

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[James Seo]

On Mon, Mar 18, 2024 at 10:57:31PM +0100, Armin Wolf wrote:
> Currently, the hp-wmi-sensors driver needs to be loaded manually
> on supported machines. This however is unnecessary since the WMI
> id table can be used to support autoloading.
> 
> However the driver might conflict with the hp-wmi driver since both
> seem to use the same WMI GUID for registering notify handler.
> 
> I am thus submitting this patch as an RFC for now.
> 
> Armin Wolf (1):
>   hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()
> 
>  drivers/hwmon/hp-wmi-sensors.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> --
> 2.39.2
>

Autoloading was deliberately left out for now because of the GUID
conflict with hp-wmi's WMI notify handler.

HP's GUID reuse across product lines for different types of WMI
objects with different names and shapes means that with a patch like
this, many systems that should only load hp-wmi-sensors but not
hp-wmi will try to autoload both. (Perhaps all of them; I want to say
that the GUID 5FB7F034-2C63-45e9-BE91-3D44E2C707E4, which is the
second of the two GUIDs that hp-wmi uses to autoload, exists on every
HP system I've examined.)

Meanwhile, hp-wmi does various other platform things, and there's so
much hardware out there that who knows, maybe there are some systems
that really should load both. I don't think so but I can't rule it
out.

Unlike hp-wmi-sensors, hp-wmi doesn't survive failure to install its
notify handler, which sets up a potential race condition depending on
when hp-wmi and hp-wmi-sensors loads on a given system.

Therefore, I intended to add autoloading at the same time as
converting hp-wmi-sensors to use the bus-based WMI interface once
aggregate WMI devices are better supported.

As you mentioned [1], I ran into issues when I tried to do the
conversion by simply adding the GUID to struct wmi_driver.id_table.
That resulted in two separate independent instances of hp_wmi_sensors
being loaded, which isn't what I wanted.

[1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@gmx.de/

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[Armin Wolf]

Am 19.03.24 um 06:47 schrieb James Seo:

> On Mon, Mar 18, 2024 at 10:57:31PM +0100, Armin Wolf wrote:
>> Currently, the hp-wmi-sensors driver needs to be loaded manually
>> on supported machines. This however is unnecessary since the WMI
>> id table can be used to support autoloading.
>>
>> However the driver might conflict with the hp-wmi driver since both
>> seem to use the same WMI GUID for registering notify handler.
>>
>> I am thus submitting this patch as an RFC for now.
>>
>> Armin Wolf (1):
>>    hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()
>>
>>   drivers/hwmon/hp-wmi-sensors.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> --
>> 2.39.2
>>
> Autoloading was deliberately left out for now because of the GUID
> conflict with hp-wmi's WMI notify handler.
>
> HP's GUID reuse across product lines for different types of WMI
> objects with different names and shapes means that with a patch like
> this, many systems that should only load hp-wmi-sensors but not
> hp-wmi will try to autoload both. (Perhaps all of them; I want to say
> that the GUID 5FB7F034-2C63-45e9-BE91-3D44E2C707E4, which is the
> second of the two GUIDs that hp-wmi uses to autoload, exists on every
> HP system I've examined.)
>
> Meanwhile, hp-wmi does various other platform things, and there's so
> much hardware out there that who knows, maybe there are some systems
> that really should load both. I don't think so but I can't rule it
> out.
>
> Unlike hp-wmi-sensors, hp-wmi doesn't survive failure to install its
> notify handler, which sets up a potential race condition depending on
> when hp-wmi and hp-wmi-sensors loads on a given system.
>
> Therefore, I intended to add autoloading at the same time as
> converting hp-wmi-sensors to use the bus-based WMI interface once
> aggregate WMI devices are better supported.
>
> As you mentioned [1], I ran into issues when I tried to do the
> conversion by simply adding the GUID to struct wmi_driver.id_table.
> That resulted in two separate independent instances of hp_wmi_sensors
> being loaded, which isn't what I wanted.

After taking a look at a ACPI table dump of a HP machine, i noticed that
HPBIOS_BIOSEvent has the GUID 2B814318-4BE8-4707-9D84-A190A859B5D0, which is
different than the event GUID used by hp-wmi.

According your comment in hp_wmi_notify(), i assume that some machines have
mixed-up event GUIDs.

I thing it would be best to create a separate WMI driver for the event and
use a notifier chain (see include/linux/notifier.h) to distribute the event data.

In case of event GUID 95F24279-4D7B-4334-9387-ACCDC67EF61C, both hp-wmi and
hp-wmi-sensors can subscribe on this notifier and receive event data without
stepping on each other's toes.

The same can be done for the event GUID 2B814318-4BE8-4707-9D84-A190A859B5D0,
with a separate notifier chain.

This would decouple the event handling from the event data consumers, allowing
both hp-wmi and hp-wmi-sensors to coexist.

I can provide a prototype implementation, but unfortunately i have no HP machine
myself for testing. But i might be able to find one to test my changes.

Thanks,
Armin Wolf

>
> [1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@gmx.de/
>

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[James Seo]

On Tue, Mar 19, 2024 at 02:00:06PM +0100, Armin Wolf wrote:
> Am 19.03.24 um 06:47 schrieb James Seo:
> 
>> On Mon, Mar 18, 2024 at 10:57:31PM +0100, Armin Wolf wrote:
>>> Currently, the hp-wmi-sensors driver needs to be loaded manually
>>> on supported machines. This however is unnecessary since the WMI
>>> id table can be used to support autoloading.
>>> 
>>> However the driver might conflict with the hp-wmi driver since both
>>> seem to use the same WMI GUID for registering notify handler.
>>> 
>>> I am thus submitting this patch as an RFC for now.
>>> 
>>> Armin Wolf (1):
>>>    hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()
>>> 
>>>   drivers/hwmon/hp-wmi-sensors.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>> 
>>> --
>>> 2.39.2
>>> 
>> Autoloading was deliberately left out for now because of the GUID
>> conflict with hp-wmi's WMI notify handler.
>> 
>> HP's GUID reuse across product lines for different types of WMI
>> objects with different names and shapes means that with a patch like
>> this, many systems that should only load hp-wmi-sensors but not
>> hp-wmi will try to autoload both. (Perhaps all of them; I want to say
>> that the GUID 5FB7F034-2C63-45e9-BE91-3D44E2C707E4, which is the
>> second of the two GUIDs that hp-wmi uses to autoload, exists on every
>> HP system I've examined.)
>> 
>> Meanwhile, hp-wmi does various other platform things, and there's so
>> much hardware out there that who knows, maybe there are some systems
>> that really should load both. I don't think so but I can't rule it
>> out.
>> 
>> Unlike hp-wmi-sensors, hp-wmi doesn't survive failure to install its
>> notify handler, which sets up a potential race condition depending on
>> when hp-wmi and hp-wmi-sensors loads on a given system.
>> 
>> Therefore, I intended to add autoloading at the same time as
>> converting hp-wmi-sensors to use the bus-based WMI interface once
>> aggregate WMI devices are better supported.
>> 
>> As you mentioned [1], I ran into issues when I tried to do the
>> conversion by simply adding the GUID to struct wmi_driver.id_table.
>> That resulted in two separate independent instances of hp_wmi_sensors
>> being loaded, which isn't what I wanted.
> 
> After taking a look at a ACPI table dump of a HP machine, i noticed that
> HPBIOS_BIOSEvent has the GUID 2B814318-4BE8-4707-9D84-A190A859B5D0, which is
> different than the event GUID used by hp-wmi.
> 
> According your comment in hp_wmi_notify(), i assume that some machines have
> mixed-up event GUIDs.

I investigated further. Every HP machine in the Linux Hardware Database that
has \\.\root\WMI\hpqBEvnt at 95F24279-4D7B-4334-9387-ACCDC67EF61C also has
\\.\root\WMI\HPBIOS_BIOSEvent at 2B814318-4BE8-4707-9D84-A190A859B5D0.

> I thing it would be best to create a separate WMI driver for the event and
> use a notifier chain (see include/linux/notifier.h) to distribute the event data.
> 
> In case of event GUID 95F24279-4D7B-4334-9387-ACCDC67EF61C, both hp-wmi and
> hp-wmi-sensors can subscribe on this notifier and receive event data without
> stepping on each other's toes.
> 
> The same can be done for the event GUID 2B814318-4BE8-4707-9D84-A190A859B5D0,
> with a separate notifier chain.
> 
> This would decouple the event handling from the event data consumers, allowing
> both hp-wmi and hp-wmi-sensors to coexist.

No objections from me for this specific use case to work around the GUID conflict.
hp-wmi-sensors should indeed subscribe on 2B814318-4BE8-4707-9D84-A190A859B5D0
for some of those machines.

Any ideas for getting rid of wmi_query_block() for fetching
\\.\root\HP\InstrumentedBIOS\HPBIOS_PlatformEvents? I know other drivers are
also using it for getting blocks other than their "main" GUID.

> I can provide a prototype implementation, but unfortunately i have no HP machine
> myself for testing. But i might be able to find one to test my changes.

Happy to test. (Also happy to try it myself, but I'd need some help.)

> Thanks,
> Armin Wolf
> 
>> 
>> [1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@gmx.de/
>> 

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[Armin Wolf]

Am 20.03.24 um 01:40 schrieb James Seo:

> On Tue, Mar 19, 2024 at 02:00:06PM +0100, Armin Wolf wrote:
>> Am 19.03.24 um 06:47 schrieb James Seo:
>>
>>> On Mon, Mar 18, 2024 at 10:57:31PM +0100, Armin Wolf wrote:
>>>> Currently, the hp-wmi-sensors driver needs to be loaded manually
>>>> on supported machines. This however is unnecessary since the WMI
>>>> id table can be used to support autoloading.
>>>>
>>>> However the driver might conflict with the hp-wmi driver since both
>>>> seem to use the same WMI GUID for registering notify handler.
>>>>
>>>> I am thus submitting this patch as an RFC for now.
>>>>
>>>> Armin Wolf (1):
>>>>     hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()
>>>>
>>>>    drivers/hwmon/hp-wmi-sensors.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> --
>>>> 2.39.2
>>>>
>>> Autoloading was deliberately left out for now because of the GUID
>>> conflict with hp-wmi's WMI notify handler.
>>>
>>> HP's GUID reuse across product lines for different types of WMI
>>> objects with different names and shapes means that with a patch like
>>> this, many systems that should only load hp-wmi-sensors but not
>>> hp-wmi will try to autoload both. (Perhaps all of them; I want to say
>>> that the GUID 5FB7F034-2C63-45e9-BE91-3D44E2C707E4, which is the
>>> second of the two GUIDs that hp-wmi uses to autoload, exists on every
>>> HP system I've examined.)
>>>
>>> Meanwhile, hp-wmi does various other platform things, and there's so
>>> much hardware out there that who knows, maybe there are some systems
>>> that really should load both. I don't think so but I can't rule it
>>> out.
>>>
>>> Unlike hp-wmi-sensors, hp-wmi doesn't survive failure to install its
>>> notify handler, which sets up a potential race condition depending on
>>> when hp-wmi and hp-wmi-sensors loads on a given system.
>>>
>>> Therefore, I intended to add autoloading at the same time as
>>> converting hp-wmi-sensors to use the bus-based WMI interface once
>>> aggregate WMI devices are better supported.
>>>
>>> As you mentioned [1], I ran into issues when I tried to do the
>>> conversion by simply adding the GUID to struct wmi_driver.id_table.
>>> That resulted in two separate independent instances of hp_wmi_sensors
>>> being loaded, which isn't what I wanted.
>> After taking a look at a ACPI table dump of a HP machine, i noticed that
>> HPBIOS_BIOSEvent has the GUID 2B814318-4BE8-4707-9D84-A190A859B5D0, which is
>> different than the event GUID used by hp-wmi.
>>
>> According your comment in hp_wmi_notify(), i assume that some machines have
>> mixed-up event GUIDs.
> I investigated further. Every HP machine in the Linux Hardware Database that
> has \\.\root\WMI\hpqBEvnt at 95F24279-4D7B-4334-9387-ACCDC67EF61C also has
> \\.\root\WMI\HPBIOS_BIOSEvent at 2B814318-4BE8-4707-9D84-A190A859B5D0.

Could it be that using 95F24279-4D7B-4334-9387-ACCDC67EF61C is a mistake?
Or do you know of a machine which indeed uses this GUID to deliver sensor events?
Because it not, then we can just avoid this GUID conflict entirely by using the
other GUID.

>> I thing it would be best to create a separate WMI driver for the event and
>> use a notifier chain (see include/linux/notifier.h) to distribute the event data.
>>
>> In case of event GUID 95F24279-4D7B-4334-9387-ACCDC67EF61C, both hp-wmi and
>> hp-wmi-sensors can subscribe on this notifier and receive event data without
>> stepping on each other's toes.
>>
>> The same can be done for the event GUID 2B814318-4BE8-4707-9D84-A190A859B5D0,
>> with a separate notifier chain.
>>
>> This would decouple the event handling from the event data consumers, allowing
>> both hp-wmi and hp-wmi-sensors to coexist.
> No objections from me for this specific use case to work around the GUID conflict.
> hp-wmi-sensors should indeed subscribe on 2B814318-4BE8-4707-9D84-A190A859B5D0
> for some of those machines.
>
> Any ideas for getting rid of wmi_query_block() for fetching
> \\.\root\HP\InstrumentedBIOS\HPBIOS_PlatformEvents? I know other drivers are
> also using it for getting blocks other than their "main" GUID.

Good question, it seems that HPBIOS_PlatformEvents is optional, so using the component
framework will not work.

If those WMI data blocks are always associated with the same ACPI device used by the
sensors GUID, then maybe i could create some sort of API for checking if a given GUID
exists the ACPI device associated with a WMI device.

However i thing the event GUID issue is more important right now.

Thanks,
Armin Wolf

>> I can provide a prototype implementation, but unfortunately i have no HP machine
>> myself for testing. But i might be able to find one to test my changes.
> Happy to test. (Also happy to try it myself, but I'd need some help.)
>
>> Thanks,
>> Armin Wolf
>>
>>> [1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@gmx.de/
>>>

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[James Seo]

On Wed, Mar 20, 2024 at 04:13:59PM +0100, Armin Wolf wrote:
> Am 20.03.24 um 01:40 schrieb James Seo:
> 
>> On Tue, Mar 19, 2024 at 02:00:06PM +0100, Armin Wolf wrote:
>>> Am 19.03.24 um 06:47 schrieb James Seo:
>>> 
>>>> On Mon, Mar 18, 2024 at 10:57:31PM +0100, Armin Wolf wrote:
>>>>> Currently, the hp-wmi-sensors driver needs to be loaded manually
>>>>> on supported machines. This however is unnecessary since the WMI
>>>>> id table can be used to support autoloading.
>>>>> 
>>>>> However the driver might conflict with the hp-wmi driver since both
>>>>> seem to use the same WMI GUID for registering notify handler.
>>>>> 
>>>>> I am thus submitting this patch as an RFC for now.
>>>>> 
>>>>> Armin Wolf (1):
>>>>>     hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()
>>>>> 
>>>>>    drivers/hwmon/hp-wmi-sensors.c | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>> 
>>>>> --
>>>>> 2.39.2
>>>>> 
>>>> Autoloading was deliberately left out for now because of the GUID
>>>> conflict with hp-wmi's WMI notify handler.
>>>> 
>>>> HP's GUID reuse across product lines for different types of WMI
>>>> objects with different names and shapes means that with a patch like
>>>> this, many systems that should only load hp-wmi-sensors but not
>>>> hp-wmi will try to autoload both. (Perhaps all of them; I want to say
>>>> that the GUID 5FB7F034-2C63-45e9-BE91-3D44E2C707E4, which is the
>>>> second of the two GUIDs that hp-wmi uses to autoload, exists on every
>>>> HP system I've examined.)
>>>> 
>>>> Meanwhile, hp-wmi does various other platform things, and there's so
>>>> much hardware out there that who knows, maybe there are some systems
>>>> that really should load both. I don't think so but I can't rule it
>>>> out.
>>>> 
>>>> Unlike hp-wmi-sensors, hp-wmi doesn't survive failure to install its
>>>> notify handler, which sets up a potential race condition depending on
>>>> when hp-wmi and hp-wmi-sensors loads on a given system.
>>>> 
>>>> Therefore, I intended to add autoloading at the same time as
>>>> converting hp-wmi-sensors to use the bus-based WMI interface once
>>>> aggregate WMI devices are better supported.
>>>> 
>>>> As you mentioned [1], I ran into issues when I tried to do the
>>>> conversion by simply adding the GUID to struct wmi_driver.id_table.
>>>> That resulted in two separate independent instances of hp_wmi_sensors
>>>> being loaded, which isn't what I wanted.
>>> After taking a look at a ACPI table dump of a HP machine, i noticed that
>>> HPBIOS_BIOSEvent has the GUID 2B814318-4BE8-4707-9D84-A190A859B5D0, which is
>>> different than the event GUID used by hp-wmi.
>>> 
>>> According your comment in hp_wmi_notify(), i assume that some machines have
>>> mixed-up event GUIDs.
>> I investigated further. Every HP machine in the Linux Hardware Database that
>> has \\.\root\WMI\hpqBEvnt at 95F24279-4D7B-4334-9387-ACCDC67EF61C also has
>> \\.\root\WMI\HPBIOS_BIOSEvent at 2B814318-4BE8-4707-9D84-A190A859B5D0.
> 
> Could it be that using 95F24279-4D7B-4334-9387-ACCDC67EF61C is a mistake?
> Or do you know of a machine which indeed uses this GUID to deliver sensor events?
> Because it not, then we can just avoid this GUID conflict entirely by using the
> other GUID.
>

No, it's not a mistake, it's HP reusing GUIDs. Both my test machines use
95F24279-4D7B-4334-9387-ACCDC67EF61C for \\.\root\WMI\HPBIOS_BIOSEvent.

Previously I examined a sample of ACPI dumps from machines with both
hpqBEvnt and HPBIOS_BIOSEvent, and concluded:

  - hpqBEvnt is for various events on both business and non-business
    machines that are of no interest to hp-wmi-sensors (e.g. hotkeys)

  - some machines with hpqBEvnt also have HPBIOS_BIOSEvent at GUID
    2B814318-4BE8-4707-9D84-A190A859B5D0

  - no machines with both hpqBEvnt and HPBIOS_BIOSEvent actually surface
    relevant sensor events (e.g. fan speed too high) via HPBIOS_BIOSEvent;
    they only surface non-sensor events (e.g. BIOS setting was changed)
    that are of no interest to hp-wmi-sensors

  - therefore, 2B814318-4BE8-4707-9D84-A190A859B5D0 does not need to be
    handled in hp-wmi-sensors

But this time I have done an exhaustive examination and concluded that a
few machines with both events do surface sensor events via HPBIOS_BIOSEvent.

>>> I thing it would be best to create a separate WMI driver for the event and
>>> use a notifier chain (see include/linux/notifier.h) to distribute the event data.
>>> 
>>> In case of event GUID 95F24279-4D7B-4334-9387-ACCDC67EF61C, both hp-wmi and
>>> hp-wmi-sensors can subscribe on this notifier and receive event data without
>>> stepping on each other's toes.
>>> 
>>> The same can be done for the event GUID 2B814318-4BE8-4707-9D84-A190A859B5D0,
>>> with a separate notifier chain.
>>> 
>>> This would decouple the event handling from the event data consumers, allowing
>>> both hp-wmi and hp-wmi-sensors to coexist.
>> No objections from me for this specific use case to work around the GUID conflict.
>> hp-wmi-sensors should indeed subscribe on 2B814318-4BE8-4707-9D84-A190A859B5D0
>> for some of those machines.
>> 
>> Any ideas for getting rid of wmi_query_block() for fetching
>> \\.\root\HP\InstrumentedBIOS\HPBIOS_PlatformEvents? I know other drivers are
>> also using it for getting blocks other than their "main" GUID.
> 
> Good question, it seems that HPBIOS_PlatformEvents is optional, so using the component
> framework will not work.
> 

Yes, HPBIOS_PlatformEvents is optional, but it's pretty much necessary for
alarm and intrusion events. Without it, it's not possible to know whether a
machine even reports such events until after they occur (rare). We'd have
to assume that all machines always support such events.

> If those WMI data blocks are always associated with the same ACPI device used by the
> sensors GUID, then maybe i could create some sort of API for checking if a given GUID
> exists the ACPI device associated with a WMI device.

For all HP machines in the Linux Hardware Database, all machines with
HPBIOS_PlatformEvents also have HPBIOS_BIOSNumericSensor. The reverse is
not true. Neither WMI object appears under multiple GUIDs.

> However i thing the event GUID issue is more important right now.

Sure. I also wonder if your idea could be expanded into a generic driver
for publishing simple WMI events. This would be usable in other drivers
that are currently using legacy handlers for receiving event data.

More broadly, if hp-wmi-drivers is any indication, aggregate WMI devices
could be a pain. Primary WMI object, associated WMI objects (optional or
mandatory), multiple aggregate devices allowed to bind to the same
objects. And if using GUIDs for identification, multiple allowable GUIDs.

Thanks,

James

> Thanks,
> Armin Wolf
> 
>>> I can provide a prototype implementation, but unfortunately i have no HP machine
>>> myself for testing. But i might be able to find one to test my changes.
>> Happy to test. (Also happy to try it myself, but I'd need some help.)
>> 
>>> Thanks,
>>> Armin Wolf
>>> 
>>>> [1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@gmx.de/
>>>> 

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[Armin Wolf]

Am 20.03.24 um 21:11 schrieb James Seo:

> On Wed, Mar 20, 2024 at 04:13:59PM +0100, Armin Wolf wrote:
>> Am 20.03.24 um 01:40 schrieb James Seo:
>>
>>> On Tue, Mar 19, 2024 at 02:00:06PM +0100, Armin Wolf wrote:
>>>> Am 19.03.24 um 06:47 schrieb James Seo:
>>>>
>>>>> On Mon, Mar 18, 2024 at 10:57:31PM +0100, Armin Wolf wrote:
>>>>>> Currently, the hp-wmi-sensors driver needs to be loaded manually
>>>>>> on supported machines. This however is unnecessary since the WMI
>>>>>> id table can be used to support autoloading.
>>>>>>
>>>>>> However the driver might conflict with the hp-wmi driver since both
>>>>>> seem to use the same WMI GUID for registering notify handler.
>>>>>>
>>>>>> I am thus submitting this patch as an RFC for now.
>>>>>>
>>>>>> Armin Wolf (1):
>>>>>>      hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()
>>>>>>
>>>>>>     drivers/hwmon/hp-wmi-sensors.c | 2 ++
>>>>>>     1 file changed, 2 insertions(+)
>>>>>>
>>>>>> --
>>>>>> 2.39.2
>>>>>>
>>>>> Autoloading was deliberately left out for now because of the GUID
>>>>> conflict with hp-wmi's WMI notify handler.
>>>>>
>>>>> HP's GUID reuse across product lines for different types of WMI
>>>>> objects with different names and shapes means that with a patch like
>>>>> this, many systems that should only load hp-wmi-sensors but not
>>>>> hp-wmi will try to autoload both. (Perhaps all of them; I want to say
>>>>> that the GUID 5FB7F034-2C63-45e9-BE91-3D44E2C707E4, which is the
>>>>> second of the two GUIDs that hp-wmi uses to autoload, exists on every
>>>>> HP system I've examined.)
>>>>>
>>>>> Meanwhile, hp-wmi does various other platform things, and there's so
>>>>> much hardware out there that who knows, maybe there are some systems
>>>>> that really should load both. I don't think so but I can't rule it
>>>>> out.
>>>>>
>>>>> Unlike hp-wmi-sensors, hp-wmi doesn't survive failure to install its
>>>>> notify handler, which sets up a potential race condition depending on
>>>>> when hp-wmi and hp-wmi-sensors loads on a given system.
>>>>>
>>>>> Therefore, I intended to add autoloading at the same time as
>>>>> converting hp-wmi-sensors to use the bus-based WMI interface once
>>>>> aggregate WMI devices are better supported.
>>>>>
>>>>> As you mentioned [1], I ran into issues when I tried to do the
>>>>> conversion by simply adding the GUID to struct wmi_driver.id_table.
>>>>> That resulted in two separate independent instances of hp_wmi_sensors
>>>>> being loaded, which isn't what I wanted.
>>>> After taking a look at a ACPI table dump of a HP machine, i noticed that
>>>> HPBIOS_BIOSEvent has the GUID 2B814318-4BE8-4707-9D84-A190A859B5D0, which is
>>>> different than the event GUID used by hp-wmi.
>>>>
>>>> According your comment in hp_wmi_notify(), i assume that some machines have
>>>> mixed-up event GUIDs.
>>> I investigated further. Every HP machine in the Linux Hardware Database that
>>> has \\.\root\WMI\hpqBEvnt at 95F24279-4D7B-4334-9387-ACCDC67EF61C also has
>>> \\.\root\WMI\HPBIOS_BIOSEvent at 2B814318-4BE8-4707-9D84-A190A859B5D0.
>> Could it be that using 95F24279-4D7B-4334-9387-ACCDC67EF61C is a mistake?
>> Or do you know of a machine which indeed uses this GUID to deliver sensor events?
>> Because it not, then we can just avoid this GUID conflict entirely by using the
>> other GUID.
>>
> No, it's not a mistake, it's HP reusing GUIDs. Both my test machines use
> 95F24279-4D7B-4334-9387-ACCDC67EF61C for \\.\root\WMI\HPBIOS_BIOSEvent.
>
> Previously I examined a sample of ACPI dumps from machines with both
> hpqBEvnt and HPBIOS_BIOSEvent, and concluded:
>
>    - hpqBEvnt is for various events on both business and non-business
>      machines that are of no interest to hp-wmi-sensors (e.g. hotkeys)
>
>    - some machines with hpqBEvnt also have HPBIOS_BIOSEvent at GUID
>      2B814318-4BE8-4707-9D84-A190A859B5D0
>
>    - no machines with both hpqBEvnt and HPBIOS_BIOSEvent actually surface
>      relevant sensor events (e.g. fan speed too high) via HPBIOS_BIOSEvent;
>      they only surface non-sensor events (e.g. BIOS setting was changed)
>      that are of no interest to hp-wmi-sensors
>
>    - therefore, 2B814318-4BE8-4707-9D84-A190A859B5D0 does not need to be
>      handled in hp-wmi-sensors
>
> But this time I have done an exhaustive examination and concluded that a
> few machines with both events do surface sensor events via HPBIOS_BIOSEvent.

This would have interesting implications for the WMI subsystem. Can you send me
the output of "acpidump" on those test machines?

It also seems that there are machines which do use the other GUID, see here:
https://github.com/lm-sensors/lm-sensors/issues/471 (acpidump at the bottom)

>>>> I thing it would be best to create a separate WMI driver for the event and
>>>> use a notifier chain (see include/linux/notifier.h) to distribute the event data.
>>>>
>>>> In case of event GUID 95F24279-4D7B-4334-9387-ACCDC67EF61C, both hp-wmi and
>>>> hp-wmi-sensors can subscribe on this notifier and receive event data without
>>>> stepping on each other's toes.
>>>>
>>>> The same can be done for the event GUID 2B814318-4BE8-4707-9D84-A190A859B5D0,
>>>> with a separate notifier chain.
>>>>
>>>> This would decouple the event handling from the event data consumers, allowing
>>>> both hp-wmi and hp-wmi-sensors to coexist.
>>> No objections from me for this specific use case to work around the GUID conflict.
>>> hp-wmi-sensors should indeed subscribe on 2B814318-4BE8-4707-9D84-A190A859B5D0
>>> for some of those machines.
>>>
>>> Any ideas for getting rid of wmi_query_block() for fetching
>>> \\.\root\HP\InstrumentedBIOS\HPBIOS_PlatformEvents? I know other drivers are
>>> also using it for getting blocks other than their "main" GUID.
>> Good question, it seems that HPBIOS_PlatformEvents is optional, so using the component
>> framework will not work.
>>
> Yes, HPBIOS_PlatformEvents is optional, but it's pretty much necessary for
> alarm and intrusion events. Without it, it's not possible to know whether a
> machine even reports such events until after they occur (rare). We'd have
> to assume that all machines always support such events.
>
>> If those WMI data blocks are always associated with the same ACPI device used by the
>> sensors GUID, then maybe i could create some sort of API for checking if a given GUID
>> exists the ACPI device associated with a WMI device.
> For all HP machines in the Linux Hardware Database, all machines with
> HPBIOS_PlatformEvents also have HPBIOS_BIOSNumericSensor. The reverse is
> not true. Neither WMI object appears under multiple GUIDs.
>
>> However i thing the event GUID issue is more important right now.
> Sure. I also wonder if your idea could be expanded into a generic driver
> for publishing simple WMI events. This would be usable in other drivers
> that are currently using legacy handlers for receiving event data.

You are completely right, this driver could allow clients to register a
notifier block for a event identified by a GUID, and this notifier block
is then called every time this event is received.

> More broadly, if hp-wmi-drivers is any indication, aggregate WMI devices
> could be a pain. Primary WMI object, associated WMI objects (optional or
> mandatory), multiple aggregate devices allowed to bind to the same
> objects. And if using GUIDs for identification, multiple allowable GUIDs.

I agree, part of it stems from many OEMs designing their interfaces in such
a way that it is impossible to discover if some optional features are present.

I suspect this happens because under Windows, the OEMs just check all GUIDs
once the system has "finished booting" (aka reached the login screen), and
this is afaik not possible with device drivers.

However we cannot export WMI method/events/etc to userspace, as this would
be a security nightmare (random RPC with buggy ACPI firmware, yay!).

In the future we might need an API for at least discovering and interacting
with WMI devices backed by the same ACPI device, however this might take some
time.

I will focus on this WMI event driver for now.

Thanks,
Armin Wolf

> Thanks,
>
> James
>
>> Thanks,
>> Armin Wolf
>>
>>>> I can provide a prototype implementation, but unfortunately i have no HP machine
>>>> myself for testing. But i might be able to find one to test my changes.
>>> Happy to test. (Also happy to try it myself, but I'd need some help.)
>>>
>>>> Thanks,
>>>> Armin Wolf
>>>>
>>>>> [1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@gmx.de/
>>>>>

Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

[James Seo]

>>> Could it be that using 95F24279-4D7B-4334-9387-ACCDC67EF61C is a mistake?
>>> Or do you know of a machine which indeed uses this GUID to deliver sensor events?
>>> Because it not, then we can just avoid this GUID conflict entirely by using the
>>> other GUID.
>>> 
>> No, it's not a mistake, it's HP reusing GUIDs. Both my test machines use
>> 95F24279-4D7B-4334-9387-ACCDC67EF61C for \\.\root\WMI\HPBIOS_BIOSEvent.
>> 
>> Previously I examined a sample of ACPI dumps from machines with both
>> hpqBEvnt and HPBIOS_BIOSEvent, and concluded:
>> 
>>    - hpqBEvnt is for various events on both business and non-business
>>      machines that are of no interest to hp-wmi-sensors (e.g. hotkeys)
>> 
>>    - some machines with hpqBEvnt also have HPBIOS_BIOSEvent at GUID
>>      2B814318-4BE8-4707-9D84-A190A859B5D0
>> 
>>    - no machines with both hpqBEvnt and HPBIOS_BIOSEvent actually surface
>>      relevant sensor events (e.g. fan speed too high) via HPBIOS_BIOSEvent;
>>      they only surface non-sensor events (e.g. BIOS setting was changed)
>>      that are of no interest to hp-wmi-sensors
>> 
>>    - therefore, 2B814318-4BE8-4707-9D84-A190A859B5D0 does not need to be
>>      handled in hp-wmi-sensors
>> 
>> But this time I have done an exhaustive examination and concluded that a
>> few machines with both events do surface sensor events via HPBIOS_BIOSEvent.
> 
> This would have interesting implications for the WMI subsystem. Can you send me
> the output of "acpidump" on those test machines?
>
> It also seems that there are machines which do use the other GUID, see here:
> https://github.com/lm-sensors/lm-sensors/issues/471 (acpidump at the bottom)

Here are examples of machines in the Linux Hardware Database with
HPBIOS_BIOSEvent at 95F24279-4D7B-4334-9387-ACCDC67EF61C:

Compaq 8100 Elite SFF PC
EliteDesk 800 G1 SFF
Z400 Workstation

https://github.com/linuxhw/ACPI/blob/master/Desktop/Hewlett-Packard/Compaq/Compaq%208100%20Elite%20SFF%20PC/AB6EADEE22B9
https://github.com/linuxhw/ACPI/blob/master/Desktop/Hewlett-Packard/EliteDesk/EliteDesk%20800%20G1%20SFF/F13506CA489E
https://github.com/linuxhw/ACPI/blob/master/Desktop/Hewlett-Packard/Z400/Z400%20Workstation/FF7C21B8CB39

Here are examples of machines that have hpqBEvnt and HPBIOS_BIOSEvent at
95F24279-4D7B-4334-9387-ACCDC67EF61C and 2B814318-4BE8-4707-9D84-A190A859B5D0
but do not surface sensor events via HPBIOS_BIOSEvent. See PEVT and how it
dereferences into CBWE, which is HPBIOS_PlatformEvents giving information
about the types of _WED/HPBIOS_BIOSEvent that may occur:

ZBook 2015 G4
ZBook Studio G5

https://github.com/linuxhw/ACPI/blob/master/Notebook/Hewlett-Packard/ZBook/ZBook%2015%20G4/89F9520CDC60
https://github.com/linuxhw/ACPI/blob/master/Notebook/Hewlett-Packard/ZBook/ZBook%20Studio%20G5/39AAE603D78B

An example of a machine that has hpqBEvnt and HPBIOS_BIOSEvent at
95F24279-4D7B-4334-9387-ACCDC67EF61C and 2B814318-4BE8-4707-9D84-A190A859B5D0
and does surface sensor events via HPBIOS_BIOSEvent is the ProDesk 405 G6
from https://github.com/lm-sensors/lm-sensors/issues/471 in your previous
message. I found a reason to reply to the GitHub issue with the decompiled
ASL for easy reference. See PEVT and how it dereferences into EVNT, which
is HPBIOS_PlatformEvents giving information about the types of
_WED/HPBIOS_BIOSEvent that may occur. Currently hp-wmi-sensors would not
recognize that this BIOS supports reporting alarm and intrusion events.

And finally, a machine that embodies "weird behavior from a HP BIOS". It has
hpqBEvnt and HPBIOS_BIOSEvent at 95F24279-4D7B-4334-9387-ACCDC67EF61C and
2B814318-4BE8-4707-9D84-A190A859B5D0, and has HPBIOS_BIOSNumericSensor at
8F1F6435-9F42-42C8-BADC-0E9424F20C9A. However, it looks like it just
defines these in BMOF without providing any instances during runtime, or
maybe it generates them in some convoluted non-obvious way:

ZHAN 99 Mobile Workstation G1

https://github.com/linuxhw/ACPI/blob/master/Notebook/Hewlett-Packard/ZHAN/ZHAN%2099%20Mobile%20Workstation%20G1/CB1EEAC36F91

>>>>> I thing it would be best to create a separate WMI driver for the event and
>>>>> use a notifier chain (see include/linux/notifier.h) to distribute the event data.
>>>>> 
>>>>> In case of event GUID 95F24279-4D7B-4334-9387-ACCDC67EF61C, both hp-wmi and
>>>>> hp-wmi-sensors can subscribe on this notifier and receive event data without
>>>>> stepping on each other's toes.
>>>>> 
>>>>> The same can be done for the event GUID 2B814318-4BE8-4707-9D84-A190A859B5D0,
>>>>> with a separate notifier chain.
>>>>> 
>>>>> This would decouple the event handling from the event data consumers, allowing
>>>>> both hp-wmi and hp-wmi-sensors to coexist.
>>>> No objections from me for this specific use case to work around the GUID conflict.
>>>> hp-wmi-sensors should indeed subscribe on 2B814318-4BE8-4707-9D84-A190A859B5D0
>>>> for some of those machines.
>>>> 
>>>> Any ideas for getting rid of wmi_query_block() for fetching
>>>> \\.\root\HP\InstrumentedBIOS\HPBIOS_PlatformEvents? I know other drivers are
>>>> also using it for getting blocks other than their "main" GUID.
>>> Good question, it seems that HPBIOS_PlatformEvents is optional, so using the component
>>> framework will not work.
>>> 
>> Yes, HPBIOS_PlatformEvents is optional, but it's pretty much necessary for
>> alarm and intrusion events. Without it, it's not possible to know whether a
>> machine even reports such events until after they occur (rare). We'd have
>> to assume that all machines always support such events.
>> 
>>> If those WMI data blocks are always associated with the same ACPI device used by the
>>> sensors GUID, then maybe i could create some sort of API for checking if a given GUID
>>> exists the ACPI device associated with a WMI device.
>> For all HP machines in the Linux Hardware Database, all machines with
>> HPBIOS_PlatformEvents also have HPBIOS_BIOSNumericSensor. The reverse is
>> not true. Neither WMI object appears under multiple GUIDs.
>> 
>>> However i thing the event GUID issue is more important right now.
>> Sure. I also wonder if your idea could be expanded into a generic driver
>> for publishing simple WMI events. This would be usable in other drivers
>> that are currently using legacy handlers for receiving event data.
> 
> You are completely right, this driver could allow clients to register a
> notifier block for a event identified by a GUID, and this notifier block
> is then called every time this event is received.
> 
>> More broadly, if hp-wmi-drivers is any indication, aggregate WMI devices
>> could be a pain. Primary WMI object, associated WMI objects (optional or
>> mandatory), multiple aggregate devices allowed to bind to the same
>> objects. And if using GUIDs for identification, multiple allowable GUIDs.
> 
> I agree, part of it stems from many OEMs designing their interfaces in such
> a way that it is impossible to discover if some optional features are present.
> 
> I suspect this happens because under Windows, the OEMs just check all GUIDs
> once the system has "finished booting" (aka reached the login screen), and
> this is afaik not possible with device drivers.
> 
> However we cannot export WMI method/events/etc to userspace, as this would
> be a security nightmare (random RPC with buggy ACPI firmware, yay!).

Interesting info here. I assumed they just worked with string names for
objects and properties and didn't care about GUIDs.

> In the future we might need an API for at least discovering and interacting
> with WMI devices backed by the same ACPI device, however this might take some
> time.
>
> I will focus on this WMI event driver for now.

Looking forward to it. Maybe tell platform-driver-x86 about it too.

Thanks,

James