* [PATCH v2 01/16] rust: io: add static `cast()` method for views
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:50 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 02/16] rust: io: add `IoRepr` trait Gary Guo
` (14 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Add a compile-time checked variant of `try_cast()` using the minimum size
and alignment information.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io.rs | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index a38c20ba3d23..adfc555de7d0 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -436,6 +436,45 @@ fn is_empty<T>(self) -> bool
self.len() == 0
}
+ /// Convert into a different typed I/O view.
+ ///
+ /// The target type must be known (statically) to be of the same or smaller size to current
+ /// type, and the current view is properly aligned for the target type.
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// use kernel::io::{
+ /// io_project,
+ /// Mmio,
+ /// Io,
+ /// Region,
+ /// };
+ /// #[derive(FromBytes, IntoBytes)]
+ /// #[repr(C)]
+ /// struct MyStruct { field: u32, }
+ ///
+ /// # fn test(mmio: &Mmio<'_, Region<0x1000>>) {
+ /// // let mmio: Mmio<'_, Region>;
+ /// let whole: Mmio<'_, MyStruct> = mmio.cast();
+ /// # }
+ /// ```
+ #[inline]
+ fn cast<U>(self) -> <Self::Backend as IoBackend>::View<'a, U>
+ where
+ Self::Target: FromBytes + IntoBytes,
+ U: FromBytes + IntoBytes,
+ {
+ let view = self.as_view();
+ let ptr = Self::Backend::as_ptr(view);
+
+ const_assert!(size_of::<U>() <= Self::Target::MIN_SIZE);
+ const_assert!(align_of::<U>() <= Self::Target::MIN_ALIGN.as_usize());
+
+ // SAFETY: We have checked bounds and alignment, so this is a valid projection.
+ unsafe { Self::Backend::project_view(view, ptr.cast()) }
+ }
+
/// Try to convert into a different typed I/O view.
///
/// A runtime check is performed to ensure that the target type is of same or smaller size to
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v2 01/16] rust: io: add static `cast()` method for views
2026-08-05 16:35 ` [PATCH v2 01/16] rust: io: add static `cast()` method for views Gary Guo
@ 2026-08-05 16:50 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-08-05 16:50 UTC (permalink / raw)
To: Gary Guo; +Cc: Alexandre Courbot, linux-pci, dri-devel, Danilo Krummrich, ojeda
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Soundness hole: `KnownSize` is an unsealed safe trait, but its constants and methods are relied upon for memory safety in unsafe projections.
--
--- Patch [1]: [PATCH v2 01/16] rust: io: add static `cast()` method for views ---
commit 5783172643ee35d4a1864537f0c76cc527b2cad9
Author: Gary Guo <gary@garyguo.net>
rust: io: add static `cast()` method for views
This commit adds a compile-time checked cast() method for I/O views in the
Rust subsystem. It uses minimum size and alignment information to statically
verify that the target type is of the same or smaller size and properly
aligned, allowing conversion without runtime checks.
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index a38c20ba3d23e..adfc555de7d0d 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
[ ... ]
> + #[inline]
> + fn cast<U>(self) -> <Self::Backend as IoBackend>::View<'a, U>
> + where
> + Self::Target: FromBytes + IntoBytes,
> + U: FromBytes + IntoBytes,
> + {
> + let view = self.as_view();
> + let ptr = Self::Backend::as_ptr(view);
> +
> + const_assert!(size_of::<U>() <= Self::Target::MIN_SIZE);
> + const_assert!(align_of::<U>() <= Self::Target::MIN_ALIGN.as_usize());
> +
> + // SAFETY: We have checked bounds and alignment, so this is a valid projection.
> + unsafe { Self::Backend::project_view(view, ptr.cast()) }
^^^^^^
[Severity: High]
This is a pre-existing issue, but does relying on KnownSize constants for
memory safety create a soundness hole?
KnownSize is an unsealed safe trait that provides the constants and methods
relied upon for these safety guarantees. Because it is a safe trait, safe
code could provide an incorrect implementation for a custom dynamically sized
type (DST), specifying an excessively large MIN_SIZE or size().
If this happens, the bounds checks in cast() or try_cast() would pass based
on bogus values from the safe trait. The unsafe project_view() projection
would then proceed, potentially creating an out-of-bounds pointer projection
and leading to memory corruption.
Since implementing a safe trait shouldn't be able to trigger undefined
behavior in safe code, should KnownSize be marked unsafe or sealed to prevent
unsound implementations?
> + }
> +
> /// Try to convert into a different typed I/O view.
> ///
> /// A runtime check is performed to ensure that the target type is of same or smaller size to
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-typed_register-v2-0-c3ca142220a0@garyguo.net?part=1
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 02/16] rust: io: add `IoRepr` trait
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
2026-08-05 16:35 ` [PATCH v2 01/16] rust: io: add static `cast()` method for views Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:46 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 03/16] rust: io: support register projections Gary Guo
` (13 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
For types that are layout-compatible with an I/O capable type, we would
want the ability to use them directly for I/O operations. E.g.
bitfield! {
pub struct Foo(u32) {
...
}
}
#[repr(C)]
struct Bar {
foo: Foo,
}
let mmio: Mmio<'_, Bar> = ...;
io_read!(mmio, .foo)
Currently this feature is available from `register!()` macro but not
otherwise available with `io_read!`, `io_write!`. Support this by adding a
`IoRepr` type to denote the underlying I/O type to use for a specific type.
This makes the `IoLoc::IoType` and `Register::Storage` redundant; thus
remove them; also convert register methods to use the `read_val` and
`write_val` instead.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/bitfield.rs | 5 ++
rust/kernel/io.rs | 203 ++++++++++++++++++++++++++++++++-------------
rust/kernel/io/register.rs | 59 +++++--------
3 files changed, 170 insertions(+), 97 deletions(-)
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index 35ede53f2b8e..4bc92c62da06 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -308,6 +308,7 @@ macro_rules! bitfield {
$(#[$attr])*
#[repr(transparent)]
#[derive(Clone, Copy, PartialEq, Eq)]
+ #[derive($crate::prelude::FromBytes, $crate::prelude::IntoBytes)]
$vis struct $name {
inner: $storage,
}
@@ -346,6 +347,10 @@ fn from(val: $storage) -> $name {
Self::from_raw(val)
}
}
+
+ impl $crate::io::IoRepr for $name {
+ type Repr = $storage;
+ }
};
// Definitions requiring knowledge of individual fields: private and public field accessors,
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index adfc555de7d0..71c6180ed745 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -276,6 +276,91 @@ pub trait IoCapable<T>: IoBackend {
fn io_write<'a>(view: Self::View<'a, T>, value: T);
}
+/// Safe transmute that performs size check on monomorphization-time.
+///
+/// Can be considered as generic version of [`zerocopy::transmute!`] macro but using the unstable
+/// `core::mem::transmute_neo` instead of [`core::mem::transmute`].
+#[inline(always)] // This is a no-op.
+fn transmute_neo<Src: IntoBytes, Dst: FromBytes>(val: Src) -> Dst {
+ const_assert!(size_of::<Src>() == size_of::<Dst>());
+
+ // SAFETY: `Src: IntoBytes` and `Dst: FromBytes` and we've checked size is the same.
+ unsafe { core::mem::transmute_copy(&core::mem::ManuallyDrop::new(val)) }
+}
+
+/// Trait indicating the underlying primitive types to be used for I/O operations.
+///
+/// Implementing trait allows arbitrary types to be used for I/O operations, not just raw
+/// primitives.
+///
+/// The layout of the type and the underlying primitive must match; this is enforced via const
+/// assertions when I/O methods are used, as the type system cannot represent this.
+/// [`IoRepr::from_repr`] and [`IoRepr::into_expr`] can be overridden for conversions, however it
+/// should be noted that they are only invoked on value read/write operations and are not invoked
+/// on byte operations such as [`Io::copy_read`].
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::io::*;
+/// #[repr(transparent)]
+/// #[derive(FromBytes, IntoBytes)]
+/// pub struct MyNewType(u32);
+///
+/// impl IoRepr for MyNewType {
+/// type Repr = u32;
+/// }
+///
+/// #[repr(C)]
+/// pub struct MyStruct {
+/// raw: u32,
+/// new_type: MyNewType,
+/// }
+///
+/// # fn test(mmio: Mmio<'_, MyStruct>) {
+/// // let mmio: Mmio<'_, MyStruct>;
+/// let val: u32 = io_read!(mmio, .raw); // Raw primitive read
+/// io_write!(mmio, .raw, val); // Raw primitve write
+/// let val: MyNewType = io_read!(mmio, .new_type); // Read via `IoRepr`.
+/// io_write!(mmio, .new_type, val); // Write via `IoRepr`.
+/// # }
+/// ```
+pub trait IoRepr: FromBytes + IntoBytes + Sized {
+ /// The backing I/O capable type.
+ type Repr: FromBytes + IntoBytes;
+
+ /// Convert from [`IoRepr::Repr`] to `Self`.
+ #[inline(always)]
+ fn from_repr(repr: Self::Repr) -> Self {
+ transmute_neo(repr)
+ }
+
+ /// Convert from `Self` to [`IoRepr::Repr`].
+ #[inline(always)]
+ fn into_repr(this: Self) -> Self::Repr {
+ transmute_neo(this)
+ }
+}
+
+macro_rules! impl_io_repr {
+ ($($ty:ty => $backing:ty,)*) => {
+ $(impl IoRepr for $ty {
+ type Repr = $backing;
+ })*
+ };
+}
+
+impl_io_repr! {
+ u8 => u8,
+ u16 => u16,
+ u32 => u32,
+ u64 => u64,
+ i8 => u8,
+ i16 => u16,
+ i32 => u32,
+ i64 => u64,
+}
+
/// Trait indicating that an I/O backend supports memory copy operations.
pub trait IoCopyable: IoBackend {
/// Copy contents of `view` to `buffer`.
@@ -352,15 +437,12 @@ fn copy_write<T: IntoBytes>(view: Self::View<'_, T>, value: T) {
///
/// - The valid `Base` to operate on. For most registers, this should be [`Region`].
/// - The offset to access (returned by [`IoLoc::offset`]),
-/// - The width of the access (determined by [`IoLoc::IoType`]),
-/// - The type `T` in which the raw data is returned or provided.
+/// - The type `T` in which the data is returned or provided.
///
-/// `T` and `IoLoc::IoType` may differ: for instance, a typed register has `T` = the register type
-/// with its bitfields, and `IoType` = its backing primitive (e.g. `u32`).
+/// `T` is not necessarily the type for underlying I/O operation. Methods that take `IoLoc` have `T:
+/// IoRepr` bound and the `<T as IoRepr>::Repr` type would be used to perform I/O and converted to
+/// `T` instead.
pub trait IoLoc<Base: ?Sized, T> {
- /// Size ([`u8`], [`u16`], etc) of the I/O performed on the returned [`offset`](IoLoc::offset).
- type IoType: Into<T> + From<T>;
-
/// Consumes `self` and returns the offset of this location.
fn offset(self) -> usize;
}
@@ -371,8 +453,6 @@ macro_rules! impl_usize_ioloc {
($($ty:ty),*) => {
$(
impl<const SIZE: usize> IoLoc<Region<SIZE>, $ty> for usize {
- type IoType = $ty;
-
#[inline(always)]
fn offset(self) -> usize {
self
@@ -536,10 +616,12 @@ fn try_cast<U>(self) -> Result<<Self::Backend as IoBackend>::View<'a, U>>
#[inline]
fn read_val(self) -> Self::Target
where
- Self::Backend: IoCapable<Self::Target>,
- Self::Target: Sized,
+ Self::Target: IoRepr,
+ Self::Backend: IoCapable<<Self::Target as IoRepr>::Repr>,
{
- Self::Backend::io_read(self.as_view())
+ Self::Target::from_repr(Self::Backend::io_read(
+ self.as_view().cast::<<Self::Target as IoRepr>::Repr>(),
+ ))
}
/// Write a value to I/O.
@@ -558,10 +640,13 @@ fn read_val(self) -> Self::Target
#[inline]
fn write_val(self, value: Self::Target)
where
- Self::Backend: IoCapable<Self::Target>,
- Self::Target: Sized,
+ Self::Target: IoRepr,
+ Self::Backend: IoCapable<<Self::Target as IoRepr>::Repr>,
{
- Self::Backend::io_write(self.as_view(), value)
+ Self::Backend::io_write(
+ self.as_view().cast::<<Self::Target as IoRepr>::Repr>(),
+ Self::Target::into_repr(value),
+ )
}
/// Copy-read from I/O memory.
@@ -683,7 +768,7 @@ fn copy_to_slice(self, data: &mut [u8])
#[inline(always)]
fn try_read8(self, offset: usize) -> Result<u8>
where
- usize: IoLoc<Self::Target, u8, IoType = u8>,
+ usize: IoLoc<Self::Target, u8>,
Self::Backend: IoCapable<u8>,
{
self.try_read(offset)
@@ -693,7 +778,7 @@ fn try_read8(self, offset: usize) -> Result<u8>
#[inline(always)]
fn try_read16(self, offset: usize) -> Result<u16>
where
- usize: IoLoc<Self::Target, u16, IoType = u16>,
+ usize: IoLoc<Self::Target, u16>,
Self::Backend: IoCapable<u16>,
{
self.try_read(offset)
@@ -703,7 +788,7 @@ fn try_read16(self, offset: usize) -> Result<u16>
#[inline(always)]
fn try_read32(self, offset: usize) -> Result<u32>
where
- usize: IoLoc<Self::Target, u32, IoType = u32>,
+ usize: IoLoc<Self::Target, u32>,
Self::Backend: IoCapable<u32>,
{
self.try_read(offset)
@@ -713,7 +798,7 @@ fn try_read32(self, offset: usize) -> Result<u32>
#[inline(always)]
fn try_read64(self, offset: usize) -> Result<u64>
where
- usize: IoLoc<Self::Target, u64, IoType = u64>,
+ usize: IoLoc<Self::Target, u64>,
Self::Backend: IoCapable<u64>,
{
self.try_read(offset)
@@ -723,7 +808,7 @@ fn try_read64(self, offset: usize) -> Result<u64>
#[inline(always)]
fn try_write8(self, value: u8, offset: usize) -> Result
where
- usize: IoLoc<Self::Target, u8, IoType = u8>,
+ usize: IoLoc<Self::Target, u8>,
Self::Backend: IoCapable<u8>,
{
self.try_write(offset, value)
@@ -733,7 +818,7 @@ fn try_write8(self, value: u8, offset: usize) -> Result
#[inline(always)]
fn try_write16(self, value: u16, offset: usize) -> Result
where
- usize: IoLoc<Self::Target, u16, IoType = u16>,
+ usize: IoLoc<Self::Target, u16>,
Self::Backend: IoCapable<u16>,
{
self.try_write(offset, value)
@@ -743,7 +828,7 @@ fn try_write16(self, value: u16, offset: usize) -> Result
#[inline(always)]
fn try_write32(self, value: u32, offset: usize) -> Result
where
- usize: IoLoc<Self::Target, u32, IoType = u32>,
+ usize: IoLoc<Self::Target, u32>,
Self::Backend: IoCapable<u32>,
{
self.try_write(offset, value)
@@ -753,7 +838,7 @@ fn try_write32(self, value: u32, offset: usize) -> Result
#[inline(always)]
fn try_write64(self, value: u64, offset: usize) -> Result
where
- usize: IoLoc<Self::Target, u64, IoType = u64>,
+ usize: IoLoc<Self::Target, u64>,
Self::Backend: IoCapable<u64>,
{
self.try_write(offset, value)
@@ -765,7 +850,7 @@ fn try_write64(self, value: u64, offset: usize) -> Result
#[inline(always)]
fn read8(self, offset: usize) -> u8
where
- usize: IoLoc<Self::Target, u8, IoType = u8>,
+ usize: IoLoc<Self::Target, u8>,
Self::Backend: IoCapable<u8>,
{
self.read(offset)
@@ -777,7 +862,7 @@ fn read8(self, offset: usize) -> u8
#[inline(always)]
fn read16(self, offset: usize) -> u16
where
- usize: IoLoc<Self::Target, u16, IoType = u16>,
+ usize: IoLoc<Self::Target, u16>,
Self::Backend: IoCapable<u16>,
{
self.read(offset)
@@ -789,7 +874,7 @@ fn read16(self, offset: usize) -> u16
#[inline(always)]
fn read32(self, offset: usize) -> u32
where
- usize: IoLoc<Self::Target, u32, IoType = u32>,
+ usize: IoLoc<Self::Target, u32>,
Self::Backend: IoCapable<u32>,
{
self.read(offset)
@@ -801,7 +886,7 @@ fn read32(self, offset: usize) -> u32
#[inline(always)]
fn read64(self, offset: usize) -> u64
where
- usize: IoLoc<Self::Target, u64, IoType = u64>,
+ usize: IoLoc<Self::Target, u64>,
Self::Backend: IoCapable<u64>,
{
self.read(offset)
@@ -813,7 +898,7 @@ fn read64(self, offset: usize) -> u64
#[inline(always)]
fn write8(self, value: u8, offset: usize)
where
- usize: IoLoc<Self::Target, u8, IoType = u8>,
+ usize: IoLoc<Self::Target, u8>,
Self::Backend: IoCapable<u8>,
{
self.write(offset, value)
@@ -825,7 +910,7 @@ fn write8(self, value: u8, offset: usize)
#[inline(always)]
fn write16(self, value: u16, offset: usize)
where
- usize: IoLoc<Self::Target, u16, IoType = u16>,
+ usize: IoLoc<Self::Target, u16>,
Self::Backend: IoCapable<u16>,
{
self.write(offset, value)
@@ -837,7 +922,7 @@ fn write16(self, value: u16, offset: usize)
#[inline(always)]
fn write32(self, value: u32, offset: usize)
where
- usize: IoLoc<Self::Target, u32, IoType = u32>,
+ usize: IoLoc<Self::Target, u32>,
Self::Backend: IoCapable<u32>,
{
self.write(offset, value)
@@ -849,7 +934,7 @@ fn write32(self, value: u32, offset: usize)
#[inline(always)]
fn write64(self, value: u64, offset: usize)
where
- usize: IoLoc<Self::Target, u64, IoType = u64>,
+ usize: IoLoc<Self::Target, u64>,
Self::Backend: IoCapable<u64>,
{
self.write(offset, value)
@@ -881,11 +966,12 @@ fn write64(self, value: u64, offset: usize)
#[inline(always)]
fn try_read<T, L>(self, location: L) -> Result<T>
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view::<Self, L::IoType>(self, location.offset())?;
- Ok(Self::Backend::io_read(view).into())
+ let view = io_view::<Self, T>(self, location.offset())?;
+ Ok(view.read_val())
}
/// Generic fallible write with runtime bounds check.
@@ -914,12 +1000,12 @@ fn try_read<T, L>(self, location: L) -> Result<T>
#[inline(always)]
fn try_write<T, L>(self, location: L, value: T) -> Result
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view::<Self, L::IoType>(self, location.offset())?;
- let io_value = value.into();
- Self::Backend::io_write(view, io_value);
+ let view = io_view::<Self, T>(self, location.offset())?;
+ view.write_val(value);
Ok(())
}
@@ -958,9 +1044,10 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
#[inline(always)]
fn try_write_reg<T, L, V>(self, value: V) -> Result
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
V: LocatedRegister<Self::Target, Location = L, Value = T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
let (location, value) = value.into_io_op();
@@ -992,16 +1079,13 @@ fn try_write_reg<T, L, V>(self, value: V) -> Result
#[inline(always)]
fn try_update<T, L, F>(self, location: L, f: F) -> Result
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
F: FnOnce(T) -> T,
{
- let view = io_view::<Self, L::IoType>(self, location.offset())?;
-
- let value: T = Self::Backend::io_read(view).into();
- let io_value = f(value).into();
- Self::Backend::io_write(view, io_value);
-
+ let view = io_view::<Self, T>(self, location.offset())?;
+ view.write_val(f(view.read_val()));
Ok(())
}
@@ -1029,11 +1113,12 @@ fn try_update<T, L, F>(self, location: L, f: F) -> Result
#[inline(always)]
fn read<T, L>(self, location: L) -> T
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view_assert::<Self, L::IoType>(self, location.offset());
- Self::Backend::io_read(view).into()
+ let view = io_view_assert::<Self, T>(self, location.offset());
+ view.read_val()
}
/// Generic infallible write with compile-time bounds check.
@@ -1060,12 +1145,12 @@ fn read<T, L>(self, location: L) -> T
#[inline(always)]
fn write<T, L>(self, location: L, value: T)
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view_assert::<Self, L::IoType>(self, location.offset());
- let io_value = value.into();
- Self::Backend::io_write(view, io_value);
+ let view = io_view_assert::<Self, T>(self, location.offset());
+ view.write_val(value)
}
/// Generic infallible write of a fully-located register value.
@@ -1102,9 +1187,10 @@ fn write<T, L>(self, location: L, value: T)
#[inline(always)]
fn write_reg<T, L, V>(self, value: V)
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
V: LocatedRegister<Self::Target, Location = L, Value = T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
let (location, value) = value.into_io_op();
@@ -1136,14 +1222,13 @@ fn write_reg<T, L, V>(self, value: V)
#[inline(always)]
fn update<T, L, F>(self, location: L, f: F)
where
+ T: IoRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as IoRepr>::Repr>,
F: FnOnce(T) -> T,
{
- let view = io_view_assert::<Self, L::IoType>(self, location.offset());
- let value: T = Self::Backend::io_read(view).into();
- let io_value = f(value).into();
- Self::Backend::io_write(view, io_value);
+ let view = io_view_assert::<Self, T>(self, location.offset());
+ view.write_val(f(view.read_val()));
}
}
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 6cb07fc92cc3..d898b2b46d52 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -117,9 +117,6 @@
/// Trait implemented by all registers.
pub trait Register: Sized {
- /// Backing primitive type of the register.
- type Storage: Into<Self> + From<Self>;
-
/// Start offset of the register.
///
/// The interpretation of this offset depends on the type of the register.
@@ -135,8 +132,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for ()
where
T: FixedRegister,
{
- type IoType = T::Storage;
-
#[inline(always)]
fn offset(self) -> usize {
T::OFFSET
@@ -149,8 +144,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for T
where
T: FixedRegister,
{
- type IoType = T::Storage;
-
#[inline(always)]
fn offset(self) -> usize {
T::OFFSET
@@ -174,8 +167,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for FixedRegisterLoc<T>
where
T: FixedRegister,
{
- type IoType = T::Storage;
-
#[inline(always)]
fn offset(self) -> usize {
T::OFFSET
@@ -246,8 +237,6 @@ impl<const SIZE: usize, T, B> IoLoc<Region<SIZE>, T> for RelativeRegisterLoc<T,
T: RelativeRegister,
B: RegisterBase<T::BaseFamily> + ?Sized,
{
- type IoType = T::Storage;
-
#[inline(always)]
fn offset(self) -> usize {
RelativeRegisterLoc::offset(self)
@@ -289,8 +278,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for RegisterArrayLoc<T>
where
T: RegisterArray,
{
- type IoType = T::Storage;
-
#[inline(always)]
fn offset(self) -> usize {
T::OFFSET + self.0 * T::STRIDE
@@ -377,8 +364,6 @@ impl<const SIZE: usize, T, B> IoLoc<Region<SIZE>, T> for RelativeRegisterArrayLo
T: RelativeRegisterArray,
B: RegisterBase<T::BaseFamily> + ?Sized,
{
- type IoType = T::Storage;
-
#[inline(always)]
fn offset(self) -> usize {
self.0.offset() + self.1 * T::STRIDE
@@ -831,8 +816,8 @@ macro_rules! register {
{ $($fields:tt)* }
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name($storage) @ $offset);
- $crate::register!(@io_fixed $(#[$attr])* $vis $name($storage));
+ $crate::register!(@io_base $name @ $offset);
+ $crate::register!(@io_fixed $(#[$attr])* $vis $name);
};
// Creates an alias register of fixed offset register `alias` with its own fields.
@@ -842,10 +827,10 @@ macro_rules! register {
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(
- @io_base $name($storage) @
+ @io_base $name @
<$alias as $crate::io::register::Register>::OFFSET
);
- $crate::register!(@io_fixed $(#[$attr])* $vis $name($storage));
+ $crate::register!(@io_fixed $(#[$attr])* $vis $name);
};
// Creates a register at a relative offset from a base address provider.
@@ -854,8 +839,8 @@ macro_rules! register {
{ $($fields:tt)* }
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name($storage) @ $offset);
- $crate::register!(@io_relative $vis $name($storage) @ $base);
+ $crate::register!(@io_base $name @ $offset);
+ $crate::register!(@io_relative $vis $name @ $base);
};
// Creates an alias register of relative offset register `alias` with its own fields.
@@ -865,9 +850,9 @@ macro_rules! register {
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(
- @io_base $name($storage) @ <$alias as $crate::io::register::Register>::OFFSET
+ @io_base $name @ <$alias as $crate::io::register::Register>::OFFSET
);
- $crate::register!(@io_relative $vis $name($storage) @ $base);
+ $crate::register!(@io_relative $vis $name @ $base);
};
// Creates an array of registers at a fixed offset of the MMIO space.
@@ -878,8 +863,8 @@ macro_rules! register {
$crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name($storage) @ $offset);
- $crate::register!(@io_array $vis $name($storage) [ $size, stride = $stride ]);
+ $crate::register!(@io_base $name @ $offset);
+ $crate::register!(@io_array $vis $name [ $size, stride = $stride ]);
};
// Shortcut for contiguous array of registers (stride == size of element).
@@ -904,11 +889,11 @@ macro_rules! register {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(
- @io_base $name($storage) @
+ @io_base $name @
<$alias as $crate::io::register::Register>::OFFSET
+ $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
);
- $crate::register!(@io_fixed $(#[$attr])* $vis $name($storage));
+ $crate::register!(@io_fixed $(#[$attr])* $vis $name);
};
// Creates an array of registers at a relative offset from a base address provider.
@@ -920,9 +905,9 @@ macro_rules! register {
$crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name($storage) @ $offset);
+ $crate::register!(@io_base $name @ $offset);
$crate::register!(
- @io_relative_array $vis $name($storage) [ $size, stride = $stride ] @ $base + $offset
+ @io_relative_array $vis $name [ $size, stride = $stride ] @ $base + $offset
);
};
@@ -949,11 +934,11 @@ macro_rules! register {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(
- @io_base $name($storage) @
+ @io_base $name @
<$alias as $crate::io::register::Register>::OFFSET +
$idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
);
- $crate::register!(@io_relative $vis $name($storage) @ $base);
+ $crate::register!(@io_relative $vis $name @ $base);
};
// Generates the bitfield for the register.
@@ -970,16 +955,14 @@ macro_rules! register {
};
// Implementations shared by all registers types.
- (@io_base $name:ident($storage:ty) @ $offset:expr) => {
+ (@io_base $name:ident @ $offset:expr) => {
impl $crate::io::register::Register for $name {
- type Storage = $storage;
-
const OFFSET: usize = $offset;
}
};
// Implementations of fixed registers.
- (@io_fixed $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)) => {
+ (@io_fixed $(#[$attr:meta])* $vis:vis $name:ident) => {
impl $crate::io::register::FixedRegister for $name {}
$(#[$attr])*
@@ -988,7 +971,7 @@ impl $crate::io::register::FixedRegister for $name {}
};
// Implementations of relative registers.
- (@io_relative $vis:vis $name:ident ($storage:ty) @ $base:ident) => {
+ (@io_relative $vis:vis $name:ident @ $base:ident) => {
impl $crate::io::register::WithBase for $name {
type BaseFamily = $base;
}
@@ -997,7 +980,7 @@ impl $crate::io::register::RelativeRegister for $name {}
};
// Implementations of register arrays.
- (@io_array $vis:vis $name:ident ($storage:ty) [ $size:expr, stride = $stride:expr ]) => {
+ (@io_array $vis:vis $name:ident [ $size:expr, stride = $stride:expr ]) => {
impl $crate::io::register::Array for $name {}
impl $crate::io::register::RegisterArray for $name {
@@ -1008,7 +991,7 @@ impl $crate::io::register::RegisterArray for $name {
// Implementations of relative array registers.
(
- @io_relative_array $vis:vis $name:ident ($storage:ty) [ $size:expr, stride = $stride:expr ]
+ @io_relative_array $vis:vis $name:ident [ $size:expr, stride = $stride:expr ]
@ $base:ident + $offset:literal
) => {
impl $crate::io::register::WithBase for $name {
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v2 02/16] rust: io: add `IoRepr` trait
2026-08-05 16:35 ` [PATCH v2 02/16] rust: io: add `IoRepr` trait Gary Guo
@ 2026-08-05 16:46 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-08-05 16:46 UTC (permalink / raw)
To: Gary Guo; +Cc: ojeda, linux-pci, dri-devel, Danilo Krummrich, Alexandre Courbot
> For types that are layout-compatible with an I/O capable type, we would
> want the ability to use them directly for I/O operations. E.g.
>
> bitfield! {
> pub struct Foo(u32) {
> ...
> }
> }
>
> #[repr(C)]
> struct Bar {
> foo: Foo,
> }
>
> let mmio: Mmio<'_, Bar> = ...;
> io_read!(mmio, .foo)
>
> Currently this feature is available from `register!()` macro but not
> otherwise available with `io_read!`, `io_write!`. Support this by adding a
> `IoRepr` type to denote the underlying I/O type to use for a specific type.
>
> This makes the `IoLoc::IoType` and `Register::Storage` redundant; thus
> remove them; also convert register methods to use the `read_val` and
> `write_val` instead.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-typed_register-v2-0-c3ca142220a0@garyguo.net?part=2
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 03/16] rust: io: support register projections
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
2026-08-05 16:35 ` [PATCH v2 01/16] rust: io: add static `cast()` method for views Gary Guo
2026-08-05 16:35 ` [PATCH v2 02/16] rust: io: add `IoRepr` trait Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:42 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 04/16] rust: io: register: handle one register at a time Gary Guo
` (12 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
`IoLoc`s themselves just describe a projection from a region to a concrete
register. Thus, support it in `io_project` macro too. Also, update methods
that operate on `IoLoc` to use I/O projection.
Documentation of `io_project!` is not expanded yet as the example works
better when `register!` type can specify base type. `io_read!` and
`io_write!` gains the ability to operate on registers as corollary of the
capability of `io_project!`. Examples are not added because `read` and
`write` is still preferrably used instead.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 11 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 71c6180ed745..ae18890866b6 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -295,7 +295,7 @@ fn transmute_neo<Src: IntoBytes, Dst: FromBytes>(val: Src) -> Dst {
///
/// The layout of the type and the underlying primitive must match; this is enforced via const
/// assertions when I/O methods are used, as the type system cannot represent this.
-/// [`IoRepr::from_repr`] and [`IoRepr::into_expr`] can be overridden for conversions, however it
+/// [`IoRepr::from_repr`] and [`IoRepr::into_repr`] can be overridden for conversions, however it
/// should be noted that they are only invoked on value read/write operations and are not invoked
/// on byte operations such as [`Io::copy_read`].
///
@@ -970,8 +970,7 @@ fn try_read<T, L>(self, location: L) -> Result<T>
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view::<Self, T>(self, location.offset())?;
- Ok(view.read_val())
+ Ok(io_read!(self, try: location))
}
/// Generic fallible write with runtime bounds check.
@@ -1004,8 +1003,7 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view::<Self, T>(self, location.offset())?;
- view.write_val(value);
+ io_write!(self, try: location, value);
Ok(())
}
@@ -1084,7 +1082,7 @@ fn try_update<T, L, F>(self, location: L, f: F) -> Result
Self::Backend: IoCapable<<T as IoRepr>::Repr>,
F: FnOnce(T) -> T,
{
- let view = io_view::<Self, T>(self, location.offset())?;
+ let view = io_project!(self, try: location);
view.write_val(f(view.read_val()));
Ok(())
}
@@ -1117,8 +1115,7 @@ fn read<T, L>(self, location: L) -> T
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view_assert::<Self, T>(self, location.offset());
- view.read_val()
+ io_read!(self, build: location)
}
/// Generic infallible write with compile-time bounds check.
@@ -1149,8 +1146,7 @@ fn write<T, L>(self, location: L, value: T)
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as IoRepr>::Repr>,
{
- let view = io_view_assert::<Self, T>(self, location.offset());
- view.write_val(value)
+ io_write!(self, build: location, value);
}
/// Generic infallible write of a fully-located register value.
@@ -1227,7 +1223,7 @@ fn update<T, L, F>(self, location: L, f: F)
Self::Backend: IoCapable<<T as IoRepr>::Repr>,
F: FnOnce(T) -> T,
{
- let view = io_view_assert::<Self, T>(self, location.offset());
+ let view = io_project!(self, build: location);
view.write_val(f(view.read_val()));
}
}
@@ -1772,6 +1768,25 @@ pub unsafe fn project_view<U: ?Sized + KnownSize>(
// SAFETY: Per safety requirement.
unsafe { T::Backend::project_view::<T::Target, _>(self.0, ptr) }
}
+
+ #[inline(always)]
+ pub fn try_project_loc<U, L>(
+ self,
+ location: L,
+ ) -> Result<<T::Backend as IoBackend>::View<'a, U>>
+ where
+ L: IoLoc<T::Target, U>,
+ {
+ io_view::<_, U>(self.0, location.offset())
+ }
+
+ #[inline(always)]
+ pub fn project_loc<U, L>(self, location: L) -> <T::Backend as IoBackend>::View<'a, U>
+ where
+ L: IoLoc<T::Target, U>,
+ {
+ io_view_assert::<_, U>(self.0, location.offset())
+ }
}
/// Project an I/O type to a subview of it.
@@ -1799,6 +1814,21 @@ pub unsafe fn project_view<U: ?Sized + KnownSize>(
#[macro_export]
#[doc(hidden)]
macro_rules! io_project {
+ // Register projection
+ ($io:expr, try: $ioloc:expr) => {{
+ #[allow(unused)]
+ use $crate::io::IoBase as _;
+ let view = $crate::io::ProjectHelper($io.as_view());
+ view.try_project_loc($ioloc)?
+ }};
+ ($io:expr, build: $ioloc:expr) => {{
+ #[allow(unused)]
+ use $crate::io::IoBase as _;
+ let view = $crate::io::ProjectHelper($io.as_view());
+ view.project_loc($ioloc)
+ }};
+
+ // Field or index projection
($io:expr, $($proj:tt)*) => {{
#[allow(unused)]
use $crate::io::IoBase as _;
@@ -1869,6 +1899,12 @@ macro_rules! io_write {
(@parse [$io:expr] [$($proj:tt)*] [[$flavor:ident: $index:expr] $($rest:tt)*]) => {
$crate::io_write!(@parse [$io] [$($proj)* [$flavor: $index]] [$($rest)*])
};
+ (@parse [$io:expr] [] [try: $ioloc:expr, $($rest:tt)*]) => {
+ $crate::io_write!(@parse [$io] [try: $ioloc] [, $($rest)*])
+ };
+ (@parse [$io:expr] [] [build: $ioloc:expr, $($rest:tt)*]) => {
+ $crate::io_write!(@parse [$io] [build: $ioloc] [, $($rest)*])
+ };
($io:expr, $($rest:tt)*) => {
$crate::io_write!(@parse [$io] [] [$($rest)*])
};
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 04/16] rust: io: register: handle one register at a time
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (2 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 03/16] rust: io: support register projections Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:43 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 05/16] rust: io: register extract offset computation to helper rules Gary Guo
` (11 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
It is easier to change rules in this form, as there is no need to define a
single rule that can match all possible register definitions.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io/register.rs | 66 +++++++++++++++++++++++-----------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index d898b2b46d52..91804c1ca117 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -786,44 +786,25 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// ```
#[macro_export]
macro_rules! register {
- // Entry point for the macro, allowing multiple registers to be defined in one call.
- // It matches all possible register declaration patterns to dispatch them to corresponding
- // `@reg` rule that defines a single register.
- (
- $(
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
- $([ $size:expr $(, stride = $stride:expr)? ])?
- $(@ $($base:ident +)? $offset:literal)?
- $(=> $alias:ident $(+ $alias_offset:ident)? $([$alias_idx:expr])? )?
- { $($fields:tt)* }
- )*
- ) => {
- $(
- $crate::register!(
- @reg $(#[$attr])* $vis $name ($storage) $([$size $(, stride = $stride)?])?
- $(@ $($base +)? $offset)?
- $(=> $alias $(+ $alias_offset)? $([$alias_idx])? )?
- { $($fields)* }
- );
- )*
- };
-
- // All the rules below are private helpers.
+ () => {};
// Creates a register at a fixed offset of the MMIO space.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $offset:literal
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $offset:literal
{ $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $offset);
$crate::register!(@io_fixed $(#[$attr])* $vis $name);
+ $crate::register!($($rest)*);
};
// Creates an alias register of fixed offset register `alias` with its own fields.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $alias:ident
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $alias:ident
{ $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(
@@ -831,57 +812,67 @@ macro_rules! register {
<$alias as $crate::io::register::Register>::OFFSET
);
$crate::register!(@io_fixed $(#[$attr])* $vis $name);
+ $crate::register!($($rest)*);
};
// Creates a register at a relative offset from a base address provider.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $base:ident + $offset:literal
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $base:ident + $offset:literal
{ $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $offset);
$crate::register!(@io_relative $vis $name @ $base);
+ $crate::register!($($rest)*);
};
// Creates an alias register of relative offset register `alias` with its own fields.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $base:ident + $alias:ident
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $base:ident + $alias:ident
{ $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(
@io_base $name @ <$alias as $crate::io::register::Register>::OFFSET
);
$crate::register!(@io_relative $vis $name @ $base);
+ $crate::register!($($rest)*);
};
// Creates an array of registers at a fixed offset of the MMIO space.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
[ $size:expr, stride = $stride:expr ] @ $offset:literal { $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $offset);
$crate::register!(@io_array $vis $name [ $size, stride = $stride ]);
+ $crate::register!($($rest)*);
};
// Shortcut for contiguous array of registers (stride == size of element).
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ] @ $offset:literal
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ] @ $offset:literal
{ $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::register!(
$(#[$attr])* $vis $name($storage) [ $size, stride = ::core::mem::size_of::<$storage>() ]
@ $offset { $($fields)* }
);
+ $crate::register!($($rest)*);
};
// Creates an alias of register `idx` of array of registers `alias` with its own fields.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $alias:ident [ $idx:expr ]
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $alias:ident [ $idx:expr ]
{ $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::build_assert::static_assert!(
$idx < <$alias as $crate::io::register::RegisterArray>::SIZE
@@ -894,13 +885,15 @@ macro_rules! register {
+ $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
);
$crate::register!(@io_fixed $(#[$attr])* $vis $name);
+ $crate::register!($($rest)*);
};
// Creates an array of registers at a relative offset from a base address provider.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
[ $size:expr, stride = $stride:expr ]
@ $base:ident + $offset:literal { $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
@@ -909,24 +902,28 @@ macro_rules! register {
$crate::register!(
@io_relative_array $vis $name [ $size, stride = $stride ] @ $base + $offset
);
+ $crate::register!($($rest)*);
};
// Shortcut for contiguous array of relative registers (stride == size of element).
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ]
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ]
@ $base:ident + $offset:literal { $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::register!(
$(#[$attr])* $vis $name($storage) [ $size, stride = ::core::mem::size_of::<$storage>() ]
@ $base + $offset { $($fields)* }
);
+ $crate::register!($($rest)*);
};
// Creates an alias of register `idx` of relative array of registers `alias` with its own
// fields.
(
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
=> $base:ident + $alias:ident [ $idx:expr ] { $($fields:tt)* }
+ $($rest:tt)*
) => {
$crate::build_assert::static_assert!(
$idx < <$alias as $crate::io::register::RegisterArray>::SIZE
@@ -939,8 +936,11 @@ macro_rules! register {
$idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
);
$crate::register!(@io_relative $vis $name @ $base);
+ $crate::register!($($rest)*);
};
+ // All the rules below are private helpers.
+
// Generates the bitfield for the register.
//
// `#[allow(non_camel_case_types)]` is added since register names typically use
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v2 04/16] rust: io: register: handle one register at a time
2026-08-05 16:35 ` [PATCH v2 04/16] rust: io: register: handle one register at a time Gary Guo
@ 2026-08-05 16:43 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-08-05 16:43 UTC (permalink / raw)
To: Gary Guo; +Cc: ojeda, Alexandre Courbot, dri-devel, linux-pci, Danilo Krummrich
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The refactored `register!` macro uses a tail-recursive TT-muncher pattern, which increases macro recursion depth linearly with the number of registers defined. This can cause compilation failures (reaching rustc's recursion limit) for macro blocks defining a large number of registers.
--
--- Patch [4]: [PATCH v2 04/16] rust: io: register: handle one register at a time ---
commit 8ec2a5aa6d4bcb8811ecb0ce6d13218a169b562d
Author: Gary Guo <gary@garyguo.net>
rust: io: register: handle one register at a time
This commit refactors the register! macro to match and handle one register
definition at a time using a recursive pattern. It aims to simplify the rule
definitions by avoiding a single catch-all repetition rule.
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index d898b2b46d521..91804c1ca117d 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
[ ... ]
> @@ -786,44 +786,25 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> /// ```
> #[macro_export]
> macro_rules! register {
[ ... ]
> - // All the rules below are private helpers.
> + () => {};
>
> // Creates a register at a fixed offset of the MMIO space.
> (
> - @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $offset:literal
> + $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $offset:literal
> { $($fields:tt)* }
> + $($rest:tt)*
> ) => {
> $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
> $crate::register!(@io_base $name @ $offset);
> $crate::register!(@io_fixed $(#[$attr])* $vis $name);
> + $crate::register!($($rest)*);
[Severity: Low]
Does this recursive pattern increase the macro recursion depth linearly with
the number of registers defined?
If a single register! block contains a large number of registers, this tail
recursion might exceed rustc's default recursion limit and cause compilation
failures. While this can be worked around by splitting the definitions into
multiple register! invocations, it is a regression compared to the previous
repetition pattern.
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-typed_register-v2-0-c3ca142220a0@garyguo.net?part=4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 05/16] rust: io: register extract offset computation to helper rules
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (3 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 04/16] rust: io: register: handle one register at a time Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:42 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 06/16] rust: io: register: allow explicit base type specification Gary Guo
` (10 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Create a helper rule for register offset computation so there can be a
single rule for fixed offset registers.
Similarly, extract stride computation to helper rules.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io/register.rs | 115 ++++++++++++++-------------------------------
1 file changed, 36 insertions(+), 79 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 91804c1ca117..e4039e31b4e7 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -789,27 +789,19 @@ macro_rules! register {
() => {};
// Creates a register at a fixed offset of the MMIO space.
+ //
+ // This handles all of the fixed offset `@ offset`, alias of register `=> alias` and alias of
+ // register array element `=> alias[idx]` cases.
(
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $offset:literal
- { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name @ $offset);
- $crate::register!(@io_fixed $(#[$attr])* $vis $name);
- $crate::register!($($rest)*);
- };
-
- // Creates an alias register of fixed offset register `alias` with its own fields.
- (
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $alias:ident
- { $($fields:tt)* }
+ $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
+ $(@ $offset:literal)?
+ $(=> $alias:path $([$alias_idx:expr])? )?
+ { $($fields:tt)* }
$($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(
- @io_base $name @
- <$alias as $crate::io::register::Register>::OFFSET
+ $crate::register!(@io_base $name
+ @ $crate::register!(@offset $(@ $offset)? $(=> $alias $([$alias_idx])?)?)
);
$crate::register!(@io_fixed $(#[$attr])* $vis $name);
$crate::register!($($rest)*);
@@ -834,9 +826,7 @@ macro_rules! register {
$($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(
- @io_base $name @ <$alias as $crate::io::register::Register>::OFFSET
- );
+ $crate::register!(@io_base $name @ $crate::register!(@offset => $alias));
$crate::register!(@io_relative $vis $name @ $base);
$crate::register!($($rest)*);
};
@@ -844,76 +834,28 @@ macro_rules! register {
// Creates an array of registers at a fixed offset of the MMIO space.
(
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
- [ $size:expr, stride = $stride:expr ] @ $offset:literal { $($fields:tt)* }
+ [ $size:expr $(, stride = $stride:expr)? ] @ $offset:literal { $($fields:tt)* }
$($rest:tt)*
) => {
- $crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
-
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $offset);
- $crate::register!(@io_array $vis $name [ $size, stride = $stride ]);
- $crate::register!($($rest)*);
- };
-
- // Shortcut for contiguous array of registers (stride == size of element).
- (
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ] @ $offset:literal
- { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::register!(
- $(#[$attr])* $vis $name($storage) [ $size, stride = ::core::mem::size_of::<$storage>() ]
- @ $offset { $($fields)* }
- );
- $crate::register!($($rest)*);
- };
-
- // Creates an alias of register `idx` of array of registers `alias` with its own fields.
- (
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $alias:ident [ $idx:expr ]
- { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::build_assert::static_assert!(
- $idx < <$alias as $crate::io::register::RegisterArray>::SIZE
+ $crate::register!(@io_array $vis $name
+ [ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ]
);
-
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(
- @io_base $name @
- <$alias as $crate::io::register::Register>::OFFSET
- + $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
- );
- $crate::register!(@io_fixed $(#[$attr])* $vis $name);
$crate::register!($($rest)*);
};
// Creates an array of registers at a relative offset from a base address provider.
(
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
- [ $size:expr, stride = $stride:expr ]
+ [ $size:expr $(, stride = $stride:expr)? ]
@ $base:ident + $offset:literal { $($fields:tt)* }
$($rest:tt)*
) => {
- $crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
-
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $offset);
- $crate::register!(
- @io_relative_array $vis $name [ $size, stride = $stride ] @ $base + $offset
- );
- $crate::register!($($rest)*);
- };
-
- // Shortcut for contiguous array of relative registers (stride == size of element).
- (
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ]
- @ $base:ident + $offset:literal { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::register!(
- $(#[$attr])* $vis $name($storage) [ $size, stride = ::core::mem::size_of::<$storage>() ]
- @ $base + $offset { $($fields)* }
+ $crate::register!(@io_relative_array $vis $name
+ [ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ] @ $base + $offset
);
$crate::register!($($rest)*);
};
@@ -930,11 +872,7 @@ macro_rules! register {
);
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(
- @io_base $name @
- <$alias as $crate::io::register::Register>::OFFSET +
- $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
- );
+ $crate::register!(@io_base $name @ $crate::register!(@offset => $alias [$idx]));
$crate::register!(@io_relative $vis $name @ $base);
$crate::register!($($rest)*);
};
@@ -954,6 +892,25 @@ macro_rules! register {
);
};
+ // Offset computation helper rules.
+ (@offset @ $offset:expr) => { $offset };
+ (@offset => $alias:path) => { <$alias as $crate::io::register::Register>::OFFSET };
+ (@offset => $alias:path [$idx:expr]) => {{
+ $crate::build_assert::static_assert!(
+ $idx < <$alias as $crate::io::register::RegisterArray>::SIZE
+ );
+
+ <$alias as $crate::io::register::Register>::OFFSET +
+ $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
+ }};
+
+ // Stride computation helper rules.
+ (@stride $ty: ty, $stride: expr) => {{
+ $crate::build_assert::static_assert!(::core::mem::size_of::<$ty>() <= $stride);
+ $stride
+ }};
+ (@stride $ty: ty) => { ::core::mem::size_of::<$ty>() };
+
// Implementations shared by all registers types.
(@io_base $name:ident @ $offset:expr) => {
impl $crate::io::register::Register for $name {
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 06/16] rust: io: register: allow explicit base type specification
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (4 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 05/16] rust: io: register extract offset computation to helper rules Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:43 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 07/16] gpu: nova-core: specify base type for registers Gary Guo
` (9 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Currently registers work for all untyped I/O regions, which is not ideal.
It allows registers defined for device A to work for another device B and
there is no safeguarding at all.
All users of the `register!` macro know what type it will be operating on,
and that type is consistent across the driver. Therefore, add a `base`
parameter to `register!`.
Currently this parameter is unused in the generated code; it will be used
when all users of `register!` is converted to gain the parameter.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io.rs | 4 +++
rust/kernel/io/register.rs | 84 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 74 insertions(+), 14 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index ae18890866b6..d92e0b6adc99 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -1022,6 +1022,8 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
/// };
///
/// register! {
+ /// base: Region;
+ ///
/// VERSION(u32) @ 0x100 {
/// 15:8 major;
/// 7:0 minor;
@@ -1164,6 +1166,8 @@ fn write<T, L>(self, location: L, value: T)
/// };
///
/// register! {
+ /// base: Region<0x1000>;
+ ///
/// VERSION(u32) @ 0x100 {
/// 15:8 major;
/// 7:0 minor;
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index e4039e31b4e7..7dca2437b551 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -13,9 +13,14 @@
//! # Simple example
//!
//! ```no_run
-//! use kernel::io::register;
+//! use kernel::io::{
+//! register,
+//! Region,
+//! };
//!
//! register! {
+//! base: Region<0x1000>;
+//!
//! /// Basic information about the chip.
//! pub BOOT_0(u32) @ 0x00000100 {
//! /// Vendor ID.
@@ -55,11 +60,14 @@
//! register,
//! Io,
//! IoLoc,
+//! Region,
//! },
//! num::Bounded,
//! };
-//! # use kernel::io::{Mmio, Region};
+//! # use kernel::io::Mmio;
//! # register! {
+//! # base: Region<0x1000>;
+//! #
//! # pub BOOT_0(u32) @ 0x00000100 {
//! # 15:8 vendor_id;
//! # 7:4 major_revision;
@@ -429,11 +437,14 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// io::{
/// register,
/// Io,
+/// Region,
/// },
/// };
-/// # use kernel::io::{Mmio, Region};
+/// # use kernel::io::Mmio;
///
/// register! {
+/// base: Region<0x1000>;
+///
/// FIXED_REG(u32) @ 0x100 {
/// 15:8 high_byte;
/// 7:0 low_byte;
@@ -464,9 +475,14 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// the context:
///
/// ```no_run
-/// use kernel::io::register;
+/// use kernel::io::{
+/// register,
+/// Region,
+/// };
///
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Scratch register.
/// pub SCRATCH(u32) @ 0x00000200 {
/// 31:0 value;
@@ -516,6 +532,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
///
/// ```ignore
/// register! {
+/// ...
/// pub RELATIVE_REG(u32) @ Base + 0x80 {
/// ...
/// }
@@ -542,9 +559,10 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// WithBase,
/// },
/// Io,
+/// Region,
/// },
/// };
-/// # use kernel::io::{Mmio, Region};
+/// # use kernel::io::Mmio;
///
/// // Type used to identify the base.
/// pub struct CpuCtlBase;
@@ -563,6 +581,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
///
/// // This makes `CPU_CTL` accessible from all implementors of `RegisterBase<CpuCtlBase>`.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// CPU core control.
/// pub CPU_CTL(u32) @ CpuCtlBase + 0x10 {
/// 0:0 start;
@@ -579,6 +599,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
///
/// // Aliases can also be defined for relative register.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Alias to CPU core control.
/// pub CPU_CTL_ALIAS(u32) => CpuCtlBase + CPU_CTL {
/// /// Start the aliased CPU core.
@@ -621,15 +643,18 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// register,
/// register::Array,
/// Io,
+/// Region,
/// },
/// };
-/// # use kernel::io::{Mmio, Region};
+/// # use kernel::io::Mmio;
/// # fn get_scratch_idx() -> usize {
/// # 0x15
/// # }
///
/// // Array of 64 consecutive registers with the same layout starting at offset `0x80`.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Scratch registers.
/// pub SCRATCH(u32)[64] @ 0x00000080 {
/// 31:0 value;
@@ -655,6 +680,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// // Alias to a specific register in an array.
/// // Here `SCRATCH[8]` is used to convey the firmware exit code.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Firmware exit status code.
/// pub FIRMWARE_STATUS(u32) => SCRATCH[8] {
/// 7:0 status;
@@ -667,6 +694,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// // Here, each of the 16 registers of the array is separated by 8 bytes, meaning that the
/// // registers of the two declarations below are interleaved.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Scratch registers bank 0.
/// pub SCRATCH_INTERLEAVED_0(u32)[16, stride = 8] @ 0x000000c0 {
/// 31:0 value;
@@ -688,6 +717,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
///
/// ```ignore
/// register! {
+/// ...
/// pub RELATIVE_REGISTER_ARRAY(u8)[10, stride = 4] @ Base + 0x100 {
/// ...
/// }
@@ -707,9 +737,10 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// WithBase,
/// },
/// Io,
+/// Region,
/// },
/// };
-/// # use kernel::io::{Mmio, Region};
+/// # use kernel::io::Mmio;
/// # fn get_scratch_idx() -> usize {
/// # 0x15
/// # }
@@ -731,6 +762,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
///
/// // 64 per-cpu scratch registers, arranged as a contiguous array.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Per-CPU scratch registers.
/// pub CPU_SCRATCH(u32)[64] @ CpuCtlBase + 0x00000080 {
/// 31:0 value;
@@ -758,6 +791,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
///
/// // Alias to `SCRATCH[8]` used to convey the firmware exit code.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Per-CPU firmware exit status code.
/// pub CPU_FIRMWARE_STATUS(u32) => CpuCtlBase + CPU_SCRATCH[8] {
/// 7:0 status;
@@ -768,6 +803,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// // Here, each of the 16 registers of the array is separated by 8 bytes, meaning that the
/// // registers of the two declarations below are interleaved.
/// register! {
+/// base: Region<0x1000>;
+///
/// /// Scratch registers bank 0.
/// pub CPU_SCRATCH_INTERLEAVED_0(u32)[16, stride = 8] @ CpuCtlBase + 0x00000d00 {
/// 31:0 value;
@@ -786,13 +823,19 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// ```
#[macro_export]
macro_rules! register {
- () => {};
+ (base: $reg_base:ty;) => {
+ const _: () = {
+ #[allow(unused)]
+ type Base = $reg_base;
+ };
+ };
// Creates a register at a fixed offset of the MMIO space.
//
// This handles all of the fixed offset `@ offset`, alias of register `=> alias` and alias of
// register array element `=> alias[idx]` cases.
(
+ base: $reg_base:ty;
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
$(@ $offset:literal)?
$(=> $alias:path $([$alias_idx:expr])? )?
@@ -804,11 +847,12 @@ macro_rules! register {
@ $crate::register!(@offset $(@ $offset)? $(=> $alias $([$alias_idx])?)?)
);
$crate::register!(@io_fixed $(#[$attr])* $vis $name);
- $crate::register!($($rest)*);
+ $crate::register!(base: $reg_base; $($rest)*);
};
// Creates a register at a relative offset from a base address provider.
(
+ base: $reg_base:ty;
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $base:ident + $offset:literal
{ $($fields:tt)* }
$($rest:tt)*
@@ -816,11 +860,12 @@ macro_rules! register {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $offset);
$crate::register!(@io_relative $vis $name @ $base);
- $crate::register!($($rest)*);
+ $crate::register!(base: $reg_base; $($rest)*);
};
// Creates an alias register of relative offset register `alias` with its own fields.
(
+ base: $reg_base:ty;
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $base:ident + $alias:ident
{ $($fields:tt)* }
$($rest:tt)*
@@ -828,11 +873,12 @@ macro_rules! register {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $crate::register!(@offset => $alias));
$crate::register!(@io_relative $vis $name @ $base);
- $crate::register!($($rest)*);
+ $crate::register!(base: $reg_base; $($rest)*);
};
// Creates an array of registers at a fixed offset of the MMIO space.
(
+ base: $reg_base:ty;
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
[ $size:expr $(, stride = $stride:expr)? ] @ $offset:literal { $($fields:tt)* }
$($rest:tt)*
@@ -842,11 +888,12 @@ macro_rules! register {
$crate::register!(@io_array $vis $name
[ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ]
);
- $crate::register!($($rest)*);
+ $crate::register!(base: $reg_base; $($rest)*);
};
// Creates an array of registers at a relative offset from a base address provider.
(
+ base: $reg_base:ty;
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
[ $size:expr $(, stride = $stride:expr)? ]
@ $base:ident + $offset:literal { $($fields:tt)* }
@@ -857,12 +904,13 @@ macro_rules! register {
$crate::register!(@io_relative_array $vis $name
[ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ] @ $base + $offset
);
- $crate::register!($($rest)*);
+ $crate::register!(base: $reg_base; $($rest)*);
};
// Creates an alias of register `idx` of relative array of registers `alias` with its own
// fields.
(
+ base: $reg_base:ty;
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
=> $base:ident + $alias:ident [ $idx:expr ] { $($fields:tt)* }
$($rest:tt)*
@@ -874,7 +922,7 @@ macro_rules! register {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
$crate::register!(@io_base $name @ $crate::register!(@offset => $alias [$idx]));
$crate::register!(@io_relative $vis $name @ $base);
- $crate::register!($($rest)*);
+ $crate::register!(base: $reg_base; $($rest)*);
};
// All the rules below are private helpers.
@@ -962,4 +1010,12 @@ impl $crate::io::register::RegisterArray for $name {
impl $crate::io::register::RelativeRegisterArray for $name {}
};
+
+ // Compatibility rule when base is not specified.
+ ($($rest:tt)*) => {
+ $crate::register!(
+ base: $crate::io::Region;
+ $($rest)*
+ );
+ }
}
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v2 06/16] rust: io: register: allow explicit base type specification
2026-08-05 16:35 ` [PATCH v2 06/16] rust: io: register: allow explicit base type specification Gary Guo
@ 2026-08-05 16:43 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-08-05 16:43 UTC (permalink / raw)
To: Gary Guo; +Cc: ojeda, Danilo Krummrich, Alexandre Courbot, dri-devel, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `register!` macro fallback rule `($($rest:tt)*)` causes infinite recursion on any syntax error.
--
--- Patch [6]: [PATCH v2 06/16] rust: io: register: allow explicit base type specification ---
commit f65d8a2bc4e7792b0378bf9b7d9359faf6c62680
Author: Gary Guo <gary@garyguo.net>
rust: io: register: allow explicit base type specification
This commit adds a base parameter to the register! macro to allow explicit
base type specification for I/O regions. It updates the macro definitions
to accept this parameter and adds a compatibility fallback rule for users
that have not yet been converted.
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index e4039e31b4e78..7dca2437b5516 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
[ ... ]
> @@ -962,4 +1010,12 @@ impl $crate::io::register::RegisterArray for $name {
>
> impl $crate::io::register::RelativeRegisterArray for $name {}
> };
> +
> + // Compatibility rule when base is not specified.
> + ($($rest:tt)*) => {
> + $crate::register!(
> + base: $crate::io::Region;
> + $($rest)*
> + );
> + }
> }
[Severity: Medium]
Will this catch-all rule cause infinite recursion if there is a syntax
error elsewhere in the macro invocation?
If a developer makes a typo inside a register! block, the strict explicit
matching rules will fail. The token stream then falls through to this
compatibility rule, which prepends the base region and recursively invokes
the macro. Because the original syntax error is still present, the explicit
rules will fail again, triggering this fallback rule repeatedly.
Could this obfuscate the original syntax error by ultimately failing with a
compiler recursion limit error instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-typed_register-v2-0-c3ca142220a0@garyguo.net?part=6
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 07/16] gpu: nova-core: specify base type for registers
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (5 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 06/16] rust: io: register: allow explicit base type specification Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:42 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 08/16] drm/tyr: " Gary Guo
` (8 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
All registers use the same base type, which is `<Bar0 as IO>::Target`. Thus
add the base parameter to `register!` invocation.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
drivers/gpu/nova-core/driver.rs | 1 +
drivers/gpu/nova-core/fb/regs.rs | 12 ++++++++++++
drivers/gpu/nova-core/gsp/regs.rs | 9 ++++++++-
drivers/gpu/nova-core/regs.rs | 37 ++++++++++++++++++++++++++++++++++++-
drivers/gpu/nova-core/vbios.rs | 11 ++++++++++-
5 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index bbd93959e0b2..cf3534dd47d4 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -37,6 +37,7 @@ pub(crate) struct NovaCore<'bound> {
const BAR0_SIZE: usize = SZ_16M;
pub(crate) type Bar0<'a> = &'a pci::Bar<'a, BAR0_SIZE>;
+pub(crate) type NovaRegisters = kernel::io::Region<BAR0_SIZE>;
kernel::pci_device_table!(
PCI_TABLE,
diff --git a/drivers/gpu/nova-core/fb/regs.rs b/drivers/gpu/nova-core/fb/regs.rs
index 95adbe124a30..c27582e376e2 100644
--- a/drivers/gpu/nova-core/fb/regs.rs
+++ b/drivers/gpu/nova-core/fb/regs.rs
@@ -5,9 +5,13 @@
sizes::SizeConstants, //
};
+use crate::driver::NovaRegisters;
+
// PDISP
register! {
+ base: NovaRegisters;
+
pub(super) NV_PDISP_VGA_WORKSPACE_BASE(u32) @ 0x00625f04 {
/// VGA workspace base address divided by 0x10000.
31:8 addr;
@@ -30,6 +34,8 @@ pub(super) fn vga_workspace_addr(self) -> Option<u64> {
// PFB
register! {
+ base: NovaRegisters;
+
/// Low bits of the physical system memory address used by the GPU to perform sysmembar
/// operations (see [`crate::fb::SysmemFlush`]).
pub(super) NV_PFB_NISO_FLUSH_SYSMEM_ADDR(u32) @ 0x00100c10 {
@@ -65,6 +71,8 @@ pub(super) fn vga_workspace_addr(self) -> Option<u64> {
pub(super) struct Hshub0Base(());
register! {
+ base: NovaRegisters;
+
// GB10x sysmem flush registers, relative to the HSHUB0 base. GB10x routes sysmembar
// through a primary and an EG (egress) pair that must both be programmed to the same
// address. Hardware ignores bits 7:0 of each LO register. The boot path uses a fixed
@@ -87,6 +95,8 @@ pub(super) fn vga_workspace_addr(self) -> Option<u64> {
}
register! {
+ base: NovaRegisters;
+
// GB20x FBHUB0 sysmem flush registers. Unlike the older
// NV_PFB_NISO_FLUSH_SYSMEM_ADDR registers, which encode the address with an
// 8-bit right-shift, these take the raw address split into lower and upper
@@ -101,6 +111,8 @@ pub(super) fn vga_workspace_addr(self) -> Option<u64> {
}
register! {
+ base: NovaRegisters;
+
/// Low bits of the physical system memory address used by the GPU to perform
/// sysmembar operations on Hopper.
///
diff --git a/drivers/gpu/nova-core/gsp/regs.rs b/drivers/gpu/nova-core/gsp/regs.rs
index 9a48aa87e7fb..3c410d65e8e4 100644
--- a/drivers/gpu/nova-core/gsp/regs.rs
+++ b/drivers/gpu/nova-core/gsp/regs.rs
@@ -2,11 +2,16 @@
use kernel::io::register;
-use crate::regs::NV_PBUS_SW_SCRATCH;
+use crate::{
+ driver::NovaRegisters,
+ regs::NV_PBUS_SW_SCRATCH, //
+};
// PGSP
register! {
+ base: NovaRegisters;
+
pub(super) NV_PGSP_QUEUE_HEAD(u32) @ 0x00110c00 {
31:0 address;
}
@@ -15,6 +20,8 @@
// PBUS
register! {
+ base: NovaRegisters;
+
/// Scratch register 0xe used as FRTS firmware error code.
pub(super) NV_PBUS_SW_SCRATCH_0E_FRTS_ERR(u32) => NV_PBUS_SW_SCRATCH[0xe] {
31:16 frts_err_code;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index caeef4d85874..1af073f3861f 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -13,7 +13,10 @@
};
use crate::{
- driver::Bar0,
+ driver::{
+ Bar0,
+ NovaRegisters, //
+ },
falcon::{
DmaTrfCmdSize,
FalconCoreRev,
@@ -37,6 +40,8 @@
// PMC
register! {
+ base: NovaRegisters;
+
/// Basic revision information about the GPU.
pub(crate) NV_PMC_BOOT_0(u32) @ 0x00000000 {
/// Lower bits of the architecture.
@@ -108,6 +113,8 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
// PBUS
register! {
+ base: NovaRegisters;
+
pub(crate) NV_PBUS_SW_SCRATCH(u32)[64] @ 0x00001400 {}
}
@@ -121,6 +128,8 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
// number.
register! {
+ base: NovaRegisters;
+
/// Boot Sequence Interface (BSI) register used to determine
/// if GSP reload/resume has completed during the boot process.
pub(crate) NV_PGC6_BSI_SECURE_SCRATCH_14(u32) @ 0x001180f8 {
@@ -175,6 +184,8 @@ pub(crate) fn usable_fb_size(self) -> u64 {
pub(crate) const NV_FUSE_OPT_FPF_SIZE: usize = 16;
register! {
+ base: NovaRegisters;
+
pub(crate) NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION(u32)[NV_FUSE_OPT_FPF_SIZE] @ 0x00824100 {
15:0 data => u16;
}
@@ -191,6 +202,8 @@ pub(crate) fn usable_fb_size(self) -> u64 {
// PFALCON
register! {
+ base: NovaRegisters;
+
pub(crate) NV_PFALCON_FALCON_IRQSCLR(u32) @ PFalconBase + 0x00000004 {
6:6 swgen0 => bool;
4:4 halt => bool;
@@ -392,6 +405,8 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
/* PFALCON2 */
register! {
+ base: NovaRegisters;
+
pub(crate) NV_PFALCON2_FALCON_MOD_SEL(u32) @ PFalcon2Base + 0x00000180 {
7:0 algo ?=> FalconModSelAlgo;
}
@@ -414,6 +429,8 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
// PRISCV
register! {
+ base: NovaRegisters;
+
/// RISC-V status register for debug (Turing and GA100 only).
/// Reflects current RISC-V core status.
pub(crate) NV_PRISCV_RISCV_CORE_SWITCH_RISCV_STATUS(u32) @ PFalcon2Base + 0x00000240 {
@@ -439,6 +456,8 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
// These registers manage falcon EMEM communication queues.
register! {
+ base: NovaRegisters;
+
pub(crate) NV_PFSP_QUEUE_HEAD(u32)[8] @ 0x008f2c00 {
31:0 address => u32;
}
@@ -462,9 +481,13 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
pub(crate) mod gm107 {
use kernel::io::register;
+ use crate::driver::NovaRegisters;
+
// FUSE
register! {
+ base: NovaRegisters;
+
pub(crate) NV_FUSE_STATUS_OPT_DISPLAY(u32) @ 0x00021c04 {
0:0 display_disabled => bool;
}
@@ -474,9 +497,13 @@ pub(crate) mod gm107 {
pub(crate) mod ga100 {
use kernel::io::register;
+ use crate::driver::NovaRegisters;
+
// FUSE
register! {
+ base: NovaRegisters;
+
pub(crate) NV_FUSE_STATUS_OPT_DISPLAY(u32) @ 0x00820c04 {
0:0 display_disabled => bool;
}
@@ -488,9 +515,13 @@ pub(crate) mod ga100 {
pub(crate) mod gh100 {
use kernel::io::register;
+ use crate::driver::NovaRegisters;
+
// PTHERM
register! {
+ base: NovaRegisters;
+
pub(crate) NV_THERM_I2CS_SCRATCH(u32) @ 0x000200bc {
31:0 data;
}
@@ -505,9 +536,13 @@ pub(crate) mod gh100 {
pub(crate) mod gb202 {
use kernel::io::register;
+ use crate::driver::NovaRegisters;
+
// PTHERM
register! {
+ base: NovaRegisters;
+
pub(crate) NV_THERM_I2CS_SCRATCH(u32) @ 0x00ad00bc {
31:0 data;
}
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index c03650ee5226..9c214b9f4dd9 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -16,7 +16,10 @@
};
use crate::{
- driver::Bar0,
+ driver::{
+ Bar0,
+ NovaRegisters, //
+ },
firmware::{
fwsec::Bcrt30Rsa3kSignature,
FalconUCodeDesc,
@@ -92,12 +95,16 @@ impl<'a> VbiosIterator<'a> {
fn rom_offset(dev: &device::Device, bar0: Bar0<'_>) -> Result<usize> {
// IFR Header in VBIOS.
register! {
+ base: NovaRegisters;
+
NV_PBUS_IFR_FMT_FIXED0(u32) @ 0x300000 {
31:0 signature;
}
}
register! {
+ base: NovaRegisters;
+
NV_PBUS_IFR_FMT_FIXED1(u32) @ 0x300004 {
30:16 fixed_data_size;
15:8 version => u8;
@@ -105,6 +112,8 @@ fn rom_offset(dev: &device::Device, bar0: Bar0<'_>) -> Result<usize> {
}
register! {
+ base: NovaRegisters;
+
NV_PBUS_IFR_FMT_FIXED2(u32) @ 0x300008 {
19:0 total_data_size;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 08/16] drm/tyr: specify base type for registers
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (6 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 07/16] gpu: nova-core: specify base type for registers Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:47 ` sashiko-bot
2026-08-05 16:59 ` Gary Guo
2026-08-05 16:35 ` [PATCH v2 09/16] samples: rust: pci: " Gary Guo
` (7 subsequent siblings)
15 siblings, 2 replies; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
All registers use the same base type, which is `<IoMem as IO>::Target`.
Thus add the base parameter to `register!` invocation.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
drivers/gpu/drm/tyr/driver.rs | 1 +
drivers/gpu/drm/tyr/regs.rs | 43 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index bfb0ba19caff..730b84e37a54 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -46,6 +46,7 @@
};
pub(crate) type IoMem<'a> = kernel::io::mem::IoMem<'a, SZ_2M>;
+pub(crate) type TyrRegisters = kernel::io::Region<SZ_2M>;
pub(crate) struct TyrDrmDriver;
diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
index a62724378ced..3e6edae6e27f 100644
--- a/drivers/gpu/drm/tyr/regs.rs
+++ b/drivers/gpu/drm/tyr/regs.rs
@@ -57,7 +57,11 @@ pub(crate) mod gpu_control {
uapi, //
};
+ use crate::driver::TyrRegisters;
+
register! {
+ base: TyrRegisters;
+
/// GPU identification register.
pub(crate) GPU_ID(u32) @ 0x0 {
/// Status of the GPU release.
@@ -315,6 +319,8 @@ fn from(mode: FlushMode) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// GPU command register.
///
/// Use the constructor methods to create commands:
@@ -380,6 +386,8 @@ pub(crate) fn clear_fault() -> Self {
}
register! {
+ base: TyrRegisters;
+
/// GPU status register. Read only.
pub(crate) GPU_STATUS(u32) @ 0x34 {
/// GPU active, a 1-bit boolean flag.
@@ -463,6 +471,8 @@ fn from(access: AccessType) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// GPU fault status register. Read only.
pub(crate) GPU_FAULTSTATUS(u32) @ 0x3c {
/// Exception type.
@@ -768,6 +778,8 @@ fn from(mode: CoherencyMode) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// Coherency enable. An index of which coherency protocols should be used.
/// This register only selects the protocol for coherency messages on the
/// interconnect. This is not to enable or disable coherency controlled by MMU.
@@ -808,6 +820,8 @@ fn from(mode: McuControlMode) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// MCU control.
pub(crate) MCU_CONTROL(u32) @ 0x700 {
/// Request MCU state change.
@@ -849,6 +863,8 @@ fn from(status: McuStatus) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// MCU status. Read only.
pub(crate) MCU_STATUS(u32) @ 0x704 {
/// Read current state of MCU.
@@ -862,7 +878,11 @@ fn from(status: McuStatus) -> Self {
pub(crate) mod job_control {
use kernel::register;
+ use crate::driver::TyrRegisters;
+
register! {
+ base: TyrRegisters;
+
/// Raw status of job interrupts.
///
/// Write to this register to trigger these interrupts.
@@ -912,7 +932,11 @@ pub(crate) mod job_control {
pub(crate) mod mmu_control {
use kernel::register;
+ use crate::driver::TyrRegisters;
+
register! {
+ base: TyrRegisters;
+
/// IRQ sources raw status.
///
/// This register contains the raw unmasked interrupt sources for MMU status and exception
@@ -966,9 +990,10 @@ pub(crate) mod mmu_as_control {
prelude::*,
register, //
};
-
use pin_init::Zeroable;
+ use crate::driver::TyrRegisters;
+
/// Maximum number of hardware address space slots.
/// The actual number of slots available is usually lower.
pub(crate) const MAX_AS: usize = 16;
@@ -977,6 +1002,8 @@ pub(crate) mod mmu_as_control {
const STRIDE: usize = 0x40;
register! {
+ base: TyrRegisters;
+
/// Translation table base address. A 64-bit pointer.
///
/// This field contains the address of the top level of a translation table structure.
@@ -1104,6 +1131,8 @@ fn from(val: MemoryType) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// Stage 1 memory attributes (8-bit bitfield).
///
/// This is not an actual register, but a bitfield definition used by the MEMATTR
@@ -1137,6 +1166,8 @@ fn from(val: MMU_MEMATTR_STAGE1) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// Memory attributes.
///
/// Each address space can configure up to 8 different memory attribute profiles.
@@ -1353,6 +1384,8 @@ fn from(cmd: MmuCommand) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// MMU command register for each address space. Write only.
pub(crate) COMMAND(u32)[MAX_AS, stride = STRIDE] @ 0x2418 {
7:0 command ?=> MmuCommand;
@@ -1480,6 +1513,8 @@ fn from(access: MmuAccessType) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// Fault status register for each address space. Read only.
pub(crate) FAULTSTATUS(u32)[MAX_AS, stride = STRIDE] @ 0x241c {
/// Exception type.
@@ -1705,6 +1740,8 @@ fn from(sh: PtwShareability) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// Translation configuration and control.
pub(crate) TRANSCFG(u64)[MAX_AS, stride = STRIDE] @ 0x2430 {
/// Address space mode.
@@ -1760,6 +1797,8 @@ fn from(sh: PtwShareability) -> Self {
pub(crate) mod doorbell_block {
use kernel::register;
+ use crate::driver::TyrRegisters;
+
/// Number of doorbells available.
pub(crate) const NUM_DOORBELLS: usize = 64;
@@ -1770,6 +1809,8 @@ pub(crate) mod doorbell_block {
const STRIDE: usize = 0x10000;
register! {
+ base: TyrRegisters;
+
/// Doorbell request register. Write-only.
pub(crate) DOORBELL(u32)[NUM_DOORBELLS, stride = STRIDE] @ 0x80000 {
/// Doorbell set. Writing 1 triggers the doorbell.
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v2 08/16] drm/tyr: specify base type for registers
2026-08-05 16:35 ` [PATCH v2 08/16] drm/tyr: " Gary Guo
@ 2026-08-05 16:47 ` sashiko-bot
2026-08-05 16:59 ` Gary Guo
1 sibling, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-08-05 16:47 UTC (permalink / raw)
To: Gary Guo; +Cc: linux-pci, ojeda, Alexandre Courbot, dri-devel, Danilo Krummrich
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete application of base parameter in register! macro invocations.
--
--- Patch [8]: [PATCH v2 08/16] drm/tyr: specify base type for registers ---
commit 53a6ffbf27038a15b41db145d107fac75047c4d1
Author: Gary Guo <gary@garyguo.net>
drm/tyr: specify base type for registers
This commit updates all register definitions to explicitly use the TyrRegisters
base type in the register! macro invocations.
> diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
> index a62724378ced2..3e6edae6e27f7 100644
> --- a/drivers/gpu/drm/tyr/regs.rs
> +++ b/drivers/gpu/drm/tyr/regs.rs
[ ... ]
> @@ -1353,6 +1384,8 @@ fn from(cmd: MmuCommand) -> Self {
> }
>
> register! {
> + base: TyrRegisters;
> +
> /// MMU command register for each address space. Write only.
> pub(crate) COMMAND(u32)[MAX_AS, stride = STRIDE] @ 0x2418 {
> 7:0 command ?=> MmuCommand;
[Severity: Medium]
The commit message states that all registers are updated to use the new base
type, but it looks like a few register! invocations were missed during the
conversion.
For instance, earlier in this same file (drivers/gpu/drm/tyr/regs.rs), the
LOCKADDR register block doesn't have the new base parameter:
register! {
/// Lock region address for each address space.
pub(crate) LOCKADDR(u64)[MAX_AS, stride = STRIDE] @ 0x2410 {
/// Lock region size.
5:0 size;
Additionally, the SectionFlags register in drivers/gpu/drm/tyr/fw.rs was also
missed by this patch:
register! {
#[allow(non_upper_case_globals)]
pub(super) SectionFlags(u32) @ 0x0 {
0:0 read => bool;
Were these intentionally left with the default base type, or were they
overlooked during the refactoring?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-typed_register-v2-0-c3ca142220a0@garyguo.net?part=8
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 08/16] drm/tyr: specify base type for registers
2026-08-05 16:35 ` [PATCH v2 08/16] drm/tyr: " Gary Guo
2026-08-05 16:47 ` sashiko-bot
@ 2026-08-05 16:59 ` Gary Guo
1 sibling, 0 replies; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:59 UTC (permalink / raw)
To: Gary Guo, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Bjorn Helgaas, Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci
On Wed Aug 5, 2026 at 5:35 PM BST, Gary Guo wrote:
> All registers use the same base type, which is `<IoMem as IO>::Target`.
> Thus add the base parameter to `register!` invocation.
This is missing a few `register!` invocation that's introduced in the new
linux-next base, as Sashiko points out. Will include in the next version.
Best,
Gary
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/drm/tyr/driver.rs | 1 +
> drivers/gpu/drm/tyr/regs.rs | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index bfb0ba19caff..730b84e37a54 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -46,6 +46,7 @@
> };
>
> pub(crate) type IoMem<'a> = kernel::io::mem::IoMem<'a, SZ_2M>;
> +pub(crate) type TyrRegisters = kernel::io::Region<SZ_2M>;
>
> pub(crate) struct TyrDrmDriver;
>
> diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
> index a62724378ced..3e6edae6e27f 100644
> --- a/drivers/gpu/drm/tyr/regs.rs
> +++ b/drivers/gpu/drm/tyr/regs.rs
> @@ -57,7 +57,11 @@ pub(crate) mod gpu_control {
> uapi, //
> };
>
> + use crate::driver::TyrRegisters;
> +
> register! {
> + base: TyrRegisters;
> +
> /// GPU identification register.
> pub(crate) GPU_ID(u32) @ 0x0 {
> /// Status of the GPU release.
> @@ -315,6 +319,8 @@ fn from(mode: FlushMode) -> Self {
> }
>
> register! {
> + base: TyrRegisters;
> +
> /// GPU command register.
> ///
> /// Use the constructor methods to create commands:
> @@ -380,6 +386,8 @@ pub(crate) fn clear_fault() -> Self {
> }
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 09/16] samples: rust: pci: specify base type for registers
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (7 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 08/16] drm/tyr: " Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:41 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 10/16] rust: io: register: make register have a typed base Gary Guo
` (6 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
The `register!` macro is going to require explicit base type, specify it
for both `register!` usages in PCI sample driver.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
samples/rust/rust_driver_pci.rs | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 2282191e6292..13b035a95756 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -23,6 +23,8 @@ mod regs {
use super::*;
register! {
+ base: kernel::io::Region<END>;
+
pub(super) TEST(u8) @ 0x0 {
7:0 index => TestIndex;
}
@@ -102,6 +104,8 @@ fn config_space(pdev: &pci::Device<Bound>) {
// Some PCI configuration space registers.
register! {
+ base: pci::Normal;
+
VENDOR_ID(u16) @ 0x0 {
15:0 vendor_id;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 10/16] rust: io: register: make register have a typed base
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (8 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 09/16] samples: rust: pci: " Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:43 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield Gary Guo
` (5 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Previously `register!` defined registers can be used on any untyped I/O
regions. With all users specifying their desired register type now,
propagate the specified type and restrict I/O access only when type
matches.
Also, add an `io_project!` example which is enabled by this change.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io.rs | 13 ++++++++++
rust/kernel/io/register.rs | 64 +++++++++++++++++++---------------------------
2 files changed, 40 insertions(+), 37 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index d92e0b6adc99..d56c8faa7d7c 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -1798,21 +1798,34 @@ pub fn project_loc<U, L>(self, location: L) -> <T::Backend as IoBackend>::View<'
/// The syntax is of form `io_project!(io, proj)` where `io` is an expression to a type that
/// implements [`Io`] and `proj` is a [projection specification](kernel::ptr::project!).
///
+/// `io_project!` can also project to subview of registers defined with [`register!`] macro.
+/// Register projection has syntax `io_project!(io, try: REGISTER)` for fallible projection and
+/// `io_project!(io, build: REGISTER)` for infallible projection.
+///
/// # Examples
///
/// ```
/// use kernel::io::{
/// io_project,
+/// register,
/// Mmio,
/// };
/// #[repr(C)]
/// struct MyStruct { field: u32, }
///
+/// register! {
+/// base: MyStruct;
+/// FIELD(u32) @ 0 {
+/// 31:0 val;
+/// }
+/// }
+///
/// # fn test(mmio: Mmio<'_, [MyStruct]>) -> Result {
/// // let mmio: Mmio<[MyStruct]>;
/// let field: Mmio<'_, u32> = io_project!(mmio, [try: 1].field);
/// let whole: Mmio<'_, MyStruct> = io_project!(mmio, [try: 2]);
/// let nested: Mmio<'_, u32> = io_project!(whole, .field);
+/// let reg: Mmio<'_, FIELD> = io_project!(whole, build: FIELD);
/// # Ok::<(), Error>(()) }
/// ```
#[macro_export]
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 7dca2437b551..dc800fc71172 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -121,10 +121,11 @@
io::IoLoc, //
};
-use super::Region;
-
/// Trait implemented by all registers.
pub trait Register: Sized {
+ /// Base type for this register.
+ type Base: ?Sized;
+
/// Start offset of the register.
///
/// The interpretation of this offset depends on the type of the register.
@@ -136,9 +137,9 @@ pub trait FixedRegister: Register {}
/// Allows `()` to be used as the `location` parameter of [`Io::write`](super::Io::write) when
/// passing a [`FixedRegister`] value.
-impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for ()
+impl<Base: ?Sized, T> IoLoc<Base, T> for ()
where
- T: FixedRegister,
+ T: FixedRegister<Base = Base>,
{
#[inline(always)]
fn offset(self) -> usize {
@@ -148,9 +149,9 @@ fn offset(self) -> usize {
/// A [`FixedRegister`] carries its location in its type. Thus `FixedRegister` values can be used
/// as an [`IoLoc`].
-impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for T
+impl<Base: ?Sized, T> IoLoc<Base, T> for T
where
- T: FixedRegister,
+ T: FixedRegister<Base = Base>,
{
#[inline(always)]
fn offset(self) -> usize {
@@ -171,9 +172,9 @@ pub const fn new() -> Self {
}
}
-impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for FixedRegisterLoc<T>
+impl<Base: ?Sized, T> IoLoc<Base, T> for FixedRegisterLoc<T>
where
- T: FixedRegister,
+ T: FixedRegister<Base = Base>,
{
#[inline(always)]
fn offset(self) -> usize {
@@ -240,9 +241,9 @@ const fn offset(self) -> usize {
}
}
-impl<const SIZE: usize, T, B> IoLoc<Region<SIZE>, T> for RelativeRegisterLoc<T, B>
+impl<SuperBase: ?Sized, T, B> IoLoc<SuperBase, T> for RelativeRegisterLoc<T, B>
where
- T: RelativeRegister,
+ T: RelativeRegister<Base = SuperBase>,
B: RegisterBase<T::BaseFamily> + ?Sized,
{
#[inline(always)]
@@ -282,9 +283,9 @@ pub fn try_new(idx: usize) -> Option<Self> {
}
}
-impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for RegisterArrayLoc<T>
+impl<Base: ?Sized, T> IoLoc<Base, T> for RegisterArrayLoc<T>
where
- T: RegisterArray,
+ T: RegisterArray<Base = Base>,
{
#[inline(always)]
fn offset(self) -> usize {
@@ -367,9 +368,9 @@ pub fn try_at(self, idx: usize) -> Option<RelativeRegisterArrayLoc<T, B>> {
}
}
-impl<const SIZE: usize, T, B> IoLoc<Region<SIZE>, T> for RelativeRegisterArrayLoc<T, B>
+impl<SuperBase: ?Sized, T, B> IoLoc<SuperBase, T> for RelativeRegisterArrayLoc<T, B>
where
- T: RelativeRegisterArray,
+ T: RelativeRegisterArray<Base = SuperBase>,
B: RegisterBase<T::BaseFamily> + ?Sized,
{
#[inline(always)]
@@ -393,9 +394,9 @@ pub trait LocatedRegister<Base: ?Sized> {
fn into_io_op(self) -> (Self::Location, Self::Value);
}
-impl<const SIZE: usize, T> LocatedRegister<Region<SIZE>> for T
+impl<Base: ?Sized, T> LocatedRegister<Base> for T
where
- T: FixedRegister,
+ T: FixedRegister<Base = Base>,
{
type Location = FixedRegisterLoc<Self::Value>;
type Value = T;
@@ -823,12 +824,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// ```
#[macro_export]
macro_rules! register {
- (base: $reg_base:ty;) => {
- const _: () = {
- #[allow(unused)]
- type Base = $reg_base;
- };
- };
+ (base: $reg_base:ty;) => {};
// Creates a register at a fixed offset of the MMIO space.
//
@@ -843,7 +839,7 @@ macro_rules! register {
$($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name
+ $crate::register!(@io_base $reg_base; $name
@ $crate::register!(@offset $(@ $offset)? $(=> $alias $([$alias_idx])?)?)
);
$crate::register!(@io_fixed $(#[$attr])* $vis $name);
@@ -858,7 +854,7 @@ macro_rules! register {
$($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name @ $offset);
+ $crate::register!(@io_base $reg_base; $name @ $offset);
$crate::register!(@io_relative $vis $name @ $base);
$crate::register!(base: $reg_base; $($rest)*);
};
@@ -871,7 +867,7 @@ macro_rules! register {
$($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name @ $crate::register!(@offset => $alias));
+ $crate::register!(@io_base $reg_base; $name @ $crate::register!(@offset => $alias));
$crate::register!(@io_relative $vis $name @ $base);
$crate::register!(base: $reg_base; $($rest)*);
};
@@ -884,7 +880,7 @@ macro_rules! register {
$($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name @ $offset);
+ $crate::register!(@io_base $reg_base; $name @ $offset);
$crate::register!(@io_array $vis $name
[ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ]
);
@@ -900,7 +896,7 @@ macro_rules! register {
$($rest:tt)*
) => {
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name @ $offset);
+ $crate::register!(@io_base $reg_base; $name @ $offset);
$crate::register!(@io_relative_array $vis $name
[ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ] @ $base + $offset
);
@@ -920,7 +916,7 @@ macro_rules! register {
);
$crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name @ $crate::register!(@offset => $alias [$idx]));
+ $crate::register!(@io_base $reg_base; $name @ $crate::register!(@offset => $alias [$idx]));
$crate::register!(@io_relative $vis $name @ $base);
$crate::register!(base: $reg_base; $($rest)*);
};
@@ -960,8 +956,10 @@ macro_rules! register {
(@stride $ty: ty) => { ::core::mem::size_of::<$ty>() };
// Implementations shared by all registers types.
- (@io_base $name:ident @ $offset:expr) => {
+ (@io_base $reg_base:ty; $name:ident @ $offset:expr) => {
impl $crate::io::register::Register for $name {
+ type Base = $reg_base;
+
const OFFSET: usize = $offset;
}
};
@@ -1010,12 +1008,4 @@ impl $crate::io::register::RegisterArray for $name {
impl $crate::io::register::RelativeRegisterArray for $name {}
};
-
- // Compatibility rule when base is not specified.
- ($($rest:tt)*) => {
- $crate::register!(
- base: $crate::io::Region;
- $($rest)*
- );
- }
}
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (9 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 10/16] rust: io: register: make register have a typed base Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:50 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers Gary Guo
` (4 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Add a rule to allow creating `IoLoc` in `regiser!()` using an existing type
and not create a bitfield. Add an example to demonstrate this for FIFO
registers.
This rule is also going to be used to create subregions for registers; the
example of doing so will be added later when relative registers are
removed.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io/register.rs | 47 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index dc800fc71172..49a61da106c7 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -182,6 +182,23 @@ fn offset(self) -> usize {
}
}
+#[doc(hidden)]
+pub struct OffsetLoc<Base: ?Sized, T>(usize, PhantomData<(T, Base)>);
+
+impl<Base: ?Sized, T> OffsetLoc<Base, T> {
+ #[inline]
+ pub const fn new(offset: usize) -> Self {
+ Self(offset, PhantomData)
+ }
+}
+
+impl<Base: ?Sized, T> IoLoc<Base, T> for OffsetLoc<Base, T> {
+ #[inline(always)]
+ fn offset(self) -> usize {
+ self.0
+ }
+}
+
/// Trait providing a base address to be added to the offset of a relative register to obtain
/// its actual offset.
///
@@ -499,6 +516,19 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// In this example, `SCRATCH_BOOT_STATUS` uses the same I/O address as `SCRATCH`, while providing
/// its own `completed` field.
///
+/// If you do not wish to have a bitfield defined, you can also create a register using an existing
+/// type.
+///
+/// ```no_run
+/// # use kernel::io::*;
+/// register! {
+/// base: Region<0x1000>;
+///
+/// /// TX FIFO register.
+/// pub TX_FIFO: u32 @ 0x00001000;
+/// }
+/// ```
+///
/// ## Relative registers
///
/// Relative registers can be instantiated several times at a relative offset of a group of bases.
@@ -826,6 +856,23 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
macro_rules! register {
(base: $reg_base:ty;) => {};
+ // Creates a register at a fixed offset of the MMIO space with provided type.
+ (
+ base: $reg_base:ty;
+ // `$ty` cannot be `:ty` due to follow-set restrictions.
+ $(#[$attr:meta])* $vis:vis $name:ident: $ty: ident $(:: $path_frag:ident)*
+ $(@ $offset:literal)?
+ $(=> $alias:path $([$alias_idx:expr])? )?;
+ $($rest:tt)*
+ ) => {
+ $(#[$attr])* $vis
+ const $name: $crate::io::register::OffsetLoc<$reg_base, $ty $(:: $path_frag)*> =
+ $crate::io::register::OffsetLoc::new(
+ $crate::register!(@offset $(@ $offset)? $(=> $alias $([$alias_idx])?)?)
+ );
+ $crate::register!(base: $reg_base; $($rest)*);
+ };
+
// Creates a register at a fixed offset of the MMIO space.
//
// This handles all of the fixed offset `@ offset`, alias of register `=> alias` and alias of
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield
2026-08-05 16:35 ` [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield Gary Guo
@ 2026-08-05 16:50 ` sashiko-bot
2026-08-05 17:05 ` Gary Guo
0 siblings, 1 reply; 35+ messages in thread
From: sashiko-bot @ 2026-08-05 16:50 UTC (permalink / raw)
To: Gary Guo; +Cc: Alexandre Courbot, linux-pci, ojeda, dri-devel, Danilo Krummrich
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `register!` macro documentation provides an example defining a FIFO register (`TX_FIFO`) with a `u32` type. This violates the subsystem guidelines which mandate that FIFOs must use stream accessors (no byteswapping). Using a `u32` register type implies standard register access (e.g., `writel`), which will byteswap on big-endian architectures and corrupt stream data. Additionally, the macro's syntax rule `$ty: ident $(:: $path_frag:ident)*` restricts the type to paths, preventing developers from directly specifying a byte array (e.g., `[u8; 4]`) as the type to properly model a byteswap-free stream.
--
--- Patch [11]: [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield ---
commit a13a899311e93c249f5925eff4ce304f82a4cb69
Author: Gary Guo <gary@garyguo.net>
rust: io: register: support fixed offset register without bitfield
Add a rule to allow creating `IoLoc` in `regiser!()` using an existing type
and not create a bitfield. Add an example to demonstrate this for FIFO
registers.
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index dc800fc711722..49a61da106c7d 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
[ ... ]
> @@ -499,6 +516,19 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> /// In this example, `SCRATCH_BOOT_STATUS` uses the same I/O address as `SCRATCH`, while providing
> /// its own `completed` field.
> ///
> +/// If you do not wish to have a bitfield defined, you can also create a register using an existing
> +/// type.
> +///
> +/// ```no_run
> +/// # use kernel::io::*;
> +/// register! {
> +/// base: Region<0x1000>;
> +///
> +/// /// TX FIFO register.
> +/// pub TX_FIFO: u32 @ 0x00001000;
[Severity: Low]
Does this example promote an anti-pattern for FIFO access?
Using u32 for a FIFO implies standard register I/O accessors (like writel or
readl), which will perform byte-swapping on big-endian architectures.
According to the I/O accessor subsystem guidelines, FIFOs should use stream
accessors without byteswapping to prevent data corruption.
Would it be better to avoid using a FIFO as the example here, or use a type
that enforces stream semantics?
> +/// }
> +/// ```
> +///
> /// ## Relative registers
[ ... ]
> @@ -826,6 +856,23 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> macro_rules! register {
> (base: $reg_base:ty;) => {};
>
> + // Creates a register at a fixed offset of the MMIO space with provided type.
> + (
> + base: $reg_base:ty;
> + // `$ty` cannot be `:ty` due to follow-set restrictions.
> + $(#[$attr:meta])* $vis:vis $name:ident: $ty: ident $(:: $path_frag:ident)*
[Severity: Low]
Does this macro rule prevent developers from specifying inline byte arrays
(like [u8; 4]) as the register type?
Since the type matcher is restricted to identifiers and paths, it seems
developers wouldn't be able to directly define a byte array to properly model
a byteswap-free stream without defining a separate type alias first.
Could this be adjusted to support array syntax, or is the limitation strict
enough to require type aliases for stream buffers?
> + $(@ $offset:literal)?
> + $(=> $alias:path $([$alias_idx:expr])? )?;
> + $($rest:tt)*
> + ) => {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-typed_register-v2-0-c3ca142220a0@garyguo.net?part=11
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield
2026-08-05 16:50 ` sashiko-bot
@ 2026-08-05 17:05 ` Gary Guo
0 siblings, 0 replies; 35+ messages in thread
From: Gary Guo @ 2026-08-05 17:05 UTC (permalink / raw)
To: sashiko-reviews, Gary Guo
Cc: Alexandre Courbot, linux-pci, ojeda, dri-devel, Danilo Krummrich
On Wed Aug 5, 2026 at 5:50 PM BST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The `register!` macro documentation provides an example defining a FIFO register (`TX_FIFO`) with a `u32` type. This violates the subsystem guidelines which mandate that FIFOs must use stream accessors (no byteswapping). Using a `u32` register type implies standard register access (e.g., `writel`), which will byteswap on big-endian architectures and corrupt stream data. Additionally, the macro's syntax rule `$ty: ident $(:: $path_frag:ident)*` restricts the type to paths, preventing developers from directly specifying a byte array (e.g., `[u8; 4]`) as the type to properly model a byteswap-free stream.
> --
>
> --- Patch [11]: [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield ---
> commit a13a899311e93c249f5925eff4ce304f82a4cb69
> Author: Gary Guo <gary@garyguo.net>
>
> rust: io: register: support fixed offset register without bitfield
>
> Add a rule to allow creating `IoLoc` in `regiser!()` using an existing type
> and not create a bitfield. Add an example to demonstrate this for FIFO
> registers.
>
>> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
>> index dc800fc711722..49a61da106c7d 100644
>> --- a/rust/kernel/io/register.rs
>> +++ b/rust/kernel/io/register.rs
>
> [ ... ]
>
>> @@ -499,6 +516,19 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
>> /// In this example, `SCRATCH_BOOT_STATUS` uses the same I/O address as `SCRATCH`, while providing
>> /// its own `completed` field.
>> ///
>> +/// If you do not wish to have a bitfield defined, you can also create a register using an existing
>> +/// type.
>> +///
>> +/// ```no_run
>> +/// # use kernel::io::*;
>> +/// register! {
>> +/// base: Region<0x1000>;
>> +///
>> +/// /// TX FIFO register.
>> +/// pub TX_FIFO: u32 @ 0x00001000;
>
> [Severity: Low]
> Does this example promote an anti-pattern for FIFO access?
>
> Using u32 for a FIFO implies standard register I/O accessors (like writel or
> readl), which will perform byte-swapping on big-endian architectures.
> According to the I/O accessor subsystem guidelines, FIFOs should use stream
> accessors without byteswapping to prevent data corruption.
Sashiko is misunderstanding the use case here, this is for intent here is for
PIO FIFO where you read one word as a time for a fixed location, not streaming
(otherwise the signature here would be `pub TX_FIFO: [u32; FIFO_SIZE] @ ...`).
I'll see if I can have a better example in the next version. Perhaps
pub UART_RX: u8 @ ...;
?
Best,
Gary
>
> Would it be better to avoid using a FIFO as the example here, or use a type
> that enforces stream semantics?
>
>> +/// }
>> +/// ```
>> +///
>> /// ## Relative registers
>
> [ ... ]
>
>> @@ -826,6 +856,23 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
>> macro_rules! register {
>> (base: $reg_base:ty;) => {};
>>
>> + // Creates a register at a fixed offset of the MMIO space with provided type.
>> + (
>> + base: $reg_base:ty;
>> + // `$ty` cannot be `:ty` due to follow-set restrictions.
>> + $(#[$attr:meta])* $vis:vis $name:ident: $ty: ident $(:: $path_frag:ident)*
>
> [Severity: Low]
> Does this macro rule prevent developers from specifying inline byte arrays
> (like [u8; 4]) as the register type?
>
> Since the type matcher is restricted to identifiers and paths, it seems
> developers wouldn't be able to directly define a byte array to properly model
> a byteswap-free stream without defining a separate type alias first.
>
> Could this be adjusted to support array syntax, or is the limitation strict
> enough to require type aliases for stream buffers?
>
>> + $(@ $offset:literal)?
>> + $(=> $alias:path $([$alias_idx:expr])? )?;
>> + $($rest:tt)*
>> + ) => {
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (10 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:46 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 13/16] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
` (3 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Add fixed size region types `PFalconRegisters` and `PFalcon2Registers` and
update PFALCON and PFALCON registers to be fixed register on them and not
relative registers on `NovaRegisters`.
Update `Falcon` struct to store projected views when constructing and
access with `self.pfalcon` and `self.pfalcon2`.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
drivers/gpu/nova-core/falcon.rs | 157 +++++++++------------
drivers/gpu/nova-core/falcon/fsp.rs | 63 +++++----
drivers/gpu/nova-core/falcon/gsp.rs | 51 ++++---
drivers/gpu/nova-core/falcon/hal/ga102.rs | 62 ++++----
drivers/gpu/nova-core/falcon/hal/tu102.rs | 9 +-
drivers/gpu/nova-core/falcon/sec2.rs | 37 +++--
drivers/gpu/nova-core/firmware/fwsec/bootloader.rs | 18 +--
drivers/gpu/nova-core/gsp/hal/tu102.rs | 7 +-
drivers/gpu/nova-core/regs.rs | 91 ++++++------
9 files changed, 238 insertions(+), 257 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index a91cbdd5d636..ed52572690ff 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -14,13 +14,12 @@
},
io::{
poll::read_poll_timeout,
- register::{
- RegisterBase,
- WithBase, //
- },
+ register::Array,
Io,
+ Mmio, //
},
prelude::*,
+ sizes::SZ_4K,
time::Delta,
};
@@ -165,18 +164,22 @@ pub(crate) enum FalconFbifMemType with From<Bounded<u32, 1>> {
}
}
-/// Type used to represent the `PFALCON` registers address base for a given falcon engine.
-pub(crate) struct PFalconBase(());
+/// Type used to represent the `PFALCON` registers.
+#[repr(align(4))]
+#[derive(FromBytes, IntoBytes)]
+pub(crate) struct PFalconRegisters([u8; SZ_4K]);
-/// Type used to represent the `PFALCON2` registers address base for a given falcon engine.
-pub(crate) struct PFalcon2Base(());
+/// Type used to represent the `PFALCON2` registers.
+#[repr(align(4))]
+#[derive(FromBytes, IntoBytes)]
+pub(crate) struct PFalcon2Registers([u8; SZ_4K]);
/// Trait defining the parameters of a given Falcon engine.
///
/// Each engine provides one base for `PFALCON` and `PFALCON2` registers.
-pub(crate) trait FalconEngine:
- Send + Sync + RegisterBase<PFalconBase> + RegisterBase<PFalcon2Base> + Sized
-{
+pub(crate) trait FalconEngine: Send + Sync + Sized {
+ fn pfalcon(io: Bar0<'_>) -> Mmio<'_, PFalconRegisters>;
+ fn pfalcon2(io: Bar0<'_>) -> Mmio<'_, PFalcon2Registers>;
}
/// Represents a portion of the firmware to be loaded into a particular memory (e.g. IMEM or DMEM)
@@ -358,6 +361,8 @@ pub(crate) struct Falcon<'a, E: FalconEngine> {
hal: KBox<dyn FalconHal<E>>,
dev: &'a device::Device<device::Bound>,
bar: Bar0<'a>,
+ pub(crate) pfalcon: Mmio<'a, PFalconRegisters>,
+ pfalcon2: Mmio<'a, PFalcon2Registers>,
}
impl<'a, E: FalconEngine + 'static> Falcon<'a, E> {
@@ -371,19 +376,19 @@ pub(crate) fn new(
hal: hal::falcon_hal(chipset)?,
dev,
bar,
+ pfalcon: E::pfalcon(bar),
+ pfalcon2: E::pfalcon2(bar),
})
}
/// Resets DMA-related registers.
pub(crate) fn dma_reset(&self) {
- self.bar.update(regs::NV_PFALCON_FBIF_CTL::of::<E>(), |v| {
+ self.pfalcon.update(regs::NV_PFALCON_FBIF_CTL, |v| {
v.with_allow_phys_no_ctx(true)
});
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_DMACTL::zeroed(),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_DMACTL::zeroed());
}
/// Reset the controller, select the falcon core, and wait for memory scrubbing to complete.
@@ -392,10 +397,9 @@ pub(crate) fn reset(&self) -> Result {
self.hal.select_core(self)?;
self.hal.reset_wait_mem_scrubbing(self)?;
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_RM::from(self.bar.read(regs::NV_PMC_BOOT_0).into_raw()),
- );
+ self.pfalcon.write_reg(regs::NV_PFALCON_FALCON_RM::from(
+ self.bar.read(regs::NV_PMC_BOOT_0).into_raw(),
+ ));
Ok(())
}
@@ -413,8 +417,8 @@ fn pio_wr_imem_slice(&self, load_offsets: FalconPioImemLoadTarget<'_>) -> Result
return Err(EINVAL);
}
- self.bar.write(
- WithBase::of::<E>().at(Self::PIO_PORT),
+ self.pfalcon.write(
+ Array::at(Self::PIO_PORT),
regs::NV_PFALCON_FALCON_IMEMC::zeroed()
.with_secure(load_offsets.secure)
.with_aincw(true)
@@ -424,14 +428,14 @@ fn pio_wr_imem_slice(&self, load_offsets: FalconPioImemLoadTarget<'_>) -> Result
for (n, block) in load_offsets.data.chunks(MEM_BLOCK_ALIGNMENT).enumerate() {
let n = u16::try_from(n)?;
let tag: u16 = load_offsets.start_tag.checked_add(n).ok_or(ERANGE)?;
- self.bar.write(
- WithBase::of::<E>().at(Self::PIO_PORT),
+ self.pfalcon.write(
+ Array::at(Self::PIO_PORT),
regs::NV_PFALCON_FALCON_IMEMT::zeroed().with_tag(tag),
);
for word in block.chunks_exact(4) {
let w = [word[0], word[1], word[2], word[3]];
- self.bar.write(
- WithBase::of::<E>().at(Self::PIO_PORT),
+ self.pfalcon.write(
+ Array::at(Self::PIO_PORT),
regs::NV_PFALCON_FALCON_IMEMD::zeroed().with_data(u32::from_le_bytes(w)),
);
}
@@ -450,8 +454,8 @@ fn pio_wr_dmem_slice(&self, load_offsets: FalconPioDmemLoadTarget<'_>) -> Result
return Err(EINVAL);
}
- self.bar.write(
- WithBase::of::<E>().at(Self::PIO_PORT),
+ self.pfalcon.write(
+ Array::at(Self::PIO_PORT),
regs::NV_PFALCON_FALCON_DMEMC::zeroed()
.with_aincw(true)
.with_offs(load_offsets.dst_start),
@@ -459,8 +463,8 @@ fn pio_wr_dmem_slice(&self, load_offsets: FalconPioDmemLoadTarget<'_>) -> Result
for word in load_offsets.data.chunks_exact(4) {
let w = [word[0], word[1], word[2], word[3]];
- self.bar.write(
- WithBase::of::<E>().at(Self::PIO_PORT),
+ self.pfalcon.write(
+ Array::at(Self::PIO_PORT),
regs::NV_PFALCON_FALCON_DMEMD::zeroed().with_data(u32::from_le_bytes(w)),
);
}
@@ -473,14 +477,12 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
&self,
fw: &F,
) -> Result {
- self.bar.update(regs::NV_PFALCON_FBIF_CTL::of::<E>(), |v| {
+ self.pfalcon.update(regs::NV_PFALCON_FBIF_CTL, |v| {
v.with_allow_phys_no_ctx(true)
});
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_DMACTL::zeroed(),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_DMACTL::zeroed());
if let Some(imem_ns) = fw.imem_ns_load_params() {
self.pio_wr_imem_slice(imem_ns)?;
@@ -492,10 +494,8 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
self.hal.program_brom(self, &fw.brom_params());
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_BOOTVEC::zeroed().with_value(fw.boot_addr()),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_BOOTVEC::zeroed().with_value(fw.boot_addr()));
Ok(())
}
@@ -563,16 +563,13 @@ fn dma_wr(
// Set up the base source DMA address.
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_DMATRFBASE::zeroed().with_base(
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_DMATRFBASE::zeroed().with_base(
// CAST: `as u32` is used on purpose since we do want to strip the upper bits,
// which will be written to `NV_PFALCON_FALCON_DMATRFBASE1`.
(dma_start >> 8) as u32,
- ),
- );
- self.bar.write(
- WithBase::of::<E>(),
+ ));
+ self.pfalcon.write_reg(
regs::NV_PFALCON_FALCON_DMATRFBASE1::zeroed().try_with_base(dma_start >> 40)?,
);
@@ -582,23 +579,21 @@ fn dma_wr(
for pos in (0..num_transfers).map(|i| i * DMA_LEN) {
// Perform a transfer of size `DMA_LEN`.
- self.bar.write(
- WithBase::of::<E>(),
+ self.pfalcon.write_reg(
regs::NV_PFALCON_FALCON_DMATRFMOFFS::zeroed()
.try_with_offs(load_offsets.dst_start + pos)?,
);
- self.bar.write(
- WithBase::of::<E>(),
+ self.pfalcon.write_reg(
regs::NV_PFALCON_FALCON_DMATRFFBOFFS::zeroed().with_offs(src_start + pos),
);
- self.bar.write(WithBase::of::<E>(), cmd);
+ self.pfalcon.write_reg(cmd);
// Wait for the transfer to complete.
// TIMEOUT: arbitrarily large value, no DMA transfer to the falcon's small memories
// should ever take that long.
read_poll_timeout(
- || Ok(self.bar.read(regs::NV_PFALCON_FALCON_DMATRFCMD::of::<E>())),
+ || Ok(self.pfalcon.read(regs::NV_PFALCON_FALCON_DMATRFCMD)),
|r| r.idle(),
Delta::ZERO,
Delta::from_secs(2),
@@ -630,8 +625,8 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(&self, fw: &F) ->
};
self.dma_reset();
- self.bar
- .update(regs::NV_PFALCON_FBIF_TRANSCFG::of::<E>().at(0), |v| {
+ self.pfalcon
+ .update(regs::NV_PFALCON_FBIF_TRANSCFG::at(0), |v| {
v.with_target(FalconFbifTarget::CoherentSysmem)
.with_mem_type(FalconFbifMemType::Physical)
});
@@ -642,10 +637,8 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(&self, fw: &F) ->
self.hal.program_brom(self, &fw.brom_params());
// Set `BootVec` to start of non-secure code.
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_BOOTVEC::zeroed().with_value(fw.boot_addr()),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_BOOTVEC::zeroed().with_value(fw.boot_addr()));
Ok(())
}
@@ -654,7 +647,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(&self, fw: &F) ->
pub(crate) fn wait_till_halted(&self) -> Result<()> {
// TIMEOUT: arbitrarily large value, firmwares should complete in less than 2 seconds.
read_poll_timeout(
- || Ok(self.bar.read(regs::NV_PFALCON_FALCON_CPUCTL::of::<E>())),
+ || Ok(self.pfalcon.read(regs::NV_PFALCON_FALCON_CPUCTL)),
|r| r.halted(),
Delta::ZERO,
Delta::from_secs(2),
@@ -665,19 +658,13 @@ pub(crate) fn wait_till_halted(&self) -> Result<()> {
/// Start the falcon CPU.
pub(crate) fn start(&self) -> Result<()> {
- match self
- .bar
- .read(regs::NV_PFALCON_FALCON_CPUCTL::of::<E>())
- .alias_en()
- {
- true => self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_CPUCTL_ALIAS::zeroed().with_startcpu(true),
- ),
- false => self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_CPUCTL::zeroed().with_startcpu(true),
- ),
+ match self.pfalcon.read(regs::NV_PFALCON_FALCON_CPUCTL).alias_en() {
+ true => self
+ .pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_CPUCTL_ALIAS::zeroed().with_startcpu(true)),
+ false => self
+ .pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_CPUCTL::zeroed().with_startcpu(true)),
}
Ok(())
@@ -686,32 +673,24 @@ pub(crate) fn start(&self) -> Result<()> {
/// Writes values to the mailbox registers if provided.
pub(crate) fn write_mailboxes(&self, mbox0: Option<u32>, mbox1: Option<u32>) {
if let Some(mbox0) = mbox0 {
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_MAILBOX0::zeroed().with_value(mbox0),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_MAILBOX0::zeroed().with_value(mbox0));
}
if let Some(mbox1) = mbox1 {
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_MAILBOX1::zeroed().with_value(mbox1),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_MAILBOX1::zeroed().with_value(mbox1));
}
}
/// Reads the value from `mbox0` register.
pub(crate) fn read_mailbox0(&self) -> u32 {
- self.bar
- .read(regs::NV_PFALCON_FALCON_MAILBOX0::of::<E>())
- .value()
+ self.pfalcon.read(regs::NV_PFALCON_FALCON_MAILBOX0).value()
}
/// Reads the value from `mbox1` register.
pub(crate) fn read_mailbox1(&self) -> u32 {
- self.bar
- .read(regs::NV_PFALCON_FALCON_MAILBOX1::of::<E>())
- .value()
+ self.pfalcon.read(regs::NV_PFALCON_FALCON_MAILBOX1).value()
}
/// Reads values from both mailbox registers.
@@ -776,9 +755,7 @@ pub(crate) fn load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(&self, fw:
/// Write the application version to the OS register.
pub(crate) fn write_os_version(&self, app_version: u32) {
- self.bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON_FALCON_OS::zeroed().with_value(app_version),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_OS::zeroed().with_value(app_version));
}
}
diff --git a/drivers/gpu/nova-core/falcon/fsp.rs b/drivers/gpu/nova-core/falcon/fsp.rs
index 0437180b8829..85f9c8c5d60e 100644
--- a/drivers/gpu/nova-core/falcon/fsp.rs
+++ b/drivers/gpu/nova-core/falcon/fsp.rs
@@ -8,13 +8,12 @@
use kernel::{
io::{
+ io_project,
poll::read_poll_timeout,
- register::{
- Array,
- RegisterBase,
- WithBase, //
- },
- Io, //
+ register,
+ register::Array,
+ Io,
+ Mmio, //
},
prelude::*,
sizes::SZ_1K,
@@ -22,11 +21,13 @@
};
use crate::{
+ driver::{
+ Bar0,
+ NovaRegisters, //
+ },
falcon::{
Falcon,
- FalconEngine,
- PFalcon2Base,
- PFalconBase, //
+ FalconEngine, //
},
num,
regs, //
@@ -41,15 +42,24 @@
/// Type specifying the `Fsp` falcon engine. Cannot be instantiated.
pub(crate) struct Fsp(());
-impl RegisterBase<PFalconBase> for Fsp {
- const BASE: usize = 0x8f2000;
-}
+register! {
+ base: NovaRegisters;
-impl RegisterBase<PFalcon2Base> for Fsp {
- const BASE: usize = 0x8f3000;
+ PFALCON: super::PFalconRegisters @ 0x8f2000;
+ PFALCON2: super::PFalcon2Registers @ 0x8f3000;
}
-impl FalconEngine for Fsp {}
+impl FalconEngine for Fsp {
+ #[inline]
+ fn pfalcon(io: Bar0<'_>) -> Mmio<'_, super::PFalconRegisters> {
+ io_project!(io, build: PFALCON)
+ }
+
+ #[inline]
+ fn pfalcon2(io: Bar0<'_>) -> Mmio<'_, super::PFalcon2Registers> {
+ io_project!(io, build: PFALCON2)
+ }
+}
impl<'a> Falcon<'a, Fsp> {
/// Writes `data` to FSP external memory at offset `0`.
@@ -62,19 +72,15 @@ fn write_emem(&mut self, data: &[u8]) -> Result {
}
// Begin a write burst at offset `0`, auto-incrementing on each write.
- self.bar.write(
- WithBase::of::<Fsp>(),
- regs::NV_PFALCON_FALCON_EMEMC::zeroed().with_aincw(true),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_EMEMC::zeroed().with_aincw(true));
for chunk in data.chunks_exact(4) {
let value = u32::from_le_bytes([chunk[0], chunk[1], chunk[2], chunk[3]]);
// Write the next 32-bit `value`; hardware advances the offset.
- self.bar.write(
- WithBase::of::<Fsp>(),
- regs::NV_PFALCON_FALCON_EMEMD::zeroed().with_data(value),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_EMEMD::zeroed().with_data(value));
}
Ok(())
@@ -90,17 +96,12 @@ fn read_emem(&mut self, data: &mut [u8]) -> Result {
}
// Begin a read burst at offset `0`, auto-incrementing on each read.
- self.bar.write(
- WithBase::of::<Fsp>(),
- regs::NV_PFALCON_FALCON_EMEMC::zeroed().with_aincr(true),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_EMEMC::zeroed().with_aincr(true));
for chunk in data.chunks_exact_mut(4) {
// Read the next 32-bit word; hardware advances the offset.
- let value = self
- .bar
- .read(regs::NV_PFALCON_FALCON_EMEMD::of::<Fsp>())
- .data();
+ let value = self.pfalcon.read(regs::NV_PFALCON_FALCON_EMEMD).data();
chunk.copy_from_slice(&value.to_le_bytes());
}
diff --git a/drivers/gpu/nova-core/falcon/gsp.rs b/drivers/gpu/nova-core/falcon/gsp.rs
index ae32f401aeb0..cbea6d7b49d3 100644
--- a/drivers/gpu/nova-core/falcon/gsp.rs
+++ b/drivers/gpu/nova-core/falcon/gsp.rs
@@ -2,23 +2,24 @@
use kernel::{
io::{
+ io_project,
poll::read_poll_timeout,
- register::{
- RegisterBase,
- WithBase, //
- },
+ register,
Io,
+ Mmio, //
},
prelude::*,
time::Delta, //
};
use crate::{
+ driver::{
+ Bar0,
+ NovaRegisters, //
+ },
falcon::{
Falcon,
- FalconEngine,
- PFalcon2Base,
- PFalconBase, //
+ FalconEngine, //
},
regs,
};
@@ -26,24 +27,31 @@
/// Type specifying the `Gsp` falcon engine. Cannot be instantiated.
pub(crate) struct Gsp(());
-impl RegisterBase<PFalconBase> for Gsp {
- const BASE: usize = 0x00110000;
-}
+register! {
+ base: NovaRegisters;
-impl RegisterBase<PFalcon2Base> for Gsp {
- const BASE: usize = 0x00111000;
+ PFALCON: super::PFalconRegisters @ 0x00110000;
+ PFALCON2: super::PFalcon2Registers @ 0x00111000;
}
-impl FalconEngine for Gsp {}
+impl FalconEngine for Gsp {
+ #[inline]
+ fn pfalcon<'a>(io: Bar0<'a>) -> Mmio<'a, super::PFalconRegisters> {
+ io_project!(io, build: PFALCON)
+ }
+
+ #[inline]
+ fn pfalcon2<'a>(io: Bar0<'a>) -> Mmio<'a, super::PFalcon2Registers> {
+ io_project!(io, build: PFALCON2)
+ }
+}
impl<'a> Falcon<'a, Gsp> {
/// Clears the SWGEN0 bit in the Falcon's IRQ status clear register to
/// allow GSP to signal CPU for processing new messages in message queue.
pub(crate) fn clear_swgen0_intr(&self) {
- self.bar.write(
- WithBase::of::<Gsp>(),
- regs::NV_PFALCON_FALCON_IRQSCLR::zeroed().with_swgen0(true),
- );
+ self.pfalcon
+ .write_reg(regs::NV_PFALCON_FALCON_IRQSCLR::zeroed().with_swgen0(true));
}
/// Checks if GSP reload/resume has completed during the boot process.
@@ -59,8 +67,8 @@ pub(crate) fn check_reload_completed(&self, timeout: Delta) -> Result<bool> {
/// Returns whether the RISC-V branch privilege lockdown bit is set.
pub(crate) fn riscv_branch_privilege_lockdown(&self) -> bool {
- self.bar
- .read(regs::NV_PFALCON_FALCON_HWCFG2::of::<Gsp>())
+ self.pfalcon
+ .read(regs::NV_PFALCON_FALCON_HWCFG2)
.riscv_br_priv_lockdown()
}
@@ -71,10 +79,7 @@ pub(crate) fn priv_target_mask_released(&self) -> bool {
const LOCKED_PATTERN: u32 = 0xbadf_4100;
const LOCKED_MASK: u32 = 0xffff_ff00;
- let hwcfg2 = self
- .bar
- .read(regs::NV_PFALCON_FALCON_HWCFG2::of::<Gsp>())
- .into_raw();
+ let hwcfg2 = self.pfalcon.read(regs::NV_PFALCON_FALCON_HWCFG2).into_raw();
hwcfg2 != 0 && (hwcfg2 & LOCKED_MASK) != LOCKED_PATTERN
}
diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index 7600ee07ca2e..ebfaff3d960f 100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -6,11 +6,9 @@
device,
io::{
poll::read_poll_timeout,
- register::{
- Array,
- WithBase, //
- },
- Io, //
+ register::Array,
+ Io,
+ Mmio, //
},
prelude::*,
time::Delta, //
@@ -24,6 +22,7 @@
FalconBromParams,
FalconEngine,
FalconModSelAlgo,
+ PFalcon2Registers,
PeregrineCoreSelect, //
},
regs,
@@ -31,17 +30,16 @@
use super::FalconHal;
-fn select_core_ga102<E: FalconEngine>(bar: Bar0<'_>) -> Result {
- let bcr_ctrl = bar.read(regs::NV_PRISCV_RISCV_BCR_CTRL::of::<E>());
+fn select_core_ga102<E: FalconEngine>(pfalcon2: Mmio<'_, PFalcon2Registers>) -> Result {
+ let bcr_ctrl = pfalcon2.read(regs::NV_PRISCV_RISCV_BCR_CTRL);
if bcr_ctrl.core_select() != PeregrineCoreSelect::Falcon {
- bar.write(
- WithBase::of::<E>(),
+ pfalcon2.write_reg(
regs::NV_PRISCV_RISCV_BCR_CTRL::zeroed().with_core_select(PeregrineCoreSelect::Falcon),
);
// TIMEOUT: falcon core should take less than 10ms to report being enabled.
read_poll_timeout(
- || Ok(bar.read(regs::NV_PRISCV_RISCV_BCR_CTRL::of::<E>())),
+ || Ok(pfalcon2.read(regs::NV_PRISCV_RISCV_BCR_CTRL)),
|r| r.valid(),
Delta::ZERO,
Delta::from_millis(10),
@@ -86,24 +84,23 @@ fn signature_reg_fuse_version_ga102(
Ok(u16::BITS - reg_fuse_version.leading_zeros())
}
-fn program_brom_ga102<E: FalconEngine>(bar: Bar0<'_>, params: &FalconBromParams) {
- bar.write(
- WithBase::of::<E>().at(0),
+fn program_brom_ga102<E: FalconEngine>(
+ pfalcon2: Mmio<'_, PFalcon2Registers>,
+ params: &FalconBromParams,
+) {
+ pfalcon2.write(
+ Array::at(0),
regs::NV_PFALCON2_FALCON_BROM_PARAADDR::zeroed().with_value(params.pkc_data_offset),
);
- bar.write(
- WithBase::of::<E>(),
+ pfalcon2.write_reg(
regs::NV_PFALCON2_FALCON_BROM_ENGIDMASK::zeroed()
.with_value(u32::from(params.engine_id_mask)),
);
- bar.write(
- WithBase::of::<E>(),
+ pfalcon2.write_reg(
regs::NV_PFALCON2_FALCON_BROM_CURR_UCODE_ID::zeroed().with_ucode_id(params.ucode_id),
);
- bar.write(
- WithBase::of::<E>(),
- regs::NV_PFALCON2_FALCON_MOD_SEL::zeroed().with_algo(FalconModSelAlgo::Rsa3k),
- );
+ pfalcon2
+ .write_reg(regs::NV_PFALCON2_FALCON_MOD_SEL::zeroed().with_algo(FalconModSelAlgo::Rsa3k));
}
pub(super) struct Ga102<E: FalconEngine>(PhantomData<E>);
@@ -116,7 +113,7 @@ pub(super) fn new() -> Self {
impl<E: FalconEngine> FalconHal<E> for Ga102<E> {
fn select_core(&self, falcon: &Falcon<'_, E>) -> Result {
- select_core_ga102::<E>(falcon.bar)
+ select_core_ga102::<E>(falcon.pfalcon2)
}
fn signature_reg_fuse_version(
@@ -129,27 +126,24 @@ fn signature_reg_fuse_version(
}
fn program_brom(&self, falcon: &Falcon<'_, E>, params: &FalconBromParams) {
- program_brom_ga102::<E>(falcon.bar, params);
+ program_brom_ga102::<E>(falcon.pfalcon2, params);
}
fn is_riscv_active(&self, falcon: &Falcon<'_, E>) -> bool {
falcon
- .bar
- .read(regs::NV_PRISCV_RISCV_CPUCTL::of::<E>())
+ .pfalcon2
+ .read(regs::NV_PRISCV_RISCV_CPUCTL)
.active_stat()
}
fn is_riscv_halted(&self, falcon: &Falcon<'_, E>) -> Result<bool> {
- Ok(falcon
- .bar
- .read(regs::NV_PRISCV_RISCV_CPUCTL::of::<E>())
- .halted())
+ Ok(falcon.pfalcon2.read(regs::NV_PRISCV_RISCV_CPUCTL).halted())
}
fn reset_wait_mem_scrubbing(&self, falcon: &Falcon<'_, E>) -> Result {
// TIMEOUT: memory scrubbing should complete in less than 20ms.
read_poll_timeout(
- || Ok(falcon.bar.read(regs::NV_PFALCON_FALCON_HWCFG2::of::<E>())),
+ || Ok(falcon.pfalcon.read(regs::NV_PFALCON_FALCON_HWCFG2)),
|r| r.mem_scrubbing_done(),
Delta::ZERO,
Delta::from_millis(20),
@@ -158,20 +152,18 @@ fn reset_wait_mem_scrubbing(&self, falcon: &Falcon<'_, E>) -> Result {
}
fn reset_eng(&self, falcon: &Falcon<'_, E>) -> Result {
- let bar = falcon.bar;
-
- let _ = bar.read(regs::NV_PFALCON_FALCON_HWCFG2::of::<E>());
+ let _ = falcon.pfalcon.read(regs::NV_PFALCON_FALCON_HWCFG2);
// According to OpenRM's `kflcnPreResetWait_GA102` documentation, HW sometimes does not set
// RESET_READY so a non-failing timeout is used.
let _ = read_poll_timeout(
- || Ok(bar.read(regs::NV_PFALCON_FALCON_HWCFG2::of::<E>())),
+ || Ok(falcon.pfalcon.read(regs::NV_PFALCON_FALCON_HWCFG2)),
|r| r.reset_ready(),
Delta::ZERO,
Delta::from_micros(150),
);
- regs::NV_PFALCON_FALCON_ENGINE::reset_engine::<E>(bar);
+ regs::NV_PFALCON_FALCON_ENGINE::reset_engine::<E>(falcon.pfalcon);
self.reset_wait_mem_scrubbing(falcon)?;
Ok(())
diff --git a/drivers/gpu/nova-core/falcon/hal/tu102.rs b/drivers/gpu/nova-core/falcon/hal/tu102.rs
index 5291598fedf7..cfb0a435698d 100644
--- a/drivers/gpu/nova-core/falcon/hal/tu102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/tu102.rs
@@ -5,7 +5,6 @@
use kernel::{
io::{
poll::read_poll_timeout,
- register::WithBase,
Io, //
},
prelude::*,
@@ -50,8 +49,8 @@ fn program_brom(&self, _falcon: &Falcon<'_, E>, _params: &FalconBromParams) {}
fn is_riscv_active(&self, falcon: &Falcon<'_, E>) -> bool {
falcon
- .bar
- .read(regs::NV_PRISCV_RISCV_CORE_SWITCH_RISCV_STATUS::of::<E>())
+ .pfalcon2
+ .read(regs::NV_PRISCV_RISCV_CORE_SWITCH_RISCV_STATUS)
.active_stat()
}
@@ -62,7 +61,7 @@ fn is_riscv_halted(&self, _falcon: &Falcon<'_, E>) -> Result<bool> {
fn reset_wait_mem_scrubbing(&self, falcon: &Falcon<'_, E>) -> Result {
// TIMEOUT: memory scrubbing should complete in less than 10ms.
read_poll_timeout(
- || Ok(falcon.bar.read(regs::NV_PFALCON_FALCON_DMACTL::of::<E>())),
+ || Ok(falcon.pfalcon.read(regs::NV_PFALCON_FALCON_DMACTL)),
|r| r.mem_scrubbing_done(),
Delta::ZERO,
Delta::from_millis(10),
@@ -71,7 +70,7 @@ fn reset_wait_mem_scrubbing(&self, falcon: &Falcon<'_, E>) -> Result {
}
fn reset_eng(&self, falcon: &Falcon<'_, E>) -> Result {
- regs::NV_PFALCON_FALCON_ENGINE::reset_engine::<E>(falcon.bar);
+ regs::NV_PFALCON_FALCON_ENGINE::reset_engine::<E>(falcon.pfalcon);
self.reset_wait_mem_scrubbing(falcon)?;
Ok(())
diff --git a/drivers/gpu/nova-core/falcon/sec2.rs b/drivers/gpu/nova-core/falcon/sec2.rs
index 91ec7d49c1f5..6648a397d38a 100644
--- a/drivers/gpu/nova-core/falcon/sec2.rs
+++ b/drivers/gpu/nova-core/falcon/sec2.rs
@@ -1,22 +1,37 @@
// SPDX-License-Identifier: GPL-2.0
-use kernel::io::register::RegisterBase;
+use kernel::io::{
+ io_project,
+ register,
+ Mmio, //
+};
-use crate::falcon::{
- FalconEngine,
- PFalcon2Base,
- PFalconBase, //
+use crate::{
+ driver::{
+ Bar0,
+ NovaRegisters, //
+ },
+ falcon::FalconEngine, //
};
/// Type specifying the `Sec2` falcon engine. Cannot be instantiated.
pub(crate) struct Sec2(());
-impl RegisterBase<PFalconBase> for Sec2 {
- const BASE: usize = 0x00840000;
-}
+register! {
+ base: NovaRegisters;
-impl RegisterBase<PFalcon2Base> for Sec2 {
- const BASE: usize = 0x00841000;
+ PFALCON: super::PFalconRegisters @ 0x00840000;
+ PFALCON2: super::PFalcon2Registers @ 0x00841000;
}
-impl FalconEngine for Sec2 {}
+impl FalconEngine for Sec2 {
+ #[inline]
+ fn pfalcon(io: Bar0<'_>) -> Mmio<'_, super::PFalconRegisters> {
+ io_project!(io, build: PFALCON)
+ }
+
+ #[inline]
+ fn pfalcon2(io: Bar0<'_>) -> Mmio<'_, super::PFalcon2Registers> {
+ io_project!(io, build: PFALCON2)
+ }
+}
diff --git a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
index d9fafd2eea5b..f139aeb73f14 100644
--- a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
@@ -12,7 +12,10 @@
Device, //
},
dma::Coherent,
- io::{register::WithBase, Io},
+ io::{
+ register::Array,
+ Io, //
+ },
prelude::*,
ptr::{
Alignable,
@@ -26,7 +29,6 @@
};
use crate::{
- driver::Bar0,
falcon::{
self,
gsp::Gsp,
@@ -272,12 +274,7 @@ pub(crate) fn new(
///
/// The bootloader will load the FWSEC firmware and then execute it. This function returns
/// after FWSEC has reached completion.
- pub(crate) fn run(
- &self,
- dev: &Device<device::Bound>,
- falcon: &Falcon<'_, Gsp>,
- bar: Bar0<'_>,
- ) -> Result<()> {
+ pub(crate) fn run(&self, dev: &Device<device::Bound>, falcon: &Falcon<'_, Gsp>) -> Result<()> {
// Reset falcon, load the firmware, and run it.
falcon
.reset()
@@ -287,9 +284,8 @@ pub(crate) fn run(
.inspect_err(|e| dev_err!(dev, "Failed to load FWSEC firmware: {:?}\n", e))?;
// Configure DMA index for the bootloader to fetch the FWSEC firmware from system memory.
- bar.update(
- regs::NV_PFALCON_FBIF_TRANSCFG::of::<Gsp>()
- .try_at(usize::from_safe_cast(self.dmem_desc.ctx_dma))
+ falcon.pfalcon.update(
+ regs::NV_PFALCON_FBIF_TRANSCFG::try_at(usize::from_safe_cast(self.dmem_desc.ctx_dma))
.ok_or(EINVAL)?,
|v| {
v.with_target(FalconFbifTarget::CoherentSysmem)
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index e3c365cf4a68..45fc9a213275 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -63,12 +63,11 @@ impl FwsecUnloadFirmware {
fn run(
&self,
dev: &device::Device<device::Bound>,
- bar: Bar0<'_>,
gsp_falcon: &Falcon<'_, GspEngine>,
) -> Result {
match self {
Self::WithoutBl(fw) => fw.run(dev, gsp_falcon),
- Self::WithBl(fw) => fw.run(dev, gsp_falcon, bar),
+ Self::WithBl(fw) => fw.run(dev, gsp_falcon),
}
}
}
@@ -89,7 +88,7 @@ fn run(&self, ctx: &mut GspBootContext<'_, '_>) -> Result {
// Log errors but keep going if it fails.
let fwsec_sb_res = self
.fwsec_sb
- .run(dev, bar, ctx.gsp_falcon)
+ .run(dev, ctx.gsp_falcon)
.inspect_err(|e| dev_err!(dev, "FWSEC-SB failed to run: {:?}\n", e));
// Remove WPR2 region if set.
@@ -169,7 +168,7 @@ fn run_fwsec_frts(
if self.needs_fwsec_bootloader {
let fwsec_frts_bl = FwsecFirmwareWithBl::new(fwsec_frts, dev, chipset)?;
// Load and run the bootloader, which will load FWSEC-FRTS and run it.
- fwsec_frts_bl.run(dev, falcon, bar)?;
+ fwsec_frts_bl.run(dev, falcon)?;
} else {
// Load and run FWSEC-FRTS directly.
fwsec_frts.run(dev, falcon)?;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 1af073f3861f..6fdfdb0b3c60 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -4,8 +4,8 @@
use kernel::{
io::{
register,
- register::WithBase,
- Io, //
+ Io,
+ Mmio, //
},
prelude::*,
sizes::SizeConstants,
@@ -13,10 +13,7 @@
};
use crate::{
- driver::{
- Bar0,
- NovaRegisters, //
- },
+ driver::NovaRegisters,
falcon::{
DmaTrfCmdSize,
FalconCoreRev,
@@ -27,8 +24,8 @@
FalconMem,
FalconModSelAlgo,
FalconSecurityModel,
- PFalcon2Base,
- PFalconBase,
+ PFalcon2Registers,
+ PFalconRegisters,
PeregrineCoreSelect, //
},
gpu::{
@@ -202,32 +199,32 @@ pub(crate) fn usable_fb_size(self) -> u64 {
// PFALCON
register! {
- base: NovaRegisters;
+ base: PFalconRegisters;
- pub(crate) NV_PFALCON_FALCON_IRQSCLR(u32) @ PFalconBase + 0x00000004 {
+ pub(crate) NV_PFALCON_FALCON_IRQSCLR(u32) @ 0x00000004 {
6:6 swgen0 => bool;
4:4 halt => bool;
}
- pub(crate) NV_PFALCON_FALCON_MAILBOX0(u32) @ PFalconBase + 0x00000040 {
+ pub(crate) NV_PFALCON_FALCON_MAILBOX0(u32) @ 0x00000040 {
31:0 value => u32;
}
- pub(crate) NV_PFALCON_FALCON_MAILBOX1(u32) @ PFalconBase + 0x00000044 {
+ pub(crate) NV_PFALCON_FALCON_MAILBOX1(u32) @ 0x00000044 {
31:0 value => u32;
}
/// Used to store version information about the firmware running
/// on the Falcon processor.
- pub(crate) NV_PFALCON_FALCON_OS(u32) @ PFalconBase + 0x00000080 {
+ pub(crate) NV_PFALCON_FALCON_OS(u32) @ 0x00000080 {
31:0 value => u32;
}
- pub(crate) NV_PFALCON_FALCON_RM(u32) @ PFalconBase + 0x00000084 {
+ pub(crate) NV_PFALCON_FALCON_RM(u32) @ 0x00000084 {
31:0 value => u32;
}
- pub(crate) NV_PFALCON_FALCON_HWCFG2(u32) @ PFalconBase + 0x000000f4 {
+ pub(crate) NV_PFALCON_FALCON_HWCFG2(u32) @ 0x000000f4 {
/// Signal indicating that reset is completed (GA102+).
31:31 reset_ready => bool;
/// RISC-V branch privilege lockdown bit.
@@ -237,17 +234,17 @@ pub(crate) fn usable_fb_size(self) -> u64 {
10:10 riscv => bool;
}
- pub(crate) NV_PFALCON_FALCON_CPUCTL(u32) @ PFalconBase + 0x00000100 {
+ pub(crate) NV_PFALCON_FALCON_CPUCTL(u32) @ 0x00000100 {
6:6 alias_en => bool;
4:4 halted => bool;
1:1 startcpu => bool;
}
- pub(crate) NV_PFALCON_FALCON_BOOTVEC(u32) @ PFalconBase + 0x00000104 {
+ pub(crate) NV_PFALCON_FALCON_BOOTVEC(u32) @ 0x00000104 {
31:0 value => u32;
}
- pub(crate) NV_PFALCON_FALCON_DMACTL(u32) @ PFalconBase + 0x0000010c {
+ pub(crate) NV_PFALCON_FALCON_DMACTL(u32) @ 0x0000010c {
7:7 secure_stat => bool;
6:3 dmaq_num;
2:2 imem_scrubbing => bool;
@@ -255,15 +252,15 @@ pub(crate) fn usable_fb_size(self) -> u64 {
0:0 require_ctx => bool;
}
- pub(crate) NV_PFALCON_FALCON_DMATRFBASE(u32) @ PFalconBase + 0x00000110 {
+ pub(crate) NV_PFALCON_FALCON_DMATRFBASE(u32) @ 0x00000110 {
31:0 base => u32;
}
- pub(crate) NV_PFALCON_FALCON_DMATRFMOFFS(u32) @ PFalconBase + 0x00000114 {
+ pub(crate) NV_PFALCON_FALCON_DMATRFMOFFS(u32) @ 0x00000114 {
23:0 offs;
}
- pub(crate) NV_PFALCON_FALCON_DMATRFCMD(u32) @ PFalconBase + 0x00000118 {
+ pub(crate) NV_PFALCON_FALCON_DMATRFCMD(u32) @ 0x00000118 {
16:16 set_dmtag;
14:12 ctxdma;
10:8 size ?=> DmaTrfCmdSize;
@@ -274,15 +271,15 @@ pub(crate) fn usable_fb_size(self) -> u64 {
0:0 full => bool;
}
- pub(crate) NV_PFALCON_FALCON_DMATRFFBOFFS(u32) @ PFalconBase + 0x0000011c {
+ pub(crate) NV_PFALCON_FALCON_DMATRFFBOFFS(u32) @ 0x0000011c {
31:0 offs => u32;
}
- pub(crate) NV_PFALCON_FALCON_DMATRFBASE1(u32) @ PFalconBase + 0x00000128 {
+ pub(crate) NV_PFALCON_FALCON_DMATRFBASE1(u32) @ 0x00000128 {
8:0 base;
}
- pub(crate) NV_PFALCON_FALCON_HWCFG1(u32) @ PFalconBase + 0x0000012c {
+ pub(crate) NV_PFALCON_FALCON_HWCFG1(u32) @ 0x0000012c {
/// Core revision subversion.
7:6 core_rev_subversion => FalconCoreRevSubversion;
/// Security model.
@@ -291,12 +288,12 @@ pub(crate) fn usable_fb_size(self) -> u64 {
3:0 core_rev ?=> FalconCoreRev;
}
- pub(crate) NV_PFALCON_FALCON_CPUCTL_ALIAS(u32) @ PFalconBase + 0x00000130 {
+ pub(crate) NV_PFALCON_FALCON_CPUCTL_ALIAS(u32) @ 0x00000130 {
1:1 startcpu => bool;
}
/// IMEM access control register. Up to 4 ports are available for IMEM access.
- pub(crate) NV_PFALCON_FALCON_IMEMC(u32)[4, stride = 16] @ PFalconBase + 0x00000180 {
+ pub(crate) NV_PFALCON_FALCON_IMEMC(u32)[4, stride = 16] @ 0x00000180 {
/// Access secure IMEM.
28:28 secure => bool;
/// Auto-increment on write.
@@ -307,17 +304,17 @@ pub(crate) fn usable_fb_size(self) -> u64 {
/// IMEM data register. Reading/writing this register accesses IMEM at the address
/// specified by the corresponding IMEMC register.
- pub(crate) NV_PFALCON_FALCON_IMEMD(u32)[4, stride = 16] @ PFalconBase + 0x00000184 {
+ pub(crate) NV_PFALCON_FALCON_IMEMD(u32)[4, stride = 16] @ 0x00000184 {
31:0 data;
}
/// IMEM tag register. Used to set the tag for the current IMEM block.
- pub(crate) NV_PFALCON_FALCON_IMEMT(u32)[4, stride = 16] @ PFalconBase + 0x00000188 {
+ pub(crate) NV_PFALCON_FALCON_IMEMT(u32)[4, stride = 16] @ 0x00000188 {
15:0 tag;
}
/// DMEM access control register. Up to 8 ports are available for DMEM access.
- pub(crate) NV_PFALCON_FALCON_DMEMC(u32)[8, stride = 8] @ PFalconBase + 0x000001c0 {
+ pub(crate) NV_PFALCON_FALCON_DMEMC(u32)[8, stride = 8] @ 0x000001c0 {
/// Auto-increment on write.
24:24 aincw => bool;
/// DMEM block and word offset.
@@ -326,29 +323,29 @@ pub(crate) fn usable_fb_size(self) -> u64 {
/// DMEM data register. Reading/writing this register accesses DMEM at the address
/// specified by the corresponding DMEMC register.
- pub(crate) NV_PFALCON_FALCON_DMEMD(u32)[8, stride = 8] @ PFalconBase + 0x000001c4 {
+ pub(crate) NV_PFALCON_FALCON_DMEMD(u32)[8, stride = 8] @ 0x000001c4 {
31:0 data;
}
/// Actually known as `NV_PSEC_FALCON_ENGINE` and `NV_PGSP_FALCON_ENGINE` depending on the
/// falcon instance.
- pub(crate) NV_PFALCON_FALCON_ENGINE(u32) @ PFalconBase + 0x000003c0 {
+ pub(crate) NV_PFALCON_FALCON_ENGINE(u32) @ 0x000003c0 {
0:0 reset => bool;
}
- pub(crate) NV_PFALCON_FBIF_TRANSCFG(u32)[8] @ PFalconBase + 0x00000600 {
+ pub(crate) NV_PFALCON_FBIF_TRANSCFG(u32)[8] @ 0x00000600 {
2:2 mem_type => FalconFbifMemType;
1:0 target ?=> FalconFbifTarget;
}
- pub(crate) NV_PFALCON_FBIF_CTL(u32) @ PFalconBase + 0x00000624 {
+ pub(crate) NV_PFALCON_FBIF_CTL(u32) @ 0x00000624 {
7:7 allow_phys_no_ctx => bool;
}
// Falcon EMEM PIO registers (used by FSP on Hopper/Blackwell).
// These provide the falcon external memory communication interface.
- pub(crate) NV_PFALCON_FALCON_EMEMC(u32) @ PFalconBase + 0x00000ac0 {
+ pub(crate) NV_PFALCON_FALCON_EMEMC(u32) @ 0x00000ac0 {
/// EMEM byte offset (4-byte aligned) within the block.
7:2 offs;
/// EMEM block to access.
@@ -359,7 +356,7 @@ pub(crate) fn usable_fb_size(self) -> u64 {
25:25 aincr => bool;
}
- pub(crate) NV_PFALCON_FALCON_EMEMD(u32) @ PFalconBase + 0x00000ac4 {
+ pub(crate) NV_PFALCON_FALCON_EMEMD(u32) @ 0x00000ac4 {
31:0 data => u32;
}
}
@@ -385,13 +382,13 @@ pub(crate) fn with_falcon_mem(self, mem: FalconMem) -> Self {
impl NV_PFALCON_FALCON_ENGINE {
/// Resets the falcon
- pub(crate) fn reset_engine<E: FalconEngine>(bar: Bar0<'_>) {
- bar.update(Self::of::<E>(), |r| r.with_reset(true));
+ pub(crate) fn reset_engine<E: FalconEngine>(pfalcon: Mmio<'_, PFalconRegisters>) {
+ pfalcon.update(NV_PFALCON_FALCON_ENGINE, |r| r.with_reset(true));
// TIMEOUT: falcon engine should not take more than 10us to reset.
time::delay::fsleep(time::Delta::from_micros(10));
- bar.update(Self::of::<E>(), |r| r.with_reset(false));
+ pfalcon.update(NV_PFALCON_FALCON_ENGINE, |r| r.with_reset(false));
}
}
@@ -405,23 +402,23 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
/* PFALCON2 */
register! {
- base: NovaRegisters;
+ base: PFalcon2Registers;
- pub(crate) NV_PFALCON2_FALCON_MOD_SEL(u32) @ PFalcon2Base + 0x00000180 {
+ pub(crate) NV_PFALCON2_FALCON_MOD_SEL(u32) @ 0x00000180 {
7:0 algo ?=> FalconModSelAlgo;
}
- pub(crate) NV_PFALCON2_FALCON_BROM_CURR_UCODE_ID(u32) @ PFalcon2Base + 0x00000198 {
+ pub(crate) NV_PFALCON2_FALCON_BROM_CURR_UCODE_ID(u32) @ 0x00000198 {
7:0 ucode_id => u8;
}
- pub(crate) NV_PFALCON2_FALCON_BROM_ENGIDMASK(u32) @ PFalcon2Base + 0x0000019c {
+ pub(crate) NV_PFALCON2_FALCON_BROM_ENGIDMASK(u32) @ 0x0000019c {
31:0 value => u32;
}
/// OpenRM defines this as a register array, but doesn't specify its size and only uses its
/// first element. Be conservative until we know the actual size or need to use more registers.
- pub(crate) NV_PFALCON2_FALCON_BROM_PARAADDR(u32)[1] @ PFalcon2Base + 0x00000210 {
+ pub(crate) NV_PFALCON2_FALCON_BROM_PARAADDR(u32)[1] @ 0x00000210 {
31:0 value => u32;
}
}
@@ -429,23 +426,23 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
// PRISCV
register! {
- base: NovaRegisters;
+ base: PFalcon2Registers;
/// RISC-V status register for debug (Turing and GA100 only).
/// Reflects current RISC-V core status.
- pub(crate) NV_PRISCV_RISCV_CORE_SWITCH_RISCV_STATUS(u32) @ PFalcon2Base + 0x00000240 {
+ pub(crate) NV_PRISCV_RISCV_CORE_SWITCH_RISCV_STATUS(u32) @ 0x00000240 {
/// RISC-V core active/inactive status.
0:0 active_stat => bool;
}
/// GA102 and later.
- pub(crate) NV_PRISCV_RISCV_CPUCTL(u32) @ PFalcon2Base + 0x00000388 {
+ pub(crate) NV_PRISCV_RISCV_CPUCTL(u32) @ 0x00000388 {
7:7 active_stat => bool;
4:4 halted => bool;
}
/// GA102 and later.
- pub(crate) NV_PRISCV_RISCV_BCR_CTRL(u32) @ PFalcon2Base + 0x00000668 {
+ pub(crate) NV_PRISCV_RISCV_BCR_CTRL(u32) @ 0x00000668 {
8:8 br_fetch => bool;
4:4 core_select => PeregrineCoreSelect;
0:0 valid => bool;
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 13/16] gpu: nova-core: convert hshub0 from relative register to projection
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (11 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:48 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 14/16] rust: io: register: remove relative registers Gary Guo
` (2 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Similar to the PFALCON and PFALCON2 conversion, the hshub0 relative access
can also be achieved cleanly with projection and a new base.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
drivers/gpu/nova-core/fb/hal/gb100.rs | 59 +++++++++++++++++------------------
drivers/gpu/nova-core/fb/regs.rs | 19 ++++++-----
2 files changed, 40 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/nova-core/fb/hal/gb100.rs b/drivers/gpu/nova-core/fb/hal/gb100.rs
index d9e4d62ae632..9fa094939600 100644
--- a/drivers/gpu/nova-core/fb/hal/gb100.rs
+++ b/drivers/gpu/nova-core/fb/hal/gb100.rs
@@ -5,11 +5,10 @@
use kernel::{
io::{
- register::{
- RegisterBase,
- WithBase, //
- },
- Io, //
+ io_project,
+ register,
+ Io,
+ Mmio, //
},
num::Bounded,
prelude::*,
@@ -21,7 +20,10 @@
};
use crate::{
- driver::Bar0,
+ driver::{
+ Bar0,
+ NovaRegisters, //
+ },
fb::{
hal::FbHal,
regs, //
@@ -31,17 +33,26 @@
struct Gb100;
-impl RegisterBase<regs::Hshub0Base> for Gb100 {
- const BASE: usize = 0x0087_0000;
+register! {
+ base: NovaRegisters;
+
+ HSHUB0: regs::Hshub0Registers @ 0x0087_0000;
+}
+
+#[inline]
+fn hshub0(bar: Bar0<'_>) -> Mmio<'_, regs::Hshub0Registers> {
+ io_project!(bar, build: HSHUB0)
}
-fn read_sysmem_flush_page_gb100(bar: Bar0<'_>) -> u64 {
+fn read_sysmem_flush_page_gb100(hshub0: Mmio<'_, regs::Hshub0Registers>) -> u64 {
let lo = u64::from(
- bar.read(regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::of::<Gb100>())
+ hshub0
+ .read(regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_LO)
.adr(),
);
let hi = u64::from(
- bar.read(regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::of::<Gb100>())
+ hshub0
+ .read(regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_HI)
.adr(),
);
@@ -52,7 +63,7 @@ fn read_sysmem_flush_page_gb100(bar: Bar0<'_>) -> u64 {
///
/// Both the primary and EG (egress) register pairs must be programmed to the same address,
/// as required by hardware.
-fn write_sysmem_flush_page_gb100(bar: Bar0<'_>, addr: Bounded<u64, 52>) {
+fn write_sysmem_flush_page_gb100(hshub0: Mmio<'_, regs::Hshub0Registers>, addr: Bounded<u64, 52>) {
// CAST: lower 32 bits. Hardware ignores bits 7:0.
let addr_lo = *addr as u32;
let addr_hi = addr.shr::<32, 20>().cast::<u32>();
@@ -60,24 +71,12 @@ fn write_sysmem_flush_page_gb100(bar: Bar0<'_>, addr: Bounded<u64, 52>) {
// Write HI first. The hardware will trigger the flush on the LO write.
// Primary HSHUB pair.
- bar.write(
- regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::of::<Gb100>(),
- regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::zeroed().with_adr(addr_hi),
- );
- bar.write(
- regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::of::<Gb100>(),
- regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::zeroed().with_adr(addr_lo),
- );
+ hshub0.write_reg(regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::zeroed().with_adr(addr_hi));
+ hshub0.write_reg(regs::NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::zeroed().with_adr(addr_lo));
// EG (egress) pair -- must match the primary pair.
- bar.write(
- regs::NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_HI::of::<Gb100>(),
- regs::NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_HI::zeroed().with_adr(addr_hi),
- );
- bar.write(
- regs::NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_LO::of::<Gb100>(),
- regs::NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_LO::zeroed().with_adr(addr_lo),
- );
+ hshub0.write_reg(regs::NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_HI::zeroed().with_adr(addr_hi));
+ hshub0.write_reg(regs::NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_LO::zeroed().with_adr(addr_lo));
}
// This PMU reservation size is r570-specific.
@@ -88,13 +87,13 @@ pub(super) const fn pmu_reserved_size_gb100() -> u32 {
impl FbHal for Gb100 {
fn read_sysmem_flush_page(&self, bar: Bar0<'_>) -> u64 {
- read_sysmem_flush_page_gb100(bar)
+ read_sysmem_flush_page_gb100(hshub0(bar))
}
fn write_sysmem_flush_page(&self, bar: Bar0<'_>, addr: u64) -> Result {
let addr = Bounded::<u64, 52>::try_new(addr).ok_or(EINVAL)?;
- write_sysmem_flush_page_gb100(bar, addr);
+ write_sysmem_flush_page_gb100(hshub0(bar), addr);
Ok(())
}
diff --git a/drivers/gpu/nova-core/fb/regs.rs b/drivers/gpu/nova-core/fb/regs.rs
index c27582e376e2..584488a3e012 100644
--- a/drivers/gpu/nova-core/fb/regs.rs
+++ b/drivers/gpu/nova-core/fb/regs.rs
@@ -2,7 +2,8 @@
use kernel::{
io::register,
- sizes::SizeConstants, //
+ prelude::*,
+ sizes::{SizeConstants, SZ_4K}, //
};
use crate::driver::NovaRegisters;
@@ -65,31 +66,33 @@ pub(super) fn vga_workspace_addr(self) -> Option<u64> {
}
}
-/// Base of the GB10x HSHUB0 register window (`NV_HSHUB0_PRIV_BASE` in Open RM).
+/// The GB10x HSHUB0 register window (Base defined as `NV_HSHUB0_PRIV_BASE` in Open RM).
///
/// The base is provided by the GB10x framebuffer HAL.
-pub(super) struct Hshub0Base(());
+#[repr(align(4))]
+#[derive(FromBytes, IntoBytes)]
+pub(super) struct Hshub0Registers([u8; SZ_4K]);
register! {
- base: NovaRegisters;
+ base: Hshub0Registers;
// GB10x sysmem flush registers, relative to the HSHUB0 base. GB10x routes sysmembar
// through a primary and an EG (egress) pair that must both be programmed to the same
// address. Hardware ignores bits 7:0 of each LO register. The boot path uses a fixed
// HSHUB0 base, so the multiple runtime-discovered HSHUB bases are not needed here.
- pub(super) NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Hshub0Base + 0x00000e50 {
+ pub(super) NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ 0x00000e50 {
31:0 adr => u32;
}
- pub(super) NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Hshub0Base + 0x00000e54 {
+ pub(super) NV_PFB_HSHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ 0x00000e54 {
19:0 adr;
}
- pub(super) NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Hshub0Base + 0x000006c0 {
+ pub(super) NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ 0x000006c0 {
31:0 adr => u32;
}
- pub(super) NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Hshub0Base + 0x000006c4 {
+ pub(super) NV_PFB_HSHUB_EG_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ 0x000006c4 {
19:0 adr;
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 14/16] rust: io: register: remove relative registers
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (12 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 13/16] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:51 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 15/16] rust: io: register: remove `Register` trait and cleanup macro Gary Guo
2026-08-05 16:35 ` [PATCH v2 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Relative registers can be better served by projection to subregion instead
of ad-hoc handling in register macro. Projection composes better (e.g. it
natively allows relative registers of relative registers without needing
additional support).
Remove relative register support, and update the documentation to
demonstrate how projection and subregions can be used to achieve this
instead.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io/register.rs | 474 +++++----------------------------------------
1 file changed, 52 insertions(+), 422 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 49a61da106c7..295b06dd53a7 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -8,7 +8,7 @@
//!
//! Note: most of the items in this module are public so they can be referenced by the macro, but
//! most are not to be used directly by users. Outside of the `register!` macro itself, the only
-//! items you might want to import from this module are [`WithBase`] and [`Array`].
+//! items you might want to import from this module is [`Array`].
//!
//! # Simple example
//!
@@ -199,76 +199,6 @@ fn offset(self) -> usize {
}
}
-/// Trait providing a base address to be added to the offset of a relative register to obtain
-/// its actual offset.
-///
-/// The `T` generic argument is used to distinguish which base to use, in case a type provides
-/// several bases. It is given to the `register!` macro to restrict the use of the register to
-/// implementors of this particular variant.
-pub trait RegisterBase<T> {
- /// Base address to which register offsets are added.
- const BASE: usize;
-}
-
-/// Trait implemented by all registers that are relative to a base.
-pub trait WithBase {
- /// Family of bases applicable to this register.
- type BaseFamily;
-
- /// Returns the absolute location of this type when using `B` as its base.
- #[inline(always)]
- fn of<B: RegisterBase<Self::BaseFamily>>() -> RelativeRegisterLoc<Self, B>
- where
- Self: Register,
- {
- RelativeRegisterLoc::new()
- }
-}
-
-/// Trait implemented by relative registers.
-pub trait RelativeRegister: Register + WithBase {}
-
-/// Location of a relative register.
-///
-/// This can either be an immediately accessible regular [`RelativeRegister`], or a
-/// [`RelativeRegisterArray`] that needs one additional resolution through
-/// [`RelativeRegisterLoc::at`].
-pub struct RelativeRegisterLoc<T: WithBase, B: ?Sized>(PhantomData<T>, PhantomData<B>);
-
-impl<T, B> RelativeRegisterLoc<T, B>
-where
- T: Register + WithBase,
- B: RegisterBase<T::BaseFamily> + ?Sized,
-{
- /// Returns the location of a relative register or register array.
- #[inline(always)]
- // We do not implement `Default` so we can be const.
- #[expect(clippy::new_without_default)]
- pub const fn new() -> Self {
- Self(PhantomData, PhantomData)
- }
-
- // Returns the absolute offset of the relative register using base `B`.
- //
- // This is implemented as a private const method so it can be reused by the [`IoLoc`]
- // implementations of both [`RelativeRegisterLoc`] and [`RelativeRegisterArrayLoc`].
- #[inline]
- const fn offset(self) -> usize {
- B::BASE + T::OFFSET
- }
-}
-
-impl<SuperBase: ?Sized, T, B> IoLoc<SuperBase, T> for RelativeRegisterLoc<T, B>
-where
- T: RelativeRegister<Base = SuperBase>,
- B: RegisterBase<T::BaseFamily> + ?Sized,
-{
- #[inline(always)]
- fn offset(self) -> usize {
- RelativeRegisterLoc::offset(self)
- }
-}
-
/// Trait implemented by arrays of registers.
pub trait RegisterArray: Register {
/// Number of elements in the registers array.
@@ -331,71 +261,6 @@ fn try_at(idx: usize) -> Option<RegisterArrayLoc<Self>>
}
}
-/// Trait implemented by arrays of relative registers.
-pub trait RelativeRegisterArray: RegisterArray + WithBase {}
-
-/// Location of a relative array register.
-pub struct RelativeRegisterArrayLoc<
- T: RelativeRegisterArray,
- B: RegisterBase<T::BaseFamily> + ?Sized,
->(RelativeRegisterLoc<T, B>, usize);
-
-impl<T, B> RelativeRegisterArrayLoc<T, B>
-where
- T: RelativeRegisterArray,
- B: RegisterBase<T::BaseFamily> + ?Sized,
-{
- /// Returns the location of register `T` from the base `B` at index `idx`, with build-time
- /// validation.
- #[inline(always)]
- pub fn new(idx: usize) -> Self {
- build_assert!(idx < T::SIZE);
-
- Self(RelativeRegisterLoc::new(), idx)
- }
-
- /// Attempts to return the location of register `T` from the base `B` at index `idx`, with
- /// runtime validation.
- #[inline(always)]
- pub fn try_new(idx: usize) -> Option<Self> {
- if idx < T::SIZE {
- Some(Self(RelativeRegisterLoc::new(), idx))
- } else {
- None
- }
- }
-}
-
-/// Methods exclusive to [`RelativeRegisterLoc`]s created with a [`RelativeRegisterArray`].
-impl<T, B> RelativeRegisterLoc<T, B>
-where
- T: RelativeRegisterArray,
- B: RegisterBase<T::BaseFamily> + ?Sized,
-{
- /// Returns the location of the register at position `idx`, with build-time validation.
- #[inline(always)]
- pub fn at(self, idx: usize) -> RelativeRegisterArrayLoc<T, B> {
- RelativeRegisterArrayLoc::new(idx)
- }
-
- /// Returns the location of the register at position `idx`, with runtime validation.
- #[inline(always)]
- pub fn try_at(self, idx: usize) -> Option<RelativeRegisterArrayLoc<T, B>> {
- RelativeRegisterArrayLoc::try_new(idx)
- }
-}
-
-impl<SuperBase: ?Sized, T, B> IoLoc<SuperBase, T> for RelativeRegisterArrayLoc<T, B>
-where
- T: RelativeRegisterArray<Base = SuperBase>,
- B: RegisterBase<T::BaseFamily> + ?Sized,
-{
- #[inline(always)]
- fn offset(self) -> usize {
- self.0.offset() + self.1 * T::STRIDE
- }
-}
-
/// Trait implemented by items that contain both a register value and the absolute I/O location at
/// which to write it.
///
@@ -430,8 +295,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// This documentation focuses on how to declare registers. See the [module-level
/// documentation](mod@kernel::io::register) for examples of how to access them.
///
-/// There are 4 possible kinds of registers: fixed offset registers, relative registers, arrays of
-/// registers, and relative arrays of registers.
+/// Registers can either be fixed offset registers or arrays of registers.
///
/// ## Fixed offset registers
///
@@ -529,122 +393,6 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// }
/// ```
///
-/// ## Relative registers
-///
-/// Relative registers can be instantiated several times at a relative offset of a group of bases.
-/// For instance, imagine the following I/O space:
-///
-/// ```text
-/// +-----------------------------+
-/// | ... |
-/// | |
-/// 0x100--->+------------CPU0-------------+
-/// | |
-/// 0x110--->+-----------------------------+
-/// | CPU_CTL |
-/// +-----------------------------+
-/// | ... |
-/// | |
-/// | |
-/// 0x200--->+------------CPU1-------------+
-/// | |
-/// 0x210--->+-----------------------------+
-/// | CPU_CTL |
-/// +-----------------------------+
-/// | ... |
-/// +-----------------------------+
-/// ```
-///
-/// `CPU0` and `CPU1` both have a `CPU_CTL` register that starts at offset `0x10` of their I/O
-/// space segment. Since both instances of `CPU_CTL` share the same layout, we don't want to define
-/// them twice and would prefer a way to select which one to use from a single definition.
-///
-/// This can be done using the `Base + Offset` syntax when specifying the register's address:
-///
-/// ```ignore
-/// register! {
-/// ...
-/// pub RELATIVE_REG(u32) @ Base + 0x80 {
-/// ...
-/// }
-/// }
-/// ```
-///
-/// This creates a register with an offset of `0x80` from a given base.
-///
-/// `Base` is an arbitrary type (typically a ZST) to be used as a generic parameter of the
-/// [`RegisterBase`] trait to provide the base as a constant, i.e. each type providing a base for
-/// this register needs to implement `RegisterBase<Base>`.
-///
-/// The location of relative registers can be built using the [`WithBase::of`] method to specify
-/// its base. All relative registers implement [`WithBase`].
-///
-/// Here is the above layout translated into code:
-///
-/// ```no_run
-/// use kernel::{
-/// io::{
-/// register,
-/// register::{
-/// RegisterBase,
-/// WithBase,
-/// },
-/// Io,
-/// Region,
-/// },
-/// };
-/// # use kernel::io::Mmio;
-///
-/// // Type used to identify the base.
-/// pub struct CpuCtlBase;
-///
-/// // ZST describing `CPU0`.
-/// struct Cpu0;
-/// impl RegisterBase<CpuCtlBase> for Cpu0 {
-/// const BASE: usize = 0x100;
-/// }
-///
-/// // ZST describing `CPU1`.
-/// struct Cpu1;
-/// impl RegisterBase<CpuCtlBase> for Cpu1 {
-/// const BASE: usize = 0x200;
-/// }
-///
-/// // This makes `CPU_CTL` accessible from all implementors of `RegisterBase<CpuCtlBase>`.
-/// register! {
-/// base: Region<0x1000>;
-///
-/// /// CPU core control.
-/// pub CPU_CTL(u32) @ CpuCtlBase + 0x10 {
-/// 0:0 start;
-/// }
-/// }
-///
-/// # fn test(io: Mmio<'_, Region<0x1000>>) {
-/// // Read the status of `Cpu0`.
-/// let cpu0_started = io.read(CPU_CTL::of::<Cpu0>());
-///
-/// // Stop `Cpu0`.
-/// io.write(WithBase::of::<Cpu0>(), CPU_CTL::zeroed());
-/// # }
-///
-/// // Aliases can also be defined for relative register.
-/// register! {
-/// base: Region<0x1000>;
-///
-/// /// Alias to CPU core control.
-/// pub CPU_CTL_ALIAS(u32) => CpuCtlBase + CPU_CTL {
-/// /// Start the aliased CPU core.
-/// 1:1 alias_start;
-/// }
-/// }
-///
-/// # fn test2(io: Mmio<'_, Region<0x1000>>) {
-/// // Start the aliased `CPU0`, leaving its other fields untouched.
-/// io.update(CPU_CTL_ALIAS::of::<Cpu0>(), |r| r.with_alias_start(true));
-/// # }
-/// ```
-///
/// ## Arrays of registers
///
/// Some I/O areas contain consecutive registers that share the same field layout. These areas can
@@ -741,115 +489,83 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// # }
/// ```
///
-/// ## Relative arrays of registers
+/// ## Relative registers
///
-/// Combining the two features described in the sections above, arrays of registers accessible from
-/// a base can also be defined:
+/// There are cases where a register region is subdivided into small subregions, and you may wish to
+/// have your register definition be relative to these subregions. This may be needed, for example,
+/// if these subregions are instantiated several times, or you just want it for encapsulation
+/// purpose.
///
-/// ```ignore
-/// register! {
-/// ...
-/// pub RELATIVE_REGISTER_ARRAY(u8)[10, stride = 4] @ Base + 0x100 {
-/// ...
-/// }
-/// }
+/// For instance, imagine the following I/O space:
+///
+/// ```text
+/// +-----------------------------+
+/// | ... |
+/// | |
+/// 0x100--->+------------CPU0-------------+
+/// | |
+/// 0x110--->+-----------------------------+
+/// | CPU_CTL |
+/// +-----------------------------+
+/// | ... |
+/// | |
+/// | |
+/// 0x200--->+------------CPU1-------------+
+/// | |
+/// 0x210--->+-----------------------------+
+/// | CPU_CTL |
+/// +-----------------------------+
+/// | ... |
+/// +-----------------------------+
/// ```
///
-/// Like relative registers, they implement the [`WithBase`] trait. However the return value of
-/// [`WithBase::of`] cannot be used directly as a location and must be further specified using the
-/// [`at`](RelativeRegisterLoc::at) method.
+/// `CPU0` and `CPU1` both have a `CPU_CTL` register that starts at offset `0x10` of their I/O
+/// space segment. Since both instances of `CPU_CTL` share the same layout, we don't want to define
+/// them twice and would prefer a way to select which one to use from a single definition.
+///
+/// This can be done define a new type for the subregion, and then define registers that use the new
+/// type as the base:
///
/// ```no_run
/// use kernel::{
/// io::{
+/// io_project,
/// register,
-/// register::{
-/// RegisterBase,
-/// WithBase,
-/// },
/// Io,
/// Region,
/// },
/// };
/// # use kernel::io::Mmio;
-/// # fn get_scratch_idx() -> usize {
-/// # 0x15
-/// # }
-///
-/// // Type used as parameter of `RegisterBase` to specify the base.
-/// pub struct CpuCtlBase;
-///
-/// // ZST describing `CPU0`.
-/// struct Cpu0;
-/// impl RegisterBase<CpuCtlBase> for Cpu0 {
-/// const BASE: usize = 0x100;
-/// }
///
-/// // ZST describing `CPU1`.
-/// struct Cpu1;
-/// impl RegisterBase<CpuCtlBase> for Cpu1 {
-/// const BASE: usize = 0x200;
-/// }
+/// // Subregion type. Make sure it has adequate size and alignment.
+/// #[repr(align(4))]
+/// #[derive(FromBytes, IntoBytes)]
+/// pub struct CpuCtl([u8; 0x100]);
///
-/// // 64 per-cpu scratch registers, arranged as a contiguous array.
/// register! {
/// base: Region<0x1000>;
///
-/// /// Per-CPU scratch registers.
-/// pub CPU_SCRATCH(u32)[64] @ CpuCtlBase + 0x00000080 {
-/// 31:0 value;
-/// }
+/// // Subregions can just be defined like normal registers.
+/// CPU0: CpuCtl @ 0x100;
+/// CPU1: CpuCtl @ 0x200;
/// }
///
-/// # fn test(io: Mmio<'_, Region<0x1000>>) -> Result<(), Error> {
-/// // Read scratch register 0 of CPU0.
-/// let scratch = io.read(CPU_SCRATCH::of::<Cpu0>().at(0));
-///
-/// // Write the retrieved value into scratch register 15 of CPU1.
-/// io.write(WithBase::of::<Cpu1>().at(15), scratch);
-///
-/// // This won't build.
-/// // let cpu0_scratch_128 = io.read(CPU_SCRATCH::of::<Cpu0>().at(128)).value();
-///
-/// // Runtime-obtained array index.
-/// let scratch_idx = get_scratch_idx();
-/// // Access on a runtime index returns an error if it is out-of-bounds.
-/// let cpu0_scratch = io.read(
-/// CPU_SCRATCH::of::<Cpu0>().try_at(scratch_idx).ok_or(EINVAL)?
-/// ).value();
-/// # Ok(())
-/// # }
-///
-/// // Alias to `SCRATCH[8]` used to convey the firmware exit code.
+/// // Then you can define new registers on the subregion.
/// register! {
-/// base: Region<0x1000>;
+/// base: CpuCtl;
///
-/// /// Per-CPU firmware exit status code.
-/// pub CPU_FIRMWARE_STATUS(u32) => CpuCtlBase + CPU_SCRATCH[8] {
-/// 7:0 status;
+/// /// CPU core control.
+/// pub CPU_CTL(u32) @ 0x10 {
+/// 0:0 start;
/// }
/// }
///
-/// // Non-contiguous relative register arrays can be defined by adding a stride parameter.
-/// // Here, each of the 16 registers of the array is separated by 8 bytes, meaning that the
-/// // registers of the two declarations below are interleaved.
-/// register! {
-/// base: Region<0x1000>;
-///
-/// /// Scratch registers bank 0.
-/// pub CPU_SCRATCH_INTERLEAVED_0(u32)[16, stride = 8] @ CpuCtlBase + 0x00000d00 {
-/// 31:0 value;
-/// }
-///
-/// /// Scratch registers bank 1.
-/// pub CPU_SCRATCH_INTERLEAVED_1(u32)[16, stride = 8] @ CpuCtlBase + 0x00000d04 {
-/// 31:0 value;
-/// }
-/// }
+/// # fn test(io: Mmio<'_, Region<0x1000>>) {
+/// // Read the status of `Cpu0`.
+/// let cpu0_started = io_project!(io, build: CPU0).read(CPU_CTL);
///
-/// # fn test2(io: Mmio<'_, Region<0x1000>>) -> Result<(), Error> {
-/// let cpu0_status = io.read(CPU_FIRMWARE_STATUS::of::<Cpu0>()).status();
-/// # Ok(())
+/// // Stop `Cpu0`.
+/// io_project!(io, build: CPU0).write_reg(CPU_CTL::zeroed());
/// # }
/// ```
#[macro_export]
@@ -893,32 +609,6 @@ macro_rules! register {
$crate::register!(base: $reg_base; $($rest)*);
};
- // Creates a register at a relative offset from a base address provider.
- (
- base: $reg_base:ty;
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) @ $base:ident + $offset:literal
- { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $reg_base; $name @ $offset);
- $crate::register!(@io_relative $vis $name @ $base);
- $crate::register!(base: $reg_base; $($rest)*);
- };
-
- // Creates an alias register of relative offset register `alias` with its own fields.
- (
- base: $reg_base:ty;
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $base:ident + $alias:ident
- { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $reg_base; $name @ $crate::register!(@offset => $alias));
- $crate::register!(@io_relative $vis $name @ $base);
- $crate::register!(base: $reg_base; $($rest)*);
- };
-
// Creates an array of registers at a fixed offset of the MMIO space.
(
base: $reg_base:ty;
@@ -934,40 +624,6 @@ macro_rules! register {
$crate::register!(base: $reg_base; $($rest)*);
};
- // Creates an array of registers at a relative offset from a base address provider.
- (
- base: $reg_base:ty;
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
- [ $size:expr $(, stride = $stride:expr)? ]
- @ $base:ident + $offset:literal { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $reg_base; $name @ $offset);
- $crate::register!(@io_relative_array $vis $name
- [ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ] @ $base + $offset
- );
- $crate::register!(base: $reg_base; $($rest)*);
- };
-
- // Creates an alias of register `idx` of relative array of registers `alias` with its own
- // fields.
- (
- base: $reg_base:ty;
- $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
- => $base:ident + $alias:ident [ $idx:expr ] { $($fields:tt)* }
- $($rest:tt)*
- ) => {
- $crate::build_assert::static_assert!(
- $idx < <$alias as $crate::io::register::RegisterArray>::SIZE
- );
-
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $reg_base; $name @ $crate::register!(@offset => $alias [$idx]));
- $crate::register!(@io_relative $vis $name @ $base);
- $crate::register!(base: $reg_base; $($rest)*);
- };
-
// All the rules below are private helpers.
// Generates the bitfield for the register.
@@ -1020,15 +676,6 @@ impl $crate::io::register::FixedRegister for $name {}
$crate::io::register::FixedRegisterLoc::<$name>::new();
};
- // Implementations of relative registers.
- (@io_relative $vis:vis $name:ident @ $base:ident) => {
- impl $crate::io::register::WithBase for $name {
- type BaseFamily = $base;
- }
-
- impl $crate::io::register::RelativeRegister for $name {}
- };
-
// Implementations of register arrays.
(@io_array $vis:vis $name:ident [ $size:expr, stride = $stride:expr ]) => {
impl $crate::io::register::Array for $name {}
@@ -1038,21 +685,4 @@ impl $crate::io::register::RegisterArray for $name {
const STRIDE: usize = $stride;
}
};
-
- // Implementations of relative array registers.
- (
- @io_relative_array $vis:vis $name:ident [ $size:expr, stride = $stride:expr ]
- @ $base:ident + $offset:literal
- ) => {
- impl $crate::io::register::WithBase for $name {
- type BaseFamily = $base;
- }
-
- impl $crate::io::register::RegisterArray for $name {
- const SIZE: usize = $size;
- const STRIDE: usize = $stride;
- }
-
- impl $crate::io::register::RelativeRegisterArray for $name {}
- };
}
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 15/16] rust: io: register: remove `Register` trait and cleanup macro
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (13 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 14/16] rust: io: register: remove relative registers Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:48 ` sashiko-bot
2026-08-05 16:35 ` [PATCH v2 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
With the removal of relative registers, there are only two type of
registers left, fixed register and register arrays. There is not much
benefit in having a common super trait for them anymore, thus remove it,
and cleanup the macro rules associated with it.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io/register.rs | 102 +++++++++++++++++++--------------------------
1 file changed, 43 insertions(+), 59 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 295b06dd53a7..fe0e6763a600 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -121,8 +121,8 @@
io::IoLoc, //
};
-/// Trait implemented by all registers.
-pub trait Register: Sized {
+/// Trait implemented by registers with a fixed offset.
+pub trait FixedRegister: Sized {
/// Base type for this register.
type Base: ?Sized;
@@ -132,9 +132,6 @@ pub trait Register: Sized {
const OFFSET: usize;
}
-/// Trait implemented by registers with a fixed offset.
-pub trait FixedRegister: Register {}
-
/// Allows `()` to be used as the `location` parameter of [`Io::write`](super::Io::write) when
/// passing a [`FixedRegister`] value.
impl<Base: ?Sized, T> IoLoc<Base, T> for ()
@@ -200,7 +197,14 @@ fn offset(self) -> usize {
}
/// Trait implemented by arrays of registers.
-pub trait RegisterArray: Register {
+pub trait RegisterArray: Sized {
+ /// Base type for this register.
+ type Base: ?Sized;
+
+ /// Start offset of the register.
+ ///
+ /// The interpretation of this offset depends on the type of the register.
+ const OFFSET: usize;
/// Number of elements in the registers array.
const SIZE: usize;
/// Number of bytes between the start of elements in the registers array.
@@ -266,8 +270,8 @@ fn try_at(idx: usize) -> Option<RegisterArrayLoc<Self>>
///
/// Implementors can be used with [`Io::write_reg`](super::Io::write_reg).
pub trait LocatedRegister<Base: ?Sized> {
- /// Register value to write.
- type Value: Register;
+ /// Value to write.
+ type Value;
/// Full location information at which to write the value.
type Location: IoLoc<Base, Self::Value>;
@@ -601,11 +605,22 @@ macro_rules! register {
{ $($fields:tt)* }
$($rest:tt)*
) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $reg_base; $name
- @ $crate::register!(@offset $(@ $offset)? $(=> $alias $([$alias_idx])?)?)
+ $crate::bitfield!(
+ #[allow(non_camel_case_types)]
+ $(#[$attr])* $vis struct $name($storage) { $($fields)* }
);
- $crate::register!(@io_fixed $(#[$attr])* $vis $name);
+
+ impl $crate::io::register::FixedRegister for $name {
+ type Base = $reg_base;
+
+ const OFFSET: usize =
+ $crate::register!(@offset $(@ $offset)? $(=> $alias $([$alias_idx])?)?);
+ }
+
+ $(#[$attr])*
+ $vis const $name: $crate::io::register::FixedRegisterLoc<$name> =
+ $crate::io::register::FixedRegisterLoc::<$name>::new();
+
$crate::register!(base: $reg_base; $($rest)*);
};
@@ -615,39 +630,36 @@ macro_rules! register {
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
[ $size:expr $(, stride = $stride:expr)? ] @ $offset:literal { $($fields:tt)* }
$($rest:tt)*
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $reg_base; $name @ $offset);
- $crate::register!(@io_array $vis $name
- [ $size, stride = $crate::register!(@stride $storage $(, $stride)?) ]
- );
- $crate::register!(base: $reg_base; $($rest)*);
- };
-
- // All the rules below are private helpers.
-
- // Generates the bitfield for the register.
- //
- // `#[allow(non_camel_case_types)]` is added since register names typically use
- // `SCREAMING_CASE`.
- (
- @bitfield $(#[$attr:meta])* $vis:vis struct $name:ident($storage:ty) { $($fields:tt)* }
) => {
$crate::bitfield!(
#[allow(non_camel_case_types)]
$(#[$attr])* $vis struct $name($storage) { $($fields)* }
);
+
+ impl $crate::io::register::Array for $name {}
+
+ impl $crate::io::register::RegisterArray for $name {
+ type Base = $reg_base;
+
+ const OFFSET: usize = $offset;
+ const SIZE: usize = $size;
+ const STRIDE: usize = $crate::register!(@stride $storage $(, $stride)?);
+ }
+
+ $crate::register!(base: $reg_base; $($rest)*);
};
+ // All the rules below are private helpers.
+
// Offset computation helper rules.
(@offset @ $offset:expr) => { $offset };
- (@offset => $alias:path) => { <$alias as $crate::io::register::Register>::OFFSET };
+ (@offset => $alias:path) => { <$alias as $crate::io::register::FixedRegister>::OFFSET };
(@offset => $alias:path [$idx:expr]) => {{
$crate::build_assert::static_assert!(
$idx < <$alias as $crate::io::register::RegisterArray>::SIZE
);
- <$alias as $crate::io::register::Register>::OFFSET +
+ <$alias as $crate::io::register::RegisterArray>::OFFSET +
$idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
}};
@@ -657,32 +669,4 @@ macro_rules! register {
$stride
}};
(@stride $ty: ty) => { ::core::mem::size_of::<$ty>() };
-
- // Implementations shared by all registers types.
- (@io_base $reg_base:ty; $name:ident @ $offset:expr) => {
- impl $crate::io::register::Register for $name {
- type Base = $reg_base;
-
- const OFFSET: usize = $offset;
- }
- };
-
- // Implementations of fixed registers.
- (@io_fixed $(#[$attr:meta])* $vis:vis $name:ident) => {
- impl $crate::io::register::FixedRegister for $name {}
-
- $(#[$attr])*
- $vis const $name: $crate::io::register::FixedRegisterLoc<$name> =
- $crate::io::register::FixedRegisterLoc::<$name>::new();
- };
-
- // Implementations of register arrays.
- (@io_array $vis:vis $name:ident [ $size:expr, stride = $stride:expr ]) => {
- impl $crate::io::register::Array for $name {}
-
- impl $crate::io::register::RegisterArray for $name {
- const SIZE: usize = $size;
- const STRIDE: usize = $stride;
- }
- };
}
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v2 16/16] rust: io: register: unify handling of register with/without bitfields
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (14 preceding siblings ...)
2026-08-05 16:35 ` [PATCH v2 15/16] rust: io: register: remove `Register` trait and cleanup macro Gary Guo
@ 2026-08-05 16:35 ` Gary Guo
2026-08-05 16:51 ` sashiko-bot
15 siblings, 1 reply; 35+ messages in thread
From: Gary Guo @ 2026-08-05 16:35 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
linux-pci, Gary Guo
Move the `FixedRegister` to be a property of register to become a property
of type. Name the new trait `FixedIoLoc` indicating if I/O location of a
type is unique for a specific base. Thus, bitfields become just a special
case of this (where type is unique because we're generating it in the
register macro).
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io/register.rs | 123 ++++++++++++++++++++-------------------------
1 file changed, 54 insertions(+), 69 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index fe0e6763a600..80282c423868 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -121,61 +121,15 @@
io::IoLoc, //
};
-/// Trait implemented by registers with a fixed offset.
-pub trait FixedRegister: Sized {
- /// Base type for this register.
- type Base: ?Sized;
-
- /// Start offset of the register.
- ///
- /// The interpretation of this offset depends on the type of the register.
- const OFFSET: usize;
-}
-
/// Allows `()` to be used as the `location` parameter of [`Io::write`](super::Io::write) when
-/// passing a [`FixedRegister`] value.
+/// passing a [`FixedIoLoc`] value.
impl<Base: ?Sized, T> IoLoc<Base, T> for ()
where
- T: FixedRegister<Base = Base>,
-{
- #[inline(always)]
- fn offset(self) -> usize {
- T::OFFSET
- }
-}
-
-/// A [`FixedRegister`] carries its location in its type. Thus `FixedRegister` values can be used
-/// as an [`IoLoc`].
-impl<Base: ?Sized, T> IoLoc<Base, T> for T
-where
- T: FixedRegister<Base = Base>,
+ T: FixedIoLoc<Base>,
{
#[inline(always)]
fn offset(self) -> usize {
- T::OFFSET
- }
-}
-
-/// Location of a fixed register.
-pub struct FixedRegisterLoc<T: FixedRegister>(PhantomData<T>);
-
-impl<T: FixedRegister> FixedRegisterLoc<T> {
- /// Returns the location of `T`.
- #[inline(always)]
- // We do not implement `Default` so we can be const.
- #[expect(clippy::new_without_default)]
- pub const fn new() -> Self {
- Self(PhantomData)
- }
-}
-
-impl<Base: ?Sized, T> IoLoc<Base, T> for FixedRegisterLoc<T>
-where
- T: FixedRegister<Base = Base>,
-{
- #[inline(always)]
- fn offset(self) -> usize {
- T::OFFSET
+ T::LOCATION.offset()
}
}
@@ -187,6 +141,11 @@ impl<Base: ?Sized, T> OffsetLoc<Base, T> {
pub const fn new(offset: usize) -> Self {
Self(offset, PhantomData)
}
+
+ #[inline]
+ pub const fn const_offset(self) -> usize {
+ self.0
+ }
}
impl<Base: ?Sized, T> IoLoc<Base, T> for OffsetLoc<Base, T> {
@@ -265,6 +224,17 @@ fn try_at(idx: usize) -> Option<RegisterArrayLoc<Self>>
}
}
+/// Trait implemented by types that indicate there is a fixed I/O location for this given type.
+///
+/// Implementors can be used with [`Io::write_reg`](super::Io::write_reg).
+pub trait FixedIoLoc<Base: ?Sized>: Sized {
+ /// Type of [`FixedIoLoc::location`].
+ type Location: IoLoc<Base, Self>;
+
+ /// Location of this type within given base.
+ const LOCATION: Self::Location;
+}
+
/// Trait implemented by items that contain both a register value and the absolute I/O location at
/// which to write it.
///
@@ -282,14 +252,14 @@ pub trait LocatedRegister<Base: ?Sized> {
impl<Base: ?Sized, T> LocatedRegister<Base> for T
where
- T: FixedRegister<Base = Base>,
+ T: FixedIoLoc<Base>,
{
- type Location = FixedRegisterLoc<Self::Value>;
+ type Location = T::Location;
type Value = T;
#[inline(always)]
- fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
- (FixedRegisterLoc::new(), self)
+ fn into_io_op(self) -> (T::Location, T) {
+ (T::LOCATION, self)
}
}
@@ -577,6 +547,9 @@ macro_rules! register {
(base: $reg_base:ty;) => {};
// Creates a register at a fixed offset of the MMIO space with provided type.
+ //
+ // This handles all of the fixed offset `@ offset`, alias of register `=> alias` and alias of
+ // register array element `=> alias[idx]` cases.
(
base: $reg_base:ty;
// `$ty` cannot be `:ty` due to follow-set restrictions.
@@ -593,10 +566,29 @@ macro_rules! register {
$crate::register!(base: $reg_base; $($rest)*);
};
+ // `#[unique]` indicates that this is the only register of this type in this given register.
+ // Thus generate a `FixedIoLoc` impl for it as well.
+ (
+ base: $reg_base:ty;
+ $(#[$attr:meta])* $vis:vis $name:ident: #[unique] $ty: ident $(:: $path_frag:ident)*
+ $(@ $offset:literal)?
+ $(=> $alias:path $([$alias_idx:expr])? )?;
+ $($rest:tt)*
+ ) => {
+ impl $crate::io::register::FixedIoLoc<$reg_base> for $name {
+ type Location = $crate::io::register::OffsetLoc<$reg_base, $ty $(:: $path_frag)*>;
+ const LOCATION: Self::Location = $name;
+ }
+
+ $crate::register!(
+ base: $reg_base;
+ $(#[$attr])* $vis $name: $ty $(:: $path_frag)*
+ $(@ $offset)? $(=> $alias $([$alias_idx])? )?;
+ $($rest)*
+ );
+ };
+
// Creates a register at a fixed offset of the MMIO space.
- //
- // This handles all of the fixed offset `@ offset`, alias of register `=> alias` and alias of
- // register array element `=> alias[idx]` cases.
(
base: $reg_base:ty;
$(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
@@ -610,18 +602,11 @@ macro_rules! register {
$(#[$attr])* $vis struct $name($storage) { $($fields)* }
);
- impl $crate::io::register::FixedRegister for $name {
- type Base = $reg_base;
-
- const OFFSET: usize =
- $crate::register!(@offset $(@ $offset)? $(=> $alias $([$alias_idx])?)?);
- }
-
- $(#[$attr])*
- $vis const $name: $crate::io::register::FixedRegisterLoc<$name> =
- $crate::io::register::FixedRegisterLoc::<$name>::new();
-
- $crate::register!(base: $reg_base; $($rest)*);
+ $crate::register!(
+ base: $reg_base;
+ $(#[$attr])* $vis $name: #[unique] $name $(@ $offset)? $(=> $alias $([$alias_idx])? )?;
+ $($rest)*
+ );
};
// Creates an array of registers at a fixed offset of the MMIO space.
@@ -653,7 +638,7 @@ impl $crate::io::register::RegisterArray for $name {
// Offset computation helper rules.
(@offset @ $offset:expr) => { $offset };
- (@offset => $alias:path) => { <$alias as $crate::io::register::FixedRegister>::OFFSET };
+ (@offset => $alias:path) => { $alias.const_offset() };
(@offset => $alias:path [$idx:expr]) => {{
$crate::build_assert::static_assert!(
$idx < <$alias as $crate::io::register::RegisterArray>::SIZE
--
2.54.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v2 16/16] rust: io: register: unify handling of register with/without bitfields
2026-08-05 16:35 ` [PATCH v2 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo
@ 2026-08-05 16:51 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-08-05 16:51 UTC (permalink / raw)
To: Gary Guo; +Cc: linux-pci, Danilo Krummrich, Alexandre Courbot, dri-devel, ojeda
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The documentation comment for `FixedIoLoc::Location` references `[`FixedIoLoc::location`]`, which does not exist.
--
--- Patch [16]: [PATCH v2 16/16] rust: io: register: unify handling of register with/without bitfields ---
commit eee244bc141adb8220a5c6420bc3a94addf8572c
Author: Gary Guo <gary@garyguo.net>
rust: io: register: unify handling of register with/without bitfields
This commit unifies the handling of registers with and without bitfields by making the fixed location a property of the type. It replaces FixedRegister with a new FixedIoLoc trait to indicate if a type's I/O location is unique for a specific base.
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index fe0e6763a600e..80282c423868a 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
[ ... ]
> @@ -265,6 +224,17 @@ fn try_at(idx: usize) -> Option<RegisterArrayLoc<Self>>
> }
> }
>
> +/// Trait implemented by types that indicate there is a fixed I/O location for this given type.
> +///
> +/// Implementors can be used with [`Io::write_reg`](super::Io::write_reg).
> +pub trait FixedIoLoc<Base: ?Sized>: Sized {
> + /// Type of [`FixedIoLoc::location`].
[Severity: Low]
This isn't a bug, but does this intra-doc link point to a non-existent item?
The trait defines a Location type and a LOCATION constant, but no location
method or field, which will likely result in a broken rustdoc link.
> + type Location: IoLoc<Base, Self>;
> +
> + /// Location of this type within given base.
> + const LOCATION: Self::Location;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-typed_register-v2-0-c3ca142220a0@garyguo.net?part=16
^ permalink raw reply [flat|nested] 35+ messages in thread