From 3f399b5d7189bcb608c75abc85fe39f7a5509cfa Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Sun, 21 Jan 2024 21:08:24 +0100 Subject: platform/x86: wmi: Use ACPI device name in netlink event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The device name inside the ACPI netlink event is limited to 15 characters, so the WMI device name will get truncated. This can be observed with kacpimon when receiving an event from WMI device "9DBB5994-A997-11DA-B012-B622A1EF5492": netlink: 9DBB5994-A997- 000000d0 00000000 Fix this by using the shorter device name from the ACPI bus device instead. This still allows users to uniquely identify the WMI device by using the notify id (0xd0). Signed-off-by: Armin Wolf Reviewed-by: Hans de Goede Link: https://lore.kernel.org/r/20240121200824.2778-1-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index bd271a5730aa..7ef1e82dc61c 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1203,7 +1203,7 @@ static int wmi_notify_device(struct device *dev, void *data) } acpi_bus_generate_netlink_event(wblock->acpi_device->pnp.device_class, - dev_name(&wblock->dev.dev), *event, 0); + acpi_dev_name(wblock->acpi_device), *event, 0); return -EBUSY; } -- cgit v1.2.3 From 6468e64ee380514b331ccb743fa491b23cd8b4dd Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Tue, 30 Jan 2024 23:19:42 +0100 Subject: platform/x86: wmi: Stop using ACPI device class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an ACPI netlink event is received by acpid, the ACPI device class is passed as its first argument. But since the class string is not initialized during probe, an empty string is being passed: netlink: PNP0C14:01 000000d0 00000000 Fix this by passing a static string instead. Tested on a Dell Inspiron 3505. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240130221942.2770-1-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 7ef1e82dc61c..3335de4e32b2 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1202,8 +1202,7 @@ static int wmi_notify_device(struct device *dev, void *data) wblock->handler(*event, wblock->handler_data); } - acpi_bus_generate_netlink_event(wblock->acpi_device->pnp.device_class, - acpi_dev_name(wblock->acpi_device), *event, 0); + acpi_bus_generate_netlink_event("wmi", acpi_dev_name(wblock->acpi_device), *event, 0); return -EBUSY; } -- cgit v1.2.3 From 10fdfd13a35994ac1dcc3003d4046cb1955e66a8 Mon Sep 17 00:00:00 2001 From: "Ricardo B. Marliere" Date: Sun, 4 Feb 2024 11:40:16 -0300 Subject: platform: x86: wmi: make wmi_bus_type const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the driver core can properly handle constant struct bus_type, move the wmi_bus_type variable to be a constant structure as well, placing it into read-only memory which can not be modified at runtime. Cc: Greg Kroah-Hartman Suggested-by: Greg Kroah-Hartman Signed-off-by: "Ricardo B. Marliere" Reviewed-by: Armin Wolf Link: https://lore.kernel.org/r/20240204-bus_cleanup-platform-drivers-x86-v1-1-1f0839b385c6@marliere.net Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 3335de4e32b2..5682c7de0394 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -219,7 +219,7 @@ static int wmidev_match_guid(struct device *dev, const void *data) return 0; } -static struct bus_type wmi_bus_type; +static const struct bus_type wmi_bus_type; static struct wmi_device *wmi_find_device_by_guid(const char *guid_string) { @@ -899,7 +899,7 @@ static struct class wmi_bus_class = { .name = "wmi_bus", }; -static struct bus_type wmi_bus_type = { +static const struct bus_type wmi_bus_type = { .name = "wmi", .dev_groups = wmi_groups, .match = wmi_dev_match, -- cgit v1.2.3 From 7f1b998a9108f7fd465039323d5fc2599b8cae77 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Tue, 6 Feb 2024 23:04:44 +0100 Subject: platform/x86: wmi: Check if WMxx control method exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some devices like the MSI GF63-12VF contain WMI method blocks without providing the necessary WMxx ACPI control methods. Avoid creating WMI devices for such WMI method blocks since the resulting WMI device is going to be unusable. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240206220447.3102-1-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 5682c7de0394..af273623bc5b 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -958,6 +958,15 @@ static int wmi_create_device(struct device *wmi_bus_dev, } if (wblock->gblock.flags & ACPI_WMI_METHOD) { + get_acpi_method_name(wblock, 'M', method); + if (!acpi_has_method(device->handle, method)) { + dev_warn(wmi_bus_dev, + FW_BUG "%s method block execution control method not found\n", + method); + + return -ENXIO; + } + wblock->dev.dev.type = &wmi_type_method; goto out_init; } -- cgit v1.2.3 From d0c595a11785573aad3b9e32ae293c48757eceff Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Tue, 6 Feb 2024 23:04:45 +0100 Subject: platform/x86: wmi: Use FW_BUG when warning about missing control methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A missing WQxx control method is a firmware bug and should be marked as such using FW_BUG so that users know that the issue is not a kernel issue. Since get_subobj_info() might fail even if the control method is present, we need to print the warning only if acpi_get_handle() fails. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240206220447.3102-2-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index af273623bc5b..43d750ff65cf 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -130,26 +130,6 @@ static const void *find_guid_context(struct wmi_block *wblock, return NULL; } -static int get_subobj_info(acpi_handle handle, const char *pathname, - struct acpi_device_info **info) -{ - acpi_handle subobj_handle; - acpi_status status; - - status = acpi_get_handle(handle, pathname, &subobj_handle); - if (status == AE_NOT_FOUND) - return -ENOENT; - - if (ACPI_FAILURE(status)) - return -EIO; - - status = acpi_get_object_info(subobj_handle, info); - if (ACPI_FAILURE(status)) - return -EIO; - - return 0; -} - static acpi_status wmi_method_enable(struct wmi_block *wblock, bool enable) { struct guid_block *block; @@ -947,9 +927,10 @@ static int wmi_create_device(struct device *wmi_bus_dev, struct wmi_block *wblock, struct acpi_device *device) { - struct acpi_device_info *info; char method[WMI_ACPI_METHOD_NAME_SIZE]; - int result; + struct acpi_device_info *info; + acpi_handle method_handle; + acpi_status status; uint count; if (wblock->gblock.flags & ACPI_WMI_EVENT) { @@ -977,15 +958,19 @@ static int wmi_create_device(struct device *wmi_bus_dev, * we ignore this data block. */ get_acpi_method_name(wblock, 'Q', method); - result = get_subobj_info(device->handle, method, &info); - - if (result) { + status = acpi_get_handle(device->handle, method, &method_handle); + if (ACPI_FAILURE(status)) { dev_warn(wmi_bus_dev, - "%s data block query control method not found\n", + FW_BUG "%s data block query control method not found\n", method); - return result; + + return -ENXIO; } + status = acpi_get_object_info(method_handle, &info); + if (ACPI_FAILURE(status)) + return -EIO; + wblock->dev.dev.type = &wmi_type_data; /* -- cgit v1.2.3 From 49c67cd5b6a4b611ac775de3831f5e739dd580f2 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Tue, 6 Feb 2024 23:04:46 +0100 Subject: platform/x86: wmi: Remove unnecessary out-of-memory message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If kzalloc() fails, an out-of-memory message is already printed. Remove the unnecessary second warning message. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240206220447.3102-3-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 43d750ff65cf..be3e35a90703 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1093,10 +1093,8 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev) continue; wblock = kzalloc(sizeof(*wblock), GFP_KERNEL); - if (!wblock) { - dev_err(wmi_bus_dev, "Failed to allocate %pUL\n", &gblock[i].guid); + if (!wblock) continue; - } wblock->acpi_device = device; wblock->gblock = gblock[i]; -- cgit v1.2.3 From fde7da1072f3c0239a80b590e7b75c9411e8b630 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Tue, 6 Feb 2024 23:04:47 +0100 Subject: platform/x86: wmi: Replace pr_err() with dev_err() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using dev_err() allows users to find out from which device the error message came from. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240206220447.3102-4-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index be3e35a90703..57026f91c396 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1295,7 +1295,7 @@ static int acpi_wmi_probe(struct platform_device *device) error = parse_wdg(wmi_bus_dev, device); if (error) { - pr_err("Failed to parse WDG method\n"); + dev_err(&device->dev, "Failed to parse _WDG method\n"); return error; } -- cgit v1.2.3 From 5b559e8ab01c8d7a92478f8143ba844161292203 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 12 Feb 2024 19:50:16 +0100 Subject: platform/x86: wmi: Make input buffer mandatory when evaluating methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ACPI-WMI specification declares in the section "ACPI Control Method Naming Conventions and Functionality for Windows 2000 Instrumentation" that a WMxx control method takes 3 arguments: instance, method id and argument buffer. This is also the case even when the underlying WMI method does not have any input arguments. So if a WMI driver evaluates a WMI method without passing an input buffer, ACPICA will log a warning complaining that the third argument is missing. Prevent this by checking that a input buffer was passed, and return an error if this was not the case. Tested on a Asus PRIME B650-Plus. Reviewed-by: Kuppuswamy Sathyanarayanan Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240212185016.5494-1-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index b83c0f0ddd5c..5a613b06b269 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -296,7 +296,7 @@ EXPORT_SYMBOL_GPL(wmidev_instance_count); * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba * @instance: Instance index * @method_id: Method ID to call - * @in: Buffer containing input for the method call + * @in: Mandatory buffer containing input for the method call * @out: Empty buffer to return the method results * * Call an ACPI-WMI method, the caller must free @out. @@ -326,7 +326,7 @@ EXPORT_SYMBOL_GPL(wmi_evaluate_method); * @wdev: A wmi bus device from a driver * @instance: Instance index * @method_id: Method ID to call - * @in: Buffer containing input for the method call + * @in: Mandatory buffer containing input for the method call * @out: Empty buffer to return the method results * * Call an ACPI-WMI method, the caller must free @out. @@ -347,26 +347,25 @@ acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, u32 met block = &wblock->gblock; handle = wblock->acpi_device->handle; + if (!in) + return AE_BAD_DATA; + if (!(block->flags & ACPI_WMI_METHOD)) return AE_BAD_DATA; if (block->instance_count <= instance) return AE_BAD_PARAMETER; - input.count = 2; + input.count = 3; input.pointer = params; + params[0].type = ACPI_TYPE_INTEGER; params[0].integer.value = instance; params[1].type = ACPI_TYPE_INTEGER; params[1].integer.value = method_id; - - if (in) { - input.count = 3; - - params[2].type = get_param_acpi_type(wblock); - params[2].buffer.length = in->length; - params[2].buffer.pointer = in->pointer; - } + params[2].type = get_param_acpi_type(wblock); + params[2].buffer.length = in->length; + params[2].buffer.pointer = in->pointer; get_acpi_method_name(wblock, 'M', method); -- cgit v1.2.3 From 4f299135d5668f56be270d224d41eb83d2002038 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 19 Feb 2024 12:59:15 +0100 Subject: platform/x86: wmi: Prevent incompatible event driver from probing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a WMI event driver has no_notify_data set, then it indicates support for WMI events which provide no notify data, otherwise the notify() callback expects a valid ACPI object as notify data. However if a WMI event driver which requires notify data is bound to a WMI event device which cannot retrieve such data due to the _WED ACPI method being absent, then the driver will be dysfunctional since all WMI events will be dropped due to the missing notify data. Fix this by not allowing such WMI event drivers to bind to WMI event devices which do not support retrieving of notify data. Also reword the description of no_notify_data a bit. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240219115919.16526-2-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 10 ++++++++++ include/linux/wmi.h | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 5a613b06b269..8fb90b726f50 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -57,6 +57,7 @@ static_assert(__alignof__(struct guid_block) == 1); enum { /* wmi_block flags */ WMI_READ_TAKES_NO_ARGS, + WMI_NO_EVENT_DATA, }; struct wmi_block { @@ -869,6 +870,11 @@ static int wmi_dev_probe(struct device *dev) struct wmi_driver *wdriver = drv_to_wdrv(dev->driver); int ret = 0; + if (wdriver->notify) { + if (test_bit(WMI_NO_EVENT_DATA, &wblock->flags) && !wdriver->no_notify_data) + return -ENODEV; + } + if (ACPI_FAILURE(wmi_method_enable(wblock, true))) dev_warn(dev, "failed to enable device -- probing anyway\n"); @@ -1094,6 +1100,7 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev) struct acpi_device *device = ACPI_COMPANION(&pdev->dev); struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL}; const struct guid_block *gblock; + bool event_data_available; struct wmi_block *wblock; union acpi_object *obj; acpi_status status; @@ -1113,6 +1120,7 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev) return -ENXIO; } + event_data_available = acpi_has_method(device->handle, "_WED"); gblock = (const struct guid_block *)obj->buffer.pointer; total = obj->buffer.length / sizeof(struct guid_block); @@ -1131,6 +1139,8 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev) wblock->acpi_device = device; wblock->gblock = gblock[i]; + if (gblock[i].flags & ACPI_WMI_EVENT && !event_data_available) + set_bit(WMI_NO_EVENT_DATA, &wblock->flags); retval = wmi_create_device(wmi_bus_dev, wblock, device); if (retval) { diff --git a/include/linux/wmi.h b/include/linux/wmi.h index 686291b87852..781958310bfb 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -48,7 +48,7 @@ u8 wmidev_instance_count(struct wmi_device *wdev); * struct wmi_driver - WMI driver structure * @driver: Driver model structure * @id_table: List of WMI GUIDs supported by this driver - * @no_notify_data: WMI events provide no event data + * @no_notify_data: Driver supports WMI events which provide no event data * @probe: Callback for device binding * @remove: Callback for device unbinding * @notify: Callback for receiving WMI events -- cgit v1.2.3 From 125619112deaf5f7d79b05e268254df3af916d10 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 19 Feb 2024 12:59:16 +0100 Subject: platform/x86: wmi: Check if event data is not NULL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WMI event drivers which do not have no_notify_data set expect that each WMI event contains valid data. Evaluating _WED however might return no data, which can cause issues with such drivers. Fix this by validating that evaluating _WED did return data. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240219115919.16526-3-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 8fb90b726f50..ff4742c40cc3 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1210,6 +1210,7 @@ static void wmi_notify_driver(struct wmi_block *wblock) { struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver); struct acpi_buffer data = { ACPI_ALLOCATE_BUFFER, NULL }; + union acpi_object *obj = NULL; acpi_status status; if (!driver->no_notify_data) { @@ -1218,12 +1219,18 @@ static void wmi_notify_driver(struct wmi_block *wblock) dev_warn(&wblock->dev.dev, "Failed to get event data\n"); return; } + + obj = data.pointer; + if (!obj) { + dev_warn(&wblock->dev.dev, "Event contains no event data\n"); + return; + } } if (driver->notify) - driver->notify(&wblock->dev, data.pointer); + driver->notify(&wblock->dev, obj); - kfree(data.pointer); + kfree(obj); } static int wmi_notify_device(struct device *dev, void *data) -- cgit v1.2.3 From 56230bd733f8cb122632c80bc49ae12b1a9365a6 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 19 Feb 2024 12:59:17 +0100 Subject: platform/x86: wmi: Always evaluate _WED when receiving an event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ACPI WMI specification states: "The _WED control method is evaluated by the mapper in response to receiving a notification from a control method." This means that _WED should be evaluated unconditionally even if no WMI event consumers are present. Some firmware implementations actually depend on this behavior by storing the event data inside a queue which will fill up if the WMI core stops retrieving event data items due to no consumers being present Fix this by always evaluating _WED even if no WMI event consumers are present. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240219115919.16526-4-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 67 +++++++++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 18 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index ff4742c40cc3..abd0183c4107 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1206,37 +1206,46 @@ acpi_wmi_ec_space_handler(u32 function, acpi_physical_address address, } } -static void wmi_notify_driver(struct wmi_block *wblock) +static int wmi_get_notify_data(struct wmi_block *wblock, union acpi_object **obj) { - struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver); struct acpi_buffer data = { ACPI_ALLOCATE_BUFFER, NULL }; - union acpi_object *obj = NULL; acpi_status status; - if (!driver->no_notify_data) { - status = get_event_data(wblock, &data); - if (ACPI_FAILURE(status)) { - dev_warn(&wblock->dev.dev, "Failed to get event data\n"); - return; - } + if (test_bit(WMI_NO_EVENT_DATA, &wblock->flags)) { + *obj = NULL; + return 0; + } - obj = data.pointer; - if (!obj) { - dev_warn(&wblock->dev.dev, "Event contains no event data\n"); - return; - } + status = get_event_data(wblock, &data); + if (ACPI_FAILURE(status)) { + dev_warn(&wblock->dev.dev, "Failed to get event data\n"); + return -EIO; + } + + *obj = data.pointer; + + return 0; +} + +static void wmi_notify_driver(struct wmi_block *wblock, union acpi_object *obj) +{ + struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver); + + if (!obj && !driver->no_notify_data) { + dev_warn(&wblock->dev.dev, "Event contains no event data\n"); + return; } if (driver->notify) driver->notify(&wblock->dev, obj); - - kfree(obj); } static int wmi_notify_device(struct device *dev, void *data) { struct wmi_block *wblock = dev_to_wblock(dev); + union acpi_object *obj; u32 *event = data; + int ret; if (!(wblock->gblock.flags & ACPI_WMI_EVENT && wblock->gblock.notify_id == *event)) return 0; @@ -1246,10 +1255,32 @@ static int wmi_notify_device(struct device *dev, void *data) * Because of this the WMI driver notify handler takes precedence. */ if (wblock->dev.dev.driver && wblock->driver_ready) { - wmi_notify_driver(wblock); + ret = wmi_get_notify_data(wblock, &obj); + if (ret >= 0) { + wmi_notify_driver(wblock, obj); + kfree(obj); + } } else { - if (wblock->handler) + if (wblock->handler) { wblock->handler(*event, wblock->handler_data); + } else { + /* The ACPI WMI specification says that _WED should be + * evaluated every time an notification is received, even + * if no consumers are present. + * + * Some firmware implementations actually depend on this + * by using a queue for events which will fill up if the + * WMI driver core stops evaluating _WED due to missing + * WMI event consumers. + * + * Because of this we need this seemingly useless call to + * wmi_get_notify_data() which in turn evaluates _WED. + */ + ret = wmi_get_notify_data(wblock, &obj); + if (ret >= 0) + kfree(obj); + } + } up_read(&wblock->notify_lock); -- cgit v1.2.3 From 928439582b3133a5e845ec15ee6eb6838651024f Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 26 Feb 2024 20:35:55 +0100 Subject: platform/x86: wmi: Ignore duplicated GUIDs in legacy matches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When matching a WMI device to a GUID used by the legacy GUID-based API, devices with a duplicated GUID should be ignored. Add an additional WMI device flag signaling that the GUID used by the WMI device is also used by another WMI device. Ignore such devices inside the match functions used by the legacy GUID-based API. Tested on a ASUS Prime B650-Plus. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240226193557.2888-1-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index abd0183c4107..29dfe52eb802 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -57,6 +57,7 @@ static_assert(__alignof__(struct guid_block) == 1); enum { /* wmi_block flags */ WMI_READ_TAKES_NO_ARGS, + WMI_GUID_DUPLICATED, WMI_NO_EVENT_DATA, }; @@ -196,6 +197,12 @@ static int wmidev_match_guid(struct device *dev, const void *data) struct wmi_block *wblock = dev_to_wblock(dev); const guid_t *guid = data; + /* Legacy GUID-based functions are restricted to only see + * a single WMI device for each GUID. + */ + if (test_bit(WMI_GUID_DUPLICATED, &wblock->flags)) + return 0; + if (guid_equal(guid, &wblock->gblock.guid)) return 1; @@ -207,6 +214,12 @@ static int wmidev_match_notify_id(struct device *dev, const void *data) struct wmi_block *wblock = dev_to_wblock(dev); const u32 *notify_id = data; + /* Legacy GUID-based functions are restricted to only see + * a single WMI device for each GUID. + */ + if (test_bit(WMI_GUID_DUPLICATED, &wblock->flags)) + return 0; + if (wblock->gblock.flags & ACPI_WMI_EVENT && wblock->gblock.notify_id == *notify_id) return 1; @@ -1036,10 +1049,12 @@ static int wmi_create_device(struct device *wmi_bus_dev, wblock->dev.dev.parent = wmi_bus_dev; count = guid_count(&wblock->gblock.guid); - if (count) + if (count) { dev_set_name(&wblock->dev.dev, "%pUL-%d", &wblock->gblock.guid, count); - else + set_bit(WMI_GUID_DUPLICATED, &wblock->flags); + } else { dev_set_name(&wblock->dev.dev, "%pUL", &wblock->gblock.guid); + } device_initialize(&wblock->dev.dev); -- cgit v1.2.3 From a66ccfc2535418b536b1203b65f87c4f501f6bdd Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 26 Feb 2024 20:35:56 +0100 Subject: platform/x86: wmi: Do not instantiate older WMI drivers multiple times MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Many older WMI drivers cannot be instantiated multiple times for two reasons: - they are using the legacy GUID-based WMI API - they are singletons (with global state) Prevent such WMI drivers from binding to WMI devices with a duplicated GUID, as this would mean that the WMI driver will be instantiated at least two times (one for the original GUID and one for the duplicated GUID). WMI drivers which can be instantiated multiple times can signal this by setting a flag inside struct wmi_driver. Tested on a ASUS Prime B650-Plus. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240226193557.2888-2-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/hwmon/dell-smm-hwmon.c | 1 + drivers/platform/x86/dell/dell-wmi-ddv.c | 1 + drivers/platform/x86/intel/wmi/sbl-fw-update.c | 1 + drivers/platform/x86/intel/wmi/thunderbolt.c | 1 + drivers/platform/x86/wmi-bmof.c | 1 + drivers/platform/x86/wmi.c | 12 ++++++++++++ include/linux/wmi.h | 2 ++ 7 files changed, 19 insertions(+) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c index 6d8c0f328b7b..168d669c4eca 100644 --- a/drivers/hwmon/dell-smm-hwmon.c +++ b/drivers/hwmon/dell-smm-hwmon.c @@ -1587,6 +1587,7 @@ static struct wmi_driver dell_smm_wmi_driver = { }, .id_table = dell_smm_wmi_id_table, .probe = dell_smm_wmi_probe, + .no_singleton = true, }; /* diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c index db1e9240dd02..0b2299f7a2de 100644 --- a/drivers/platform/x86/dell/dell-wmi-ddv.c +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c @@ -882,6 +882,7 @@ static struct wmi_driver dell_wmi_ddv_driver = { }, .id_table = dell_wmi_ddv_id_table, .probe = dell_wmi_ddv_probe, + .no_singleton = true, }; module_wmi_driver(dell_wmi_ddv_driver); diff --git a/drivers/platform/x86/intel/wmi/sbl-fw-update.c b/drivers/platform/x86/intel/wmi/sbl-fw-update.c index 040153ad67c1..75c82c08117f 100644 --- a/drivers/platform/x86/intel/wmi/sbl-fw-update.c +++ b/drivers/platform/x86/intel/wmi/sbl-fw-update.c @@ -131,6 +131,7 @@ static struct wmi_driver intel_wmi_sbl_fw_update_driver = { .probe = intel_wmi_sbl_fw_update_probe, .remove = intel_wmi_sbl_fw_update_remove, .id_table = intel_wmi_sbl_id_table, + .no_singleton = true, }; module_wmi_driver(intel_wmi_sbl_fw_update_driver); diff --git a/drivers/platform/x86/intel/wmi/thunderbolt.c b/drivers/platform/x86/intel/wmi/thunderbolt.c index e2ad3f46f356..08df560a2c7a 100644 --- a/drivers/platform/x86/intel/wmi/thunderbolt.c +++ b/drivers/platform/x86/intel/wmi/thunderbolt.c @@ -63,6 +63,7 @@ static struct wmi_driver intel_wmi_thunderbolt_driver = { .dev_groups = tbt_groups, }, .id_table = intel_wmi_thunderbolt_id_table, + .no_singleton = true, }; module_wmi_driver(intel_wmi_thunderbolt_driver); diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c index 644d2fd889c0..df6f0ae6e6c7 100644 --- a/drivers/platform/x86/wmi-bmof.c +++ b/drivers/platform/x86/wmi-bmof.c @@ -94,6 +94,7 @@ static struct wmi_driver wmi_bmof_driver = { .probe = wmi_bmof_probe, .remove = wmi_bmof_remove, .id_table = wmi_bmof_id_table, + .no_singleton = true, }; module_wmi_driver(wmi_bmof_driver); diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 29dfe52eb802..349deced87e8 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -883,6 +883,18 @@ static int wmi_dev_probe(struct device *dev) struct wmi_driver *wdriver = drv_to_wdrv(dev->driver); int ret = 0; + /* Some older WMI drivers will break if instantiated multiple times, + * so they are blocked from probing WMI devices with a duplicated GUID. + * + * New WMI drivers should support being instantiated multiple times. + */ + if (test_bit(WMI_GUID_DUPLICATED, &wblock->flags) && !wdriver->no_singleton) { + dev_warn(dev, "Legacy driver %s cannot be instantiated multiple times\n", + dev->driver->name); + + return -ENODEV; + } + if (wdriver->notify) { if (test_bit(WMI_NO_EVENT_DATA, &wblock->flags) && !wdriver->no_notify_data) return -ENODEV; diff --git a/include/linux/wmi.h b/include/linux/wmi.h index 781958310bfb..63cca3b58d6d 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -49,6 +49,7 @@ u8 wmidev_instance_count(struct wmi_device *wdev); * @driver: Driver model structure * @id_table: List of WMI GUIDs supported by this driver * @no_notify_data: Driver supports WMI events which provide no event data + * @no_singleton: Driver can be instantiated multiple times * @probe: Callback for device binding * @remove: Callback for device unbinding * @notify: Callback for receiving WMI events @@ -59,6 +60,7 @@ struct wmi_driver { struct device_driver driver; const struct wmi_device_id *id_table; bool no_notify_data; + bool no_singleton; int (*probe)(struct wmi_device *wdev, const void *context); void (*remove)(struct wmi_device *wdev); -- cgit v1.2.3 From f86f09ad9ab234a90eef8bba6239b3ca82dc281e Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Mon, 26 Feb 2024 20:35:57 +0100 Subject: platform/x86: wmi: Remove obsolete duplicate GUID allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whitelist-based approach for preventing older WMI drivers from being instantiated multiple times has many drawbacks: - uses cannot see all available WMI devices (if not whitelisted) - whitelisting a WMI driver requires changes in the WMI driver core - maintenance burden for driver and subsystem developers Since the WMI driver core already takes care that older WMI drivers are not being instantiated multiple times, remove the now redundant whitelist. Tested on a ASUS Prime B650-Plus. Signed-off-by: Armin Wolf Link: https://lore.kernel.org/r/20240226193557.2888-3-W_Armin@gmx.de Reviewed-by: Ilpo Järvinen Signed-off-by: Ilpo Järvinen --- drivers/platform/x86/wmi.c | 39 --------------------------------------- 1 file changed, 39 deletions(-) (limited to 'drivers/platform/x86/wmi.c') diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 349deced87e8..1920e115da89 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -90,16 +90,6 @@ static const struct acpi_device_id wmi_device_ids[] = { }; MODULE_DEVICE_TABLE(acpi, wmi_device_ids); -/* allow duplicate GUIDs as these device drivers use struct wmi_driver */ -static const char * const allow_duplicates[] = { - "05901221-D566-11D1-B2F0-00A0C9062910", /* wmi-bmof */ - "8A42EA14-4F2A-FD45-6422-0087F7A7E608", /* dell-wmi-ddv */ - "44FADEB1-B204-40F2-8581-394BBDC1B651", /* intel-wmi-sbl-fw-update */ - "86CCFD48-205E-4A77-9C48-2021CBEDE341", /* intel-wmi-thunderbolt */ - "F1DDEE52-063C-4784-A11E-8A06684B9B01", /* dell-smm-hwmon */ - NULL -}; - #define dev_to_wblock(__dev) container_of_const(__dev, struct wmi_block, dev.dev) #define dev_to_wdev(__dev) container_of_const(__dev, struct wmi_device, dev) @@ -1093,32 +1083,6 @@ static int wmi_add_device(struct platform_device *pdev, struct wmi_device *wdev) return device_add(&wdev->dev); } -static bool guid_already_parsed_for_legacy(struct acpi_device *device, const guid_t *guid) -{ - struct wmi_block *wblock; - - list_for_each_entry(wblock, &wmi_block_list, list) { - /* skip warning and register if we know the driver will use struct wmi_driver */ - for (int i = 0; allow_duplicates[i] != NULL; i++) { - if (guid_parse_and_compare(allow_duplicates[i], guid)) - return false; - } - if (guid_equal(&wblock->gblock.guid, guid)) { - /* - * 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,9 +1121,6 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev) continue; } - if (guid_already_parsed_for_legacy(device, &gblock[i].guid)) - continue; - wblock = kzalloc(sizeof(*wblock), GFP_KERNEL); if (!wblock) continue; -- cgit v1.2.3