Device / Driver PCI / Platform Rust abstractions

From: Wedson Almeida Filho <[email protected]>

Revocable allows access to objects to be safely revoked at run time.

This is useful, for example, for resources allocated during device probe; when the device is removed, the driver should stop accessing the device resources even if another state is kept in memory due to existing references (i.e., device context data is ref-counted and has a non-zero refcount after removal of the device).

Signed-off-by Wedson Almeida Filho <[email protected]>

Co-developed-by Danilo Krummrich <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/lib.rs       |   1 +
 rust/kernel/revocable.rs | 223 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 224 insertions(+)
 create mode 100644 rust/kernel/revocable.rs

rust/kernel/lib.rs +1 -0

index 66149ac5c0c9..5702ce32ec8e 100644
@@ -60,6 +60,7 @@
 pub mod prelude;
 pub mod print;
 pub mod rbtree;
+pub mod revocable;
 pub mod security;
 pub mod seq_file;
 pub mod sizes;

rust/kernel/revocable.rs new file +124 -0

new file mode 100644
index 000000000000..e464d59eb6b5
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Revocable objects.
+//!
+//! The [`Revocable`] type wraps other types and allows access to them to be revoked. The existence
+//! of a [`RevocableGuard`] ensures that objects remain valid.
+
+use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
+use core::{
+    marker::PhantomData,
+    ops::Deref,
+    ptr::drop_in_place,
+    sync::atomic::{AtomicBool, Ordering},
+};
+
+/// An object that can become inaccessible at runtime.
+///
+/// Once access is revoked and all concurrent users complete (i.e., all existing instances of
+/// [`RevocableGuard`] are dropped), the wrapped object is also dropped.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::revocable::Revocable;
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// fn add_two(v: &Revocable<Example>) -> Option<u32> {
+///     let guard = v.try_access()?;
+///     Some(guard.a + guard.b)
+/// }
+///
+/// let v = KBox::pin_init(Revocable::new(Example { a: 10, b: 20 }), GFP_KERNEL).unwrap();
+/// assert_eq!(add_two(&v), Some(30));
+/// v.revoke();
+/// assert_eq!(add_two(&v), None);
+/// ```
+///
+/// Sample example as above, but explicitly using the rcu read side lock.
+///
+/// ```
+/// # use kernel::revocable::Revocable;
+/// use kernel::sync::rcu;
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// fn add_two(v: &Revocable<Example>) -> Option<u32> {
+///     let guard = rcu::read_lock();
+///     let e = v.try_access_with_guard(&guard)?;
+///     Some(e.a + e.b)
+/// }
+///
+/// let v = KBox::pin_init(Revocable::new(Example { a: 10, b: 20 }), GFP_KERNEL).unwrap();
+/// assert_eq!(add_two(&v), Some(30));
+/// v.revoke();
+/// assert_eq!(add_two(&v), None);
+/// ```
+#[pin_data(PinnedDrop)]
+pub struct Revocable<T> {
+    is_available: AtomicBool,
+    #[pin]
+    data: Opaque<T>,
+}
+
+// SAFETY: `Revocable` is `Send` if the wrapped object is also `Send`. This is because while the
+// functionality exposed by `Revocable` can be accessed from any thread/CPU, it is possible that
+// this isn't supported by the wrapped object.
+unsafe impl<T: Send> Send for Revocable<T> {}
+
+// SAFETY: `Revocable` is `Sync` if the wrapped object is both `Send` and `Sync`. We require `Send`
+// from the wrapped object as well because  of `Revocable::revoke`, which can trigger the `Drop`
+// implementation of the wrapped object from an arbitrary thread.
+unsafe impl<T: Sync + Send> Sync for Revocable<T> {}
+
+impl<T> Revocable<T> {
+    /// Creates a new revocable instance of the given data.
+    pub fn new(data: impl PinInit<T>) -> impl PinInit<Self> {
+        pin_init!(Self {
+            is_available: AtomicBool::new(true),
+            data <- Opaque::pin_init(data),
+        })
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a guard that gives access to the object otherwise; the object is guaranteed to
+    /// remain accessible while the guard is alive. In such cases, callers are not allowed to sleep
+    /// because another CPU may be waiting to complete the revocation of this object.
+    pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
+        let guard = rcu::read_lock();
+        if self.is_available.load(Ordering::Relaxed) {
+            // Since `self.is_available` is true, data is initialised and has to remain valid
+            // because the RCU read side lock prevents it from being dropped.
+            Some(RevocableGuard::new(self.data.get(), guard))
+        } else {
+            None
+        }
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a shared reference to the object otherwise; the object is guaranteed to
+    /// remain accessible while the rcu read side guard is alive. In such cases, callers are not
+    /// allowed to sleep because another CPU may be waiting to complete the revocation of this
+    /// object.
+    pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
+        if self.is_available.load(Ordering::Relaxed) {
+            // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
+            // valid because the RCU read side lock prevents it from being dropped.
+            Some(unsafe { &*self.data.get() })
+        } else {
+            None
+        }
+    }

rust/kernel/revocable.rs new file +89 -1

+            if SYNC {
+                // SAFETY: Just an FFI call, there are no further requirements.
+                unsafe { bindings::synchronize_rcu() };
+            }
+
+            // SAFETY: We know `self.data` is valid because only one CPU can succeed the
+            // `compare_exchange` above that takes `is_available` from `true` to `false`.
+            unsafe { drop_in_place(self.data.get()) };
+        }
+    }
+
+    /// Revokes access to and drops the wrapped object.
+    ///
+    /// Access to the object is revoked immediately to new callers of [`Revocable::try_access`],
+    /// expecting that there are no concurrent users of the object.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that there are no more concurrent users of the revocable object.
+    pub unsafe fn revoke_nosync(&self) {
+        // SAFETY: By the safety requirement of this function, the caller ensures that nobody is
+        // accessing the data anymore and hence we don't have to wait for the grace period to
+        // finish.
+        unsafe { self.revoke_internal::<false>() }
+    }
+
+    /// Revokes access to and drops the wrapped object.
+    ///
+    /// Access to the object is revoked immediately to new callers of [`Revocable::try_access`].
+    ///
+    /// If there are concurrent users of the object (i.e., ones that called
+    /// [`Revocable::try_access`] beforehand and still haven't dropped the returned guard), this
+    /// function waits for the concurrent access to complete before dropping the wrapped object.
+    pub fn revoke(&self) {
+        // SAFETY: By passing `true` we ask `revoke_internal` to wait for the grace period to
+        // finish.
+        unsafe { self.revoke_internal::<true>() }
+    }
+}
+
+#[pinned_drop]
+impl<T> PinnedDrop for Revocable<T> {
+    fn drop(self: Pin<&mut Self>) {
+        // Drop only if the data hasn't been revoked yet (in which case it has already been
+        // dropped).
+        // SAFETY: We are not moving out of `p`, only dropping in place
+        let p = unsafe { self.get_unchecked_mut() };
+        if *p.is_available.get_mut() {
+            // SAFETY: We know `self.data` is valid because no other CPU has changed
+            // `is_available` to `false` yet, and no other CPU can do it anymore because this CPU
+            // holds the only reference (mutable) to `self` now.
+            unsafe { drop_in_place(p.data.get()) };
+        }
+    }
+}
+
+/// A guard that allows access to a revocable object and keeps it alive.
+///
+/// CPUs may not sleep while holding on to [`RevocableGuard`] because it's in atomic context
+/// holding the RCU read-side lock.
+///
+/// # Invariants
+///
+/// The RCU read-side lock is held while the guard is alive.
+pub struct RevocableGuard<'a, T> {
+    data_ref: *const T,
+    _rcu_guard: rcu::Guard,
+    _p: PhantomData<&'a ()>,
+}
+
+impl<T> RevocableGuard<'_, T> {
+    fn new(data_ref: *const T, rcu_guard: rcu::Guard) -> Self {
+        Self {
+            data_ref,
+            _rcu_guard: rcu_guard,
+            _p: PhantomData,
+        }
+    }
+}
+
+impl<T> Deref for RevocableGuard<'_, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: By the type invariants, we hold the rcu read-side lock, so the object is
+        // guaranteed to remain valid.
+        unsafe { &*self.data_ref }
+    }
+}
--

2.47.1

Quoted 167 lines · On Thu, 12 Dec 2024 17:33:37 +0100 Danilo Krummrich <[email protected]> wrote:

From: Wedson Almeida Filho <[email protected]>

Revocable allows access to objects to be safely revoked at run time.

This is useful, for example, for resources allocated during device probe; when the device is removed, the driver should stop accessing the device resources even if another state is kept in memory due to existing references (i.e., device context data is ref-counted and has a non-zero refcount after removal of the device).

Signed-off-by Wedson Almeida Filho <[email protected]>

Co-developed-by Danilo Krummrich <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/lib.rs       |   1 +
 rust/kernel/revocable.rs | 223 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 224 insertions(+)
 create mode 100644 rust/kernel/revocable.rs

rust/kernel/lib.rs +1 -0

index 66149ac5c0c9..5702ce32ec8e 100644
@@ -60,6 +60,7 @@
 pub mod prelude;
 pub mod print;
 pub mod rbtree;
+pub mod revocable;
 pub mod security;
 pub mod seq_file;
 pub mod sizes;

rust/kernel/revocable.rs new file +130 -0

new file mode 100644
index 000000000000..e464d59eb6b5
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Revocable objects.
+//!
+//! The [`Revocable`] type wraps other types and allows access to them to be revoked. The existence
+//! of a [`RevocableGuard`] ensures that objects remain valid.
+
+use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
+use core::{
+    marker::PhantomData,
+    ops::Deref,
+    ptr::drop_in_place,
+    sync::atomic::{AtomicBool, Ordering},
+};
+
+/// An object that can become inaccessible at runtime.
+///
+/// Once access is revoked and all concurrent users complete (i.e., all existing instances of
+/// [`RevocableGuard`] are dropped), the wrapped object is also dropped.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::revocable::Revocable;
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// fn add_two(v: &Revocable<Example>) -> Option<u32> {
+///     let guard = v.try_access()?;
+///     Some(guard.a + guard.b)
+/// }
+///
+/// let v = KBox::pin_init(Revocable::new(Example { a: 10, b: 20 }), GFP_KERNEL).unwrap();
+/// assert_eq!(add_two(&v), Some(30));
+/// v.revoke();
+/// assert_eq!(add_two(&v), None);
+/// ```
+///
+/// Sample example as above, but explicitly using the rcu read side lock.
+///
+/// ```
+/// # use kernel::revocable::Revocable;
+/// use kernel::sync::rcu;
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// fn add_two(v: &Revocable<Example>) -> Option<u32> {
+///     let guard = rcu::read_lock();
+///     let e = v.try_access_with_guard(&guard)?;
+///     Some(e.a + e.b)
+/// }
+///
+/// let v = KBox::pin_init(Revocable::new(Example { a: 10, b: 20 }), GFP_KERNEL).unwrap();
+/// assert_eq!(add_two(&v), Some(30));
+/// v.revoke();
+/// assert_eq!(add_two(&v), None);
+/// ```
+#[pin_data(PinnedDrop)]
+pub struct Revocable<T> {
+    is_available: AtomicBool,
+    #[pin]
+    data: Opaque<T>,
+}
+
+// SAFETY: `Revocable` is `Send` if the wrapped object is also `Send`. This is because while the
+// functionality exposed by `Revocable` can be accessed from any thread/CPU, it is possible that
+// this isn't supported by the wrapped object.
+unsafe impl<T: Send> Send for Revocable<T> {}
+
+// SAFETY: `Revocable` is `Sync` if the wrapped object is both `Send` and `Sync`. We require `Send`
+// from the wrapped object as well because  of `Revocable::revoke`, which can trigger the `Drop`
+// implementation of the wrapped object from an arbitrary thread.
+unsafe impl<T: Sync + Send> Sync for Revocable<T> {}
+
+impl<T> Revocable<T> {
+    /// Creates a new revocable instance of the given data.
+    pub fn new(data: impl PinInit<T>) -> impl PinInit<Self> {
+        pin_init!(Self {
+            is_available: AtomicBool::new(true),
+            data <- Opaque::pin_init(data),
+        })
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a guard that gives access to the object otherwise; the object is guaranteed to
+    /// remain accessible while the guard is alive. In such cases, callers are not allowed to sleep
+    /// because another CPU may be waiting to complete the revocation of this object.
+    pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
+        let guard = rcu::read_lock();
+        if self.is_available.load(Ordering::Relaxed) {
+            // Since `self.is_available` is true, data is initialised and has to remain valid
+            // because the RCU read side lock prevents it from being dropped.
+            Some(RevocableGuard::new(self.data.get(), guard))
+        } else {
+            None
+        }
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a shared reference to the object otherwise; the object is guaranteed to
+    /// remain accessible while the rcu read side guard is alive. In such cases, callers are not
+    /// allowed to sleep because another CPU may be waiting to complete the revocation of this
+    /// object.
+    pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
+        if self.is_available.load(Ordering::Relaxed) {
+            // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
+            // valid because the RCU read side lock prevents it from being dropped.
+            Some(unsafe { &*self.data.get() })
+        } else {
+            None
+        }
+    }
+
+    /// # Safety
+    ///
+    /// Callers must ensure that there are no more concurrent users of the revocable object.
+    unsafe fn revoke_internal<const SYNC: bool>(&self) {
+        if self

rust/kernel/revocable.rs line 134

Quoted 4 lines

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

+            .is_available
+            .compare_exchange(true, false, Ordering::Relaxed, Ordering::Relaxed)
+            .is_ok()
+        {

The comment I made in a previous series was somehow lost, so I put it back here:
You can use self.is_available.swap(false, Ordering::Relaxed) instead of a CAS, it is IMO clearer and optimizes better on some architectures.

Quoted 91 lines

rust/kernel/revocable.rs new file +89 -1

+            if SYNC {
+                // SAFETY: Just an FFI call, there are no further requirements.
+                unsafe { bindings::synchronize_rcu() };
+            }
+
+            // SAFETY: We know `self.data` is valid because only one CPU can succeed the
+            // `compare_exchange` above that takes `is_available` from `true` to `false`.
+            unsafe { drop_in_place(self.data.get()) };
+        }
+    }
+
+    /// Revokes access to and drops the wrapped object.
+    ///
+    /// Access to the object is revoked immediately to new callers of [`Revocable::try_access`],
+    /// expecting that there are no concurrent users of the object.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that there are no more concurrent users of the revocable object.
+    pub unsafe fn revoke_nosync(&self) {
+        // SAFETY: By the safety requirement of this function, the caller ensures that nobody is
+        // accessing the data anymore and hence we don't have to wait for the grace period to
+        // finish.
+        unsafe { self.revoke_internal::<false>() }
+    }
+
+    /// Revokes access to and drops the wrapped object.
+    ///
+    /// Access to the object is revoked immediately to new callers of [`Revocable::try_access`].
+    ///
+    /// If there are concurrent users of the object (i.e., ones that called
+    /// [`Revocable::try_access`] beforehand and still haven't dropped the returned guard), this
+    /// function waits for the concurrent access to complete before dropping the wrapped object.
+    pub fn revoke(&self) {
+        // SAFETY: By passing `true` we ask `revoke_internal` to wait for the grace period to
+        // finish.
+        unsafe { self.revoke_internal::<true>() }
+    }
+}
+
+#[pinned_drop]
+impl<T> PinnedDrop for Revocable<T> {
+    fn drop(self: Pin<&mut Self>) {
+        // Drop only if the data hasn't been revoked yet (in which case it has already been
+        // dropped).
+        // SAFETY: We are not moving out of `p`, only dropping in place
+        let p = unsafe { self.get_unchecked_mut() };
+        if *p.is_available.get_mut() {
+            // SAFETY: We know `self.data` is valid because no other CPU has changed
+            // `is_available` to `false` yet, and no other CPU can do it anymore because this CPU
+            // holds the only reference (mutable) to `self` now.
+            unsafe { drop_in_place(p.data.get()) };
+        }
+    }
+}
+
+/// A guard that allows access to a revocable object and keeps it alive.
+///
+/// CPUs may not sleep while holding on to [`RevocableGuard`] because it's in atomic context
+/// holding the RCU read-side lock.
+///
+/// # Invariants
+///
+/// The RCU read-side lock is held while the guard is alive.
+pub struct RevocableGuard<'a, T> {
+    data_ref: *const T,
+    _rcu_guard: rcu::Guard,
+    _p: PhantomData<&'a ()>,
+}
+
+impl<T> RevocableGuard<'_, T> {
+    fn new(data_ref: *const T, rcu_guard: rcu::Guard) -> Self {
+        Self {
+            data_ref,
+            _rcu_guard: rcu_guard,
+            _p: PhantomData,
+        }
+    }
+}
+
+impl<T> Deref for RevocableGuard<'_, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: By the type invariants, we hold the rcu read-side lock, so the object is
+        // guaranteed to remain valid.
+        unsafe { &*self.data_ref }
+    }
+}
--

Quoted 3 lines

2.47.1

Benoît du Garreau

I/O memory is typically either mapped through direct calls to ioremap() or subsystem / bus specific ones such as pci_iomap().

Even though subsystem / bus specific functions to map I/O memory are based on ioremap() / iounmap() it is not desirable to re-implement them in Rust.

Instead, implement a base type for I/O mapped memory, which generically provides the corresponding accessors, such as Io::readb or Io:try_readb.

Io supports an optional const generic, such that a driver can indicate the minimal expected and required size of the mapping at compile time. Correspondingly, calls to the 'non-try' accessors, support compile time checks of the I/O memory offset to read / write, while the 'try' accessors, provide boundary checks on runtime.

IoRaw is meant to be embedded into a structure (e.g. pci::Bar or io::IoMem) which creates the actual I/O memory mapping and initializes IoRaw accordingly.

To ensure that I/O mapped memory can't out-live the device it may be bound to, subsystems must embed the corresponding I/O memory type (e.g. pci::Bar) into a Devres container, such that it gets revoked once the device is unbound.

Reviewed-by Alice Ryhl <[email protected]>

Tested-by Daniel Almeida <[email protected]>

Reviewed-by Daniel Almeida <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers/helpers.c |   1 +
 rust/helpers/io.c      | 101 ++++++++++++++++
 rust/kernel/io.rs      | 260 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs     |   1 +
 4 files changed, 363 insertions(+)
 create mode 100644 rust/helpers/io.c
 create mode 100644 rust/kernel/io.rs

rust/helpers/helpers.c +1 -0

index 060750af6524..63f9b1da179f 100644
@@ -14,6 +14,7 @@
 #include "cred.c"
 #include "err.c"
 #include "fs.c"
+#include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
 #include "mutex.c"

rust/helpers/io.c new file +2 -0

new file mode 100644
index 000000000000..1dde6374c0e2
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+

rust/helpers/io.c new file +89 -0

+}
+
+u8 rust_helper_readb(const volatile void __iomem *addr)
+{
+	return readb(addr);
+}
+
+u16 rust_helper_readw(const volatile void __iomem *addr)
+{
+	return readw(addr);
+}
+
+u32 rust_helper_readl(const volatile void __iomem *addr)
+{
+	return readl(addr);
+}
+
+#ifdef CONFIG_64BIT
+u64 rust_helper_readq(const volatile void __iomem *addr)
+{
+	return readq(addr);
+}
+#endif
+
+void rust_helper_writeb(u8 value, volatile void __iomem *addr)
+{
+	writeb(value, addr);
+}
+
+void rust_helper_writew(u16 value, volatile void __iomem *addr)
+{
+	writew(value, addr);
+}
+
+void rust_helper_writel(u32 value, volatile void __iomem *addr)
+{
+	writel(value, addr);
+}
+
+#ifdef CONFIG_64BIT
+void rust_helper_writeq(u64 value, volatile void __iomem *addr)
+{
+	writeq(value, addr);
+}
+#endif
+
+u8 rust_helper_readb_relaxed(const volatile void __iomem *addr)
+{
+	return readb_relaxed(addr);
+}
+
+u16 rust_helper_readw_relaxed(const volatile void __iomem *addr)
+{
+	return readw_relaxed(addr);
+}
+
+u32 rust_helper_readl_relaxed(const volatile void __iomem *addr)
+{
+	return readl_relaxed(addr);
+}
+
+#ifdef CONFIG_64BIT
+u64 rust_helper_readq_relaxed(const volatile void __iomem *addr)
+{
+	return readq_relaxed(addr);
+}
+#endif
+
+void rust_helper_writeb_relaxed(u8 value, volatile void __iomem *addr)
+{
+	writeb_relaxed(value, addr);
+}
+
+void rust_helper_writew_relaxed(u16 value, volatile void __iomem *addr)
+{
+	writew_relaxed(value, addr);
+}
+
+void rust_helper_writel_relaxed(u32 value, volatile void __iomem *addr)
+{
+	writel_relaxed(value, addr);
+}
+
+#ifdef CONFIG_64BIT
+void rust_helper_writeq_relaxed(u64 value, volatile void __iomem *addr)
+{
+	writeq_relaxed(value, addr);
+}
+#endif

rust/kernel/io.rs new file +260 -0

new file mode 100644
index 000000000000..7ec3341bb411
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Memory-mapped IO.
+//!
+//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
+
+use crate::error::{code::EINVAL, Result};
+use crate::{bindings, build_assert};
+
+/// Raw representation of an MMIO region.
+///
+/// By itself, the existence of an instance of this structure does not provide any guarantees that
+/// the represented MMIO region does exist or is properly mapped.
+///
+/// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
+/// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
+/// any guarantees are given.
+pub struct IoRaw<const SIZE: usize = 0> {
+    addr: usize,
+    maxsize: usize,
+}
+
+impl<const SIZE: usize> IoRaw<SIZE> {
+    /// Returns a new `IoRaw` instance on success, an error otherwise.
+    pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
+        if maxsize < SIZE {
+            return Err(EINVAL);
+        }
+
+        Ok(Self { addr, maxsize })
+    }
+
+    /// Returns the base address of the MMIO region.
+    #[inline]
+    pub fn addr(&self) -> usize {
+        self.addr
+    }
+
+    /// Returns the maximum size of the MMIO region.
+    #[inline]
+    pub fn maxsize(&self) -> usize {
+        self.maxsize
+    }
+}
+
+/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
+///
+/// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
+/// mapping, performing an additional region request etc.
+///
+/// # Invariant
+///
+/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
+/// `maxsize`.
+///
+/// # Examples
+///
+/// ```no_run
+/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+///
+/// impl<const SIZE: usize> IoMem<SIZE> {
+///     /// # Safety
+///     ///
+///     /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
+///     /// virtual address space.
+///     unsafe fn new(paddr: usize) -> Result<Self>{
+///         // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
+///         // valid for `ioremap`.
+///         let addr = unsafe { bindings::ioremap(paddr as _, SIZE.try_into().unwrap()) };
+///         if addr.is_null() {
+///             return Err(ENOMEM);
+///         }
+///
+///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
+///         unsafe { bindings::iounmap(self.0.addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
+///         unsafe { Io::from_raw(&self.0) }
+///    }
+/// }
+///
+///# fn no_run() -> Result<(), Error> {
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// iomem.writel(0x42, 0x0);
+/// assert!(iomem.try_writel(0x42, 0x0).is_ok());
+/// assert!(iomem.try_writel(0x42, 0x4).is_err());
+/// # Ok(())
+/// # }
+/// ```
+#[repr(transparent)]
+pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
+
+macro_rules! define_read {
+    ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => {
+        /// Read IO data from a given offset known at compile time.
+        ///
+        /// Bound checks are performed on compile time, hence if the offset is not known at compile
+        /// time, the build will fail.
+        $(#[$attr])*
+        #[inline]
+        pub fn $name(&self, offset: usize) -> $type_name {
+            let addr = self.io_addr_assert::<$type_name>(offset);
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(addr as _) }
+        }
+
+        /// Read IO data from a given offset.
+        ///
+        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
+        /// out of bounds.
+        $(#[$attr])*
+        pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
+            let addr = self.io_addr::<$type_name>(offset)?;
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            Ok(unsafe { bindings::$name(addr as _) })
+        }
+    };
+}
+
+macro_rules! define_write {
+    ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => {
+        /// Write IO data from a given offset known at compile time.
+        ///
+        /// Bound checks are performed on compile time, hence if the offset is not known at compile
+        /// time, the build will fail.
+        $(#[$attr])*
+        #[inline]
+        pub fn $name(&self, value: $type_name, offset: usize) {
+            let addr = self.io_addr_assert::<$type_name>(offset);
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(value, addr as _, ) }
+        }
+
+        /// Write IO data from a given offset.
+        ///
+        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
+        /// out of bounds.
+        $(#[$attr])*
+        pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
+            let addr = self.io_addr::<$type_name>(offset)?;
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(value, addr as _) }
+            Ok(())
+        }
+    };
+}
+
+impl<const SIZE: usize> Io<SIZE> {
+    /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
+    /// `maxsize`.
+    pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self {
+        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
+        unsafe { &*core::ptr::from_ref(raw).cast() }
+    }
+
+    /// Returns the base address of this mapping.
+    #[inline]
+    pub fn addr(&self) -> usize {
+        self.0.addr()
+    }
+
+    /// Returns the maximum size of this mapping.
+    #[inline]
+    pub fn maxsize(&self) -> usize {
+        self.0.maxsize()
+    }
+
+    #[inline]
+    const fn offset_valid<U>(offset: usize, size: usize) -> bool {
+        let type_size = core::mem::size_of::<U>();
+        if let Some(end) = offset.checked_add(type_size) {
+            end <= size && offset % type_size == 0
+        } else {
+            false
+        }
+    }
+
+    #[inline]
+    fn io_addr<U>(&self, offset: usize) -> Result<usize> {
+        if !Self::offset_valid::<U>(offset, self.maxsize()) {
+            return Err(EINVAL);
+        }
+
+        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
+        // this can't overflow.
+        self.addr().checked_add(offset).ok_or(EINVAL)
+    }
+
+    #[inline]
+    fn io_addr_assert<U>(&self, offset: usize) -> usize {
+        build_assert!(Self::offset_valid::<U>(offset, SIZE));
+
+        self.addr() + offset
+    }
+
+    define_read!(readb, try_readb, u8);
+    define_read!(readw, try_readw, u16);
+    define_read!(readl, try_readl, u32);
+    define_read!(
+        #[cfg(CONFIG_64BIT)]
+        readq,
+        try_readq,
+        u64
+    );
+
+    define_read!(readb_relaxed, try_readb_relaxed, u8);
+    define_read!(readw_relaxed, try_readw_relaxed, u16);
+    define_read!(readl_relaxed, try_readl_relaxed, u32);
+    define_read!(
+        #[cfg(CONFIG_64BIT)]
+        readq_relaxed,
+        try_readq_relaxed,
+        u64
+    );
+
+    define_write!(writeb, try_writeb, u8);
+    define_write!(writew, try_writew, u16);
+    define_write!(writel, try_writel, u32);
+    define_write!(
+        #[cfg(CONFIG_64BIT)]
+        writeq,
+        try_writeq,
+        u64
+    );
+
+    define_write!(writeb_relaxed, try_writeb_relaxed, u8);
+    define_write!(writew_relaxed, try_writew_relaxed, u16);
+    define_write!(writel_relaxed, try_writel_relaxed, u32);
+    define_write!(
+        #[cfg(CONFIG_64BIT)]
+        writeq_relaxed,
+        try_writeq_relaxed,
+        u64
+    );
+}

rust/kernel/lib.rs +1 -0

index 5702ce32ec8e..6c836ab73771 100644
@@ -79,6 +79,7 @@
 
 #[doc(hidden)]
 pub use bindings;
+pub mod io;
 pub use macros;
 pub use uapi;
 

Signature · 1 line

2.47.1

Quoted 59 lines · Danilo Krummrich

I/O memory is typically either mapped through direct calls to ioremap() or subsystem / bus specific ones such as pci_iomap().

Even though subsystem / bus specific functions to map I/O memory are based on ioremap() / iounmap() it is not desirable to re-implement them in Rust.

Instead, implement a base type for I/O mapped memory, which generically provides the corresponding accessors, such as Io::readb or Io:try_readb.

Io supports an optional const generic, such that a driver can indicate the minimal expected and required size of the mapping at compile time. Correspondingly, calls to the 'non-try' accessors, support compile time checks of the I/O memory offset to read / write, while the 'try' accessors, provide boundary checks on runtime.

IoRaw is meant to be embedded into a structure (e.g. pci::Bar or io::IoMem) which creates the actual I/O memory mapping and initializes IoRaw accordingly.

To ensure that I/O mapped memory can't out-live the device it may be bound to, subsystems must embed the corresponding I/O memory type (e.g. pci::Bar) into a Devres container, such that it gets revoked once the device is unbound.

Reviewed-by Alice Ryhl <[email protected]>

Tested-by Daniel Almeida <[email protected]>

Reviewed-by Daniel Almeida <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers/helpers.c |   1 +
 rust/helpers/io.c      | 101 ++++++++++++++++
 rust/kernel/io.rs      | 260 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs     |   1 +
 4 files changed, 363 insertions(+)
 create mode 100644 rust/helpers/io.c
 create mode 100644 rust/kernel/io.rs

rust/helpers/helpers.c +1 -0

index 060750af6524..63f9b1da179f 100644
@@ -14,6 +14,7 @@
 #include "cred.c"
 #include "err.c"
 #include "fs.c"
+#include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
 #include "mutex.c"

rust/helpers/io.c new file +2 -0

new file mode 100644
index 000000000000..1dde6374c0e2
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+

rust/helpers/io.c line 12

Quoted 10 lines

rust/helpers/io.c new file +10 -0

+#include <linux/io.h>
+
+void __iomem *rust_helper_ioremap(phys_addr_t offset, size_t size)
+{
+	return ioremap(offset, size);
+}
+
+void rust_helper_iounmap(volatile void __iomem *addr)
+{
+	return iounmap(addr);

Copy-paste mistake, obviously this return statement shouldn't be here.

Quoted 160 lines

rust/helpers/io.c new file +89 -0

+}
+
+u8 rust_helper_readb(const volatile void __iomem *addr)
+{
+	return readb(addr);
+}
+
+u16 rust_helper_readw(const volatile void __iomem *addr)
+{
+	return readw(addr);
+}
+
+u32 rust_helper_readl(const volatile void __iomem *addr)
+{
+	return readl(addr);
+}
+
+#ifdef CONFIG_64BIT
+u64 rust_helper_readq(const volatile void __iomem *addr)
+{
+	return readq(addr);
+}
+#endif
+
+void rust_helper_writeb(u8 value, volatile void __iomem *addr)
+{
+	writeb(value, addr);
+}
+
+void rust_helper_writew(u16 value, volatile void __iomem *addr)
+{
+	writew(value, addr);
+}
+
+void rust_helper_writel(u32 value, volatile void __iomem *addr)
+{
+	writel(value, addr);
+}
+
+#ifdef CONFIG_64BIT
+void rust_helper_writeq(u64 value, volatile void __iomem *addr)
+{
+	writeq(value, addr);
+}
+#endif
+
+u8 rust_helper_readb_relaxed(const volatile void __iomem *addr)
+{
+	return readb_relaxed(addr);
+}
+
+u16 rust_helper_readw_relaxed(const volatile void __iomem *addr)
+{
+	return readw_relaxed(addr);
+}
+
+u32 rust_helper_readl_relaxed(const volatile void __iomem *addr)
+{
+	return readl_relaxed(addr);
+}
+
+#ifdef CONFIG_64BIT
+u64 rust_helper_readq_relaxed(const volatile void __iomem *addr)
+{
+	return readq_relaxed(addr);
+}
+#endif
+
+void rust_helper_writeb_relaxed(u8 value, volatile void __iomem *addr)
+{
+	writeb_relaxed(value, addr);
+}
+
+void rust_helper_writew_relaxed(u16 value, volatile void __iomem *addr)
+{
+	writew_relaxed(value, addr);
+}
+
+void rust_helper_writel_relaxed(u32 value, volatile void __iomem *addr)
+{
+	writel_relaxed(value, addr);
+}
+
+#ifdef CONFIG_64BIT
+void rust_helper_writeq_relaxed(u64 value, volatile void __iomem *addr)
+{
+	writeq_relaxed(value, addr);
+}
+#endif

rust/kernel/io.rs new file +65 -0

new file mode 100644
index 000000000000..7ec3341bb411
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Memory-mapped IO.
+//!
+//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
+
+use crate::error::{code::EINVAL, Result};
+use crate::{bindings, build_assert};
+
+/// Raw representation of an MMIO region.
+///
+/// By itself, the existence of an instance of this structure does not provide any guarantees that
+/// the represented MMIO region does exist or is properly mapped.
+///
+/// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
+/// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
+/// any guarantees are given.
+pub struct IoRaw<const SIZE: usize = 0> {
+    addr: usize,
+    maxsize: usize,
+}
+
+impl<const SIZE: usize> IoRaw<SIZE> {
+    /// Returns a new `IoRaw` instance on success, an error otherwise.
+    pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
+        if maxsize < SIZE {
+            return Err(EINVAL);
+        }
+
+        Ok(Self { addr, maxsize })
+    }
+
+    /// Returns the base address of the MMIO region.
+    #[inline]
+    pub fn addr(&self) -> usize {
+        self.addr
+    }
+
+    /// Returns the maximum size of the MMIO region.
+    #[inline]
+    pub fn maxsize(&self) -> usize {
+        self.maxsize
+    }
+}
+
+/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
+///
+/// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
+/// mapping, performing an additional region request etc.
+///
+/// # Invariant
+///
+/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
+/// `maxsize`.
+///
+/// # Examples
+///
+/// ```no_run
+/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+///
+/// impl<const SIZE: usize> IoMem<SIZE> {

rust/kernel/io.rs line 73

Quoted 8 lines

rust/kernel/io.rs new file +8 -0

+///     /// # Safety
+///     ///
+///     /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
+///     /// virtual address space.
+///     unsafe fn new(paddr: usize) -> Result<Self>{
+///         // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
+///         // valid for `ioremap`.
+///         let addr = unsafe { bindings::ioremap(paddr as _, SIZE.try_into().unwrap()) };

Quoted 199 lines

rust/kernel/io.rs new file +187 -0

+///         if addr.is_null() {
+///             return Err(ENOMEM);
+///         }
+///
+///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
+///         unsafe { bindings::iounmap(self.0.addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
+///         unsafe { Io::from_raw(&self.0) }
+///    }
+/// }
+///
+///# fn no_run() -> Result<(), Error> {
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// iomem.writel(0x42, 0x0);
+/// assert!(iomem.try_writel(0x42, 0x0).is_ok());
+/// assert!(iomem.try_writel(0x42, 0x4).is_err());
+/// # Ok(())
+/// # }
+/// ```
+#[repr(transparent)]
+pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
+
+macro_rules! define_read {
+    ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => {
+        /// Read IO data from a given offset known at compile time.
+        ///
+        /// Bound checks are performed on compile time, hence if the offset is not known at compile
+        /// time, the build will fail.
+        $(#[$attr])*
+        #[inline]
+        pub fn $name(&self, offset: usize) -> $type_name {
+            let addr = self.io_addr_assert::<$type_name>(offset);
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(addr as _) }
+        }
+
+        /// Read IO data from a given offset.
+        ///
+        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
+        /// out of bounds.
+        $(#[$attr])*
+        pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
+            let addr = self.io_addr::<$type_name>(offset)?;
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            Ok(unsafe { bindings::$name(addr as _) })
+        }
+    };
+}
+
+macro_rules! define_write {
+    ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => {
+        /// Write IO data from a given offset known at compile time.
+        ///
+        /// Bound checks are performed on compile time, hence if the offset is not known at compile
+        /// time, the build will fail.
+        $(#[$attr])*
+        #[inline]
+        pub fn $name(&self, value: $type_name, offset: usize) {
+            let addr = self.io_addr_assert::<$type_name>(offset);
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(value, addr as _, ) }
+        }
+
+        /// Write IO data from a given offset.
+        ///
+        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
+        /// out of bounds.
+        $(#[$attr])*
+        pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
+            let addr = self.io_addr::<$type_name>(offset)?;
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(value, addr as _) }
+            Ok(())
+        }
+    };
+}
+
+impl<const SIZE: usize> Io<SIZE> {
+    /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
+    /// `maxsize`.
+    pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self {
+        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
+        unsafe { &*core::ptr::from_ref(raw).cast() }
+    }
+
+    /// Returns the base address of this mapping.
+    #[inline]
+    pub fn addr(&self) -> usize {
+        self.0.addr()
+    }
+
+    /// Returns the maximum size of this mapping.
+    #[inline]
+    pub fn maxsize(&self) -> usize {
+        self.0.maxsize()
+    }
+
+    #[inline]
+    const fn offset_valid<U>(offset: usize, size: usize) -> bool {
+        let type_size = core::mem::size_of::<U>();
+        if let Some(end) = offset.checked_add(type_size) {
+            end <= size && offset % type_size == 0
+        } else {
+            false
+        }
+    }
+
+    #[inline]
+    fn io_addr<U>(&self, offset: usize) -> Result<usize> {
+        if !Self::offset_valid::<U>(offset, self.maxsize()) {
+            return Err(EINVAL);
+        }
+
+        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
+        // this can't overflow.
+        self.addr().checked_add(offset).ok_or(EINVAL)
+    }
+
+    #[inline]
+    fn io_addr_assert<U>(&self, offset: usize) -> usize {
+        build_assert!(Self::offset_valid::<U>(offset, SIZE));
+
+        self.addr() + offset
+    }
+
+    define_read!(readb, try_readb, u8);
+    define_read!(readw, try_readw, u16);
+    define_read!(readl, try_readl, u32);
+    define_read!(
+        #[cfg(CONFIG_64BIT)]
+        readq,
+        try_readq,
+        u64
+    );
+
+    define_read!(readb_relaxed, try_readb_relaxed, u8);
+    define_read!(readw_relaxed, try_readw_relaxed, u16);
+    define_read!(readl_relaxed, try_readl_relaxed, u32);
+    define_read!(
+        #[cfg(CONFIG_64BIT)]
+        readq_relaxed,
+        try_readq_relaxed,
+        u64
+    );
+
+    define_write!(writeb, try_writeb, u8);
+    define_write!(writew, try_writew, u16);
+    define_write!(writel, try_writel, u32);
+    define_write!(
+        #[cfg(CONFIG_64BIT)]
+        writeq,
+        try_writeq,
+        u64
+    );
+
+    define_write!(writeb_relaxed, try_writeb_relaxed, u8);
+    define_write!(writew_relaxed, try_writew_relaxed, u16);
+    define_write!(writel_relaxed, try_writel_relaxed, u32);
+    define_write!(
+        #[cfg(CONFIG_64BIT)]
+        writeq_relaxed,
+        try_writeq_relaxed,
+        u64
+    );
+}

rust/kernel/lib.rs +1 -0

index 5702ce32ec8e..6c836ab73771 100644
@@ -79,6 +79,7 @@
 
 #[doc(hidden)]
 pub use bindings;
+pub mod io;
 pub use macros;
 pub use uapi;
 

Quoted 3 lines

Signature · 1 line

2.47.1

Quoted 21 lines · Danilo Krummrich

Quoted 20 lines · Danilo Krummrich

rust/kernel/io.rs new file +20 -0

+/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
+///
+/// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
+/// mapping, performing an additional region request etc.
+///
+/// # Invariant
+///
+/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
+/// `maxsize`.
+///
+/// # Examples
+///
+/// ```no_run
+/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+///
+/// impl<const SIZE: usize> IoMem<SIZE> {

Quoted 8 lines

rust/kernel/io.rs line 73

Quoted 8 lines

rust/kernel/io.rs new file +8 -0

+///     /// # Safety
+///     ///
+///     /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
+///     /// virtual address space.
+///     unsafe fn new(paddr: usize) -> Result<Self>{
+///         // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
+///         // valid for `ioremap`.
+///         let addr = unsafe { bindings::ioremap(paddr as _, SIZE.try_into().unwrap()) };

This is a problematic API. ioremap() does not work on some platforms like Apple Silicon. Instead, you have to use ioremap_np() for most devices.

Please add a bindings::resource abstraction and use that to construct IoMem. Then, you can check the flags for bindings::IORESOURCE_MEM_NONPOSTED and use the appropriate function, like this:

https://github.com/AsahiLinux/linux/blob/fce34c83f1dca5b10cc2c866fd8832a362de7974/rust/kernel/io_mem.rs#L152

Quoted 35 lines

Quoted 35 lines

rust/kernel/io.rs new file +35 -0

+///         if addr.is_null() {
+///             return Err(ENOMEM);
+///         }
+///
+///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
+///         unsafe { bindings::iounmap(self.0.addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
+///         unsafe { Io::from_raw(&self.0) }
+///    }
+/// }
+///
+///# fn no_run() -> Result<(), Error> {
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// iomem.writel(0x42, 0x0);
+/// assert!(iomem.try_writel(0x42, 0x0).is_ok());
+/// assert!(iomem.try_writel(0x42, 0x4).is_err());
+/// # Ok(())
+/// # }
+/// ```
+#[repr(transparent)]
+pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);

Hi Lina,

Quoted 36 lines · Asahi Lina

Quoted 21 lines · Danilo Krummrich

Quoted 20 lines · Danilo Krummrich

rust/kernel/io.rs new file +20 -0

+/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
+///
+/// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
+/// mapping, performing an additional region request etc.
+///
+/// # Invariant
+///
+/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
+/// `maxsize`.
+///
+/// # Examples
+///
+/// ```no_run
+/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+///
+/// impl<const SIZE: usize> IoMem<SIZE> {

Quoted 8 lines

rust/kernel/io.rs line 73

Quoted 8 lines

rust/kernel/io.rs new file +8 -0

+///     /// # Safety
+///     ///
+///     /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
+///     /// virtual address space.
+///     unsafe fn new(paddr: usize) -> Result<Self>{
+///         // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
+///         // valid for `ioremap`.
+///         let addr = unsafe { bindings::ioremap(paddr as _, SIZE.try_into().unwrap()) };

This is a problematic API. ioremap() does not work on some platforms like Apple Silicon. Instead, you have to use ioremap_np() for most devices.

Quoted 4 lines

Please add a bindings::resource abstraction and use that to construct IoMem. Then, you can check the flags for bindings::IORESOURCE_MEM_NONPOSTED and use the appropriate function, like this:

This is just a very simplified example of how to use IoRaw and Io base types in the scope of an example section within a doc-comment.

There is an actual IoMem implementation including a struct resource abstraction [1] from Daniel though. Maybe you want to have a look at this instead.

[1] https://lore.kernel.org/rust-for-linux/[email protected]/

Quoted 31 lines

https://github.com/AsahiLinux/linux/blob/fce34c83f1dca5b10cc2c866fd8832a362de7974/rust/kernel/io_mem.rs#L152

Quoted 27 lines

Quoted 27 lines

rust/kernel/io.rs new file +27 -0

+///         if addr.is_null() {
+///             return Err(ENOMEM);
+///         }
+///
+///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
+///         unsafe { bindings::iounmap(self.0.addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
+///         unsafe { Io::from_raw(&self.0) }
+///    }
+/// }
+///
+///# fn no_run() -> Result<(), Error> {
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };

Quoted 8 lines

Quoted 8 lines

rust/kernel/io.rs line 108

Quoted 8 lines

rust/kernel/io.rs new file +8 -0

+/// iomem.writel(0x42, 0x0);
+/// assert!(iomem.try_writel(0x42, 0x0).is_ok());
+/// assert!(iomem.try_writel(0x42, 0x4).is_err());
+/// # Ok(())
+/// # }
+/// ```
+#[repr(transparent)]
+pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);

rust/kernel/io.rs new file +6 -0

+/// ```no_run
+/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);

rust/kernel/io.rs new file +33 -0

+///         if addr.is_null() {
+///             return Err(ENOMEM);
+///         }
+///
+///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
+///         unsafe { bindings::iounmap(self.0.addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
+///         unsafe { Io::from_raw(&self.0) }
+///    }
+/// }
+///
+///# fn no_run() -> Result<(), Error> {
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// iomem.writel(0x42, 0x0);
+/// assert!(iomem.try_writel(0x42, 0x0).is_ok());
+/// assert!(iomem.try_writel(0x42, 0x4).is_err());
+/// # Ok(())
+/// # }
+/// ```

Hi Danilo,

Quoted 6 lines

rust/kernel/io.rs new file +6 -0

+/// ```no_run
+/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);

rust/kernel/io.rs line 73

Quoted 10 lines

rust/kernel/io.rs new file +10 -0

+///
+/// impl<const SIZE: usize> IoMem<SIZE> {
+///     /// # Safety
+///     ///
+///     /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
+///     /// virtual address space.
+///     unsafe fn new(paddr: usize) -> Result<Self>{
+///         // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
+///         // valid for `ioremap`.
+///         let addr = unsafe { bindings::ioremap(paddr as _, SIZE.try_into().unwrap()) };

This line generates a warning when building the doctests on arm64:

warning: useless conversion to the same type: usize

    --> rust/doctests_kernel_generated.rs:3601:59
     |

3601 | let addr = unsafe { bindings::ioremap(paddr as _,
SIZE.try_into().unwrap()) };
| ^^^^^^^^^^^^^^^
|
= help: consider removing .try_into()
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion

Same things happens as well in devres.rs

Quoted 34 lines

rust/kernel/io.rs new file +33 -0

+///         if addr.is_null() {
+///             return Err(ENOMEM);
+///         }
+///
+///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
+///         unsafe { bindings::iounmap(self.0.addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
+///         unsafe { Io::from_raw(&self.0) }
+///    }
+/// }
+///
+///# fn no_run() -> Result<(), Error> {
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// iomem.writel(0x42, 0x0);
+/// assert!(iomem.try_writel(0x42, 0x0).is_ok());
+/// assert!(iomem.try_writel(0x42, 0x4).is_err());
+/// # Ok(())
+/// # }
+/// ```

rust/kernel/pci.rs new file +9 -0

+impl DeviceId {
+    const PCI_ANY_ID: u32 = !0;
+
+    /// PCI_DEVICE macro.
+    pub const fn new(vendor: u32, device: u32) -> Self {
+        Self(bindings::pci_device_id {
+            vendor,
+            device,
+            subvendor: DeviceId::PCI_ANY_ID,

Quoted 15 lines · Danilo Krummrich

rust/kernel/pci.rs new file +15 -0

+impl DeviceId {
+    const PCI_ANY_ID: u32 = !0;
+
+    /// PCI_DEVICE macro.
+    pub const fn new(vendor: u32, device: u32) -> Self {
+        Self(bindings::pci_device_id {
+            vendor,
+            device,
+            subvendor: DeviceId::PCI_ANY_ID,
+            subdevice: DeviceId::PCI_ANY_ID,
+            class: 0,
+            class_mask: 0,
+            driver_data: 0,
+            override_only: 0,
+        })

rust/kernel/pci.rs line 135

Quoted 4 lines

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

+    }
+
+    /// PCI_DEVICE_CLASS macro.
+    pub const fn with_class(class: u32, class_mask: u32) -> Self {

I know naming is hard, and I'm not going to object to this at all, but using "new()" and "with_class()" feels a bit odd and mis-matched. How about spelling it out, pci_device(), and pci_device_class()?

Anyway, not a bit deal at all, let's see how this plays out with real drivers and we can always change it later.

rust/kernel/pci.rs line 150

Quoted 2 lines

rust/kernel/pci.rs new file +2 -0

+// Allow drivers R/O access to the fields of `pci_device_id`; should we prefer accessor functions
+// to void exposing C structure fields?

Minor nit, do you mean "to avoid exposing..."?

Other than these, this looks good! If I can get an ack from the PCI maintainer, I'll be glad to queue these up in my driver core tree...

thanks,

greg k-h

In order to not duplicate code in bus specific implementations (e.g. platform), implement a generic driver::Adapter to represent the connection of matched drivers and devices.

Bus specific Adapter implementations can simply implement this trait to inherit generic functionality, such as matching OF or ACPI device IDs and ID table entries.

Suggested-by Rob Herring (Arm) <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---

RUSTC L rust/kernel.o
warning: unused import: device_id
--> /home/aeh/src/linux-rust/rnvme-v6.13-rc3/rust/kernel/driver.rs:10:13

   |
10 |     device, device_id, init::PinInit, of, str::CStr, try_pin_init, types::Opaque, ThisModule,
   |             ^^^^^^^^^
   |

= note: #[warn(unused_imports)] on by default

warning: missing documentation for an associated function
--> /home/aeh/src/linux-rust/rnvme-v6.13-rc3/rust/kernel/driver.rs:158:5

    |
158 |     fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |

= note: requested on the command line with -W missing-docs

warning: 2 warnings emitted

Hi Danilo,

Quoted 8 lines · Danilo Krummrich

In order to not duplicate code in bus specific implementations (e.g. platform), implement a generic driver::Adapter to represent the connection of matched drivers and devices.

Bus specific Adapter implementations can simply implement this trait to inherit generic functionality, such as matching OF or ACPI device IDs and ID table entries.

Quoted 3 lines

Suggested-by Rob Herring (Arm) <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---

I get some warnings when applying this patch:

Quoted 12 lines

RUSTC L rust/kernel.o
warning: unused import: device_id
--> /home/aeh/src/linux-rust/rnvme-v6.13-rc3/rust/kernel/driver.rs:10:13

   |
10 |     device, device_id, init::PinInit, of, str::CStr, try_pin_init, types::Opaque, ThisModule,
   |             ^^^^^^^^^
   |

= note: #[warn(unused_imports)] on by default

warning: missing documentation for an associated function
--> /home/aeh/src/linux-rust/rnvme-v6.13-rc3/rust/kernel/driver.rs:158:5

Quoted 7 lines

    |
158 |     fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |

= note: requested on the command line with -W missing-docs

warning: 2 warnings emitted

Looks like the latter one is from patch 13.

Best regards,
Andreas Hindborg

Add a sample Rust platform driver illustrating the usage of the platform bus abstractions.

This driver probes through either a match of device / driver name or a match within the OF ID table.

Reviewed-by Rob Herring (Arm) <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                                  |  1 +
 drivers/of/unittest-data/tests-platform.dtsi |  5 ++
 samples/rust/Kconfig                         | 10 ++++
 samples/rust/Makefile                        |  1 +
 samples/rust/rust_driver_platform.rs         | 49 ++++++++++++++++++++
 5 files changed, 66 insertions(+)
 create mode 100644 samples/rust/rust_driver_platform.rs

MAINTAINERS +1 -0

index fec876068c40..95bd7dc88ad8 100644
@@ -7037,6 +7037,7 @@ F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
 F:	rust/kernel/platform.rs
+F:	samples/rust/rust_driver_platform.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
 M:	Nishanth Menon <[email protected]>

drivers/of/unittest-data/tests-platform.dtsi +5 -0

index fa39611071b3..2caaf1c10ee6 100644
@@ -33,6 +33,11 @@ dev@100 {
 					reg = <0x100>;
 				};
 			};
+
+			test-device@2 {
+				compatible = "test,rust-device";
+				reg = <0x2>;
+			};
 		};
 	};
 };

samples/rust/Kconfig +10 -0

index 6d468193cdd8..70126b750426 100644
@@ -41,6 +41,16 @@ config SAMPLE_RUST_DRIVER_PCI
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRIVER_PLATFORM
+	tristate "Platform Driver"
+	help
+	  This option builds the Rust Platform driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_driver_platform.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_HOSTPROGS
 	bool "Host programs"
 	help

samples/rust/Makefile +1 -0

index 2f5b6bdb2fa5..761d13fff018 100644
@@ -4,6 +4,7 @@ ccflags-y += -I$(src)				# needed for trace events
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
+obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
 
 rust_print-y := rust_print_main.o rust_print_events.o
 

samples/rust/rust_driver_platform.rs new file +0 -0

new file mode 100644
index 000000000000..8120609e2940

samples/rust/rust_driver_platform.rs new file +35 -0

+use kernel::{c_str, of, platform, prelude::*};
+
+struct SampleDriver {
+    pdev: platform::Device,
+}
+
+struct Info(u32);
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <SampleDriver as platform::Driver>::IdInfo,
+    [(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
+);
+
+impl platform::Driver for SampleDriver {
+    type IdInfo = Info;
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    fn probe(pdev: &mut platform::Device, info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
+        dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n");
+
+        if let Some(info) = info {
+            dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
+        }
+
+        let drvdata = KBox::new(Self { pdev: pdev.clone() }, GFP_KERNEL)?;
+
+        Ok(drvdata.into())
+    }
+}
+
+impl Drop for SampleDriver {
+    fn drop(&mut self) {
+        dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");

Quoted 84 lines · Danilo Krummrich

Add a sample Rust platform driver illustrating the usage of the platform bus abstractions.

This driver probes through either a match of device / driver name or a match within the OF ID table.

Reviewed-by Rob Herring (Arm) <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                                  |  1 +
 drivers/of/unittest-data/tests-platform.dtsi |  5 ++
 samples/rust/Kconfig                         | 10 ++++
 samples/rust/Makefile                        |  1 +
 samples/rust/rust_driver_platform.rs         | 49 ++++++++++++++++++++
 5 files changed, 66 insertions(+)
 create mode 100644 samples/rust/rust_driver_platform.rs

MAINTAINERS +1 -0

index fec876068c40..95bd7dc88ad8 100644
@@ -7037,6 +7037,7 @@ F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
 F:	rust/kernel/platform.rs
+F:	samples/rust/rust_driver_platform.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
 M:	Nishanth Menon <[email protected]>

drivers/of/unittest-data/tests-platform.dtsi +5 -0

index fa39611071b3..2caaf1c10ee6 100644
@@ -33,6 +33,11 @@ dev@100 {
 					reg = <0x100>;
 				};
 			};
+
+			test-device@2 {
+				compatible = "test,rust-device";
+				reg = <0x2>;
+			};
 		};
 	};
 };

samples/rust/Kconfig +10 -0

index 6d468193cdd8..70126b750426 100644
@@ -41,6 +41,16 @@ config SAMPLE_RUST_DRIVER_PCI
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRIVER_PLATFORM
+	tristate "Platform Driver"
+	help
+	  This option builds the Rust Platform driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_driver_platform.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_HOSTPROGS
 	bool "Host programs"
 	help

samples/rust/Makefile +1 -0

index 2f5b6bdb2fa5..761d13fff018 100644
@@ -4,6 +4,7 @@ ccflags-y += -I$(src)				# needed for trace events
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
+obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
 
 rust_print-y := rust_print_main.o rust_print_events.o
 

samples/rust/rust_driver_platform.rs new file +0 -0

new file mode 100644
index 000000000000..8120609e2940

samples/rust/rust_driver_platform.rs line 3

Quoted 3 lines

samples/rust/rust_driver_platform.rs new file +3 -0

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust Platform driver sample.

Nit, I think your employer will want a copyright line on these, but hey, I could be wrong! Your call...

Quoted 41 lines

samples/rust/rust_driver_platform.rs new file +41 -0

+use kernel::{c_str, of, platform, prelude::*};
+
+struct SampleDriver {
+    pdev: platform::Device,
+}
+
+struct Info(u32);
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <SampleDriver as platform::Driver>::IdInfo,
+    [(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
+);
+
+impl platform::Driver for SampleDriver {
+    type IdInfo = Info;
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    fn probe(pdev: &mut platform::Device, info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
+        dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n");
+
+        if let Some(info) = info {
+            dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
+        }
+
+        let drvdata = KBox::new(Self { pdev: pdev.clone() }, GFP_KERNEL)?;
+
+        Ok(drvdata.into())
+    }
+}
+
+impl Drop for SampleDriver {
+    fn drop(&mut self) {
+        dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");
+    }
+}
+
+kernel::module_platform_driver! {
+    type: SampleDriver,
+    name: "rust_driver_platform",

samples/rust/rust_driver_platform.rs line 49

Quoted 4 lines

samples/rust/rust_driver_platform.rs new file +4 -0

+    author: "Danilo Krummrich",
+    description: "Rust Platform driver",
+    license: "GPL v2",
+}

That is almost too simple. Seriously nice work, let's wait a few days for some others to review the series and I'll be glad to apply it to my tree!

greg k-h

Thanks! If nothing else comes up, I'll send you a v7 end of this week addressing the two minor things I just replied to (remove wrong return statement in iounmap() helper, pci::DeviceId naming and Deref impl).

Quoted 4 lines · Danilo Krummrich

Thanks! If nothing else comes up, I'll send you a v7 end of this week addressing the two minor things I just replied to (remove wrong return statement in iounmap() helper, pci::DeviceId naming and Deref impl).

If you are going to send v7, then could you please take the chance to update core::ffi::c_* to kernel::ffi::c_*? (since we are migrating to our custom mapping -- rust-next completes the migration and enables the new mapping)

I think you have only 3 cases here, and the change should not break anything in your case, i.e. it is just a replacement.

Thanks!

Cheers,
Miguel

This patch series implements the necessary Rust abstractions to implement device drivers in Rust.

This includes some basic generalizations for driver registration, handling of ID tables, MMIO operations and device resource handling.

Those generalizations are used to implement device driver support for two busses, the PCI and platform bus (with OF IDs) in order to provide some evidence that the generalizations work as intended.

The patch series also includes two patches adding two driver samples, one PCI driver and one platform driver.

The PCI bits are motivated by the Nova driver project [1], but are used by at least one more OOT driver (rnvme [2]).

The platform bits, besides adding some more evidence to the base abstractions, are required by a few more OOT drivers aiming at going upstream, i.e. rvkms [3], cpufreq-dt [4], asahi [5] and the i2c work from Fabien [6].

The patches of this series can also be [7], [8] and [9].

Quoted 13 lines · Danilo Krummrich

This patch series implements the necessary Rust abstractions to implement device drivers in Rust.

This includes some basic generalizations for driver registration, handling of ID tables, MMIO operations and device resource handling.

Those generalizations are used to implement device driver support for two busses, the PCI and platform bus (with OF IDs) in order to provide some evidence that the generalizations work as intended.

The patch series also includes two patches adding two driver samples, one PCI driver and one platform driver.

Quoted 8 lines

The PCI bits are motivated by the Nova driver project [1], but are used by at least one more OOT driver (rnvme [2]).

The platform bits, besides adding some more evidence to the base abstractions, are required by a few more OOT drivers aiming at going upstream, i.e. rvkms [3], cpufreq-dt [4], asahi [5] and the i2c work from Fabien [6].

The patches of this series can also be [7], [8] and [9].

With v6.13-rc2 and Fabien's regmap/regulator/I2C on top I gave it a try on x86 with qemu. Additionally cross compiled for arm64 and will try it on real hardware once I have it available. But previous versions of this series have been fine on that, already. No issues observed for running the samples and for the examples/KUnit. So:

Tested-by Dirk Behme <[email protected]>

Many thanks!

Dirk

This patch series implements the necessary Rust abstractions to implement device drivers in Rust.

This includes some basic generalizations for driver registration, handling of ID tables, MMIO operations and device resource handling.

Those generalizations are used to implement device driver support for two busses, the PCI and platform bus (with OF IDs) in order to provide some evidence that the generalizations work as intended.

The patch series also includes two patches adding two driver samples, one PCI driver and one platform driver.

The PCI bits are motivated by the Nova driver project [1], but are used by at least one more OOT driver (rnvme [2]).

The platform bits, besides adding some more evidence to the base abstractions, are required by a few more OOT drivers aiming at going upstream, i.e. rvkms [3], cpufreq-dt [4], asahi [5] and the i2c work from Fabien [6].

The patches of this series can also be [7], [8] and [9].

Changes in v6:
==============
- Module:
- remove patch to pass the module name in Module::init
- add a patch adding the ModuleMetadata trait to retreive the module name

- Driver:
- generate module structure within module_driver!, such that we get a unique module type for which ModuleMetadata can be implemented to retreive the module name
- let Adapter::of_id_table return Option<of::IdTable<Self::IdInfo>>

- Platform:
- change type of platform::Driver::OF_ID_TABLE to Option<of::IdTable>

- RCU:
- add RCU abstraction to MAINTAINERS file

- MISC:
- pick up a couple of RBs

Changes in v5:
==============
- Opaque:
- add Opaque::pin_init

- Revocable:
- use const generic instead of bool in revoke_internal
- don't open code things with Opaque::try_ffi_init, use Opaque::pin_init instead

- Driver:
- fix DriverOps -> RegistrationOps in commit message
- fix signature of register and unregister to take an Opaque<T::RegType> instead of a mutable reference
- implement generic driver::Adapter

- ID table:
- fix module_device_table! macro, according to commit 054a9cd395a7 (modpost: rename alias symbol for MODULE_DEVICE_TABLE())

- PCI:
- remove pci::DeviceId argument from probe()
- provide vendor_id and device_id accessor functions
- use addr_of_mut! for the device in probe()

- OF:
- move OF IdTable type alias from platform.rs to of.rs

- Platform:
- use addr_of_mut! for the device in probe()
- move generic OF code to driver::Adapter

- MISC:
- rebase onto latest rust-next (-rc2)

Changes in v4:
==============
- Revocable
- convert Revocable::data to Opaque
- introduce revoke_nosync, which does not wait for an RCU grace period
- fix minor typo

- Device ID
- fix ID table export name to always be unique
- remove #![allow(stable_features)]
- move #![allow(const_refs_to_cell)] to the "stable in 1.83" list

- I/O
- split Io<SIZE> in IoRaw<SIZE> and Io<SIZE>
- add rust helper for ioremap() and iounmap() (needed by doctests)

- Devres
- optimze drop by using revoke_nosync (we're guaranteed there are no more concurrent users, hence no need to wait for a full grace period)

- OF
- rename "Open Firmware" -> "Device Tree / Open Firmware" (Rob)
- remove unused C header reference
- add TODO comment to loop in DeviceId::new
- remove DeviceId::compatible; was only used in sample code
- add missing #[repr(transparent)]
- use #[cfg(CONFIG_OF)] for the relevant functions

- PCI
- let Rust helper for pci_resource_len() return resource_size_t

- PCI Sample
- remove unnecessary from_le

- Platform
- fix example compatible string
- add example for platform::Driver
- rename ID_TABLE to OF_ID_TABLE
- don't allow public use of of_match_table; convert to of_id_info to retrieve the of::DeviceId to gather the corresponding IdInfo
- remove wrong example reference in the documentation of platform::Driver
- remove as_dev, as we already implement AsRef for platform::Device

- Platform Sample
- fix compatible string; add corresponding entry to drivers/of/unittest-data/tests-platform.dtsi
- remove usage of of_match_table

- MISC
- fix a few spelling mistakes
- rebase onto rust-next (v6.13-rc1)

Changes in v3:
==============
- add commits for Opaque::try_ffi_init and InPlaceModule
- rename DriverOps to RegistrationOps
- rework device ID abstractions to get rid of almost all macro magic (thanks to Gary Guo for working this out!)
- this is possible due to recently stabilized language features
- add modpost alias generation for device ID tables
- unfortunately, this is the part that still requires some macro magic in the device ID abstractions
- PCI
- represent the driver private data with the driver specific Driver instance and bind it's lifetime to the time of the driver being bound to a device
- this allows us to handle class / subsystem registrations in a cleaner way
- get rid of Driver::remove
- Rust drivers should bind cleanup code to the Drop implementation of the corresponding structure instead and put it into their driver structure for automatic cleanup
- add a sample PCI driver
- add abstractions for struct of_device_id
- add abstractions for the platform bus, including a sample driver
- update the MAINTAINERS file accordingly
- currently this turns out a bit messy, but it should become better once the build system supports a treewide distribution of the kernel crate
- I didn't add myself as maintainer, but (if requested) I'm willing to do so and help with maintenance

Changes in v2:
==============
- statically initialize driver structures (Greg)
- move base device ID abstractions to a separate source file (Greg)
- remove DeviceRemoval trait in favor of using a Devres callback to unregister drivers
- remove device::Data, we don't need this abstraction anymore now that we Devres to revoke resources and registrations
- pass the module name to Module::init and InPlaceModule::init in a separate patch
- rework of Io including compile time boundary checks (Miguel, Wedson)
- adjust PCI abstractions accordingly and implement a module_pci_driver! macro
- rework pci::Bar to support a const SIZE
- increase the total amount of Documentation, rephrase some safety comments and commit messages for less ambiguity
- fix compilation issues with some documentation examples

[1] https://gitlab.freedesktop.org/drm/nova/-/tree/nova-next
[2] https://github.com/metaspace/linux/tree/rnvme
[3] https://lore.kernel.org/all/[email protected]/
[4] https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git/log/?h=rust/cpufreq-dt
[5] https://github.com/AsahiLinux/linux
[6] https://github.com/Fabo/linux/tree/fparent/rust-i2c
[7] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device
[8] https://github.com/Rust-for-Linux/linux/tree/staging/rust-pci
[9] https://github.com/Rust-for-Linux/linux/tree/staging/dev

Danilo Krummrich (14):
rust: module: add trait ModuleMetadata
rust: implement generic driver registration
rust: implement IdArray, IdTable and RawDeviceId
rust: types: add Opaque::pin_init
rust: add io::{Io, IoRaw} base types
rust: add devres abstraction
rust: pci: add basic PCI device / driver abstractions
rust: pci: implement I/O mappable pci::Bar
samples: rust: add Rust PCI sample driver
rust: of: add of::DeviceId abstraction
rust: driver: implement Adapter
rust: platform: add basic platform device / driver abstractions
samples: rust: add Rust platform sample driver
MAINTAINERS: add Danilo to DRIVER CORE

Wedson Almeida Filho (2):
rust: add rcu abstraction
rust: add Revocable type

 MAINTAINERS                                  |  10 +
 drivers/of/unittest-data/tests-platform.dtsi |   5 +
 rust/bindings/bindings_helper.h              |   3 +
 rust/helpers/device.c                        |  10 +
 rust/helpers/helpers.c                       |   5 +
 rust/helpers/io.c                            | 101 +++++
 rust/helpers/pci.c                           |  18 +
 rust/helpers/platform.c                      |  13 +
 rust/helpers/rcu.c                           |  13 +
 rust/kernel/device_id.rs                     | 165 +++++++
 rust/kernel/devres.rs                        | 179 ++++++++
 rust/kernel/driver.rs                        | 174 ++++++++
 rust/kernel/io.rs                            | 260 +++++++++++
 rust/kernel/lib.rs                           |  20 +
 rust/kernel/of.rs                            |  60 +++
 rust/kernel/pci.rs                           | 438 +++++++++++++++++++
 rust/kernel/platform.rs                      | 198 +++++++++
 rust/kernel/revocable.rs                     | 223 ++++++++++
 rust/kernel/sync.rs                          |   1 +
 rust/kernel/sync/rcu.rs                      |  47 ++
 rust/kernel/types.rs                         |  11 +
 rust/macros/module.rs                        |   4 +
 samples/rust/Kconfig                         |  21 +
 samples/rust/Makefile                        |   2 +
 samples/rust/rust_driver_pci.rs              | 110 +++++
 samples/rust/rust_driver_platform.rs         |  49 +++
 26 files changed, 2140 insertions(+)
 create mode 100644 rust/helpers/device.c
 create mode 100644 rust/helpers/io.c
 create mode 100644 rust/helpers/pci.c
 create mode 100644 rust/helpers/platform.c
 create mode 100644 rust/helpers/rcu.c
 create mode 100644 rust/kernel/device_id.rs
 create mode 100644 rust/kernel/devres.rs
 create mode 100644 rust/kernel/driver.rs
 create mode 100644 rust/kernel/io.rs
 create mode 100644 rust/kernel/of.rs
 create mode 100644 rust/kernel/pci.rs
 create mode 100644 rust/kernel/platform.rs
 create mode 100644 rust/kernel/revocable.rs
 create mode 100644 rust/kernel/sync/rcu.rs
 create mode 100644 samples/rust/rust_driver_pci.rs
 create mode 100644 samples/rust/rust_driver_platform.rs