Proxmox-Port/linux-loongson
1
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson synced 2025-08-27 06:18:54 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

platform/x86: wmi: Rework WCxx/WExx ACPI method handling

Browse Source 
The handling of the WExx ACPI methods used for enabling and disabling
WMI events has multiple flaws:

- the ACPI methods are called even when the WMI device has not been
  marked as expensive.

- WExx ACPI methods might be called for inappropriate WMI devices.

- the error code AE_NOT_FOUND is treated as success.

The handling of the WCxx ACPI methods used for enabling and disabling
WMI data blocks is also flawed:

- WMI data blocks are enabled and disabled for every single "query"
  operation. This is racy and inefficient.

Unify the handling of both ACPI methods by introducing a common
helper function for enabling and disabling WMI devices.

Also enable/disable WMI data blocks during probe/remove and shutdown
to match the handling of WMI events.

Legacy GUID-based functions still have to enable/disable the WMI
device manually and thus still suffer from a potential race condition.
Since those functions are deprecated and suffer from various other
flaws this issue is purposefully not fixed.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Link: https://lore.kernel.org/r/20250216193251.866125-7-W_Armin@gmx.de
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
This commit is contained in:
Armin Wolf 2025-02-16 20:32:49 +01:00 committed by Ilpo Järvinen
parent b6b566908c
commit 656f0961d1
No known key found for this signature in database
GPG Key ID: 59AC4F6153E5CE31
1 changed files with 53 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

113
drivers/platform/x86/wmi.c
View File

@ -123,24 +123,6 @@ static const void *find_guid_context(struct wmi_block *wblock,
return NULL;
}
static acpi_status wmi_method_enable(struct wmi_block *wblock, bool enable)
{
struct guid_block *block;
char method[5];
acpi_status status;
acpi_handle handle;
block = &wblock->gblock;
handle = wblock->acpi_device->handle;
snprintf(method, 5, "WE%02X", block->notify_id);
status = acpi_execute_simple_method(handle, method, enable);
if (status == AE_NOT_FOUND)
return AE_OK;
return status;
}
#define WMI_ACPI_METHOD_NAME_SIZE 5
static inline void get_acpi_method_name(const struct wmi_block *wblock,
@ -184,6 +166,44 @@ static int wmidev_match_guid(struct device *dev, const void *data)
static const struct bus_type wmi_bus_type;
static const struct device_type wmi_type_event;
static const struct device_type wmi_type_method;
static int wmi_device_enable(struct wmi_device *wdev, bool enable)
{
struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev);
char method[WMI_ACPI_METHOD_NAME_SIZE];
acpi_handle handle;
acpi_status status;
if (!(wblock->gblock.flags & ACPI_WMI_EXPENSIVE))
return 0;
if (wblock->dev.dev.type == &wmi_type_method)
return 0;
if (wblock->dev.dev.type == &wmi_type_event)
snprintf(method, sizeof(method), "WE%02X", wblock->gblock.notify_id);
else
get_acpi_method_name(wblock, 'C', method);
/*
* Not all WMI devices marked as expensive actually implement the
* necessary ACPI method. Ignore this missing ACPI method to match
* the behaviour of the Windows driver.
*/
status = acpi_get_handle(wblock->acpi_device->handle, method, &handle);
if (ACPI_FAILURE(status))
return 0;
status = acpi_execute_simple_method(handle, NULL, enable);
if (ACPI_FAILURE(status))
return -EIO;
return 0;
}
static struct wmi_device *wmi_find_device_by_guid(const char *guid_string)
{
struct device *dev;
@ -337,10 +357,8 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
{
struct guid_block *block;
acpi_handle handle;
acpi_status status, wc_status = AE_ERROR;
struct acpi_object_list input;
union acpi_object wq_params[1];
char wc_method[WMI_ACPI_METHOD_NAME_SIZE];
char method[WMI_ACPI_METHOD_NAME_SIZE];
if (!out)
@ -364,40 +382,9 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
if (instance == 0 && test_bit(WMI_READ_TAKES_NO_ARGS, &wblock->flags))
input.count = 0;
/*
* If ACPI_WMI_EXPENSIVE, call the relevant WCxx method first to
* enable collection.
*/
if (block->flags & ACPI_WMI_EXPENSIVE) {
get_acpi_method_name(wblock, 'C', wc_method);
/*
* Some GUIDs break the specification by declaring themselves
* expensive, but have no corresponding WCxx method. So we
* should not fail if this happens.
*/
wc_status = acpi_execute_simple_method(handle, wc_method, 1);
}
get_acpi_method_name(wblock, 'Q', method);
status = acpi_evaluate_object(handle, method, &input, out);
/*
* If ACPI_WMI_EXPENSIVE, call the relevant WCxx method, even if
* the WQxx method failed - we should disable collection anyway.
*/
if ((block->flags & ACPI_WMI_EXPENSIVE) && ACPI_SUCCESS(wc_status)) {
/*
* Ignore whether this WCxx call succeeds or not since
* the previously executed WQxx method call might have
* succeeded, and returning the failing status code
* of this call would throw away the result of the WQxx
* call, potentially leaking memory.
*/
acpi_execute_simple_method(handle, wc_method, 0);
}
return status;
return acpi_evaluate_object(handle, method, &input, out);
}
/**
@ -421,9 +408,15 @@ acpi_status wmi_query_block(const char *guid_string, u8 instance,
if (IS_ERR(wdev))
return AE_ERROR;
if (wmi_device_enable(wdev, true) < 0)
dev_warn(&wdev->dev, "Failed to enable device\n");
wblock = container_of(wdev, struct wmi_block, dev);
status = __query_block(wblock, instance, out);
if (wmi_device_enable(wdev, false) < 0)
dev_warn(&wdev->dev, "Failed to disable device\n");
wmi_device_put(wdev);
return status;
@ -551,7 +544,7 @@ acpi_status wmi_install_notify_handler(const char *guid,
wblock->handler = handler;
wblock->handler_data = data;
if (ACPI_FAILURE(wmi_method_enable(wblock, true)))
if (wmi_device_enable(wdev, true) < 0)
dev_warn(&wblock->dev.dev, "Failed to enable device\n");
status = AE_OK;
@ -588,7 +581,7 @@ acpi_status wmi_remove_notify_handler(const char *guid)
if (!wblock->handler) {
status = AE_NULL_ENTRY;
} else {
if (ACPI_FAILURE(wmi_method_enable(wblock, false)))
if (wmi_device_enable(wdev, false) < 0)
dev_warn(&wblock->dev.dev, "Failed to disable device\n");
wblock->handler = NULL;
@ -823,10 +816,10 @@ static int wmi_dev_match(struct device *dev, const struct device_driver *driver)
static void wmi_dev_disable(void *data)
{
struct wmi_block *wblock = data;
struct device *dev = data;
if (ACPI_FAILURE(wmi_method_enable(wblock, false)))
dev_warn(&wblock->dev.dev, "Failed to disable device\n");
if (wmi_device_enable(to_wmi_device(dev), false) < 0)
dev_warn(dev, "Failed to disable device\n");
}
static int wmi_dev_probe(struct device *dev)
@ -852,14 +845,14 @@ static int wmi_dev_probe(struct device *dev)
return -ENODEV;
}
if (ACPI_FAILURE(wmi_method_enable(wblock, true)))
if (wmi_device_enable(to_wmi_device(dev), true) < 0)
dev_warn(dev, "failed to enable device -- probing anyway\n");
/*
* We have to make sure that all devres-managed resources are released first because
* some might still want to access the underlying WMI device.
*/
ret = devm_add_action_or_reset(dev, wmi_dev_disable, wblock);
ret = devm_add_action_or_reset(dev, wmi_dev_disable, dev);
if (ret < 0)
return ret;
@ -915,7 +908,7 @@ static void wmi_dev_shutdown(struct device *dev)
* We still need to disable the WMI device here since devres-managed resources
* like wmi_dev_disable() will not be release during shutdown.
*/
if (ACPI_FAILURE(wmi_method_enable(wblock, false)))
if (wmi_device_enable(to_wmi_device(dev), false) < 0)
dev_warn(dev, "Failed to disable device\n");
}
}