public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] rust: WMI abstractions
@ 2025-12-21 18:22 Gladyshev Ilya
  2025-12-21 18:22 ` [RFC PATCH 1/3] rust: implement wrapper for acpi_object Gladyshev Ilya
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-21 18:22 UTC (permalink / raw)
  To: foxido @ foxido . dev-cc= Rafael J. Wysocki
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein, Armin Wolf,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi,
	Gladyshev Ilya

Overview
========
This patchset was developed some time ago out of pure curiosity
about the R4L project, but I believe it may still be useful, so
I decided to resume and post this work.
The goal of my curiosity is to develop a simple WMI driver in Rust
for unsupported laptop (I have some laying around) or, as a last
resort, submit a rewrite of redmi-wmi as a sample driver -- if that
would be acceptable.

I know that there is an undergoing API change in the WMI subsystem[1],
however it doesn't change the abstracted surface a lot.

This patchset consists of 3 patches:
1. Wrapper around ACPI objects (this may be unneeded after [1])
2. WMI abstractions
3. Sample driver for demonstration purposes only

Why RFC?
========
1. No real users for now
2. I have a feeling that I am doing something very wrong

[1]: https://lore.kernel.org/platform-driver-x86/20251220204622.3541-1-W_Armin@gmx.de/

Gladyshev Ilya (3):
  rust: implement wrapper for acpi_object
  rust: introduce WMI abstractions
  rust: sample driver for WMI demonstrations

 drivers/platform/x86/Makefile        |   1 +
 drivers/platform/x86/redmi_wmi_rs.rs |  60 ++++++
 rust/bindings/bindings_helper.h      |   1 +
 rust/kernel/acpi.rs                  | 103 +++++++++++
 rust/kernel/lib.rs                   |   2 +
 rust/kernel/wmi.rs                   | 267 +++++++++++++++++++++++++++
 6 files changed, 434 insertions(+)
 create mode 100644 drivers/platform/x86/redmi_wmi_rs.rs
 create mode 100644 rust/kernel/wmi.rs


base-commit: 9094662f6707d1d4b53d18baba459604e8bb0783
-- 
2.51.1.dirty


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 1/3] rust: implement wrapper for acpi_object
  2025-12-21 18:22 [RFC PATCH 0/3] rust: WMI abstractions Gladyshev Ilya
@ 2025-12-21 18:22 ` Gladyshev Ilya
  2025-12-22 11:35   ` Danilo Krummrich
  2025-12-22 19:32   ` Rafael J. Wysocki
  2025-12-21 18:22 ` [RFC PATCH 2/3] rust: introduce WMI abstractions Gladyshev Ilya
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-21 18:22 UTC (permalink / raw)
  To: foxido @ foxido . dev-cc= Rafael J. Wysocki
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein, Armin Wolf,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi,
	Gladyshev Ilya

ACPI Object is represented via union on C-side. On Rust side, it's
zero-cost type wrapper for each ACPI Type, with individual methods for
getters and other interactions.

Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
---
 rust/kernel/acpi.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 9b8efa623130..0a164ca8cceb 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -63,3 +63,106 @@ macro_rules! acpi_device_table {
         $crate::module_device_table!("acpi", $module_table_name, $table_name);
     };
 }
+
+/// An ACPI object.
+///
+/// This structure represents the Rust abstraction for a C [`struct acpi_object`].
+/// You probably want to convert it into actual object type.
+///
+/// # Example
+/// ```
+/// # use kernel::prelude::*
+/// use kernel::acpi::{AcpiObject};
+///
+/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
+///     if obj.type_id() != AcpiBuffer::ACPI_TYPE {
+///         return Err(EINVAL);
+///     }
+///
+///     let obj: &AcpiBuffer = obj.try_into()?;
+///
+///     Ok(obj.payload()[0])
+/// }
+/// ```
+#[repr(transparent)]
+pub struct AcpiObject(bindings::acpi_object);
+
+impl AcpiObject {
+    /// Returns object type (see `acpitypes.h`)
+    pub fn type_id(&self) -> u32 {
+        // SAFETY: `type` field is valid in all union variants
+        unsafe { self.0.type_ }
+    }
+}
+
+/// Generate AcpiObject subtype
+///
+/// For given subtype implements
+/// - `TryFrom<&AcpiObject> for &SubType` trait
+/// - unsafe try_from_unchecked() with same semantics, but without type check
+macro_rules! acpi_object_subtype {
+    ($subtype_name:ident <- ($acpi_type:ident, $field_name:ident, $union_type:ty)) => {
+        /// Wraps `acpi_object` subtype
+        #[repr(transparent)]
+        pub struct $subtype_name($union_type);
+
+        impl TryFrom<&AcpiObject> for &$subtype_name {
+            type Error = Error;
+
+            fn try_from(value: &AcpiObject) -> core::result::Result<Self, Self::Error> {
+                // SAFETY: type_ field present in all union types and is always valid
+                let real_type = unsafe { value.0.type_ };
+
+                if (real_type != $subtype_name::ACPI_TYPE) {
+                    return Err(EINVAL);
+                }
+
+                // SAFETY: We validated union subtype
+                Ok(unsafe {
+                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
+                })
+            }
+        }
+
+        impl $subtype_name {
+            /// This ACPI type int value (see `acpitypes.h`)
+            pub const ACPI_TYPE: u32 = bindings::$acpi_type;
+
+            /// Converts AcpiObject reference into exact ACPI type wrapper
+            ///
+            /// # Safety
+            ///
+            /// Assumes that value is correct (`Self`) subtype
+            pub unsafe fn try_from_unchecked(value: &AcpiObject) -> &Self {
+                // SAFETY: Only unsafety comes from unchecked transformation and
+                // we transfered
+                unsafe {
+                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
+                }
+            }
+        }
+    };
+}
+
+acpi_object_subtype!(AcpiInteger
+    <- (ACPI_TYPE_INTEGER, integer, bindings::acpi_object__bindgen_ty_1));
+acpi_object_subtype!(AcpiString
+    <- (ACPI_TYPE_STRING, string, bindings::acpi_object__bindgen_ty_2));
+acpi_object_subtype!(AcpiBuffer
+    <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
+acpi_object_subtype!(AcpiPackage
+    <- (ACPI_TYPE_PACKAGE, package, bindings::acpi_object__bindgen_ty_4));
+acpi_object_subtype!(AcpiReference
+    <- (ACPI_TYPE_LOCAL_REFERENCE, reference, bindings::acpi_object__bindgen_ty_5));
+acpi_object_subtype!(AcpiProcessor
+    <- (ACPI_TYPE_PROCESSOR, processor, bindings::acpi_object__bindgen_ty_6));
+acpi_object_subtype!(AcpiPowerResource
+    <- (ACPI_TYPE_POWER, power_resource, bindings::acpi_object__bindgen_ty_7));
+
+impl AcpiBuffer {
+    /// Get Buffer's content
+    pub fn payload(&self) -> &[u8] {
+        // SAFETY: (pointer, length) indeed represents byte slice
+        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
+    }
+}
-- 
2.51.1.dirty


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 2/3] rust: introduce WMI abstractions
  2025-12-21 18:22 [RFC PATCH 0/3] rust: WMI abstractions Gladyshev Ilya
  2025-12-21 18:22 ` [RFC PATCH 1/3] rust: implement wrapper for acpi_object Gladyshev Ilya
@ 2025-12-21 18:22 ` Gladyshev Ilya
  2025-12-22 11:50   ` Danilo Krummrich
  2025-12-25 18:06   ` Armin Wolf
  2025-12-21 18:22 ` [RFC PATCH 3/3] rust: sample driver for WMI demonstrations Gladyshev Ilya
  2025-12-22 11:52 ` [RFC PATCH 0/3] rust: WMI abstractions Danilo Krummrich
  3 siblings, 2 replies; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-21 18:22 UTC (permalink / raw)
  To: foxido @ foxido . dev-cc= Rafael J. Wysocki
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein, Armin Wolf,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi,
	Gladyshev Ilya

Implement the basic WMI subsystem abstractions required to write a
WMI device driver with or without the need for initial device data.
This includes the following data structures:

The `wmi::Driver` trait represents the interface to the driver.

The `wmi::Device` abstraction represents a `struct wmi_device`.

In order to provide the WMI specific parts to a generic
`driver::Registration` the `driver::RegistrationOps` trait is
implemented by `wmi::Adapter`.

Everything from C side is supported, except shutdown action

Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/wmi.rs              | 267 ++++++++++++++++++++++++++++++++
 3 files changed, 270 insertions(+)
 create mode 100644 rust/kernel/wmi.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index a067038b4b42..f9671280c6b5 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -85,6 +85,7 @@
 #include <linux/usb.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
+#include <linux/wmi.h>
 #include <linux/xarray.h>
 #include <trace/events/rust_sample.h>
 
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f812cf120042..db3e649d26eb 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -151,6 +151,8 @@
 pub mod uaccess;
 #[cfg(CONFIG_USB = "y")]
 pub mod usb;
+#[cfg(CONFIG_ACPI_WMI)]
+pub mod wmi;
 pub mod workqueue;
 pub mod xarray;
 
diff --git a/rust/kernel/wmi.rs b/rust/kernel/wmi.rs
new file mode 100644
index 000000000000..018e88d01e8c
--- /dev/null
+++ b/rust/kernel/wmi.rs
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the WMI devices.
+//!
+//! C header: [`include/linux/wmi.h`](srctree/include/linux/wmi.h)
+
+use crate::{
+    acpi::AcpiObject,
+    device,
+    device_id::{RawDeviceId, RawDeviceIdIndex},
+    driver,
+    error::{from_result, to_result, VTABLE_DEFAULT_ERROR},
+    prelude::*,
+    types::Opaque,
+};
+use core::{
+    marker::PhantomData,
+    mem::MaybeUninit,
+    ptr::{addr_of_mut, NonNull},
+};
+use macros::vtable;
+
+/// [`IdTable`](kernel::device_id::IdTable) type for WMI.
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// The WMI driver trait.
+#[vtable]
+pub trait Driver: Send {
+    /// The type holding information about each one of the device ids supported by the driver.
+    type IdInfo: 'static;
+
+    /// The table of device ids supported by the driver.
+    const TABLE: IdTable<Self::IdInfo>;
+
+    /// WMI driver probe.
+    ///
+    /// Called when a new WMI device is bound to this driver.
+    /// Implementers should attempt to initialize the driver here.
+    fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo) -> impl PinInit<Self, Error>;
+
+    /// WMI device notify.
+    ///
+    /// Called when new WMI event received from bounded device
+    fn notify(&self, _dev: &Device<device::Core>, _event: &AcpiObject) {
+        build_error!(VTABLE_DEFAULT_ERROR);
+    }
+
+    /// WMI driver remove.
+    fn remove(self: Pin<KBox<Self>>, _dev: &Device<device::Core>) {
+        build_error!(VTABLE_DEFAULT_ERROR);
+    }
+}
+
+/// A WMI device.
+///
+/// This structure represents the Rust abstraction for a C [`struct wmi_device`].
+/// The implementation abstracts the usage of a C [`struct wmi_device`] passed
+/// in from the C side.
+pub struct Device<Cxt: device::DeviceContext = device::Normal> {
+    inner: Opaque<bindings::wmi_device>,
+    _p: PhantomData<Cxt>,
+}
+
+impl<Cxt: device::DeviceContext> Device<Cxt> {
+    fn as_raw(&self) -> *mut bindings::wmi_device {
+        self.inner.get()
+    }
+}
+
+/// An adapter for the registration of WMI drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::wmi_driver;
+
+    unsafe fn register(
+        wdrv: &Opaque<Self::RegType>,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        macro_rules! map_callback {
+            ($flag:ident -> $callback:ident) => {
+                if T::$flag {
+                    Some(Self::$callback)
+                } else {
+                    None
+                }
+            };
+        }
+
+        // SAFETY: It's safe to set the fields of `struct wmi_driver` on initialization.
+        unsafe {
+            (*wdrv.get()).driver.name = name.as_char_ptr();
+            (*wdrv.get()).driver.probe_type = bindings::probe_type_PROBE_PREFER_ASYNCHRONOUS;
+            (*wdrv.get()).id_table = T::TABLE.as_ptr();
+            (*wdrv.get()).probe = map_callback!(HAS_PROBE -> probe_callback);
+            (*wdrv.get()).notify = map_callback!(HAS_NOTIFY -> notify_callback);
+            (*wdrv.get()).remove = map_callback!(HAS_REMOVE -> remove_callback);
+            (*wdrv.get()).shutdown = None;
+            (*wdrv.get()).no_singleton = true;
+        }
+
+        // SAFETY: `wdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::__wmi_driver_register(wdrv.get(), module.0) })
+    }
+
+    unsafe fn unregister(wdrv: &Opaque<Self::RegType>) {
+        // SAFETY: `wdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::wmi_driver_unregister(wdrv.get()) };
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    extern "C" fn probe_callback(
+        wdev: *mut bindings::wmi_device,
+        id: *const c_void,
+    ) -> kernel::ffi::c_int {
+        // SAFETY: The WMI core only ever calls the probe callback with a valid pointer to a
+        // `struct wmi_device`.
+        //
+        // INVARIANT: `wdev` is valid for the duration of `probe_callback()`.
+        let wdev = unsafe { &*wdev.cast::<Device<device::CoreInternal>>() };
+
+        let id = id as usize;
+        let info = T::TABLE.info(id);
+
+        from_result(|| {
+            let data = T::probe(wdev, info);
+
+            wdev.as_ref().set_drvdata(data)?;
+            Ok(0)
+        })
+    }
+
+    extern "C" fn notify_callback(
+        wdev: *mut bindings::wmi_device,
+        obj: *mut bindings::acpi_object,
+    ) {
+        // SAFETY: The WMI system only ever calls the notify callback with a valid pointer to a
+        // `struct wmi_device`.
+        let wdev = unsafe { &*wdev.cast::<Device<device::CoreInternal>>() };
+        // SAFETY: AcpiObject is repr(transparent) wrapper
+        let obj: &AcpiObject = unsafe { &*(obj as *const AcpiObject) };
+
+        // SAFETY: `notify_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
+        // and stored a `T`.
+        let this: &T = unsafe { &wdev.as_ref().drvdata_borrow::<T>() };
+        this.notify(wdev, obj);
+    }
+
+    extern "C" fn remove_callback(wdev: *mut bindings::wmi_device) {
+        // SAFETY: The WMI system only ever calls the remove callback with a valid pointer to a
+        // `struct wmi_device`.
+        let wdev = unsafe { &*wdev.cast::<Device<device::CoreInternal>>() };
+
+        // SAFETY: `remove_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
+        // and stored a `T`.
+        let this = unsafe { wdev.as_ref().drvdata_obtain::<T>() };
+        this.remove(wdev);
+    }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+    fn as_ref(&self) -> &device::Device<Ctx> {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct platform_device`.
+        let dev = unsafe { addr_of_mut!((*self.inner.get()).dev) };
+
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+kernel::impl_device_context_deref!(unsafe { Device });
+kernel::impl_device_context_into_aref!(Device);
+
+// SAFETY: Instances of `Device` are always reference-counted.
+unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
+        unsafe { bindings::get_device(self.as_ref().as_raw()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+        unsafe { bindings::put_device(&raw mut (*obj.as_ref().as_raw()).dev) }
+    }
+}
+
+/// Abstraction for the WMI device ID structure, i.e. [`struct wmi_device_id`].
+#[repr(transparent)]
+pub struct DeviceId(bindings::wmi_device_id);
+
+impl DeviceId {
+    /// Constructs new DeviceId from GUID string
+    pub const fn new(guid: &[u8; bindings::UUID_STRING_LEN as usize]) -> Self {
+        // SAFETY: FFI type is valid to be zero-initialized.
+        let mut inner: bindings::wmi_device_id = unsafe { MaybeUninit::zeroed().assume_init() };
+
+        build_assert!(inner.guid_string.len() == bindings::UUID_STRING_LEN as usize + 1);
+
+        // SAFETY: It's safe to copy UUID_STRING_LEN, because we validated lengths
+        // Also we leave last byte zeroed -> guid_string is valid C string
+        unsafe {
+            ::core::ptr::copy_nonoverlapping(
+                guid.as_ptr(),
+                &raw mut inner.guid_string[0],
+                bindings::UUID_STRING_LEN as usize,
+            );
+        }
+
+        Self(inner)
+    }
+}
+
+// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `wmi_device_id` and does not add
+// additional invariants, so it's safe to transmute to `RawType`.
+unsafe impl RawDeviceId for DeviceId {
+    type RawType = bindings::wmi_device_id;
+}
+
+// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `context` field.
+unsafe impl RawDeviceIdIndex for DeviceId {
+    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::wmi_device_id, context);
+
+    fn index(&self) -> usize {
+        self.0.context as usize
+    }
+}
+
+/// Declares a kernel module that exposes a single WMI driver.
+///
+/// # Examples
+///
+/// ```ignore
+/// module_wmi_driver! {
+///     type: MyDriver,
+///     name: "Module name",
+///     author: ["Author name"],
+///     description: "Description",
+///     license: "GPL v2",
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_wmi_driver {
+    ($($f:tt)*) => {
+        $crate::module_driver!(<T>, $crate::wmi::Adapter<T>, { $($f)* });
+    }
+}
+
+/// Create a WMI `IdTable` with its alias for modpost.
+#[macro_export]
+macro_rules! wmi_device_table {
+    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+        const $table_name: $crate::device_id::IdArray<
+            $crate::wmi::DeviceId,
+            $id_info_type,
+            { $table_data.len() },
+        > = $crate::device_id::IdArray::new($table_data);
+
+        $crate::module_device_table!("wmi", $module_table_name, $table_name);
+    };
+}
-- 
2.51.1.dirty


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 3/3] rust: sample driver for WMI demonstrations
  2025-12-21 18:22 [RFC PATCH 0/3] rust: WMI abstractions Gladyshev Ilya
  2025-12-21 18:22 ` [RFC PATCH 1/3] rust: implement wrapper for acpi_object Gladyshev Ilya
  2025-12-21 18:22 ` [RFC PATCH 2/3] rust: introduce WMI abstractions Gladyshev Ilya
@ 2025-12-21 18:22 ` Gladyshev Ilya
  2025-12-22 11:52 ` [RFC PATCH 0/3] rust: WMI abstractions Danilo Krummrich
  3 siblings, 0 replies; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-21 18:22 UTC (permalink / raw)
  To: foxido @ foxido . dev-cc= Rafael J. Wysocki
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein, Armin Wolf,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi,
	Gladyshev Ilya

This driver is for RFC demonstration only. It will be removed before
real submission, however it's working and can be transformed into
sample driver
---
 drivers/platform/x86/Makefile        |  1 +
 drivers/platform/x86/redmi_wmi_rs.rs | 60 ++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 drivers/platform/x86/redmi_wmi_rs.rs

diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index d25762f7114f..3045e42618ef 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MXM_WMI)			+= mxm-wmi.o
 obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT)	+= nvidia-wmi-ec-backlight.o
 obj-$(CONFIG_XIAOMI_WMI)		+= xiaomi-wmi.o
 obj-$(CONFIG_REDMI_WMI)			+= redmi-wmi.o
+obj-m					+= redmi_wmi_rs.o
 obj-$(CONFIG_GIGABYTE_WMI)		+= gigabyte-wmi.o
 
 # Acer
diff --git a/drivers/platform/x86/redmi_wmi_rs.rs b/drivers/platform/x86/redmi_wmi_rs.rs
new file mode 100644
index 000000000000..65300bad022d
--- /dev/null
+++ b/drivers/platform/x86/redmi_wmi_rs.rs
@@ -0,0 +1,60 @@
+//! This is SAMPLE DRIVER
+//! It doesn't aim into upstream and serves only demonstration purposes for
+//! RFC patchset
+
+use kernel::sync::atomic::{Atomic, Relaxed};
+
+use kernel::{
+    acpi::AcpiObject,
+    device, module_wmi_driver, pr_info,
+    prelude::*,
+    wmi::{self, Device, DeviceId, Driver},
+    wmi_device_table,
+};
+
+wmi_device_table!(
+    REDMI_TABLE,
+    MODULE_REDMI_TABLE,
+    <RedmiWMIDriver as Driver>::IdInfo,
+    [(DeviceId::new(b"46C93E13-EE9B-4262-8488-563BCA757FEF"), 12)]
+);
+
+struct RedmiWMIDriver {
+    probed_val: i32,
+    invokation_cnt: Atomic<i64>,
+}
+
+#[vtable]
+impl wmi::Driver for RedmiWMIDriver {
+    type IdInfo = i32;
+
+    const TABLE: &'static dyn kernel::device_id::IdTable<DeviceId, Self::IdInfo> = &REDMI_TABLE;
+
+    fn probe(
+        _: &Device<kernel::device::Core>,
+        id_info: &Self::IdInfo,
+    ) -> impl PinInit<Self, Error> {
+        pr_info!("Rust WMI Sample Driver probed with val {id_info}\n");
+
+        Ok(Self {
+            probed_val: *id_info,
+            invokation_cnt: Atomic::new(0),
+        })
+    }
+
+    fn notify(&self, _: &Device<device::Core>, _: &AcpiObject) {
+        pr_info!(
+            "Notified driver with probed_val: {}, invokation cnt: {}",
+            self.probed_val,
+            self.invokation_cnt.fetch_add(1, Relaxed)
+        );
+    }
+}
+
+module_wmi_driver!(
+    type: RedmiWMIDriver,
+    name: "redmi_wmi_sample",
+    authors: ["Gladyshev Ilya"],
+    description: "SAMPLE DRIVER for RFC demonstration only",
+    license: "GPL v2",
+);
-- 
2.51.1.dirty


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 1/3] rust: implement wrapper for acpi_object
  2025-12-21 18:22 ` [RFC PATCH 1/3] rust: implement wrapper for acpi_object Gladyshev Ilya
@ 2025-12-22 11:35   ` Danilo Krummrich
  2025-12-22 21:47     ` Gladyshev Ilya
  2025-12-22 19:32   ` Rafael J. Wysocki
  1 sibling, 1 reply; 17+ messages in thread
From: Danilo Krummrich @ 2025-12-22 11:35 UTC (permalink / raw)
  To: Gladyshev Ilya
  Cc: foxido @ foxido . dev-cc= Rafael J. Wysocki, Len Brown,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Tamir Duberstein, Armin Wolf, platform-driver-x86, linux-kernel,
	rust-for-linux, linux-acpi

On Sun Dec 21, 2025 at 7:22 PM CET, Gladyshev Ilya wrote:
> +/// An ACPI object.
> +///
> +/// This structure represents the Rust abstraction for a C [`struct acpi_object`].
> +/// You probably want to convert it into actual object type.

I think this is a good place to link the corresponding types.

> +///
> +/// # Example
> +/// ```
> +/// # use kernel::prelude::*
> +/// use kernel::acpi::{AcpiObject};

Braces not needed.

> +///
> +/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
> +///     if obj.type_id() != AcpiBuffer::ACPI_TYPE {
> +///         return Err(EINVAL);
> +///     }

Given the try_into() conversion below this check is unnecessary.

> +///     let obj: &AcpiBuffer = obj.try_into()?;
> +///
> +///     Ok(obj.payload()[0])
> +/// }
> +/// ```
> +#[repr(transparent)]
> +pub struct AcpiObject(bindings::acpi_object);
> +
> +impl AcpiObject {
> +    /// Returns object type (see `acpitypes.h`)
> +    pub fn type_id(&self) -> u32 {
> +        // SAFETY: `type` field is valid in all union variants

Here and in a lot of other places, please end with a period.

> +        unsafe { self.0.type_ }
> +    }
> +}
> +
> +/// Generate AcpiObject subtype
> +///
> +/// For given subtype implements
> +/// - `TryFrom<&AcpiObject> for &SubType` trait
> +/// - unsafe try_from_unchecked() with same semantics, but without type check
> +macro_rules! acpi_object_subtype {
> +    ($subtype_name:ident <- ($acpi_type:ident, $field_name:ident, $union_type:ty)) => {
> +        /// Wraps `acpi_object` subtype
> +        #[repr(transparent)]
> +        pub struct $subtype_name($union_type);
> +
> +        impl TryFrom<&AcpiObject> for &$subtype_name {
> +            type Error = Error;
> +
> +            fn try_from(value: &AcpiObject) -> core::result::Result<Self, Self::Error> {
> +                // SAFETY: type_ field present in all union types and is always valid
> +                let real_type = unsafe { value.0.type_ };
> +
> +                if (real_type != $subtype_name::ACPI_TYPE) {
> +                    return Err(EINVAL);
> +                }

This should just be

	if (value.type_id() != $subtype_name::ACPI_TYPE) {
	    return Err(EINVAL);
	}

> +
> +                // SAFETY: We validated union subtype

When writing safety comments, please read the safety documentation of the
corresponding function and try to cover all requirements listed as bullet
points.

> +                Ok(unsafe {
> +                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
> +                })
> +            }
> +        }
> +
> +        impl $subtype_name {
> +            /// This ACPI type int value (see `acpitypes.h`)
> +            pub const ACPI_TYPE: u32 = bindings::$acpi_type;
> +
> +            /// Converts AcpiObject reference into exact ACPI type wrapper
> +            ///
> +            /// # Safety
> +            ///
> +            /// Assumes that value is correct (`Self`) subtype
> +            pub unsafe fn try_from_unchecked(value: &AcpiObject) -> &Self {

The name try_from_unchecked() implies that the function is fallible, but it
isn't. I suggest calling it something along the lines of cast_unchecked().

> +                // SAFETY: Only unsafety comes from unchecked transformation and
> +                // we transfered
> +                unsafe {
> +                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
> +                }
> +            }
> +        }
> +    };
> +}
> +
> +acpi_object_subtype!(AcpiInteger
> +    <- (ACPI_TYPE_INTEGER, integer, bindings::acpi_object__bindgen_ty_1));
> +acpi_object_subtype!(AcpiString
> +    <- (ACPI_TYPE_STRING, string, bindings::acpi_object__bindgen_ty_2));
> +acpi_object_subtype!(AcpiBuffer
> +    <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
> +acpi_object_subtype!(AcpiPackage
> +    <- (ACPI_TYPE_PACKAGE, package, bindings::acpi_object__bindgen_ty_4));
> +acpi_object_subtype!(AcpiReference
> +    <- (ACPI_TYPE_LOCAL_REFERENCE, reference, bindings::acpi_object__bindgen_ty_5));
> +acpi_object_subtype!(AcpiProcessor
> +    <- (ACPI_TYPE_PROCESSOR, processor, bindings::acpi_object__bindgen_ty_6));
> +acpi_object_subtype!(AcpiPowerResource
> +    <- (ACPI_TYPE_POWER, power_resource, bindings::acpi_object__bindgen_ty_7));
> +
> +impl AcpiBuffer {
> +    /// Get Buffer's content
> +    pub fn payload(&self) -> &[u8] {
> +        // SAFETY: (pointer, length) indeed represents byte slice
> +        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
> +    }
> +}

What about the values of the other types? How are they accessed?

Also, I think it would be better to use a Deref impl rather than a method.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 2/3] rust: introduce WMI abstractions
  2025-12-21 18:22 ` [RFC PATCH 2/3] rust: introduce WMI abstractions Gladyshev Ilya
@ 2025-12-22 11:50   ` Danilo Krummrich
  2025-12-25 18:06   ` Armin Wolf
  1 sibling, 0 replies; 17+ messages in thread
From: Danilo Krummrich @ 2025-12-22 11:50 UTC (permalink / raw)
  To: Gladyshev Ilya
  Cc: foxido @ foxido . dev-cc= Rafael J. Wysocki, Len Brown,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Tamir Duberstein, Armin Wolf, platform-driver-x86, linux-kernel,
	rust-for-linux, linux-acpi

On Sun Dec 21, 2025 at 7:22 PM CET, Gladyshev Ilya wrote:
> diff --git a/rust/kernel/wmi.rs b/rust/kernel/wmi.rs
> new file mode 100644
> index 000000000000..018e88d01e8c
> --- /dev/null
> +++ b/rust/kernel/wmi.rs
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Abstractions for the WMI devices.
> +//!
> +//! C header: [`include/linux/wmi.h`](srctree/include/linux/wmi.h)
> +
> +use crate::{
> +    acpi::AcpiObject,
> +    device,
> +    device_id::{RawDeviceId, RawDeviceIdIndex},
> +    driver,
> +    error::{from_result, to_result, VTABLE_DEFAULT_ERROR},
> +    prelude::*,
> +    types::Opaque,
> +};
> +use core::{
> +    marker::PhantomData,
> +    mem::MaybeUninit,
> +    ptr::{addr_of_mut, NonNull},
> +};
> +use macros::vtable;

Please use kernel vertical style.

[1] https://docs.kernel.org/rust/coding-guidelines.html#imports

> +
> +/// [`IdTable`](kernel::device_id::IdTable) type for WMI.
> +pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
> +
> +/// The WMI driver trait.
> +#[vtable]
> +pub trait Driver: Send {
> +    /// The type holding information about each one of the device ids supported by the driver.
> +    type IdInfo: 'static;
> +
> +    /// The table of device ids supported by the driver.
> +    const TABLE: IdTable<Self::IdInfo>;
> +
> +    /// WMI driver probe.
> +    ///
> +    /// Called when a new WMI device is bound to this driver.
> +    /// Implementers should attempt to initialize the driver here.
> +    fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo) -> impl PinInit<Self, Error>;
> +
> +    /// WMI device notify.
> +    ///
> +    /// Called when new WMI event received from bounded device
> +    fn notify(&self, _dev: &Device<device::Core>, _event: &AcpiObject) {

Please use Pin<&Self> instead.

> +        build_error!(VTABLE_DEFAULT_ERROR);
> +    }
> +
> +    /// WMI driver remove.
> +    fn remove(self: Pin<KBox<Self>>, _dev: &Device<device::Core>) {

We can't pass the driver's device private data by value here. As long as the
device is not fully unbound, there may still be calls to Device::drvdata();
please see also [2]). Hence, please use Pin<&Self> instead.

Also, please call this method unbind() to be consistent with other busses.

[2] https://lore.kernel.org/rust-for-linux/DEZMS6Y4A7XE.XE7EUBT5SJFJ@kernel.org/

> +        build_error!(VTABLE_DEFAULT_ERROR);
> +    }
> +}

<snip>

> +impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> +    fn as_ref(&self) -> &device::Device<Ctx> {
> +        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> +        // `struct platform_device`.
> +        let dev = unsafe { addr_of_mut!((*self.inner.get()).dev) };

Please use &raw mut instead.

> +
> +        // SAFETY: `dev` points to a valid `struct device`.
> +        unsafe { device::Device::from_raw(dev) }
> +    }
> +}
> +
> +kernel::impl_device_context_deref!(unsafe { Device });
> +kernel::impl_device_context_into_aref!(Device);
> +
> +// SAFETY: Instances of `Device` are always reference-counted.
> +unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +    fn inc_ref(&self) {
> +        // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> +        unsafe { bindings::get_device(self.as_ref().as_raw()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: NonNull<Self>) {
> +        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> +        unsafe { bindings::put_device(&raw mut (*obj.as_ref().as_raw()).dev) }
> +    }
> +}
> +
> +/// Abstraction for the WMI device ID structure, i.e. [`struct wmi_device_id`].
> +#[repr(transparent)]
> +pub struct DeviceId(bindings::wmi_device_id);
> +
> +impl DeviceId {
> +    /// Constructs new DeviceId from GUID string
> +    pub const fn new(guid: &[u8; bindings::UUID_STRING_LEN as usize]) -> Self {
> +        // SAFETY: FFI type is valid to be zero-initialized.
> +        let mut inner: bindings::wmi_device_id = unsafe { MaybeUninit::zeroed().assume_init() };
> +
> +        build_assert!(inner.guid_string.len() == bindings::UUID_STRING_LEN as usize + 1);
> +
> +        // SAFETY: It's safe to copy UUID_STRING_LEN, because we validated lengths
> +        // Also we leave last byte zeroed -> guid_string is valid C string
> +        unsafe {
> +            ::core::ptr::copy_nonoverlapping(
> +                guid.as_ptr(),
> +                &raw mut inner.guid_string[0],
> +                bindings::UUID_STRING_LEN as usize,
> +            );
> +        }

This does not compile without const_intrinsic_copy. I think we can enable it
though, since it should be stable since 1.83.

> +
> +        Self(inner)
> +    }
> +}

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 0/3] rust: WMI abstractions
  2025-12-21 18:22 [RFC PATCH 0/3] rust: WMI abstractions Gladyshev Ilya
                   ` (2 preceding siblings ...)
  2025-12-21 18:22 ` [RFC PATCH 3/3] rust: sample driver for WMI demonstrations Gladyshev Ilya
@ 2025-12-22 11:52 ` Danilo Krummrich
  2025-12-22 21:30   ` Gladyshev Ilya
  3 siblings, 1 reply; 17+ messages in thread
From: Danilo Krummrich @ 2025-12-22 11:52 UTC (permalink / raw)
  To: Gladyshev Ilya
  Cc: foxido @ foxido . dev-cc= Rafael J. Wysocki, Len Brown,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Tamir Duberstein, Armin Wolf, platform-driver-x86, linux-kernel,
	rust-for-linux, linux-acpi

On Sun Dec 21, 2025 at 7:22 PM CET, Gladyshev Ilya wrote:
> Overview
> ========
> This patchset was developed some time ago out of pure curiosity
> about the R4L project, but I believe it may still be useful, so
> I decided to resume and post this work.
> The goal of my curiosity is to develop a simple WMI driver in Rust
> for unsupported laptop (I have some laying around) or, as a last
> resort, submit a rewrite of redmi-wmi as a sample driver -- if that
> would be acceptable.

It depends on the subsystem maintainer, please also see [1].

[1] https://rust-for-linux.com/rust-reference-drivers

> Why RFC?
> ========
> 1. No real users for now

Above it does sound like you are working on a new WMI driver as well?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 1/3] rust: implement wrapper for acpi_object
  2025-12-21 18:22 ` [RFC PATCH 1/3] rust: implement wrapper for acpi_object Gladyshev Ilya
  2025-12-22 11:35   ` Danilo Krummrich
@ 2025-12-22 19:32   ` Rafael J. Wysocki
  2025-12-23 16:36     ` Gladyshev Ilya
  1 sibling, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2025-12-22 19:32 UTC (permalink / raw)
  To: Gladyshev Ilya
  Cc: foxido @ foxido . dev-cc= Rafael J. Wysocki, Len Brown,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, Armin Wolf,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi

On Sun, Dec 21, 2025 at 7:24 PM Gladyshev Ilya <foxido@foxido.dev> wrote:
>
> ACPI Object is represented via union on C-side. On Rust side, it's
> zero-cost type wrapper for each ACPI Type, with individual methods for
> getters and other interactions.

This is ACPICA stuff though and switching over ACPICA to Rust any time
soon is rather unlikely.

Is this really needed on the Rust side?

> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> ---
>  rust/kernel/acpi.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 103 insertions(+)
>
> diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
> index 9b8efa623130..0a164ca8cceb 100644
> --- a/rust/kernel/acpi.rs
> +++ b/rust/kernel/acpi.rs
> @@ -63,3 +63,106 @@ macro_rules! acpi_device_table {
>          $crate::module_device_table!("acpi", $module_table_name, $table_name);
>      };
>  }
> +
> +/// An ACPI object.
> +///
> +/// This structure represents the Rust abstraction for a C [`struct acpi_object`].
> +/// You probably want to convert it into actual object type.
> +///
> +/// # Example
> +/// ```
> +/// # use kernel::prelude::*
> +/// use kernel::acpi::{AcpiObject};
> +///
> +/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
> +///     if obj.type_id() != AcpiBuffer::ACPI_TYPE {
> +///         return Err(EINVAL);
> +///     }
> +///
> +///     let obj: &AcpiBuffer = obj.try_into()?;
> +///
> +///     Ok(obj.payload()[0])
> +/// }
> +/// ```
> +#[repr(transparent)]
> +pub struct AcpiObject(bindings::acpi_object);
> +
> +impl AcpiObject {
> +    /// Returns object type (see `acpitypes.h`)
> +    pub fn type_id(&self) -> u32 {
> +        // SAFETY: `type` field is valid in all union variants
> +        unsafe { self.0.type_ }
> +    }
> +}
> +
> +/// Generate AcpiObject subtype
> +///
> +/// For given subtype implements
> +/// - `TryFrom<&AcpiObject> for &SubType` trait
> +/// - unsafe try_from_unchecked() with same semantics, but without type check
> +macro_rules! acpi_object_subtype {
> +    ($subtype_name:ident <- ($acpi_type:ident, $field_name:ident, $union_type:ty)) => {
> +        /// Wraps `acpi_object` subtype
> +        #[repr(transparent)]
> +        pub struct $subtype_name($union_type);
> +
> +        impl TryFrom<&AcpiObject> for &$subtype_name {
> +            type Error = Error;
> +
> +            fn try_from(value: &AcpiObject) -> core::result::Result<Self, Self::Error> {
> +                // SAFETY: type_ field present in all union types and is always valid
> +                let real_type = unsafe { value.0.type_ };
> +
> +                if (real_type != $subtype_name::ACPI_TYPE) {
> +                    return Err(EINVAL);
> +                }
> +
> +                // SAFETY: We validated union subtype
> +                Ok(unsafe {
> +                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
> +                })
> +            }
> +        }
> +
> +        impl $subtype_name {
> +            /// This ACPI type int value (see `acpitypes.h`)
> +            pub const ACPI_TYPE: u32 = bindings::$acpi_type;
> +
> +            /// Converts AcpiObject reference into exact ACPI type wrapper
> +            ///
> +            /// # Safety
> +            ///
> +            /// Assumes that value is correct (`Self`) subtype
> +            pub unsafe fn try_from_unchecked(value: &AcpiObject) -> &Self {
> +                // SAFETY: Only unsafety comes from unchecked transformation and
> +                // we transfered
> +                unsafe {
> +                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
> +                }
> +            }
> +        }
> +    };
> +}
> +
> +acpi_object_subtype!(AcpiInteger
> +    <- (ACPI_TYPE_INTEGER, integer, bindings::acpi_object__bindgen_ty_1));
> +acpi_object_subtype!(AcpiString
> +    <- (ACPI_TYPE_STRING, string, bindings::acpi_object__bindgen_ty_2));
> +acpi_object_subtype!(AcpiBuffer
> +    <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
> +acpi_object_subtype!(AcpiPackage
> +    <- (ACPI_TYPE_PACKAGE, package, bindings::acpi_object__bindgen_ty_4));
> +acpi_object_subtype!(AcpiReference
> +    <- (ACPI_TYPE_LOCAL_REFERENCE, reference, bindings::acpi_object__bindgen_ty_5));
> +acpi_object_subtype!(AcpiProcessor
> +    <- (ACPI_TYPE_PROCESSOR, processor, bindings::acpi_object__bindgen_ty_6));
> +acpi_object_subtype!(AcpiPowerResource
> +    <- (ACPI_TYPE_POWER, power_resource, bindings::acpi_object__bindgen_ty_7));
> +
> +impl AcpiBuffer {
> +    /// Get Buffer's content
> +    pub fn payload(&self) -> &[u8] {
> +        // SAFETY: (pointer, length) indeed represents byte slice
> +        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
> +    }
> +}
> --

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 0/3] rust: WMI abstractions
  2025-12-22 11:52 ` [RFC PATCH 0/3] rust: WMI abstractions Danilo Krummrich
@ 2025-12-22 21:30   ` Gladyshev Ilya
  2025-12-25 17:56     ` Armin Wolf
  0 siblings, 1 reply; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-22 21:30 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Rafael J. Wysocki, Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Tamir Duberstein, Armin Wolf, platform-driver-x86,
	linux-kernel, rust-for-linux, linux-acpi

On 12/22/25 14:52, Danilo Krummrich wrote:
> On Sun Dec 21, 2025 at 7:22 PM CET, Gladyshev Ilya wrote:
>> Overview
>> ========
>> This patchset was developed some time ago out of pure curiosity
>> about the R4L project, but I believe it may still be useful, so
>> I decided to resume and post this work.
>> The goal of my curiosity is to develop a simple WMI driver in Rust
>> for unsupported laptop (I have some laying around) or, as a last
>> resort, submit a rewrite of redmi-wmi as a sample driver -- if that
>> would be acceptable.
> 
> It depends on the subsystem maintainer, please also see [1].
> 
> [1] https://rust-for-linux.com/rust-reference-drivers

Yes, I Cc'ed platform-drivers and WMI maintainer in a hope to gather 
their opinions.

>> Why RFC?
>> ========
>> 1. No real users for now
> 
> Above it does sound like you are working on a new WMI driver as well?
Well, I am planning to, but I can't guarantee any success at this point) 
So "no real users" is honest answer for now

But in a case if a) WMI subsystem is OK with reference drivers, and b) I 
fail to write a new driver, I will submit redmi-wmi-rs as a reference 
driver (since I can test it on my hardware).

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 1/3] rust: implement wrapper for acpi_object
  2025-12-22 11:35   ` Danilo Krummrich
@ 2025-12-22 21:47     ` Gladyshev Ilya
  2025-12-22 22:44       ` Danilo Krummrich
  0 siblings, 1 reply; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-22 21:47 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: foxido @ foxido . dev-cc= Rafael J. Wysocki, Len Brown,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Tamir Duberstein, Armin Wolf, platform-driver-x86, linux-kernel,
	rust-for-linux, linux-acpi

On 12/22/25 14:35, Danilo Krummrich wrote:
>> +acpi_object_subtype!(AcpiInteger
>> +    <- (ACPI_TYPE_INTEGER, integer, bindings::acpi_object__bindgen_ty_1));
>> +acpi_object_subtype!(AcpiString
>> +    <- (ACPI_TYPE_STRING, string, bindings::acpi_object__bindgen_ty_2));
>> +acpi_object_subtype!(AcpiBuffer
>> +    <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
>> +acpi_object_subtype!(AcpiPackage
>> +    <- (ACPI_TYPE_PACKAGE, package, bindings::acpi_object__bindgen_ty_4));
>> +acpi_object_subtype!(AcpiReference
>> +    <- (ACPI_TYPE_LOCAL_REFERENCE, reference, bindings::acpi_object__bindgen_ty_5));
>> +acpi_object_subtype!(AcpiProcessor
>> +    <- (ACPI_TYPE_PROCESSOR, processor, bindings::acpi_object__bindgen_ty_6));
>> +acpi_object_subtype!(AcpiPowerResource
>> +    <- (ACPI_TYPE_POWER, power_resource, bindings::acpi_object__bindgen_ty_7));
>> +
>> +impl AcpiBuffer {
>> +    /// Get Buffer's content
>> +    pub fn payload(&self) -> &[u8] {
>> +        // SAFETY: (pointer, length) indeed represents byte slice
>> +        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
>> +    }
>> +}
> 
> What about the values of the other types? How are they accessed?

I couldn't really decide between implementing all types or only the one 
needed... Probably, I should provide simple implementations for all the 
others, I will fix that.

> Also, I think it would be better to use a Deref impl rather than a method.

Wouldn't it be confusing to overload Deref on a non "pointer-like" type 
just for an implicit cast?

Overall, thank you for your patience and review. I will fix the other 
comments from both of your emails.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 1/3] rust: implement wrapper for acpi_object
  2025-12-22 21:47     ` Gladyshev Ilya
@ 2025-12-22 22:44       ` Danilo Krummrich
  2025-12-23 15:02         ` Gladyshev Ilya
  0 siblings, 1 reply; 17+ messages in thread
From: Danilo Krummrich @ 2025-12-22 22:44 UTC (permalink / raw)
  To: Gladyshev Ilya
  Cc: foxido @ foxido . dev-cc= Rafael J. Wysocki, Len Brown,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Tamir Duberstein, Armin Wolf, platform-driver-x86, linux-kernel,
	rust-for-linux, linux-acpi

On Mon Dec 22, 2025 at 10:47 PM CET, Gladyshev Ilya wrote:
> I couldn't really decide between implementing all types or only the one 
> needed... Probably, I should provide simple implementations for all the 
> others, I will fix that.

If they are not needed by any of the drivers you're aiming at, you should
probably just drop them.

> Wouldn't it be confusing to overload Deref on a non "pointer-like" type 
> just for an implicit cast?

What do you mean with overload Deref? What I mean is

	impl Deref for AcpiBuffer {
		type Target = [u8];

		[...]
	}

	impl Deref for AcpiInteger {
		type Target = u64;

		[...]
	}

	impl Deref for AcpiString {
		type Target = CStr;

		[...]
	}

etc.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 1/3] rust: implement wrapper for acpi_object
  2025-12-22 22:44       ` Danilo Krummrich
@ 2025-12-23 15:02         ` Gladyshev Ilya
  0 siblings, 0 replies; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-23 15:02 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Rafael J. Wysocki, Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Tamir Duberstein, Armin Wolf, platform-driver-x86,
	linux-kernel, rust-for-linux, linux-acpi

On 12/23/25 01:44, Danilo Krummrich wrote:
> On Mon Dec 22, 2025 at 10:47 PM CET, Gladyshev Ilya wrote:
>> I couldn't really decide between implementing all types or only the one
>> needed... Probably, I should provide simple implementations for all the
>> others, I will fix that.
> 
> If they are not needed by any of the drivers you're aiming at, you should
> probably just drop them.

Ack.

>> Wouldn't it be confusing to overload Deref on a non "pointer-like" type
>> just for an implicit cast?
> 
> What do you mean with overload Deref? What I mean is
> 
> 	impl Deref for AcpiBuffer {
> 		type Target = [u8];
> 
> 		[...]
> 	}

I meant the same, just used strange terminology, sorry. If I understand 
it correctly, Deref trait gives you "overloaded" dereference operator as 
well as implicit coercion in many cases, and I don't know if I want them.

Personally, I prefer the explicit style like:
```
let a: &AcpiInteger = /* ... */;
call_func(/* u64: */ a.val())
```

rather than:

```
let a: &AcpiInteger = /* ... */;
call_func(/* u64: */ *a)
```

The former feels clearer to me; the latter gives me "smart pointer" 
vibes and feels a bit confusing, because there are no smart pointers.

That said, I'm not a Rust expert at all -- so if you believe this change 
is better, I'll implement it your way.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 1/3] rust: implement wrapper for acpi_object
  2025-12-22 19:32   ` Rafael J. Wysocki
@ 2025-12-23 16:36     ` Gladyshev Ilya
  0 siblings, 0 replies; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-23 16:36 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein, Armin Wolf,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi

On 12/22/25 22:32, Rafael J. Wysocki wrote:
> On Sun, Dec 21, 2025 at 7:24 PM Gladyshev Ilya <foxido@foxido.dev> wrote:
>>
>> ACPI Object is represented via union on C-side. On Rust side, it's
>> zero-cost type wrapper for each ACPI Type, with individual methods for
>> getters and other interactions.
> 
> This is ACPICA stuff though and switching over ACPICA to Rust any time
> soon is rather unlikely.
> 
> Is this really needed on the Rust side?

At least acpi_buffer is needed, and for now, only that will be wrapped 
(I will remove other types).

Note that there are no actual types on the Rust side, only transparent 
wrappers around C acpi structs (for type safety).

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 0/3] rust: WMI abstractions
  2025-12-22 21:30   ` Gladyshev Ilya
@ 2025-12-25 17:56     ` Armin Wolf
  0 siblings, 0 replies; 17+ messages in thread
From: Armin Wolf @ 2025-12-25 17:56 UTC (permalink / raw)
  To: Gladyshev Ilya, Danilo Krummrich
  Cc: Rafael J. Wysocki, Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Tamir Duberstein, platform-driver-x86, linux-kernel,
	rust-for-linux, linux-acpi

Am 22.12.25 um 22:30 schrieb Gladyshev Ilya:

> On 12/22/25 14:52, Danilo Krummrich wrote:
>> On Sun Dec 21, 2025 at 7:22 PM CET, Gladyshev Ilya wrote:
>>> Overview
>>> ========
>>> This patchset was developed some time ago out of pure curiosity
>>> about the R4L project, but I believe it may still be useful, so
>>> I decided to resume and post this work.
>>> The goal of my curiosity is to develop a simple WMI driver in Rust
>>> for unsupported laptop (I have some laying around) or, as a last
>>> resort, submit a rewrite of redmi-wmi as a sample driver -- if that
>>> would be acceptable.
>>
>> It depends on the subsystem maintainer, please also see [1].
>>
>> [1] https://rust-for-linux.com/rust-reference-drivers
>
> Yes, I Cc'ed platform-drivers and WMI maintainer in a hope to gather 
> their opinions.
>
I am OK with you using the redmi-wmi driver as a reference driver. TBH i am very interested in having rust
abstractions for the WMI subsystem, however i currently have no clue about the rust programing language itself :(
I plan to complete a rust tutorial in the future, but till then i can give you only limited feedback.

>>> Why RFC?
>>> ========
>>> 1. No real users for now
>>
>> Above it does sound like you are working on a new WMI driver as well?
> Well, I am planning to, but I can't guarantee any success at this 
> point) So "no real users" is honest answer for now
>
> But in a case if a) WMI subsystem is OK with reference drivers, and b) 
> I fail to write a new driver, I will submit redmi-wmi-rs as a 
> reference driver (since I can test it on my hardware).

Fine with me.

Thanks,
Armin Wolf


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 2/3] rust: introduce WMI abstractions
  2025-12-21 18:22 ` [RFC PATCH 2/3] rust: introduce WMI abstractions Gladyshev Ilya
  2025-12-22 11:50   ` Danilo Krummrich
@ 2025-12-25 18:06   ` Armin Wolf
  2025-12-25 20:37     ` Gladyshev Ilya
  1 sibling, 1 reply; 17+ messages in thread
From: Armin Wolf @ 2025-12-25 18:06 UTC (permalink / raw)
  To: Gladyshev Ilya, foxido @ foxido . dev-cc= Rafael J. Wysocki
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi

Am 21.12.25 um 19:22 schrieb Gladyshev Ilya:

> Implement the basic WMI subsystem abstractions required to write a
> WMI device driver with or without the need for initial device data.
> This includes the following data structures:
>
> The `wmi::Driver` trait represents the interface to the driver.
>
> The `wmi::Device` abstraction represents a `struct wmi_device`.
>
> In order to provide the WMI specific parts to a generic
> `driver::Registration` the `driver::RegistrationOps` trait is
> implemented by `wmi::Adapter`.
>
> Everything from C side is supported, except shutdown action
>
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> ---
>   rust/bindings/bindings_helper.h |   1 +
>   rust/kernel/lib.rs              |   2 +
>   rust/kernel/wmi.rs              | 267 ++++++++++++++++++++++++++++++++
>   3 files changed, 270 insertions(+)
>   create mode 100644 rust/kernel/wmi.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index a067038b4b42..f9671280c6b5 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -85,6 +85,7 @@
>   #include <linux/usb.h>
>   #include <linux/wait.h>
>   #include <linux/workqueue.h>
> +#include <linux/wmi.h>
>   #include <linux/xarray.h>
>   #include <trace/events/rust_sample.h>
>   
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index f812cf120042..db3e649d26eb 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -151,6 +151,8 @@
>   pub mod uaccess;
>   #[cfg(CONFIG_USB = "y")]
>   pub mod usb;
> +#[cfg(CONFIG_ACPI_WMI)]
> +pub mod wmi;
>   pub mod workqueue;
>   pub mod xarray;
>   
> diff --git a/rust/kernel/wmi.rs b/rust/kernel/wmi.rs
> new file mode 100644
> index 000000000000..018e88d01e8c
> --- /dev/null
> +++ b/rust/kernel/wmi.rs
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Abstractions for the WMI devices.
> +//!
> +//! C header: [`include/linux/wmi.h`](srctree/include/linux/wmi.h)
> +
> +use crate::{
> +    acpi::AcpiObject,
> +    device,
> +    device_id::{RawDeviceId, RawDeviceIdIndex},
> +    driver,
> +    error::{from_result, to_result, VTABLE_DEFAULT_ERROR},
> +    prelude::*,
> +    types::Opaque,
> +};
> +use core::{
> +    marker::PhantomData,
> +    mem::MaybeUninit,
> +    ptr::{addr_of_mut, NonNull},
> +};
> +use macros::vtable;
> +
> +/// [`IdTable`](kernel::device_id::IdTable) type for WMI.
> +pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
> +
> +/// The WMI driver trait.
> +#[vtable]
> +pub trait Driver: Send {
> +    /// The type holding information about each one of the device ids supported by the driver.
> +    type IdInfo: 'static;
> +
> +    /// The table of device ids supported by the driver.
> +    const TABLE: IdTable<Self::IdInfo>;
> +
> +    /// WMI driver probe.
> +    ///
> +    /// Called when a new WMI device is bound to this driver.
> +    /// Implementers should attempt to initialize the driver here.
> +    fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo) -> impl PinInit<Self, Error>;
> +
> +    /// WMI device notify.
> +    ///
> +    /// Called when new WMI event received from bounded device
> +    fn notify(&self, _dev: &Device<device::Core>, _event: &AcpiObject) {
> +        build_error!(VTABLE_DEFAULT_ERROR);
> +    }
> +
> +    /// WMI driver remove.
> +    fn remove(self: Pin<KBox<Self>>, _dev: &Device<device::Core>) {
> +        build_error!(VTABLE_DEFAULT_ERROR);
> +    }
> +}
> +
> +/// A WMI device.
> +///
> +/// This structure represents the Rust abstraction for a C [`struct wmi_device`].
> +/// The implementation abstracts the usage of a C [`struct wmi_device`] passed
> +/// in from the C side.
> +pub struct Device<Cxt: device::DeviceContext = device::Normal> {
> +    inner: Opaque<bindings::wmi_device>,
> +    _p: PhantomData<Cxt>,
> +}
> +
> +impl<Cxt: device::DeviceContext> Device<Cxt> {
> +    fn as_raw(&self) -> *mut bindings::wmi_device {
> +        self.inner.get()
> +    }
> +}
> +
> +/// An adapter for the registration of WMI drivers.
> +pub struct Adapter<T: Driver>(T);
> +
> +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
> +// a preceding call to `register` has been successful.
> +unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
> +    type RegType = bindings::wmi_driver;
> +
> +    unsafe fn register(
> +        wdrv: &Opaque<Self::RegType>,
> +        name: &'static CStr,
> +        module: &'static ThisModule,
> +    ) -> Result {
> +        macro_rules! map_callback {
> +            ($flag:ident -> $callback:ident) => {
> +                if T::$flag {
> +                    Some(Self::$callback)
> +                } else {
> +                    None
> +                }
> +            };
> +        }
> +
> +        // SAFETY: It's safe to set the fields of `struct wmi_driver` on initialization.
> +        unsafe {
> +            (*wdrv.get()).driver.name = name.as_char_ptr();
> +            (*wdrv.get()).driver.probe_type = bindings::probe_type_PROBE_PREFER_ASYNCHRONOUS;
> +            (*wdrv.get()).id_table = T::TABLE.as_ptr();
> +            (*wdrv.get()).probe = map_callback!(HAS_PROBE -> probe_callback);
> +            (*wdrv.get()).notify = map_callback!(HAS_NOTIFY -> notify_callback);

I think it should be possible to handle WMI drivers requiring .no_notify_data to be set. Is there
a way to declare the WMI event data passed to the notify() callback as optional? If yes, then i suggest
that we always set .no_notify_data and simply require the WMI drivers themselves to verify that a given
WMI event does contain additional event data.

(Said no_notify_data flag only exists because most WMI drivers currently assume that the event data
passed to the .notify() callback is not NULL)

Thanks,
Armin Wolf

> +            (*wdrv.get()).remove = map_callback!(HAS_REMOVE -> remove_callback);
> +            (*wdrv.get()).shutdown = None;
> +            (*wdrv.get()).no_singleton = true;
> +        }
> +
> +        // SAFETY: `wdrv` is guaranteed to be a valid `RegType`.
> +        to_result(unsafe { bindings::__wmi_driver_register(wdrv.get(), module.0) })
> +    }
> +
> +    unsafe fn unregister(wdrv: &Opaque<Self::RegType>) {
> +        // SAFETY: `wdrv` is guaranteed to be a valid `RegType`.
> +        unsafe { bindings::wmi_driver_unregister(wdrv.get()) };
> +    }
> +}
> +
> +impl<T: Driver + 'static> Adapter<T> {
> +    extern "C" fn probe_callback(
> +        wdev: *mut bindings::wmi_device,
> +        id: *const c_void,
> +    ) -> kernel::ffi::c_int {
> +        // SAFETY: The WMI core only ever calls the probe callback with a valid pointer to a
> +        // `struct wmi_device`.
> +        //
> +        // INVARIANT: `wdev` is valid for the duration of `probe_callback()`.
> +        let wdev = unsafe { &*wdev.cast::<Device<device::CoreInternal>>() };
> +
> +        let id = id as usize;
> +        let info = T::TABLE.info(id);
> +
> +        from_result(|| {
> +            let data = T::probe(wdev, info);
> +
> +            wdev.as_ref().set_drvdata(data)?;
> +            Ok(0)
> +        })
> +    }
> +
> +    extern "C" fn notify_callback(
> +        wdev: *mut bindings::wmi_device,
> +        obj: *mut bindings::acpi_object,
> +    ) {
> +        // SAFETY: The WMI system only ever calls the notify callback with a valid pointer to a
> +        // `struct wmi_device`.
> +        let wdev = unsafe { &*wdev.cast::<Device<device::CoreInternal>>() };
> +        // SAFETY: AcpiObject is repr(transparent) wrapper
> +        let obj: &AcpiObject = unsafe { &*(obj as *const AcpiObject) };
> +
> +        // SAFETY: `notify_callback` is only ever called after a successful call to
> +        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> +        // and stored a `T`.
> +        let this: &T = unsafe { &wdev.as_ref().drvdata_borrow::<T>() };
> +        this.notify(wdev, obj);
> +    }
> +
> +    extern "C" fn remove_callback(wdev: *mut bindings::wmi_device) {
> +        // SAFETY: The WMI system only ever calls the remove callback with a valid pointer to a
> +        // `struct wmi_device`.
> +        let wdev = unsafe { &*wdev.cast::<Device<device::CoreInternal>>() };
> +
> +        // SAFETY: `remove_callback` is only ever called after a successful call to
> +        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> +        // and stored a `T`.
> +        let this = unsafe { wdev.as_ref().drvdata_obtain::<T>() };
> +        this.remove(wdev);
> +    }
> +}
> +
> +impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> +    fn as_ref(&self) -> &device::Device<Ctx> {
> +        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> +        // `struct platform_device`.
> +        let dev = unsafe { addr_of_mut!((*self.inner.get()).dev) };
> +
> +        // SAFETY: `dev` points to a valid `struct device`.
> +        unsafe { device::Device::from_raw(dev) }
> +    }
> +}
> +
> +kernel::impl_device_context_deref!(unsafe { Device });
> +kernel::impl_device_context_into_aref!(Device);
> +
> +// SAFETY: Instances of `Device` are always reference-counted.
> +unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +    fn inc_ref(&self) {
> +        // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> +        unsafe { bindings::get_device(self.as_ref().as_raw()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: NonNull<Self>) {
> +        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> +        unsafe { bindings::put_device(&raw mut (*obj.as_ref().as_raw()).dev) }
> +    }
> +}
> +
> +/// Abstraction for the WMI device ID structure, i.e. [`struct wmi_device_id`].
> +#[repr(transparent)]
> +pub struct DeviceId(bindings::wmi_device_id);
> +
> +impl DeviceId {
> +    /// Constructs new DeviceId from GUID string
> +    pub const fn new(guid: &[u8; bindings::UUID_STRING_LEN as usize]) -> Self {
> +        // SAFETY: FFI type is valid to be zero-initialized.
> +        let mut inner: bindings::wmi_device_id = unsafe { MaybeUninit::zeroed().assume_init() };
> +
> +        build_assert!(inner.guid_string.len() == bindings::UUID_STRING_LEN as usize + 1);
> +
> +        // SAFETY: It's safe to copy UUID_STRING_LEN, because we validated lengths
> +        // Also we leave last byte zeroed -> guid_string is valid C string
> +        unsafe {
> +            ::core::ptr::copy_nonoverlapping(
> +                guid.as_ptr(),
> +                &raw mut inner.guid_string[0],
> +                bindings::UUID_STRING_LEN as usize,
> +            );
> +        }
> +
> +        Self(inner)
> +    }
> +}
> +
> +// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `wmi_device_id` and does not add
> +// additional invariants, so it's safe to transmute to `RawType`.
> +unsafe impl RawDeviceId for DeviceId {
> +    type RawType = bindings::wmi_device_id;
> +}
> +
> +// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `context` field.
> +unsafe impl RawDeviceIdIndex for DeviceId {
> +    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::wmi_device_id, context);
> +
> +    fn index(&self) -> usize {
> +        self.0.context as usize
> +    }
> +}
> +
> +/// Declares a kernel module that exposes a single WMI driver.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// module_wmi_driver! {
> +///     type: MyDriver,
> +///     name: "Module name",
> +///     author: ["Author name"],
> +///     description: "Description",
> +///     license: "GPL v2",
> +/// }
> +/// ```
> +#[macro_export]
> +macro_rules! module_wmi_driver {
> +    ($($f:tt)*) => {
> +        $crate::module_driver!(<T>, $crate::wmi::Adapter<T>, { $($f)* });
> +    }
> +}
> +
> +/// Create a WMI `IdTable` with its alias for modpost.
> +#[macro_export]
> +macro_rules! wmi_device_table {
> +    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
> +        const $table_name: $crate::device_id::IdArray<
> +            $crate::wmi::DeviceId,
> +            $id_info_type,
> +            { $table_data.len() },
> +        > = $crate::device_id::IdArray::new($table_data);
> +
> +        $crate::module_device_table!("wmi", $module_table_name, $table_name);
> +    };
> +}

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 2/3] rust: introduce WMI abstractions
  2025-12-25 18:06   ` Armin Wolf
@ 2025-12-25 20:37     ` Gladyshev Ilya
  2025-12-28 21:02       ` Armin Wolf
  0 siblings, 1 reply; 17+ messages in thread
From: Gladyshev Ilya @ 2025-12-25 20:37 UTC (permalink / raw)
  To: Armin Wolf
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi

On 12/25/25 21:06, Armin Wolf wrote:
>> +// SAFETY: A call to `unregister` for a given instance of `RegType` 
>> is guaranteed to be valid if
>> +// a preceding call to `register` has been successful.
>> +unsafe impl<T: Driver + 'static> driver::RegistrationOps for 
>> Adapter<T> {
>> +    type RegType = bindings::wmi_driver;
>> +
>> +    unsafe fn register(
>> +        wdrv: &Opaque<Self::RegType>,
>> +        name: &'static CStr,
>> +        module: &'static ThisModule,
>> +    ) -> Result {
>> +        macro_rules! map_callback {
>> +            ($flag:ident -> $callback:ident) => {
>> +                if T::$flag {
>> +                    Some(Self::$callback)
>> +                } else {
>> +                    None
>> +                }
>> +            };
>> +        }
>> +
>> +        // SAFETY: It's safe to set the fields of `struct wmi_driver` 
>> on initialization.
>> +        unsafe {
>> +            (*wdrv.get()).driver.name = name.as_char_ptr();
>> +            (*wdrv.get()).driver.probe_type = 
>> bindings::probe_type_PROBE_PREFER_ASYNCHRONOUS;
>> +            (*wdrv.get()).id_table = T::TABLE.as_ptr();
>> +            (*wdrv.get()).probe = map_callback!(HAS_PROBE -> 
>> probe_callback);
>> +            (*wdrv.get()).notify = map_callback!(HAS_NOTIFY -> 
>> notify_callback);
> 
> I think it should be possible to handle WMI drivers 
> requiring .no_notify_data to be set. Is there
> a way to declare the WMI event data passed to the notify() callback as 
> optional? If yes, then i suggest
> that we always set .no_notify_data and simply require the WMI drivers 
> themselves to verify that a given
> WMI event does contain additional event data.

Yes, I can change the notify API to receive Optional<&AcpiBuffer> 
instead of &AcpiBuffer, so every driver will be forced to verify payload 
existence by Rust's type system.

IIRC, casting raw (C) pointer to Optional<&T> is actually a no-op in 
Rust, since NULL is automatically mapped to None (empty Optional), so it 
will be a zero-cost typesystem win :)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 2/3] rust: introduce WMI abstractions
  2025-12-25 20:37     ` Gladyshev Ilya
@ 2025-12-28 21:02       ` Armin Wolf
  0 siblings, 0 replies; 17+ messages in thread
From: Armin Wolf @ 2025-12-28 21:02 UTC (permalink / raw)
  To: Gladyshev Ilya
  Cc: Len Brown, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Tamir Duberstein,
	platform-driver-x86, linux-kernel, rust-for-linux, linux-acpi

Am 25.12.25 um 21:37 schrieb Gladyshev Ilya:

> On 12/25/25 21:06, Armin Wolf wrote:
>>> +// SAFETY: A call to `unregister` for a given instance of `RegType` 
>>> is guaranteed to be valid if
>>> +// a preceding call to `register` has been successful.
>>> +unsafe impl<T: Driver + 'static> driver::RegistrationOps for 
>>> Adapter<T> {
>>> +    type RegType = bindings::wmi_driver;
>>> +
>>> +    unsafe fn register(
>>> +        wdrv: &Opaque<Self::RegType>,
>>> +        name: &'static CStr,
>>> +        module: &'static ThisModule,
>>> +    ) -> Result {
>>> +        macro_rules! map_callback {
>>> +            ($flag:ident -> $callback:ident) => {
>>> +                if T::$flag {
>>> +                    Some(Self::$callback)
>>> +                } else {
>>> +                    None
>>> +                }
>>> +            };
>>> +        }
>>> +
>>> +        // SAFETY: It's safe to set the fields of `struct 
>>> wmi_driver` on initialization.
>>> +        unsafe {
>>> +            (*wdrv.get()).driver.name = name.as_char_ptr();
>>> +            (*wdrv.get()).driver.probe_type = 
>>> bindings::probe_type_PROBE_PREFER_ASYNCHRONOUS;
>>> +            (*wdrv.get()).id_table = T::TABLE.as_ptr();
>>> +            (*wdrv.get()).probe = map_callback!(HAS_PROBE -> 
>>> probe_callback);
>>> +            (*wdrv.get()).notify = map_callback!(HAS_NOTIFY -> 
>>> notify_callback);
>>
>> I think it should be possible to handle WMI drivers requiring 
>> .no_notify_data to be set. Is there
>> a way to declare the WMI event data passed to the notify() callback 
>> as optional? If yes, then i suggest
>> that we always set .no_notify_data and simply require the WMI drivers 
>> themselves to verify that a given
>> WMI event does contain additional event data.
>
> Yes, I can change the notify API to receive Optional<&AcpiBuffer> 
> instead of &AcpiBuffer, so every driver will be forced to verify 
> payload existence by Rust's type system.
>
> IIRC, casting raw (C) pointer to Optional<&T> is actually a no-op in 
> Rust, since NULL is automatically mapped to None (empty Optional), so 
> it will be a zero-cost typesystem win :)

Nice, then i suggest you always set .no_notify_data and pass the event data inside said Optional.

Thanks,
Armin Wolf


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-12-28 21:02 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-21 18:22 [RFC PATCH 0/3] rust: WMI abstractions Gladyshev Ilya
2025-12-21 18:22 ` [RFC PATCH 1/3] rust: implement wrapper for acpi_object Gladyshev Ilya
2025-12-22 11:35   ` Danilo Krummrich
2025-12-22 21:47     ` Gladyshev Ilya
2025-12-22 22:44       ` Danilo Krummrich
2025-12-23 15:02         ` Gladyshev Ilya
2025-12-22 19:32   ` Rafael J. Wysocki
2025-12-23 16:36     ` Gladyshev Ilya
2025-12-21 18:22 ` [RFC PATCH 2/3] rust: introduce WMI abstractions Gladyshev Ilya
2025-12-22 11:50   ` Danilo Krummrich
2025-12-25 18:06   ` Armin Wolf
2025-12-25 20:37     ` Gladyshev Ilya
2025-12-28 21:02       ` Armin Wolf
2025-12-21 18:22 ` [RFC PATCH 3/3] rust: sample driver for WMI demonstrations Gladyshev Ilya
2025-12-22 11:52 ` [RFC PATCH 0/3] rust: WMI abstractions Danilo Krummrich
2025-12-22 21:30   ` Gladyshev Ilya
2025-12-25 17:56     ` Armin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox