From ea7e18289f44b0aa597026f16e7f4f6daa0f13ee Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 19 Dec 2024 18:04:04 +0100 Subject: rust: implement generic driver registration Implement the generic `Registration` type and the `RegistrationOps` trait. The `Registration` structure is the common type that represents a driver registration and is typically bound to the lifetime of a module. However, it doesn't implement actual calls to the kernel's driver core to register drivers itself. Instead the `RegistrationOps` trait is provided to subsystems, which have to implement `RegistrationOps::register` and `RegistrationOps::unregister`. Subsystems have to provide an implementation for both of those methods where the subsystem specific variants to register / unregister a driver have to implemented. For instance, the PCI subsystem would call __pci_register_driver() from `RegistrationOps::register` and pci_unregister_driver() from `DrvierOps::unregister`. Co-developed-by: Wedson Almeida Filho Signed-off-by: Wedson Almeida Filho Signed-off-by: Danilo Krummrich Tested-by: Dirk Behme Tested-by: Fabien Parent Link: https://lore.kernel.org/r/20241219170425.12036-3-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/driver.rs | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 rust/kernel/driver.rs (limited to 'rust/kernel/driver.rs') diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs new file mode 100644 index 000000000000..c1957ee7bb7e --- /dev/null +++ b/rust/kernel/driver.rs @@ -0,0 +1,117 @@ +// 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 [`RegistrationOps`], which allows drivers to +//! register using the [`Registration`] class. + +use crate::error::{Error, Result}; +use crate::{init::PinInit, str::CStr, try_pin_init, types::Opaque, ThisModule}; +use core::pin::Pin; +use macros::{pin_data, pinned_drop}; + +/// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform, +/// Amba, etc.) to provide the corresponding subsystem specific implementation to register / +/// unregister a driver of the particular type (`RegType`). +/// +/// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call +/// `bindings::__pci_register_driver` from `RegistrationOps::register` and +/// `bindings::pci_unregister_driver` from `RegistrationOps::unregister`. +pub trait RegistrationOps { + /// 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. + /// + /// On success, `reg` must remain pinned and valid until the matching call to + /// [`RegistrationOps::unregister`]. + fn register( + reg: &Opaque, + name: &'static CStr, + module: &'static ThisModule, + ) -> Result; + + /// Unregisters a driver previously registered with [`RegistrationOps::register`]. + fn unregister(reg: &Opaque); +} + +/// A [`Registration`] is a generic type that represents the registration of some driver type (e.g. +/// `bindings::pci_driver`). Therefore a [`Registration`] must be initialized with a type that +/// implements the [`RegistrationOps`] trait, such that the generic `T::register` and +/// `T::unregister` calls result in the subsystem specific registration calls. +/// +///Once the `Registration` structure is dropped, the driver is unregistered. +#[pin_data(PinnedDrop)] +pub struct Registration { + #[pin] + reg: Opaque, +} + +// 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 Sync for Registration {} + +// SAFETY: Both registration and unregistration are implemented in C and safe to be performed from +// any thread, so `Registration` is `Send`. +unsafe impl Send for Registration {} + +impl Registration { + /// Creates a new instance of the registration object. + pub fn new(name: &'static CStr, module: &'static ThisModule) -> impl PinInit { + try_pin_init!(Self { + reg <- Opaque::try_ffi_init(|ptr: *mut T::RegType| { + // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write. + unsafe { ptr.write(T::RegType::default()) }; + + // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write, and it has + // just been initialised above, so it's also valid for read. + let drv = unsafe { &*(ptr as *const Opaque) }; + + T::register(drv, name, module) + }), + }) + } +} + +#[pinned_drop] +impl PinnedDrop for Registration { + fn drop(self: Pin<&mut Self>) { + T::unregister(&self.reg); + } +} + +/// Declares a kernel module that exposes a single driver. +/// +/// It is meant to be used as a helper by other subsystems so they can more easily expose their own +/// macros. +#[macro_export] +macro_rules! module_driver { + (<$gen_type:ident>, $driver_ops:ty, { type: $type:ty, $($f:tt)* }) => { + type Ops<$gen_type> = $driver_ops; + + #[$crate::prelude::pin_data] + struct DriverModule { + #[pin] + _driver: $crate::driver::Registration>, + } + + impl $crate::InPlaceModule for DriverModule { + fn init( + module: &'static $crate::ThisModule + ) -> impl $crate::init::PinInit { + $crate::try_pin_init!(Self { + _driver <- $crate::driver::Registration::new( + ::NAME, + module, + ), + }) + } + } + + $crate::prelude::module! { + type: DriverModule, + $($f)* + } + } +} -- cgit v1.2.3 From 7a718a1f26d1697465f0f4e402a69e29d6c4dd33 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 19 Dec 2024 18:04:15 +0100 Subject: rust: driver: implement `Adapter` 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) Signed-off-by: Danilo Krummrich Tested-by: Dirk Behme Link: https://lore.kernel.org/r/20241219170425.12036-14-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/bindings/bindings_helper.h | 1 + rust/kernel/driver.rs | 58 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) (limited to 'rust/kernel/driver.rs') diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 6d7a68e2ecb7..8fe70183a392 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index c1957ee7bb7e..c630e65098ed 100644 --- a/rust/kernel/driver.rs +++ b/rust/kernel/driver.rs @@ -6,7 +6,7 @@ //! register using the [`Registration`] class. use crate::error::{Error, Result}; -use crate::{init::PinInit, str::CStr, try_pin_init, types::Opaque, ThisModule}; +use crate::{device, init::PinInit, of, str::CStr, try_pin_init, types::Opaque, ThisModule}; use core::pin::Pin; use macros::{pin_data, pinned_drop}; @@ -115,3 +115,59 @@ macro_rules! module_driver { } } } + +/// The bus independent adapter to match a drivers and a devices. +/// +/// This trait should be implemented by the bus specific adapter, which represents the connection +/// of a device and a driver. +/// +/// It provides bus independent functions for device / driver interactions. +pub trait Adapter { + /// The type holding driver private data about each device id supported by the driver. + type IdInfo: 'static; + + /// The [`of::IdTable`] of the corresponding driver. + fn of_id_table() -> Option>; + + /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any. + /// + /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`]. + #[cfg(CONFIG_OF)] + fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { + let table = Self::of_id_table()?; + + // SAFETY: + // - `table` has static lifetime, hence it's valid for read, + // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`. + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) }; + + if raw_id.is_null() { + None + } else { + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and + // does not add additional invariants, so it's safe to transmute. + let id = unsafe { &*raw_id.cast::() }; + + Some(table.info(::index(id))) + } + } + + #[cfg(not(CONFIG_OF))] + #[allow(missing_docs)] + fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> { + None + } + + /// Returns the driver's private data from the matching entry of any of the ID tables, if any. + /// + /// If this returns `None`, it means that there is no match in any of the ID tables directly + /// associated with a [`device::Device`]. + fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { + let id = Self::of_id_info(dev); + if id.is_some() { + return id; + } + + None + } +} -- cgit v1.2.3 From e1a51c2bf4b3b20868a0e6e9520b11639bd363f1 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 3 Jan 2025 17:46:03 +0100 Subject: rust: driver: address soundness issue in `RegistrationOps` The `RegistrationOps` trait holds some obligations to the caller and implementers. While being documented, the trait and the corresponding functions haven't been marked as unsafe. Hence, markt the trait and functions unsafe and add the corresponding safety comments. This patch does not include any fuctional changes. Reported-by: Gary Guo Closes: https://lore.kernel.org/rust-for-linux/20241224195821.3b43302b.gary@garyguo.net/ Signed-off-by: Danilo Krummrich Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20250103164655.96590-4-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/driver.rs | 25 ++++++++++++++++++++----- rust/kernel/pci.rs | 8 +++++--- rust/kernel/platform.rs | 8 +++++--- 3 files changed, 30 insertions(+), 11 deletions(-) (limited to 'rust/kernel/driver.rs') diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index c630e65098ed..2a16d5e64e6c 100644 --- a/rust/kernel/driver.rs +++ b/rust/kernel/driver.rs @@ -17,23 +17,35 @@ use macros::{pin_data, pinned_drop}; /// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call /// `bindings::__pci_register_driver` from `RegistrationOps::register` and /// `bindings::pci_unregister_driver` from `RegistrationOps::unregister`. -pub trait RegistrationOps { +/// +/// # Safety +/// +/// A call to [`RegistrationOps::unregister`] for a given instance of `RegType` is only valid if a +/// preceding call to [`RegistrationOps::register`] has been successful. +pub unsafe trait RegistrationOps { /// 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 + /// /// On success, `reg` must remain pinned and valid until the matching call to /// [`RegistrationOps::unregister`]. - fn register( + unsafe fn register( reg: &Opaque, name: &'static CStr, module: &'static ThisModule, ) -> Result; /// Unregisters a driver previously registered with [`RegistrationOps::register`]. - fn unregister(reg: &Opaque); + /// + /// # Safety + /// + /// Must only be called after a preceding successful call to [`RegistrationOps::register`] for + /// the same `reg`. + unsafe fn unregister(reg: &Opaque); } /// A [`Registration`] is a generic type that represents the registration of some driver type (e.g. @@ -68,7 +80,8 @@ impl Registration { // just been initialised above, so it's also valid for read. let drv = unsafe { &*(ptr as *const Opaque) }; - T::register(drv, name, module) + // SAFETY: `drv` is guaranteed to be pinned until `T::unregister`. + unsafe { T::register(drv, name, module) } }), }) } @@ -77,7 +90,9 @@ impl Registration { #[pinned_drop] impl PinnedDrop for Registration { fn drop(self: Pin<&mut Self>) { - T::unregister(&self.reg); + // SAFETY: The existence of `self` guarantees that `self.reg` has previously been + // successfully registered with `T::register` + unsafe { T::unregister(&self.reg) }; } } diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index d5e7f0b15303..4c98b5b9aa1e 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -23,10 +23,12 @@ use kernel::prelude::*; /// An adapter for the registration of PCI drivers. pub struct Adapter(T); -impl driver::RegistrationOps for Adapter { +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if +// a preceding call to `register` has been successful. +unsafe impl driver::RegistrationOps for Adapter { type RegType = bindings::pci_driver; - fn register( + unsafe fn register( pdrv: &Opaque, name: &'static CStr, module: &'static ThisModule, @@ -45,7 +47,7 @@ impl driver::RegistrationOps for Adapter { }) } - fn unregister(pdrv: &Opaque) { + unsafe fn unregister(pdrv: &Opaque) { // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. unsafe { bindings::pci_unregister_driver(pdrv.get()) } } diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 03287794f9d0..50e6b0421813 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -19,10 +19,12 @@ use core::ptr::addr_of_mut; /// An adapter for the registration of platform drivers. pub struct Adapter(T); -impl driver::RegistrationOps for Adapter { +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if +// a preceding call to `register` has been successful. +unsafe impl driver::RegistrationOps for Adapter { type RegType = bindings::platform_driver; - fn register( + unsafe fn register( pdrv: &Opaque, name: &'static CStr, module: &'static ThisModule, @@ -44,7 +46,7 @@ impl driver::RegistrationOps for Adapter { to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.0) }) } - fn unregister(pdrv: &Opaque) { + unsafe fn unregister(pdrv: &Opaque) { // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. unsafe { bindings::platform_driver_unregister(pdrv.get()) }; } -- cgit v1.2.3