[PATCH 0/3] rust: WMI abstractions

[PATCH 0/3] rust: WMI abstractions

[Gladyshev Ilya]

Overview
========
This patchset implements WMI abstractions for Rust drivers. It is the successor
of the previous RFC posting [0]. These abstractions allow most WMI drivers to be
implemented in Rust (provided other dependencies are abstracted as well).

Currently, the only driver in existence is a stripped-down rewrite of redmi-wmi
that can probe itself with custom data and print messages on notification
(posted in [0]).

I am currently working on wrapping redmi-wmi's other dependencies (sparse-keyboard),
so I can provide it as a reference driver (Armin is OK with that [1]).

Notes
=====
I do not know how these abstractions should be handled in MAINTAINERS, so for now I have
simply added them to the original WMI entry. I would be happy to be added as a reviewer
to keep the Rust version 'synced'/valid though.

[0]: https://lore.kernel.org/rust-for-linux/cover.1766331321.git.foxido@foxido.dev/
[1]: https://lore.kernel.org/rust-for-linux/c7384f13-e286-45a4-95c6-24d389217185@gmx.de/

Gladyshev Ilya (3):
  rust: enable const_intrinsic_copy feature
  rust: implement wrapper for acpi_object
  rust: add WMI abstractions

 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/acpi.rs             |  91 +++++++++++
 rust/kernel/lib.rs              |   3 +
 rust/kernel/wmi.rs              | 277 ++++++++++++++++++++++++++++++++
 5 files changed, 373 insertions(+)
 create mode 100644 rust/kernel/wmi.rs


base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb
-- 
2.52.0

[PATCH 1/3] rust: enable const_intrinsic_copy feature

[Gladyshev Ilya]

Since Rust 1.83.0 the `const_intrinsic_copy` feature is stable.
It allows usage of `ptr::copy` and `ptr::copy_nonoverlapping` in const
context.

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

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f812cf120042..a6eccdba50b5 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,7 @@
 #![feature(const_option)]
 #![feature(const_ptr_write)]
 #![feature(const_refs_to_cell)]
+#![feature(const_intrinsic_copy)]
 //
 // Expected to become stable.
 #![feature(arbitrary_self_types)]
-- 
2.52.0

[PATCH 2/3] rust: implement wrapper for acpi_object

[Gladyshev Ilya]

ACPI Object is represented via union on C-side. On Rust side, this union
is transparently wrapped for each ACPI Type, with individual methods and
Defer implementation to represented type (integer, string, buffer, etc).

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

diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 9b8efa623130..ac1a9f305f6c 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -2,6 +2,8 @@
 
 //! Advanced Configuration and Power Interface abstractions.
 
+use core::ops::Deref;
+
 use crate::{
     bindings,
     device_id::{RawDeviceId, RawDeviceIdIndex},
@@ -63,3 +65,92 @@ 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 (e.g [`AcpiBuffer`]).
+///
+/// # Example
+/// ```
+/// # use kernel::prelude::*;
+/// use kernel::acpi::{AcpiObject, AcpiBuffer};
+///
+/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
+///     let buf: &AcpiBuffer = obj.try_into()?;
+///
+///     Ok(buf[0])
+/// }
+/// ```
+///
+/// [`struct acpi_object`]: srctree/include/acpi/actypes.h
+#[repr(transparent)]
+pub struct AcpiObject(bindings::acpi_object);
+
+impl AcpiObject {
+    /// Returns object type id (see [`actypes.h`](srctree/include/acpi/actypes.h)).
+    pub fn type_id(&self) -> u32 {
+        // SAFETY: `type` field is valid in all union variants.
+        unsafe { self.0.type_ }
+    }
+}
+
+/// Generate wrapper type for AcpiObject subtype.
+///
+/// For given subtype implements
+/// - `#[repr(transparent)]` type wrapper,
+/// - `TryFrom<&AcpiObject> for &SubType` trait,
+/// - unsafe from_unchecked() for 'trusted' conversion.
+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<'a> TryFrom<&'a AcpiObject> for &'a $subtype_name {
+            type Error = Error;
+
+            fn try_from(value: &'a AcpiObject) -> core::result::Result<Self, Self::Error> {
+                if (value.type_id() != $subtype_name::ACPI_TYPE) {
+                    return Err(EINVAL);
+                }
+
+                // SAFETY: Requested cast is valid because we validated type_id
+                Ok(unsafe { $subtype_name::from_unchecked(&value) })
+            }
+        }
+
+        impl $subtype_name {
+            /// Int value, representing this ACPI type (see [`acpitypes.h`]).
+            ///
+            /// [`acpitypes.h`]: srctree/include/linux/acpitypes.h
+            pub const ACPI_TYPE: u32 = bindings::$acpi_type;
+
+            /// Converts opaque AcpiObject reference into exact ACPI type reference.
+            ///
+            /// # Safety
+            ///
+            /// - Requested cast should be valid (value.type_id() is `Self::ACPI_TYPE`).
+            pub unsafe fn from_unchecked(value: &AcpiObject) -> &Self {
+                // SAFETY:
+                // - $field_name is currently active union's field due to external safety contract,
+                // - Transmuting to `repr(transparent)` wrapper is safe.
+                unsafe {
+                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
+                }
+            }
+        }
+    };
+}
+
+acpi_object_subtype!(AcpiBuffer
+    <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
+
+impl Deref for AcpiBuffer {
+    type Target = [u8];
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: (pointer, length) indeed represents byte slice.
+        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
+    }
+}
-- 
2.52.0

[PATCH 3/3] rust: add WMI abstractions

[Gladyshev Ilya]

This introduces Rust abstraction for WMI subsystem via wmi::Driver trait
and module_wmi_driver/wmi_device_table macros. Driver can be probed,
notified on events and removed -- almost everything C-side can do.

This abstraction will be used by subsequent redmi_wmi_rs reference
driver.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 765ad2daa218..4909ae8be6e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -404,6 +404,7 @@ F:	Documentation/driver-api/wmi.rst
 F:	Documentation/wmi/
 F:	drivers/platform/wmi/
 F:	include/uapi/linux/wmi.h
+F:	rust/kernel/wmi.rs
 
 ACRN HYPERVISOR SERVICE MODULE
 M:	Fei Li <fei1.li@intel.com>
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 a6eccdba50b5..29bc2b87a103 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -152,6 +152,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..a15258365a2e
--- /dev/null
+++ b/rust/kernel/wmi.rs
@@ -0,0 +1,277 @@
+// 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::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: Pin<&Self>, _dev: &Device<device::Core>, _event: Option<&AcpiObject>) {
+        build_error!(VTABLE_DEFAULT_ERROR);
+    }
+
+    /// WMI driver remove.
+    fn unbind(self: Pin<&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_UNBIND -> remove_callback);
+            (*wdrv.get()).shutdown = None;
+            (*wdrv.get()).no_singleton = true;
+            (*wdrv.get()).no_notify_data = 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 around FFI object, so it's safe to cast
+        //    raw pointer to reference (in terms of alignment and etc),
+        // - Option<&ref> is guaranteed to have same layout as raw pointer (with NULL representing
+        //    None) by Rust's "nullable pointer optimization".
+        let obj: Option<&AcpiObject> = unsafe { core::mem::transmute(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 = 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_borrow::<T>() };
+        this.unbind(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 { &raw 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`].
+///
+/// [`struct wmi_device_id`]: https://docs.kernel.org/driver-api/basics.html#c.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, so 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.52.0

Re: [PATCH 2/3] rust: implement wrapper for acpi_object

[Gary Guo]

On Wed,  7 Jan 2026 23:35:32 +0300
Gladyshev Ilya <foxido@foxido.dev> wrote:

> ACPI Object is represented via union on C-side. On Rust side, this union
> is transparently wrapped for each ACPI Type, with individual methods and
> Defer implementation to represented type (integer, string, buffer, etc).
> 
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>


Hi Gladyshev,

I've checked the `acpi_object` implementation on the C side and it appears
that the buffer is not owned by the object (however managed externally,
could either be resting in ACPI tables directly or be allocated).

Therefore, you might want to carry a lifetime to represent the lifetime of
underlying buffers?

> ---
>  rust/kernel/acpi.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
> index 9b8efa623130..ac1a9f305f6c 100644
> --- a/rust/kernel/acpi.rs
> +++ b/rust/kernel/acpi.rs
> @@ -2,6 +2,8 @@
>  
>  //! Advanced Configuration and Power Interface abstractions.
>  
> +use core::ops::Deref;
> +
>  use crate::{
>      bindings,
>      device_id::{RawDeviceId, RawDeviceIdIndex},
> @@ -63,3 +65,92 @@ 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 (e.g [`AcpiBuffer`]).
> +///
> +/// # Example
> +/// ```
> +/// # use kernel::prelude::*;
> +/// use kernel::acpi::{AcpiObject, AcpiBuffer};
> +///
> +/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
> +///     let buf: &AcpiBuffer = obj.try_into()?;
> +///
> +///     Ok(buf[0])
> +/// }
> +/// ```
> +///
> +/// [`struct acpi_object`]: srctree/include/acpi/actypes.h
> +#[repr(transparent)]
> +pub struct AcpiObject(bindings::acpi_object);
> +
> +impl AcpiObject {
> +    /// Returns object type id (see [`actypes.h`](srctree/include/acpi/actypes.h)).
> +    pub fn type_id(&self) -> u32 {
> +        // SAFETY: `type` field is valid in all union variants.
> +        unsafe { self.0.type_ }
> +    }

This should probably be an enum instead of just integer.

> +}
> +
> +/// Generate wrapper type for AcpiObject subtype.
> +///
> +/// For given subtype implements
> +/// - `#[repr(transparent)]` type wrapper,
> +/// - `TryFrom<&AcpiObject> for &SubType` trait,
> +/// - unsafe from_unchecked() for 'trusted' conversion.
> +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);

Instead of wrapping the bindgen-generated subtypes, I would rather this to
be a transparent wrapper of `acpi_object`, with an invariant that the
specific union field is active.

This way you do not have to name the bindgen-generated names.

> +
> +        impl<'a> TryFrom<&'a AcpiObject> for &'a $subtype_name {
> +            type Error = Error;
> +
> +            fn try_from(value: &'a AcpiObject) -> core::result::Result<Self, Self::Error> {
> +                if (value.type_id() != $subtype_name::ACPI_TYPE) {
> +                    return Err(EINVAL);
> +                }
> +
> +                // SAFETY: Requested cast is valid because we validated type_id
> +                Ok(unsafe { $subtype_name::from_unchecked(&value) })
> +            }
> +        }

It feels like this can be better implemented by having a sealed trait for
all possible ACPI object types?

> +
> +        impl $subtype_name {
> +            /// Int value, representing this ACPI type (see [`acpitypes.h`]).
> +            ///
> +            /// [`acpitypes.h`]: srctree/include/linux/acpitypes.h
> +            pub const ACPI_TYPE: u32 = bindings::$acpi_type;
> +
> +            /// Converts opaque AcpiObject reference into exact ACPI type reference.
> +            ///
> +            /// # Safety
> +            ///
> +            /// - Requested cast should be valid (value.type_id() is `Self::ACPI_TYPE`).
> +            pub unsafe fn from_unchecked(value: &AcpiObject) -> &Self {
> +                // SAFETY:
> +                // - $field_name is currently active union's field due to external safety contract,
> +                // - Transmuting to `repr(transparent)` wrapper is safe.
> +                unsafe {
> +                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
> +                }
> +            }
> +        }
> +    };
> +}
> +
> +acpi_object_subtype!(AcpiBuffer
> +    <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
> +
> +impl Deref for AcpiBuffer {
> +    type Target = [u8];
> +
> +    fn deref(&self) -> &Self::Target {
> +        // SAFETY: (pointer, length) indeed represents byte slice.
> +        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
> +    }
> +}

Re: [PATCH 2/3] rust: implement wrapper for acpi_object

[Gladyshev Ilya]

On 1/8/26 16:21, Gary Guo wrote:
> On Wed,  7 Jan 2026 23:35:32 +0300
> Gladyshev Ilya <foxido@foxido.dev> wrote:
> 
>> ACPI Object is represented via union on C-side. On Rust side, this union
>> is transparently wrapped for each ACPI Type, with individual methods and
>> Defer implementation to represented type (integer, string, buffer, etc).
>>
>> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> 
> 
> Hi Gladyshev,
> 
> I've checked the `acpi_object` implementation on the C side and it appears
> that the buffer is not owned by the object (however managed externally,
> could either be resting in ACPI tables directly or be allocated).
Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object() 
implementation, and it seems to me that the acpi_object's lifetime is 
the same as its internal buffer. Overall, it is indeed managed 
externally, but acpi_object and acpi_object::buffer->pointer live 
together. I’m not an ACPI expert, though, so maybe I’m missing something.

Anyway, the current Rust setup seems fine for now:
0. AcpiObject validity is guaranteed by whoever constructed/passed it (C 
side for WMI abstractions, for example)
1. You can only convert &AcpiObject to &AcpiSubType (reference to 
reference), so AcpiSubType can't outlive AcpiObject
2. You can't steal the data pointer from &AcpiSubType either, because 
the Deref impl is "logically safe" and only gives you a reference to the 
inner data, which can't outlive AcpiSubType's reference -> can't outlive 
AcpiObject.

So for now until AcpiObject lives _less_ than it's inner data, 
everything is OK.

> Therefore, you might want to carry a lifetime to represent the lifetime of
> underlying buffers?
>> ---
>>   rust/kernel/acpi.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 91 insertions(+)
>>
>> diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
>> index 9b8efa623130..ac1a9f305f6c 100644
>> --- a/rust/kernel/acpi.rs
>> +++ b/rust/kernel/acpi.rs
>> @@ -2,6 +2,8 @@
>>   
>>   //! Advanced Configuration and Power Interface abstractions.
>>   
>> +use core::ops::Deref;
>> +
>>   use crate::{
>>       bindings,
>>       device_id::{RawDeviceId, RawDeviceIdIndex},
>> @@ -63,3 +65,92 @@ 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 (e.g [`AcpiBuffer`]).
>> +///
>> +/// # Example
>> +/// ```
>> +/// # use kernel::prelude::*;
>> +/// use kernel::acpi::{AcpiObject, AcpiBuffer};
>> +///
>> +/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
>> +///     let buf: &AcpiBuffer = obj.try_into()?;
>> +///
>> +///     Ok(buf[0])
>> +/// }
>> +/// ```
>> +///
>> +/// [`struct acpi_object`]: srctree/include/acpi/actypes.h
>> +#[repr(transparent)]
>> +pub struct AcpiObject(bindings::acpi_object);
>> +
>> +impl AcpiObject {
>> +    /// Returns object type id (see [`actypes.h`](srctree/include/acpi/actypes.h)).
>> +    pub fn type_id(&self) -> u32 {
>> +        // SAFETY: `type` field is valid in all union variants.
>> +        unsafe { self.0.type_ }
>> +    }
> 
> This should probably be an enum instead of just integer.

Using an enum would require keeping Rust's enum synced with the C side, 
as well as implementing some simple but non-trivial checks that the 
`type_` field is a valid enum value (and the valid range isn't 
continuous). I think that keeping it as an integer is simpler and better 
matches C side.

>> +}
>> +
>> +/// Generate wrapper type for AcpiObject subtype.
>> +///
>> +/// For given subtype implements
>> +/// - `#[repr(transparent)]` type wrapper,
>> +/// - `TryFrom<&AcpiObject> for &SubType` trait,
>> +/// - unsafe from_unchecked() for 'trusted' conversion.
>> +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);
> 
> Instead of wrapping the bindgen-generated subtypes, I would rather this to
> be a transparent wrapper of `acpi_object`, with an invariant that the
> specific union field is active.
> 
> This way you do not have to name the bindgen-generated names.

Sounds reasonable, thanks!

>> +
>> +        impl<'a> TryFrom<&'a AcpiObject> for &'a $subtype_name {
>> +            type Error = Error;
>> +
>> +            fn try_from(value: &'a AcpiObject) -> core::result::Result<Self, Self::Error> {
>> +                if (value.type_id() != $subtype_name::ACPI_TYPE) {
>> +                    return Err(EINVAL);
>> +                }
>> +
>> +                // SAFETY: Requested cast is valid because we validated type_id
>> +                Ok(unsafe { $subtype_name::from_unchecked(&value) })
>> +            }
>> +        }
> 
> It feels like this can be better implemented by having a sealed trait for
> all possible ACPI object types?

I don’t see any particular benefit except moving the `impl TryFrom` out 
of macro-generated code, but you’re probably right -- I’ll try 
implementing it this way.

(will rustdoc show TryFrom on the AcpiSubType's page if it's implemented 
via sealed trait?)

>> +
>> +        impl $subtype_name {
>> +            /// Int value, representing this ACPI type (see [`acpitypes.h`]).
>> +            ///
>> +            /// [`acpitypes.h`]: srctree/include/linux/acpitypes.h
>> +            pub const ACPI_TYPE: u32 = bindings::$acpi_type;
>> +
>> +            /// Converts opaque AcpiObject reference into exact ACPI type reference.
>> +            ///
>> +            /// # Safety
>> +            ///
>> +            /// - Requested cast should be valid (value.type_id() is `Self::ACPI_TYPE`).
>> +            pub unsafe fn from_unchecked(value: &AcpiObject) -> &Self {
>> +                // SAFETY:
>> +                // - $field_name is currently active union's field due to external safety contract,
>> +                // - Transmuting to `repr(transparent)` wrapper is safe.
>> +                unsafe {
>> +                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
>> +                }
>> +            }
>> +        }
>> +    };
>> +}
>> +
>> +acpi_object_subtype!(AcpiBuffer
>> +    <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
>> +
>> +impl Deref for AcpiBuffer {
>> +    type Target = [u8];
>> +
>> +    fn deref(&self) -> &Self::Target {
>> +        // SAFETY: (pointer, length) indeed represents byte slice.
>> +        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
>> +    }
>> +}
> 

Re: [PATCH 2/3] rust: implement wrapper for acpi_object

[Miguel Ojeda]

On Thu, Jan 8, 2026 at 6:11 PM Gladyshev Ilya <foxido@foxido.dev> wrote:
>
> Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object()
> implementation, and it seems to me that the acpi_object's lifetime is
> the same as its internal buffer. Overall, it is indeed managed
> externally, but acpi_object and acpi_object::buffer->pointer live
> together. I’m not an ACPI expert, though, so maybe I’m missing something.
>
> Anyway, the current Rust setup seems fine for now:
> 0. AcpiObject validity is guaranteed by whoever constructed/passed it (C
> side for WMI abstractions, for example)
> 1. You can only convert &AcpiObject to &AcpiSubType (reference to
> reference), so AcpiSubType can't outlive AcpiObject
> 2. You can't steal the data pointer from &AcpiSubType either, because
> the Deref impl is "logically safe" and only gives you a reference to the
> inner data, which can't outlive AcpiSubType's reference -> can't outlive
> AcpiObject.
>
> So for now until AcpiObject lives _less_ than it's inner data,
> everything is OK.

Assuming this is correct, this sort of analysis is typically nice to
keep documented as as an implementation comment (`//`) somewhere
(instead of just in the mailing list or the commit message) -- would
that make sense?

> Using an enum would require keeping Rust's enum synced with the C side,
> as well as implementing some simple but non-trivial checks that the
> `type_` field is a valid enum value (and the valid range isn't
> continuous). I think that keeping it as an integer is simpler and better
> matches C side.

If this refers to the `ACPI_TYPE_*` constants, there seems to be a
comment in the C docs that requires it to be kept in sync already with
elsewhere, so perhaps it could be reasonable to add one more place to
sync? (Though I don't see the mentioned arrays from a quick grep?)

     * NOTE: Types must be kept in sync with the global acpi_ns_properties
     * and acpi_ns_type_names arrays.

Ideally, these would be actual `enum`s on the C side and then
eventually we should be able to have `bindgen` generate the new kind
of `enum` that keeps this in sync and generates those checks for us,
see my suggestion at:

    Support a new "enum variant" kind, similar to `rustified_enum`, that
    provides a safe method to transform a C `enum` to Rust.
    https://github.com/Rust-for-Linux/linux/issues/353

(But we can't do it just know, and even if it lands in `bindgen` we
will probably need to wait to bump the minimum etc.)

Cheers,
Miguel

Re: [PATCH 3/3] rust: add WMI abstractions

[Kari Argillander]

On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@foxido.dev> wrote:
>
> This introduces Rust abstraction for WMI subsystem via wmi::Driver trait
> and module_wmi_driver/wmi_device_table macros. Driver can be probed,
> notified on events and removed -- almost everything C-side can do.
>
> This abstraction will be used by subsequent redmi_wmi_rs reference
> driver.
>
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> ---
>  MAINTAINERS                     |   1 +
>  rust/bindings/bindings_helper.h |   1 +
>  rust/kernel/lib.rs              |   2 +
>  rust/kernel/wmi.rs              | 277 ++++++++++++++++++++++++++++++++
>  4 files changed, 281 insertions(+)
>  create mode 100644 rust/kernel/wmi.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 765ad2daa218..4909ae8be6e3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,7 @@ F:  Documentation/driver-api/wmi.rst
>  F:     Documentation/wmi/
>  F:     drivers/platform/wmi/
>  F:     include/uapi/linux/wmi.h
> +F:     rust/kernel/wmi.rs
>
>  ACRN HYPERVISOR SERVICE MODULE
>  M:     Fei Li <fei1.li@intel.com>
> 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 a6eccdba50b5..29bc2b87a103 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -152,6 +152,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..a15258365a2e
> --- /dev/null
> +++ b/rust/kernel/wmi.rs
> @@ -0,0 +1,277 @@
> +// 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::NonNull};

Also for these use vertical style.

> +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: Pin<&Self>, _dev: &Device<device::Core>, _event: Option<&AcpiObject>) {

This should be device::Bound

Also probably _ marks are not needed. I think compiler does give
unused build warnings.

I do not know reason but usually other drivers use this over self. And
device first so this
would be:

    fn notify(dev: &Device<device::Bound>, this: Pin<&Self>, event:
Option<&AcpiObject>) {

Same also in unbind. But like I said I'm not completely sure about this.

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

unbind should not be mandatory so here just do

    let _ = (self, dev);

Also comment can say little more.

> +}
> +
> +/// 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_UNBIND -> remove_callback);
> +            (*wdrv.get()).shutdown = None;
> +            (*wdrv.get()).no_singleton = true;
> +            (*wdrv.get()).no_notify_data = true;
> +        }
> +
> +        // SAFETY: `wdrv` is guaranteed to be a valid `RegType`.
> +        to_result(unsafe { bindings::__wmi_driver_register(wdrv.get(), module.0) })

Many drivers use .0 but there is ongoing work that .0 is not used. So
can you use
.as_ptr() here.

> +    }
> +
> +    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 around FFI object, so it's safe to cast
> +        //    raw pointer to reference (in terms of alignment and etc),
> +        // - Option<&ref> is guaranteed to have same layout as raw pointer (with NULL representing
> +        //    None) by Rust's "nullable pointer optimization".
> +        let obj: Option<&AcpiObject> = unsafe { core::mem::transmute(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 = 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_borrow::<T>() };
> +        this.unbind(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 { &raw 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 });

Missing safety comment.

> +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`].
> +///
> +/// [`struct wmi_device_id`]: https://docs.kernel.org/driver-api/basics.html#c.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() };

Use pin_init::zeroed() so no unsafe needed.

> +        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, so 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.52.0
>
>

Re: [PATCH 2/3] rust: implement wrapper for acpi_object

[Gary Guo]

On Thu Jan 8, 2026 at 5:11 PM GMT, Gladyshev Ilya wrote:
> On 1/8/26 16:21, Gary Guo wrote:
>> On Wed,  7 Jan 2026 23:35:32 +0300
>> Gladyshev Ilya <foxido@foxido.dev> wrote:
>> 
>>> ACPI Object is represented via union on C-side. On Rust side, this union
>>> is transparently wrapped for each ACPI Type, with individual methods and
>>> Defer implementation to represented type (integer, string, buffer, etc).
>>>
>>> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
>> 
>> 
>> Hi Gladyshev,
>> 
>> I've checked the `acpi_object` implementation on the C side and it appears
>> that the buffer is not owned by the object (however managed externally,
>> could either be resting in ACPI tables directly or be allocated).
> Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object() 
> implementation, and it seems to me that the acpi_object's lifetime is 
> the same as its internal buffer.

No, it's not the same. acpi_object's lifetime needs to be shorter than
the internal buffer.

In Rust this is a typical case where you'd put lifetime on the struct
where you write

struct AcpiObject<'a> { ... }

> Overall, it is indeed managed 
> externally, but acpi_object and acpi_object::buffer->pointer live 
> together. I’m not an ACPI expert, though, so maybe I’m missing something.
>
> Anyway, the current Rust setup seems fine for now:
> 0. AcpiObject validity is guaranteed by whoever constructed/passed it (C 
> side for WMI abstractions, for example)

When you construct a `AcpiObject`; it becomes incorrect: the constructed
`AcpiObject` now can outlive the internal buffer, which is broken.

Your code indeed works fine today, but the reason is that nobody can
construct a `AcpiObject`. They can only ever receive a reference, which
makes the issue disappear, as the lifetime gets "absorbed" by the
lifetime on the reference. In essense, whenever `&'a AcpiObject` appears
in your code, it's actually meant to be `&'a AcpiObject<'a>`.

If someone were to add a Rust-side constructor for `AcpiObject`, then
the lifetime becomes broken.

So it works today, but I think it gives the wrong impression to the user
that the buffer is managed by `AcpiObject`.

Best,
Gary

> 1. You can only convert &AcpiObject to &AcpiSubType (reference to 
> reference), so AcpiSubType can't outlive AcpiObject
> 2. You can't steal the data pointer from &AcpiSubType either, because 
> the Deref impl is "logically safe" and only gives you a reference to the 
> inner data, which can't outlive AcpiSubType's reference -> can't outlive 
> AcpiObject.
>
> So for now until AcpiObject lives _less_ than it's inner data, 
> everything is OK.
>
>> Therefore, you might want to carry a lifetime to represent the lifetime of
>> underlying buffers?

Re: [PATCH 3/3] rust: add WMI abstractions

[Kari Argillander]

On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@foxido.dev> wrote:
<snip>

> +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, so 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,
> +            );
> +        }

Just use while here so no unsafe is needed at all. Then probably patch
1/3 is not needed.

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

Re: [PATCH 2/3] rust: implement wrapper for acpi_object

[Gladyshev Ilya]

On 1/8/26 23:06, Gary Guo wrote:
> On Thu Jan 8, 2026 at 5:11 PM GMT, Gladyshev Ilya wrote:
>> On 1/8/26 16:21, Gary Guo wrote:
>>> On Wed,  7 Jan 2026 23:35:32 +0300
>>> Gladyshev Ilya <foxido@foxido.dev> wrote:
>>>
>>>> ACPI Object is represented via union on C-side. On Rust side, this union
>>>> is transparently wrapped for each ACPI Type, with individual methods and
>>>> Defer implementation to represented type (integer, string, buffer, etc).
>>>>
>>>> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
>>>
>>>
>>> Hi Gladyshev,
>>>
>>> I've checked the `acpi_object` implementation on the C side and it appears
>>> that the buffer is not owned by the object (however managed externally,
>>> could either be resting in ACPI tables directly or be allocated).
>> Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object()
>> implementation, and it seems to me that the acpi_object's lifetime is
>> the same as its internal buffer.
> 
> No, it's not the same. acpi_object's lifetime needs to be shorter than
> the internal buffer.

Okay, I agree than) Will fix in next revision

Re: [PATCH 2/3] rust: implement wrapper for acpi_object

[Gladyshev Ilya]

On 1/8/26 21:46, Miguel Ojeda wrote:
>> Using an enum would require keeping Rust's enum synced with the C side,
>> as well as implementing some simple but non-trivial checks that the
>> `type_` field is a valid enum value (and the valid range isn't
>> continuous). I think that keeping it as an integer is simpler and better
>> matches C side.
> 
> If this refers to the `ACPI_TYPE_*` constants, there seems to be a
> comment in the C docs that requires it to be kept in sync already with
> elsewhere, so perhaps it could be reasonable to add one more place to
> sync? (Though I don't see the mentioned arrays from a quick grep?)
> 
>       * NOTE: Types must be kept in sync with the global acpi_ns_properties
>       * and acpi_ns_type_names arrays.
> 
> [...snip about bindgen...]

If I understand correctly, acpi_object is something untrusted in general 
and broken hardware can provide arbitrary _type value. So ergonomics of 
enum solution would be kinda strange:

```
type_id() -> Result<Enum> // Result because it can be invalid

// ...

// '?' here makes it look like non-trivial operation
if x.type_id()? == Enum::Buffer
```

And it's unclear should ACPI_TYPE_INVALID be Ok(INVALID) or Err...

Also users of AcpiObject generally doesn't care if received object has 
valid, but unexpected type or invalid type, so this conversion int->enum 
will introduce (small but) unneeded overhead.

That's why I preferred int-style approach, matching C side. Here is 
alternative proposal from my side:

1. Enum with type values like you want
2. This Enum implements .id() method that returns underlying <int>
3. AcpiObject::type_id() still returns <int> value
4. There is method to convert int into enum (like TryFrom)

Probably you can 'newtype' <int> and implement TryFrom AcpiTypeId -> 
Enum and write something like

```
match x.type_id().try_into()? /* or as_enum() or smth */ {
   Enum::Integer => /* ... */,
   Enum::String => /* ... */
}

```


---
Sorry for my chaotic explanations...

Re: [PATCH 3/3] rust: add WMI abstractions

[Gladyshev Ilya]

On 1/8/26 23:48, Kari Argillander wrote:
> On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@foxido.dev> wrote:
> <snip>
> 
>> +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, so 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,
>> +            );
>> +        }
> 
> Just use while here so no unsafe is needed at all. Then probably patch
> 1/3 is not needed.

Overall this operation is still unsafe because we are constructing C 
string in FFI object. So for me avoiding `unsafe` via less readable 
(imo) loop will just mask unsafe operation without any real benefits.

Ideally this function should receive c string and just validate it's 
length, but IIRC I had troubles with build-time validation of C string 
length

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

Re: [PATCH 3/3] rust: add WMI abstractions

[Gladyshev Ilya]

On 1/8/26 22:48, Kari Argillander wrote:
> On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@foxido.dev> wrote:
[snip]
>> +    /// WMI device notify.
>> +    ///
>> +    /// Called when new WMI event received from bounded device.
>> +    fn notify(self: Pin<&Self>, _dev: &Device<device::Core>, _event: Option<&AcpiObject>) {
> 
> This should be device::Bound
> 
> Also probably _ marks are not needed. I think compiler does give
> unused build warnings.
> 
> I do not know reason but usually other drivers use this over self. And
> device first so this
> would be:
> 
>      fn notify(dev: &Device<device::Bound>, this: Pin<&Self>, event:
> Option<&AcpiObject>) {
> 
> Same also in unbind. But like I said I'm not completely sure about this.

I thought the reason for using this instead of self was because of the 
limited set of possible self types in previous versions of Rust...

IMO using Rust's self is more readable

>> +        build_error!(VTABLE_DEFAULT_ERROR);
>> +    }
>> +
>> +    /// WMI driver remove.
>> +    fn unbind(self: Pin<&Self>, _dev: &Device<device::Core>) {
>> +        build_error!(VTABLE_DEFAULT_ERROR);
>> +    }
> 
> unbind should not be mandatory so here just do
It's not mandatory, that why there is default implementation. See 
https://rust.docs.kernel.org/macros/attr.vtable.html


For other comments: Ack, thanks!

Re: [PATCH 3/3] rust: add WMI abstractions

[Kari Argillander]

pe 9.1.2026 klo 13.01 Gladyshev Ilya (foxido@foxido.dev) kirjoitti:
>
> On 1/8/26 23:48, Kari Argillander wrote:
> > On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@foxido.dev> wrote:
> > <snip>
> >
> >> +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, so 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,
> >> +            );
> >> +        }
> >
> > Just use while here so no unsafe is needed at all. Then probably patch
> > 1/3 is not needed.
>
> Overall this operation is still unsafe because we are constructing C
> string in FFI object. So for me avoiding `unsafe` via less readable
> (imo) loop will just mask unsafe operation without any real benefits.

It is not unsafe if you also use pin_init::zeroed()

        let mut inner: bindings::wmi_device_id = pin_init::zeroed();

        let mut i = 0usize;
        while i < bindings::UUID_STRING_LEN as usize {
            inner.guid_string[i] = guid[i];
            i += 1;
        }

you can then also remove 'core::mem::MaybeUninit'

> Ideally this function should receive c string and just validate it's
> length, but IIRC I had troubles with build-time validation of C string
> length
>
> >> +
> >> +        Self(inner)
> >> +    }
> >> +}

Re: [PATCH 3/3] rust: add WMI abstractions

[Kari Argillander]

pe 9.1.2026 klo 13.12 Gladyshev Ilya (foxido@foxido.dev) kirjoitti:
>
> On 1/8/26 22:48, Kari Argillander wrote:
> > On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@foxido.dev> wrote:
> [snip]
> >> +    /// WMI device notify.
> >> +    ///
> >> +    /// Called when new WMI event received from bounded device.
> >> +    fn notify(self: Pin<&Self>, _dev: &Device<device::Core>, _event: Option<&AcpiObject>) {
> >
> > This should be device::Bound
> >
> > Also probably _ marks are not needed. I think compiler does give
> > unused build warnings.
> >
> > I do not know reason but usually other drivers use this over self. And
> > device first so this
> > would be:
> >
> >      fn notify(dev: &Device<device::Bound>, this: Pin<&Self>, event:
> > Option<&AcpiObject>) {
> >
> > Same also in unbind. But like I said I'm not completely sure about this.
>
> I thought the reason for using this instead of self was because of the
> limited set of possible self types in previous versions of Rust...

Nice. Didn't know this. Thanks.

> IMO using Rust's self is more readable

Agreed.

> >> +        build_error!(VTABLE_DEFAULT_ERROR);
> >> +    }
> >> +
> >> +    /// WMI driver remove.
> >> +    fn unbind(self: Pin<&Self>, _dev: &Device<device::Core>) {
> >> +        build_error!(VTABLE_DEFAULT_ERROR);
> >> +    }
> >
> > unbind should not be mandatory so here just do
> It's not mandatory, that why there is default implementation. See
> https://rust.docs.kernel.org/macros/attr.vtable.html
>
>
> For other comments: Ack, thanks!

Re: [PATCH 3/3] rust: add WMI abstractions

[Gladyshev Ilya]

On 1/9/26 14:15, Kari Argillander wrote:
> pe 9.1.2026 klo 13.01 Gladyshev Ilya (foxido@foxido.dev) kirjoitti:
>>
>> On 1/8/26 23:48, Kari Argillander wrote:
>>> On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@foxido.dev> wrote:
>>> <snip>
>>>
>>>> +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, so 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,
>>>> +            );
>>>> +        }
>>>
>>> Just use while here so no unsafe is needed at all. Then probably patch
>>> 1/3 is not needed.
>>
>> Overall this operation is still unsafe because we are constructing C
>> string in FFI object. So for me avoiding `unsafe` via less readable
>> (imo) loop will just mask unsafe operation without any real benefits.
> 
> It is not unsafe if you also use pin_init::zeroed()
>          let mut inner: bindings::wmi_device_id = pin_init::zeroed();
> 
>          let mut i = 0usize;
>          while i < bindings::UUID_STRING_LEN as usize {
>              inner.guid_string[i] = guid[i];
>              i += 1;
>          }
> 
> you can then also remove 'core::mem::MaybeUninit'

I was talking more about "constructing C string is still 'unsafe' 
because you shouldn't miss the \0 byte". IMO unsafe but primitive memcpy 
is more readable and alerting than while loop, but that's not something 
I will insist on

I'll play around more with `guid: &CStr` API, probably I missed 
something simple)

Re: [PATCH 2/3] rust: implement wrapper for acpi_object

[Miguel Ojeda]

On Fri, Jan 9, 2026 at 11:57 AM Gladyshev Ilya <foxido@foxido.dev> wrote:
>
> If I understand correctly, acpi_object is something untrusted in general
> and broken hardware can provide arbitrary _type value. So ergonomics of
> enum solution would be kinda strange:
>
> ```
> type_id() -> Result<Enum> // Result because it can be invalid
>
> // ...
>
> // '?' here makes it look like non-trivial operation
> if x.type_id()? == Enum::Buffer
> ```

Yeah, I was mentioning the `bindgen` bit because returning
`Result<Enum>` is essentially what we asked for the generated code,
i.e. we generate a panicking `From` impl, a fallible `TryFrom` impl
and an unsafe conversion method too.

Whether that may add overhead or not, it depends, of course. Sometimes
the result (i.e. the enum) may need to be used later on in several
places, and knowing statically that the value is in-bounds means there
is no need to recheck anymore for that reason.

That can actually mean reducing overhead (e.g. if checks exist in
several APIs, possibly defensively) or at least reduce the mistakes
that may be made tracking whether the value is valid or not.

But, of course, if the value is not used for anything on the Rust
side, and is just passed along, and the consumer of the value in the C
side do not have UB if it is out of range, and you generally don't
expect that to change, then checking validity upfront may indeed not
add much value like you say. From what you say (I don't know), it
seems that is the case and thus a newtype may be simpler and best.

But if those conditions change, e.g. you later start to want to
manipulate the value on the Rust side for any reason, then it will
likely be a different story. In such a case, then we shouldn't be
scared of the ergonomics of conversions, i.e. it is fine and expected
to have to do that, which is why we want to generate all that
boilerplate in `bindgen` eventually.

Or perhaps you may need both options, i.e. the newtype, and the other
one, depending on what you are doing with the value.

Regarding `ACPI_TYPE_INVALID`, yeah, it would depend on what is used
for. If nobody should be calling a given API with such a value, then
that API should return `Err`. But if it gets used as a useful marker
value somewhere, then it may need to be a valid value and thus return
`Ok`. If both cases are true, then perhaps again two ways may be
needed to model those.

Cheers,
Miguel