Device / Driver PCI / Platform Rust abstractions

From: Wedson Almeida Filho <[email protected]>

This is the same as Opaque::ffi_init, but returns a Result.

This is used by subsequent patches to implement the FFI init of static driver structures on registration.

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

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

From: Wedson Almeida Filho <[email protected]>

This allows modules to be initialised in-place in pinned memory, which enables the usage of pinned types (e.g., mutexes, spinlocks, driver registrations, etc.) in modules without any extra allocations.

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

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

Quoted 7 lines · Danilo Krummrich

From: Wedson Almeida Filho <[email protected]>

This allows modules to be initialised in-place in pinned memory, which enables the usage of pinned types (e.g., mutexes, spinlocks, driver registrations, etc.) in modules without any extra allocations.

Quoted 2 lines

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

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

Reviewed-by Alice Ryhl <[email protected]>

Miguel: I think this patch would be good to land in 6.13. I'd like to submit a miscdevice sample once 6.14 comes around, and having this already in will make the miscdevice sample nicer.

If no one objects, I'll just add this to the char/misc tree that already has the other misc device rust code in it.

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

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

+#![allow(stable_features)]

From: Wedson Almeida Filho <[email protected]>

Add a simple abstraction to guard critical code sections with an rcu read lock.

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

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

 rust/helpers/helpers.c  |  1 +
 rust/helpers/rcu.c      | 13 +++++++++++
 rust/kernel/sync.rs     |  1 +
 rust/kernel/sync/rcu.rs | 52 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+)
 create mode 100644 rust/helpers/rcu.c
 create mode 100644 rust/kernel/sync/rcu.rs

rust/helpers/helpers.c +1 -0

index 20a0c69d5cc7..0720debccdd4 100644
@@ -16,6 +16,7 @@
 #include "mutex.c"
 #include "page.c"
 #include "rbtree.c"
+#include "rcu.c"
 #include "refcount.c"
 #include "signal.c"
 #include "slab.c"

rust/helpers/rcu.c new file +13 -0

new file mode 100644
index 000000000000..f1cec6583513
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/rcupdate.h>
+
+void rust_helper_rcu_read_lock(void)
+{
+       rcu_read_lock();
+}
+
+void rust_helper_rcu_read_unlock(void)
+{
+       rcu_read_unlock();
+}

rust/kernel/sync.rs +1 -0

index 0ab20975a3b5..1806767359fe 100644
@@ -11,6 +11,7 @@
 mod condvar;
 pub mod lock;
 mod locked_by;
+pub mod rcu;
 
 pub use arc::{Arc, ArcBorrow, UniqueArc};
 pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult};

rust/kernel/sync/rcu.rs new file +8 -0

new file mode 100644
index 000000000000..5a35495f69a4
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! RCU support.
+//!
+//! C header: [`include/linux/rcupdate.h`](srctree/include/linux/rcupdate.h)
+
+use crate::bindings;
+use core::marker::PhantomData;

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

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

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

rust/kernel/lib.rs +1 -0

index 89f6bd2efcc0..b603b67dcd71 100644
@@ -54,6 +54,7 @@
 pub mod prelude;
 pub mod print;
 pub mod rbtree;
+pub mod revocable;
 pub mod sizes;
 mod static_assert;
 #[doc(hidden)]

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

new file mode 100644
index 000000000000..83455558d795
@@ -0,0 +1,211 @@
+// 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,
+    init::{self},
+    prelude::*,
+    sync::rcu,
+};
+use core::{
+    cell::UnsafeCell,
+    marker::PhantomData,
+    mem::MaybeUninit,
+    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: MaybeUninit<UnsafeCell<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> {

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

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

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

+    /// 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.assume_init_ref().get() })
+        } else {
+            None
+        }
+    }
+
+    /// 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) {
+        if self
+            .is_available
+            .compare_exchange(true, false, Ordering::Relaxed, Ordering::Relaxed)
+            .is_ok()
+        {
+            // 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.assume_init_ref().get()) };
+        }
+    }
+}
+
+#[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.assume_init_ref().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

rust/kernel/revocable.rs line 191

Quoted 13 lines · Danilo Krummrich

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

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

Is this needed? Can't all users just use try_access_with_guard?

Alice

Quoted 16 lines · Alice Ryhl

rust/kernel/revocable.rs line 191

Quoted 13 lines · Danilo Krummrich

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

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

Is this needed? Can't all users just use try_access_with_guard?

Without this guard, how to we access T with just the rcu::Guard?

Quoted 19 lines · Danilo Krummrich

Quoted 16 lines · Alice Ryhl

rust/kernel/revocable.rs line 191

Quoted 13 lines · Danilo Krummrich

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

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

Is this needed? Can't all users just use try_access_with_guard?

Quoted 1 line

Without this guard, how to we access T with just the rcu::Guard?

I don't think try_access_with_guard provides any access that you can't get by doing try_access_with_guard.

That said, I guess this guard functions as a convenience accessors, so I don't mind it.

Alice

Quoted 20 lines · Alice Ryhl

Quoted 19 lines · Danilo Krummrich

Quoted 16 lines · Alice Ryhl

rust/kernel/revocable.rs line 191

Quoted 13 lines · Danilo Krummrich

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

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

Is this needed? Can't all users just use try_access_with_guard?

Quoted 7 lines

Quoted 1 line

Without this guard, how to we access T with just the rcu::Guard?

I don't think try_access_with_guard provides any access that you can't get by doing try_access_with_guard.

That said, I guess this guard functions as a convenience accessors, so I don't mind it.

What I mean is, how does the following work without RevocableGuard?

```
struct Foo;

impl Foo {
    pub fn bar() { ... }
}

let data: Revocable<Foo> = ...;
let guard = data.try_access()?;

guard.bar();
```

From: Wedson Almeida Filho <[email protected]>

Implement dev_* print macros for device::Device.

They behave like the macros with the same names in C, i.e., they print messages to the kernel ring buffer with the given level, prefixing the messages with corresponding device information.

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

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

---
 rust/kernel/device.rs  | 319 ++++++++++++++++++++++++++++++++++++++++-
 rust/kernel/prelude.rs |   2 +
 2 files changed, 320 insertions(+), 1 deletion(-)

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.

Io 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 Io accordingly.

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

Co-developed-by Philipp Stanner <[email protected]>

Signed-off-by Philipp Stanner <[email protected]>

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

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

new file mode 100644
index 000000000000..750af938f83e
@@ -0,0 +1,234 @@
+// 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};

Quoted 12 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.

Quoted 5 lines

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.

And using zero works because the user then statically knows that zero bytes are available ... ?

Quoted 9 lines

Io 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 Io accordingly.

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

I wonder if Io should be a reference type instead. That is:

struct Io<'a, const SIZE: usize> {
    addr: usize,
    maxsize: usize,
    _lifetime: PhantomData<&'a ()>,
}

and then the constructor requires "addr must be valid I/O mapped memory for maxsize bytes for the duration of 'a". And instead of embedding it in another struct, the other struct creates an Io on each access?

Quoted 4 lines

Co-developed-by Philipp Stanner <[email protected]>

Signed-off-by Philipp Stanner <[email protected]>

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

[...]

Quoted 18 lines

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

new file mode 100644
index 000000000000..750af938f83e
@@ -0,0 +1,234 @@
+// 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};
+
+/// 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

rust/kernel/io.rs line 18

Quoted 6 lines

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

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

Do you not also need an invariant that SIZE <= maxsize?

Alice

Quoted 19 lines · Alice Ryhl

Quoted 17 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.

Quoted 2 lines

And using zero works because the user then statically knows that zero bytes are available ... ?

Zero would mean that the (minimum) resource size is unknown at compile time. Correspondingly, any call to read and write would not compile, since the compile time check requires that offset + type_size <= SIZE.

(Hope this answers the questions, I'm not sure I got it correctly.)

Quoted 17 lines

Quoted 8 lines

Io 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 Io accordingly.

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

I wonder if Io should be a reference type instead. That is:

struct Io<'a, const SIZE: usize> {
    addr: usize,
    maxsize: usize,
    _lifetime: PhantomData<&'a ()>,
}

Quoted 4 lines

and then the constructor requires "addr must be valid I/O mapped memory for maxsize bytes for the duration of 'a". And instead of embedding it in another struct, the other struct creates an Io on each access?

So, we'd create the Io instance in deref of the parent structure, right? What would be the advantage?

Quoted 18 lines

Quoted 18 lines

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

new file mode 100644
index 000000000000..750af938f83e
@@ -0,0 +1,234 @@
+// 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};
+
+/// 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

Quoted 8 lines

rust/kernel/io.rs line 18

Quoted 6 lines

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

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

Do you not also need an invariant that SIZE <= maxsize?

I guess so, yes. It's enforced by Io::new, which fails if SIZE > maxsize.

What you're doing now is a bit awkward to use. You have to make sure that it never escapes the struct it's created for, so e.g. you can't give out a mutable reference as the user could otherwise mem::swap it with another Io. Similarly, the Io can never be in a public field. Your safety comment on Io::new really needs to say something like "while this struct exists, the addr must be a valid I/O region", since I assume such regions are not valid forever? Similarly if we

look at [1], the I/O region actually gets unmapped *before* the Io is destroyed since IoMem::drop runs before the fields of IoMem are destroyed, so you really need something like "until the last use of this Io" and not "until this Io is destroyed" in the safety comment.

If you compare similar cases in Rust, then they also do what I suggested. For example, Vec<T> holds a raw pointer, and it uses unsafe to assert that it's valid on each use of the raw pointer - it does not create e.g. an &'static mut [T] to hold in a field of the Vec<T>. Having an IoRaw<S> and an Io<'a, S> corresponds to what Vec<T> does with IoRaw being like NonNull<T> and Io<'a, S> being like &'a T.

[1]: https://lore.kernel.org/all/20241024-topic-panthor-rs-platform_io_support-v1-1-3d1addd96e30@collabora.com/

Quoted 12 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.

Quoted 5 lines

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.

And using zero works because the user then statically knows that zero bytes are available ... ?

Quoted 8 lines

Io 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 Io accordingly.

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

I wonder if Io should be a reference type instead. That is:

struct Io<'a, const SIZE: usize> {
   addr: usize,
   maxsize: usize,
   _lifetime: PhantomData<&'a ()>,
}

and then the constructor requires "addr must be valid I/O mapped memory for maxsize bytes for the duration of 'a". And instead of embedding it in another struct, the other struct creates an Io on each access?

Quoted 4 lines

Co-developed-by Philipp Stanner <[email protected]>

Signed-off-by Philipp Stanner <[email protected]>

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

[...]

Quoted 14 lines

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

new file mode 100644
index 000000000000..750af938f83e
@@ -0,0 +1,234 @@
+// 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};

rust/kernel/io.rs line 18

Quoted 10 lines

rust/kernel/io.rs new file +10 -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`.

Do you not also need an invariant that SIZE <= maxsize?

Alice

Add a Rust abstraction for the kernel's devres (device resource management) implementation.

The Devres type acts as a container to manage the lifetime and accessibility of device bound resources. Therefore it registers a devres callback and revokes access to the resource on invocation.

Users of the Devres abstraction can simply free the corresponding resources in their Drop implementation, which is invoked when either the Devres instance goes out of scope or the devres callback leads to the resource being revoked, which implies a call to drop_in_place().

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

Add a Rust abstraction for the kernel's devres (device resource management) implementation.

The Devres type acts as a container to manage the lifetime and accessibility of device bound resources. Therefore it registers a devres callback and revokes access to the resource on invocation.

Users of the Devres abstraction can simply free the corresponding resources in their Drop implementation, which is invoked when either the Devres instance goes out of scope or the devres callback leads to the resource being revoked, which implies a call to drop_in_place().

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

---
 MAINTAINERS            |   1 +
 rust/helpers/device.c  |  10 +++
 rust/helpers/helpers.c |   1 +
 rust/kernel/devres.rs  | 180 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs     |   1 +
 5 files changed, 193 insertions(+)
 create mode 100644 rust/helpers/device.c
 create mode 100644 rust/kernel/devres.rs

MAINTAINERS +1 -0

index 0a8882252257..97914d0752fb 100644
@@ -6983,6 +6983,7 @@ F:        include/linux/property.h
 F:     lib/kobj*
 F:     rust/kernel/device.rs
 F:     rust/kernel/device_id.rs
+F:     rust/kernel/devres.rs
 F:     rust/kernel/driver.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)

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

new file mode 100644
index 000000000000..b2135c6686b0
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/device.h>
+
+int rust_helper_devm_add_action(struct device *dev,
+                               void (*action)(void *),
+                               void *data)
+{
+       return devm_add_action(dev, action, data);
+}

rust/helpers/helpers.c +1 -0

index e2f6b2197061..3acb2b9e52ec 100644
@@ -11,6 +11,7 @@
 #include "bug.c"
 #include "build_assert.c"
 #include "build_bug.c"
+#include "device.c"
 #include "err.c"
 #include "io.c"
 #include "kunit.c"

rust/kernel/devres.rs new file +168 -0

new file mode 100644
index 000000000000..b23559f55214
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Devres abstraction
+//!
+//! [`Devres`] represents an abstraction for the kernel devres (device resource management)
+//! implementation.
+
+use crate::{
+    alloc::Flags,
+    bindings,
+    device::Device,
+    error::{Error, Result},
+    prelude::*,
+    revocable::Revocable,
+    sync::Arc,
+};
+
+use core::ffi::c_void;
+use core::ops::Deref;
+
+#[pin_data]
+struct DevresInner<T> {
+    #[pin]
+    data: Revocable<T>,
+}
+
+/// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to
+/// manage their lifetime.
+///
+/// [`Device`] bound resources should be freed when either the resource goes out of scope or the
+/// [`Device`] is unbound respectively, depending on what happens first.
+///
+/// To achieve that [`Devres`] registers a devres callback on creation, which is called once the
+/// [`Device`] is unbound, revoking access to the encapsulated resource (see also [`Revocable`]).
+///
+/// After the [`Devres`] has been unbound it is not possible to access the encapsulated resource
+/// anymore.
+///
+/// [`Devres`] users should make sure to simply free the corresponding backing resource in `T`'s
+/// [`Drop`] implementation.
+///
+/// # Example
+///
+/// ```no_run
+/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::Io};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(Io<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);
+///         }
+///
+///         // SAFETY: `addr` is guaranteed to be the start of a valid I/O mapped memory region of
+///         // size `SIZE`.
+///         let io = unsafe { Io::new(addr as _, SIZE)? };
+///
+///         Ok(IoMem(io))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: Safe as by the invariant of `Io`.
+///         unsafe { bindings::iounmap(self.0.base_addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///        &self.0
+///    }
+/// }
+///
+/// # fn no_run() -> Result<(), Error> {
+/// # // SAFETY: Invalid usage; just for the example to get an `ARef<Device>` instance.
+/// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) };
+///
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// let devres = Devres::new(&dev, iomem, GFP_KERNEL)?;
+///
+/// let res = devres.try_access().ok_or(ENXIO)?;
+/// res.writel(0x42, 0x0);
+/// # Ok(())
+/// # }
+/// ```
+pub struct Devres<T>(Arc<DevresInner<T>>);
+
+impl<T> DevresInner<T> {
+    fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
+        let inner = Arc::pin_init(
+            pin_init!( DevresInner {
+                data <- Revocable::new(data),
+            }),
+            flags,
+        )?;
+
+        // Convert `Arc<DevresInner>` into a raw pointer and make devres own this reference until
+        // `Self::devres_callback` is called.
+        let data = inner.clone().into_raw();
+
+        // SAFETY: `devm_add_action` guarantees to call `Self::devres_callback` once `dev` is
+        // detached.
+        let ret = unsafe {
+            bindings::devm_add_action(dev.as_raw(), Some(Self::devres_callback), data as _)
+        };
+
+        if ret != 0 {
+            // SAFETY: We just created another reference to `inner` in order to pass it to
+            // `bindings::devm_add_action`. If `bindings::devm_add_action` fails, we have to drop
+            // this reference accordingly.
+            let _ = unsafe { Arc::from_raw(data) };
+            return Err(Error::from_errno(ret));
+        }
+
+        Ok(inner)
+    }
+
+    #[allow(clippy::missing_safety_doc)]
+    unsafe extern "C" fn devres_callback(ptr: *mut c_void) {
+        let ptr = ptr as *mut DevresInner<T>;
+        // Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
+        // reference.
+        // SAFETY: Safe, since we leaked an `Arc` reference to devm_add_action() in
+        //         `DevresInner::new`.
+        let inner = unsafe { Arc::from_raw(ptr) };
+
+        inner.data.revoke();
+    }
+}
+
+impl<T> Devres<T> {
+    /// Creates a new [`Devres`] instance of the given `data`. The `data` encapsulated within the
+    /// returned `Devres` instance' `data` will be revoked once the device is detached.
+    pub fn new(dev: &Device, data: T, flags: Flags) -> Result<Self> {
+        let inner = DevresInner::new(dev, data, flags)?;
+
+        Ok(Devres(inner))
+    }
+
+    /// Same as [`Devres::new`], but does not return a `Devres` instance. Instead the given `data`
+    /// is owned by devres and will be revoked / dropped, once the device is detached.
+    pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result {
+        let _ = DevresInner::new(dev, data, flags)?;
+
+        Ok(())
+    }
+}
+
+impl<T> Deref for Devres<T> {
+    type Target = Revocable<T>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0.data

Quoted 245 lines · Danilo Krummrich

Add a Rust abstraction for the kernel's devres (device resource management) implementation.

The Devres type acts as a container to manage the lifetime and accessibility of device bound resources. Therefore it registers a devres callback and revokes access to the resource on invocation.

Users of the Devres abstraction can simply free the corresponding resources in their Drop implementation, which is invoked when either the Devres instance goes out of scope or the devres callback leads to the resource being revoked, which implies a call to drop_in_place().

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

---
  MAINTAINERS            |   1 +
  rust/helpers/device.c  |  10 +++
  rust/helpers/helpers.c |   1 +
  rust/kernel/devres.rs  | 180 +++++++++++++++++++++++++++++++++++++++++
  rust/kernel/lib.rs     |   1 +
  5 files changed, 193 insertions(+)

create mode 100644 rust/helpers/device.c
create mode 100644 rust/kernel/devres.rs

MAINTAINERS +1 -0

index 0a8882252257..97914d0752fb 100644
@@ -6983,6 +6983,7 @@ F:        include/linux/property.h
  F:     lib/kobj*
  F:     rust/kernel/device.rs
  F:     rust/kernel/device_id.rs
+F:     rust/kernel/devres.rs
  F:     rust/kernel/driver.rs
 
  DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)

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

new file mode 100644
index 000000000000..b2135c6686b0
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/device.h>
+
+int rust_helper_devm_add_action(struct device *dev,
+                               void (*action)(void *),
+                               void *data)
+{
+       return devm_add_action(dev, action, data);
+}

rust/helpers/helpers.c +1 -0

index e2f6b2197061..3acb2b9e52ec 100644
@@ -11,6 +11,7 @@
  #include "bug.c"
  #include "build_assert.c"
  #include "build_bug.c"
+#include "device.c"
  #include "err.c"
  #include "io.c"
  #include "kunit.c"

rust/kernel/devres.rs new file +175 -0

new file mode 100644
index 000000000000..b23559f55214
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Devres abstraction
+//!
+//! [`Devres`] represents an abstraction for the kernel devres (device resource management)
+//! implementation.
+
+use crate::{
+    alloc::Flags,
+    bindings,
+    device::Device,
+    error::{Error, Result},
+    prelude::*,
+    revocable::Revocable,
+    sync::Arc,
+};
+
+use core::ffi::c_void;
+use core::ops::Deref;
+
+#[pin_data]
+struct DevresInner<T> {
+    #[pin]
+    data: Revocable<T>,
+}
+
+/// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to
+/// manage their lifetime.
+///
+/// [`Device`] bound resources should be freed when either the resource goes out of scope or the
+/// [`Device`] is unbound respectively, depending on what happens first.
+///
+/// To achieve that [`Devres`] registers a devres callback on creation, which is called once the
+/// [`Device`] is unbound, revoking access to the encapsulated resource (see also [`Revocable`]).
+///
+/// After the [`Devres`] has been unbound it is not possible to access the encapsulated resource
+/// anymore.
+///
+/// [`Devres`] users should make sure to simply free the corresponding backing resource in `T`'s
+/// [`Drop`] implementation.
+///
+/// # Example
+///
+/// ```no_run
+/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::Io};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(Io<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);
+///         }
+///
+///         // SAFETY: `addr` is guaranteed to be the start of a valid I/O mapped memory region of
+///         // size `SIZE`.
+///         let io = unsafe { Io::new(addr as _, SIZE)? };
+///
+///         Ok(IoMem(io))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: Safe as by the invariant of `Io`.
+///         unsafe { bindings::iounmap(self.0.base_addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<SIZE>;
+///
+///    fn deref(&self) -> &Self::Target {
+///        &self.0
+///    }
+/// }
+///
+/// # fn no_run() -> Result<(), Error> {
+/// # // SAFETY: Invalid usage; just for the example to get an `ARef<Device>` instance.
+/// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) };
+///
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// let devres = Devres::new(&dev, iomem, GFP_KERNEL)?;
+///
+/// let res = devres.try_access().ok_or(ENXIO)?;
+/// res.writel(0x42, 0x0);
+/// # Ok(())
+/// # }
+/// ```
+pub struct Devres<T>(Arc<DevresInner<T>>);
+
+impl<T> DevresInner<T> {
+    fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
+        let inner = Arc::pin_init(
+            pin_init!( DevresInner {
+                data <- Revocable::new(data),
+            }),
+            flags,
+        )?;
+
+        // Convert `Arc<DevresInner>` into a raw pointer and make devres own this reference until
+        // `Self::devres_callback` is called.
+        let data = inner.clone().into_raw();
+
+        // SAFETY: `devm_add_action` guarantees to call `Self::devres_callback` once `dev` is
+        // detached.
+        let ret = unsafe {
+            bindings::devm_add_action(dev.as_raw(), Some(Self::devres_callback), data as _)
+        };
+
+        if ret != 0 {
+            // SAFETY: We just created another reference to `inner` in order to pass it to
+            // `bindings::devm_add_action`. If `bindings::devm_add_action` fails, we have to drop
+            // this reference accordingly.
+            let _ = unsafe { Arc::from_raw(data) };
+            return Err(Error::from_errno(ret));
+        }
+
+        Ok(inner)
+    }
+
+    #[allow(clippy::missing_safety_doc)]
+    unsafe extern "C" fn devres_callback(ptr: *mut c_void) {
+        let ptr = ptr as *mut DevresInner<T>;
+        // Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
+        // reference.
+        // SAFETY: Safe, since we leaked an `Arc` reference to devm_add_action() in
+        //         `DevresInner::new`.
+        let inner = unsafe { Arc::from_raw(ptr) };
+
+        inner.data.revoke();
+    }
+}
+
+impl<T> Devres<T> {
+    /// Creates a new [`Devres`] instance of the given `data`. The `data` encapsulated within the
+    /// returned `Devres` instance' `data` will be revoked once the device is detached.
+    pub fn new(dev: &Device, data: T, flags: Flags) -> Result<Self> {
+        let inner = DevresInner::new(dev, data, flags)?;
+
+        Ok(Devres(inner))
+    }
+
+    /// Same as [`Devres::new`], but does not return a `Devres` instance. Instead the given `data`
+    /// is owned by devres and will be revoked / dropped, once the device is detached.
+    pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result {
+        let _ = DevresInner::new(dev, data, flags)?;
+
+        Ok(())
+    }
+}
+
+impl<T> Deref for Devres<T> {
+    type Target = Revocable<T>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0.data
+    }
+}
+
+impl<T> Drop for Devres<T> {
+    fn drop(&mut self) {
+        // Revoke the data, such that it gets dropped already and the actual resource is freed.
+        // `DevresInner` has to stay alive until the devres callback has been called. This is

rust/kernel/devres.rs line 178

Quoted 3 lines

rust/kernel/devres.rs new file +3 -0

+        // necessary since we don't know when `Devres` is dropped and calling
+        // `devm_remove_action()` instead could race with `devres_release_all()`.
+        self.revoke();

When the destructor runs, it's guaranteed that nobody is accessing the inner resource since the only way to do that is through the Devres handle, but its destructor is running. Therefore, you can skip the synchronize_rcu() call in this case.

Alice

Quoted 9 lines · Alice Ryhl

Quoted 7 lines · Danilo Krummrich

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

+impl<T> Drop for Devres<T> {
+    fn drop(&mut self) {
+        // Revoke the data, such that it gets dropped already and the actual resource is freed.
+        // `DevresInner` has to stay alive until the devres callback has been called. This is
+        // necessary since we don't know when `Devres` is dropped and calling
+        // `devm_remove_action()` instead could race with `devres_release_all()`.
+        self.revoke();

Quoted 4 lines

When the destructor runs, it's guaranteed that nobody is accessing the inner resource since the only way to do that is through the Devres handle, but its destructor is running. Therefore, you can skip the synchronize_rcu() call in this case.

Yeah, I think this optimization should be possible.

We'd require Revocable to have a revoke_nosync method for that I guess...

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

+impl Device {
+    /// Create a PCI Device instance from an existing `device::Device`.
+    ///
+    /// # Safety
+    ///
+    /// `dev` must be an `ARef<device::Device>` whose underlying `bindings::device` is a member of
+    /// a `bindings::pci_dev`.
+    pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
+        Self(dev)
+    }
+
+    fn as_raw(&self) -> *mut bindings::pci_dev {
+        // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
+        // embedded in `struct pci_dev`.
+        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+    }
+
+    /// Enable memory resources for this device.
+    pub fn enable_device_mem(&self) -> Result {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        let ret = unsafe { bindings::pci_enable_device_mem(self.as_raw()) };
+        if ret != 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Enable bus-mastering for this device.
+    pub fn set_master(&self) {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        unsafe { bindings::pci_set_master(self.as_raw()) };
+    }
+}
+
+impl AsRef<device::Device> for Device {
+    fn as_ref(&self) -> &device::Device {
+        &self.0
+    }
+}

Signature · 1 line

2.46.2

Hi Danilo,

On Tue, Oct 22, 2024 at 11:31:48PM +0200, Danilo Krummrich wrote:
[...]

rust/kernel/pci.rs line 244

Quoted 7 lines

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

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);
+

Similar to https://lore.kernel.org/rust-for-linux/ZgG7TlybSa00cuoy@boqun-archlinux/

Could you also avoid wrapping a point to a PCI device? Instead, wrap the object type:

    #[repr(transparent)]
    pub struct Device(Opaque<bindings::pci_dev>);

    impl AlwaysRefCounted for Device {
        <put_device() and get_device() on ->dev>
    }

Regards,
Boqun

Quoted 44 lines

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

+impl Device {
+    /// Create a PCI Device instance from an existing `device::Device`.
+    ///
+    /// # Safety
+    ///
+    /// `dev` must be an `ARef<device::Device>` whose underlying `bindings::device` is a member of
+    /// a `bindings::pci_dev`.
+    pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
+        Self(dev)
+    }
+
+    fn as_raw(&self) -> *mut bindings::pci_dev {
+        // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
+        // embedded in `struct pci_dev`.
+        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+    }
+
+    /// Enable memory resources for this device.
+    pub fn enable_device_mem(&self) -> Result {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        let ret = unsafe { bindings::pci_enable_device_mem(self.as_raw()) };
+        if ret != 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Enable bus-mastering for this device.
+    pub fn set_master(&self) {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        unsafe { bindings::pci_set_master(self.as_raw()) };
+    }
+}
+
+impl AsRef<device::Device> for Device {
+    fn as_ref(&self) -> &device::Device {
+        &self.0
+    }
+}

Signature · 1 line

2.46.2

Hi Danilo,

On Tue, Oct 22, 2024 at 11:31:48PM +0200, Danilo Krummrich wrote:
[...]

rust/kernel/pci.rs line 244

Quoted 7 lines

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

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);
+

Similar to https://lore.kernel.org/rust-for-linux/ZgG7TlybSa00cuoy@boqun-archlinux/

Could you also avoid wrapping a point to a PCI device? Instead, wrap the object type:

    #[repr(transparent)]
    pub struct Device(Opaque<bindings::pci_dev>);

    impl AlwaysRefCounted for Device {
        <put_device() and get_device() on ->dev>
    }

Regards,
Boqun

Quoted 44 lines

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

+impl Device {
+    /// Create a PCI Device instance from an existing `device::Device`.
+    ///
+    /// # Safety
+    ///
+    /// `dev` must be an `ARef<device::Device>` whose underlying `bindings::device` is a member of
+    /// a `bindings::pci_dev`.
+    pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
+        Self(dev)
+    }
+
+    fn as_raw(&self) -> *mut bindings::pci_dev {
+        // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
+        // embedded in `struct pci_dev`.
+        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+    }
+
+    /// Enable memory resources for this device.
+    pub fn enable_device_mem(&self) -> Result {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        let ret = unsafe { bindings::pci_enable_device_mem(self.as_raw()) };
+        if ret != 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Enable bus-mastering for this device.
+    pub fn set_master(&self) {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        unsafe { bindings::pci_set_master(self.as_raw()) };
+    }
+}
+
+impl AsRef<device::Device> for Device {
+    fn as_ref(&self) -> &device::Device {
+        &self.0
+    }
+}

Signature · 1 line

2.46.2

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

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

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/pci.c              |  18 ++
 rust/kernel/lib.rs              |   2 +
 rust/kernel/pci.rs              | 284 ++++++++++++++++++++++++++++++++
 6 files changed, 307 insertions(+)
 create mode 100644 rust/helpers/pci.c
 create mode 100644 rust/kernel/pci.rs

MAINTAINERS +1 -0

index 97914d0752fb..2d00d3845b4a 100644
@@ -17939,6 +17939,7 @@ F:	include/asm-generic/pci*
 F:	include/linux/of_pci.h
 F:	include/linux/pci*
 F:	include/uapi/linux/pci*
+F:	rust/kernel/pci.rs
 
 PCIE DRIVER FOR AMAZON ANNAPURNA LABS
 M:	Jonathan Chocron <[email protected]>

rust/bindings/bindings_helper.h +1 -0

index a80783fcbe04..cd4edd6496ae 100644
@@ -15,6 +15,7 @@
 #include <linux/firmware.h>
 #include <linux/jiffies.h>
 #include <linux/mdio.h>
+#include <linux/pci.h>
 #include <linux/phy.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>

rust/helpers/helpers.c +1 -0

index 3acb2b9e52ec..8bc6e9735589 100644
@@ -17,6 +17,7 @@
 #include "kunit.c"
 #include "mutex.c"
 #include "page.c"
+#include "pci.c"
 #include "rbtree.c"
 #include "rcu.c"
 #include "refcount.c"

rust/helpers/pci.c new file +8 -0

new file mode 100644
index 000000000000..7c4f3ba49486
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/pci.h>
+
+void rust_helper_pci_set_drvdata(struct pci_dev *pdev, void *data)
+{
+	pci_set_drvdata(pdev, data);
+}

rust/kernel/lib.rs +2 -0

index 5cb892419453..3ec690eb6d43 100644
@@ -73,6 +73,8 @@
 pub use bindings;
 pub mod io;
 pub use macros;
+#[cfg(all(CONFIG_PCI, CONFIG_PCI_MSI))]
+pub mod pci;
 pub use uapi;
 
 #[doc(hidden)]

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

new file mode 100644
index 000000000000..ccc9a5f322e4
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the PCI bus.
+//!
+//! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h)
+
+use crate::{
+    bindings, container_of, device,
+    device_id::RawDeviceId,
+    driver,
+    error::{to_result, Result},
+    str::CStr,
+    types::{ARef, ForeignOwnable},
+    ThisModule,
+};
+use core::ops::Deref;
+use kernel::prelude::*;
+
+/// An adapter for the registration of PCI drivers.
+pub struct Adapter<T: Driver>(T);
+
+impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::pci_driver;
+
+    fn register(
+        pdrv: &mut Self::RegType,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        pdrv.name = name.as_char_ptr();
+        pdrv.probe = Some(Self::probe_callback);
+        pdrv.remove = Some(Self::remove_callback);
+        pdrv.id_table = T::ID_TABLE.as_ptr();
+
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe {
+            bindings::__pci_register_driver(pdrv as _, module.0, name.as_char_ptr())
+        })
+    }
+
+    fn unregister(pdrv: &mut Self::RegType) {
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::pci_unregister_driver(pdrv) }
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    extern "C" fn probe_callback(
+        pdev: *mut bindings::pci_dev,
+        id: *const bindings::pci_device_id,
+    ) -> core::ffi::c_int {
+        // SAFETY: The PCI bus only ever calls the probe callback with a valid pointer to a
+        // `struct pci_dev`.
+        let dev = unsafe { device::Device::from_raw(&mut (*pdev).dev) };
+        // SAFETY: `dev` is guaranteed to be embedded in a valid `struct pci_dev` by the call
+        // above.
+        let mut pdev = unsafe { Device::from_dev(dev) };
+
+        // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct pci_device_id` and
+        // does not add additional invariants, so it's safe to transmute.
+        let id = unsafe { &*id.cast::<DeviceId>() };
+        let info = T::ID_TABLE.info(id.index());
+
+        match T::probe(&mut pdev, id, info) {
+            Ok(data) => {
+                // Let the `struct pci_dev` own a reference of the driver's private data.
+                // SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
+                // `struct pci_dev`.
+                unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
+            }
+            Err(err) => return Error::to_errno(err),
+        }
+
+        0
+    }
+
+    extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {
+        // SAFETY: The PCI bus only ever calls the remove callback with a valid pointer to a
+        // `struct pci_dev`.
+        let ptr = unsafe { bindings::pci_get_drvdata(pdev) };
+
+        // SAFETY: `remove_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
+        // `KBox<T>` pointer created through `KBox::into_foreign`.
+        let _ = unsafe { KBox::<T>::from_foreign(ptr) };
+    }
+}
+
+/// Declares a kernel module that exposes a single PCI driver.
+///
+/// # Example
+///
+///```ignore
+/// kernel::module_pci_driver! {
+///     type: MyDriver,
+///     name: "Module name",
+///     author: "Author name",
+///     description: "Description",
+///     license: "GPL v2",
+/// }
+///```
+#[macro_export]
+macro_rules! module_pci_driver {
+($($f:tt)*) => {
+    $crate::module_driver!(<T>, $crate::pci::Adapter<T>, { $($f)* });
+};
+}
+
+/// Abstraction for bindings::pci_device_id.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(bindings::pci_device_id);
+
+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,
+        })
+    }
+
+    /// PCI_DEVICE_CLASS macro.
+    pub const fn with_class(class: u32, class_mask: u32) -> Self {
+        Self(bindings::pci_device_id {
+            vendor: DeviceId::PCI_ANY_ID,
+            device: DeviceId::PCI_ANY_ID,
+            subvendor: DeviceId::PCI_ANY_ID,
+            subdevice: DeviceId::PCI_ANY_ID,
+            class,
+            class_mask,
+            driver_data: 0,
+            override_only: 0,
+        })
+    }
+}
+
+// Allow drivers R/O access to the fields of `pci_device_id`; should we prefer accessor functions
+// to void exposing C structure fields?
+impl Deref for DeviceId {
+    type Target = bindings::pci_device_id;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+// SAFETY:
+// * `DeviceId` is a `#[repr(transparent)` wrapper of `pci_device_id` and does not add
+//   additional invariants, so it's safe to transmute to `RawType`.
+// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceId for DeviceId {
+    type RawType = bindings::pci_device_id;
+
+    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
+
+    fn index(&self) -> usize {
+        self.driver_data as _
+    }
+}
+
+/// IdTable type for PCI
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// Create a PCI `IdTable` with its alias for modpost.
+#[macro_export]
+macro_rules! pci_device_table {
+    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+        const $table_name: $crate::device_id::IdArray<
+            $crate::pci::DeviceId,
+            $id_info_type,
+            { $table_data.len() },
+        > = $crate::device_id::IdArray::new($table_data);
+
+        $crate::module_device_table!("pci", $module_table_name, $table_name);
+    };
+}
+
+/// The PCI driver trait.
+///
+/// # Example
+///
+///```
+/// # use kernel::{bindings, pci};
+///
+/// struct MyDriver;
+///
+/// kernel::pci_device_table!(
+///     PCI_TABLE,
+///     MODULE_PCI_TABLE,
+///     <MyDriver as pci::Driver>::IdInfo,
+///     [
+///         (pci::DeviceId::new(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as u32), ())
+///     ]
+/// );
+///
+/// impl pci::Driver for MyDriver {
+///     type IdInfo = ();
+///     const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
+///
+///     fn probe(
+///         _pdev: &mut pci::Device,
+///         _id: &pci::DeviceId,
+///         _id_info: &Self::IdInfo,
+///     ) -> Result<Pin<KBox<Self>>> {
+///         Err(ENODEV)
+///     }
+/// }
+///```
+/// Drivers must implement this trait in order to get a PCI driver registered. Please refer to the
+/// `Adapter` documentation for an example.
+pub trait Driver {
+    /// The type holding information about each device id supported by the driver.
+    ///
+    /// TODO: Use associated_type_defaults once stabilized:
+    ///
+    /// type IdInfo: 'static = ();
+    type IdInfo: 'static;
+
+    /// The table of device ids supported by the driver.
+    const ID_TABLE: IdTable<Self::IdInfo>;
+
+    /// PCI driver probe.
+    ///
+    /// Called when a new platform device is added or discovered.
+    /// Implementers should attempt to initialize the device here.
+    fn probe(dev: &mut Device, id: &DeviceId, id_info: &Self::IdInfo) -> Result<Pin<KBox<Self>>>;
+}
+
+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);
+
+impl Device {
+    /// Create a PCI Device instance from an existing `device::Device`.
+    ///
+    /// # Safety
+    ///
+    /// `dev` must be an `ARef<device::Device>` whose underlying `bindings::device` is a member of
+    /// a `bindings::pci_dev`.
+    pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
+        Self(dev)
+    }
+
+    fn as_raw(&self) -> *mut bindings::pci_dev {
+        // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
+        // embedded in `struct pci_dev`.
+        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+    }
+
+    /// Enable memory resources for this device.
+    pub fn enable_device_mem(&self) -> Result {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        let ret = unsafe { bindings::pci_enable_device_mem(self.as_raw()) };
+        if ret != 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Enable bus-mastering for this device.
+    pub fn set_master(&self) {