Re: [PATCH 2/2] platform/x86: wmi: Allow creating WMI devices with the same GUID

Andy Lutomirski
2017-12-09T20:44:18Z

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
>