Re: [RFC PATCH 02/11] rust: add driver abstraction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux