* [PATCH 0/2] Parse multiple duplicate WMI GUIDs
@ 2017-12-09 2:34 Mario Limonciello
2017-12-09 2:34 ` [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device Mario Limonciello
2017-12-09 2:34 ` [PATCH 2/2] platform/x86: wmi: Allow creating WMI devices with the same GUID Mario Limonciello
0 siblings, 2 replies; 10+ messages in thread
From: Mario Limonciello @ 2017-12-09 2:34 UTC (permalink / raw)
To: dvhart, Andy Shevchenko
Cc: LKML, platform-driver-x86, Andy Lutomirski, Mario Limonciello
I recently discovered that multiple instances of the WMI BMOF
GUID are present on some machines with more advanced WMI implementations.
Only the first found instance is parsed today with the rest ignored.
The rest of the instances should be readable by the wmi-bmof driver
(and userspace) to allow improving any parsing implementations.
Andy L. indicated he thought fixing this should require changing the
WMI driver to no longer track a block list of devices, but I don't
think that's necessary. The only significant change is that the WMI
bus will need to build the symlinks in /sys/bus/wmi/devices in a way
to prevent clashes with multiple devices sharing the same GUID.
The most obvious solution (and that which I implemented) is to include
the ACPI device associated with the GUID.
Since this is a symlink and the actual path remains stable I don't
know if that's considered changing a userspace interface. If so, then
an alternative would be to append a number when a second instance of
a GUID has been discovered and keep the "old" symlink path for the first
instance stable.
Mario Limonciello (2):
platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI
device
platform/x86: wmi: Allow creating WMI devices with the same GUID
drivers/platform/x86/wmi.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
--
2.14.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device 2017-12-09 2:34 [PATCH 0/2] Parse multiple duplicate WMI GUIDs Mario Limonciello @ 2017-12-09 2:34 ` Mario Limonciello 2017-12-09 2:52 ` Andy Lutomirski 2017-12-09 2:34 ` [PATCH 2/2] platform/x86: wmi: Allow creating WMI devices with the same GUID Mario Limonciello 1 sibling, 1 reply; 10+ messages in thread From: Mario Limonciello @ 2017-12-09 2:34 UTC (permalink / raw) To: dvhart, Andy Shevchenko Cc: LKML, platform-driver-x86, Andy Lutomirski, Mario Limonciello It's possible for the same GUID to show up on as system twice. This means using solely the GUID for identify the file will not be sufficient. Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> --- drivers/platform/x86/wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 791449a2370f..45d9010aafcf 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1081,7 +1081,8 @@ static int wmi_create_device(struct device *wmi_bus_dev, wblock->dev.dev.bus = &wmi_bus_type; wblock->dev.dev.parent = wmi_bus_dev; - dev_set_name(&wblock->dev.dev, "%pUL", gblock->guid); + dev_set_name(&wblock->dev.dev, "%s-%pUL", + dev_name(&wblock->acpi_device->dev), gblock->guid); device_initialize(&wblock->dev.dev); -- 2.14.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device 2017-12-09 2:34 ` [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device Mario Limonciello @ 2017-12-09 2:52 ` Andy Lutomirski 2017-12-09 3:41 ` Mario.Limonciello 0 siblings, 1 reply; 10+ messages in thread From: Andy Lutomirski @ 2017-12-09 2:52 UTC (permalink / raw) To: Mario Limonciello Cc: dvhart, Andy Shevchenko, LKML, platform-driver-x86, Andy Lutomirski --Andy > On Dec 8, 2017, at 6:34 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: > > It's possible for the same GUID to show up on as system twice. > This means using solely the GUID for identify the file will not > be sufficient. Isn't the file already in a per-bus directory? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device 2017-12-09 2:52 ` Andy Lutomirski @ 2017-12-09 3:41 ` Mario.Limonciello 0 siblings, 0 replies; 10+ messages in thread From: Mario.Limonciello @ 2017-12-09 3:41 UTC (permalink / raw) To: luto; +Cc: dvhart, andy.shevchenko, linux-kernel, platform-driver-x86, luto >> On Dec 8, 2017, at 6:34 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: >> >> It's possible for the same GUID to show up on as system twice. >> This means using solely the GUID for identify the file will not >> be sufficient. > >Isn't the file already in a per-bus directory? Yep, but the symlink created in /sys/bus/wmi/devices isn't. That's where the kernel complains about duplicate sysfs attributes. It's not exactly a pretty path I submitted, but it does avoid those collisions. Example (with this in place from /sys/bus/wmi/devices): lrwxrwxrwx 1 root root 0 Dec 8 21:39 PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A -> ../../../devices/platform/PNP0C14:04/wmi_bus/wmi_bus-PNP0C14:04/PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device @ 2017-12-09 3:41 ` Mario.Limonciello 0 siblings, 0 replies; 10+ messages in thread From: Mario.Limonciello @ 2017-12-09 3:41 UTC (permalink / raw) To: luto; +Cc: dvhart, andy.shevchenko, linux-kernel, platform-driver-x86, luto >> On Dec 8, 2017, at 6:34 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: >> >> It's possible for the same GUID to show up on as system twice. >> This means using solely the GUID for identify the file will not >> be sufficient. > >Isn't the file already in a per-bus directory? Yep, but the symlink created in /sys/bus/wmi/devices isn't. That's where the kernel complains about duplicate sysfs attributes. It's not exactly a pretty path I submitted, but it does avoid those collisions. Example (with this in place from /sys/bus/wmi/devices): lrwxrwxrwx 1 root root 0 Dec 8 21:39 PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A -> ../../../devices/platform/PNP0C14:04/wmi_bus/wmi_bus-PNP0C14:04/PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device 2017-12-09 3:41 ` Mario.Limonciello (?) @ 2017-12-09 21:32 ` Andy Lutomirski 2018-07-06 23:00 ` Darren Hart 2018-11-29 0:22 ` Darren Hart -1 siblings, 2 replies; 10+ messages in thread From: Andy Lutomirski @ 2017-12-09 21:32 UTC (permalink / raw) To: Mario Limonciello Cc: Darren Hart, Andy Shevchenko, linux-kernel, Platform Driver, Andrew Lutomirski On Fri, Dec 8, 2017 at 7:41 PM, <Mario.Limonciello@dell.com> wrote: >>> On Dec 8, 2017, at 6:34 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: >>> >>> It's possible for the same GUID to show up on as system twice. >>> This means using solely the GUID for identify the file will not >>> be sufficient. >> >>Isn't the file already in a per-bus directory? > > Yep, but the symlink created in /sys/bus/wmi/devices isn't. > That's where the kernel complains about duplicate sysfs > attributes. > > It's not exactly a pretty path I submitted, but it does avoid > those collisions. > > Example (with this in place from /sys/bus/wmi/devices): > lrwxrwxrwx 1 root root 0 Dec 8 21:39 PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A -> ../../../devices/platform/PNP0C14:04/wmi_bus/wmi_bus-PNP0C14:04/PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A Right, I saw that in the cover letter right after sending this. Greg, is there a cleaner way to deal with this? There are two instances of the same bus type, each of which would like to have a device called "70FE8229-D03B-4214-A1C6-1F884B1A892A". Can we somehow rename the symlinks without renaming the device, or are we just supposed to prefix the device name like Mario is doing here? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device 2017-12-09 21:32 ` Andy Lutomirski @ 2018-07-06 23:00 ` Darren Hart 2018-11-29 0:22 ` Darren Hart 1 sibling, 0 replies; 10+ messages in thread From: Darren Hart @ 2018-07-06 23:00 UTC (permalink / raw) To: Andy Lutomirski Cc: Mario Limonciello, Andy Shevchenko, linux-kernel, Platform Driver On Sat, Dec 09, 2017 at 01:32:29PM -0800, Andy Lutomirski wrote: > On Fri, Dec 8, 2017 at 7:41 PM, <Mario.Limonciello@dell.com> wrote: > >>> On Dec 8, 2017, at 6:34 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: > >>> > >>> It's possible for the same GUID to show up on as system twice. > >>> This means using solely the GUID for identify the file will not > >>> be sufficient. > >> > >>Isn't the file already in a per-bus directory? > > > > Yep, but the symlink created in /sys/bus/wmi/devices isn't. > > That's where the kernel complains about duplicate sysfs > > attributes. > > > > It's not exactly a pretty path I submitted, but it does avoid > > those collisions. > > > > Example (with this in place from /sys/bus/wmi/devices): > > lrwxrwxrwx 1 root root 0 Dec 8 21:39 PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A -> ../../../devices/platform/PNP0C14:04/wmi_bus/wmi_bus-PNP0C14:04/PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A > > Right, I saw that in the cover letter right after sending this. > > Greg, is there a cleaner way to deal with this? There are two > instances of the same bus type, each of which would like to have a > device called "70FE8229-D03B-4214-A1C6-1F884B1A892A". Can we somehow > rename the symlinks without renaming the device, or are we just > supposed to prefix the device name like Mario is doing here? > Mario, This one has been lingering at the bottom of my patch stack for a while now - sorry for not poking on this earlier. Is this still something you have a need for? If so, would you resend and add Greg to Cc - specifically directing Andy's question above to him? -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device 2017-12-09 21:32 ` Andy Lutomirski 2018-07-06 23:00 ` Darren Hart @ 2018-11-29 0:22 ` Darren Hart 1 sibling, 0 replies; 10+ messages in thread From: Darren Hart @ 2018-11-29 0:22 UTC (permalink / raw) To: Andy Lutomirski Cc: Mario Limonciello, Andy Shevchenko, linux-kernel, Platform Driver On Sat, Dec 09, 2017 at 01:32:29PM -0800, Andy Lutomirski wrote: > On Fri, Dec 8, 2017 at 7:41 PM, <Mario.Limonciello@dell.com> wrote: > >>> On Dec 8, 2017, at 6:34 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: > >>> > >>> It's possible for the same GUID to show up on as system twice. > >>> This means using solely the GUID for identify the file will not > >>> be sufficient. > >> > >>Isn't the file already in a per-bus directory? > > > > Yep, but the symlink created in /sys/bus/wmi/devices isn't. > > That's where the kernel complains about duplicate sysfs > > attributes. > > > > It's not exactly a pretty path I submitted, but it does avoid > > those collisions. > > > > Example (with this in place from /sys/bus/wmi/devices): > > lrwxrwxrwx 1 root root 0 Dec 8 21:39 PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A -> ../../../devices/platform/PNP0C14:04/wmi_bus/wmi_bus-PNP0C14:04/PNP0C14:04-70FE8229-D03B-4214-A1C6-1F884B1A892A > > Right, I saw that in the cover letter right after sending this. > > Greg, is there a cleaner way to deal with this? There are two > instances of the same bus type, each of which would like to have a > device called "70FE8229-D03B-4214-A1C6-1F884B1A892A". Can we somehow > rename the symlinks without renaming the device, or are we just > supposed to prefix the device name like Mario is doing here? > Mario, if this is still a concern (I presume it is) would you please resend the patch Cc gregkh directly? -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] platform/x86: wmi: Allow creating WMI devices with the same GUID 2017-12-09 2:34 [PATCH 0/2] Parse multiple duplicate WMI GUIDs Mario Limonciello 2017-12-09 2:34 ` [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device Mario Limonciello @ 2017-12-09 2:34 ` Mario Limonciello 2017-12-09 20:43 ` Andy Lutomirski 1 sibling, 1 reply; 10+ messages in thread From: Mario Limonciello @ 2017-12-09 2:34 UTC (permalink / raw) To: dvhart, Andy Shevchenko Cc: LKML, platform-driver-x86, Andy Lutomirski, Mario Limonciello In: commit d1f9e4970742 ("ACPI: WMI: Survive BIOS with duplicate GUIDs") parsing two of the same GUID was prevented in the WMI bus driver. At the time no one understood why GUID 05901221-D566-11D1-B2F0-00A0C9062910 was being duplicated. It's now known that GUID is used for the binary MOF file of a _WDG entry in the ASL. It's entirely possible for multiple _WDG entries and for multiple instances to bind in a given driver. NOTE: The only known instance of duplicated GUID's is the WMI BMOF GUID above, but it is possible that other vendors may duplicate GUIDs as well. It would be better for drivers to not interact with the WMI bus by GUID string but by the struct wmi_device provided by the WMI bus during probing. Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> --- drivers/platform/x86/wmi.c | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 45d9010aafcf..5ac17e360fa2 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1102,28 +1102,6 @@ static void wmi_free_devices(struct acpi_device *device) } } -static bool guid_already_parsed(struct acpi_device *device, - const u8 *guid) -{ - struct wmi_block *wblock; - - list_for_each_entry(wblock, &wmi_block_list, list) { - if (memcmp(wblock->gblock.guid, guid, 16) == 0) { - /* - * Because we historically didn't track the relationship - * between GUIDs and ACPI nodes, we don't know whether - * we need to suppress GUIDs that are unique on a - * given node but duplicated across nodes. - */ - dev_warn(&device->dev, "duplicate WMI GUID %pUL (first instance was on %s)\n", - guid, dev_name(&wblock->acpi_device->dev)); - return true; - } - } - - return false; -} - /* * Parse the _WDG method for the GUID data blocks */ @@ -1157,15 +1135,6 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) if (debug_dump_wdg) wmi_dump_wdg(&gblock[i]); - /* - * Some WMI devices, like those for nVidia hooks, have a - * duplicate GUID. It's not clear what we should do in this - * case yet, so for now, we'll just ignore the duplicate - * for device creation. - */ - if (guid_already_parsed(device, gblock[i].guid)) - continue; - wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL); if (!wblock) { retval = -ENOMEM; -- 2.14.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] platform/x86: wmi: Allow creating WMI devices with the same GUID 2017-12-09 2:34 ` [PATCH 2/2] platform/x86: wmi: Allow creating WMI devices with the same GUID Mario Limonciello @ 2017-12-09 20:43 ` Andy Lutomirski 0 siblings, 0 replies; 10+ messages in thread From: Andy Lutomirski @ 2017-12-09 20:43 UTC (permalink / raw) To: Mario Limonciello Cc: Darren Hart, Andy Shevchenko, LKML, Platform Driver, Andy Lutomirski On Fri, Dec 8, 2017 at 6:34 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: > In: commit d1f9e4970742 ("ACPI: WMI: Survive BIOS with duplicate GUIDs") > parsing two of the same GUID was prevented in the WMI bus driver. > > At the time no one understood why GUID 05901221-D566-11D1-B2F0-00A0C9062910 > was being duplicated. It's now known that GUID is used for the binary > MOF file of a _WDG entry in the ASL. It's entirely possible for multiple > _WDG entries and for multiple instances to bind in a given driver. > > NOTE: > The only known instance of duplicated GUID's is the WMI BMOF GUID above, > but it is possible that other vendors may duplicate GUIDs as well. It > would be better for drivers to not interact with the WMI bus by GUID > string but by the struct wmi_device provided by the WMI bus during > probing. I think you also need to audit all the users of wmi_block_list for duplicate handling. For example, wmi_install_notify_handler() probably needs a break statement inside the if (memcmp(...)). > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> > --- > drivers/platform/x86/wmi.c | 31 ------------------------------- > 1 file changed, 31 deletions(-) > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index 45d9010aafcf..5ac17e360fa2 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -1102,28 +1102,6 @@ static void wmi_free_devices(struct acpi_device *device) > } > } > > -static bool guid_already_parsed(struct acpi_device *device, > - const u8 *guid) > -{ > - struct wmi_block *wblock; > - > - list_for_each_entry(wblock, &wmi_block_list, list) { > - if (memcmp(wblock->gblock.guid, guid, 16) == 0) { > - /* > - * Because we historically didn't track the relationship > - * between GUIDs and ACPI nodes, we don't know whether > - * we need to suppress GUIDs that are unique on a > - * given node but duplicated across nodes. > - */ > - dev_warn(&device->dev, "duplicate WMI GUID %pUL (first instance was on %s)\n", > - guid, dev_name(&wblock->acpi_device->dev)); > - return true; > - } > - } > - > - return false; > -} > - > /* > * Parse the _WDG method for the GUID data blocks > */ > @@ -1157,15 +1135,6 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) > if (debug_dump_wdg) > wmi_dump_wdg(&gblock[i]); > > - /* > - * Some WMI devices, like those for nVidia hooks, have a > - * duplicate GUID. It's not clear what we should do in this > - * case yet, so for now, we'll just ignore the duplicate > - * for device creation. > - */ > - if (guid_already_parsed(device, gblock[i].guid)) > - continue; > - > wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL); > if (!wblock) { > retval = -ENOMEM; > -- > 2.14.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-11-29 0:22 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-09 2:34 [PATCH 0/2] Parse multiple duplicate WMI GUIDs Mario Limonciello 2017-12-09 2:34 ` [PATCH 1/2] platform/x86: wmi: prefix sysfs files in /sys/bus/wmi with the ACPI device Mario Limonciello 2017-12-09 2:52 ` Andy Lutomirski 2017-12-09 3:41 ` Mario.Limonciello 2017-12-09 3:41 ` Mario.Limonciello 2017-12-09 21:32 ` Andy Lutomirski 2018-07-06 23:00 ` Darren Hart 2018-11-29 0:22 ` Darren Hart 2017-12-09 2:34 ` [PATCH 2/2] platform/x86: wmi: Allow creating WMI devices with the same GUID Mario Limonciello 2017-12-09 20:43 ` Andy Lutomirski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.