rust: WMI abstractions

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 <[email protected]>

---
 rust/kernel/acpi.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

rust/kernel/acpi.rs +20 -0

index 9b8efa623130..ac1a9f305f6c 100644
@@ -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])
+/// }
+/// ```
+///

rust/kernel/acpi.rs +35 -0

+
+        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) }
+    }
+}

Quoted 4 lines · On Wed, 7 Jan 2026 23:35:32 +0300 Gladyshev Ilya <[email protected]> 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).

Quoted 1 line

Signed-off-by Gladyshev Ilya <[email protected]>

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?

Quoted 43 lines

---
  rust/kernel/acpi.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 91 insertions(+)

rust/kernel/acpi.rs +24 -0

index 9b8efa623130..ac1a9f305f6c 100644
@@ -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);
+

rust/kernel/acpi.rs macro_rules! acpi_device_table { · line 95

Quoted 6 lines

rust/kernel/acpi.rs +6 -0

+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.

rust/kernel/acpi.rs macro_rules! acpi_device_table { · line 108

Quoted 13 lines

rust/kernel/acpi.rs +13 -0

+}
+
+/// 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.

Quoted 8 lines

rust/kernel/acpi.rs +8 -0

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

rust/kernel/acpi.rs macro_rules! acpi_device_table { · line 121

Quoted 5 lines

rust/kernel/acpi.rs +5 -0

+
+                // 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?

Quoted 35 lines

rust/kernel/acpi.rs +35 -0

+
+        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) }
+    }
+}

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.

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.

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 <[email protected]>

---
 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

MAINTAINERS +1 -0

index 765ad2daa218..4909ae8be6e3 100644
@@ -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 <[email protected]>

rust/bindings/bindings_helper.h +1 -0

index a067038b4b42..f9671280c6b5 100644
@@ -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>
 

rust/kernel/lib.rs +2 -0

index a6eccdba50b5..29bc2b87a103 100644
@@ -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;
 

rust/kernel/wmi.rs new file +13 -0

new file mode 100644
index 000000000000..a15258365a2e
@@ -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, //
+    },

rust/kernel/wmi.rs new file +14 -0

+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>;
+

rust/kernel/wmi.rs new file +46 -0

+}
+
+/// 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();

rust/kernel/wmi.rs new file +66 -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> {

rust/kernel/wmi.rs new file +16 -0

+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`].

rust/kernel/wmi.rs new file +64 -1

+        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

Quoted 10 lines · Gary Guo

Quoted 5 lines · On Wed, 7 Jan 2026 23:35:32 +0300 Gladyshev Ilya <[email protected]> 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 <[email protected]>

Quoted 5 lines

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.

Quoted 2 lines

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

rust/kernel/wmi.rs new file +7 -0

+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);

rust/kernel/wmi.rs new file +4 -0

+
+        Self(inner)
+    }
+}

Quoted 16 lines · Gladyshev Ilya

Quoted 15 lines · Gary Guo

Quoted 5 lines · On Wed, 7 Jan 2026 23:35:32 +0300 Gladyshev Ilya <[email protected]> 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 <[email protected]>

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).

Quoted 3 lines

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.

Quoted 5 lines

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...]

On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <[email protected]> wrote:
<snip>

Quoted 12 lines

rust/kernel/wmi.rs new file +12 -0

+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(

rust/kernel/wmi.rs line 224

Quoted 5 lines

rust/kernel/wmi.rs new file +5 -0

+                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.

Quoted 4 lines

rust/kernel/wmi.rs new file +4 -0

+
+        Self(inner)
+    }
+}

On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <[email protected]> wrote:

rust/kernel/wmi.rs line 47

Quoted 4 lines

rust/kernel/wmi.rs new file +4 -0

+    /// 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.

rust/kernel/wmi.rs line 54

Quoted 7 lines

rust/kernel/wmi.rs new file +7 -0

+        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

Quoted 15 lines · Gladyshev Ilya

<snip>

Quoted 12 lines

rust/kernel/wmi.rs new file +12 -0

+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(

Quoted 8 lines

rust/kernel/wmi.rs line 224

Quoted 5 lines

rust/kernel/wmi.rs new file +5 -0

+                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

Quoted 4 lines

Quoted 4 lines

rust/kernel/wmi.rs new file +4 -0

+
+        Self(inner)
+    }
+}

Quoted 1 line · Gladyshev Ilya

[snip]

Quoted 14 lines

rust/kernel/wmi.rs line 47

Quoted 4 lines

rust/kernel/wmi.rs new file +4 -0

+    /// 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:

Quoted 4 lines

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

Quoted 9 lines

rust/kernel/wmi.rs line 54

Quoted 7 lines

rust/kernel/wmi.rs new file +7 -0

+        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!

pe 9.1.2026 klo 13.01 Gladyshev Ilya ([email protected]) kirjoitti:

Quoted 26 lines · Kari Argillander

Quoted 15 lines · Gladyshev Ilya

<snip>

Quoted 12 lines

rust/kernel/wmi.rs new file +12 -0

+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(

Quoted 8 lines

rust/kernel/wmi.rs line 224

Quoted 5 lines

rust/kernel/wmi.rs new file +5 -0

+                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.

Quoted 3 lines

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'

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
```