Skip to contentSkip to conversation

rust: WMI abstractions

PATCH In review Gladyshev Ilya 4 participants 6 messages Jan 11 – Jan 12

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover rust: WMI abstractions

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 existance is a reference driver (rewrite of redmi-wmi, Armin is OK with that [1]). While being fully functional, it depends on sparse-keyboard abstractions, which I am still polishing. You can view WIP abstractions & driver at [2].

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/[email protected]/
[1]: https://lore.kernel.org/rust-for-linux/[email protected]/
[2]: https://git.foxido.dev/foxido/linux/-/commits/rust_wmi?ref_type=heads

---
Changes since v1:
- Add lifetime to AcpiObject
- Add forgotten Sync requirement to the WMI driver trait
- Drop unsafe in DeviceId::new() (so no need for the const_intrinsic_copy feature)
- Device<Core> -> Device<Bound> in notify
- Small fixes (styling, missing safety comment)
- Add link to the WIP reference driver

I didn't touch AcpiObject::type_id() in v2 because it does its job for now. It's currently small (only one subtype, whereas both enum options would introduce a larger integration surface), and it would be an easy change if someone needs a more powerful solution.

I've also played around with some other suggestions, but:
- Suggested wrapping acpi_object instead of bindgen union subtypes will only introduce unneeded unsafety in each subtype's operation. I decided it's better to expose the bindgen type once than to validate a safety invariant each time.
- Suggested implementation via a sealed trait didn't really provide any benefits; it just raised code complexity a bit. Maybe I did it wrong, though...

Link to v1: https://lore.kernel.org/rust-for-linux/[email protected]/
---

Gladyshev Ilya (2):
rust: implement wrapper for acpi_object
rust: add WMI abstractions

 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/acpi.rs             |  97 +++++++++++
 rust/kernel/lib.rs              |   2 +
 rust/kernel/wmi.rs              | 286 ++++++++++++++++++++++++++++++++
 5 files changed, 387 insertions(+)
 create mode 100644 rust/kernel/wmi.rs

base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb

Signature · 1 line
2.52.0
#1 lore raw

PATCH v2 1/2 rust: implement wrapper for acpi_object

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

---
 rust/kernel/acpi.rs | 97 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
rust/kernel/acpi.rs +97 -0
index 9b8efa623130..c730c12d3979 100644
@@ -2,6 +2,11 @@
 
 //! Advanced Configuration and Power Interface abstractions.
 
+use core::{
+    marker::PhantomData,
+    ops::Deref, //
+};
+
 use crate::{
     bindings,
     device_id::{RawDeviceId, RawDeviceIdIndex},
@@ -63,3 +68,95 @@ 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<'a> {
+    inner: bindings::acpi_object,
+    _p: PhantomData<&'a 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.inner.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<'a>> for &'a $subtype_name {
+            type Error = Error;
+
+            fn try_from(value: &'a AcpiObject<'a>) -> 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<'a>(value: &'a AcpiObject<'a>) -> &'a 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.inner.$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) }
+    }
+}
Signature · 1 line
2.52.0
reviewer
#2 lore raw

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

Quoted 42 lines · 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).

---
 rust/kernel/acpi.rs | 97 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
rust/kernel/acpi.rs +18 -0
index 9b8efa623130..c730c12d3979 100644
@@ -2,6 +2,11 @@
 
 //! Advanced Configuration and Power Interface abstractions.
 
+use core::{
+    marker::PhantomData,
+    ops::Deref, //
+};
+
 use crate::{
     bindings,
     device_id::{RawDeviceId, RawDeviceIdIndex},
@@ -63,3 +68,95 @@ 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()?;

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

Quoted 10 lines
rust/kernel/acpi.rs +10 -0
+///
+///     Ok(buf[0])
+/// }
+/// ```
+///
+/// [`struct acpi_object`]: srctree/include/acpi/actypes.h
+#[repr(transparent)]
+pub struct AcpiObject<'a> {
+    inner: bindings::acpi_object,
+    _p: PhantomData<&'a bindings::acpi_object>,

This can just be &'a ().

Quoted 49 lines
rust/kernel/acpi.rs +49 -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.inner.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<'a>> for &'a $subtype_name {
+            type Error = Error;
+
+            fn try_from(value: &'a AcpiObject<'a>) -> 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<'a>(value: &'a AcpiObject<'a>) -> &'a Self {
+                // SAFETY:
+                // - $field_name is currently active union's field due to external safety contract,

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

Quoted 10 lines
rust/kernel/acpi.rs +10 -0
+                // - Transmuting to `repr(transparent)` wrapper is safe.
+                unsafe {
+                    ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.inner.$field_name)
+                }
+            }
+        }
+    };
+}
+
+acpi_object_subtype!(AcpiBuffer

The AcpiBuffer type needs a lifetime too.

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

Quoted 7 lines
rust/kernel/acpi.rs +7 -0
+    <- (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.

The safety comment needs to mention on the lifetime.

To make this explict you could do

    fn as_slice(&self) -> &'a [u8] { ... }

and then just call it inside Deref.

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

Quoted 3 lines
rust/kernel/acpi.rs +3 -0
+        unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
+    }
+}

Best,
Gary

#3 lore raw

PATCH v2 2/2 rust: add WMI abstractions

In reply to 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.

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/wmi.rs              | 286 ++++++++++++++++++++++++++++++++
 4 files changed, 290 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 f812cf120042..db3e649d26eb 100644
@@ -151,6 +151,8 @@
 pub mod uaccess;
 #[cfg(CONFIG_USB = "y")]
 pub mod usb;
+#[cfg(CONFIG_ACPI_WMI)]
+pub mod wmi;
 pub mod workqueue;
 pub mod xarray;
 
rust/kernel/wmi.rs new file +286 -0

286 lines not on this page · raw message

Signature · 1 line
2.52.0
#4 lore raw

Re: [PATCH v2 2/2] rust: add WMI abstractions

Still found couple small things. Overall these looks really good to me. Waiting for more. Please CC also me to this series. Usually you CC people who do some reviewing as then they notice right away when new series comes and can give another look.

Quoted 84 lines · 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.

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/wmi.rs              | 286 ++++++++++++++++++++++++++++++++
 4 files changed, 290 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 f812cf120042..db3e649d26eb 100644
@@ -151,6 +151,8 @@
 pub mod uaccess;
 #[cfg(CONFIG_USB = "y")]
 pub mod usb;
+#[cfg(CONFIG_ACPI_WMI)]
+pub mod wmi;
 pub mod workqueue;
 pub mod xarray;
 
rust/kernel/wmi.rs new file +24 -0
new file mode 100644
index 000000000000..d6b9bc29a4d6
@@ -0,0 +1,286 @@
+// 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,

rust/kernel/wmi.rs line 34

Quoted 10 lines
rust/kernel/wmi.rs new file +10 -0
+    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.
+///
+/// Driver can be called from arbitary thread without any ordering guarantees.

s/arbitary/arbitrary

Quoted 7 lines
rust/kernel/wmi.rs new file +7 -0
+#[vtable]
+pub trait Driver: Send + Sync {
+    /// 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 line 51

Quoted 10 lines
rust/kernel/wmi.rs new file +10 -0
+
+    /// 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.

Maybe add here little bit info what does it mean that AcpiObject is Optional. Something like:

/// Some notification events do not provide any additional ACPI data. /// In those cases, event will be None.

rust/kernel/wmi.rs line 58

Quoted 7 lines
rust/kernel/wmi.rs new file +7 -0
+    fn notify(self: Pin<&Self>, _dev: &Device<device::Bound>, _event: Option<&AcpiObject<'_>>) {
+        build_error!(VTABLE_DEFAULT_ERROR);
+    }
+
+    /// WMI driver remove.
+    ///
+    /// Called when the WMI driver is unbound from a WMI device.

Just copy paste this from example PCI. That way it is clear that usually this should not be used.

    /// WMI driver unbind.
    ///
    /// Called when a [`Device`] is unbound from its bound [`Driver`].
Implementing this callback
    /// is optional.
    ///
    /// This callback serves as a place for drivers to perform
teardown operations that require a
    /// `&Device<Core>` or `&Device<Bound>` reference. For instance,
drivers may try to perform I/O
    /// operations to gracefully tear down the device.
    ///
    /// Otherwise, release operations for driver resources should be

performed in Self::drop.

Quoted 44 lines
rust/kernel/wmi.rs new file +44 -0
+    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
+                }
+            };
+        }
+

rust/kernel/wmi.rs line 112

Quoted 10 lines
rust/kernel/wmi.rs new file +10 -0
+        // 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;

Maybe comment for no_singleton

    // Every new WMI driver needs to set this true.

rust/kernel/wmi.rs line 113

Quoted 1 line
rust/kernel/wmi.rs new file +1 -0
+            (*wdrv.get()).no_notify_data = true;

Maybe add comment for no_notify_data

    // With this notify_callback() can receive empty notification
    // which is we then transform to 'None'.
Quoted 14 lines
rust/kernel/wmi.rs new file +14 -0
+        }
+
+        // SAFETY: `wdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::__wmi_driver_register(wdrv.get(), module.as_ptr()) })
+    }
+
+    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(

rust/kernel/wmi.rs line 137

Quoted 10 lines
rust/kernel/wmi.rs new file +10 -0
+        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;

Convention has been that "as" cast has comments like

    // CAST:

This can be added also other places. This is just nit from me and I do not require this. So up to you.

Quoted 39 lines
rust/kernel/wmi.rs new file +39 -0
+        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

rust/kernel/wmi.rs line 186

Quoted 10 lines
rust/kernel/wmi.rs new file +10 -0
+        // 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`.

Safety comment seems copy pasted so it is wrong.

Quoted 104 lines
rust/kernel/wmi.rs new file +100 -1
+        let dev = unsafe { &raw mut (*self.inner.get()).dev };
+
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
+// argument.
+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 {
+    const GUID_LEN: usize = bindings::UUID_STRING_LEN as usize;
+
+    /// Constructs new DeviceId from GUID string.
+    pub const fn new(guid: &[u8; Self::GUID_LEN]) -> Self {
+        let mut inner: bindings::wmi_device_id = pin_init::zeroed();
+
+        build_assert!(inner.guid_string.len() == Self::GUID_LEN + 1);
+
+        // We are copying UUID_STRING_LEN bytes and we verified that UUID_STRING_LEN + 1 byte
+        // exists and will remain '\0'. So we will construct valid C string.
+        let mut i = 0;
+        while i < Self::GUID_LEN {
+            inner.guid_string[i] = guid[i];
+            i += 1;
+        }
+
+        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

#5 lore raw
In reply to Gladyshev Ilya

Am 11.01.26 um 19:31 schrieb Gladyshev Ilya:

Quoted 16 lines

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 existance is a reference driver (rewrite of redmi-wmi, Armin is OK with that [1]). While being fully functional, it depends on sparse-keyboard abstractions, which I am still polishing. You can view WIP abstractions & driver at [2].

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.

Quoted 3 lines

[0]: https://lore.kernel.org/rust-for-linux/[email protected]/
[1]: https://lore.kernel.org/rust-for-linux/[email protected]/
[2]: https://git.foxido.dev/foxido/linux/-/commits/rust_wmi?ref_type=heads

Just a side note:

The patch series for the buffer-based WMI API (https://lore.kernel.org/platform-driver-x86/[email protected]/T/#mb8d1591e475a6c346421e8875a1307f3ed1528af)
was accepted into the pdx86 kernel tree and will soon show up on the for-next branch. If you want you can target this API instead because
it replaces the usage of ACPI objects with a plain buffer that might be easier to model in rust.

Thanks,
Armin Wolf

Quoted 37 lines

---
Changes since v1:
- Add lifetime to AcpiObject
- Add forgotten Sync requirement to the WMI driver trait
- Drop unsafe in DeviceId::new() (so no need for the const_intrinsic_copy feature)
- Device<Core> -> Device<Bound> in notify
- Small fixes (styling, missing safety comment)
- Add link to the WIP reference driver

I didn't touch AcpiObject::type_id() in v2 because it does its job for now. It's currently small (only one subtype, whereas both enum options would introduce a larger integration surface), and it would be an easy change if someone needs a more powerful solution.

I've also played around with some other suggestions, but:
- Suggested wrapping acpi_object instead of bindgen union subtypes will only introduce unneeded unsafety in each subtype's operation. I decided it's better to expose the bindgen type once than to validate a safety invariant each time.
- Suggested implementation via a sealed trait didn't really provide any benefits; it just raised code complexity a bit. Maybe I did it wrong, though...

Link to v1: https://lore.kernel.org/rust-for-linux/[email protected]/
---

Gladyshev Ilya (2):
rust: implement wrapper for acpi_object
rust: add WMI abstractions

  MAINTAINERS                     |   1 +
  rust/bindings/bindings_helper.h |   1 +
  rust/kernel/acpi.rs             |  97 +++++++++++
  rust/kernel/lib.rs              |   2 +
  rust/kernel/wmi.rs              | 286 ++++++++++++++++++++++++++++++++
  5 files changed, 387 insertions(+)

create mode 100644 rust/kernel/wmi.rs

base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb