Re: [RFC PATCH 02/11] rust: add driver abstraction
From: Dirk Behme
Date: Thu May 30 2024 - 01:59:14 EST
On 20.05.2024 19:25, Danilo Krummrich wrote:
From: Wedson Almeida Filho <wedsonaf@xxxxxxxxx>
This defines general functionality related to registering drivers with
their respective subsystems, and registering modules that implement
drivers.
Co-developed-by: Asahi Lina <lina@xxxxxxxxxxxxx>
Signed-off-by: Asahi Lina <lina@xxxxxxxxxxxxx>
Co-developed-by: Andreas Hindborg <a.hindborg@xxxxxxxxxxx>
Signed-off-by: Andreas Hindborg <a.hindborg@xxxxxxxxxxx>
Signed-off-by: Wedson Almeida Filho <wedsonaf@xxxxxxxxx>
Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
---
rust/kernel/driver.rs | 492 +++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 4 +-
rust/macros/module.rs | 2 +-
samples/rust/rust_minimal.rs | 2 +-
samples/rust/rust_print.rs | 2 +-
5 files changed, 498 insertions(+), 4 deletions(-)
create mode 100644 rust/kernel/driver.rs
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
new file mode 100644
index 000000000000..e0cfc36d47ff
--- /dev/null
+++ b/rust/kernel/driver.rs
@@ -0,0 +1,492 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generic support for drivers of different buses (e.g., PCI, Platform, Amba, etc.).
+//!
+//! Each bus/subsystem is expected to implement [`DriverOps`], which allows drivers to register
+//! using the [`Registration`] class.
+
+use crate::{
+ alloc::{box_ext::BoxExt, flags::*},
+ error::code::*,
+ error::Result,
+ str::CStr,
+ sync::Arc,
+ ThisModule,
+};
+use alloc::boxed::Box;
+use core::{cell::UnsafeCell, marker::PhantomData, ops::Deref, pin::Pin};
+
+/// A subsystem (e.g., PCI, Platform, Amba, etc.) that allows drivers to be written for it.
+pub trait DriverOps {
+ /// The type that holds information about the registration. This is typically a struct defined
+ /// by the C portion of the kernel.
+ type RegType: Default;
+
+ /// Registers a driver.
+ ///
+ /// # Safety
+ ///
+ /// `reg` must point to valid, initialised, and writable memory. It may be modified by this
+ /// function to hold registration state.
+ ///
+ /// On success, `reg` must remain pinned and valid until the matching call to
+ /// [`DriverOps::unregister`].
+ unsafe fn register(
+ reg: *mut Self::RegType,
+ name: &'static CStr,
+ module: &'static ThisModule,
+ ) -> Result;
+
+ /// Unregisters a driver previously registered with [`DriverOps::register`].
+ ///
+ /// # Safety
+ ///
+ /// `reg` must point to valid writable memory, initialised by a previous successful call to
+ /// [`DriverOps::register`].
+ unsafe fn unregister(reg: *mut Self::RegType);
+}
+
+/// The registration of a driver.
+pub struct Registration<T: DriverOps> {
+ is_registered: bool,
+ concrete_reg: UnsafeCell<T::RegType>,
+}
+
+// SAFETY: `Registration` has no fields or methods accessible via `&Registration`, so it is safe to
+// share references to it with multiple threads as nothing can be done.
+unsafe impl<T: DriverOps> Sync for Registration<T> {}
+
+impl<T: DriverOps> Registration<T> {
+ /// Creates a new instance of the registration object.
+ pub fn new() -> Self {
+ Self {
+ is_registered: false,
+ concrete_reg: UnsafeCell::new(T::RegType::default()),
+ }
+ }
+
+ /// Allocates a pinned registration object and registers it.
+ ///
+ /// Returns a pinned heap-allocated representation of the registration.
+ pub fn new_pinned(name: &'static CStr, module: &'static ThisModule) -> Result<Pin<Box<Self>>> {
+ let mut reg = Pin::from(Box::new(Self::new(), GFP_KERNEL)?);
+ reg.as_mut().register(name, module)?;
+ Ok(reg)
+ }
+
+ /// Registers a driver with its subsystem.
+ ///
+ /// It must be pinned because the memory block that represents the registration is potentially
+ /// self-referential.
+ pub fn register(
+ self: Pin<&mut Self>,
+ name: &'static CStr,
+ module: &'static ThisModule,
+ ) -> Result {
+ // SAFETY: We never move out of `this`.
+ let this = unsafe { self.get_unchecked_mut() };
+ if this.is_registered {
+ // Already registered.
+ return Err(EINVAL);
+ }
+
+ // SAFETY: `concrete_reg` was initialised via its default constructor. It is only freed
+ // after `Self::drop` is called, which first calls `T::unregister`.
+ unsafe { T::register(this.concrete_reg.get(), name, module) }?;
+
+ this.is_registered = true;
+ Ok(())
+ }
+}
+
+impl<T: DriverOps> Default for Registration<T> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+impl<T: DriverOps> Drop for Registration<T> {
+ fn drop(&mut self) {
+ if self.is_registered {
+ // SAFETY: This path only runs if a previous call to `T::register` completed
+ // successfully.
+ unsafe { T::unregister(self.concrete_reg.get()) };
+ }
+ }
+}
+
+/// Conversion from a device id to a raw device id.
+///
+/// This is meant to be implemented by buses/subsystems so that they can use [`IdTable`] to
+/// guarantee (at compile-time) zero-termination of device id tables provided by drivers.
+///
+/// Originally, RawDeviceId was implemented as a const trait. However, this unstable feature is
+/// broken/gone in 1.73. To work around this, turn IdArray::new() into a macro such that it can use
+/// concrete types (which can still have const associated functions) instead of a trait.
+///
+/// # Safety
+///
+/// Implementers must ensure that:
+/// - [`RawDeviceId::ZERO`] is actually a zeroed-out version of the raw device id.
+/// - [`RawDeviceId::to_rawid`] stores `offset` in the context/data field of the raw device id so
+/// that buses can recover the pointer to the data.
In his I2C branch Fabien has a patch [1] [2] to remove the
RawDeviceId::to_rawid part above. Maybe it could be aligned that that
patch isn't required any more?
Best regards
Dirk
[1]
https://github.com/Fabo/linux/commit/4b65b8d7ffe07057672b8eb89d376759d67bf060
[2]
From 4b65b8d7ffe07057672b8eb89d376759d67bf060 Mon Sep 17 00:00:00 2001
From: Fabien Parent <fabien.parent@xxxxxxxxxx>
Date: Sun, 28 Apr 2024 11:12:46 -0700
Subject: [PATCH] fixup! rust: add driver abstraction
RawDeviceId::to_rawid is not part of the RawDeviceId trait. Nonetheless
this function must be defined by the type that will implement
RawDeviceId, but to keep `rustdoc` from throwing a warning, let's just
remove it from the docs.
warning: unresolved link to `RawDeviceId::to_rawid`
--> rust/kernel/driver.rs:120:11
|
120 | /// - [`RawDeviceId::to_rawid`] stores `offset` in the
context/data field of the raw device id so
| ^^^^^^^^^^^^^^^^^^^^^ the trait `RawDeviceId` has no
associated item named `to_rawid`
|
= note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default
warning: 1 warning emitted
---
rust/kernel/driver.rs | 2 --
1 file changed, 2 deletions(-)
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index c1258ce0d041af..d141e23224d3db 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -128,8 +128,6 @@ impl<T: DriverOps> Drop for Registration<T> {
///
/// Implementers must ensure that:
/// - [`RawDeviceId::ZERO`] is actually a zeroed-out version of the
raw device id.
-/// - [`RawDeviceId::to_rawid`] stores `offset` in the context/data
field of the raw device id so
-/// that buses can recover the pointer to the data.
pub unsafe trait RawDeviceId {
/// The raw type that holds the device id.
///