Message-Id: <20251122203803.6154-2-W_Armin@gmx.de>
Date: Sat, 22 Nov 2025 21:37:55 +0100
From: Armin Wolf <W_Armin@....de>
To: hansg@...nel.org,
	ilpo.jarvinen@...ux.intel.com
Cc: platform-driver-x86@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux@...ssschuh.net,
	Dell.Client.Kernel@...l.com,
	corbet@....net,
	linux-doc@...r.kernel.org
Subject: [PATCH 1/9] platform/wmi: Introduce marshalling support

The Windows WMI-ACPI driver likely uses wmilib [1] to interact with
the WMI service in userspace. Said library uses plain byte buffers
for exchanging data, so the WMI-ACPI driver has to convert between
those byte buffers and ACPI objects returned by the ACPI firmware.

The format of the byte buffer is publicly documented [2], and after
some reverse eingineering of the WMI-ACPI driver using a set of custom
ACPI tables, the following conversion rules have been discovered:

- ACPI integers are always converted into a uint32
- ACPI strings are converted into special WMI strings
- ACPI buffers are copied as-is
- ACPI packages are unpacked

Extend the ACPI-WMI driver to also perform this kind of marshalling
for WMI data blocks, methods and events. During so gives us a number
of benefits:

- WMI drivers are not restricted to a fixed set of supported ACPI data
  types anymore, see dell-wmi-aio (integer vs buffer) and
  hp-wmi-sensors (string vs buffer)

- correct marshalling of WMI strings when data blocks are marked
  as requiring ACPI strings instead of ACPI buffers

- development of WMI drivers without having to understand ACPI

This eventually should result in better compatibility with some
ACPI firmware implementations and in simpler WMI drivers. There are
however some differences between the original Windows driver and
the ACPI-WMI driver when it comes to ACPI object conversions:

- the Windows driver copies internal _ACPI_METHOD_ARGUMENT_V1 data
  structures into the output buffer when encountering nested ACPI
  packages. This is very likely an error inside the driver itself, so
  we do not support nested ACPI packages.

- when converting WMI strings (UTF-16LE) into ACPI strings (ASCII),
  the Windows driver replaces non-ascii characters (ä -> a, & -> ?)
  instead of returning an error. This behavior is not documented
  anywhere and might lead to severe errors in some cases (like
  setting BIOS passwords over WMI), so we simply return an error.

As the current bus-based WMI API is based on ACPI buffers, a new
API is necessary. The legacy GUID-based WMI API is not extended to
support marshalling, as WMI drivers using said API are expected to
move to the bus-based WMI API in the future.

[1] https://learn.microsoft.com/de-de/windows-hardware/drivers/ddi/wmilib/
[2] https://learn.microsoft.com/en-us/windows-hardware/drivers/kernel/
    driver-defined-wmi-data-items

Signed-off-by: Armin Wolf <W_Armin@....de>
---
 drivers/platform/wmi/Makefile      |   2 +-
 drivers/platform/wmi/core.c        | 160 +++++++++++++++++++-
 drivers/platform/wmi/internal.h    |  14 ++
 drivers/platform/wmi/marshalling.c | 233 +++++++++++++++++++++++++++++
 include/linux/wmi.h                |  39 ++++-
 5 files changed, 441 insertions(+), 7 deletions(-)
 create mode 100644 drivers/platform/wmi/internal.h
 create mode 100644 drivers/platform/wmi/marshalling.c

diff --git a/drivers/platform/wmi/Makefile b/drivers/platform/wmi/Makefile
index 98393d7391ec..6f2bf8cc709e 100644
--- a/drivers/platform/wmi/Makefile
+++ b/drivers/platform/wmi/Makefile
@@ -4,5 +4,5 @@
 # ACPI WMI core
 #
 
-wmi-y			:= core.o
+wmi-y			:= core.o marshalling.o
 obj-$(CONFIG_ACPI_WMI)	+= wmi.o
diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
index 6878c4fcb0b5..1601bf9fe135 100644
--- a/drivers/platform/wmi/core.c
+++ b/drivers/platform/wmi/core.c
@@ -23,6 +23,7 @@
 #include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/rwsem.h>
@@ -33,6 +34,8 @@
 #include <linux/wmi.h>
 #include <linux/fs.h>
 
+#include "internal.h"
+
 MODULE_AUTHOR("Carlos Corbacho");
 MODULE_DESCRIPTION("ACPI-WMI Mapping Driver");
 MODULE_LICENSE("GPL");
@@ -302,7 +305,7 @@ acpi_status wmi_evaluate_method(const char *guid_string, u8 instance, u32 method
 EXPORT_SYMBOL_GPL(wmi_evaluate_method);
 
 /**
- * wmidev_evaluate_method - Evaluate a WMI method
+ * wmidev_evaluate_method - Evaluate a WMI method (deprecated)
  * @wdev: A wmi bus device from a driver
  * @instance: Instance index
  * @method_id: Method ID to call
@@ -360,6 +363,70 @@ acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, u32 met
 }
 EXPORT_SYMBOL_GPL(wmidev_evaluate_method);
 
+/**
+ * wmidev_invoke_method - Invoke a WMI method
+ * @wdev: A wmi bus device from a driver
+ * @instance: Instance index
+ * @method_id: Method ID to call
+ * @in: Mandatory WMI buffer containing input for the method call
+ * @out: Optional WMI buffer to return the method results
+ *
+ * Invoke a WMI method, the caller must free the resulting data inside @out.
+ * Said data is guaranteed to be aligned on a 8-byte boundary.
+ *
+ * Return: 0 on success or negative error code on failure.
+ */
+int wmidev_invoke_method(struct wmi_device *wdev, u8 instance, u32 method_id,
+			 const struct wmi_buffer *in, struct wmi_buffer *out)
+{
+	struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev);
+	struct acpi_buffer aout = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_buffer ain;
+	union acpi_object *obj;
+	acpi_status status;
+	int ret;
+
+	if (wblock->gblock.flags & ACPI_WMI_STRING) {
+		ret = wmi_marshal_string(in, &ain);
+		if (ret < 0)
+			return ret;
+	} else {
+		if (in->length > U32_MAX)
+			return -E2BIG;
+
+		ain.length = in->length;
+		ain.pointer = in->data;
+	}
+
+	if (out)
+		status = wmidev_evaluate_method(wdev, instance, method_id, &ain, &aout);
+	else
+		status = wmidev_evaluate_method(wdev, instance, method_id, &ain, NULL);
+
+	if (wblock->gblock.flags & ACPI_WMI_STRING)
+		kfree(ain.pointer);
+
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	if (!out)
+		return 0;
+
+	obj = aout.pointer;
+	if (!obj) {
+		out->length = 0;
+		out->data = ZERO_SIZE_PTR;
+
+		return 0;
+	}
+
+	ret = wmi_unmarshal_acpi_object(obj, out);
+	kfree(obj);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wmidev_invoke_method);
+
 static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
 				 struct acpi_buffer *out)
 {
@@ -432,7 +499,7 @@ acpi_status wmi_query_block(const char *guid_string, u8 instance,
 EXPORT_SYMBOL_GPL(wmi_query_block);
 
 /**
- * wmidev_block_query - Return contents of a WMI block
+ * wmidev_block_query - Return contents of a WMI block (deprectated)
  * @wdev: A wmi bus device from a driver
  * @instance: Instance index
  *
@@ -452,6 +519,33 @@ union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance)
 }
 EXPORT_SYMBOL_GPL(wmidev_block_query);
 
+/**
+ * wmidev_query_block - Return contents of a WMI data block
+ * @wdev: A wmi bus device from a driver
+ * @instance: Instance index
+ * @out: WMI buffer to fill
+ *
+ * Query a WMI data block, the caller must free the resulting data inside @out.
+ * Said data is guaranteed to be aligned on a 8-byte boundary.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int wmidev_query_block(struct wmi_device *wdev, u8 instance, struct wmi_buffer *out)
+{
+	union acpi_object *obj;
+	int ret;
+
+	obj = wmidev_block_query(wdev, instance);
+	if (!obj)
+		return -EIO;
+
+	ret = wmi_unmarshal_acpi_object(obj, out);
+	kfree(obj);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wmidev_query_block);
+
 /**
  * wmi_set_block - Write to a WMI block (deprecated)
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
@@ -486,7 +580,7 @@ acpi_status wmi_set_block(const char *guid_string, u8 instance, const struct acp
 EXPORT_SYMBOL_GPL(wmi_set_block);
 
 /**
- * wmidev_block_set - Write to a WMI block
+ * wmidev_block_set - Write to a WMI block (deprecated)
  * @wdev: A wmi bus device from a driver
  * @instance: Instance index
  * @in: Buffer containing new values for the data block
@@ -535,6 +629,46 @@ acpi_status wmidev_block_set(struct wmi_device *wdev, u8 instance, const struct
 }
 EXPORT_SYMBOL_GPL(wmidev_block_set);
 
+/**
+ * wmidev_set_block - Write to a WMI data block
+ * @wdev: A wmi bus device from a driver
+ * @instance: Instance index
+ * @in: WMI buffer containing new values for the data block
+ *
+ * Write the content of @in into a WMI data block.
+ *
+ * Return: 0 on success or negative error code on failure.
+ */
+int wmidev_set_block(struct wmi_device *wdev, u8 instance, const struct wmi_buffer *in)
+{
+	struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev);
+	struct acpi_buffer buffer;
+	acpi_status status;
+	int ret;
+
+	if (wblock->gblock.flags & ACPI_WMI_STRING) {
+		ret = wmi_marshal_string(in, &buffer);
+		if (ret < 0)
+			return ret;
+	} else {
+		if (in->length > U32_MAX)
+			return -E2BIG;
+
+		buffer.length = in->length;
+		buffer.pointer = in->data;
+	}
+
+	status = wmidev_block_set(wdev, instance, &buffer);
+	if (wblock->gblock.flags & ACPI_WMI_STRING)
+		kfree(buffer.pointer);
+
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wmidev_set_block);
+
 /**
  * wmi_install_notify_handler - Register handler for WMI events (deprecated)
  * @guid: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
@@ -862,7 +996,7 @@ static int wmi_dev_probe(struct device *dev)
 		return -ENODEV;
 	}
 
-	if (wdriver->notify) {
+	if (wdriver->notify || wdriver->notify_new) {
 		if (test_bit(WMI_NO_EVENT_DATA, &wblock->flags) && !wdriver->no_notify_data)
 			return -ENODEV;
 	}
@@ -1221,6 +1355,8 @@ static int wmi_get_notify_data(struct wmi_block *wblock, union acpi_object **obj
 static void wmi_notify_driver(struct wmi_block *wblock, union acpi_object *obj)
 {
 	struct wmi_driver *driver = to_wmi_driver(wblock->dev.dev.driver);
+	struct wmi_buffer buffer;
+	int ret;
 
 	if (!obj && !driver->no_notify_data) {
 		dev_warn(&wblock->dev.dev, "Event contains no event data\n");
@@ -1229,6 +1365,22 @@ static void wmi_notify_driver(struct wmi_block *wblock, union acpi_object *obj)
 
 	if (driver->notify)
 		driver->notify(&wblock->dev, obj);
+
+	if (driver->notify_new) {
+		if (!obj) {
+			driver->notify_new(&wblock->dev, NULL);
+			return;
+		}
+
+		ret = wmi_unmarshal_acpi_object(obj, &buffer);
+		if (ret < 0) {
+			dev_warn(&wblock->dev.dev, "Failed to unmarshal event data: %d\n", ret);
+			return;
+		}
+
+		driver->notify_new(&wblock->dev, &buffer);
+		kfree(buffer.data);
+	}
 }
 
 static int wmi_notify_device(struct device *dev, void *data)
diff --git a/drivers/platform/wmi/internal.h b/drivers/platform/wmi/internal.h
new file mode 100644
index 000000000000..707d1a4711e0
--- /dev/null
+++ b/drivers/platform/wmi/internal.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Internal interfaces used by the WMI core.
+ *
+ * Copyright (C) 2025 Armin Wolf <W_Armin@....de>
+ */
+
+#ifndef _WMI_INTERNAL_H_
+#define _WMI_INTERNAL_H_
+
+int wmi_unmarshal_acpi_object(const union acpi_object *obj, struct wmi_buffer *buffer);
+int wmi_marshal_string(const struct wmi_buffer *buffer, struct acpi_buffer *out);
+
+#endif /* _WMI_INTERNAL_H_ */
diff --git a/drivers/platform/wmi/marshalling.c b/drivers/platform/wmi/marshalling.c
new file mode 100644
index 000000000000..476b87b145e3
--- /dev/null
+++ b/drivers/platform/wmi/marshalling.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * ACPI-WMI buffer marshalling.
+ *
+ * Copyright (C) 2025 Armin Wolf <W_Armin@....de>
+ */
+
+#include <linux/acpi.h>
+#include <linux/overflow.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
+#include <linux/wmi.h>
+
+#include <kunit/visibility.h>
+
+#include "internal.h"
+
+static int wmi_adjust_buffer_length(size_t *length, const union acpi_object *obj)
+{
+	size_t alignment, size;
+
+	switch (obj->type) {
+	case ACPI_TYPE_INTEGER:
+		/*
+		 * Integers are threated as 32 bit even if the ACPI DSDT
+		 * declares 64 bit integer width.
+		 */
+		alignment = 4;
+		size = sizeof(u32);
+		break;
+	case ACPI_TYPE_STRING:
+		/*
+		 * Strings begin with a single little-endian 16-bit field containing
+		 * the string length in bytes and are encoded as UTF-16LE with a terminating
+		 * nul character.
+		 */
+		if (obj->string.length + 1 > U16_MAX / 2)
+			return -EOVERFLOW;
+
+		alignment = 2;
+		size = struct_size_t(struct wmi_string, chars, obj->string.length + 1);
+		break;
+	case ACPI_TYPE_BUFFER:
+		/*
+		 * Buffers are copied as-is.
+		 */
+		alignment = 1;
+		size = obj->buffer.length;
+		break;
+	default:
+		return -EPROTO;
+	}
+
+	*length = size_add(ALIGN(*length, alignment), size);
+
+	return 0;
+}
+
+static int wmi_obj_get_buffer_length(const union acpi_object *obj, size_t *length)
+{
+	size_t total = 0;
+	int ret;
+
+	if (obj->type == ACPI_TYPE_PACKAGE) {
+		for (int i = 0; i < obj->package.count; i++) {
+			ret = wmi_adjust_buffer_length(&total, &obj->package.elements[i]);
+			if (ret < 0)
+				return ret;
+		}
+	} else {
+		ret = wmi_adjust_buffer_length(&total, obj);
+		if (ret < 0)
+			return ret;
+	}
+
+	*length = total;
+
+	return 0;
+}
+
+static int wmi_obj_transform_simple(const union acpi_object *obj, u8 *buffer, size_t *consumed)
+{
+	struct wmi_string *string;
+	size_t length;
+	__le32 value;
+	u8 *aligned;
+
+	switch (obj->type) {
+	case ACPI_TYPE_INTEGER:
+		aligned = PTR_ALIGN(buffer, 4);
+		length = sizeof(value);
+
+		value = cpu_to_le32(obj->integer.value);
+		memcpy(aligned, &value, length);
+		break;
+	case ACPI_TYPE_STRING:
+		aligned = PTR_ALIGN(buffer, 2);
+		string = (struct wmi_string *)aligned;
+		length = struct_size(string, chars, obj->string.length + 1);
+
+		/* We do not have to worry about unaligned accesses here as the WMI
+		 * string will already be aligned on a two-byte boundary.
+		 */
+		string->length = cpu_to_le16((obj->string.length + 1) * 2);
+		for (int i = 0; i < obj->string.length; i++)
+			string->chars[i] = cpu_to_le16(obj->string.pointer[i]);
+
+		/*
+		 * The Windows WMI-ACPI driver always emits a terminating nul character,
+		 * so we emulate this behavior here as well.
+		 */
+		string->chars[obj->string.length] = '\0';
+		break;
+	case ACPI_TYPE_BUFFER:
+		aligned = buffer;
+		length = obj->buffer.length;
+
+		memcpy(aligned, obj->buffer.pointer, length);
+		break;
+	default:
+		return -EPROTO;
+	}
+
+	*consumed = (aligned - buffer) + length;
+
+	return 0;
+}
+
+static int wmi_obj_transform(const union acpi_object *obj, u8 *buffer)
+{
+	size_t consumed;
+	int ret;
+
+	if (obj->type == ACPI_TYPE_PACKAGE) {
+		for (int i = 0; i < obj->package.count; i++) {
+			ret = wmi_obj_transform_simple(&obj->package.elements[i], buffer,
+						       &consumed);
+			if (ret < 0)
+				return ret;
+
+			buffer += consumed;
+		}
+	} else {
+		ret = wmi_obj_transform_simple(obj, buffer, &consumed);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+int wmi_unmarshal_acpi_object(const union acpi_object *obj, struct wmi_buffer *buffer)
+{
+	ssize_t length;
+	u8 *data;
+	int ret;
+
+	ret = wmi_obj_get_buffer_length(obj, &length);
+	if (ret < 0)
+		return ret;
+
+	data = kzalloc(length, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = wmi_obj_transform(obj, data);
+	if (ret < 0) {
+		kfree(data);
+		return ret;
+	}
+
+	buffer->length = length;
+	buffer->data = data;
+
+	return 0;
+}
+EXPORT_SYMBOL_IF_KUNIT(wmi_unmarshal_acpi_object);
+
+int wmi_marshal_string(const struct wmi_buffer *buffer, struct acpi_buffer *out)
+{
+	const struct wmi_string *string;
+	u16 length, value;
+	size_t chars;
+	char *str;
+
+	if (buffer->length < sizeof(*string))
+		return -ENODATA;
+
+	string = buffer->data;
+	length = get_unaligned_le16(&string->length);
+	if (buffer->length < sizeof(*string) + length)
+		return -ENODATA;
+
+	/* Each character needs to be 16 bits long */
+	if (length % 2)
+		return -EINVAL;
+
+	chars = length / 2;
+	str = kmalloc(chars + 1, GFP_KERNEL);
+	if (!str)
+		return -ENOMEM;
+
+	for (int i = 0; i < chars; i++) {
+		value = get_unaligned_le16(&string->chars[i]);
+
+		/* ACPI only accepts ASCII strings */
+		if (value > 0x7F) {
+			kfree(str);
+			return -EINVAL;
+		}
+
+		str[i] = value & 0xFF;
+
+		/*
+		 * ACPI strings should only contain a single nul character at the end.
+		 * Because of this we must not copy any padding from the WMI string.
+		 */
+		if (!value) {
+			/* ACPICA wants the length of the string without the nul character */
+			out->length = i;
+			out->pointer = str;
+			return 0;
+		}
+	}
+
+	str[chars] = '\0';
+
+	out->length = chars;
+	out->pointer = str;
+
+	return 0;
+}
+EXPORT_SYMBOL_IF_KUNIT(wmi_marshal_string);
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index 665ea7dc8a92..4c2fc3c1f0de 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -11,6 +11,7 @@
 #include <linux/device.h>
 #include <linux/acpi.h>
 #include <linux/mod_devicetable.h>
+#include <linux/types.h>
 
 /**
  * struct wmi_device - WMI device structure
@@ -36,6 +37,37 @@ struct wmi_device {
  */
 #define to_wmi_device(device)	container_of_const(device, struct wmi_device, dev)
 
+/**
+ * struct wmi_buffer - WMI data buffer
+ * @length: Buffer length in bytes
+ * @data: Pointer to the buffer content
+ *
+ * This structure is used to exchange data with the WMI driver core.
+ */
+struct wmi_buffer {
+	size_t length;
+	void *data;
+};
+
+/**
+ * struct wmi_string - WMI string representation
+ * @length: Size of @chars in bytes
+ * @chars: UTF16-LE characters with optional nul termination and padding
+ *
+ * This structure is used when exchanging string data over the WMI interface.
+ */
+struct wmi_string {
+	__le16 length;
+	__le16 chars[];
+} __packed;
+
+int wmidev_invoke_method(struct wmi_device *wdev, u8 instance, u32 method_id,
+			 const struct wmi_buffer *in, struct wmi_buffer *out);
+
+int wmidev_query_block(struct wmi_device *wdev, u8 instance, struct wmi_buffer *out);
+
+int wmidev_set_block(struct wmi_device *wdev, u8 instance, const struct wmi_buffer *in);
+
 acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, u32 method_id,
 				   const struct acpi_buffer *in, struct acpi_buffer *out);
 
@@ -54,9 +86,11 @@ u8 wmidev_instance_count(struct wmi_device *wdev);
  * @probe: Callback for device binding
  * @remove: Callback for device unbinding
  * @shutdown: Callback for device shutdown
- * @notify: Callback for receiving WMI events
+ * @notify: Callback for receiving WMI events (deprecated)
+ * @notify_new: Callback for receiving WMI events
  *
- * This represents WMI drivers which handle WMI devices.
+ * This represents WMI drivers which handle WMI devices. The data inside the buffer
+ * passed to the @notify_new callback is guaranteed to be aligned on a 8-byte boundary.
  */
 struct wmi_driver {
 	struct device_driver driver;
@@ -68,6 +102,7 @@ struct wmi_driver {
 	void (*remove)(struct wmi_device *wdev);
 	void (*shutdown)(struct wmi_device *wdev);
 	void (*notify)(struct wmi_device *device, union acpi_object *data);
+	void (*notify_new)(struct wmi_device *device, const struct wmi_buffer *data);
 };
 
 /**
-- 
2.39.5

Powered by blists - more mailing lists