* [PATCH v3 01/16] rust: io: register: reimplement as proc macro
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:17 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 02/16] rust: mem: add `transmute` with deferred size check Gary Guo
` (14 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 existing `register!` macro is implemented as a declarative macro.
Reimplement it as proc macro instead, with no functional changes intended.
The old implementation produces unhelpful diagnostics when things go wrong.
For example, for code like
register! {
pub(crate) TESTREG(u32) {
31:0 data;
}
}
which misses out the "@ offset" part of the specification, and the
following error is produced:
error: no rules expected `{`
--> test.rs:42:5
|
42 | / register! {
43 | | pub(crate) TESTREG(u32) {
44 | | 31:0 data;
... |
100 | | }
| |_____^ no rules expected this token in macro call
which isn't very helpful. With the proc macro implementation, the following
error is produced:
error: expected `@` or `=>`
--> tests.rs:43:33
|
43 | pub(crate) TESTREG(u32) {
| ^
which is much more helpful. Apart from diagnostics, proc macro also has a
benefit of not having follow-set restrictions, which makes syntax like
register!(name: ty @ offset);
possible; declarative macro will reject this as `@` is not in the
follow-set of "ty" metavariable kind.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
MAINTAINERS | 1 +
rust/kernel/io/register.rs | 224 +-------------------------------------
rust/macros/io/mod.rs | 3 +
rust/macros/io/register.rs | 263 +++++++++++++++++++++++++++++++++++++++++++++
rust/macros/lib.rs | 10 ++
5 files changed, 279 insertions(+), 222 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 637bdf68135e..c9f11fa9f2b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7486,6 +7486,7 @@ F: rust/kernel/io.rs
F: rust/kernel/io/
F: rust/kernel/irq.rs
F: rust/kernel/irq/
+F: rust/macros/io/
DEVICE RESOURCE MANAGEMENT HELPERS
M: Hans de Goede <hansg@kernel.org>
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 03dfd2ff48c7..6a19552ffb95 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -801,227 +801,7 @@ 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.
- //
- // TODO: change `alias:ident` to `alias:path` once relative registers are replaced by I/O
- // projections.
- (
- $(
- $(#[$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
- { $($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);
- };
-
- // Creates an alias register of fixed offset register `alias` with its own fields.
- (
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $alias:path
- { $($fields:tt)* }
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(
- @io_base $name($storage) @
- <$alias as $crate::io::register::Register>::OFFSET
- );
- $crate::register!(@io_fixed $(#[$attr])* $vis $name);
- };
-
- // 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
- { $($fields:tt)* }
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(@io_base $name($storage) @ $offset);
- $crate::register!(@io_relative $name @ $base);
- };
-
- // 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
- { $($fields:tt)* }
- ) => {
- $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
- $crate::register!(
- @io_base $name($storage) @ <$alias as $crate::io::register::Register>::OFFSET
- );
- $crate::register!(@io_relative $name @ $base);
- };
-
- // Creates an array of registers at a fixed offset of the MMIO space.
- (
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
- [ $size:expr, stride = $stride:expr ] @ $offset:literal { $($fields: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($storage) @ $offset);
- $crate::register!(@io_array $name [ $size, stride = $stride ]);
- };
-
- // Shortcut for contiguous array of registers (stride == size of element).
- (
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ] @ $offset:literal
- { $($fields:tt)* }
- ) => {
- $crate::register!(
- @reg $(#[$attr])* $vis $name($storage)
- [ $size, stride = ::core::mem::size_of::<$storage>() ]
- @ $offset { $($fields)* }
- );
- };
-
- // 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:path [ $idx:expr ]
- { $($fields: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 $name($storage) @
- <$alias as $crate::io::register::Register>::OFFSET
- + $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
- );
- $crate::register!(@io_fixed $(#[$attr])* $vis $name);
- };
-
- // Creates an array of registers at a relative offset from a base address provider.
- (
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
- [ $size:expr, stride = $stride:expr ]
- @ $base:ident + $offset:literal { $($fields: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($storage) @ $offset);
- $crate::register!(@io_relative_array $name [ $size, stride = $stride ] @ $base);
- };
-
- // Shortcut for contiguous array of relative registers (stride == size of element).
- (
- @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) [ $size:expr ]
- @ $base:ident + $offset:literal { $($fields:tt)* }
- ) => {
- $crate::register!(
- @reg $(#[$attr])* $vis $name($storage)
- [ $size, stride = ::core::mem::size_of::<$storage>() ]
- @ $base + $offset { $($fields)* }
- );
- };
-
- // 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)
- => $base:ident + $alias:ident [ $idx:expr ] { $($fields: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 $name($storage) @
- <$alias as $crate::io::register::Register>::OFFSET +
- $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
- );
- $crate::register!(@io_relative $name @ $base);
- };
-
- // 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)* }
- );
- };
-
- // Implementations shared by all registers types.
- (@io_base $name:ident($storage:ty) @ $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) => {
- impl $crate::io::register::FixedRegister for $name {}
-
- $(#[$attr])*
- $vis const $name: $crate::io::register::FixedRegisterLoc<$name> =
- $crate::io::register::FixedRegisterLoc::<$name>::new();
- };
-
- // Implementations of relative registers.
- (@io_relative $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 $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;
- }
- };
-
- // Implementations of relative array registers.
- (
- @io_relative_array $name:ident [ $size:expr, stride = $stride:expr ] @ $base:ident
- ) => {
- 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 {}
+ ($($tt:tt)*) => {
+ $crate::macros::register!($($tt)*);
};
}
diff --git a/rust/macros/io/mod.rs b/rust/macros/io/mod.rs
new file mode 100644
index 000000000000..39fa5bc302ba
--- /dev/null
+++ b/rust/macros/io/mod.rs
@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+pub(crate) mod register;
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
new file mode 100644
index 000000000000..61d076ab570a
--- /dev/null
+++ b/rust/macros/io/register.rs
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use proc_macro2::{
+ Group,
+ Literal,
+ Span,
+ TokenStream, //
+};
+use quote::{
+ quote,
+ quote_spanned, //
+};
+use syn::{
+ bracketed,
+ parenthesized,
+ parse::Parse,
+ spanned::Spanned,
+ token,
+ Attribute,
+ Expr,
+ Ident,
+ Path,
+ Result,
+ Token,
+ Type,
+ Visibility, //
+};
+
+mod kw {
+ syn::custom_keyword!(stride);
+}
+
+struct RegArrayDef {
+ size: Expr,
+ stride: Option<Expr>,
+}
+
+enum RegOffset {
+ /// Register is located at fixed address.
+ Fixed { offset: Literal },
+ /// Register is an alias of a fixed register.
+ Alias { alias: Path },
+ /// Register is an alias of an element of a register array.
+ ElementAlias { alias: Path, idx: Expr },
+}
+
+struct Reg {
+ attrs: Vec<Attribute>,
+ vis: Visibility,
+ name: Ident,
+ storage: Type,
+ array: Option<RegArrayDef>,
+ relative_base: Option<Path>,
+ offset: RegOffset,
+ bitfield_args: Group,
+}
+
+impl Parse for Reg {
+ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
+ let attrs = input.call(Attribute::parse_outer)?;
+ let vis = input.parse()?;
+ let name = input.parse()?;
+ let storage = {
+ let content;
+ parenthesized!(content in input);
+ content.parse()?
+ };
+ let array = if input.peek(token::Bracket) {
+ let content;
+ bracketed!(content in input);
+ let size = content.parse()?;
+ let stride = if content.peek(Token![,]) {
+ let _: Token![,] = content.parse()?;
+ let _: kw::stride = content.parse()?;
+ let _: Token![=] = content.parse()?;
+ Some(content.parse()?)
+ } else {
+ None
+ };
+ Some(RegArrayDef { size, stride })
+ } else {
+ None
+ };
+
+ let lh = input.lookahead1();
+ let mut relative_base = None;
+ let offset = if lh.peek(Token![@]) {
+ let _: Token![@] = input.parse()?;
+
+ if input.peek(Ident) {
+ relative_base = Some(input.parse()?);
+ let _: Token![+] = input.parse()?;
+ }
+
+ RegOffset::Fixed {
+ offset: input.parse()?,
+ }
+ } else if lh.peek(Token![=>]) {
+ let _: Token![=>] = input.parse()?;
+ let mut alias: Path = input.parse()?;
+ if input.peek(Token![+]) {
+ let _: Token![+] = input.parse()?;
+ relative_base = Some(alias);
+ alias = input.parse()?;
+ }
+
+ if input.peek(token::Bracket) {
+ let content;
+ bracketed!(content in input);
+ RegOffset::ElementAlias {
+ alias,
+ idx: content.parse()?,
+ }
+ } else {
+ RegOffset::Alias { alias }
+ }
+ } else {
+ Err(lh.error())?
+ };
+
+ let lh = input.lookahead1();
+ let bitfield_args = if lh.peek(token::Brace) {
+ input.parse()?
+ } else {
+ Err(lh.error())?
+ };
+
+ Ok(Self {
+ attrs,
+ vis,
+ name,
+ storage,
+ array,
+ relative_base,
+ offset,
+ bitfield_args,
+ })
+ }
+}
+
+pub(crate) struct RegDef {
+ regs: Vec<Reg>,
+}
+
+impl Parse for RegDef {
+ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
+ let mut regs = Vec::new();
+ while !input.is_empty() {
+ regs.push(input.parse()?);
+ }
+ Ok(RegDef { regs })
+ }
+}
+
+pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
+ let mut outputs = TokenStream::new();
+
+ for reg in def.regs {
+ let Reg {
+ attrs,
+ vis,
+ name,
+ storage,
+ array,
+ relative_base,
+ offset,
+ bitfield_args,
+ } = reg;
+
+ // Use register name's span for generated code, so error messages (if any) can point to it
+ // instead of the entire register allocation.
+ let span = name.span().resolved_at(Span::mixed_site());
+
+ let offset = match offset {
+ RegOffset::Fixed { offset } => quote!(#offset),
+ RegOffset::Alias { alias } => {
+ quote_spanned!(alias.span().resolved_at(span) =>
+ <#alias as ::kernel::io::register::Register>::OFFSET
+ )
+ }
+ RegOffset::ElementAlias { alias, idx } => {
+ outputs.extend(quote_spanned!(idx.span().resolved_at(span) =>
+ ::kernel::build_assert::static_assert!(
+ #idx < <#alias as ::kernel::io::register::RegisterArray>::SIZE
+ );
+ ));
+ quote_spanned!(alias.span().resolved_at(span) =>
+ <#alias as ::kernel::io::register::Register>::OFFSET
+ + #idx * <#alias as ::kernel::io::register::RegisterArray>::STRIDE
+ )
+ }
+ };
+
+ outputs.extend(quote_spanned!(span =>
+ ::kernel::bitfield!(
+ // `#[allow(non_camel_case_types)]` is added since register names typically use
+ // `SCREAMING_CASE`.
+ #[allow(non_camel_case_types)]
+ #(#attrs)* #vis struct #name(#storage) #bitfield_args
+ );
+
+ impl ::kernel::io::register::Register for #name {
+ type Storage = #storage;
+
+ const OFFSET: usize = #offset;
+ }
+ ));
+
+ match array {
+ None => match relative_base {
+ None => outputs.extend(quote_spanned!(span =>
+ impl ::kernel::io::register::FixedRegister for #name {}
+
+ #(#attrs)* #vis const #name: ::kernel::io::register::FixedRegisterLoc<#name> =
+ ::kernel::io::register::FixedRegisterLoc::<#name>::new();
+ )),
+ Some(relative_base) => outputs.extend(quote_spanned!(span =>
+ impl ::kernel::io::register::WithBase for #name {
+ type BaseFamily = #relative_base;
+ }
+
+ impl ::kernel::io::register::RelativeRegister for #name {}
+ )),
+ },
+
+ Some(def) => {
+ let size = &def.size;
+ let stride = if let Some(stride) = &def.stride {
+ outputs.extend(quote_spanned!(stride.span().resolved_at(span) =>
+ ::kernel::build_assert::static_assert!(
+ ::core::mem::size_of::<#storage>() <= #stride
+ );
+ ));
+ quote!(#stride)
+ } else {
+ quote_spanned!(span => ::core::mem::size_of::<#storage>())
+ };
+
+ outputs.extend(quote_spanned!(span =>
+ impl ::kernel::io::register::RegisterArray for #name {
+ const SIZE: usize = #size;
+ const STRIDE: usize = #stride;
+ }
+ ));
+
+ match relative_base {
+ None => outputs.extend(quote_spanned!(span =>
+ impl ::kernel::io::register::Array for #name {}
+ )),
+ Some(relative_base) => outputs.extend(quote_spanned!(span =>
+ impl ::kernel::io::register::WithBase for #name {
+ type BaseFamily = #relative_base;
+ }
+
+ impl ::kernel::io::register::RelativeRegisterArray for #name {}
+ )),
+ }
+ }
+ };
+ }
+
+ Ok(outputs)
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 24f96feaeb34..5807dee84747 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -19,6 +19,7 @@
mod fmt;
mod for_lt;
mod helpers;
+mod io;
mod kunit;
mod module;
mod paste;
@@ -481,6 +482,15 @@ pub fn paste(input: TokenStream) -> TokenStream {
.into()
}
+#[doc(hidden)] // Documented in `kernel` crate.
+#[proc_macro]
+#[allow(non_snake_case)]
+pub fn register(input: TokenStream) -> TokenStream {
+ io::register::register(parse_macro_input!(input))
+ .unwrap_or_else(|e| e.into_compile_error())
+ .into()
+}
+
/// Registers a KUnit test suite and its test cases using a user-space like syntax.
///
/// This macro should be used on modules. If `CONFIG_KUNIT` (in `.config`) is `n`, the target module
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 01/16] rust: io: register: reimplement as proc macro
2026-08-19 11:09 ` [PATCH v3 01/16] rust: io: register: reimplement as proc macro Gary Guo
@ 2026-08-19 11:17 ` sashiko-bot
0 siblings, 0 replies; 33+ messages in thread
From: sashiko-bot @ 2026-08-19 11:17 UTC (permalink / raw)
To: Gary Guo; +Cc: dri-devel, ojeda, linux-pci, Danilo Krummrich, Alexandre Courbot
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing parentheses around `#idx` in the generated register offset expression causes operator precedence inversion.
--
--- Patch [1]: [PATCH v3 01/16] rust: io: register: reimplement as proc macro ---
commit e2035dd40ab99d9c728dbbe1a208159d8a85be92
Author: Gary Guo <gary@garyguo.net>
rust: io: register: reimplement as proc macro
This commit reimplements the existing declarative macro for register allocation
as a proc macro. This change provides more helpful diagnostics for syntax
errors and allows more flexible syntax without follow-set restrictions.
> diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
> new file mode 100644
> index 0000000000000..61d076ab570a0
> --- /dev/null
> +++ b/rust/macros/io/register.rs
[ ... ]
> + let offset = match offset {
> + RegOffset::Fixed { offset } => quote!(#offset),
> + RegOffset::Alias { alias } => {
> + quote_spanned!(alias.span().resolved_at(span) =>
> + <#alias as ::kernel::io::register::Register>::OFFSET
> + )
> + }
> + RegOffset::ElementAlias { alias, idx } => {
> + outputs.extend(quote_spanned!(idx.span().resolved_at(span) =>
> + ::kernel::build_assert::static_assert!(
> + #idx < <#alias as ::kernel::io::register::RegisterArray>::SIZE
> + );
> + ));
> + quote_spanned!(alias.span().resolved_at(span) =>
> + <#alias as ::kernel::io::register::Register>::OFFSET
> + + #idx * <#alias as ::kernel::io::register::RegisterArray>::STRIDE
[Severity: High]
Will this cause operator precedence issues if the parsed index expression
contains addition or subtraction?
Because idx is parsed as an Expr and inserted directly into the quote block
without parentheses, complex expressions will be emitted verbatim. If a
developer uses an expression like A + B for the array index, the generated
code would expand to:
OFFSET + A + B * STRIDE
Because multiplication has higher precedence, this calculates
OFFSET + A + (B * STRIDE) instead of the intended OFFSET + ((A + B) * STRIDE).
This could result in calculating incorrect I/O memory offsets and writing to
the wrong device registers.
Could idx be wrapped in parentheses here?
> + )
> + }
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-typed_register-v3-0-3699a75fadf1@garyguo.net?part=1
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 02/16] rust: mem: add `transmute` with deferred size check
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
2026-08-19 11:09 ` [PATCH v3 01/16] rust: io: register: reimplement as proc macro Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:16 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut` Gary Guo
` (13 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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
Implement a `transmute_unchecked/transmute` that checks size at
monomorphization time instead of type-checking time. This allows more cases
where we know that the size matches but this is not generically checkable.
The signature is equivalent to the unstable `transmute_neo` function in the
standard library. A safe variant is provided to use with types implementing
`FromBytes` and `IntoBytes`.
Existing users of `transmute_copy` to bypass size checks are converted.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/device_id.rs | 3 +--
rust/kernel/lib.rs | 1 +
rust/kernel/mem.rs | 37 +++++++++++++++++++++++++++++++++++++
rust/kernel/sync/atomic.rs | 4 ++--
4 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index c81fca5b4986..cb8661dec2ab 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -146,8 +146,7 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
/// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
pub const fn new_without_index(ids: [T; N]) -> Self {
// SAFETY: `T` is layout-wise compatible with `T::RawType`, so is the array of them.
- let raw_ids: [MaybeUninit<T::RawType>; N] = unsafe { core::mem::transmute_copy(&ids) };
- core::mem::forget(ids);
+ let raw_ids: [MaybeUninit<T::RawType>; N] = unsafe { crate::mem::transmute_unchecked(ids) };
Self {
ids: raw_ids,
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 4d5c96ddc49c..7225abc64084 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -98,6 +98,7 @@
pub mod kunit;
pub mod list;
pub mod maple_tree;
+pub mod mem;
pub mod miscdevice;
pub mod mm;
pub mod module;
diff --git a/rust/kernel/mem.rs b/rust/kernel/mem.rs
new file mode 100644
index 000000000000..a0901cbe1b2d
--- /dev/null
+++ b/rust/kernel/mem.rs
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Basic utilities for dealing with memory, values, and types.
+
+use crate::prelude::*;
+
+/// Version of `transmute` that performs size check at monomorphization-time.
+///
+/// Use this instead of [`core::mem::transmute`] when it is known that sizes are identical but this
+/// cannot be proven by the compiler during type checking.
+///
+/// The signature is equivalent after Rust standard library's unstable `transmute_neo` and that of
+/// [RFC 3844](https://github.com/rust-lang/rfcs/pull/3844).
+///
+/// # Safety
+///
+/// Same as [`core::mem::transmute`].
+#[inline(always)]
+pub const unsafe fn transmute_unchecked<Src, Dst>(val: Src) -> Dst {
+ const_assert!(size_of::<Src>() == size_of::<Dst>());
+
+ // SAFETY: This is identical to `transmute` except that we bypassed the size check; which we
+ // used `const_assert!` to check above.
+ unsafe { core::mem::transmute_copy(&core::mem::ManuallyDrop::new(val)) }
+}
+
+/// Safely transmutes a value of one type to a value of another type of the same size.
+///
+/// The sizes are checked during monomorphization.
+///
+/// This can be considered as generic version of [`zerocopy::transmute!`] macro that defers the size
+/// check and thus can be used in more cases.
+#[inline(always)]
+pub const fn transmute<Src: IntoBytes, Dst: FromBytes>(val: Src) -> Dst {
+ // SAFETY: transmute is safe with `IntoBytes` and `FromBytes` bounds.
+ unsafe { transmute_unchecked(val) }
+}
diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index 9cd009d57e35..d5f4ef285193 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -140,7 +140,7 @@ pub unsafe trait AtomicAdd<Rhs = Self>: AtomicType {
const fn into_repr<T: AtomicType>(v: T) -> T::Repr {
// SAFETY: Per the safety requirement of `AtomicType`, `T` is round-trip transmutable to
// `T::Repr`, therefore the transmute operation is sound.
- unsafe { core::mem::transmute_copy(&v) }
+ unsafe { crate::mem::transmute_unchecked(v) }
}
/// # Safety
@@ -149,7 +149,7 @@ const fn into_repr<T: AtomicType>(v: T) -> T::Repr {
#[inline(always)]
const unsafe fn from_repr<T: AtomicType>(r: T::Repr) -> T {
// SAFETY: Per the safety requirement of the function, the transmute operation is sound.
- unsafe { core::mem::transmute_copy(&r) }
+ unsafe { crate::mem::transmute_unchecked(r) }
}
impl<T: AtomicType> Atomic<T> {
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut`
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
2026-08-19 11:09 ` [PATCH v3 01/16] rust: io: register: reimplement as proc macro Gary Guo
2026-08-19 11:09 ` [PATCH v3 02/16] rust: mem: add `transmute` with deferred size check Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:19 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 04/16] rust: io: perform conversions using `AsRepr` Gary Guo
` (12 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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
Some API like atomics and I/O operate on primitives only; therefore other
types would need to converted to these primitive first. Add two traits
`AsRepr` and `AsReprMut` to indicate that the type can be turned into a
primitive for these operations.
Conceptually, `T: AsRepr` means that `&T` can be viewed as `&T::Repr` and
thus it has only a round-trip transmutability requirement. `T: AsReprMut`
means that `&mut T` can be viewed as `&mut T::Repr` and thus it needs to
support bi-directional transmutability.
To avoid duplicating implementation, all repr types are normalized to
unsigned integers.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/mem.rs | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 148 insertions(+)
diff --git a/rust/kernel/mem.rs b/rust/kernel/mem.rs
index a0901cbe1b2d..5bce381d8895 100644
--- a/rust/kernel/mem.rs
+++ b/rust/kernel/mem.rs
@@ -35,3 +35,151 @@ pub const fn transmute<Src: IntoBytes, Dst: FromBytes>(val: Src) -> Dst {
// SAFETY: transmute is safe with `IntoBytes` and `FromBytes` bounds.
unsafe { transmute_unchecked(val) }
}
+
+/// Type that is layout-compatible with a primitive representation.
+///
+/// # Round-trip transmutability
+///
+/// `T` is round-trip transmutable to `U` if and only if both of these properties hold:
+///
+/// - Any valid bit pattern for `T` is also a valid bit pattern for `U`.
+/// - Transmuting a value of type `T` to `U` and then to `T` again
+/// yields a value that is in all aspects equivalent to the original value.
+///
+/// # Safety
+///
+/// - [`Self`] must have the same size and alignment as [`Self::Repr`].
+/// - [`Self`] must be [round-trip transmutable] to [`Self::Repr`].
+///
+/// [round-trip transmutable]: AsRepr#round-trip-transmutability
+pub unsafe trait AsRepr: Sized {
+ /// Primitive representation of this type.
+ type Repr;
+
+ /// Convert from [`AsRepr::Repr`] to `Self`.
+ ///
+ /// # Safety
+ ///
+ /// `repr` must be a valid bit patern of `Self`. If `repr` is previously obtained using
+ /// [`AsRepr::into_repr`], then it will always be safe.
+ #[inline(always)]
+ unsafe fn from_repr_unchecked(repr: Self::Repr) -> Self {
+ // SAFETY: Per safety requirement of the trait.
+ unsafe { transmute_unchecked(repr) }
+ }
+
+ /// Convert from `Self` to [`AsRepr::Repr`].
+ #[inline(always)]
+ fn into_repr(this: Self) -> Self::Repr {
+ // SAFETY: Per safety requirement of the trait.
+ unsafe { transmute_unchecked(this) }
+ }
+}
+
+/// Type that is bi-directionally transmutable with a primitive representation.
+///
+/// # Safety
+///
+/// - [`Self`] must be [transmutable] from [`Self::Repr`].
+/// - Note that [`Self::Repr`] must be [transmutable] from `Self` as well, however that is a
+/// requirement of the [`AsRepr`] super trait already.
+///
+/// [`transmutable`]: core::mem::transmute
+pub unsafe trait AsReprMut: AsRepr {
+ /// Convert from [`AsRepr::Repr`] to `Self`.
+ #[inline(always)]
+ fn from_repr(repr: Self::Repr) -> Self {
+ // SAFETY: Per safety requirement of the trait.
+ unsafe { transmute_unchecked(repr) }
+ }
+}
+
+// SAFETY: `bool` has the same size and alignment as `u8`, and Rust guarantees that `bool` has
+// only two valid bit patterns: 0 (false) and 1 (true). Those are valid `u8` values, so `bool` is
+// round-trip transmutable to `u8`.
+unsafe impl AsRepr for bool {
+ type Repr = u8;
+}
+
+// SAFETY: `*mut T` has the same size and alignment with `*const c_void`, and is round-trip
+// transmutable to `*const c_void`.
+unsafe impl<T> AsRepr for *mut T {
+ type Repr = *const c_void;
+}
+
+// SAFETY: `*mut T` is transmutable from `*const c_void`.
+unsafe impl<T> AsReprMut for *mut T {}
+
+// SAFETY: `*const T` has the same size and alignment with `*const c_void`, and is round-trip
+// transmutable to `*const c_void`.
+unsafe impl<T> AsRepr for *const T {
+ type Repr = *const c_void;
+}
+
+// SAFETY: `*const T` is transmutable from `*const c_void`.
+unsafe impl<T> AsReprMut for *const T {}
+
+macro_rules! int_impl {
+ ($($unsigned:ident $signed:ident ,)*) => {$(
+ // SAFETY: $unsigned has the same size and alignment with itself, and is round-trip
+ // transmutable to itself.
+ unsafe impl AsRepr for $unsigned {
+ type Repr = $unsigned;
+ }
+
+ // SAFETY: $unsigned is transmutable from itself.
+ unsafe impl AsReprMut for $unsigned {}
+
+ // SAFETY: $signed has the same size and alignment with $unsigned, and is round-trip
+ // transmutable to it.
+ unsafe impl AsRepr for $signed {
+ type Repr = $unsigned;
+ }
+
+ // SAFETY: $signed is transmutable from $unsigned.
+ unsafe impl AsReprMut for $signed {}
+ )*};
+}
+
+int_impl! {
+ u8 i8,
+ u16 i16,
+ u32 i32,
+ u64 i64,
+}
+
+#[cfg(target_pointer_width = "32")]
+const _: () = {
+ // SAFETY: usize has the same size and alignment with u32, and is round-trip transmutable to it.
+ unsafe impl AsRepr for usize {
+ type Repr = u32;
+ }
+
+ // SAFETY: isize has the same size and alignment with u32, and is round-trip transmutable to it.
+ unsafe impl AsRepr for isize {
+ type Repr = u32;
+ }
+
+ // SAFETY: usize is transmutable from u32.
+ unsafe impl AsReprMut for usize {}
+ // SAFETY: isize is transmutable from u32.
+ unsafe impl AsReprMut for isize {}
+};
+
+#[cfg(target_pointer_width = "64")]
+const _: () = {
+ // SAFETY: usize has the same size and alignment with u64, and is round-trip transmutable to it.
+ unsafe impl AsRepr for usize {
+ type Repr = u64;
+ }
+
+ // SAFETY: isize has the same size and alignment with u64, and is round-trip transmutable to it.
+ unsafe impl AsRepr for isize {
+ type Repr = u64;
+ }
+
+ // SAFETY: usize is transmutable from u64.
+ unsafe impl AsReprMut for usize {}
+ // SAFETY: isize is transmutable from u64.
+ unsafe impl AsReprMut for isize {}
+};
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut`
2026-08-19 11:09 ` [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut` Gary Guo
@ 2026-08-19 11:19 ` sashiko-bot
0 siblings, 0 replies; 33+ messages in thread
From: sashiko-bot @ 2026-08-19 11:19 UTC (permalink / raw)
To: Gary Guo; +Cc: ojeda, Danilo Krummrich, Alexandre Courbot, linux-pci, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Incorrect use of `#[inline(always)]` instead of `#[inline]` for abstraction trait methods.
- [Low] The commit message asserts that 'all repr types are normalized to unsigned integers', but pointer types use `*const c_void` as their representation.
--
--- Patch [3]: [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut` ---
commit 0638c4f7e52f9eb87681654296d18ff15c916f15
Author: Gary Guo <gary@garyguo.net>
rust: mem: add AsRepr and AsReprMut
This commit adds AsRepr and AsReprMut traits to indicate that a type
can be turned into a primitive for operations like atomics and I/O. It
also provides implementations for booleans, pointers, and integers.
> To avoid duplicating implementation, all repr types are normalized to
> unsigned integers.
[Severity: Low]
This isn't a bug, but the commit message states that all repr types are
normalized to unsigned integers. However, the implementations for *mut T
and *const T use *const c_void as their representation instead of an
unsigned integer. Should the commit message or the pointer implementations
be updated to match?
> diff --git a/rust/kernel/mem.rs b/rust/kernel/mem.rs
> index a0901cbe1b2d6..5bce381d88952 100644
> --- a/rust/kernel/mem.rs
> +++ b/rust/kernel/mem.rs
[ ... ]
> +pub unsafe trait AsRepr: Sized {
> + /// Primitive representation of this type.
> + type Repr;
> +
> + /// Convert from [`AsRepr::Repr`] to `Self`.
> + ///
> + /// # Safety
> + ///
> + /// `repr` must be a valid bit patern of `Self`. If `repr` is previously obtained using
> + /// [`AsRepr::into_repr`], then it will always be safe.
> + #[inline(always)]
> + unsafe fn from_repr_unchecked(repr: Self::Repr) -> Self {
[Severity: Low]
This isn't a bug, but is #[inline(always)] the intended attribute here?
The Rust subsystem guidelines state that functions for abstractions should
be annotated with #[inline] unless they use build_assert!() dependent on
function parameters.
> + // SAFETY: Per safety requirement of the trait.
> + unsafe { transmute_unchecked(repr) }
> + }
> +
> + /// Convert from `Self` to [`AsRepr::Repr`].
> + #[inline(always)]
> + fn into_repr(this: Self) -> Self::Repr {
[Severity: Low]
This isn't a bug, but this method might also need to use #[inline] rather
than #[inline(always)] for the same reason.
> + // SAFETY: Per safety requirement of the trait.
> + unsafe { transmute_unchecked(this) }
> + }
> +}
> +
> +/// Type that is bi-directionally transmutable with a primitive representation.
> +///
> +/// # Safety
> +///
> +/// - [`Self`] must be [transmutable] from [`Self::Repr`].
> +/// - Note that [`Self::Repr`] must be [transmutable] from `Self` as well, however that is a
> +/// requirement of the [`AsRepr`] super trait already.
> +///
> +/// [`transmutable`]: core::mem::transmute
> +pub unsafe trait AsReprMut: AsRepr {
> + /// Convert from [`AsRepr::Repr`] to `Self`.
> + #[inline(always)]
> + fn from_repr(repr: Self::Repr) -> Self {
[Severity: Low]
This isn't a bug, but would it be better to use #[inline] here to follow
the abstraction guidelines?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-typed_register-v3-0-3699a75fadf1@garyguo.net?part=3
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 04/16] rust: io: perform conversions using `AsRepr`
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (2 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut` Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:19 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 05/16] rust: io: support register projections Gary Guo
` (11 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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
performing conversions to I/O primitives via the `AsRepr`/`AsReprMut`
trait.
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 | 10 ++++
rust/kernel/io.rs | 135 +++++++++++++++++++++++++--------------------
rust/kernel/io/register.rs | 15 -----
rust/macros/io/register.rs | 2 -
4 files changed, 86 insertions(+), 76 deletions(-)
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index a0d089423f21..619c5e2189d1 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,15 @@ fn from(val: $storage) -> $name {
Self::from_raw(val)
}
}
+
+ // SAFETY: `$name` is transparent over `$storage`.
+ unsafe impl $crate::mem::AsRepr for $name {
+ // Normalize `$storage` to the canonical repr type in case it is signed.
+ type Repr = <$storage as $crate::mem::AsRepr>::Repr;
+ }
+
+ // SAFETY: `$name` is transparent over `$storage`.
+ unsafe impl $crate::mem::AsReprMut for $name {}
};
// Definitions requiring knowledge of individual fields: private and public field accessors,
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 5ce9fd129068..6c087d30a166 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -11,6 +11,10 @@
use crate::{
bindings,
+ mem::{
+ AsRepr,
+ AsReprMut, //
+ },
prelude::*,
ptr::{
Alignment,
@@ -353,15 +357,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:
+/// AsRepr` bound and the `<T as AsRepr>::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;
}
@@ -372,8 +373,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
@@ -498,10 +497,19 @@ 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: AsReprMut,
+ Self::Backend: IoCapable<<Self::Target as AsRepr>::Repr>,
{
- Self::Backend::io_read(self.as_view())
+ let view = self.as_view();
+ // SAFETY: `AsRepr` guarantees layout compatibility.
+ let repr_view = unsafe {
+ Self::Backend::project_view(
+ view,
+ Self::Backend::as_ptr(view).cast::<<Self::Target as AsRepr>::Repr>(),
+ )
+ };
+
+ Self::Target::from_repr(Self::Backend::io_read(repr_view))
}
/// Write a value to I/O.
@@ -520,10 +528,19 @@ 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: AsRepr,
+ Self::Backend: IoCapable<<Self::Target as AsRepr>::Repr>,
{
- Self::Backend::io_write(self.as_view(), value)
+ let view = self.as_view();
+ // SAFETY: `AsRepr` guarantees layout compatibility.
+ let repr_view = unsafe {
+ Self::Backend::project_view(
+ view,
+ Self::Backend::as_ptr(view).cast::<<Self::Target as AsRepr>::Repr>(),
+ )
+ };
+
+ Self::Backend::io_write(repr_view, Self::Target::into_repr(value))
}
/// Copy-read from I/O memory.
@@ -645,7 +662,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)
@@ -655,7 +672,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)
@@ -665,7 +682,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)
@@ -675,7 +692,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)
@@ -685,7 +702,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)
@@ -695,7 +712,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)
@@ -705,7 +722,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)
@@ -715,7 +732,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)
@@ -727,7 +744,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)
@@ -739,7 +756,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)
@@ -751,7 +768,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)
@@ -763,7 +780,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)
@@ -775,7 +792,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)
@@ -787,7 +804,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)
@@ -799,7 +816,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)
@@ -811,7 +828,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)
@@ -843,11 +860,12 @@ fn write64(self, value: u64, offset: usize)
#[inline(always)]
fn try_read<T, L>(self, location: L) -> Result<T>
where
+ T: AsReprMut,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::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.
@@ -876,12 +894,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: AsRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::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(())
}
@@ -920,9 +938,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: AsRepr,
L: IoLoc<Self::Target, T>,
V: LocatedRegister<Self::Target, Location = L, Value = T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::Repr>,
{
let (location, value) = value.into_io_op();
@@ -954,16 +973,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: AsReprMut,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::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(())
}
@@ -991,11 +1007,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: AsReprMut,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::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.
@@ -1022,12 +1039,12 @@ fn read<T, L>(self, location: L) -> T
#[inline(always)]
fn write<T, L>(self, location: L, value: T)
where
+ T: AsRepr,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::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.
@@ -1064,9 +1081,10 @@ fn write<T, L>(self, location: L, value: T)
#[inline(always)]
fn write_reg<T, L, V>(self, value: V)
where
+ T: AsRepr,
L: IoLoc<Self::Target, T>,
V: LocatedRegister<Self::Target, Location = L, Value = T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::Repr>,
{
let (location, value) = value.into_io_op();
@@ -1098,14 +1116,13 @@ fn write_reg<T, L, V>(self, value: V)
#[inline(always)]
fn update<T, L, F>(self, location: L, f: F)
where
+ T: AsReprMut,
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: IoCapable<<T as AsRepr>::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 6a19552ffb95..45d2e73a92e7 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
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index 61d076ab570a..8af12caa964b 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -200,8 +200,6 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
);
impl ::kernel::io::register::Register for #name {
- type Storage = #storage;
-
const OFFSET: usize = #offset;
}
));
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 04/16] rust: io: perform conversions using `AsRepr`
2026-08-19 11:09 ` [PATCH v3 04/16] rust: io: perform conversions using `AsRepr` Gary Guo
@ 2026-08-19 11:19 ` sashiko-bot
0 siblings, 0 replies; 33+ messages in thread
From: sashiko-bot @ 2026-08-19 11:19 UTC (permalink / raw)
To: Gary Guo; +Cc: ojeda, Alexandre Courbot, linux-pci, Danilo Krummrich, dri-devel
> 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
> performing conversions to I/O primitives via the `AsRepr`/`AsReprMut`
> trait.
>
> 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/20260819-typed_register-v3-0-3699a75fadf1@garyguo.net?part=4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 05/16] rust: io: support register projections
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (3 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 04/16] rust: io: perform conversions using `AsRepr` Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:16 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 06/16] rust: io: register: allow explicit base type specification Gary Guo
` (10 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 | 56 +++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 46 insertions(+), 10 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 6c087d30a166..ea072c098b13 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -864,8 +864,7 @@ fn try_read<T, L>(self, location: L) -> Result<T>
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as AsRepr>::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.
@@ -898,8 +897,7 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as AsRepr>::Repr>,
{
- let view = io_view::<Self, T>(self, location.offset())?;
- view.write_val(value);
+ io_write!(self, try: location, value);
Ok(())
}
@@ -978,7 +976,7 @@ fn try_update<T, L, F>(self, location: L, f: F) -> Result
Self::Backend: IoCapable<<T as AsRepr>::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(())
}
@@ -1011,8 +1009,7 @@ fn read<T, L>(self, location: L) -> T
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as AsRepr>::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.
@@ -1043,8 +1040,7 @@ fn write<T, L>(self, location: L, value: T)
L: IoLoc<Self::Target, T>,
Self::Backend: IoCapable<<T as AsRepr>::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.
@@ -1121,7 +1117,7 @@ fn update<T, L, F>(self, location: L, f: F)
Self::Backend: IoCapable<<T as AsRepr>::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()));
}
}
@@ -1666,6 +1662,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.
@@ -1693,6 +1708,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 _;
@@ -1763,6 +1793,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] 33+ messages in thread* [PATCH v3 06/16] rust: io: register: allow explicit base type specification
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (4 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 05/16] rust: io: support register projections Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:22 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 07/16] gpu: nova-core: specify base type for registers Gary Guo
` (9 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 | 51 +++++++++++++++++++++++++++++++++++++++-------
rust/macros/io/register.rs | 13 +++++++++++-
3 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index ea072c098b13..84dd876b3407 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -916,6 +916,8 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
/// };
///
/// register! {
+ /// base: Region;
+ ///
/// VERSION(u32) @ 0x100 {
/// 15:8 major;
/// 7:0 minor;
@@ -1058,6 +1060,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 45d2e73a92e7..49a5fe069e47 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;
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index 8af12caa964b..2fb48e1be82d 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -27,6 +27,7 @@
};
mod kw {
+ syn::custom_keyword!(base);
syn::custom_keyword!(stride);
}
@@ -139,16 +140,26 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
}
pub(crate) struct RegDef {
+ base: Option<Type>,
regs: Vec<Reg>,
}
impl Parse for RegDef {
fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
+ let base = if input.peek(kw::base) {
+ let _: kw::base = input.parse()?;
+ let _: Token![:] = input.parse()?;
+ let base = input.parse()?;
+ let _: Token![;] = input.parse()?;
+ Some(base)
+ } else {
+ None
+ };
let mut regs = Vec::new();
while !input.is_empty() {
regs.push(input.parse()?);
}
- Ok(RegDef { regs })
+ Ok(RegDef { base, regs })
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 06/16] rust: io: register: allow explicit base type specification
2026-08-19 11:09 ` [PATCH v3 06/16] rust: io: register: allow explicit base type specification Gary Guo
@ 2026-08-19 11:22 ` sashiko-bot
0 siblings, 0 replies; 33+ messages in thread
From: sashiko-bot @ 2026-08-19 11:22 UTC (permalink / raw)
To: Gary Guo; +Cc: dri-devel, Danilo Krummrich, ojeda, Alexandre Courbot, linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `register!` macro's parsing logic for the optional `base:` parameter creates an ambiguity that breaks parsing if the first register is named `base`.
- [Medium] Discarding the `base` parameter in the macro output causes `unused_import` warnings in updated doc tests, breaking `make rusttest`.
--
--- Patch [6]: [PATCH v3 06/16] rust: io: register: allow explicit base type specification ---
commit 35780b69bb052cc1fae4709e732645521408cd48
Author: Gary Guo <gary@garyguo.net>
rust: io: register: allow explicit base type specification
This commit modifies the register! macro to accept an optional base
parameter to specify the base type it will be operating on. Currently,
the parameter is parsed but ignored in the generated code until all
users are converted.
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index 45d2e73a92e7e..49a5fe069e47f 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>;
> +//!
[Severity: Medium]
Will this result in an unused import warning that breaks make rusttest?
Since the base parameter is parsed but entirely discarded from the emitted
token stream, rustc will never see Region being used. The kernel compiles doc
tests with -Dwarnings enabled, so this unused import could cause the doc
tests to fail.
[ ... ]
> diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
> index 8af12caa964b8..2fb48e1be82d6 100644
> --- a/rust/macros/io/register.rs
> +++ b/rust/macros/io/register.rs
[ ... ]
> @@ -139,16 +140,26 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
> }
>
> pub(crate) struct RegDef {
> + base: Option<Type>,
> regs: Vec<Reg>,
> }
>
> impl Parse for RegDef {
> fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
> + let base = if input.peek(kw::base) {
> + let _: kw::base = input.parse()?;
> + let _: Token![:] = input.parse()?;
[Severity: Medium]
Does this parsing logic break if a user names their first register base?
By only using input.peek(kw::base), the macro eagerly matches any identifier
named base at the start of the block. If a user defines a register like
base(u32) @ 0x100 (without a visibility modifier), it seems the macro will
consume base, expect a colon, and then fail with a syntax error when it
encounters the parenthesis.
Would checking for the trailing colon using input.peek2(Token![:]) prevent
this ambiguity?
> + let base = input.parse()?;
> + let _: Token![;] = input.parse()?;
> + Some(base)
> + } else {
> + None
> + };
> let mut regs = Vec::new();
> while !input.is_empty() {
> regs.push(input.parse()?);
> }
> - Ok(RegDef { regs })
> + Ok(RegDef { base, regs })
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-typed_register-v3-0-3699a75fadf1@garyguo.net?part=6
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 07/16] gpu: nova-core: specify base type for registers
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (5 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 06/16] rust: io: register: allow explicit base type specification Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:19 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 08/16] drm/tyr: " Gary Guo
` (8 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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] 33+ messages in thread* [PATCH v3 08/16] drm/tyr: specify base type for registers
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (6 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 07/16] gpu: nova-core: specify base type for registers Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:17 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 09/16] samples: rust: pci: " Gary Guo
` (7 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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/fw.rs | 5 ++++-
drivers/gpu/drm/tyr/regs.rs | 45 ++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 49 insertions(+), 2 deletions(-)
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/fw.rs b/drivers/gpu/drm/tyr/fw.rs
index 47d25c901bd0..7edb5eff1707 100644
--- a/drivers/gpu/drm/tyr/fw.rs
+++ b/drivers/gpu/drm/tyr/fw.rs
@@ -39,7 +39,8 @@
use crate::{
driver::{
IoMem,
- TyrDrmDevice, //
+ TyrDrmDevice,
+ TyrRegisters, //
},
fw::parser::{
FwParser,
@@ -101,6 +102,8 @@ fn from(value: CacheMode) -> Self {
}
register! {
+ base: TyrRegisters;
+
#[allow(non_upper_case_globals)]
pub(super) SectionFlags(u32) @ 0x0 {
0:0 read => bool;
diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
index a62724378ced..0c419c4e1186 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.
@@ -1292,6 +1323,8 @@ pub(crate) fn from_mair(mair: u64) -> Self {
}
register! {
+ base: TyrRegisters;
+
/// Lock region address for each address space.
pub(crate) LOCKADDR(u64)[MAX_AS, stride = STRIDE] @ 0x2410 {
/// Lock region size.
@@ -1353,6 +1386,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 +1515,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 +1742,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 +1799,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 +1811,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] 33+ messages in thread* Re: [PATCH v3 08/16] drm/tyr: specify base type for registers
2026-08-19 11:09 ` [PATCH v3 08/16] drm/tyr: " Gary Guo
@ 2026-08-19 11:17 ` sashiko-bot
0 siblings, 0 replies; 33+ messages in thread
From: sashiko-bot @ 2026-08-19 11:17 UTC (permalink / raw)
To: Gary Guo; +Cc: Alexandre Courbot, ojeda, dri-devel, Danilo Krummrich, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Violation of vertical import style by adding a separate import statement instead of grouping with existing imports.
--
--- Patch [8]: [PATCH v3 08/16] drm/tyr: specify base type for registers ---
commit 34aca3035b4614556abef99eebb9d34305820b18
Author: Gary Guo <gary@garyguo.net>
drm/tyr: specify base type for registers
This commit specifies the base parameter for the register definitions in the
drm/tyr driver. It introduces and utilizes TyrRegisters to standardize the
base type across the register! macro invocations.
> diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
> index a62724378ced2..0c419c4e1186c 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;
[Severity: Low]
This isn't a bug, but should this import be grouped with the existing
crate::driver imports above to follow the kernel's vertical import style
guidelines?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-typed_register-v3-0-3699a75fadf1@garyguo.net?part=8
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 09/16] samples: rust: pci: specify base type for registers
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (7 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 08/16] drm/tyr: " Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:14 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 10/16] rust: io: register: make register have a typed base Gary Guo
` (6 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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] 33+ messages in thread* [PATCH v3 10/16] rust: io: register: make register have a typed base
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (8 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 09/16] samples: rust: pci: " Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:18 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 11/16] rust: io: register: support fixed offset register without bitfield Gary Guo
` (5 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 | 55 ++++++++++++++++++++++++++++++++--------------
rust/macros/io/register.rs | 34 ++++++++++++++--------------
3 files changed, 68 insertions(+), 34 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 84dd876b3407..4542187d6b91 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -1692,21 +1692,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 49a5fe069e47..f622057f7346 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;
@@ -406,6 +407,28 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
}
}
+/// Helper function for register alias implementation.
+///
+/// This is used to enforce base matching. Only called during const eval.
+#[doc(hidden)]
+#[inline(always)]
+pub const fn alias_offset<Base: ?Sized, Alias: Register<Base = Base>>() -> usize {
+ Alias::OFFSET
+}
+
+/// Helper function for register element alias implementation.
+///
+/// This is used to enforce base matching and provide bounds checking. Only called during const
+/// eval.
+#[doc(hidden)]
+#[inline(always)]
+pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base>>(
+ idx: usize,
+) -> usize {
+ build_assert!(idx < Alias::SIZE);
+ Alias::OFFSET + idx * Alias::STRIDE
+}
+
/// Defines a dedicated type for a register, including getter and setter methods for its fields and
/// methods to read and write it from an [`Io`](kernel::io::Io) region.
///
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index 2fb48e1be82d..cb02e850b23f 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -140,21 +140,23 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
}
pub(crate) struct RegDef {
- base: Option<Type>,
+ base: Type,
regs: Vec<Reg>,
}
impl Parse for RegDef {
fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
- let base = if input.peek(kw::base) {
- let _: kw::base = input.parse()?;
- let _: Token![:] = input.parse()?;
- let base = input.parse()?;
- let _: Token![;] = input.parse()?;
- Some(base)
- } else {
- None
- };
+ if !input.peek(kw::base) {
+ Err(input.error(
+ "a base type needs to be specified for `register!` invocation with `base: ty;`",
+ ))?;
+ }
+
+ let _: kw::base = input.parse()?;
+ let _: Token![:] = input.parse()?;
+ let base = input.parse()?;
+ let _: Token![;] = input.parse()?;
+
let mut regs = Vec::new();
while !input.is_empty() {
regs.push(input.parse()?);
@@ -166,6 +168,7 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
let mut outputs = TokenStream::new();
+ let base = &def.base;
for reg in def.regs {
let Reg {
attrs,
@@ -186,18 +189,12 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
RegOffset::Fixed { offset } => quote!(#offset),
RegOffset::Alias { alias } => {
quote_spanned!(alias.span().resolved_at(span) =>
- <#alias as ::kernel::io::register::Register>::OFFSET
+ ::kernel::io::register::alias_offset::<#base, #alias>()
)
}
RegOffset::ElementAlias { alias, idx } => {
- outputs.extend(quote_spanned!(idx.span().resolved_at(span) =>
- ::kernel::build_assert::static_assert!(
- #idx < <#alias as ::kernel::io::register::RegisterArray>::SIZE
- );
- ));
quote_spanned!(alias.span().resolved_at(span) =>
- <#alias as ::kernel::io::register::Register>::OFFSET
- + #idx * <#alias as ::kernel::io::register::RegisterArray>::STRIDE
+ ::kernel::io::register::element_alias_offset::<#base, #alias>(#idx)
)
}
};
@@ -211,6 +208,7 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
);
impl ::kernel::io::register::Register for #name {
+ type Base = #base;
const OFFSET: usize = #offset;
}
));
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 11/16] rust: io: register: support fixed offset register without bitfield
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (9 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 10/16] rust: io: register: make register have a typed base Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:17 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers Gary Guo
` (4 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 `register!()` 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 | 30 ++++++++++++++++
rust/macros/io/register.rs | 86 +++++++++++++++++++++++++++++++---------------
2 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index f622057f7346..63c054162e54 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.
///
@@ -521,6 +538,19 @@ pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base
/// 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>;
+///
+/// /// UART RX register.
+/// pub UART_RX: u8 @ 0x100;
+/// }
+/// ```
+///
/// ## Relative registers
///
/// Relative registers can be instantiated several times at a relative offset of a group of bases.
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index cb02e850b23f..0b4e0d1903dd 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -14,9 +14,11 @@
bracketed,
parenthesized,
parse::Parse,
+ parse_quote,
spanned::Spanned,
token,
Attribute,
+ Error,
Expr,
Ident,
Path,
@@ -49,11 +51,11 @@ struct Reg {
attrs: Vec<Attribute>,
vis: Visibility,
name: Ident,
- storage: Type,
+ ty: Type,
array: Option<RegArrayDef>,
relative_base: Option<Path>,
offset: RegOffset,
- bitfield_args: Group,
+ bitfield: Option<(Type, Group)>,
}
impl Parse for Reg {
@@ -61,11 +63,23 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
let attrs = input.call(Attribute::parse_outer)?;
let vis = input.parse()?;
let name = input.parse()?;
- let storage = {
+
+ let lh = input.lookahead1();
+ let mut bitfield_storage = None;
+ let ty = if lh.peek(Token![:]) {
+ let _: Token![:] = input.parse()?;
+ input.parse()?
+ } else if lh.peek(token::Paren) {
let content;
parenthesized!(content in input);
- content.parse()?
+ bitfield_storage = Some(content.parse()?);
+
+ // For bitfields, bitfield macro will generate a type with the same name as `name`.
+ parse_quote!(#name)
+ } else {
+ Err(lh.error())?
};
+
let array = if input.peek(token::Bracket) {
let content;
bracketed!(content in input);
@@ -119,22 +133,28 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
Err(lh.error())?
};
- let lh = input.lookahead1();
- let bitfield_args = if lh.peek(token::Brace) {
- input.parse()?
+ let bitfield = if let Some(storage) = bitfield_storage {
+ let lh = input.lookahead1();
+ let args = if lh.peek(token::Brace) {
+ input.parse()?
+ } else {
+ Err(lh.error())?
+ };
+ Some((storage, args))
} else {
- Err(lh.error())?
+ let _: Token![;] = input.parse()?;
+ None
};
Ok(Self {
attrs,
vis,
name,
- storage,
+ ty,
array,
relative_base,
offset,
- bitfield_args,
+ bitfield,
})
}
}
@@ -174,11 +194,11 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
attrs,
vis,
name,
- storage,
+ ty,
array,
relative_base,
offset,
- bitfield_args,
+ bitfield,
} = reg;
// Use register name's span for generated code, so error messages (if any) can point to it
@@ -199,21 +219,33 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
}
};
- outputs.extend(quote_spanned!(span =>
- ::kernel::bitfield!(
- // `#[allow(non_camel_case_types)]` is added since register names typically use
- // `SCREAMING_CASE`.
- #[allow(non_camel_case_types)]
- #(#attrs)* #vis struct #name(#storage) #bitfield_args
- );
-
- impl ::kernel::io::register::Register for #name {
- type Base = #base;
- const OFFSET: usize = #offset;
- }
- ));
+ if let Some((storage, args)) = &bitfield {
+ outputs.extend(quote_spanned!(span =>
+ ::kernel::bitfield!(
+ // `#[allow(non_camel_case_types)]` is added since register names typically use
+ // `SCREAMING_CASE`.
+ #[allow(non_camel_case_types)]
+ #(#attrs)* #vis struct #name(#storage) #args
+ );
+
+ impl ::kernel::io::register::Register for #name {
+ type Base = #base;
+ const OFFSET: usize = #offset;
+ }
+ ));
+ }
match array {
+ None if bitfield.is_none() && relative_base.is_none() => outputs.extend(quote!(
+ #(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
+ ::kernel::io::register::OffsetLoc::new(#offset);
+ )),
+
+ _ if bitfield.is_none() => Err(Error::new_spanned(
+ ty,
+ "defining without bitfield is not yet supported for this type of register",
+ ))?,
+
None => match relative_base {
None => outputs.extend(quote_spanned!(span =>
impl ::kernel::io::register::FixedRegister for #name {}
@@ -235,12 +267,12 @@ impl ::kernel::io::register::RelativeRegister for #name {}
let stride = if let Some(stride) = &def.stride {
outputs.extend(quote_spanned!(stride.span().resolved_at(span) =>
::kernel::build_assert::static_assert!(
- ::core::mem::size_of::<#storage>() <= #stride
+ ::core::mem::size_of::<#ty>() <= #stride
);
));
quote!(#stride)
} else {
- quote_spanned!(span => ::core::mem::size_of::<#storage>())
+ quote_spanned!(span => ::core::mem::size_of::<#ty>())
};
outputs.extend(quote_spanned!(span =>
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (10 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 11/16] rust: io: register: support fixed offset register without bitfield Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:17 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 13/16] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
` (3 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 PFALCON2 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 | 92 ++++++------
9 files changed, 238 insertions(+), 258 deletions(-)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 65cb12d26e2b..119d305e2ea3 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -14,13 +14,12 @@
io::{
io_project,
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(())
}
@@ -547,16 +547,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_address >> 8) as u32,
- ),
- );
- self.bar.write(
- WithBase::of::<E>(),
+ ));
+ self.pfalcon.write_reg(
regs::NV_PFALCON_FALCON_DMATRFBASE1::zeroed().try_with_base(dma_address >> 40)?,
);
@@ -566,23 +563,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),
@@ -614,8 +609,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)
});
@@ -626,10 +621,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(())
}
@@ -638,7 +631,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),
@@ -649,19 +642,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(())
@@ -670,32 +657,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.
@@ -760,9 +739,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..4c96ae325fda 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(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, 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..7880c145f8f7 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(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(
+ 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(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(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(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..7fc6e83c2566 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(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 ec4d92317a93..02a36fd59902 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,
@@ -23,7 +26,6 @@
};
use crate::{
- driver::Bar0,
falcon::{
self,
gsp::Gsp,
@@ -235,12 +237,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()
@@ -250,9 +247,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 a5c0ca355493..d2ef35080689 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -62,12 +62,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),
}
}
}
@@ -88,7 +87,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.
@@ -168,7 +167,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..90ac8dac5349 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,22 +13,18 @@
};
use crate::{
- driver::{
- Bar0,
- NovaRegisters, //
- },
+ driver::NovaRegisters,
falcon::{
DmaTrfCmdSize,
FalconCoreRev,
FalconCoreRevSubversion,
- FalconEngine,
FalconFbifMemType,
FalconFbifTarget,
FalconMem,
FalconModSelAlgo,
FalconSecurityModel,
- PFalcon2Base,
- PFalconBase,
+ PFalcon2Registers,
+ PFalconRegisters,
PeregrineCoreSelect, //
},
gpu::{
@@ -202,32 +198,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 +233,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 +251,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 +270,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 +287,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 +303,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 +322,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 +355,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 +381,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(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 +401,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 +425,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] 33+ messages in thread* [PATCH v3 13/16] gpu: nova-core: convert hshub0 from relative register to projection
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (11 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:23 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 14/16] rust: io: register: remove relative registers Gary Guo
` (2 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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] 33+ messages in thread* Re: [PATCH v3 13/16] gpu: nova-core: convert hshub0 from relative register to projection
2026-08-19 11:09 ` [PATCH v3 13/16] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
@ 2026-08-19 11:23 ` sashiko-bot
0 siblings, 0 replies; 33+ messages in thread
From: sashiko-bot @ 2026-08-19 11:23 UTC (permalink / raw)
To: Gary Guo; +Cc: linux-pci, dri-devel, ojeda, Danilo Krummrich, Alexandre Courbot
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Incorrect usage of `#[inline]` annotation in a driver crate.
- [Low] Imports within the `kernel::io` block are not alphabetically sorted.
--
--- Patch [13]: [PATCH v3 13/16] gpu: nova-core: convert hshub0 from relative register to projection ---
commit f28bbcca2d960a3571d5473146fa91faad20810e
Author: Gary Guo <gary@garyguo.net>
gpu: nova-core: convert hshub0 from relative register to projection
This commit converts the hshub0 relative access to use projection and a new
base, similar to previous conversions.
> diff --git a/drivers/gpu/nova-core/fb/hal/gb100.rs b/drivers/gpu/nova-core/fb/hal/gb100.rs
> index d9e4d62ae6321..9fa0949396009 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, //
[Severity: Low]
This isn't a bug, but should these imports be alphabetically sorted?
The kernel vertical import style usually requires sorting items alphabetically,
so Io and Mmio would typically precede the lowercase entries.
[ ... ]
> @@ -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> {
[Severity: Low]
This isn't a bug, but is the #[inline] annotation needed here?
The Rust subsystem guidelines suggest reserving #[inline] for abstractions,
and specifically note that leaf crates like drivers are exempt from needing it.
> + io_project!(bar, build: HSHUB0)
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-typed_register-v3-0-3699a75fadf1@garyguo.net?part=13
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 14/16] rust: io: register: remove relative registers
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (12 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 13/16] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:23 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 15/16] rust: io: register: remove `Register` trait and cleanup macro Gary Guo
2026-08-19 11:09 ` [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 | 388 ++++++---------------------------------------
rust/macros/io/register.rs | 52 ++----
2 files changed, 61 insertions(+), 379 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 63c054162e54..4d4afd3ae94a 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`].
+//! item 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.
///
@@ -452,8 +317,7 @@ pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base
/// 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
///
@@ -551,122 +415,6 @@ pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base
/// }
/// ```
///
-/// ## 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
@@ -763,115 +511,83 @@ pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base
/// # }
/// ```
///
-/// ## 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;
-/// }
+/// // Subregion type. Make sure it has adequate size and alignment.
+/// #[repr(align(4))]
+/// #[derive(FromBytes, IntoBytes)]
+/// pub struct CpuCtl([u8; 0x100]);
///
-/// // ZST describing `CPU1`.
-/// struct Cpu1;
-/// impl RegisterBase<CpuCtlBase> for Cpu1 {
-/// const BASE: usize = 0x200;
-/// }
-///
-/// // 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]
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index 0b4e0d1903dd..757ea8094f61 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -53,7 +53,6 @@ struct Reg {
name: Ident,
ty: Type,
array: Option<RegArrayDef>,
- relative_base: Option<Path>,
offset: RegOffset,
bitfield: Option<(Type, Group)>,
}
@@ -98,26 +97,15 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
};
let lh = input.lookahead1();
- let mut relative_base = None;
let offset = if lh.peek(Token![@]) {
let _: Token![@] = input.parse()?;
- if input.peek(Ident) {
- relative_base = Some(input.parse()?);
- let _: Token![+] = input.parse()?;
- }
-
RegOffset::Fixed {
offset: input.parse()?,
}
} else if lh.peek(Token![=>]) {
let _: Token![=>] = input.parse()?;
- let mut alias: Path = input.parse()?;
- if input.peek(Token![+]) {
- let _: Token![+] = input.parse()?;
- relative_base = Some(alias);
- alias = input.parse()?;
- }
+ let alias: Path = input.parse()?;
if input.peek(token::Bracket) {
let content;
@@ -152,7 +140,6 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
name,
ty,
array,
- relative_base,
offset,
bitfield,
})
@@ -196,7 +183,6 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
name,
ty,
array,
- relative_base,
offset,
bitfield,
} = reg;
@@ -236,7 +222,7 @@ impl ::kernel::io::register::Register for #name {
}
match array {
- None if bitfield.is_none() && relative_base.is_none() => outputs.extend(quote!(
+ None if bitfield.is_none() => outputs.extend(quote!(
#(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
::kernel::io::register::OffsetLoc::new(#offset);
)),
@@ -246,21 +232,12 @@ impl ::kernel::io::register::Register for #name {
"defining without bitfield is not yet supported for this type of register",
))?,
- None => match relative_base {
- None => outputs.extend(quote_spanned!(span =>
- impl ::kernel::io::register::FixedRegister for #name {}
-
- #(#attrs)* #vis const #name: ::kernel::io::register::FixedRegisterLoc<#name> =
- ::kernel::io::register::FixedRegisterLoc::<#name>::new();
- )),
- Some(relative_base) => outputs.extend(quote_spanned!(span =>
- impl ::kernel::io::register::WithBase for #name {
- type BaseFamily = #relative_base;
- }
+ None => outputs.extend(quote_spanned!(span =>
+ impl ::kernel::io::register::FixedRegister for #name {}
- impl ::kernel::io::register::RelativeRegister for #name {}
- )),
- },
+ #(#attrs)* #vis const #name: ::kernel::io::register::FixedRegisterLoc<#name> =
+ ::kernel::io::register::FixedRegisterLoc::<#name>::new();
+ )),
Some(def) => {
let size = &def.size;
@@ -276,24 +253,13 @@ impl ::kernel::io::register::RelativeRegister for #name {}
};
outputs.extend(quote_spanned!(span =>
+ impl ::kernel::io::register::Array for #name {}
+
impl ::kernel::io::register::RegisterArray for #name {
const SIZE: usize = #size;
const STRIDE: usize = #stride;
}
));
-
- match relative_base {
- None => outputs.extend(quote_spanned!(span =>
- impl ::kernel::io::register::Array for #name {}
- )),
- Some(relative_base) => outputs.extend(quote_spanned!(span =>
- impl ::kernel::io::register::WithBase for #name {
- type BaseFamily = #relative_base;
- }
-
- impl ::kernel::io::register::RelativeRegisterArray for #name {}
- )),
- }
}
};
}
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 15/16] rust: io: register: remove `Register` trait and cleanup macro
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (13 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 14/16] rust: io: register: remove relative registers Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:21 ` sashiko-bot
2026-08-19 11:09 ` [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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 | 22 +++++++++++++---------
rust/macros/io/register.rs | 12 ++++++------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 4d4afd3ae94a..6aeee6c0a5f2 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>;
@@ -294,7 +298,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
/// This is used to enforce base matching. Only called during const eval.
#[doc(hidden)]
#[inline(always)]
-pub const fn alias_offset<Base: ?Sized, Alias: Register<Base = Base>>() -> usize {
+pub const fn alias_offset<Base: ?Sized, Alias: FixedRegister<Base = Base>>() -> usize {
Alias::OFFSET
}
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index 757ea8094f61..e0bba6b71ebc 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -213,11 +213,6 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
#[allow(non_camel_case_types)]
#(#attrs)* #vis struct #name(#storage) #args
);
-
- impl ::kernel::io::register::Register for #name {
- type Base = #base;
- const OFFSET: usize = #offset;
- }
));
}
@@ -233,7 +228,10 @@ impl ::kernel::io::register::Register for #name {
))?,
None => outputs.extend(quote_spanned!(span =>
- impl ::kernel::io::register::FixedRegister for #name {}
+ impl ::kernel::io::register::FixedRegister for #name {
+ type Base = #base;
+ const OFFSET: usize = #offset;
+ }
#(#attrs)* #vis const #name: ::kernel::io::register::FixedRegisterLoc<#name> =
::kernel::io::register::FixedRegisterLoc::<#name>::new();
@@ -256,6 +254,8 @@ impl ::kernel::io::register::FixedRegister for #name {}
impl ::kernel::io::register::Array for #name {}
impl ::kernel::io::register::RegisterArray for #name {
+ type Base = #base;
+ const OFFSET: usize = #offset;
const SIZE: usize = #size;
const STRIDE: usize = #stride;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
` (14 preceding siblings ...)
2026-08-19 11:09 ` [PATCH v3 15/16] rust: io: register: remove `Register` trait and cleanup macro Gary Guo
@ 2026-08-19 11:09 ` Gary Guo
2026-08-19 11:26 ` sashiko-bot
15 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2026-08-19 11:09 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), and expose feature to registers without inline bitfield
definition with the `#[unique]` attribute.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/io/register.rs | 110 ++++++++++++++++++++-------------------------
rust/macros/io/register.rs | 58 +++++++++++++++---------
2 files changed, 85 insertions(+), 83 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 6aeee6c0a5f2..6d4ffe6e7b5f 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>,
-{
- #[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>,
+ T: FixedIoLoc<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,26 +252,17 @@ 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)
}
}
-/// Helper function for register alias implementation.
-///
-/// This is used to enforce base matching. Only called during const eval.
-#[doc(hidden)]
-#[inline(always)]
-pub const fn alias_offset<Base: ?Sized, Alias: FixedRegister<Base = Base>>() -> usize {
- Alias::OFFSET
-}
-
/// Helper function for register element alias implementation.
///
/// This is used to enforce base matching and provide bounds checking. Only called during const
@@ -419,6 +380,31 @@ pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base
/// }
/// ```
///
+/// In case there is a fixed register associated with a specific type in the base, you can apply
+/// `#[unique]` attribute which enables `write_reg` shorthand. This is automatically applied to
+/// bitfields instantiated via the `register!` macro.
+///
+/// ```no_run
+/// # use kernel::{bitfield, io::*};
+///
+/// bitfield! {
+/// pub struct Reset(u32) {
+/// 0:0 reset;
+/// }
+/// }
+///
+/// register! {
+/// base: Region<0x1000>;
+///
+/// pub RESET: #[unique] Reset @ 0x100;
+/// }
+///
+/// # fn test(mmio: Mmio<'_, Region<0x1000>>) {
+/// // let mmio: Mmio<'_, Region<0x1000>>;
+/// mmio.write_reg(Reset::zeroed().with_const_reset::<1>());
+/// # }
+/// ```
+///
/// ## Arrays of registers
///
/// Some I/O areas contain consecutive registers that share the same field layout. These areas can
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index e0bba6b71ebc..84be1a593959 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -51,6 +51,7 @@ struct Reg {
attrs: Vec<Attribute>,
vis: Visibility,
name: Ident,
+ unique: bool,
ty: Type,
array: Option<RegArrayDef>,
offset: RegOffset,
@@ -65,16 +66,26 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
let lh = input.lookahead1();
let mut bitfield_storage = None;
- let ty = if lh.peek(Token![:]) {
+ let (unique, ty) = if lh.peek(Token![:]) {
let _: Token![:] = input.parse()?;
- input.parse()?
+
+ let mut attrs = input.call(Attribute::parse_outer)?;
+ let unique = attrs
+ .extract_if(.., |attr| attr.path().is_ident("unique"))
+ .count()
+ != 0;
+ if !attrs.is_empty() {
+ Err(Error::new_spanned(&attrs[0], "unexpected attributes"))?
+ }
+
+ (unique, input.parse()?)
} else if lh.peek(token::Paren) {
let content;
parenthesized!(content in input);
bitfield_storage = Some(content.parse()?);
// For bitfields, bitfield macro will generate a type with the same name as `name`.
- parse_quote!(#name)
+ (true, parse_quote!(#name))
} else {
Err(lh.error())?
};
@@ -138,6 +149,7 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> {
attrs,
vis,
name,
+ unique,
ty,
array,
offset,
@@ -181,6 +193,7 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
attrs,
vis,
name,
+ unique,
ty,
array,
offset,
@@ -195,7 +208,7 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
RegOffset::Fixed { offset } => quote!(#offset),
RegOffset::Alias { alias } => {
quote_spanned!(alias.span().resolved_at(span) =>
- ::kernel::io::register::alias_offset::<#base, #alias>()
+ ::kernel::io::register::OffsetLoc::<#base, _>::const_offset(#alias)
)
}
RegOffset::ElementAlias { alias, idx } => {
@@ -217,27 +230,30 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
}
match array {
- None if bitfield.is_none() => outputs.extend(quote!(
- #(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
- ::kernel::io::register::OffsetLoc::new(#offset);
- )),
-
- _ if bitfield.is_none() => Err(Error::new_spanned(
- ty,
- "defining without bitfield is not yet supported for this type of register",
- ))?,
-
- None => outputs.extend(quote_spanned!(span =>
- impl ::kernel::io::register::FixedRegister for #name {
- type Base = #base;
- const OFFSET: usize = #offset;
+ None => {
+ if unique {
+ outputs.extend(quote!(
+ impl ::kernel::io::register::FixedIoLoc<#base> for #ty {
+ type Location = ::kernel::io::register::OffsetLoc<#base, #ty>;
+ const LOCATION: Self::Location = #name;
+ }
+ ))
}
- #(#attrs)* #vis const #name: ::kernel::io::register::FixedRegisterLoc<#name> =
- ::kernel::io::register::FixedRegisterLoc::<#name>::new();
- )),
+ outputs.extend(quote_spanned!(span =>
+ #(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
+ ::kernel::io::register::OffsetLoc::new(#offset);
+ ));
+ }
Some(def) => {
+ if !unique {
+ Err(Error::new_spanned(
+ &ty,
+ "defining without bitfield is not yet supported for this type of register",
+ ))?
+ }
+
let size = &def.size;
let stride = if let Some(stride) = &def.stride {
outputs.extend(quote_spanned!(stride.span().resolved_at(span) =>
--
2.54.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields
2026-08-19 11:09 ` [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo
@ 2026-08-19 11:26 ` sashiko-bot
0 siblings, 0 replies; 33+ messages in thread
From: sashiko-bot @ 2026-08-19 11:26 UTC (permalink / raw)
To: Gary Guo; +Cc: linux-pci, Danilo Krummrich, dri-devel, Alexandre Courbot, ojeda
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `#[unique]` attribute allows implementing `FixedIoLoc` for standard primitive types (like `u32`), which silently binds all implicit type-based writes in a Base to a single specific register.
- [Low] The `register!` macro incorrectly uses the `unique` flag instead of checking for an inline bitfield to validate array registers, allowing `#[unique]` on non-bitfield arrays to bypass validation and generate invalid Rust code.
--
--- Patch [16]: [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields ---
commit 154a352ba1f637b66e6182a851b9260fee49b4d6
Author: Gary Guo <gary@garyguo.net>
rust: io: register: unify handling of register with/without bitfields
This commit moves the FixedRegister to become a property of the type by
introducing a new FixedIoLoc trait. It exposes this to registers without
inline bitfield definitions via the #[unique] attribute.
> diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
> index e0bba6b71ebc1..84be1a5939596 100644
> --- a/rust/macros/io/register.rs
> +++ b/rust/macros/io/register.rs
[ ... ]
> @@ -217,27 +230,30 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
> }
>
> match array {
> - None if bitfield.is_none() => outputs.extend(quote!(
> - #(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
> - ::kernel::io::register::OffsetLoc::new(#offset);
> - )),
> -
> - _ if bitfield.is_none() => Err(Error::new_spanned(
> - ty,
> - "defining without bitfield is not yet supported for this type of register",
> - ))?,
> -
> - None => outputs.extend(quote_spanned!(span =>
> - impl ::kernel::io::register::FixedRegister for #name {
> - type Base = #base;
> - const OFFSET: usize = #offset;
> + None => {
> + if unique {
> + outputs.extend(quote!(
> + impl ::kernel::io::register::FixedIoLoc<#base> for #ty {
> + type Location = ::kernel::io::register::OffsetLoc<#base, #ty>;
> + const LOCATION: Self::Location = #name;
> + }
> + ))
> }
[Severity: Medium]
Does this allow implementing FixedIoLoc for standard primitive types like u32?
If a register is defined with a standard primitive type and #[unique] is
applied (e.g., pub REG: #[unique] u32 @ 0x100;), this generates
impl FixedIoLoc<Base> for u32. This would silently bind all implicit
type-based writes in that Base to a single specific register.
If someone accidentally types io.write_reg(val) intending to write to another
register, and val is a u32, the compiler might silently accept it and write to
REG, bypassing the intended type-safety.
Is it possible to restrict this behavior to ensure it doesn't apply to shared
primitive types?
>
> - #(#attrs)* #vis const #name: ::kernel::io::register::FixedRegisterLoc<#name> =
> - ::kernel::io::register::FixedRegisterLoc::<#name>::new();
> - )),
> + outputs.extend(quote_spanned!(span =>
> + #(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
> + ::kernel::io::register::OffsetLoc::new(#offset);
> + ));
> + }
>
> Some(def) => {
> + if !unique {
> + Err(Error::new_spanned(
> + &ty,
> + "defining without bitfield is not yet supported for this type of register",
> + ))?
> + }
[Severity: Low]
Does this incorrectly check for the unique flag instead of an inline bitfield
to validate array registers?
For array registers, the macro must ensure a type named #name exists so it can
implement the Array trait on it. If a user defines an array register without a
bitfield but explicitly adds the #[unique] attribute, unique becomes true,
which bypasses the macro's validation check.
This would then generate impl ::kernel::io::register::Array for #name {}
without actually defining #name as a type or constant, resulting in a confusing
"cannot find type" compiler error rather than the intended descriptive macro
error.
Should this check be based on whether a bitfield is present?
> +
> let size = &def.size;
> let stride = if let Some(stride) = &def.stride {
> outputs.extend(quote_spanned!(stride.span().resolved_at(span) =>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-typed_register-v3-0-3699a75fadf1@garyguo.net?part=16
^ permalink raw reply [flat|nested] 33+ messages in thread