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 device_id trait. Signed-off-by: FUJITA Tomonori --- rust/kernel/auxiliary.rs | 7 +++- rust/kernel/device_id.rs | 80 +++++++++++++++++++++++++++++++--------- rust/kernel/of.rs | 11 +++++- rust/kernel/pci.rs | 7 +++- 4 files changed, 81 insertions(+), 24 deletions(-) diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index d2cfe1eeefb6..655ed61247d0 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::{to_result, Result}, prelude::*, @@ -143,10 +143,13 @@ pub const fn new(modname: &'static CStr, name: &'static CStr) -> Self { // 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. 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 3dc72ca8cfc2..3bcab2310087 100644 --- a/rust/kernel/device_id.rs +++ b/rust/kernel/device_id.rs @@ -24,22 +24,36 @@ /// 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. 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 ideally be added during `Self` to `RawType` conversion, +/// but there's currently no way to do it when using traits in const. +/// +/// - The `index` method must return the value stored at the location specified +/// by `DRIVER_DATA_OFFSET`, assuming `self` is layout-compatible with `RawType`. +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 +82,14 @@ 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 + /// + /// If `offset` is `Some(offset)`, then: + /// - `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 `offset` must be correctly sized to hold a `usize`. + const unsafe fn build(ids: [(T, U); N], offset: Option) -> Self { let mut raw_ids = [const { MaybeUninit::::uninit() }; N]; let mut infos = [const { MaybeUninit::uninit() }; N]; @@ -77,14 +98,17 @@ 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_add(T::DRIVER_DATA_OFFSET) - .cast::() - .write(i); + + if let Some(offset) = 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_add(offset) + .cast::() + .write(i); + } } // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but @@ -92,7 +116,6 @@ impl IdArray { infos[i] = MaybeUninit::new(unsafe { core::ptr::read(&ids[i].1) }); i += 1; } - core::mem::forget(ids); Self { @@ -109,12 +132,33 @@ impl IdArray { } } + /// Creates a new instance of the array without writing index values. + /// + /// The contents are derived from the given identifiers and context information. + 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/of.rs b/rust/kernel/of.rs index 40d1bd13682c..c690bceb0d73 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; @@ -15,10 +19,13 @@ // 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. 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 6b94fd7a3ce9..96d6275c7afb 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -7,7 +7,7 @@ use crate::{ alloc::flags::*, bindings, container_of, device, - device_id::RawDeviceId, + device_id::{RawDeviceId, RawDeviceIdIndex}, devres::Devres, driver, error::{to_result, Result}, @@ -164,10 +164,13 @@ pub const fn from_class(class: u32, class_mask: u32) -> Self { // SAFETY: // * `DeviceId` is a `#[repr(transparent)` wrapper of `pci_device_id` and does not add // additional invariants, so it's safe to transmute to `RawType`. -// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field. unsafe impl RawDeviceId for DeviceId { type RawType = bindings::pci_device_id; +} +// 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 { -- 2.43.0