From e832374ccadf4d1ce7bd40a85b9320bd7fbb3628 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Thu, 29 May 2025 10:10:24 +0200 Subject: rust: pin-init: change blanket impls for `[Pin]Init` and add one for `Result` Remove the error from the blanket implementations `impl Init for T` (and also for `PinInit`). Add implementations for `Result`. This allows one to easily construct (un)conditional failing initializers. It also improves the compatibility with APIs that do not use pin-init, because users can supply a `Result` to a function taking an `impl PinInit`. Suggested-by: Alice Ryhl Link: https://github.com/Rust-for-Linux/pin-init/pull/62/commits/58612514b256c6f4a4a0718be25298410e67387a [ Also fix a compile error in block. - Benno ] Reviewed-by: Boqun Feng Link: https://lore.kernel.org/all/20250529081027.297648-2-lossin@kernel.org [ Add title prefix `rust: pin-init`. - Benno ] Signed-off-by: Benno Lossin --- rust/kernel/block/mq/tag_set.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs index bcf4214ad149..c3cf56d52bee 100644 --- a/rust/kernel/block/mq/tag_set.rs +++ b/rust/kernel/block/mq/tag_set.rs @@ -9,7 +9,7 @@ use core::pin::Pin; use crate::{ bindings, block::mq::{operations::OperationsVTable, request::RequestDataWrapper, Operations}, - error, + error::{self, Result}, prelude::try_pin_init, types::Opaque, }; @@ -41,7 +41,7 @@ impl TagSet { // SAFETY: `blk_mq_tag_set` only contains integers and pointers, which // all are allowed to be 0. let tag_set: bindings::blk_mq_tag_set = unsafe { core::mem::zeroed() }; - let tag_set = core::mem::size_of::() + let tag_set: Result<_> = core::mem::size_of::() .try_into() .map(|cmd_size| { bindings::blk_mq_tag_set { @@ -56,12 +56,14 @@ impl TagSet { nr_maps: num_maps, ..tag_set } - }); + }) + .map(Opaque::new) + .map_err(|e| e.into()); try_pin_init!(TagSet { - inner <- PinInit::<_, error::Error>::pin_chain(Opaque::new(tag_set?), |tag_set| { + inner <- tag_set.pin_chain(|tag_set| { // SAFETY: we do not move out of `tag_set`. - let tag_set = unsafe { Pin::get_unchecked_mut(tag_set) }; + let tag_set: &mut Opaque<_> = unsafe { Pin::get_unchecked_mut(tag_set) }; // SAFETY: `tag_set` is a reference to an initialized `blk_mq_tag_set`. error::to_result( unsafe { bindings::blk_mq_alloc_tag_set(tag_set.get())}) }), -- cgit v1.2.3 From a2801affa7103862d549050401a9f53b3365fca4 Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Wed, 11 Jun 2025 12:29:00 +0200 Subject: rust: device: Create FwNode abstraction for accessing device properties Accessing device properties is currently done via methods on `Device` itself, using bindings to device_property_* functions. This is sufficient for the existing method property_present. However, it's not sufficient for other device properties we want to access. For example, iterating over child nodes of a device will yield a fwnode_handle. That's not a device, so it wouldn't be possible to read the properties of that child node. Thus, we need an abstraction over fwnode_handle and methods for reading its properties. Add a struct FwNode which abstracts over the C struct fwnode_handle. Implement its reference counting analogous to other Rust abstractions over reference-counted C structs. Subsequent patches will add functionality to access FwNode and read properties with it. Tested-by: Dirk Behme Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250611102908.212514-2-remo@buenzli.dev [ Add temporary #[expect(dead_code)] to avoid a warning. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 2 ++ rust/kernel/device/property.rs | 74 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 rust/kernel/device/property.rs (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index dea06b79ecb5..d6237827a936 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -14,6 +14,8 @@ use core::{fmt, marker::PhantomData, ptr}; #[cfg(CONFIG_PRINTK)] use crate::c_str; +pub mod property; + /// A reference-counted device. /// /// This structure represents the Rust abstraction for a C `struct device`. This implementation diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs new file mode 100644 index 000000000000..82ef54b54f18 --- /dev/null +++ b/rust/kernel/device/property.rs @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Unified device property interface. +//! +//! C header: [`include/linux/property.h`](srctree/include/linux/property.h) + +use core::ptr; + +use crate::{ + bindings, + types::{ARef, Opaque}, +}; + +/// A reference-counted fwnode_handle. +/// +/// This structure represents the Rust abstraction for a +/// C `struct fwnode_handle`. This implementation abstracts the usage of an +/// already existing C `struct fwnode_handle` within Rust code that we get +/// passed from the C side. +/// +/// # Invariants +/// +/// A `FwNode` instance represents a valid `struct fwnode_handle` created by the +/// C portion of the kernel. +/// +/// Instances of this type are always reference-counted, that is, a call to +/// `fwnode_handle_get` ensures that the allocation remains valid at least until +/// the matching call to `fwnode_handle_put`. +#[repr(transparent)] +pub struct FwNode(Opaque); + +impl FwNode { + /// # Safety + /// + /// Callers must ensure that: + /// - The reference count was incremented at least once. + /// - They relinquish that increment. That is, if there is only one + /// increment, callers must not use the underlying object anymore -- it is + /// only safe to do so via the newly created `ARef`. + #[expect(dead_code)] + unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef { + // SAFETY: As per the safety requirements of this function: + // - `NonNull::new_unchecked`: + // - `raw` is not null. + // - `ARef::from_raw`: + // - `raw` has an incremented refcount. + // - that increment is relinquished, i.e. it won't be decremented + // elsewhere. + // CAST: It is safe to cast from a `*mut fwnode_handle` to + // `*mut FwNode`, because `FwNode` is defined as a + // `#[repr(transparent)]` wrapper around `fwnode_handle`. + unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(raw.cast())) } + } + + /// Obtain the raw `struct fwnode_handle *`. + pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { + self.0.get() + } +} + +// SAFETY: Instances of `FwNode` are always reference-counted. +unsafe impl crate::types::AlwaysRefCounted for FwNode { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the + // refcount is non-zero. + unsafe { bindings::fwnode_handle_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: ptr::NonNull) { + // SAFETY: The safety requirements guarantee that the refcount is + // non-zero. + unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) } + } +} -- cgit v1.2.3 From 658f23b59251e15cc9263cfe5157d5757a293017 Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Wed, 11 Jun 2025 12:29:01 +0200 Subject: rust: device: Enable accessing the FwNode of a Device Subsequent patches will add methods for reading properties to FwNode. The first step to accessing these methods will be to access the "root" FwNode of a Device. Add the method `fwnode` to `Device`. Tested-by: Dirk Behme Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250611102908.212514-3-remo@buenzli.dev Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index d6237827a936..48d45af1cfb2 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -205,6 +205,21 @@ impl Device { }; } + /// Obtain the [`FwNode`](property::FwNode) corresponding to this [`Device`]. + pub fn fwnode(&self) -> Option<&property::FwNode> { + // SAFETY: `self` is valid. + let fwnode_handle = unsafe { bindings::__dev_fwnode(self.as_raw()) }; + if fwnode_handle.is_null() { + return None; + } + // SAFETY: `fwnode_handle` is valid. Its lifetime is tied to `&self`. We + // return a reference instead of an `ARef` because `dev_fwnode()` + // doesn't increment the refcount. It is safe to cast from a + // `struct fwnode_handle*` to a `*const FwNode` because `FwNode` is + // defined as a `#[repr(transparent)]` wrapper around `fwnode_handle`. + Some(unsafe { &*fwnode_handle.cast() }) + } + /// Checks if property is present or not. pub fn property_present(&self, name: &CStr) -> bool { // SAFETY: By the invariant of `CStr`, `name` is null-terminated. -- cgit v1.2.3 From d3393e845038f5fd32c24b841bb4b6026aa1cf4b Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Wed, 11 Jun 2025 12:29:02 +0200 Subject: rust: device: Move property_present() to FwNode The new FwNode abstraction will be used for accessing all device properties. It would be possible to duplicate the methods on the device itself, but since some of the methods on Device would have different type sigatures as the ones on FwNode, this would only lead to inconsistency and confusion. For this reason, property_present is removed from Device and existing users are updated. Acked-by: Viresh Kumar Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250611102908.212514-4-remo@buenzli.dev Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 7 ------- rust/kernel/device/property.rs | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 48d45af1cfb2..665f5ceadecc 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -6,7 +6,6 @@ use crate::{ bindings, - str::CStr, types::{ARef, Opaque}, }; use core::{fmt, marker::PhantomData, ptr}; @@ -219,12 +218,6 @@ impl Device { // defined as a `#[repr(transparent)]` wrapper around `fwnode_handle`. Some(unsafe { &*fwnode_handle.cast() }) } - - /// Checks if property is present or not. - pub fn property_present(&self, name: &CStr) -> bool { - // SAFETY: By the invariant of `CStr`, `name` is null-terminated. - unsafe { bindings::device_property_present(self.as_raw().cast_const(), name.as_char_ptr()) } - } } // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 82ef54b54f18..ed17b20f7ae1 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -8,6 +8,7 @@ use core::ptr; use crate::{ bindings, + str::CStr, types::{ARef, Opaque}, }; @@ -56,6 +57,12 @@ impl FwNode { pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0.get() } + + /// Checks if property is present or not. + pub fn property_present(&self, name: &CStr) -> bool { + // SAFETY: By the invariant of `CStr`, `name` is null-terminated. + unsafe { bindings::fwnode_property_present(self.as_raw().cast_const(), name.as_char_ptr()) } + } } // SAFETY: Instances of `FwNode` are always reference-counted. -- cgit v1.2.3 From ecea2459818383c2886ec1cff81cce7e70d99893 Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Wed, 11 Jun 2025 12:29:03 +0200 Subject: rust: device: Enable printing fwnode name and path Add two new public methods `display_name` and `display_path` to `FwNode`. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property. Tested-by: Dirk Behme Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250611102908.212514-5-remo@buenzli.dev [ Remove #[expect(dead_code)] from FwNode::from_raw(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device/property.rs | 77 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index ed17b20f7ae1..4cac335bad78 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -38,7 +38,6 @@ impl FwNode { /// - They relinquish that increment. That is, if there is only one /// increment, callers must not use the underlying object anymore -- it is /// only safe to do so via the newly created `ARef`. - #[expect(dead_code)] unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef { // SAFETY: As per the safety requirements of this function: // - `NonNull::new_unchecked`: @@ -58,6 +57,32 @@ impl FwNode { self.0.get() } + /// Returns an object that implements [`Display`](core::fmt::Display) for + /// printing the name of a node. + /// + /// This is an alternative to the default `Display` implementation, which + /// prints the full path. + pub fn display_name(&self) -> impl core::fmt::Display + '_ { + struct FwNodeDisplayName<'a>(&'a FwNode); + + impl core::fmt::Display for FwNodeDisplayName<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + // SAFETY: `self` is valid by its type invariant. + let name = unsafe { bindings::fwnode_get_name(self.0.as_raw()) }; + if name.is_null() { + return Ok(()); + } + // SAFETY: + // - `fwnode_get_name` returns null or a valid C string. + // - `name` was checked to be non-null. + let name = unsafe { CStr::from_char_ptr(name) }; + write!(f, "{name}") + } + } + + FwNodeDisplayName(self) + } + /// Checks if property is present or not. pub fn property_present(&self, name: &CStr) -> bool { // SAFETY: By the invariant of `CStr`, `name` is null-terminated. @@ -79,3 +104,53 @@ unsafe impl crate::types::AlwaysRefCounted for FwNode { unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) } } } + +enum Node<'a> { + Borrowed(&'a FwNode), + Owned(ARef), +} + +impl core::fmt::Display for FwNode { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + // The logic here is the same as the one in lib/vsprintf.c + // (fwnode_full_name_string). + + // SAFETY: `self.as_raw()` is valid by its type invariant. + let num_parents = unsafe { bindings::fwnode_count_parents(self.as_raw()) }; + + for depth in (0..=num_parents).rev() { + let fwnode = if depth == 0 { + Node::Borrowed(self) + } else { + // SAFETY: `self.as_raw()` is valid. + let ptr = unsafe { bindings::fwnode_get_nth_parent(self.as_raw(), depth) }; + // SAFETY: + // - The depth passed to `fwnode_get_nth_parent` is + // within the valid range, so the returned pointer is + // not null. + // - The reference count was incremented by + // `fwnode_get_nth_parent`. + // - That increment is relinquished to + // `FwNode::from_raw`. + Node::Owned(unsafe { FwNode::from_raw(ptr) }) + }; + // Take a reference to the owned or borrowed `FwNode`. + let fwnode: &FwNode = match &fwnode { + Node::Borrowed(f) => f, + Node::Owned(f) => f, + }; + + // SAFETY: `fwnode` is valid by its type invariant. + let prefix = unsafe { bindings::fwnode_get_name_prefix(fwnode.as_raw()) }; + if !prefix.is_null() { + // SAFETY: `fwnode_get_name_prefix` returns null or a + // valid C string. + let prefix = unsafe { CStr::from_char_ptr(prefix) }; + write!(f, "{prefix}")?; + } + write!(f, "{}", fwnode.display_name())?; + } + + Ok(()) + } +} -- cgit v1.2.3 From 9bd791d9413b4b65e203c4ff84c8b8b2c8c3b770 Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Wed, 11 Jun 2025 12:29:04 +0200 Subject: rust: device: Introduce PropertyGuard This abstraction is a way to force users to specify whether a property is supposed to be required or not. This allows us to move error logging of missing required properties into core, preventing a lot of boilerplate in drivers. It will be used by upcoming methods for reading device properties. Tested-by: Dirk Behme Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250611102908.212514-6-remo@buenzli.dev [ Use prelude::* to avoid build failure; move PropertyGuard below Display impl of FwNode. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device/property.rs | 60 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 4cac335bad78..2c9482578987 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -8,6 +8,7 @@ use core::ptr; use crate::{ bindings, + prelude::*, str::CStr, types::{ARef, Opaque}, }; @@ -154,3 +155,62 @@ impl core::fmt::Display for FwNode { Ok(()) } } + +/// A helper for reading device properties. +/// +/// Use [`Self::required_by`] if a missing property is considered a bug and +/// [`Self::optional`] otherwise. +/// +/// For convenience, [`Self::or`] and [`Self::or_default`] are provided. +pub struct PropertyGuard<'fwnode, 'name, T> { + /// The result of reading the property. + inner: Result, + /// The fwnode of the property, used for logging in the "required" case. + fwnode: &'fwnode FwNode, + /// The name of the property, used for logging in the "required" case. + name: &'name CStr, +} + +impl PropertyGuard<'_, '_, T> { + /// Access the property, indicating it is required. + /// + /// If the property is not present, the error is automatically logged. If a + /// missing property is not an error, use [`Self::optional`] instead. The + /// device is required to associate the log with it. + pub fn required_by(self, dev: &super::Device) -> Result { + if self.inner.is_err() { + dev_err!( + dev, + "{}: property '{}' is missing\n", + self.fwnode, + self.name + ); + } + self.inner + } + + /// Access the property, indicating it is optional. + /// + /// In contrast to [`Self::required_by`], no error message is logged if + /// the property is not present. + pub fn optional(self) -> Option { + self.inner.ok() + } + + /// Access the property or the specified default value. + /// + /// Do not pass a sentinel value as default to detect a missing property. + /// Use [`Self::required_by`] or [`Self::optional`] instead. + pub fn or(self, default: T) -> T { + self.inner.unwrap_or(default) + } +} + +impl PropertyGuard<'_, '_, T> { + /// Access the property or a default value. + /// + /// Use [`Self::or`] to specify a custom default value. + pub fn or_default(self) -> T { + self.inner.unwrap_or_default() + } +} -- cgit v1.2.3 From 2db611374cef12bd793b72d5728f0ecd1affeb17 Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Wed, 11 Jun 2025 12:29:05 +0200 Subject: rust: device: Implement accessors for firmware properties Add methods to FwNode for reading several firmware property types like strings, integers and arrays. Most types are read with the generic `property_read` method. There are two exceptions: * `property_read_bool` cannot fail, so the fallible function signature of `property_read` would not make sense for reading booleans. * `property_read_array_vec` can fail because of a dynamic memory allocation. This error must be handled separately, leading to a different function signature than `property_read`. The traits `Property` and `PropertyInt` drive the generic behavior of `property_read`. `PropertyInt` is necessary to associate specific integer types with the C functions to read them. While there is a C function to read integers of generic sizes called `fwnode_property_read_int_array`, it was preferred not to make this public. Tested-by: Dirk Behme Co-developed-by: Rob Herring (Arm) Signed-off-by: Rob Herring (Arm) Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250611102908.212514-7-remo@buenzli.dev [ Properly include kernel::device::private::Sealed; add explicit type annotations for core::mem::transmute(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device/property.rs | 254 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 252 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 2c9482578987..838509111e57 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -4,12 +4,15 @@ //! //! C header: [`include/linux/property.h`](srctree/include/linux/property.h) -use core::ptr; +use core::{mem::MaybeUninit, ptr}; +use super::private::Sealed; use crate::{ + alloc::KVec, bindings, + error::{to_result, Result}, prelude::*, - str::CStr, + str::{CStr, CString}, types::{ARef, Opaque}, }; @@ -89,6 +92,104 @@ impl FwNode { // SAFETY: By the invariant of `CStr`, `name` is null-terminated. unsafe { bindings::fwnode_property_present(self.as_raw().cast_const(), name.as_char_ptr()) } } + + /// Returns firmware property `name` boolean value. + pub fn property_read_bool(&self, name: &CStr) -> bool { + // SAFETY: + // - `name` is non-null and null-terminated. + // - `self.as_raw()` is valid because `self` is valid. + unsafe { bindings::fwnode_property_read_bool(self.as_raw(), name.as_char_ptr()) } + } + + /// Returns the index of matching string `match_str` for firmware string + /// property `name`. + pub fn property_match_string(&self, name: &CStr, match_str: &CStr) -> Result { + // SAFETY: + // - `name` and `match_str` are non-null and null-terminated. + // - `self.as_raw` is valid because `self` is valid. + let ret = unsafe { + bindings::fwnode_property_match_string( + self.as_raw(), + name.as_char_ptr(), + match_str.as_char_ptr(), + ) + }; + to_result(ret)?; + Ok(ret as usize) + } + + /// Returns firmware property `name` integer array values in a [`KVec`]. + pub fn property_read_array_vec<'fwnode, 'name, T: PropertyInt>( + &'fwnode self, + name: &'name CStr, + len: usize, + ) -> Result>> { + let mut val: KVec = KVec::with_capacity(len, GFP_KERNEL)?; + + let res = T::read_array_from_fwnode_property(self, name, val.spare_capacity_mut()); + let res = match res { + Ok(_) => { + // SAFETY: + // - `len` is equal to `val.capacity - val.len`, because + // `val.capacity` is `len` and `val.len` is zero. + // - All elements within the interval [`0`, `len`) were initialized + // by `read_array_from_fwnode_property`. + unsafe { val.inc_len(len) } + Ok(val) + } + Err(e) => Err(e), + }; + Ok(PropertyGuard { + inner: res, + fwnode: self, + name, + }) + } + + /// Returns integer array length for firmware property `name`. + pub fn property_count_elem(&self, name: &CStr) -> Result { + T::read_array_len_from_fwnode_property(self, name) + } + + /// Returns the value of firmware property `name`. + /// + /// This method is generic over the type of value to read. The types that + /// can be read are strings, integers and arrays of integers. + /// + /// Reading a [`KVec`] of integers is done with the separate + /// method [`Self::property_read_array_vec`], because it takes an + /// additional `len` argument. + /// + /// Reading a boolean is done with the separate method + /// [`Self::property_read_bool`], because this operation is infallible. + /// + /// For more precise documentation about what types can be read, see + /// the [implementors of Property][Property#implementors] and [its + /// implementations on foreign types][Property#foreign-impls]. + /// + /// # Examples + /// + /// ``` + /// # use kernel::{c_str, device::{Device, property::FwNode}, str::CString}; + /// fn examples(dev: &Device) -> Result { + /// let fwnode = dev.fwnode().ok_or(ENOENT)?; + /// let b: u32 = fwnode.property_read(c_str!("some-number")).required_by(dev)?; + /// if let Some(s) = fwnode.property_read::(c_str!("some-str")).optional() { + /// // ... + /// } + /// Ok(()) + /// } + /// ``` + pub fn property_read<'fwnode, 'name, T: Property>( + &'fwnode self, + name: &'name CStr, + ) -> PropertyGuard<'fwnode, 'name, T> { + PropertyGuard { + inner: T::read_from_fwnode_property(self, name), + fwnode: self, + name, + } + } } // SAFETY: Instances of `FwNode` are always reference-counted. @@ -156,6 +257,155 @@ impl core::fmt::Display for FwNode { } } +/// Implemented for types that can be read as properties. +/// +/// This is implemented for strings, integers and arrays of integers. It's used +/// to make [`FwNode::property_read`] generic over the type of property being +/// read. There are also two dedicated methods to read other types, because they +/// require more specialized function signatures: +/// - [`property_read_bool`](FwNode::property_read_bool) +/// - [`property_read_array_vec`](FwNode::property_read_array_vec) +/// +/// It must be public, because it appears in the signatures of other public +/// functions, but its methods shouldn't be used outside the kernel crate. +pub trait Property: Sized + Sealed { + /// Used to make [`FwNode::property_read`] generic. + fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result; +} + +impl Sealed for CString {} + +impl Property for CString { + fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result { + let mut str: *mut u8 = ptr::null_mut(); + let pstr: *mut _ = &mut str; + + // SAFETY: + // - `name` is non-null and null-terminated. + // - `fwnode.as_raw` is valid because `fwnode` is valid. + let ret = unsafe { + bindings::fwnode_property_read_string(fwnode.as_raw(), name.as_char_ptr(), pstr.cast()) + }; + to_result(ret)?; + + // SAFETY: + // - `pstr` is a valid pointer to a NUL-terminated C string. + // - It is valid for at least as long as `fwnode`, but it's only used + // within the current function. + // - The memory it points to is not mutated during that time. + let str = unsafe { CStr::from_char_ptr(*pstr) }; + Ok(str.try_into()?) + } +} + +/// Implemented for all integers that can be read as properties. +/// +/// This helper trait is needed on top of the existing [`Property`] +/// trait to associate the integer types of various sizes with their +/// corresponding `fwnode_property_read_*_array` functions. +/// +/// It must be public, because it appears in the signatures of other public +/// functions, but its methods shouldn't be used outside the kernel crate. +pub trait PropertyInt: Copy + Sealed { + /// Reads a property array. + fn read_array_from_fwnode_property<'a>( + fwnode: &FwNode, + name: &CStr, + out: &'a mut [MaybeUninit], + ) -> Result<&'a mut [Self]>; + + /// Reads the length of a property array. + fn read_array_len_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result; +} +// This macro generates implementations of the traits `Property` and +// `PropertyInt` for integers of various sizes. Its input is a list +// of pairs separated by commas. The first element of the pair is the +// type of the integer, the second one is the name of its corresponding +// `fwnode_property_read_*_array` function. +macro_rules! impl_property_for_int { + ($($int:ty: $f:ident),* $(,)?) => { $( + impl Sealed for $int {} + impl Sealed for [$int; N] {} + + impl PropertyInt for $int { + fn read_array_from_fwnode_property<'a>( + fwnode: &FwNode, + name: &CStr, + out: &'a mut [MaybeUninit], + ) -> Result<&'a mut [Self]> { + // SAFETY: + // - `fwnode`, `name` and `out` are all valid by their type + // invariants. + // - `out.len()` is a valid bound for the memory pointed to by + // `out.as_mut_ptr()`. + // CAST: It's ok to cast from `*mut MaybeUninit<$int>` to a + // `*mut $int` because they have the same memory layout. + let ret = unsafe { + bindings::$f( + fwnode.as_raw(), + name.as_char_ptr(), + out.as_mut_ptr().cast(), + out.len(), + ) + }; + to_result(ret)?; + // SAFETY: Transmuting from `&'a mut [MaybeUninit]` to + // `&'a mut [Self]` is sound, because the previous call to a + // `fwnode_property_read_*_array` function (which didn't fail) + // fully initialized the slice. + Ok(unsafe { core::mem::transmute::<&mut [MaybeUninit], &mut [Self]>(out) }) + } + + fn read_array_len_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result { + // SAFETY: + // - `fwnode` and `name` are valid by their type invariants. + // - It's ok to pass a null pointer to the + // `fwnode_property_read_*_array` functions if `nval` is zero. + // This will return the length of the array. + let ret = unsafe { + bindings::$f( + fwnode.as_raw(), + name.as_char_ptr(), + ptr::null_mut(), + 0, + ) + }; + to_result(ret)?; + Ok(ret as usize) + } + } + + impl Property for $int { + fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result { + let val: [_; 1] = <[$int; 1]>::read_from_fwnode_property(fwnode, name)?; + Ok(val[0]) + } + } + + impl Property for [$int; N] { + fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result { + let mut val: [MaybeUninit<$int>; N] = [const { MaybeUninit::uninit() }; N]; + + <$int>::read_array_from_fwnode_property(fwnode, name, &mut val)?; + + // SAFETY: `val` is always initialized when + // `fwnode_property_read_*_array` is successful. + Ok(val.map(|v| unsafe { v.assume_init() })) + } + } + )* }; +} +impl_property_for_int! { + u8: fwnode_property_read_u8_array, + u16: fwnode_property_read_u16_array, + u32: fwnode_property_read_u32_array, + u64: fwnode_property_read_u64_array, + i8: fwnode_property_read_u8_array, + i16: fwnode_property_read_u16_array, + i32: fwnode_property_read_u32_array, + i64: fwnode_property_read_u64_array, +} + /// A helper for reading device properties. /// /// Use [`Self::required_by`] if a missing property is considered a bug and -- cgit v1.2.3 From c942dba38064cd35214c6b3249120f3f2945e810 Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Mon, 16 Jun 2025 17:45:09 +0200 Subject: rust: device: Add child accessor and iterator Allow Rust drivers to access children of a fwnode either by name or by iterating over all of them. In C, there is the function `fwnode_get_next_child_node` for iteration and the macro `fwnode_for_each_child_node` that helps with handling the pointers. Instead of a macro, a native iterator is used in Rust such that regular for-loops can be used. Tested-by: Dirk Behme Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250616154511.1862909-2-remo@buenzli.dev Signed-off-by: Danilo Krummrich --- rust/kernel/device/property.rs | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 838509111e57..04a13d05785a 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -190,6 +190,62 @@ impl FwNode { name, } } + + /// Returns first matching named child node handle. + pub fn get_child_by_name(&self, name: &CStr) -> Option> { + // SAFETY: `self` and `name` are valid by their type invariants. + let child = + unsafe { bindings::fwnode_get_named_child_node(self.as_raw(), name.as_char_ptr()) }; + if child.is_null() { + return None; + } + // SAFETY: + // - `fwnode_get_named_child_node` returns a pointer with its refcount + // incremented. + // - That increment is relinquished, i.e. the underlying object is not + // used anymore except via the newly created `ARef`. + Some(unsafe { Self::from_raw(child) }) + } + + /// Returns an iterator over a node's children. + pub fn children<'a>(&'a self) -> impl Iterator> + 'a { + let mut prev: Option> = None; + + core::iter::from_fn(move || { + let prev_ptr = match prev.take() { + None => ptr::null_mut(), + Some(prev) => { + // We will pass `prev` to `fwnode_get_next_child_node`, + // which decrements its refcount, so we use + // `ARef::into_raw` to avoid decrementing the refcount + // twice. + let prev = ARef::into_raw(prev); + prev.as_ptr().cast() + } + }; + // SAFETY: + // - `self.as_raw()` is valid by its type invariant. + // - `prev_ptr` may be null, which is allowed and corresponds to + // getting the first child. Otherwise, `prev_ptr` is valid, as it + // is the stored return value from the previous invocation. + // - `prev_ptr` has its refount incremented. + // - The increment of `prev_ptr` is relinquished, i.e. the + // underlying object won't be used anymore. + let next = unsafe { bindings::fwnode_get_next_child_node(self.as_raw(), prev_ptr) }; + if next.is_null() { + return None; + } + // SAFETY: + // - `next` is valid because `fwnode_get_next_child_node` returns a + // pointer with its refcount incremented. + // - That increment is relinquished, i.e. the underlying object + // won't be used anymore, except via the newly created + // `ARef`. + let next = unsafe { FwNode::from_raw(next) }; + prev = Some(next.clone()); + Some(next) + }) + } } // SAFETY: Instances of `FwNode` are always reference-counted. -- cgit v1.2.3 From c3e05bd15e0c99f3ff45e0b1f01814778bc1128c Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Mon, 16 Jun 2025 17:45:10 +0200 Subject: rust: device: Add property_get_reference_args Allow Rust code to read reference args from device properties. The wrapper type `FwNodeReferenceArgs` allows callers to access the buffer of read args safely. Signed-off-by: Remo Senekowitsch Link: https://lore.kernel.org/r/20250616154511.1862909-3-remo@buenzli.dev [ Move up NArgs; refer to FwNodeReferenceArgs in NArgs doc-comment. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device/property.rs | 102 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 04a13d05785a..2f6f3ef17db7 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -246,6 +246,108 @@ impl FwNode { Some(next) }) } + + /// Finds a reference with arguments. + pub fn property_get_reference_args( + &self, + prop: &CStr, + nargs: NArgs<'_>, + index: u32, + ) -> Result { + let mut out_args = FwNodeReferenceArgs::default(); + + let (nargs_prop, nargs) = match nargs { + NArgs::Prop(nargs_prop) => (nargs_prop.as_char_ptr(), 0), + NArgs::N(nargs) => (ptr::null(), nargs), + }; + + // SAFETY: + // - `self.0.get()` is valid. + // - `prop.as_char_ptr()` is valid and zero-terminated. + // - `nargs_prop` is valid and zero-terminated if `nargs` + // is zero, otherwise it is allowed to be a null-pointer. + // - The function upholds the type invariants of `out_args`, + // namely: + // - It may fill the field `fwnode` with a valid pointer, + // in which case its refcount is incremented. + // - It may modify the field `nargs`, in which case it + // initializes at least as many elements in `args`. + let ret = unsafe { + bindings::fwnode_property_get_reference_args( + self.0.get(), + prop.as_char_ptr(), + nargs_prop, + nargs, + index, + &mut out_args.0, + ) + }; + to_result(ret)?; + + Ok(out_args) + } +} + +/// The number of arguments to request [`FwNodeReferenceArgs`]. +pub enum NArgs<'a> { + /// The name of the property of the reference indicating the number of + /// arguments. + Prop(&'a CStr), + /// The known number of arguments. + N(u32), +} + +/// The return value of [`FwNode::property_get_reference_args`]. +/// +/// This structure represents the Rust abstraction for a C +/// `struct fwnode_reference_args` which was initialized by the C side. +/// +/// # Invariants +/// +/// If the field `fwnode` is valid, it owns an increment of its refcount. +/// +/// The field `args` contains at least as many initialized elements as indicated +/// by the field `nargs`. +#[repr(transparent)] +#[derive(Default)] +pub struct FwNodeReferenceArgs(bindings::fwnode_reference_args); + +impl Drop for FwNodeReferenceArgs { + fn drop(&mut self) { + if !self.0.fwnode.is_null() { + // SAFETY: + // - By the type invariants of `FwNodeReferenceArgs`, its field + // `fwnode` owns an increment of its refcount. + // - That increment is relinquished. The underlying object won't be + // used anymore because we are dropping it. + let _ = unsafe { FwNode::from_raw(self.0.fwnode) }; + } + } +} + +impl FwNodeReferenceArgs { + /// Returns the slice of reference arguments. + pub fn as_slice(&self) -> &[u64] { + // SAFETY: As per the safety invariant of `FwNodeReferenceArgs`, `nargs` + // is the minimum number of elements in `args` that is valid. + unsafe { core::slice::from_raw_parts(self.0.args.as_ptr(), self.0.nargs as usize) } + } + + /// Returns the number of reference arguments. + pub fn len(&self) -> usize { + self.0.nargs as usize + } + + /// Returns `true` if there are no reference arguments. + pub fn is_empty(&self) -> bool { + self.0.nargs == 0 + } +} + +impl core::fmt::Debug for FwNodeReferenceArgs { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{:?}", self.as_slice()) + } } // SAFETY: Instances of `FwNode` are always reference-counted. -- cgit v1.2.3 From 56a789f776f24e6b132ec00d4c27672ed4e2ec57 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 20 Jun 2025 16:15:04 +0100 Subject: rust: device: implement FwNode::is_of_node() Implement FwNode::is_of_node() in order to check whether a FwNode instance is embedded in a struct device_node. Reviewed-by: Rob Herring (Arm) Signed-off-by: Igor Korotin Link: https://lore.kernel.org/r/20250620151504.278766-1-igor.korotin.linux@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/device/property.rs | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 2f6f3ef17db7..49ee12a906db 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -61,6 +61,13 @@ impl FwNode { self.0.get() } + /// Returns `true` if `&self` is an OF node, `false` otherwise. + pub fn is_of_node(&self) -> bool { + // SAFETY: The type invariant of `Self` guarantees that `self.as_raw() is a pointer to a + // valid `struct fwnode_handle`. + unsafe { bindings::is_of_node(self.as_raw()) } + } + /// Returns an object that implements [`Display`](core::fmt::Display) for /// printing the name of a node. /// -- cgit v1.2.3 From a74931eb59cf6b914e1b9471d9abd4429176f6af Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Fri, 20 Jun 2025 16:24:25 +0100 Subject: rust: acpi: add `acpi::DeviceId` abstraction `acpi::DeviceId` is an abstraction around `struct acpi_device_id`. Enable drivers to build ACPI device ID tables, to be consumed by the corresponding bus abstractions, such as platform or I2C. Signed-off-by: Igor Korotin Acked-by: Rafael J. Wysocki Link: https://lore.kernel.org/r/20250620152425.285683-1-igor.korotin.linux@gmail.com [ Always inline DeviceId::new() and use &'static CStr; slightly reword commit message. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/acpi.rs | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/lib.rs | 1 + 2 files changed, 66 insertions(+) create mode 100644 rust/kernel/acpi.rs (limited to 'rust/kernel') diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs new file mode 100644 index 000000000000..eaae3bc8e54c --- /dev/null +++ b/rust/kernel/acpi.rs @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Advanced Configuration and Power Interface abstractions. + +use crate::{bindings, device_id::RawDeviceId, prelude::*}; + +/// IdTable type for ACPI drivers. +pub type IdTable = &'static dyn kernel::device_id::IdTable; + +/// An ACPI device id. +#[repr(transparent)] +#[derive(Clone, Copy)] +pub struct DeviceId(bindings::acpi_device_id); + +// SAFETY: +// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct acpi_device_id` and does not add +// additional invariants, so it's safe to transmute to `RawType`. +// * `DRIVER_DATA_OFFSET` is the offset to the `data` field. +unsafe impl RawDeviceId for DeviceId { + type RawType = bindings::acpi_device_id; + + const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data); + + fn index(&self) -> usize { + self.0.driver_data as _ + } +} + +impl DeviceId { + const ACPI_ID_LEN: usize = 16; + + /// Create a new device id from an ACPI 'id' string. + #[inline(always)] + pub const fn new(id: &'static CStr) -> Self { + build_assert!( + id.len_with_nul() <= Self::ACPI_ID_LEN, + "ID exceeds 16 bytes" + ); + let src = id.as_bytes_with_nul(); + // Replace with `bindings::acpi_device_id::default()` once stabilized for `const`. + // SAFETY: FFI type is valid to be zero-initialized. + let mut acpi: bindings::acpi_device_id = unsafe { core::mem::zeroed() }; + let mut i = 0; + while i < src.len() { + acpi.id[i] = src[i]; + i += 1; + } + + Self(acpi) + } +} + +/// Create an ACPI `IdTable` with an "alias" for modpost. +#[macro_export] +macro_rules! acpi_device_table { + ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => { + const $table_name: $crate::device_id::IdArray< + $crate::acpi::DeviceId, + $id_info_type, + { $table_data.len() }, + > = $crate::device_id::IdArray::new($table_data); + + $crate::module_device_table!("acpi", $module_table_name, $table_name); + }; +} diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 6b4774b2b1c3..5bbf3627212f 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -51,6 +51,7 @@ extern crate self as kernel; pub use ffi; +pub mod acpi; pub mod alloc; #[cfg(CONFIG_AUXILIARY_BUS)] pub mod auxiliary; -- cgit v1.2.3 From 0f549d25858dfef9decd0b99a82ff4bc9095a51f Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Fri, 20 Jun 2025 16:36:56 +0100 Subject: rust: driver: Consolidate `Adapter::of_id_info` methods using `#[cfg]` Refactor the `of_id_info` methods in the `Adapter` trait to reduce duplication. Previously, the method had two versions selected via `#[cfg(...)]` and `#[cfg(not(...))]`. This change merges them into a single method by using `#[cfg]` blocks within the method body. Suggested-by: Benno Lossin Signed-off-by: Igor Korotin Link: https://lore.kernel.org/r/20250620153656.294468-1-igor.korotin.linux@gmail.com [ Fix clippy warning if #[cfg(not(CONFIG_OF))]; fix checkpatch.pl line length warnings. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/driver.rs | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index ec9166cedfa7..b072b88e7cc1 100644 --- a/rust/kernel/driver.rs +++ b/rust/kernel/driver.rs @@ -147,30 +147,32 @@ pub trait Adapter { /// 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()?; + #[cfg(not(CONFIG_OF))] + { + let _ = dev; + None + } - // 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()) }; + #[cfg(CONFIG_OF)] + { + let table = Self::of_id_table()?; - 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::() }; + // SAFETY: + // - `table` has static lifetime, hence it's valid for read, + // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`. + let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) }; - Some(table.info(::index(id))) - } - } + 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::() }; - #[cfg(not(CONFIG_OF))] - #[allow(missing_docs)] - fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> { - None + Some(table.info(::index(id))) + } + } } /// Returns the driver's private data from the matching entry of any of the ID tables, if any. -- cgit v1.2.3 From 7a5cb145a9ce844be41ca5ed26e7d8d7c41dec7d Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Fri, 20 Jun 2025 16:39:13 +0100 Subject: rust: driver: Add ACPI id table support to Adapter trait Extend the `Adapter` trait to support ACPI device identification. This mirrors the existing Open Firmware (OF) support (`of_id_table`) and enables Rust drivers to match and retrieve ACPI-specific device data when `CONFIG_ACPI` is enabled. To avoid breaking compilation, a stub implementation of `acpi_id_table()` is added to the Platform adapter; the full implementation will be provided in a subsequent patch. Signed-off-by: Igor Korotin Link: https://lore.kernel.org/r/20250620153914.295679-1-igor.korotin.linux@gmail.com [ Fix clippy warning if #[cfg(not(CONFIG_OF))]; fix checkpatch.pl line length warnings. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/driver.rs | 41 ++++++++++++++++++++++++++++++++++++++++- rust/kernel/platform.rs | 6 +++++- 2 files changed, 45 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index b072b88e7cc1..a4bf4498e592 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::{device, of, str::CStr, try_pin_init, types::Opaque, ThisModule}; +use crate::{acpi, device, of, str::CStr, try_pin_init, types::Opaque, ThisModule}; use core::pin::Pin; use pin_init::{pin_data, pinned_drop, PinInit}; @@ -141,6 +141,40 @@ pub trait Adapter { /// The type holding driver private data about each device id supported by the driver. type IdInfo: 'static; + /// The [`acpi::IdTable`] of the corresponding driver + fn acpi_id_table() -> Option>; + + /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any. + /// + /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`]. + fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { + #[cfg(not(CONFIG_ACPI))] + { + let _ = dev; + None + } + + #[cfg(CONFIG_ACPI)] + { + let table = Self::acpi_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 `dev.as_raw()`. + let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) }; + + if raw_id.is_null() { + None + } else { + // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct acpi_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))) + } + } + } + /// The [`of::IdTable`] of the corresponding driver. fn of_id_table() -> Option>; @@ -180,6 +214,11 @@ pub trait Adapter { /// 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::acpi_id_info(dev); + if id.is_some() { + return id; + } + let id = Self::of_id_info(dev); if id.is_some() { return id; diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 5b21fa517e55..5923d29a0511 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) use crate::{ - bindings, container_of, device, driver, + acpi, bindings, container_of, device, driver, error::{to_result, Result}, of, prelude::*, @@ -94,6 +94,10 @@ impl driver::Adapter for Adapter { fn of_id_table() -> Option> { T::OF_ID_TABLE } + + fn acpi_id_table() -> Option> { + None + } } /// Declares a kernel module that exposes a single platform driver. -- cgit v1.2.3 From ec3ef2175e16360605c7e1b409ceaa77be6521a8 Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Fri, 20 Jun 2025 16:41:24 +0100 Subject: rust: platform: Set `OF_ID_TABLE` default to `None` in `Driver` trait Provide a default value of `None` for `Driver::OF_ID_TABLE` to simplify driver implementations. Drivers that do not require OpenFirmware matching no longer need to import the `of` module or define the constant explicitly. This reduces unnecessary boilerplate and avoids pulling in unused dependencies. Signed-off-by: Igor Korotin Link: https://lore.kernel.org/r/20250620154124.297158-1-igor.korotin.linux@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 5923d29a0511..2436f55b579b 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -162,7 +162,7 @@ pub trait Driver: Send { type IdInfo: 'static; /// The table of OF device ids supported by the driver. - const OF_ID_TABLE: Option>; + const OF_ID_TABLE: Option> = None; /// Platform driver probe. /// -- cgit v1.2.3 From 8411e6f06a022679a642c236724944057b90e60e Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Fri, 20 Jun 2025 16:43:34 +0100 Subject: rust: platform: Add ACPI match table support to `Driver` trait Extend the `platform::Driver` trait to support ACPI device matching by adding the `ACPI_ID_TABLE` constant. This allows Rust platform drivers to define ACPI match tables alongside their existing OF match tables. These changes mirror the existing OF support and allow Rust platform drivers to match devices based on ACPI identifiers. Signed-off-by: Igor Korotin Link: https://lore.kernel.org/r/20250620154334.298320-1-igor.korotin.linux@gmail.com [ Use 'LNUXBEEF' as ACPI ID. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 2436f55b579b..86f9d73c64b3 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -37,12 +37,18 @@ unsafe impl driver::RegistrationOps for Adapter { None => core::ptr::null(), }; + let acpi_table = match T::ACPI_ID_TABLE { + Some(table) => table.as_ptr(), + None => core::ptr::null(), + }; + // SAFETY: It's safe to set the fields of `struct platform_driver` on initialization. unsafe { (*pdrv.get()).driver.name = name.as_char_ptr(); (*pdrv.get()).probe = Some(Self::probe_callback); (*pdrv.get()).remove = Some(Self::remove_callback); (*pdrv.get()).driver.of_match_table = of_table; + (*pdrv.get()).driver.acpi_match_table = acpi_table; } // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. @@ -96,7 +102,7 @@ impl driver::Adapter for Adapter { } fn acpi_id_table() -> Option> { - None + T::ACPI_ID_TABLE } } @@ -127,7 +133,7 @@ macro_rules! module_platform_driver { /// # Example /// ///``` -/// # use kernel::{bindings, c_str, device::Core, of, platform}; +/// # use kernel::{acpi, bindings, c_str, device::Core, of, platform}; /// /// struct MyDriver; /// @@ -140,9 +146,19 @@ macro_rules! module_platform_driver { /// ] /// ); /// +/// kernel::acpi_device_table!( +/// ACPI_TABLE, +/// MODULE_ACPI_TABLE, +/// ::IdInfo, +/// [ +/// (acpi::DeviceId::new(c_str!("LNUXBEEF")), ()) +/// ] +/// ); +/// /// impl platform::Driver for MyDriver { /// type IdInfo = (); /// const OF_ID_TABLE: Option> = Some(&OF_TABLE); +/// const ACPI_ID_TABLE: Option> = Some(&ACPI_TABLE); /// /// fn probe( /// _pdev: &platform::Device, @@ -164,6 +180,9 @@ pub trait Driver: Send { /// The table of OF device ids supported by the driver. const OF_ID_TABLE: Option> = None; + /// The table of ACPI device ids supported by the driver. + const ACPI_ID_TABLE: Option> = None; + /// Platform driver probe. /// /// Called when a new platform device is added or discovered. -- cgit v1.2.3 From 0dab138d0f4c0b3ce7f835d577e52a2b5ebdd536 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 26 Jun 2025 15:24:46 +0200 Subject: rust: devres: require T: Send for Devres Due to calling Revocable::revoke() from Devres::devres_callback() T may be dropped from Devres::devres_callback() and hence must be Send. Fix this by adding the corresponding bound to Devres and DevresInner. Reported-by: Boqun Feng Closes: https://lore.kernel.org/lkml/aFzI5L__OcB9hqdG@Mac.home/ Fixes: 76c01ded724b ("rust: add devres abstraction") Reviewed-by: Boqun Feng Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250626132544.72866-1-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 57502534d985..8ede607414fd 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -18,7 +18,7 @@ use crate::{ }; #[pin_data] -struct DevresInner { +struct DevresInner { dev: ARef, callback: unsafe extern "C" fn(*mut c_void), #[pin] @@ -95,9 +95,9 @@ struct DevresInner { /// # Ok(()) /// # } /// ``` -pub struct Devres(Arc>); +pub struct Devres(Arc>); -impl DevresInner { +impl DevresInner { fn new(dev: &Device, data: T, flags: Flags) -> Result>> { let inner = Arc::pin_init( pin_init!( DevresInner { @@ -175,7 +175,7 @@ impl DevresInner { } } -impl Devres { +impl Devres { /// 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 { @@ -247,7 +247,7 @@ impl Devres { } } -impl Drop for Devres { +impl Drop for Devres { fn drop(&mut self) { // SAFETY: When `drop` runs, it is guaranteed that nobody is accessing the revocable data // anymore, hence it is safe not to wait for the grace period to finish. -- cgit v1.2.3 From 64888dfdfac7f7d2013a9734755c11322ff6eaa5 Mon Sep 17 00:00:00 2001 From: Christian Schrefl Date: Tue, 10 Jun 2025 22:27:55 +0200 Subject: rust: implement `Wrapper` for `Opaque` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the implementation for `pin-init` from an associated function to the trait function of the `Wrapper` trait and extends the implementation to support pin-initializers with error types. Adds a use for the `Wrapper` trait in `revocable.rs`, to use the new `pin-init` function. This is currently the only usage in the kernel. Reviewed-by: Gerald Wisböck Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Acked-by: Miguel Ojeda Signed-off-by: Christian Schrefl Link: https://lore.kernel.org/r/20250610-b4-rust_miscdevice_registrationdata-v6-1-b03f5dfce998@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/revocable.rs | 2 ++ rust/kernel/types.rs | 26 ++++++++++++++------------ 2 files changed, 16 insertions(+), 12 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index 06a3cdfce344..fa1fd70efa27 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -5,6 +5,8 @@ //! 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 pin_init::Wrapper; + use crate::{bindings, prelude::*, sync::rcu, types::Opaque}; use core::{ marker::PhantomData, diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 22985b6f6982..3958a5f44d56 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -9,7 +9,7 @@ use core::{ ops::{Deref, DerefMut}, ptr::NonNull, }; -use pin_init::{PinInit, Zeroable}; +use pin_init::{PinInit, Wrapper, Zeroable}; /// Used to transfer ownership to and from foreign (non-Rust) languages. /// @@ -353,17 +353,6 @@ impl Opaque { } } - /// Create an opaque pin-initializer from the given pin-initializer. - pub fn pin_init(slot: impl PinInit) -> impl PinInit { - Self::ffi_init(|ptr: *mut T| { - // SAFETY: - // - `ptr` is a valid pointer to uninitialized memory, - // - `slot` is not accessed on error; the call is infallible, - // - `slot` is pinned in memory. - let _ = unsafe { PinInit::::__pinned_init(slot, ptr) }; - }) - } - /// Creates a pin-initializer from the given initializer closure. /// /// The returned initializer calls the given closure with the pointer to the inner `T` of this @@ -415,6 +404,19 @@ impl Opaque { } } +impl Wrapper for Opaque { + /// Create an opaque pin-initializer from the given pin-initializer. + fn pin_init(slot: impl PinInit) -> impl PinInit { + Self::try_ffi_init(|ptr: *mut T| { + // SAFETY: + // - `ptr` is a valid pointer to uninitialized memory, + // - `slot` is not accessed on error, + // - `slot` is pinned in memory. + unsafe { PinInit::::__pinned_init(slot, ptr) } + }) + } +} + /// Types that are _always_ reference counted. /// /// It allows such types to define their own custom ref increment and decrement functions. -- cgit v1.2.3 From ce7c22b2e1fb1db467d33bd050546941ce82f21f Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 26 Jun 2025 22:00:39 +0200 Subject: rust: revocable: support fallible PinInit types Currently, Revocable::new() only supports infallible PinInit implementations, i.e. impl PinInit. This has been sufficient so far, since users such as Devres do not support fallibility. Since this is about to change, make Revocable::new() generic over the error type E. Reviewed-by: Benno Lossin Reviewed-by: Alice Ryhl Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250626200054.243480-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 2 +- rust/kernel/revocable.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 8ede607414fd..fd8b75aa03bc 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -100,7 +100,7 @@ pub struct Devres(Arc>); impl DevresInner { fn new(dev: &Device, data: T, flags: Flags) -> Result>> { let inner = Arc::pin_init( - pin_init!( DevresInner { + try_pin_init!( DevresInner { dev: dev.into(), callback: Self::devres_callback, data <- Revocable::new(data), diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index fa1fd70efa27..46768b374656 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -82,11 +82,11 @@ unsafe impl Sync for Revocable {} impl Revocable { /// Creates a new revocable instance of the given data. - pub fn new(data: impl PinInit) -> impl PinInit { - pin_init!(Self { + pub fn new(data: impl PinInit) -> impl PinInit { + try_pin_init!(Self { is_available: AtomicBool::new(true), data <- Opaque::pin_init(data), - }) + }? E) } /// Tries to access the revocable wrapped object. -- cgit v1.2.3 From 46ae8fd7386abf809355d1857abac5cf2d7c3f62 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 26 Jun 2025 22:00:40 +0200 Subject: rust: devres: replace Devres::new_foreign_owned() Replace Devres::new_foreign_owned() with devres::register(). The current implementation of Devres::new_foreign_owned() creates a full Devres container instance, including the internal Revocable and completion. However, none of that is necessary for the intended use of giving full ownership of an object to devres and getting it dropped once the given device is unbound. Hence, implement devres::register(), which is limited to consume the given data, wrap it in a KBox and drop the KBox once the given device is unbound, without any other synchronization. Cc: Dave Airlie Cc: Simona Vetter Cc: Viresh Kumar Acked-by: Viresh Kumar Reviewed-by: Benno Lossin Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250626200054.243480-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/cpufreq.rs | 11 ++++--- rust/kernel/devres.rs | 73 ++++++++++++++++++++++++++++++++++++++++------- rust/kernel/drm/driver.rs | 14 +++++---- 3 files changed, 78 insertions(+), 20 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index 11b03e9d7e89..dd84e2b4d7ae 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -13,7 +13,7 @@ use crate::{ cpu::CpuId, cpumask, device::{Bound, Device}, - devres::Devres, + devres, error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR}, ffi::{c_char, c_ulong}, prelude::*, @@ -1046,10 +1046,13 @@ impl Registration { /// Same as [`Registration::new`], but does not return a [`Registration`] instance. /// - /// Instead the [`Registration`] is owned by [`Devres`] and will be revoked / dropped, once the + /// Instead the [`Registration`] is owned by [`devres::register`] and will be dropped, once the /// device is detached. - pub fn new_foreign_owned(dev: &Device) -> Result { - Devres::new_foreign_owned(dev, Self::new()?, GFP_KERNEL) + pub fn new_foreign_owned(dev: &Device) -> Result + where + T: 'static, + { + devres::register(dev, Self::new()?, GFP_KERNEL) } } diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index fd8b75aa03bc..64458ca3d69f 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -9,12 +9,12 @@ use crate::{ alloc::Flags, bindings, device::{Bound, Device}, - error::{Error, Result}, + error::{to_result, Error, Result}, ffi::c_void, prelude::*, revocable::{Revocable, RevocableGuard}, sync::{rcu, Arc, Completion}, - types::ARef, + types::{ARef, ForeignOwnable}, }; #[pin_data] @@ -184,14 +184,6 @@ impl Devres { 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(()) - } - /// Obtain `&'a T`, bypassing the [`Revocable`]. /// /// This method allows to directly obtain a `&'a T`, bypassing the [`Revocable`], by presenting @@ -261,3 +253,64 @@ impl Drop for Devres { } } } + +/// Consume `data` and [`Drop::drop`] `data` once `dev` is unbound. +fn register_foreign

(dev: &Device, data: P) -> Result +where + P: ForeignOwnable + Send + 'static, +{ + let ptr = data.into_foreign(); + + #[allow(clippy::missing_safety_doc)] + unsafe extern "C" fn callback(ptr: *mut kernel::ffi::c_void) { + // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked above and hence valid. + drop(unsafe { P::from_foreign(ptr.cast()) }); + } + + // SAFETY: + // - `dev.as_raw()` is a pointer to a valid and bound device. + // - `ptr` is a valid pointer the `ForeignOwnable` devres takes ownership of. + to_result(unsafe { + // `devm_add_action_or_reset()` also calls `callback` on failure, such that the + // `ForeignOwnable` is released eventually. + bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::

), ptr.cast()) + }) +} + +/// Encapsulate `data` in a [`KBox`] and [`Drop::drop`] `data` once `dev` is unbound. +/// +/// # Examples +/// +/// ```no_run +/// use kernel::{device::{Bound, Device}, devres}; +/// +/// /// Registration of e.g. a class device, IRQ, etc. +/// struct Registration; +/// +/// impl Registration { +/// fn new() -> Self { +/// // register +/// +/// Self +/// } +/// } +/// +/// impl Drop for Registration { +/// fn drop(&mut self) { +/// // unregister +/// } +/// } +/// +/// fn from_bound_context(dev: &Device) -> Result { +/// devres::register(dev, Registration::new(), GFP_KERNEL) +/// } +/// ``` +pub fn register(dev: &Device, data: impl PinInit, flags: Flags) -> Result +where + T: Send + 'static, + Error: From, +{ + let data = KBox::pin_init(data, flags)?; + + register_foreign(dev, data) +} diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index acb638086131..f63addaf7235 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -5,9 +5,7 @@ //! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/drm_drv.h) use crate::{ - bindings, device, - devres::Devres, - drm, + bindings, device, devres, drm, error::{to_result, Result}, prelude::*, str::CStr, @@ -130,18 +128,22 @@ impl Registration { } /// Same as [`Registration::new`}, but transfers ownership of the [`Registration`] to - /// [`Devres`]. + /// [`devres::register`]. pub fn new_foreign_owned( drm: &drm::Device, dev: &device::Device, flags: usize, - ) -> Result { + ) -> Result + where + T: 'static, + { if drm.as_ref().as_raw() != dev.as_raw() { return Err(EINVAL); } let reg = Registration::::new(drm, flags)?; - Devres::new_foreign_owned(dev, reg, GFP_KERNEL) + + devres::register(dev, reg, GFP_KERNEL) } /// Returns a reference to the `Device` instance for this registration. -- cgit v1.2.3 From f5d3ef25d238901a76fe0277787afa44f7714739 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 26 Jun 2025 22:00:41 +0200 Subject: rust: devres: get rid of Devres' inner Arc So far Devres uses an inner memory allocation and reference count, i.e. an inner Arc, in order to ensure that the devres callback can't run into a use-after-free in case where the Devres object is dropped while the devres callback runs concurrently. Instead, use a completion in order to avoid a potential UAF: In Devres::drop(), if we detect that we can't remove the devres action anymore, we wait for the completion that is completed from the devres callback. If, in turn, we were able to successfully remove the devres action, we can just go ahead. This, again, allows us to get rid of the internal Arc, and instead let Devres consume an `impl PinInit` in order to return an `impl PinInit, E>`, which enables us to get away with less memory allocations. Additionally, having the resulting explicit synchronization in Devres::drop() prevents potential subtle undesired side effects of the devres callback dropping the final Arc reference asynchronously within the devres callback. Reviewed-by: Benno Lossin Reviewed-by: Boqun Feng Link: https://lore.kernel.org/r/20250626200054.243480-4-dakr@kernel.org [ Move '# Invariants' below '# Examples'. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 213 +++++++++++++++++++++++++++++--------------------- rust/kernel/pci.rs | 20 ++--- 2 files changed, 136 insertions(+), 97 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 64458ca3d69f..7c5c5de8bcb6 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -13,16 +13,21 @@ use crate::{ ffi::c_void, prelude::*, revocable::{Revocable, RevocableGuard}, - sync::{rcu, Arc, Completion}, - types::{ARef, ForeignOwnable}, + sync::{rcu, Completion}, + types::{ARef, ForeignOwnable, Opaque, ScopeGuard}, }; +use pin_init::Wrapper; + +/// [`Devres`] inner data accessed from [`Devres::callback`]. #[pin_data] -struct DevresInner { - dev: ARef, - callback: unsafe extern "C" fn(*mut c_void), +struct Inner { #[pin] data: Revocable, + /// Tracks whether [`Devres::callback`] has been completed. + #[pin] + devm: Completion, + /// Tracks whether revoking [`Self::data`] has been completed. #[pin] revoke: Completion, } @@ -88,100 +93,115 @@ struct DevresInner { /// # fn no_run(dev: &Device) -> Result<(), Error> { /// // SAFETY: Invalid usage for example purposes. /// let iomem = unsafe { IoMem::<{ core::mem::size_of::() }>::new(0xBAAAAAAD)? }; -/// let devres = Devres::new(dev, iomem, GFP_KERNEL)?; +/// let devres = KBox::pin_init(Devres::new(dev, iomem), GFP_KERNEL)?; /// /// let res = devres.try_access().ok_or(ENXIO)?; /// res.write8(0x42, 0x0); /// # Ok(()) /// # } /// ``` -pub struct Devres(Arc>); +/// +/// # Invariants +/// +/// [`Self::inner`] is guaranteed to be initialized and is always accessed read-only. +#[pin_data(PinnedDrop)] +pub struct Devres { + dev: ARef, + /// Pointer to [`Self::devres_callback`]. + /// + /// Has to be stored, since Rust does not guarantee to always return the same address for a + /// function. However, the C API uses the address as a key. + callback: unsafe extern "C" fn(*mut c_void), + /// Contains all the fields shared with [`Self::callback`]. + // TODO: Replace with `UnsafePinned`, once available. + // + // Subsequently, the `drop_in_place()` in `Devres::drop` and the explicit `Send` and `Sync' + // impls can be removed. + #[pin] + inner: Opaque>, +} + +impl Devres { + /// Creates a new [`Devres`] instance of the given `data`. + /// + /// The `data` encapsulated within the returned `Devres` instance' `data` will be + /// (revoked)[`Revocable`] once the device is detached. + pub fn new<'a, E>( + dev: &'a Device, + data: impl PinInit + 'a, + ) -> impl PinInit + 'a + where + T: 'a, + Error: From, + { + let callback = Self::devres_callback; -impl DevresInner { - fn new(dev: &Device, data: T, flags: Flags) -> Result>> { - let inner = Arc::pin_init( - try_pin_init!( DevresInner { - dev: dev.into(), - callback: Self::devres_callback, + try_pin_init!(&this in Self { + // INVARIANT: `inner` is properly initialized. + inner <- Opaque::pin_init(try_pin_init!(Inner { data <- Revocable::new(data), + devm <- Completion::new(), revoke <- Completion::new(), - }), - flags, - )?; - - // Convert `Arc` into a raw pointer and make devres own this reference until - // `Self::devres_callback` is called. - let data = inner.clone().into_raw(); + })), + callback, + dev: { + // SAFETY: `this` is a valid pointer to uninitialized memory. + let inner = unsafe { &raw mut (*this.as_ptr()).inner }; - // 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(inner.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)); - } + // SAFETY: + // - `dev.as_raw()` is a pointer to a valid bound device. + // - `inner` is guaranteed to be a valid for the duration of the lifetime of `Self`. + // - `devm_add_action()` is guaranteed not to call `callback` until `this` has been + // properly initialized, because we require `dev` (i.e. the *bound* device) to + // live at least as long as the returned `impl PinInit`. + to_result(unsafe { + bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast()) + })?; - Ok(inner) + dev.into() + }, + }) } - fn as_ptr(&self) -> *const Self { - self as _ + fn inner(&self) -> &Inner { + // SAFETY: By the type invairants of `Self`, `inner` is properly initialized and always + // accessed read-only. + unsafe { &*self.inner.get() } } - fn remove_action(this: &Arc) -> bool { - // SAFETY: - // - `self.inner.dev` is a valid `Device`, - // - the `action` and `data` pointers are the exact same ones as given to devm_add_action() - // previously, - // - `self` is always valid, even if the action has been released already. - let success = unsafe { - bindings::devm_remove_action_nowarn( - this.dev.as_raw(), - Some(this.callback), - this.as_ptr() as _, - ) - } == 0; - - if success { - // SAFETY: We leaked an `Arc` reference to devm_add_action() in `DevresInner::new`; if - // devm_remove_action_nowarn() was successful we can (and have to) claim back ownership - // of this reference. - let _ = unsafe { Arc::from_raw(this.as_ptr()) }; - } - - success + fn data(&self) -> &Revocable { + &self.inner().data } #[allow(clippy::missing_safety_doc)] unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) { - let ptr = ptr as *mut DevresInner; - // 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) }; + // SAFETY: In `Self::new` we've passed a valid pointer to `Inner` to `devm_add_action()`, + // hence `ptr` must be a valid pointer to `Inner`. + let inner = unsafe { &*ptr.cast::>() }; + + // Ensure that `inner` can't be used anymore after we signal completion of this callback. + let inner = ScopeGuard::new_with_data(inner, |inner| inner.devm.complete_all()); if !inner.data.revoke() { // If `revoke()` returns false, it means that `Devres::drop` already started revoking - // `inner.data` for us. Hence we have to wait until `Devres::drop()` signals that it - // completed revoking `inner.data`. + // `data` for us. Hence we have to wait until `Devres::drop` signals that it + // completed revoking `data`. inner.revoke.wait_for_completion(); } } -} -impl Devres { - /// 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 { - let inner = DevresInner::new(dev, data, flags)?; - - Ok(Devres(inner)) + fn remove_action(&self) -> bool { + // SAFETY: + // - `self.dev` is a valid `Device`, + // - the `action` and `data` pointers are the exact same ones as given to + // `devm_add_action()` previously, + (unsafe { + bindings::devm_remove_action_nowarn( + self.dev.as_raw(), + Some(self.callback), + core::ptr::from_ref(self.inner()).cast_mut().cast(), + ) + } == 0) } /// Obtain `&'a T`, bypassing the [`Revocable`]. @@ -213,44 +233,63 @@ impl Devres { /// } /// ``` pub fn access<'a>(&'a self, dev: &'a Device) -> Result<&'a T> { - if self.0.dev.as_raw() != dev.as_raw() { + if self.dev.as_raw() != dev.as_raw() { return Err(EINVAL); } // SAFETY: `dev` being the same device as the device this `Devres` has been created for - // proves that `self.0.data` hasn't been revoked and is guaranteed to not be revoked as - // long as `dev` lives; `dev` lives at least as long as `self`. - Ok(unsafe { self.0.data.access() }) + // proves that `self.data` hasn't been revoked and is guaranteed to not be revoked as long + // as `dev` lives; `dev` lives at least as long as `self`. + Ok(unsafe { self.data().access() }) } /// [`Devres`] accessor for [`Revocable::try_access`]. pub fn try_access(&self) -> Option> { - self.0.data.try_access() + self.data().try_access() } /// [`Devres`] accessor for [`Revocable::try_access_with`]. pub fn try_access_with R>(&self, f: F) -> Option { - self.0.data.try_access_with(f) + self.data().try_access_with(f) } /// [`Devres`] accessor for [`Revocable::try_access_with_guard`]. pub fn try_access_with_guard<'a>(&'a self, guard: &'a rcu::Guard) -> Option<&'a T> { - self.0.data.try_access_with_guard(guard) + self.data().try_access_with_guard(guard) } } -impl Drop for Devres { - fn drop(&mut self) { +// SAFETY: `Devres` can be send to any task, if `T: Send`. +unsafe impl Send for Devres {} + +// SAFETY: `Devres` can be shared with any task, if `T: Sync`. +unsafe impl Sync for Devres {} + +#[pinned_drop] +impl PinnedDrop for Devres { + fn drop(self: Pin<&mut Self>) { // SAFETY: When `drop` runs, it is guaranteed that nobody is accessing the revocable data // anymore, hence it is safe not to wait for the grace period to finish. - if unsafe { self.0.data.revoke_nosync() } { - // We revoked `self.0.data` before the devres action did, hence try to remove it. - if !DevresInner::remove_action(&self.0) { + if unsafe { self.data().revoke_nosync() } { + // We revoked `self.data` before the devres action did, hence try to remove it. + if !self.remove_action() { // We could not remove the devres action, which means that it now runs concurrently, - // hence signal that `self.0.data` has been revoked successfully. - self.0.revoke.complete_all(); + // hence signal that `self.data` has been revoked by us successfully. + self.inner().revoke.complete_all(); + + // Wait for `Self::devres_callback` to be done using this object. + self.inner().devm.wait_for_completion(); } + } else { + // `Self::devres_callback` revokes `self.data` for us, hence wait for it to be done + // using this object. + self.inner().devm.wait_for_completion(); } + + // INVARIANT: At this point it is guaranteed that `inner` can't be accessed any more. + // + // SAFETY: `inner` is valid for dropping. + unsafe { core::ptr::drop_in_place(self.inner.get()) }; } } diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 8435f8132e38..db0eb7eaf9b1 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -5,7 +5,6 @@ //! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h) use crate::{ - alloc::flags::*, bindings, container_of, device, device_id::RawDeviceId, devres::Devres, @@ -398,19 +397,20 @@ impl Device { impl Device { /// Mapps an entire PCI-BAR after performing a region-request on it. I/O operation bound checks /// can be performed on compile time for offsets (plus the requested type size) < SIZE. - pub fn iomap_region_sized( - &self, + pub fn iomap_region_sized<'a, const SIZE: usize>( + &'a self, bar: u32, - name: &CStr, - ) -> Result>> { - let bar = Bar::::new(self, bar, name)?; - let devres = Devres::new(self.as_ref(), bar, GFP_KERNEL)?; - - Ok(devres) + name: &'a CStr, + ) -> impl PinInit>, Error> + 'a { + Devres::new(self.as_ref(), Bar::::new(self, bar, name)) } /// Mapps an entire PCI-BAR after performing a region-request on it. - pub fn iomap_region(&self, bar: u32, name: &CStr) -> Result> { + pub fn iomap_region<'a>( + &'a self, + bar: u32, + name: &'a CStr, + ) -> impl PinInit, Error> + 'a { self.iomap_region_sized::<0>(bar, name) } } -- cgit v1.2.3 From 9b5cdd5f40191d11d3b535ab7978751a4a3e4bc5 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Tue, 24 Jun 2025 07:58:46 +0900 Subject: rust: fix typo in #[repr(transparent)] comments Fix a typo in several comments where `#[repr(transparent)]` was mistakenly written as `#[repr(transparent)` (missing closing bracket). Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250623225846.169805-1-fujita.tomonori@gmail.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/driver.rs | 4 ++-- rust/kernel/of.rs | 2 +- rust/kernel/pci.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index a4bf4498e592..f8dd7593e8dc 100644 --- a/rust/kernel/driver.rs +++ b/rust/kernel/driver.rs @@ -166,7 +166,7 @@ pub trait Adapter { if raw_id.is_null() { None } else { - // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct acpi_device_id` + // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct acpi_device_id` // and does not add additional invariants, so it's safe to transmute. let id = unsafe { &*raw_id.cast::() }; @@ -200,7 +200,7 @@ pub trait Adapter { if raw_id.is_null() { None } else { - // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` + // 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::() }; diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs index 04f2d8ef29cb..52fe0c77b9a1 100644 --- a/rust/kernel/of.rs +++ b/rust/kernel/of.rs @@ -13,7 +13,7 @@ pub type IdTable = &'static dyn kernel::device_id::IdTable; pub struct DeviceId(bindings::of_device_id); // SAFETY: -// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and does not add +// * `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id` and does not add // additional invariants, so it's safe to transmute to `RawType`. // * `DRIVER_DATA_OFFSET` is the offset to the `data` field. unsafe impl RawDeviceId for DeviceId { diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index db0eb7eaf9b1..9afe2a8ed637 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -67,7 +67,7 @@ impl Adapter { // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. let pdev = unsafe { &*pdev.cast::>() }; - // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct pci_device_id` and + // 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::() }; let info = T::ID_TABLE.info(id.index()); @@ -161,7 +161,7 @@ impl DeviceId { } // SAFETY: -// * `DeviceId` is a `#[repr(transparent)` wrapper of `pci_device_id` and does not add +// * `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 { -- cgit v1.2.3 From b9ff1c2a26fa31216be18e9b14c419ff8fe39e72 Mon Sep 17 00:00:00 2001 From: Shankari Anand Date: Thu, 26 Jun 2025 16:15:20 +0530 Subject: rust: miscdevice: clarify invariant for `MiscDeviceRegistration` Reword and expand the invariant documentation for `MiscDeviceRegistration` to clarify what it means for the inner device to be "registered". It expands to explain: - `inner` points to a `miscdevice` registered via `misc_register`. - This registration stays valid for the entire lifetime of the object. - Deregistration is guaranteed on `Drop`, via `misc_deregister`. Reported-by: Benno Lossin Closes: https://github.com/Rust-for-Linux/linux/issues/1168 Fixes: f893691e7426 ("rust: miscdevice: add base miscdevice abstraction") Signed-off-by: Shankari Anand Link: https://lore.kernel.org/r/20250626104520.563036-1-shankari.ak0208@gmail.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 939278bc7b03..4f7a8714ad36 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -45,7 +45,13 @@ impl MiscDeviceOptions { /// /// # Invariants /// -/// `inner` is a registered misc device. +/// - `inner` contains a `struct miscdevice` that is registered using +/// `misc_register()`. +/// - This registration remains valid for the entire lifetime of the +/// [`MiscDeviceRegistration`] instance. +/// - Deregistration occurs exactly once in [`Drop`] via `misc_deregister()`. +/// - `inner` wraps a valid, pinned `miscdevice` created using +/// [`MiscDeviceOptions::into_raw`]. #[repr(transparent)] #[pin_data(PinnedDrop)] pub struct MiscDeviceRegistration { -- cgit v1.2.3 From 97ba207a994f7d0cc58f19d448e98db6735f5e77 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 1 Jul 2025 19:46:56 +0200 Subject: rust: acpi: remove unneeded cast to clean future Clippy warning A future Clippy warning, `clippy::as_underscore`, is getting enabled in parallel in the rust-next tree: error: using `as _` conversion --> rust/kernel/acpi.rs:25:9 | 25 | self.0.driver_data as _ | ^^^^^^^^^^^^^^^^^^^^^^- | | | help: consider giving the type explicitly: `usize` The type is already `ulong`, which nowadays is always `usize`, so the cast is unneeded. Thus remove it, which in turn will avoid the warning in the future. Other abstractions of device tables do not use a cast here either. Signed-off-by: Miguel Ojeda Reviewed-by: Trevor Gross Link: https://lore.kernel.org/r/20250701174656.62205-1-ojeda@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/acpi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs index eaae3bc8e54c..2af4d4f92924 100644 --- a/rust/kernel/acpi.rs +++ b/rust/kernel/acpi.rs @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId { const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data); fn index(&self) -> usize { - self.0.driver_data as _ + self.0.driver_data } } -- cgit v1.2.3 From 8ae33576ead8fb71505515df8c938b3e0ae5b37c Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 4 Jul 2025 15:49:54 -0400 Subject: rust: platform: remove unnecessary import `kernel::str::CStr` is included in the prelude. Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250704-cstr-include-platform-v1-1-ff7803ee7a81@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 86f9d73c64b3..7484d7e5c2cf 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -9,7 +9,6 @@ use crate::{ error::{to_result, Result}, of, prelude::*, - str::CStr, types::{ForeignOwnable, Opaque}, ThisModule, }; -- cgit v1.2.3 From 65f8f0d4e0b45dc784f18a7ca4739e1df12b950a Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 4 Jul 2025 15:50:44 -0400 Subject: rust: auxiliary: remove unnecessary import `kernel::str::CStr` is included in the prelude. Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250704-cstr-include-aux-v1-1-e1a404ae92ac@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index d2cfe1eeefb6..4fe62bbf8b3a 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -10,7 +10,6 @@ use crate::{ driver, error::{to_result, Result}, prelude::*, - str::CStr, types::{ForeignOwnable, Opaque}, ThisModule, }; -- cgit v1.2.3 From 6d16cd5769bbb5eb62974e8eddb97fca830b49fd Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 4 Jul 2025 16:11:02 -0400 Subject: rust: devres: remove unused import As far as I can tell, `c_str` was never used, hence remove it. Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250704-cstr-include-devres-v1-1-4ee9e56fca09@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 7c5c5de8bcb6..f43de3d77d61 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -52,7 +52,7 @@ struct Inner { /// # Example /// /// ```no_run -/// # use kernel::{bindings, c_str, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}}; +/// # use kernel::{bindings, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}}; /// # use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. -- cgit v1.2.3 From 5cddd546df0fa21316735d1d60fe826886e0dc21 Mon Sep 17 00:00:00 2001 From: Rahul Rameshbabu Date: Sun, 6 Jul 2025 04:00:32 +0000 Subject: rust: pci: fix documentation related to Device instances Device instances in the pci crate represent a valid struct pci_dev, not a struct device. Fixes: 7b948a2af6b5 ("rust: pci: fix unrestricted &mut pci::Device") Signed-off-by: Rahul Rameshbabu Link: https://lore.kernel.org/r/20250706035944.18442-3-sergeantsagara@protonmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 9afe2a8ed637..a1febfbfc7d7 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -250,7 +250,8 @@ pub trait Driver: Send { /// /// # Invariants /// -/// A [`Device`] instance represents a valid `struct device` created by the C portion of the kernel. +/// A [`Device`] instance represents a valid `struct pci_dev` created by the C portion of the +/// kernel. #[repr(transparent)] pub struct Device( Opaque, -- cgit v1.2.3 From fa7486d3f9470c58c5971caba7244cc8773672e0 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 21 Jun 2025 21:43:27 +0200 Subject: rust: device: introduce device::CoreInternal Introduce an internal device context, which is semantically equivalent to the Core device context, but reserved for bus abstractions. This allows implementing methods for the Device type, which are limited to be used within the core context of bus abstractions, i.e. restrict the availability for drivers. Link: https://lore.kernel.org/r/20250621195118.124245-2-dakr@kernel.org [ Rename device::Internal to device::CoreInternal. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 665f5ceadecc..6b8acee4a378 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -261,6 +261,10 @@ pub struct Normal; /// any of the bus callbacks, such as `probe()`. pub struct Core; +/// Semantically the same as [`Core`] but reserved for internal usage of the corresponding bus +/// abstraction. +pub struct CoreInternal; + /// The [`Bound`] context is the context of a bus specific device reference when it is guaranteed to /// be bound for the duration of its lifetime. pub struct Bound; @@ -270,11 +274,13 @@ mod private { impl Sealed for super::Bound {} impl Sealed for super::Core {} + impl Sealed for super::CoreInternal {} impl Sealed for super::Normal {} } impl DeviceContext for Bound {} impl DeviceContext for Core {} +impl DeviceContext for CoreInternal {} impl DeviceContext for Normal {} /// # Safety @@ -312,6 +318,13 @@ macro_rules! __impl_device_context_deref { #[macro_export] macro_rules! impl_device_context_deref { (unsafe { $device:ident }) => { + // SAFETY: This macro has the exact same safety requirement as + // `__impl_device_context_deref!`. + ::kernel::__impl_device_context_deref!(unsafe { + $device, + $crate::device::CoreInternal => $crate::device::Core + }); + // SAFETY: This macro has the exact same safety requirement as // `__impl_device_context_deref!`. ::kernel::__impl_device_context_deref!(unsafe { @@ -345,6 +358,7 @@ macro_rules! __impl_device_context_into_aref { #[macro_export] macro_rules! impl_device_context_into_aref { ($device:tt) => { + ::kernel::__impl_device_context_into_aref!($crate::device::CoreInternal, $device); ::kernel::__impl_device_context_into_aref!($crate::device::Core, $device); ::kernel::__impl_device_context_into_aref!($crate::device::Bound, $device); }; -- cgit v1.2.3 From 880dec12a25890e8f5626f04c58d38003f1a5585 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 21 Jun 2025 21:43:28 +0200 Subject: rust: device: add drvdata accessors Implement generic accessors for the private data of a driver bound to a device. Those accessors should be used by bus abstractions from their corresponding core callbacks, such as probe(), remove(), etc. Implementing them for device::CoreInternal guarantees that driver's can't interfere with the logic implemented by the bus abstraction. Acked-by: Benno Lossin Link: https://lore.kernel.org/r/20250621195118.124245-3-dakr@kernel.org [ Improve safety comment as proposed by Benno. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 6b8acee4a378..d527ceef829e 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -6,7 +6,7 @@ use crate::{ bindings, - types::{ARef, Opaque}, + types::{ARef, ForeignOwnable, Opaque}, }; use core::{fmt, marker::PhantomData, ptr}; @@ -62,6 +62,53 @@ impl Device { } } +impl Device { + /// Store a pointer to the bound driver's private data. + pub fn set_drvdata(&self, data: impl ForeignOwnable) { + // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`. + unsafe { bindings::dev_set_drvdata(self.as_raw(), data.into_foreign().cast()) } + } + + /// Take ownership of the private data stored in this [`Device`]. + /// + /// # Safety + /// + /// - Must only be called once after a preceding call to [`Device::set_drvdata`]. + /// - The type `T` must match the type of the `ForeignOwnable` previously stored by + /// [`Device::set_drvdata`]. + pub unsafe fn drvdata_obtain(&self) -> T { + // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`. + let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) }; + + // SAFETY: + // - By the safety requirements of this function, `ptr` comes from a previous call to + // `into_foreign()`. + // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()` + // in `into_foreign()`. + unsafe { T::from_foreign(ptr.cast()) } + } + + /// Borrow the driver's private data bound to this [`Device`]. + /// + /// # Safety + /// + /// - Must only be called after a preceding call to [`Device::set_drvdata`] and before + /// [`Device::drvdata_obtain`]. + /// - The type `T` must match the type of the `ForeignOwnable` previously stored by + /// [`Device::set_drvdata`]. + pub unsafe fn drvdata_borrow(&self) -> T::Borrowed<'_> { + // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`. + let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) }; + + // SAFETY: + // - By the safety requirements of this function, `ptr` comes from a previous call to + // `into_foreign()`. + // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()` + // in `into_foreign()`. + unsafe { T::borrow(ptr.cast()) } + } +} + impl Device { /// Obtain the raw `struct device *`. pub(crate) fn as_raw(&self) -> *mut bindings::device { -- cgit v1.2.3 From f0a68a912c673d8899d863c2f01f1ef7006e0b11 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 21 Jun 2025 21:43:29 +0200 Subject: rust: platform: use generic device drvdata accessors Take advantage of the generic drvdata accessors of the generic Device type. While at it, use from_result() instead of match. Link: https://lore.kernel.org/r/20250621195118.124245-4-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 7484d7e5c2cf..e9e95c5bb02b 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -6,10 +6,10 @@ use crate::{ acpi, bindings, container_of, device, driver, - error::{to_result, Result}, + error::{from_result, to_result, Result}, of, prelude::*, - types::{ForeignOwnable, Opaque}, + types::Opaque, ThisModule, }; @@ -66,30 +66,28 @@ impl Adapter { // `struct platform_device`. // // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. - let pdev = unsafe { &*pdev.cast::>() }; - + let pdev = unsafe { &*pdev.cast::>() }; let info = ::id_info(pdev.as_ref()); - match T::probe(pdev, info) { - Ok(data) => { - // Let the `struct platform_device` own a reference of the driver's private data. - // SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a - // `struct platform_device`. - unsafe { bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign() as _) }; - } - Err(err) => return Error::to_errno(err), - } - 0 + from_result(|| { + let data = T::probe(pdev, info)?; + + pdev.as_ref().set_drvdata(data); + Ok(0) + }) } extern "C" fn remove_callback(pdev: *mut bindings::platform_device) { - // SAFETY: `pdev` is a valid pointer to a `struct platform_device`. - let ptr = unsafe { bindings::platform_get_drvdata(pdev) }.cast(); + // SAFETY: The platform bus only ever calls the remove callback with a valid pointer to a + // `struct platform_device`. + // + // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. + let pdev = unsafe { &*pdev.cast::>() }; // 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` pointer created through `KBox::into_foreign`. - let _ = unsafe { KBox::::from_foreign(ptr) }; + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called + // and stored a `Pin>`. + let _ = unsafe { pdev.as_ref().drvdata_obtain::>>() }; } } -- cgit v1.2.3 From 4231712c8e9840c023192032d438f98061b9ee1f Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 21 Jun 2025 21:43:30 +0200 Subject: rust: pci: use generic device drvdata accessors Take advantage of the generic drvdata accessors of the generic Device type. While at it, use from_result() instead of match. Link: https://lore.kernel.org/r/20250621195118.124245-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index a1febfbfc7d7..34f0db8054fe 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -9,11 +9,11 @@ use crate::{ device_id::RawDeviceId, devres::Devres, driver, - error::{to_result, Result}, + error::{from_result, to_result, Result}, io::Io, io::IoRaw, str::CStr, - types::{ARef, ForeignOwnable, Opaque}, + types::{ARef, Opaque}, ThisModule, }; use core::{ @@ -65,35 +65,32 @@ impl Adapter { // `struct pci_dev`. // // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. - let pdev = unsafe { &*pdev.cast::>() }; + let pdev = unsafe { &*pdev.cast::>() }; // 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::() }; let info = T::ID_TABLE.info(id.index()); - match T::probe(pdev, 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), - } + from_result(|| { + let data = T::probe(pdev, info)?; - 0 + pdev.as_ref().set_drvdata(data); + Ok(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) }.cast(); + // + // INVARIANT: `pdev` is valid for the duration of `remove_callback()`. + let pdev = unsafe { &*pdev.cast::>() }; // 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` pointer created through `KBox::into_foreign`. - let _ = unsafe { KBox::::from_foreign(ptr) }; + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called + // and stored a `Pin>`. + let _ = unsafe { pdev.as_ref().drvdata_obtain::>>() }; } } -- cgit v1.2.3 From c46f60246f9ae372ecc1f10976a8af3914b3f79e Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 21 Jun 2025 21:43:31 +0200 Subject: rust: auxiliary: use generic device drvdata accessors Take advantage of the generic drvdata accessors of the generic Device type. While at it, use from_result() instead of match. Link: https://lore.kernel.org/r/20250621195118.124245-6-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index 4fe62bbf8b3a..2985673181b7 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -8,9 +8,9 @@ use crate::{ bindings, container_of, device, device_id::RawDeviceId, driver, - error::{to_result, Result}, + error::{from_result, to_result, Result}, prelude::*, - types::{ForeignOwnable, Opaque}, + types::Opaque, ThisModule, }; use core::{ @@ -60,37 +60,32 @@ impl Adapter { // `struct auxiliary_device`. // // INVARIANT: `adev` is valid for the duration of `probe_callback()`. - let adev = unsafe { &*adev.cast::>() }; + let adev = unsafe { &*adev.cast::>() }; // SAFETY: `DeviceId` is a `#[repr(transparent)`] wrapper of `struct auxiliary_device_id` // and does not add additional invariants, so it's safe to transmute. let id = unsafe { &*id.cast::() }; let info = T::ID_TABLE.info(id.index()); - match T::probe(adev, info) { - Ok(data) => { - // Let the `struct auxiliary_device` own a reference of the driver's private data. - // SAFETY: By the type invariant `adev.as_raw` returns a valid pointer to a - // `struct auxiliary_device`. - unsafe { - bindings::auxiliary_set_drvdata(adev.as_raw(), data.into_foreign().cast()) - }; - } - Err(err) => return Error::to_errno(err), - } + from_result(|| { + let data = T::probe(adev, info)?; - 0 + adev.as_ref().set_drvdata(data); + Ok(0) + }) } extern "C" fn remove_callback(adev: *mut bindings::auxiliary_device) { - // SAFETY: The auxiliary bus only ever calls the remove callback with a valid pointer to a + // SAFETY: The auxiliary bus only ever calls the probe callback with a valid pointer to a // `struct auxiliary_device`. - let ptr = unsafe { bindings::auxiliary_get_drvdata(adev) }; + // + // INVARIANT: `adev` is valid for the duration of `probe_callback()`. + let adev = unsafe { &*adev.cast::>() }; // 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` pointer created through `KBox::into_foreign`. - drop(unsafe { KBox::::from_foreign(ptr.cast()) }); + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called + // and stored a `Pin>`. + drop(unsafe { adev.as_ref().drvdata_obtain::>>() }); } } -- cgit v1.2.3 From 4be5f3fff151e49b0e7a82d33d4d12e91840abde Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 21 Jun 2025 21:43:32 +0200 Subject: rust: platform: implement Driver::unbind() Currently, there's really only one core callback for drivers, which is probe(). Now, this isn't entirely true, since there is also the drop() callback of the driver type (serving as the driver's private data), which is returned by probe() and is dropped in remove(). On the C side remove() mainly serves two purposes: (1) Tear down the device that is operated by the driver, e.g. call bus specific functions, write I/O memory to reset the device, etc. (2) Free the resources that have been allocated by a driver for a specific device. The drop() callback mentioned above is intended to cover (2) as the Rust idiomatic way. However, it is partially insufficient and inefficient to cover (1) properly, since drop() can't be called with additional arguments, such as the reference to the corresponding device that has the correct device context, i.e. the Core device context. This makes it inefficient (but not impossible) to access device resources, e.g. to write device registers, and impossible to call device methods, which are only accessible under the Core device context. In order to solve this, add an additional callback for (1), which we call unbind(). The reason for calling it unbind() is that, unlike remove(), it is *only* meant to be used to perform teardown operations on the device (1), but *not* to release resources (2). Link: https://lore.kernel.org/r/20250621195118.124245-7-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index e9e95c5bb02b..ced43367d900 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -87,7 +87,9 @@ impl Adapter { // SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called // and stored a `Pin>`. - let _ = unsafe { pdev.as_ref().drvdata_obtain::>>() }; + let data = unsafe { pdev.as_ref().drvdata_obtain::>>() }; + + T::unbind(pdev, data.as_ref()); } } @@ -186,6 +188,20 @@ pub trait Driver: Send { /// Implementers should attempt to initialize the device here. fn probe(dev: &Device, id_info: Option<&Self::IdInfo>) -> Result>>; + + /// Platform driver unbind. + /// + /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback + /// is optional. + /// + /// This callback serves as a place for drivers to perform teardown operations that require a + /// `&Device` or `&Device` reference. For instance, drivers may try to perform I/O + /// operations to gracefully tear down the device. + /// + /// Otherwise, release operations for driver resources should be performed in `Self::drop`. + fn unbind(dev: &Device, this: Pin<&Self>) { + let _ = (dev, this); + } } /// The platform device representation. -- cgit v1.2.3 From 18ebb25dfa18c09474fed78b20808fa445370666 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 21 Jun 2025 21:43:33 +0200 Subject: rust: pci: implement Driver::unbind() Currently, there's really only one core callback for drivers, which is probe(). Now, this isn't entirely true, since there is also the drop() callback of the driver type (serving as the driver's private data), which is returned by probe() and is dropped in remove(). On the C side remove() mainly serves two purposes: (1) Tear down the device that is operated by the driver, e.g. call bus specific functions, write I/O memory to reset the device, etc. (2) Free the resources that have been allocated by a driver for a specific device. The drop() callback mentioned above is intended to cover (2) as the Rust idiomatic way. However, it is partially insufficient and inefficient to cover (1) properly, since drop() can't be called with additional arguments, such as the reference to the corresponding device that has the correct device context, i.e. the Core device context. This makes it inefficient (but not impossible) to access device resources, e.g. to write device registers, and impossible to call device methods, which are only accessible under the Core device context. In order to solve this, add an additional callback for (1), which we call unbind(). The reason for calling it unbind() is that, unlike remove(), it is *only* meant to be used to perform teardown operations on the device (1), but *not* to release resources (2). Link: https://lore.kernel.org/r/20250621195118.124245-8-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 34f0db8054fe..8b884e324dcf 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -90,7 +90,9 @@ impl Adapter { // SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called // and stored a `Pin>`. - let _ = unsafe { pdev.as_ref().drvdata_obtain::>>() }; + let data = unsafe { pdev.as_ref().drvdata_obtain::>>() }; + + T::unbind(pdev, data.as_ref()); } } @@ -237,6 +239,20 @@ pub trait Driver: Send { /// Called when a new platform device is added or discovered. /// Implementers should attempt to initialize the device here. fn probe(dev: &Device, id_info: &Self::IdInfo) -> Result>>; + + /// Platform driver unbind. + /// + /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback + /// is optional. + /// + /// This callback serves as a place for drivers to perform teardown operations that require a + /// `&Device` or `&Device` reference. For instance, drivers may try to perform I/O + /// operations to gracefully tear down the device. + /// + /// Otherwise, release operations for driver resources should be performed in `Self::drop`. + fn unbind(dev: &Device, this: Pin<&Self>) { + let _ = (dev, this); + } } /// The PCI device representation. -- cgit v1.2.3 From 91ae26b06aab476bdd8b56cd99e127f029490330 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Jul 2025 13:32:35 +0200 Subject: rust: devres: initialize Devres::inner::data last Users may want to access the Devres object from callbacks registered through the initialization of Devres::inner::data. For those accesses to be valid, Devres::inner::data must be initialized last [1]. Credit to Boqun for spotting this [2]. Link: https://lore.kernel.org/lkml/DBBPHO26CPBS.2OVI1OERCB2J5@kernel.org/ [1] Link: https://lore.kernel.org/lkml/aHSmxWeIy3L-AKIV@Mac.home/ [2] Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250714113712.22158-1-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index f43de3d77d61..ac5e5d94ee96 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -137,14 +137,10 @@ impl Devres { let callback = Self::devres_callback; try_pin_init!(&this in Self { - // INVARIANT: `inner` is properly initialized. - inner <- Opaque::pin_init(try_pin_init!(Inner { - data <- Revocable::new(data), - devm <- Completion::new(), - revoke <- Completion::new(), - })), + dev: dev.into(), callback, - dev: { + // INVARIANT: `inner` is properly initialized. + inner <- { // SAFETY: `this` is a valid pointer to uninitialized memory. let inner = unsafe { &raw mut (*this.as_ptr()).inner }; @@ -158,7 +154,11 @@ impl Devres { bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast()) })?; - dev.into() + Opaque::pin_init(try_pin_init!(Inner { + devm <- Completion::new(), + revoke <- Completion::new(), + data <- Revocable::new(data), + })) }, }) } -- cgit v1.2.3 From 85aa5b16fef7040213581df9ff093dae27bf8675 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Jul 2025 20:26:53 +0200 Subject: rust: devres: provide an accessor for the device Provide an accessor for the Device a Devres instance has been created with. For instance, this is useful when registrations want to provide a &Device for a scope that is protected by Devres. Suggested-by: Benno Lossin Acked-by: Greg Kroah-Hartman Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250713182737.64448-1-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index ac5e5d94ee96..152a89b78943 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -204,6 +204,11 @@ impl Devres { } == 0) } + /// Return a reference of the [`Device`] this [`Devres`] instance has been created with. + pub fn device(&self) -> &Device { + &self.dev + } + /// Obtain `&'a T`, bypassing the [`Revocable`]. /// /// This method allows to directly obtain a `&'a T`, bypassing the [`Revocable`], by presenting -- cgit v1.2.3 From 47e6715bb7ea388a9a3a13d31918827ba3aa3f4a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Jul 2025 20:26:54 +0200 Subject: rust: device: implement Device::as_bound() Provide an unsafe functions for abstractions to convert a regular &Device to a &Device. This is useful for registrations that provide certain guarantees for the scope of their callbacks, such as IRQs or certain class device registrations (e.g. PWM, miscdevice). Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250713182737.64448-2-dakr@kernel.org [ Remove unnecessary cast(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index d527ceef829e..b6b81546a0da 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -60,6 +60,25 @@ impl Device { // SAFETY: By the safety requirements ptr is valid unsafe { Self::as_ref(ptr) }.into() } + + /// Convert a [`&Device`](Device) into a [`&Device`](Device). + /// + /// # Safety + /// + /// The caller is responsible to ensure that the returned [`&Device`](Device) + /// only lives as long as it can be guaranteed that the [`Device`] is actually bound. + pub unsafe fn as_bound(&self) -> &Device { + let ptr = core::ptr::from_ref(self); + + // CAST: By the safety requirements the caller is responsible to guarantee that the + // returned reference only lives as long as the device is actually bound. + let ptr = ptr.cast(); + + // SAFETY: + // - `ptr` comes from `from_ref(self)` above, hence it's guaranteed to be valid. + // - Any valid `Device` pointer is also a valid pointer for `Device`. + unsafe { &*ptr } + } } impl Device { -- cgit v1.2.3 From 2f5606afa4c2bcabd45cb34c92faf93ca5ffe75e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 11 Jul 2025 08:04:37 +0000 Subject: device: rust: rename Device::as_ref() to Device::from_raw() The prefix as_* should not be used for a constructor. Constructors usually use the prefix from_* instead. Some prior art in the stdlib: Box::from_raw, CString::from_raw, Rc::from_raw, Arc::from_raw, Waker::from_raw, File::from_raw_fd. There is also prior art in the kernel crate: cpufreq::Policy::from_raw, fs::File::from_raw_file, Kuid::from_raw, ARef::from_raw, SeqFile::from_raw, VmaNew::from_raw, Io::from_raw. Link: https://lore.kernel.org/r/aCd8D5IA0RXZvtcv@pollux Signed-off-by: Alice Ryhl Reviewed-by: Benno Lossin Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250711-device-as-ref-v2-1-1b16ab6402d7@google.com Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 2 +- rust/kernel/cpu.rs | 2 +- rust/kernel/device.rs | 6 +++--- rust/kernel/drm/device.rs | 2 +- rust/kernel/faux.rs | 2 +- rust/kernel/miscdevice.rs | 2 +- rust/kernel/net/phy.rs | 2 +- rust/kernel/pci.rs | 2 +- rust/kernel/platform.rs | 2 +- 9 files changed, 11 insertions(+), 11 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index 2985673181b7..e1f2a2e6b607 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -270,7 +270,7 @@ impl AsRef> for Device { let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; // SAFETY: `dev` points to a valid `struct device`. - unsafe { device::Device::as_ref(dev) } + unsafe { device::Device::from_raw(dev) } } } diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs index b75403b0eb56..5de730c8d817 100644 --- a/rust/kernel/cpu.rs +++ b/rust/kernel/cpu.rs @@ -147,5 +147,5 @@ pub unsafe fn from_cpu(cpu: CpuId) -> Result<&'static Device> { // SAFETY: The pointer returned by `get_cpu_device()`, if not `NULL`, is a valid pointer to // a `struct device` and is never freed by the C code. - Ok(unsafe { Device::as_ref(ptr) }) + Ok(unsafe { Device::from_raw(ptr) }) } diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index b6b81546a0da..ca82926fd67f 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -58,7 +58,7 @@ impl Device { /// While not officially documented, this should be the case for any `struct device`. pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef { // SAFETY: By the safety requirements ptr is valid - unsafe { Self::as_ref(ptr) }.into() + unsafe { Self::from_raw(ptr) }.into() } /// Convert a [`&Device`](Device) into a [`&Device`](Device). @@ -149,7 +149,7 @@ impl Device { // - Since `parent` is not NULL, it must be a valid pointer to a `struct device`. // - `parent` is valid for the lifetime of `self`, since a `struct device` holds a // reference count of its parent. - Some(unsafe { Self::as_ref(parent) }) + Some(unsafe { Self::from_raw(parent) }) } } @@ -161,7 +161,7 @@ impl Device { /// i.e. it must be ensured that the reference count of the C `struct device` `ptr` points to /// can't drop to zero, for the duration of this function call and the entire duration when the /// returned reference exists. - pub unsafe fn as_ref<'a>(ptr: *mut bindings::device) -> &'a Self { + pub unsafe fn from_raw<'a>(ptr: *mut bindings::device) -> &'a Self { // SAFETY: Guaranteed by the safety requirements of the function. unsafe { &*ptr.cast() } } diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 624d7a4c83ea..11ce1e7f2d52 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -190,7 +190,7 @@ impl AsRef for Device { fn as_ref(&self) -> &device::Device { // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid, // which is guaranteed by the type invariant. - unsafe { device::Device::as_ref((*self.as_raw()).dev) } + unsafe { device::Device::from_raw((*self.as_raw()).dev) } } } diff --git a/rust/kernel/faux.rs b/rust/kernel/faux.rs index 8a50fcd4c9bb..7a906099993f 100644 --- a/rust/kernel/faux.rs +++ b/rust/kernel/faux.rs @@ -54,7 +54,7 @@ impl AsRef for Registration { fn as_ref(&self) -> &device::Device { // SAFETY: The underlying `device` in `faux_device` is guaranteed by the C API to be // a valid initialized `device`. - unsafe { device::Device::as_ref(addr_of_mut!((*self.as_raw()).dev)) } + unsafe { device::Device::from_raw(addr_of_mut!((*self.as_raw()).dev)) } } } diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 4f7a8714ad36..443e5a3c3fe0 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -98,7 +98,7 @@ impl MiscDeviceRegistration { // function tells the borrow-checker that the `&Device` reference must not outlive the // `&MiscDeviceRegistration` used to obtain it, so the last use of the reference must be // before the underlying `struct miscdevice` is destroyed. - unsafe { Device::as_ref((*self.as_raw()).this_device) } + unsafe { Device::from_raw((*self.as_raw()).this_device) } } } diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 32ea43ece646..bd43a726f7d3 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -285,7 +285,7 @@ impl AsRef for Device { fn as_ref(&self) -> &kernel::device::Device { let phydev = self.0.get(); // SAFETY: The struct invariant ensures that `mdio.dev` is valid. - unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } + unsafe { kernel::device::Device::from_raw(addr_of_mut!((*phydev).mdio.dev)) } } } diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 8b884e324dcf..c9f7cb04d2e5 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -468,7 +468,7 @@ impl AsRef> for Device { let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; // SAFETY: `dev` points to a valid `struct device`. - unsafe { device::Device::as_ref(dev) } + unsafe { device::Device::from_raw(dev) } } } diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index ced43367d900..3d7a532550c8 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -251,7 +251,7 @@ impl AsRef> for Device { let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; // SAFETY: `dev` points to a valid `struct device`. - unsafe { device::Device::as_ref(dev) } + unsafe { device::Device::from_raw(dev) } } } -- cgit v1.2.3 From 8d84b32075fb2d9bd95c7e47b165942411d74bba Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Fri, 11 Jul 2025 13:09:45 +0900 Subject: rust: device_id: split out index support into a separate trait Introduce a new trait `RawDeviceIdIndex`, which extends `RawDeviceId` to provide support for device ID types that include an index or context field (e.g., `driver_data`). This separates the concerns of layout compatibility and index-based data embedding, and allows `RawDeviceId` to be implemented for types that do not contain a `driver_data` field. Several such structures are defined in include/linux/mod_devicetable.h. Refactor `IdArray::new()` into a generic `build()` function, which takes an optional offset. Based on the presence of `RawDeviceIdIndex`, index writing is conditionally enabled. A new `new_without_index()` constructor is also provided for use cases where no index should be written. This refactoring is a preparation for enabling the PHY abstractions to use the RawDeviceId trait. The changes to acpi.rs and driver.rs were made by Danilo. Reviewed-by: Trevor Gross Signed-off-by: FUJITA Tomonori Acked-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250711040947.1252162-2-fujita.tomonori@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/acpi.rs | 15 +++++--- rust/kernel/auxiliary.rs | 11 +++--- rust/kernel/device_id.rs | 91 +++++++++++++++++++++++++++++++++++------------- rust/kernel/driver.rs | 8 +++-- rust/kernel/of.rs | 15 +++++--- rust/kernel/pci.rs | 11 +++--- 6 files changed, 104 insertions(+), 47 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs index 2af4d4f92924..7ae317368b00 100644 --- a/rust/kernel/acpi.rs +++ b/rust/kernel/acpi.rs @@ -2,7 +2,11 @@ //! Advanced Configuration and Power Interface abstractions. -use crate::{bindings, device_id::RawDeviceId, prelude::*}; +use crate::{ + bindings, + device_id::{RawDeviceId, RawDeviceIdIndex}, + prelude::*, +}; /// IdTable type for ACPI drivers. pub type IdTable = &'static dyn kernel::device_id::IdTable; @@ -12,13 +16,14 @@ pub type IdTable = &'static dyn kernel::device_id::IdTable; #[derive(Clone, Copy)] pub struct DeviceId(bindings::acpi_device_id); -// SAFETY: -// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct acpi_device_id` and does not add -// additional invariants, so it's safe to transmute to `RawType`. -// * `DRIVER_DATA_OFFSET` is the offset to the `data` field. +// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `acpi_device_id` and does not add +// additional invariants, so it's safe to transmute to `RawType`. unsafe impl RawDeviceId for DeviceId { type RawType = bindings::acpi_device_id; +} +// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field. +unsafe impl RawDeviceIdIndex for DeviceId { const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data); fn index(&self) -> usize { diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index e1f2a2e6b607..4749fb6bffef 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -6,7 +6,7 @@ use crate::{ bindings, container_of, device, - device_id::RawDeviceId, + device_id::{RawDeviceId, RawDeviceIdIndex}, driver, error::{from_result, to_result, Result}, prelude::*, @@ -134,13 +134,14 @@ impl DeviceId { } } -// SAFETY: -// * `DeviceId` is a `#[repr(transparent)`] wrapper of `auxiliary_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. +// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `auxiliary_device_id` and does not add +// additional invariants, so it's safe to transmute to `RawType`. unsafe impl RawDeviceId for DeviceId { type RawType = bindings::auxiliary_device_id; +} +// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field. +unsafe impl RawDeviceIdIndex for DeviceId { const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::auxiliary_device_id, driver_data); diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs index 0a4eb56d98f2..8ed2c946144c 100644 --- a/rust/kernel/device_id.rs +++ b/rust/kernel/device_id.rs @@ -14,32 +14,41 @@ use core::mem::MaybeUninit; /// /// # Safety /// -/// Implementers must ensure that: -/// - `Self` is layout-compatible with [`RawDeviceId::RawType`]; i.e. it's safe to transmute to -/// `RawDeviceId`. +/// Implementers must ensure that `Self` is layout-compatible with [`RawDeviceId::RawType`]; +/// i.e. it's safe to transmute to `RawDeviceId`. /// -/// This requirement is needed so `IdArray::new` can convert `Self` to `RawType` when building -/// the ID table. +/// This requirement is needed so `IdArray::new` can convert `Self` to `RawType` when building +/// the ID table. /// -/// Ideally, this should be achieved using a const function that does conversion instead of -/// transmute; however, const trait functions relies on `const_trait_impl` unstable feature, -/// which is broken/gone in Rust 1.73. -/// -/// - `DRIVER_DATA_OFFSET` is the offset of context/data field of the device ID (usually named -/// `driver_data`) of the device ID, the field is suitable sized to write a `usize` value. -/// -/// Similar to the previous requirement, the data should ideally be added during `Self` to -/// `RawType` conversion, but there's currently no way to do it when using traits in const. +/// Ideally, this should be achieved using a const function that does conversion instead of +/// transmute; however, const trait functions relies on `const_trait_impl` unstable feature, +/// which is broken/gone in Rust 1.73. pub unsafe trait RawDeviceId { /// The raw type that holds the device id. /// /// Id tables created from [`Self`] are going to hold this type in its zero-terminated array. type RawType: Copy; +} - /// The offset to the context/data field. +/// Extension trait for [`RawDeviceId`] for devices that embed an index or context value. +/// +/// This is typically used when the device ID struct includes a field like `driver_data` +/// that is used to store a pointer-sized value (e.g., an index or context pointer). +/// +/// # Safety +/// +/// Implementers must ensure that `DRIVER_DATA_OFFSET` is the correct offset (in bytes) to +/// the context/data field (e.g., the `driver_data` field) within the raw device ID structure. +/// This field must be correctly sized to hold a `usize`. +/// +/// Ideally, the data should be added during `Self` to `RawType` conversion, +/// but there's currently no way to do it when using traits in const. +pub unsafe trait RawDeviceIdIndex: RawDeviceId { + /// The offset (in bytes) to the context/data field in the raw device ID. const DRIVER_DATA_OFFSET: usize; - /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceId`] trait. + /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceIdIndex`] + /// trait. fn index(&self) -> usize; } @@ -68,7 +77,15 @@ impl IdArray { /// Creates a new instance of the array. /// /// The contents are derived from the given identifiers and context information. - pub const fn new(ids: [(T, U); N]) -> Self { + /// + /// # Safety + /// + /// `data_offset` as `None` is always safe. + /// If `data_offset` is `Some(data_offset)`, then: + /// - `data_offset` must be the correct offset (in bytes) to the context/data field + /// (e.g., the `driver_data` field) within the raw device ID structure. + /// - The field at `data_offset` must be correctly sized to hold a `usize`. + const unsafe fn build(ids: [(T, U); N], data_offset: Option) -> Self { let mut raw_ids = [const { MaybeUninit::::uninit() }; N]; let mut infos = [const { MaybeUninit::uninit() }; N]; @@ -77,14 +94,16 @@ impl IdArray { // SAFETY: by the safety requirement of `RawDeviceId`, we're guaranteed that `T` is // layout-wise compatible with `RawType`. raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) }; - // SAFETY: by the safety requirement of `RawDeviceId`, this would be effectively - // `raw_ids[i].driver_data = i;`. - unsafe { - raw_ids[i] - .as_mut_ptr() - .byte_offset(T::DRIVER_DATA_OFFSET as _) - .cast::() - .write(i); + if let Some(data_offset) = data_offset { + // SAFETY: by the safety requirement of this function, this would be effectively + // `raw_ids[i].driver_data = i;`. + unsafe { + raw_ids[i] + .as_mut_ptr() + .byte_offset(data_offset as _) + .cast::() + .write(i); + } } // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but @@ -109,12 +128,34 @@ impl IdArray { } } + /// Creates a new instance of the array without writing index values. + /// + /// The contents are derived from the given identifiers and context information. + /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead. + pub const fn new_without_index(ids: [(T, U); N]) -> Self { + // SAFETY: Calling `Self::build` with `offset = None` is always safe, + // because no raw memory writes are performed in this case. + unsafe { Self::build(ids, None) } + } + /// Reference to the contained [`RawIdArray`]. pub const fn raw_ids(&self) -> &RawIdArray { &self.raw_ids } } +impl IdArray { + /// Creates a new instance of the array. + /// + /// The contents are derived from the given identifiers and context information. + pub const fn new(ids: [(T, U); N]) -> Self { + // SAFETY: by the safety requirement of `RawDeviceIdIndex`, + // `T::DRIVER_DATA_OFFSET` is guaranteed to be the correct offset (in bytes) to + // a field within `T::RawType`. + unsafe { Self::build(ids, Some(T::DRIVER_DATA_OFFSET)) } + } +} + /// A device id table. /// /// This trait is only implemented by `IdArray`. diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index f8dd7593e8dc..a8f2675ba7a7 100644 --- a/rust/kernel/driver.rs +++ b/rust/kernel/driver.rs @@ -170,7 +170,7 @@ pub trait Adapter { // and does not add additional invariants, so it's safe to transmute. let id = unsafe { &*raw_id.cast::() }; - Some(table.info(::index(id))) + Some(table.info(::index(id))) } } } @@ -204,7 +204,11 @@ pub trait Adapter { // and does not add additional invariants, so it's safe to transmute. let id = unsafe { &*raw_id.cast::() }; - Some(table.info(::index(id))) + Some( + table.info(::index( + id, + )), + ) } } } diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs index 52fe0c77b9a1..0888469bddb7 100644 --- a/rust/kernel/of.rs +++ b/rust/kernel/of.rs @@ -2,7 +2,11 @@ //! Device Tree / Open Firmware abstractions. -use crate::{bindings, device_id::RawDeviceId, prelude::*}; +use crate::{ + bindings, + device_id::{RawDeviceId, RawDeviceIdIndex}, + prelude::*, +}; /// IdTable type for OF drivers. pub type IdTable = &'static dyn kernel::device_id::IdTable; @@ -12,13 +16,14 @@ pub type IdTable = &'static dyn kernel::device_id::IdTable; #[derive(Clone, Copy)] pub struct DeviceId(bindings::of_device_id); -// SAFETY: -// * `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id` and does not add -// additional invariants, so it's safe to transmute to `RawType`. -// * `DRIVER_DATA_OFFSET` is the offset to the `data` field. +// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id` and +// does not add additional invariants, so it's safe to transmute to `RawType`. unsafe impl RawDeviceId for DeviceId { type RawType = bindings::of_device_id; +} +// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `data` field. +unsafe impl RawDeviceIdIndex for DeviceId { const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data); fn index(&self) -> usize { diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index c9f7cb04d2e5..6a4854cf4bf9 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -6,7 +6,7 @@ use crate::{ bindings, container_of, device, - device_id::RawDeviceId, + device_id::{RawDeviceId, RawDeviceIdIndex}, devres::Devres, driver, error::{from_result, to_result, Result}, @@ -159,13 +159,14 @@ impl DeviceId { } } -// 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. +// 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`. unsafe impl RawDeviceId for DeviceId { type RawType = bindings::pci_device_id; +} +// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field. +unsafe impl RawDeviceIdIndex for DeviceId { const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data); fn index(&self) -> usize { -- cgit v1.2.3 From f65a3218fd92cbe3e00f25427e1c9255b8973b31 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Fri, 11 Jul 2025 13:09:46 +0900 Subject: rust: net::phy represent DeviceId as transparent wrapper over mdio_device_id Refactor the DeviceId struct to be a #[repr(transparent)] wrapper around the C struct bindings::mdio_device_id. This refactoring is a preparation for enabling the PHY abstractions to use the RawDeviceId trait. Acked-by: Jakub Kicinski Reviewed-by: Trevor Gross Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250711040947.1252162-3-fujita.tomonori@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/net/phy.rs | 55 +++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 27 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index bd43a726f7d3..11a6809f3bea 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -507,7 +507,7 @@ pub const fn create_phy_driver() -> DriverVTable { DriverVTable(Opaque::new(bindings::phy_driver { name: T::NAME.as_char_ptr().cast_mut(), flags: T::FLAGS, - phy_id: T::PHY_DEVICE_ID.id, + phy_id: T::PHY_DEVICE_ID.id(), phy_id_mask: T::PHY_DEVICE_ID.mask_as_int(), soft_reset: if T::HAS_SOFT_RESET { Some(Adapter::::soft_reset_callback) @@ -691,42 +691,41 @@ impl Drop for Registration { /// /// Represents the kernel's `struct mdio_device_id`. This is used to find an appropriate /// PHY driver. -pub struct DeviceId { - id: u32, - mask: DeviceMask, -} +#[repr(transparent)] +#[derive(Clone, Copy)] +pub struct DeviceId(bindings::mdio_device_id); impl DeviceId { /// Creates a new instance with the exact match mask. pub const fn new_with_exact_mask(id: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Exact, - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Exact.as_int(), + }) } /// Creates a new instance with the model match mask. pub const fn new_with_model_mask(id: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Model, - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Model.as_int(), + }) } /// Creates a new instance with the vendor match mask. pub const fn new_with_vendor_mask(id: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Vendor, - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Vendor.as_int(), + }) } /// Creates a new instance with a custom match mask. pub const fn new_with_custom_mask(id: u32, mask: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Custom(mask), - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Custom(mask).as_int(), + }) } /// Creates a new instance from [`Driver`]. @@ -734,18 +733,20 @@ impl DeviceId { T::PHY_DEVICE_ID } - /// Get a `mask` as u32. + /// Get the MDIO device's PHY ID. + pub const fn id(&self) -> u32 { + self.0.phy_id + } + + /// Get the MDIO device's match mask. pub const fn mask_as_int(&self) -> u32 { - self.mask.as_int() + self.0.phy_id_mask } // macro use only #[doc(hidden)] pub const fn mdio_device_id(&self) -> bindings::mdio_device_id { - bindings::mdio_device_id { - phy_id: self.id, - phy_id_mask: self.mask.as_int(), - } + self.0 } } -- cgit v1.2.3 From 9a8682f0875b8cedad42bdfe601e6ab204fad06d Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Fri, 11 Jul 2025 13:09:47 +0900 Subject: rust: net::phy Change module_phy_driver macro to use module_device_table macro Change module_phy_driver macro to build device tables which are exported to userspace by using module_device_table macro. Acked-by: Jakub Kicinski Reviewed-by: Trevor Gross Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250711040947.1252162-4-fujita.tomonori@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/net/phy.rs | 51 ++++++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 27 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 11a6809f3bea..95bd17d3e927 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -6,7 +6,7 @@ //! //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). -use crate::{error::*, prelude::*, types::Opaque}; +use crate::{device_id::RawDeviceId, error::*, prelude::*, types::Opaque}; use core::{marker::PhantomData, ptr::addr_of_mut}; pub mod reg; @@ -750,6 +750,12 @@ impl DeviceId { } } +// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct mdio_device_id` +// and does not add additional invariants, so it's safe to transmute to `RawType`. +unsafe impl RawDeviceId for DeviceId { + type RawType = bindings::mdio_device_id; +} + enum DeviceMask { Exact, Model, @@ -850,19 +856,18 @@ impl DeviceMask { /// } /// }; /// -/// const _DEVICE_TABLE: [::kernel::bindings::mdio_device_id; 2] = [ -/// ::kernel::bindings::mdio_device_id { -/// phy_id: 0x00000001, -/// phy_id_mask: 0xffffffff, -/// }, -/// ::kernel::bindings::mdio_device_id { -/// phy_id: 0, -/// phy_id_mask: 0, -/// }, -/// ]; -/// #[cfg(MODULE)] -/// #[no_mangle] -/// static __mod_device_table__mdio__phydev: [::kernel::bindings::mdio_device_id; 2] = _DEVICE_TABLE; +/// const N: usize = 1; +/// +/// const TABLE: ::kernel::device_id::IdArray<::kernel::net::phy::DeviceId, (), N> = +/// ::kernel::device_id::IdArray::new_without_index([ +/// ::kernel::net::phy::DeviceId( +/// ::kernel::bindings::mdio_device_id { +/// phy_id: 0x00000001, +/// phy_id_mask: 0xffffffff, +/// }), +/// ]); +/// +/// ::kernel::module_device_table!("mdio", phydev, TABLE); /// ``` #[macro_export] macro_rules! module_phy_driver { @@ -873,20 +878,12 @@ macro_rules! module_phy_driver { }; (@device_table [$($dev:expr),+]) => { - // SAFETY: C will not read off the end of this constant since the last element is zero. - const _DEVICE_TABLE: [$crate::bindings::mdio_device_id; - $crate::module_phy_driver!(@count_devices $($dev),+) + 1] = [ - $($dev.mdio_device_id()),+, - $crate::bindings::mdio_device_id { - phy_id: 0, - phy_id_mask: 0 - } - ]; + const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+); + + const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> = + $crate::device_id::IdArray::new_without_index([ $(($dev,())),+, ]); - #[cfg(MODULE)] - #[no_mangle] - static __mod_device_table__mdio__phydev: [$crate::bindings::mdio_device_id; - $crate::module_phy_driver!(@count_devices $($dev),+) + 1] = _DEVICE_TABLE; + $crate::module_device_table!("mdio", phydev, TABLE); }; (drivers: [$($driver:ident),+ $(,)?], device_table: [$($dev:expr),+ $(,)?], $($f:tt)*) => { -- cgit v1.2.3 From d06d5f66f5494e29c1520583eac8382f0ab5f8a7 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 16 Jul 2025 17:02:46 +0200 Subject: rust: dma: implement `dma::Device` trait Add a trait that defines the DMA specific methods of devices. The `dma::Device` trait is to be implemented by bus device representations, where the underlying bus is capable of DMA, such as `pci::Device` or `platform::Device`. Reviewed-by: Abdiel Janulgue Reviewed-by: Daniel Almeida Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250716150354.51081-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/dma.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index a33261c62e0c..586524488e5d 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -5,14 +5,21 @@ //! C header: [`include/linux/dma-mapping.h`](srctree/include/linux/dma-mapping.h) use crate::{ - bindings, build_assert, - device::{Bound, Device}, + bindings, build_assert, device, + device::Bound, error::code::*, error::Result, transmute::{AsBytes, FromBytes}, types::ARef, }; +/// Trait to be implemented by DMA capable bus devices. +/// +/// The [`dma::Device`](Device) trait should be implemented by bus specific device representations, +/// where the underlying bus is DMA capable, such as [`pci::Device`](::kernel::pci::Device) or +/// [`platform::Device`](::kernel::platform::Device). +pub trait Device: AsRef> {} + /// Possible attributes associated with a DMA mapping. /// /// They can be combined with the operators `|`, `&`, and `!`. @@ -130,7 +137,7 @@ pub mod attrs { // Hence, find a way to revoke the device resources of a `CoherentAllocation`, but not the // entire `CoherentAllocation` including the allocated memory itself. pub struct CoherentAllocation { - dev: ARef, + dev: ARef, dma_handle: bindings::dma_addr_t, count: usize, cpu_addr: *mut T, @@ -152,7 +159,7 @@ impl CoherentAllocation { /// # Ok::<(), Error>(()) } /// ``` pub fn alloc_attrs( - dev: &Device, + dev: &device::Device, count: usize, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, @@ -194,7 +201,7 @@ impl CoherentAllocation { /// Performs the same functionality as [`CoherentAllocation::alloc_attrs`], except the /// `dma_attrs` is 0 by default. pub fn alloc_coherent( - dev: &Device, + dev: &device::Device, count: usize, gfp_flags: kernel::alloc::Flags, ) -> Result> { -- cgit v1.2.3 From 101d66828a4eefb89b72256155feb6ec9abc144a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 16 Jul 2025 17:02:47 +0200 Subject: rust: dma: add DMA addressing capabilities Implement `dma_set_mask()`, `dma_set_coherent_mask()` and `dma_set_mask_and_coherent()` in the `dma::Device` trait. Those methods are used to set up the device's DMA addressing capabilities. Reviewed-by: Abdiel Janulgue Reviewed-by: Daniel Almeida Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250716150354.51081-3-dakr@kernel.org [ Add DmaMask::try_new(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/dma.rs | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 139 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index 586524488e5d..99dcf79f0897 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -6,9 +6,9 @@ use crate::{ bindings, build_assert, device, - device::Bound, - error::code::*, - error::Result, + device::{Bound, Core}, + error::{to_result, Result}, + prelude::*, transmute::{AsBytes, FromBytes}, types::ARef, }; @@ -18,7 +18,142 @@ use crate::{ /// The [`dma::Device`](Device) trait should be implemented by bus specific device representations, /// where the underlying bus is DMA capable, such as [`pci::Device`](::kernel::pci::Device) or /// [`platform::Device`](::kernel::platform::Device). -pub trait Device: AsRef> {} +pub trait Device: AsRef> { + /// Set up the device's DMA streaming addressing capabilities. + /// + /// This method is usually called once from `probe()` as soon as the device capabilities are + /// known. + /// + /// # Safety + /// + /// This method must not be called concurrently with any DMA allocation or mapping primitives, + /// such as [`CoherentAllocation::alloc_attrs`]. + unsafe fn dma_set_mask(&self, mask: DmaMask) -> Result { + // SAFETY: + // - By the type invariant of `device::Device`, `self.as_ref().as_raw()` is valid. + // - The safety requirement of this function guarantees that there are no concurrent calls + // to DMA allocation and mapping primitives using this mask. + to_result(unsafe { bindings::dma_set_mask(self.as_ref().as_raw(), mask.value()) }) + } + + /// Set up the device's DMA coherent addressing capabilities. + /// + /// This method is usually called once from `probe()` as soon as the device capabilities are + /// known. + /// + /// # Safety + /// + /// This method must not be called concurrently with any DMA allocation or mapping primitives, + /// such as [`CoherentAllocation::alloc_attrs`]. + unsafe fn dma_set_coherent_mask(&self, mask: DmaMask) -> Result { + // SAFETY: + // - By the type invariant of `device::Device`, `self.as_ref().as_raw()` is valid. + // - The safety requirement of this function guarantees that there are no concurrent calls + // to DMA allocation and mapping primitives using this mask. + to_result(unsafe { bindings::dma_set_coherent_mask(self.as_ref().as_raw(), mask.value()) }) + } + + /// Set up the device's DMA addressing capabilities. + /// + /// This is a combination of [`Device::dma_set_mask`] and [`Device::dma_set_coherent_mask`]. + /// + /// This method is usually called once from `probe()` as soon as the device capabilities are + /// known. + /// + /// # Safety + /// + /// This method must not be called concurrently with any DMA allocation or mapping primitives, + /// such as [`CoherentAllocation::alloc_attrs`]. + unsafe fn dma_set_mask_and_coherent(&self, mask: DmaMask) -> Result { + // SAFETY: + // - By the type invariant of `device::Device`, `self.as_ref().as_raw()` is valid. + // - The safety requirement of this function guarantees that there are no concurrent calls + // to DMA allocation and mapping primitives using this mask. + to_result(unsafe { + bindings::dma_set_mask_and_coherent(self.as_ref().as_raw(), mask.value()) + }) + } +} + +/// A DMA mask that holds a bitmask with the lowest `n` bits set. +/// +/// Use [`DmaMask::new`] or [`DmaMask::try_new`] to construct a value. Values +/// are guaranteed to never exceed the bit width of `u64`. +/// +/// This is the Rust equivalent of the C macro `DMA_BIT_MASK()`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct DmaMask(u64); + +impl DmaMask { + /// Constructs a `DmaMask` with the lowest `n` bits set to `1`. + /// + /// For `n <= 64`, sets exactly the lowest `n` bits. + /// For `n > 64`, results in a build error. + /// + /// # Examples + /// + /// ``` + /// use kernel::dma::DmaMask; + /// + /// let mask0 = DmaMask::new::<0>(); + /// assert_eq!(mask0.value(), 0); + /// + /// let mask1 = DmaMask::new::<1>(); + /// assert_eq!(mask1.value(), 0b1); + /// + /// let mask64 = DmaMask::new::<64>(); + /// assert_eq!(mask64.value(), u64::MAX); + /// + /// // Build failure. + /// // let mask_overflow = DmaMask::new::<100>(); + /// ``` + #[inline] + pub const fn new() -> Self { + let Ok(mask) = Self::try_new(N) else { + build_error!("Invalid DMA Mask."); + }; + + mask + } + + /// Constructs a `DmaMask` with the lowest `n` bits set to `1`. + /// + /// For `n <= 64`, sets exactly the lowest `n` bits. + /// For `n > 64`, returns [`EINVAL`]. + /// + /// # Examples + /// + /// ``` + /// use kernel::dma::DmaMask; + /// + /// let mask0 = DmaMask::try_new(0)?; + /// assert_eq!(mask0.value(), 0); + /// + /// let mask1 = DmaMask::try_new(1)?; + /// assert_eq!(mask1.value(), 0b1); + /// + /// let mask64 = DmaMask::try_new(64)?; + /// assert_eq!(mask64.value(), u64::MAX); + /// + /// let mask_overflow = DmaMask::try_new(100); + /// assert!(mask_overflow.is_err()); + /// # Ok::<(), Error>(()) + /// ``` + #[inline] + pub const fn try_new(n: u32) -> Result { + Ok(Self(match n { + 0 => 0, + 1..=64 => u64::MAX >> (64 - n), + _ => return Err(EINVAL), + })) + } + + /// Returns the underlying `u64` bitmask value. + #[inline] + pub const fn value(&self) -> u64 { + self.0 + } +} /// Possible attributes associated with a DMA mapping. /// -- cgit v1.2.3 From 8eb698f54736b44a398ca883d0a33f661c12da9d Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 16 Jul 2025 17:02:48 +0200 Subject: rust: pci: implement the `dma::Device` trait The PCI bus is potentially capable of performing DMA, hence implement the `dma:Device` trait for `pci::Device`. Reviewed-by: Daniel Almeida Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250716150354.51081-4-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 6a4854cf4bf9..44a2f3d2884a 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -449,6 +449,8 @@ impl Device { kernel::impl_device_context_deref!(unsafe { Device }); kernel::impl_device_context_into_aref!(Device); +impl crate::dma::Device for Device {} + // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { fn inc_ref(&self) { -- cgit v1.2.3 From 256de48f2cad85153b271b66320a9355773db64e Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 16 Jul 2025 17:02:49 +0200 Subject: rust: platform: implement the `dma::Device` trait The platform bus is potentially capable of performing DMA, hence implement the `dma:Device` trait for `platform::Device`. Reviewed-by: Daniel Almeida Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250716150354.51081-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 3d7a532550c8..3c0c507c2432 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -231,6 +231,8 @@ impl Device { kernel::impl_device_context_deref!(unsafe { Device }); kernel::impl_device_context_into_aref!(Device); +impl crate::dma::Device for Device {} + // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { fn inc_ref(&self) { -- cgit v1.2.3 From 493fc33ec25294cb2e444dfa77c105aa774c83f2 Mon Sep 17 00:00:00 2001 From: Daniel Almeida Date: Thu, 17 Jul 2025 12:55:22 -0300 Subject: rust: io: add resource abstraction In preparation for ioremap support, add a Rust abstraction for struct resource. A future commit will introduce the Rust API to ioremap a resource from a platform device. The current abstraction, therefore, adds only the minimum API needed to get that done. Acked-by: Miguel Ojeda Reviewed-by: Alice Ryhl Co-developed-by: Fiona Behrens Signed-off-by: Fiona Behrens Signed-off-by: Daniel Almeida Link: https://lore.kernel.org/r/20250717-topics-tyr-platform_iomem-v15-1-beca780b77e3@collabora.com [ Capitalize safety comments and end it with a period. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/io.rs | 4 + rust/kernel/io/resource.rs | 229 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+) create mode 100644 rust/kernel/io/resource.rs (limited to 'rust/kernel') diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index 72d80a6f131e..7b70d5b5477e 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -7,6 +7,10 @@ use crate::error::{code::EINVAL, Result}; use crate::{bindings, build_assert}; +pub mod resource; + +pub use resource::Resource; + /// Raw representation of an MMIO region. /// /// By itself, the existence of an instance of this structure does not provide any guarantees that diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs new file mode 100644 index 000000000000..1d5f367a6e8a --- /dev/null +++ b/rust/kernel/io/resource.rs @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Abstractions for [system +//! resources](https://docs.kernel.org/core-api/kernel-api.html#resources-management). +//! +//! C header: [`include/linux/ioport.h`](srctree/include/linux/ioport.h) + +use core::ops::Deref; +use core::ptr::NonNull; + +use crate::prelude::*; +use crate::str::{CStr, CString}; +use crate::types::Opaque; + +/// Resource Size type. +/// +/// This is a type alias to either `u32` or `u64` depending on the config option +/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures. +pub type ResourceSize = bindings::phys_addr_t; + +/// A region allocated from a parent [`Resource`]. +/// +/// # Invariants +/// +/// - `self.0` points to a valid `bindings::resource` that was obtained through +/// `bindings::__request_region`. +pub struct Region { + /// The resource returned when the region was requested. + resource: NonNull, + /// The name that was passed in when the region was requested. We need to + /// store it for ownership reasons. + _name: CString, +} + +impl Deref for Region { + type Target = Resource; + + fn deref(&self) -> &Self::Target { + // SAFETY: Safe as per the invariant of `Region`. + unsafe { Resource::from_raw(self.resource.as_ptr()) } + } +} + +impl Drop for Region { + fn drop(&mut self) { + let (flags, start, size) = { + let res = &**self; + (res.flags(), res.start(), res.size()) + }; + + let release_fn = if flags.contains(Flags::IORESOURCE_MEM) { + bindings::release_mem_region + } else { + bindings::release_region + }; + + // SAFETY: Safe as per the invariant of `Region`. + unsafe { release_fn(start, size) }; + } +} + +// SAFETY: `Region` only holds a pointer to a C `struct resource`, which is safe to be used from +// any thread. +unsafe impl Send for Region {} + +// SAFETY: `Region` only holds a pointer to a C `struct resource`, references to which are +// safe to be used from any thread. +unsafe impl Sync for Region {} + +/// A resource abstraction. +/// +/// # Invariants +/// +/// [`Resource`] is a transparent wrapper around a valid `bindings::resource`. +#[repr(transparent)] +pub struct Resource(Opaque); + +impl Resource { + /// Creates a reference to a [`Resource`] from a valid pointer. + /// + /// # Safety + /// + /// The caller must ensure that for the duration of 'a, the pointer will + /// point at a valid `bindings::resource`. + /// + /// The caller must also ensure that the [`Resource`] is only accessed via the + /// returned reference for the duration of 'a. + pub(crate) const unsafe fn from_raw<'a>(ptr: *mut bindings::resource) -> &'a Self { + // SAFETY: Self is a transparent wrapper around `Opaque`. + unsafe { &*ptr.cast() } + } + + /// Requests a resource region. + /// + /// Exclusive access will be given and the region will be marked as busy. + /// Further calls to [`Self::request_region`] will return [`None`] if + /// the region, or a part of it, is already in use. + pub fn request_region( + &self, + start: ResourceSize, + size: ResourceSize, + name: CString, + flags: Flags, + ) -> Option { + // SAFETY: + // - Safe as per the invariant of `Resource`. + // - `__request_region` will store a reference to the name, but that is + // safe as we own it and it will not be dropped until the `Region` is + // dropped. + let region = unsafe { + bindings::__request_region( + self.0.get(), + start, + size, + name.as_char_ptr(), + flags.0 as c_int, + ) + }; + + Some(Region { + resource: NonNull::new(region)?, + _name: name, + }) + } + + /// Returns the size of the resource. + pub fn size(&self) -> ResourceSize { + let inner = self.0.get(); + // SAFETY: Safe as per the invariants of `Resource`. + unsafe { bindings::resource_size(inner) } + } + + /// Returns the start address of the resource. + pub fn start(&self) -> ResourceSize { + let inner = self.0.get(); + // SAFETY: Safe as per the invariants of `Resource`. + unsafe { (*inner).start } + } + + /// Returns the name of the resource. + pub fn name(&self) -> Option<&CStr> { + let inner = self.0.get(); + + // SAFETY: Safe as per the invariants of `Resource`. + let name = unsafe { (*inner).name }; + + if name.is_null() { + return None; + } + + // SAFETY: In the C code, `resource::name` either contains a null + // pointer or points to a valid NUL-terminated C string, and at this + // point we know it is not null, so we can safely convert it to a + // `CStr`. + Some(unsafe { CStr::from_char_ptr(name) }) + } + + /// Returns the flags associated with the resource. + pub fn flags(&self) -> Flags { + let inner = self.0.get(); + // SAFETY: Safe as per the invariants of `Resource`. + let flags = unsafe { (*inner).flags }; + + Flags(flags) + } +} + +// SAFETY: `Resource` only holds a pointer to a C `struct resource`, which is +// safe to be used from any thread. +unsafe impl Send for Resource {} + +// SAFETY: `Resource` only holds a pointer to a C `struct resource`, references +// to which are safe to be used from any thread. +unsafe impl Sync for Resource {} + +/// Resource flags as stored in the C `struct resource::flags` field. +/// +/// They can be combined with the operators `|`, `&`, and `!`. +/// +/// Values can be used from the [`flags`] module. +#[derive(Clone, Copy, PartialEq)] +pub struct Flags(c_ulong); + +impl Flags { + /// Check whether `flags` is contained in `self`. + pub fn contains(self, flags: Flags) -> bool { + (self & flags) == flags + } +} + +impl core::ops::BitOr for Flags { + type Output = Self; + fn bitor(self, rhs: Self) -> Self::Output { + Self(self.0 | rhs.0) + } +} + +impl core::ops::BitAnd for Flags { + type Output = Self; + fn bitand(self, rhs: Self) -> Self::Output { + Self(self.0 & rhs.0) + } +} + +impl core::ops::Not for Flags { + type Output = Self; + fn not(self) -> Self::Output { + Self(!self.0) + } +} + +impl Flags { + /// PCI/ISA I/O ports. + pub const IORESOURCE_IO: Flags = Flags::new(bindings::IORESOURCE_IO); + + /// Resource is software muxed. + pub const IORESOURCE_MUXED: Flags = Flags::new(bindings::IORESOURCE_MUXED); + + /// Resource represents a memory region. + pub const IORESOURCE_MEM: Flags = Flags::new(bindings::IORESOURCE_MEM); + + /// Resource represents a memory region that must be ioremaped using `ioremap_np`. + pub const IORESOURCE_MEM_NONPOSTED: Flags = Flags::new(bindings::IORESOURCE_MEM_NONPOSTED); + + const fn new(value: u32) -> Self { + crate::build_assert!(value as u64 <= c_ulong::MAX as u64); + Flags(value as c_ulong) + } +} -- cgit v1.2.3 From 1d0d4b28513b5e0e9e87e09c8da289e1b8d88f84 Mon Sep 17 00:00:00 2001 From: Daniel Almeida Date: Thu, 17 Jul 2025 12:55:23 -0300 Subject: rust: io: mem: add a generic iomem abstraction Add a generic iomem abstraction to safely read and write ioremapped regions. This abstraction requires a previously acquired IoRequest instance. This makes it so that both the resource and the device match, or, in other words, that the resource is indeed a valid resource for a given bound device. A subsequent patch will add the ability to retrieve IoRequest instances from platform devices. The reads and writes are done through IoRaw, and are thus checked either at compile-time, if the size of the region is known at that point, or at runtime otherwise. Non-exclusive access to the underlying memory region is made possible to cater to cases where overlapped regions are unavoidable. Acked-by: Miguel Ojeda Reviewed-by: Alice Ryhl Signed-off-by: Daniel Almeida Link: https://lore.kernel.org/r/20250717-topics-tyr-platform_iomem-v15-2-beca780b77e3@collabora.com [ Add #[expect(dead_code)] to avoid a temporary warning, remove unnecessary OF_ID_TABLE constants in doc-tests and ignore doc-tests for now to avoid a temporary build failure. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/io.rs | 1 + rust/kernel/io/mem.rs | 280 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 281 insertions(+) create mode 100644 rust/kernel/io/mem.rs (limited to 'rust/kernel') diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index 7b70d5b5477e..b7fc759f8b5d 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -7,6 +7,7 @@ use crate::error::{code::EINVAL, Result}; use crate::{bindings, build_assert}; +pub mod mem; pub mod resource; pub use resource::Resource; diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs new file mode 100644 index 000000000000..8e4dfd79079e --- /dev/null +++ b/rust/kernel/io/mem.rs @@ -0,0 +1,280 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Generic memory-mapped IO. + +use core::ops::Deref; + +use crate::c_str; +use crate::device::Bound; +use crate::device::Device; +use crate::devres::Devres; +use crate::io; +use crate::io::resource::Region; +use crate::io::resource::Resource; +use crate::io::Io; +use crate::io::IoRaw; +use crate::prelude::*; + +/// An IO request for a specific device and resource. +pub struct IoRequest<'a> { + device: &'a Device, + resource: &'a Resource, +} + +impl<'a> IoRequest<'a> { + /// Creates a new [`IoRequest`] instance. + /// + /// # Safety + /// + /// Callers must ensure that `resource` is valid for `device` during the + /// lifetime `'a`. + #[expect(dead_code)] + pub(crate) unsafe fn new(device: &'a Device, resource: &'a Resource) -> Self { + IoRequest { device, resource } + } + + /// Maps an [`IoRequest`] where the size is known at compile time. + /// + /// This uses the [`ioremap()`] C API. + /// + /// [`ioremap()`]: https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device + /// + /// # Examples + /// + /// The following example uses a [`platform::Device`] for illustration + /// purposes. + /// + /// ```ignore + /// use kernel::{bindings, c_str, platform, of, device::Core}; + /// struct SampleDriver; + /// + /// impl platform::Driver for SampleDriver { + /// # type IdInfo = (); + /// + /// fn probe( + /// pdev: &platform::Device, + /// info: Option<&Self::IdInfo>, + /// ) -> Result>> { + /// let offset = 0; // Some offset. + /// + /// // If the size is known at compile time, use [`Self::iomap_sized`]. + /// // + /// // No runtime checks will apply when reading and writing. + /// let request = pdev.io_request_by_index(0).ok_or(ENODEV)?; + /// let iomem = request.iomap_sized::<42>(); + /// let iomem = KBox::pin_init(iomem, GFP_KERNEL)?; + /// + /// let io = iomem.access(pdev.as_ref())?; + /// + /// // Read and write a 32-bit value at `offset`. + /// let data = io.read32_relaxed(offset); + /// + /// io.write32_relaxed(data, offset); + /// + /// # Ok(KBox::new(SampleDriver, GFP_KERNEL)?.into()) + /// } + /// } + /// ``` + pub fn iomap_sized(self) -> impl PinInit>, Error> + 'a { + IoMem::new(self) + } + + /// Same as [`Self::iomap_sized`] but with exclusive access to the + /// underlying region. + /// + /// This uses the [`ioremap()`] C API. + /// + /// [`ioremap()`]: https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device + pub fn iomap_exclusive_sized( + self, + ) -> impl PinInit>, Error> + 'a { + ExclusiveIoMem::new(self) + } + + /// Maps an [`IoRequest`] where the size is not known at compile time, + /// + /// This uses the [`ioremap()`] C API. + /// + /// [`ioremap()`]: https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device + /// + /// # Examples + /// + /// The following example uses a [`platform::Device`] for illustration + /// purposes. + /// + /// ```ignore + /// use kernel::{bindings, c_str, platform, of, device::Core}; + /// struct SampleDriver; + /// + /// impl platform::Driver for SampleDriver { + /// # type IdInfo = (); + /// + /// fn probe( + /// pdev: &platform::Device, + /// info: Option<&Self::IdInfo>, + /// ) -> Result>> { + /// let offset = 0; // Some offset. + /// + /// // Unlike [`Self::iomap_sized`], here the size of the memory region + /// // is not known at compile time, so only the `try_read*` and `try_write*` + /// // family of functions should be used, leading to runtime checks on every + /// // access. + /// let request = pdev.io_request_by_index(0).ok_or(ENODEV)?; + /// let iomem = request.iomap(); + /// let iomem = KBox::pin_init(iomem, GFP_KERNEL)?; + /// + /// let io = iomem.access(pdev.as_ref())?; + /// + /// let data = io.try_read32_relaxed(offset)?; + /// + /// io.try_write32_relaxed(data, offset)?; + /// + /// # Ok(KBox::new(SampleDriver, GFP_KERNEL)?.into()) + /// } + /// } + /// ``` + pub fn iomap(self) -> impl PinInit>, Error> + 'a { + Self::iomap_sized::<0>(self) + } + + /// Same as [`Self::iomap`] but with exclusive access to the underlying + /// region. + pub fn iomap_exclusive(self) -> impl PinInit>, Error> + 'a { + Self::iomap_exclusive_sized::<0>(self) + } +} + +/// An exclusive memory-mapped IO region. +/// +/// # Invariants +/// +/// - [`ExclusiveIoMem`] has exclusive access to the underlying [`IoMem`]. +pub struct ExclusiveIoMem { + /// The underlying `IoMem` instance. + iomem: IoMem, + + /// The region abstraction. This represents exclusive access to the + /// range represented by the underlying `iomem`. + /// + /// This field is needed for ownership of the region. + _region: Region, +} + +impl ExclusiveIoMem { + /// Creates a new `ExclusiveIoMem` instance. + fn ioremap(resource: &Resource) -> Result { + let start = resource.start(); + let size = resource.size(); + let name = resource.name().unwrap_or(c_str!("")); + + let region = resource + .request_region( + start, + size, + name.to_cstring()?, + io::resource::Flags::IORESOURCE_MEM, + ) + .ok_or(EBUSY)?; + + let iomem = IoMem::ioremap(resource)?; + + let iomem = ExclusiveIoMem { + iomem, + _region: region, + }; + + Ok(iomem) + } + + /// Creates a new `ExclusiveIoMem` instance from a previously acquired [`IoRequest`]. + pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit, Error> + 'a { + let dev = io_request.device; + let res = io_request.resource; + + Devres::new(dev, Self::ioremap(res)) + } +} + +impl Deref for ExclusiveIoMem { + type Target = Io; + + fn deref(&self) -> &Self::Target { + &self.iomem + } +} + +/// A generic memory-mapped IO region. +/// +/// Accesses to the underlying region is checked either at compile time, if the +/// region's size is known at that point, or at runtime otherwise. +/// +/// # Invariants +/// +/// [`IoMem`] always holds an [`IoRaw`] instance that holds a valid pointer to the +/// start of the I/O memory mapped region. +pub struct IoMem { + io: IoRaw, +} + +impl IoMem { + fn ioremap(resource: &Resource) -> Result { + // Note: Some ioremap() implementations use types that depend on the CPU + // word width rather than the bus address width. + // + // TODO: Properly address this in the C code to avoid this `try_into`. + let size = resource.size().try_into()?; + if size == 0 { + return Err(EINVAL); + } + + let res_start = resource.start(); + + let addr = if resource + .flags() + .contains(io::resource::Flags::IORESOURCE_MEM_NONPOSTED) + { + // SAFETY: + // - `res_start` and `size` are read from a presumably valid `struct resource`. + // - `size` is known not to be zero at this point. + unsafe { bindings::ioremap_np(res_start, size) } + } else { + // SAFETY: + // - `res_start` and `size` are read from a presumably valid `struct resource`. + // - `size` is known not to be zero at this point. + unsafe { bindings::ioremap(res_start, size) } + }; + + if addr.is_null() { + return Err(ENOMEM); + } + + let io = IoRaw::new(addr as usize, size)?; + let io = IoMem { io }; + + Ok(io) + } + + /// Creates a new `IoMem` instance from a previously acquired [`IoRequest`]. + pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit, Error> + 'a { + let dev = io_request.device; + let res = io_request.resource; + + Devres::new(dev, Self::ioremap(res)) + } +} + +impl Drop for IoMem { + fn drop(&mut self) { + // SAFETY: Safe as by the invariant of `Io`. + unsafe { bindings::iounmap(self.io.addr() as *mut c_void) } + } +} + +impl Deref for IoMem { + type Target = Io; + + fn deref(&self) -> &Self::Target { + // SAFETY: Safe as by the invariant of `IoMem`. + unsafe { Io::from_raw(&self.io) } + } +} -- cgit v1.2.3 From bc4f9045a59963abb142f6a648195ccd73ec39dd Mon Sep 17 00:00:00 2001 From: Daniel Almeida Date: Thu, 17 Jul 2025 12:55:24 -0300 Subject: rust: platform: add resource accessors The previous patches have added the abstractions for Resources and the ability to map them and therefore read and write the underlying memory . The only thing missing to make this accessible for platform devices is to provide accessors that return instances of IoRequest<'a>. These ensure that the resource are valid only for the lifetime of the platform device, and that the platform device is in the Bound state. Therefore, add these accessors. Also make it possible to retrieve resources from platform devices in Rust using either a name or an index. Acked-by: Miguel Ojeda Signed-off-by: Daniel Almeida Link: https://lore.kernel.org/r/20250717-topics-tyr-platform_iomem-v15-3-beca780b77e3@collabora.com [ Remove #[expect(dead_code)] from IoRequest::new() and move SAFETY comments right on top of unsafe blocks to avoid clippy warnings for some (older) clippy versions. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/io/mem.rs | 1 - rust/kernel/platform.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs index 8e4dfd79079e..50d4ec3eb623 100644 --- a/rust/kernel/io/mem.rs +++ b/rust/kernel/io/mem.rs @@ -28,7 +28,6 @@ impl<'a> IoRequest<'a> { /// /// Callers must ensure that `resource` is valid for `device` during the /// lifetime `'a`. - #[expect(dead_code)] pub(crate) unsafe fn new(device: &'a Device, resource: &'a Resource) -> Self { IoRequest { device, resource } } diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 3c0c507c2432..b4d3087aff52 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -5,8 +5,11 @@ //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) use crate::{ - acpi, bindings, container_of, device, driver, + acpi, bindings, container_of, + device::{self, Bound}, + driver, error::{from_result, to_result, Result}, + io::{mem::IoRequest, Resource}, of, prelude::*, types::Opaque, @@ -224,6 +227,61 @@ impl Device { fn as_raw(&self) -> *mut bindings::platform_device { self.0.get() } + + /// Returns the resource at `index`, if any. + pub fn resource_by_index(&self, index: u32) -> Option<&Resource> { + // SAFETY: `self.as_raw()` returns a valid pointer to a `struct platform_device`. + let resource = unsafe { + bindings::platform_get_resource(self.as_raw(), bindings::IORESOURCE_MEM, index) + }; + + if resource.is_null() { + return None; + } + + // SAFETY: `resource` is a valid pointer to a `struct resource` as + // returned by `platform_get_resource`. + Some(unsafe { Resource::from_raw(resource) }) + } + + /// Returns the resource with a given `name`, if any. + pub fn resource_by_name(&self, name: &CStr) -> Option<&Resource> { + // SAFETY: `self.as_raw()` returns a valid pointer to a `struct + // platform_device` and `name` points to a valid C string. + let resource = unsafe { + bindings::platform_get_resource_byname( + self.as_raw(), + bindings::IORESOURCE_MEM, + name.as_char_ptr(), + ) + }; + + if resource.is_null() { + return None; + } + + // SAFETY: `resource` is a valid pointer to a `struct resource` as + // returned by `platform_get_resource`. + Some(unsafe { Resource::from_raw(resource) }) + } +} + +impl Device { + /// Returns an `IoRequest` for the resource at `index`, if any. + pub fn io_request_by_index(&self, index: u32) -> Option> { + self.resource_by_index(index) + // SAFETY: `resource` is a valid resource for `&self` during the + // lifetime of the `IoRequest`. + .map(|resource| unsafe { IoRequest::new(self.as_ref(), resource) }) + } + + /// Returns an `IoRequest` for the resource with a given `name`, if any. + pub fn io_request_by_name(&self, name: &CStr) -> Option> { + self.resource_by_name(name) + // SAFETY: `resource` is a valid resource for `&self` during the + // lifetime of the `IoRequest`. + .map(|resource| unsafe { IoRequest::new(self.as_ref(), resource) }) + } } // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic -- cgit v1.2.3 From 696b2a6ce9487ae278fd239658ca2714cd211e8e Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 19 Jul 2025 16:08:16 +0200 Subject: rust: io: mem: enable IoRequest doc-tests When introduced, the IoRequest doc-tests did depend on infrastructure added in subsequent patches, hence they temporarily had to be disabled. Now that we have the corresponding platform device infrastructure, enable them. Link: https://lore.kernel.org/r/DBG39YMN2TX6.1VR4PEQSI8PSG@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/io/mem.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs index 50d4ec3eb623..cc9feb2897d8 100644 --- a/rust/kernel/io/mem.rs +++ b/rust/kernel/io/mem.rs @@ -43,7 +43,7 @@ impl<'a> IoRequest<'a> { /// The following example uses a [`platform::Device`] for illustration /// purposes. /// - /// ```ignore + /// ```no_run /// use kernel::{bindings, c_str, platform, of, device::Core}; /// struct SampleDriver; /// @@ -101,7 +101,7 @@ impl<'a> IoRequest<'a> { /// The following example uses a [`platform::Device`] for illustration /// purposes. /// - /// ```ignore + /// ```no_run /// use kernel::{bindings, c_str, platform, of, device::Core}; /// struct SampleDriver; /// -- cgit v1.2.3 From 83fb6160727874a588d70b727bc34f367362e6ed Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 22 Jul 2025 10:54:59 +0200 Subject: rust: io: fix broken intra-doc link to missing `flags` module There is no `mod flags` in this case, unlike others. Instead, they are associated constants for the `Flags` type. Thus reword the sentence to fix the broken intra-doc link, providing an example of constant and linking to it to clarify which ones we are referring to. Fixes: 493fc33ec252 ("rust: io: add resource abstraction") Signed-off-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250722085500.1360401-1-ojeda@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/io/resource.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs index 1d5f367a6e8a..bea3ee0ed87b 100644 --- a/rust/kernel/io/resource.rs +++ b/rust/kernel/io/resource.rs @@ -177,7 +177,8 @@ unsafe impl Sync for Resource {} /// /// They can be combined with the operators `|`, `&`, and `!`. /// -/// Values can be used from the [`flags`] module. +/// Values can be used from the associated constants such as +/// [`Flags::IORESOURCE_IO`]. #[derive(Clone, Copy, PartialEq)] pub struct Flags(c_ulong); -- cgit v1.2.3 From 51a486feac0ca002bee6429f03da0a6c206d0dc5 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 22 Jul 2025 10:55:00 +0200 Subject: rust: io: fix broken intra-doc links to `platform::Device` `platform` is not accessible from here. Thus fix the intra-doc links by qualifying the paths a bit more. Fixes: 1d0d4b28513b ("rust: io: mem: add a generic iomem abstraction") Signed-off-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250722085500.1360401-2-ojeda@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/io/mem.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs index cc9feb2897d8..6f99510bfc3a 100644 --- a/rust/kernel/io/mem.rs +++ b/rust/kernel/io/mem.rs @@ -40,8 +40,8 @@ impl<'a> IoRequest<'a> { /// /// # Examples /// - /// The following example uses a [`platform::Device`] for illustration - /// purposes. + /// The following example uses a [`kernel::platform::Device`] for + /// illustration purposes. /// /// ```no_run /// use kernel::{bindings, c_str, platform, of, device::Core}; @@ -98,8 +98,8 @@ impl<'a> IoRequest<'a> { /// /// # Examples /// - /// The following example uses a [`platform::Device`] for illustration - /// purposes. + /// The following example uses a [`kernel::platform::Device`] for + /// illustration purposes. /// /// ```no_run /// use kernel::{bindings, c_str, platform, of, device::Core}; -- cgit v1.2.3