Skip to content

Commit 8d84b32

Browse files
fujitaDanilo Krummrich
authored andcommitted
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 <tmgross@umich.edu> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Link: https://lore.kernel.org/r/20250711040947.1252162-2-fujita.tomonori@gmail.com Signed-off-by: Danilo Krummrich <dakr@kernel.org>
1 parent 2f5606a commit 8d84b32

6 files changed

Lines changed: 104 additions & 47 deletions

File tree

rust/kernel/acpi.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
//! Advanced Configuration and Power Interface abstractions.
44
5-
use crate::{bindings, device_id::RawDeviceId, prelude::*};
5+
use crate::{
6+
bindings,
7+
device_id::{RawDeviceId, RawDeviceIdIndex},
8+
prelude::*,
9+
};
610

711
/// IdTable type for ACPI drivers.
812
pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
@@ -12,13 +16,14 @@ pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
1216
#[derive(Clone, Copy)]
1317
pub struct DeviceId(bindings::acpi_device_id);
1418

15-
// SAFETY:
16-
// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct acpi_device_id` and does not add
17-
// additional invariants, so it's safe to transmute to `RawType`.
18-
// * `DRIVER_DATA_OFFSET` is the offset to the `data` field.
19+
// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `acpi_device_id` and does not add
20+
// additional invariants, so it's safe to transmute to `RawType`.
1921
unsafe impl RawDeviceId for DeviceId {
2022
type RawType = bindings::acpi_device_id;
23+
}
2124

25+
// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
26+
unsafe impl RawDeviceIdIndex for DeviceId {
2227
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data);
2328

2429
fn index(&self) -> usize {

rust/kernel/auxiliary.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
77
use crate::{
88
bindings, container_of, device,
9-
device_id::RawDeviceId,
9+
device_id::{RawDeviceId, RawDeviceIdIndex},
1010
driver,
1111
error::{from_result, to_result, Result},
1212
prelude::*,
@@ -134,13 +134,14 @@ impl DeviceId {
134134
}
135135
}
136136

137-
// SAFETY:
138-
// * `DeviceId` is a `#[repr(transparent)`] wrapper of `auxiliary_device_id` and does not add
139-
// additional invariants, so it's safe to transmute to `RawType`.
140-
// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
137+
// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `auxiliary_device_id` and does not add
138+
// additional invariants, so it's safe to transmute to `RawType`.
141139
unsafe impl RawDeviceId for DeviceId {
142140
type RawType = bindings::auxiliary_device_id;
141+
}
143142

143+
// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
144+
unsafe impl RawDeviceIdIndex for DeviceId {
144145
const DRIVER_DATA_OFFSET: usize =
145146
core::mem::offset_of!(bindings::auxiliary_device_id, driver_data);
146147

rust/kernel/device_id.rs

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,32 +14,41 @@ use core::mem::MaybeUninit;
1414
///
1515
/// # Safety
1616
///
17-
/// Implementers must ensure that:
18-
/// - `Self` is layout-compatible with [`RawDeviceId::RawType`]; i.e. it's safe to transmute to
19-
/// `RawDeviceId`.
17+
/// Implementers must ensure that `Self` is layout-compatible with [`RawDeviceId::RawType`];
18+
/// i.e. it's safe to transmute to `RawDeviceId`.
2019
///
21-
/// This requirement is needed so `IdArray::new` can convert `Self` to `RawType` when building
22-
/// the ID table.
20+
/// This requirement is needed so `IdArray::new` can convert `Self` to `RawType` when building
21+
/// the ID table.
2322
///
24-
/// Ideally, this should be achieved using a const function that does conversion instead of
25-
/// transmute; however, const trait functions relies on `const_trait_impl` unstable feature,
26-
/// which is broken/gone in Rust 1.73.
27-
///
28-
/// - `DRIVER_DATA_OFFSET` is the offset of context/data field of the device ID (usually named
29-
/// `driver_data`) of the device ID, the field is suitable sized to write a `usize` value.
30-
///
31-
/// Similar to the previous requirement, the data should ideally be added during `Self` to
32-
/// `RawType` conversion, but there's currently no way to do it when using traits in const.
23+
/// Ideally, this should be achieved using a const function that does conversion instead of
24+
/// transmute; however, const trait functions relies on `const_trait_impl` unstable feature,
25+
/// which is broken/gone in Rust 1.73.
3326
pub unsafe trait RawDeviceId {
3427
/// The raw type that holds the device id.
3528
///
3629
/// Id tables created from [`Self`] are going to hold this type in its zero-terminated array.
3730
type RawType: Copy;
31+
}
3832

39-
/// The offset to the context/data field.
33+
/// Extension trait for [`RawDeviceId`] for devices that embed an index or context value.
34+
///
35+
/// This is typically used when the device ID struct includes a field like `driver_data`
36+
/// that is used to store a pointer-sized value (e.g., an index or context pointer).
37+
///
38+
/// # Safety
39+
///
40+
/// Implementers must ensure that `DRIVER_DATA_OFFSET` is the correct offset (in bytes) to
41+
/// the context/data field (e.g., the `driver_data` field) within the raw device ID structure.
42+
/// This field must be correctly sized to hold a `usize`.
43+
///
44+
/// Ideally, the data should be added during `Self` to `RawType` conversion,
45+
/// but there's currently no way to do it when using traits in const.
46+
pub unsafe trait RawDeviceIdIndex: RawDeviceId {
47+
/// The offset (in bytes) to the context/data field in the raw device ID.
4048
const DRIVER_DATA_OFFSET: usize;
4149

42-
/// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceId`] trait.
50+
/// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceIdIndex`]
51+
/// trait.
4352
fn index(&self) -> usize;
4453
}
4554

@@ -68,7 +77,15 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
6877
/// Creates a new instance of the array.
6978
///
7079
/// The contents are derived from the given identifiers and context information.
71-
pub const fn new(ids: [(T, U); N]) -> Self {
80+
///
81+
/// # Safety
82+
///
83+
/// `data_offset` as `None` is always safe.
84+
/// If `data_offset` is `Some(data_offset)`, then:
85+
/// - `data_offset` must be the correct offset (in bytes) to the context/data field
86+
/// (e.g., the `driver_data` field) within the raw device ID structure.
87+
/// - The field at `data_offset` must be correctly sized to hold a `usize`.
88+
const unsafe fn build(ids: [(T, U); N], data_offset: Option<usize>) -> Self {
7289
let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
7390
let mut infos = [const { MaybeUninit::uninit() }; N];
7491

@@ -77,14 +94,16 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
7794
// SAFETY: by the safety requirement of `RawDeviceId`, we're guaranteed that `T` is
7895
// layout-wise compatible with `RawType`.
7996
raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) };
80-
// SAFETY: by the safety requirement of `RawDeviceId`, this would be effectively
81-
// `raw_ids[i].driver_data = i;`.
82-
unsafe {
83-
raw_ids[i]
84-
.as_mut_ptr()
85-
.byte_offset(T::DRIVER_DATA_OFFSET as _)
86-
.cast::<usize>()
87-
.write(i);
97+
if let Some(data_offset) = data_offset {
98+
// SAFETY: by the safety requirement of this function, this would be effectively
99+
// `raw_ids[i].driver_data = i;`.
100+
unsafe {
101+
raw_ids[i]
102+
.as_mut_ptr()
103+
.byte_offset(data_offset as _)
104+
.cast::<usize>()
105+
.write(i);
106+
}
88107
}
89108

90109
// SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
@@ -109,12 +128,34 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
109128
}
110129
}
111130

131+
/// Creates a new instance of the array without writing index values.
132+
///
133+
/// The contents are derived from the given identifiers and context information.
134+
/// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
135+
pub const fn new_without_index(ids: [(T, U); N]) -> Self {
136+
// SAFETY: Calling `Self::build` with `offset = None` is always safe,
137+
// because no raw memory writes are performed in this case.
138+
unsafe { Self::build(ids, None) }
139+
}
140+
112141
/// Reference to the contained [`RawIdArray`].
113142
pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
114143
&self.raw_ids
115144
}
116145
}
117146

147+
impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
148+
/// Creates a new instance of the array.
149+
///
150+
/// The contents are derived from the given identifiers and context information.
151+
pub const fn new(ids: [(T, U); N]) -> Self {
152+
// SAFETY: by the safety requirement of `RawDeviceIdIndex`,
153+
// `T::DRIVER_DATA_OFFSET` is guaranteed to be the correct offset (in bytes) to
154+
// a field within `T::RawType`.
155+
unsafe { Self::build(ids, Some(T::DRIVER_DATA_OFFSET)) }
156+
}
157+
}
158+
118159
/// A device id table.
119160
///
120161
/// This trait is only implemented by `IdArray`.

rust/kernel/driver.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ pub trait Adapter {
170170
// and does not add additional invariants, so it's safe to transmute.
171171
let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
172172

173-
Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
173+
Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id)))
174174
}
175175
}
176176
}
@@ -204,7 +204,11 @@ pub trait Adapter {
204204
// and does not add additional invariants, so it's safe to transmute.
205205
let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
206206

207-
Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id)))
207+
Some(
208+
table.info(<of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(
209+
id,
210+
)),
211+
)
208212
}
209213
}
210214
}

rust/kernel/of.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
//! Device Tree / Open Firmware abstractions.
44
5-
use crate::{bindings, device_id::RawDeviceId, prelude::*};
5+
use crate::{
6+
bindings,
7+
device_id::{RawDeviceId, RawDeviceIdIndex},
8+
prelude::*,
9+
};
610

711
/// IdTable type for OF drivers.
812
pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
@@ -12,13 +16,14 @@ pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
1216
#[derive(Clone, Copy)]
1317
pub struct DeviceId(bindings::of_device_id);
1418

15-
// SAFETY:
16-
// * `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id` and does not add
17-
// additional invariants, so it's safe to transmute to `RawType`.
18-
// * `DRIVER_DATA_OFFSET` is the offset to the `data` field.
19+
// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id` and
20+
// does not add additional invariants, so it's safe to transmute to `RawType`.
1921
unsafe impl RawDeviceId for DeviceId {
2022
type RawType = bindings::of_device_id;
23+
}
2124

25+
// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `data` field.
26+
unsafe impl RawDeviceIdIndex for DeviceId {
2227
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
2328

2429
fn index(&self) -> usize {

rust/kernel/pci.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
77
use crate::{
88
bindings, container_of, device,
9-
device_id::RawDeviceId,
9+
device_id::{RawDeviceId, RawDeviceIdIndex},
1010
devres::Devres,
1111
driver,
1212
error::{from_result, to_result, Result},
@@ -159,13 +159,14 @@ impl DeviceId {
159159
}
160160
}
161161

162-
// SAFETY:
163-
// * `DeviceId` is a `#[repr(transparent)]` wrapper of `pci_device_id` and does not add
164-
// additional invariants, so it's safe to transmute to `RawType`.
165-
// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
162+
// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `pci_device_id` and does not add
163+
// additional invariants, so it's safe to transmute to `RawType`.
166164
unsafe impl RawDeviceId for DeviceId {
167165
type RawType = bindings::pci_device_id;
166+
}
168167

168+
// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
169+
unsafe impl RawDeviceIdIndex for DeviceId {
169170
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
170171

171172
fn index(&self) -> usize {

0 commit comments

Comments
 (0)