Joel Fernandes
2025-Sep-20 18:22 UTC
[PATCH v4 0/6] Introduce bitfield and move register macro to rust/kernel/
Hello!
These patches extract and enhance the bitfield support in the register macro in
nova to define Rust structures with bitfields. This is extremely useful as it
allows clean Rust structure definitions without requiring explicit masks and
shifts.
See [1] example code using it.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/patch/?id=76797b31facae8f1a1be139412c78568df1da9f3
v3 of the patches is at:
https://lore.kernel.org/all/20250909212039.227221-1-joelagnelf at nvidia.com/
v2 of the patches is at:
https://lore.kernel.org/all/20250903215428.1296517-1-joelagnelf at nvidia.com/
v1 of the patches is at:
https://lore.kernel.org/all/20250824135954.2243774-1-joelagnelf at nvidia.com/
v3->v4:
* Rebased on -next.
* Added more test cases.
* Added support for fields larger than the struct
(ex, using 'as u32' for a u8 struct.)
v2->v3:
* Renamed bitstruct to bitfield.
* Various suggestions to improve code (Alex, Yury, Miguel).
* Added reviewed-by tags from Elle Rhumsaa.
* Added KUNIT tests including tests for overlap.
* Added F: maintainers file entry for new files under BITOPS.
v1->v2:
* Use build_assert in bitstruct
* Split move and enhance patches for easier review
* Move out of Nova into kernel crate for other drivers like Tyr which will use.
* Miscellaneous cosmetic improvements.
Joel Fernandes (6):
nova-core: bitfield: Move bitfield-specific code from register! into
new macro
nova-core: bitfield: Add support for different storage widths
nova-core: bitfield: Add support for custom visiblity
rust: Move register and bitfield macros out of Nova
rust: Add KUNIT tests for bitfield
rust: bitfield: Use 'as' operator for setter type conversion
MAINTAINERS | 1 +
drivers/gpu/nova-core/falcon.rs | 2 +-
drivers/gpu/nova-core/falcon/gsp.rs | 4 +-
drivers/gpu/nova-core/falcon/sec2.rs | 2 +-
drivers/gpu/nova-core/regs.rs | 6 +-
rust/kernel/bits.rs | 2 +
rust/kernel/bits/bitfield.rs | 736 ++++++++++++++++++
rust/kernel/io.rs | 1 +
.../macros.rs => rust/kernel/io/register.rs | 289 +------
9 files changed, 774 insertions(+), 269 deletions(-)
create mode 100644 rust/kernel/bits/bitfield.rs
rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/io/register.rs
(73%)
--
2.34.1
Joel Fernandes
2025-Sep-20 18:23 UTC
[PATCH v4 2/6] nova-core: bitfield: Add support for different storage widths
Previously, bitfields were hardcoded to use u32 as the underlying
storage type. Add support for different storage types (u8, u16, u32,
u64) to the bitfield macro.
New syntax is: struct Name: <type ex., u32> { ... }
Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev>
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
drivers/gpu/nova-core/bitfield.rs | 71 +++++++++++++++++-----------
drivers/gpu/nova-core/regs/macros.rs | 16 +++----
2 files changed, 52 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/nova-core/bitfield.rs
b/drivers/gpu/nova-core/bitfield.rs
index ba6b7caa05d9..687ef234be75 100644
--- a/drivers/gpu/nova-core/bitfield.rs
+++ b/drivers/gpu/nova-core/bitfield.rs
@@ -51,7 +51,7 @@
//! }
//!
//! bitfield! {
-//! struct ControlReg {
+//! struct ControlReg: u32 {
//! 3:0 mode as u8 ?=> Mode;
//! 7 state as bool => State;
//! }
@@ -61,6 +61,8 @@
//! This generates a struct with:
//! - Field accessors: `mode()`, `state()`, etc.
//! - Field setters: `set_mode()`, `set_state()`, etc. (supports chaining with
builder pattern).
+//! Note that the compiler will error out if the size of the setter's arg
exceeds the
+//! struct's storage size.
//! - Debug and Default implementations
//!
//! The field setters can be used with the builder pattern, example:
@@ -77,21 +79,21 @@
//!
macro_rules! bitfield {
// Main entry point - defines the bitfield struct with fields
- (struct $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
- bitfield!(@core $name $(, $comment)? { $($fields)* });
+ (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)*
}) => {
+ bitfield!(@core $name $storage $(, $comment)? { $($fields)* });
};
// All rules below are helpers.
// Defines the wrapper `$name` type, as well as its relevant
implementations (`Debug`,
// `Default`, `BitOr`, and conversion to the value type) and field accessor
methods.
- (@core $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
+ (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* })
=> {
$(
#[doc=$comment]
)?
#[repr(transparent)]
#[derive(Clone, Copy)]
- pub(crate) struct $name(u32);
+ pub(crate) struct $name($storage);
impl ::core::ops::BitOr for $name {
type Output = Self;
@@ -101,20 +103,26 @@ fn bitor(self, rhs: Self) -> Self::Output {
}
}
- impl ::core::convert::From<$name> for u32 {
- fn from(val: $name) -> u32 {
+ impl ::core::convert::From<$name> for $storage {
+ fn from(val: $name) -> $storage {
val.0
}
}
- bitfield!(@fields_dispatcher $name { $($fields)* });
+ impl ::core::convert::From<$storage> for $name {
+ fn from(val: $storage) -> Self {
+ Self(val)
+ }
+ }
+
+ bitfield!(@fields_dispatcher $name $storage { $($fields)* });
};
// Captures the fields and passes them to all the implementers that require
field information.
//
// Used to simplify the matching rules for implementers, so they don't
need to match the entire
// complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $name:ident {
+ (@fields_dispatcher $name:ident $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -123,7 +131,7 @@ fn from(val: $name) -> u32 {
)*
}
) => {
- bitfield!(@field_accessors $name {
+ bitfield!(@field_accessors $name $storage {
$(
$hi:$lo $field as $type
$(?=> $try_into_type)?
@@ -138,7 +146,7 @@ fn from(val: $name) -> u32 {
// Defines all the field getter/setter methods for `$name`.
(
- @field_accessors $name:ident {
+ @field_accessors $name:ident $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -154,7 +162,7 @@ fn from(val: $name) -> u32 {
#[allow(dead_code)]
impl $name {
$(
- bitfield!(@field_accessor $name $hi:$lo $field as $type
+ bitfield!(@field_accessor $name $storage, $hi:$lo $field as $type
$(?=> $try_into_type)?
$(=> $into_type)?
$(, $comment)?
@@ -188,11 +196,11 @@ impl $name {
// Catches fields defined as `bool` and convert them into a boolean value.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool =>
$into_type:ty
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
bool => $into_type:ty
$(, $comment:literal)?;
) => {
bitfield!(
- @leaf_accessor $name $hi:$lo $field
+ @leaf_accessor $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(if f != 0 { true } else { false }) }
$into_type => $into_type $(, $comment)?;
);
@@ -200,17 +208,17 @@ impl $name {
// Shortcut for fields defined as `bool` without the `=>` syntax.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(,
$comment:literal)?;
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
bool $(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $hi:$lo $field as bool => bool $(,
$comment)?;);
+ bitfield!(@field_accessor $name $storage, $hi:$lo $field as bool =>
bool $(, $comment)?;);
};
// Catches the `?=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
?=> $try_into_type:ty
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
$type:tt ?=> $try_into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $hi:$lo $field
+ bitfield!(@leaf_accessor $name $storage, $hi:$lo $field
{ |f| <$try_into_type>::try_from(f as $type) } $try_into_type
=>
::core::result::Result<
$try_into_type,
@@ -221,29 +229,38 @@ impl $name {
// Catches the `=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
=> $into_type:ty
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
$type:tt => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $hi:$lo $field
+ bitfield!(@leaf_accessor $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(f as $type) } $into_type =>
$into_type $(, $comment)?;);
};
// Shortcut for non-boolean fields defined without the `=>` or `?=>`
syntax.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
$type:tt
$(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $hi:$lo $field as $type => $type $(,
$comment)?;);
+ bitfield!(@field_accessor $name $storage, $hi:$lo $field as $type =>
$type $(, $comment)?;);
};
// Generates the accessor methods for a single field.
(
- @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
+ @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
{ $process:expr } $to_type:ty => $res_type:ty $(,
$comment:literal)?;
) => {
::kernel::macros::paste!(
const [<$field:upper _RANGE>]:
::core::ops::RangeInclusive<u8> = $lo..=$hi;
- const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1)
<< 1) + 1) - ((1 << $lo) - 1);
+ const [<$field:upper _MASK>]: $storage = {
+ // Generate mask for shifting
+ match ::core::mem::size_of::<$storage>() {
+ 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
+ 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
+ 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
+ 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
+ _ => ::kernel::build_error!("Unsupported storage type
size")
+ }
+ };
const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper
_MASK>].trailing_zeros();
);
@@ -254,7 +271,7 @@ impl $name {
#[inline(always)]
pub(crate) fn $field(self) -> $res_type {
::kernel::macros::paste!(
- const MASK: u32 = $name::[<$field:upper _MASK>];
+ const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
);
let field = ((self.0 & MASK) >> SHIFT);
@@ -269,9 +286,9 @@ pub(crate) fn $field(self) -> $res_type {
)?
#[inline(always)]
pub(crate) fn [<set_ $field>](mut self, value: $to_type) ->
Self {
- const MASK: u32 = $name::[<$field:upper _MASK>];
+ const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = (u32::from(value) << SHIFT) & MASK;
+ let value = (<$storage>::from(value) << SHIFT) &
MASK;
self.0 = (self.0 & !MASK) | value;
self
diff --git a/drivers/gpu/nova-core/regs/macros.rs
b/drivers/gpu/nova-core/regs/macros.rs
index 945d15a2c529..d34c7f37fb93 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -276,25 +276,25 @@ pub(crate) trait RegisterBase<T> {
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } )
=> {
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own
fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } )
=> {
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? {
$($fields:tt)* } ) => {
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its
own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? {
$($fields:tt)* }) => {
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -305,7 +305,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -326,7 +326,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride
] ]);
};
@@ -348,7 +348,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx *
$alias::STRIDE ] );
};
@@ -357,7 +357,7 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array
alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? {
$($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(struct $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
--
2.34.1
Joel Fernandes
2025-Sep-20 18:23 UTC
[PATCH v4 1/6] nova-core: bitfield: Move bitfield-specific code from register! into new macro
The bitfield-specific into new macro. This will be used to define
structs with bitfields, similar to C language.
Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev>
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
drivers/gpu/nova-core/bitfield.rs | 314 +++++++++++++++++++++++++++
drivers/gpu/nova-core/nova_core.rs | 3 +
drivers/gpu/nova-core/regs/macros.rs | 259 +---------------------
3 files changed, 327 insertions(+), 249 deletions(-)
create mode 100644 drivers/gpu/nova-core/bitfield.rs
diff --git a/drivers/gpu/nova-core/bitfield.rs
b/drivers/gpu/nova-core/bitfield.rs
new file mode 100644
index 000000000000..ba6b7caa05d9
--- /dev/null
+++ b/drivers/gpu/nova-core/bitfield.rs
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Bitfield library for Rust structures
+//!
+//! Support for defining bitfields in Rust structures. Also used by the
[`register!`] macro.
+//!
+//! # Syntax
+//!
+//! ```rust
+//! #[derive(Debug, Clone, Copy)]
+//! enum Mode {
+//! Low = 0,
+//! High = 1,
+//! Auto = 2,
+//! }
+//!
+//! impl TryFrom<u8> for Mode {
+//! type Error = u8;
+//! fn try_from(value: u8) -> Result<Self, Self::Error> {
+//! match value {
+//! 0 => Ok(Mode::Low),
+//! 1 => Ok(Mode::High),
+//! 2 => Ok(Mode::Auto),
+//! _ => Err(value),
+//! }
+//! }
+//! }
+//!
+//! impl From<Mode> for u32 {
+//! fn from(mode: Mode) -> u32 {
+//! mode as u32
+//! }
+//! }
+//!
+//! #[derive(Debug, Clone, Copy)]
+//! enum State {
+//! Inactive = 0,
+//! Active = 1,
+//! }
+//!
+//! impl From<bool> for State {
+//! fn from(value: bool) -> Self {
+//! if value { State::Active } else { State::Inactive }
+//! }
+//! }
+//!
+//! impl From<State> for u32 {
+//! fn from(state: State) -> u32 {
+//! state as u32
+//! }
+//! }
+//!
+//! bitfield! {
+//! struct ControlReg {
+//! 3:0 mode as u8 ?=> Mode;
+//! 7 state as bool => State;
+//! }
+//! }
+//! ```
+//!
+//! This generates a struct with:
+//! - Field accessors: `mode()`, `state()`, etc.
+//! - Field setters: `set_mode()`, `set_state()`, etc. (supports chaining with
builder pattern).
+//! - Debug and Default implementations
+//!
+//! The field setters can be used with the builder pattern, example:
+//! ControlReg::default().set_mode(mode).set_state(state);
+//!
+//! Fields are defined as follows:
+//!
+//! - `as <type>` simply returns the field value casted to <type>,
typically `u32`, `u16`, `u8` or
+//! `bool`. Note that `bool` fields must have a range of 1 bit.
+//! - `as <type> => <into_type>` calls `<into_type>`'s
`From::<<type>>` implementation and returns
+//! the result.
+//! - `as <type> ?=> <try_into_type>` calls
`<try_into_type>`'s `TryFrom::<<type>>` implementation
+//! and returns the result. This is useful with fields for which not all
values are valid.
+//!
+macro_rules! bitfield {
+ // Main entry point - defines the bitfield struct with fields
+ (struct $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
+ bitfield!(@core $name $(, $comment)? { $($fields)* });
+ };
+
+ // All rules below are helpers.
+
+ // Defines the wrapper `$name` type, as well as its relevant
implementations (`Debug`,
+ // `Default`, `BitOr`, and conversion to the value type) and field accessor
methods.
+ (@core $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
+ $(
+ #[doc=$comment]
+ )?
+ #[repr(transparent)]
+ #[derive(Clone, Copy)]
+ pub(crate) struct $name(u32);
+
+ impl ::core::ops::BitOr for $name {
+ type Output = Self;
+
+ fn bitor(self, rhs: Self) -> Self::Output {
+ Self(self.0 | rhs.0)
+ }
+ }
+
+ impl ::core::convert::From<$name> for u32 {
+ fn from(val: $name) -> u32 {
+ val.0
+ }
+ }
+
+ bitfield!(@fields_dispatcher $name { $($fields)* });
+ };
+
+ // Captures the fields and passes them to all the implementers that require
field information.
+ //
+ // Used to simplify the matching rules for implementers, so they don't
need to match the entire
+ // complex fields rule even though they only make use of part of it.
+ (@fields_dispatcher $name:ident {
+ $($hi:tt:$lo:tt $field:ident as $type:tt
+ $(?=> $try_into_type:ty)?
+ $(=> $into_type:ty)?
+ $(, $comment:literal)?
+ ;
+ )*
+ }
+ ) => {
+ bitfield!(@field_accessors $name {
+ $(
+ $hi:$lo $field as $type
+ $(?=> $try_into_type)?
+ $(=> $into_type)?
+ $(, $comment)?
+ ;
+ )*
+ });
+ bitfield!(@debug $name { $($field;)* });
+ bitfield!(@default $name { $($field;)* });
+ };
+
+ // Defines all the field getter/setter methods for `$name`.
+ (
+ @field_accessors $name:ident {
+ $($hi:tt:$lo:tt $field:ident as $type:tt
+ $(?=> $try_into_type:ty)?
+ $(=> $into_type:ty)?
+ $(, $comment:literal)?
+ ;
+ )*
+ }
+ ) => {
+ $(
+ bitfield!(@check_field_bounds $hi:$lo $field as $type);
+ )*
+
+ #[allow(dead_code)]
+ impl $name {
+ $(
+ bitfield!(@field_accessor $name $hi:$lo $field as $type
+ $(?=> $try_into_type)?
+ $(=> $into_type)?
+ $(, $comment)?
+ ;
+ );
+ )*
+ }
+ };
+
+ // Boolean fields must have `$hi == $lo`.
+ (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
+ #[allow(clippy::eq_op)]
+ const _: () = {
+ ::kernel::build_assert!(
+ $hi == $lo,
+ concat!("boolean field `", stringify!($field),
"` covers more than one bit")
+ );
+ };
+ };
+
+ // Non-boolean fields must have `$hi >= $lo`.
+ (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
+ #[allow(clippy::eq_op)]
+ const _: () = {
+ ::kernel::build_assert!(
+ $hi >= $lo,
+ concat!("field `", stringify!($field), "`'s
MSB is smaller than its LSB")
+ );
+ };
+ };
+
+ // Catches fields defined as `bool` and convert them into a boolean value.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool =>
$into_type:ty
+ $(, $comment:literal)?;
+ ) => {
+ bitfield!(
+ @leaf_accessor $name $hi:$lo $field
+ { |f| <$into_type>::from(if f != 0 { true } else { false }) }
+ $into_type => $into_type $(, $comment)?;
+ );
+ };
+
+ // Shortcut for fields defined as `bool` without the `=>` syntax.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(,
$comment:literal)?;
+ ) => {
+ bitfield!(@field_accessor $name $hi:$lo $field as bool => bool $(,
$comment)?;);
+ };
+
+ // Catches the `?=>` syntax for non-boolean fields.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
?=> $try_into_type:ty
+ $(, $comment:literal)?;
+ ) => {
+ bitfield!(@leaf_accessor $name $hi:$lo $field
+ { |f| <$try_into_type>::try_from(f as $type) } $try_into_type
=>
+ ::core::result::Result<
+ $try_into_type,
+ <$try_into_type as
::core::convert::TryFrom<$type>>::Error
+ >
+ $(, $comment)?;);
+ };
+
+ // Catches the `=>` syntax for non-boolean fields.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
=> $into_type:ty
+ $(, $comment:literal)?;
+ ) => {
+ bitfield!(@leaf_accessor $name $hi:$lo $field
+ { |f| <$into_type>::from(f as $type) } $into_type =>
$into_type $(, $comment)?;);
+ };
+
+ // Shortcut for non-boolean fields defined without the `=>` or `?=>`
syntax.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
+ $(, $comment:literal)?;
+ ) => {
+ bitfield!(@field_accessor $name $hi:$lo $field as $type => $type $(,
$comment)?;);
+ };
+
+ // Generates the accessor methods for a single field.
+ (
+ @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
+ { $process:expr } $to_type:ty => $res_type:ty $(,
$comment:literal)?;
+ ) => {
+ ::kernel::macros::paste!(
+ const [<$field:upper _RANGE>]:
::core::ops::RangeInclusive<u8> = $lo..=$hi;
+ const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1)
<< 1) + 1) - ((1 << $lo) - 1);
+ const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper
_MASK>].trailing_zeros();
+ );
+
+ $(
+ #[doc="Returns the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ pub(crate) fn $field(self) -> $res_type {
+ ::kernel::macros::paste!(
+ const MASK: u32 = $name::[<$field:upper _MASK>];
+ const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
+ );
+ let field = ((self.0 & MASK) >> SHIFT);
+
+ $process(field)
+ }
+
+ ::kernel::macros::paste!(
+ $(
+ #[doc="Sets the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ pub(crate) fn [<set_ $field>](mut self, value: $to_type) ->
Self {
+ const MASK: u32 = $name::[<$field:upper _MASK>];
+ const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
+ let value = (u32::from(value) << SHIFT) & MASK;
+ self.0 = (self.0 & !MASK) | value;
+
+ self
+ }
+ );
+ };
+
+ // Generates the `Debug` implementation for `$name`.
+ (@debug $name:ident { $($field:ident;)* }) => {
+ impl ::core::fmt::Debug for $name {
+ fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>)
-> ::core::fmt::Result {
+ f.debug_struct(stringify!($name))
+ .field("<raw>",
&format_args!("{:#x}", &self.0))
+ $(
+ .field(stringify!($field), &self.$field())
+ )*
+ .finish()
+ }
+ }
+ };
+
+ // Generates the `Default` implementation for `$name`.
+ (@default $name:ident { $($field:ident;)* }) => {
+ /// Returns a value for the bitfield where all fields are set to their
default value.
+ impl ::core::default::Default for $name {
+ fn default() -> Self {
+ #[allow(unused_mut)]
+ let mut value = Self(Default::default());
+
+ ::kernel::macros::paste!(
+ $(
+ value.[<set_ $field>](Default::default());
+ )*
+ );
+
+ value
+ }
+ }
+ };
+}
diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
index fffcaee2249f..112277c7921e 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,9 @@
//! Nova Core GPU Driver
+#[macro_use]
+mod bitfield;
+
mod dma;
mod driver;
mod falcon;
diff --git a/drivers/gpu/nova-core/regs/macros.rs
b/drivers/gpu/nova-core/regs/macros.rs
index 754c14ee7f40..945d15a2c529 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -8,7 +8,8 @@
//!
//! The `register!` macro in this module provides an intuitive and readable
syntax for defining a
//! dedicated type for each register. Each such type comes with its own field
accessors that can
-//! return an error if a field's value is invalid.
+//! return an error if a field's value is invalid. Please look at the
[`bitfield`] macro for the
+//! complete syntax of fields definitions.
/// Trait providing a base address to be added to the offset of a relative
register to obtain
/// its actual offset.
@@ -54,15 +55,6 @@ pub(crate) trait RegisterBase<T> {
/// BOOT_0::alter(&bar, |r|
r.set_major_revision(3).set_minor_revision(10));
/// ```
///
-/// Fields are defined as follows:
-///
-/// - `as <type>` simply returns the field value casted to <type>,
typically `u32`, `u16`, `u8` or
-/// `bool`. Note that `bool` fields must have a range of 1 bit.
-/// - `as <type> => <into_type>` calls `<into_type>`'s
`From::<<type>>` implementation and returns
-/// the result.
-/// - `as <type> ?=> <try_into_type>` calls
`<try_into_type>`'s `TryFrom::<<type>>` implementation
-/// and returns the result. This is useful with fields for which not all
values are valid.
-///
/// The documentation strings are optional. If present, they will be added to
the type's
/// definition, or the field getter and setter methods they are attached to.
///
@@ -284,25 +276,25 @@ pub(crate) trait RegisterBase<T> {
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } )
=> {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own
fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } )
=> {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? {
$($fields:tt)* } ) => {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its
own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? {
$($fields:tt)* }) => {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -313,7 +305,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -334,7 +326,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride
] ]);
};
@@ -356,7 +348,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx *
$alias::STRIDE ] );
};
@@ -365,241 +357,10 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array
alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? {
$($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitfield!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
- // All rules below are helpers.
-
- // Defines the wrapper `$name` type, as well as its relevant
implementations (`Debug`,
- // `Default`, `BitOr`, and conversion to the value type) and field accessor
methods.
- (@core $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
- $(
- #[doc=$comment]
- )?
- #[repr(transparent)]
- #[derive(Clone, Copy)]
- pub(crate) struct $name(u32);
-
- impl ::core::ops::BitOr for $name {
- type Output = Self;
-
- fn bitor(self, rhs: Self) -> Self::Output {
- Self(self.0 | rhs.0)
- }
- }
-
- impl ::core::convert::From<$name> for u32 {
- fn from(reg: $name) -> u32 {
- reg.0
- }
- }
-
- register!(@fields_dispatcher $name { $($fields)* });
- };
-
- // Captures the fields and passes them to all the implementers that require
field information.
- //
- // Used to simplify the matching rules for implementers, so they don't
need to match the entire
- // complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $name:ident {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- register!(@field_accessors $name {
- $(
- $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- )*
- });
- register!(@debug $name { $($field;)* });
- register!(@default $name { $($field;)* });
- };
-
- // Defines all the field getter/methods methods for `$name`.
- (
- @field_accessors $name:ident {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- $(
- register!(@check_field_bounds $hi:$lo $field as $type);
- )*
-
- #[allow(dead_code)]
- impl $name {
- $(
- register!(@field_accessor $name $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- );
- )*
- }
- };
-
- // Boolean fields must have `$hi == $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert!(
- $hi == $lo,
- concat!("boolean field `", stringify!($field),
"` covers more than one bit")
- );
- };
- };
-
- // Non-boolean fields must have `$hi >= $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert!(
- $hi >= $lo,
- concat!("field `", stringify!($field), "`'s
MSB is smaller than its LSB")
- );
- };
- };
-
- // Catches fields defined as `bool` and convert them into a boolean value.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool =>
$into_type:ty
- $(, $comment:literal)?;
- ) => {
- register!(
- @leaf_accessor $name $hi:$lo $field
- { |f| <$into_type>::from(if f != 0 { true } else { false }) }
- $into_type => $into_type $(, $comment)?;
- );
- };
-
- // Shortcut for fields defined as `bool` without the `=>` syntax.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(,
$comment:literal)?;
- ) => {
- register!(@field_accessor $name $hi:$lo $field as bool => bool $(,
$comment)?;);
- };
-
- // Catches the `?=>` syntax for non-boolean fields.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
?=> $try_into_type:ty
- $(, $comment:literal)?;
- ) => {
- register!(@leaf_accessor $name $hi:$lo $field
- { |f| <$try_into_type>::try_from(f as $type) } $try_into_type
=>
- ::core::result::Result<
- $try_into_type,
- <$try_into_type as
::core::convert::TryFrom<$type>>::Error
- >
- $(, $comment)?;);
- };
-
- // Catches the `=>` syntax for non-boolean fields.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
=> $into_type:ty
- $(, $comment:literal)?;
- ) => {
- register!(@leaf_accessor $name $hi:$lo $field
- { |f| <$into_type>::from(f as $type) } $into_type =>
$into_type $(, $comment)?;);
- };
-
- // Shortcut for non-boolean fields defined without the `=>` or `?=>`
syntax.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
- $(, $comment:literal)?;
- ) => {
- register!(@field_accessor $name $hi:$lo $field as $type => $type $(,
$comment)?;);
- };
-
- // Generates the accessor methods for a single field.
- (
- @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
- { $process:expr } $to_type:ty => $res_type:ty $(,
$comment:literal)?;
- ) => {
- ::kernel::macros::paste!(
- const [<$field:upper _RANGE>]:
::core::ops::RangeInclusive<u8> = $lo..=$hi;
- const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1)
<< 1) + 1) - ((1 << $lo) - 1);
- const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper
_MASK>].trailing_zeros();
- );
-
- $(
- #[doc="Returns the value of this field:"]
- #[doc=$comment]
- )?
- #[inline(always)]
- pub(crate) fn $field(self) -> $res_type {
- ::kernel::macros::paste!(
- const MASK: u32 = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- );
- let field = ((self.0 & MASK) >> SHIFT);
-
- $process(field)
- }
-
- ::kernel::macros::paste!(
- $(
- #[doc="Sets the value of this field:"]
- #[doc=$comment]
- )?
- #[inline(always)]
- pub(crate) fn [<set_ $field>](mut self, value: $to_type) ->
Self {
- const MASK: u32 = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = (u32::from(value) << SHIFT) & MASK;
- self.0 = (self.0 & !MASK) | value;
-
- self
- }
- );
- };
-
- // Generates the `Debug` implementation for `$name`.
- (@debug $name:ident { $($field:ident;)* }) => {
- impl ::core::fmt::Debug for $name {
- fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>)
-> ::core::fmt::Result {
- f.debug_struct(stringify!($name))
- .field("<raw>",
&format_args!("{:#x}", &self.0))
- $(
- .field(stringify!($field), &self.$field())
- )*
- .finish()
- }
- }
- };
-
- // Generates the `Default` implementation for `$name`.
- (@default $name:ident { $($field:ident;)* }) => {
- /// Returns a value for the register where all fields are set to their
default value.
- impl ::core::default::Default for $name {
- fn default() -> Self {
- #[allow(unused_mut)]
- let mut value = Self(Default::default());
-
- ::kernel::macros::paste!(
- $(
- value.[<set_ $field>](Default::default());
- )*
- );
-
- value
- }
- }
- };
-
// Generates the IO accessors for a fixed offset register.
(@io_fixed $name:ident @ $offset:expr) => {
#[allow(dead_code)]
--
2.34.1
Joel Fernandes
2025-Sep-20 18:23 UTC
[PATCH v4 3/6] nova-core: bitfield: Add support for custom visiblity
Add support for custom visiblity to allow for users to control visibility
of the structure and helpers.
Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev>
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
drivers/gpu/nova-core/bitfield.rs | 55 ++++++++++++++++------------
drivers/gpu/nova-core/regs/macros.rs | 16 ++++----
2 files changed, 40 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/nova-core/bitfield.rs
b/drivers/gpu/nova-core/bitfield.rs
index 687ef234be75..9c022fc2bd6a 100644
--- a/drivers/gpu/nova-core/bitfield.rs
+++ b/drivers/gpu/nova-core/bitfield.rs
@@ -51,7 +51,7 @@
//! }
//!
//! bitfield! {
-//! struct ControlReg: u32 {
+//! pub struct ControlReg: u32 {
//! 3:0 mode as u8 ?=> Mode;
//! 7 state as bool => State;
//! }
@@ -65,6 +65,9 @@
//! struct's storage size.
//! - Debug and Default implementations
//!
+//! Note: Field accessors and setters inherit the same visibility as the struct
itself.
+//! In the example above, both `mode()` and `set_mode()` methods will be `pub`.
+//!
//! The field setters can be used with the builder pattern, example:
//! ControlReg::default().set_mode(mode).set_state(state);
//!
@@ -79,21 +82,21 @@
//!
macro_rules! bitfield {
// Main entry point - defines the bitfield struct with fields
- (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)*
}) => {
- bitfield!(@core $name $storage $(, $comment)? { $($fields)* });
+ ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? {
$($fields:tt)* }) => {
+ bitfield!(@core $vis $name $storage $(, $comment)? { $($fields)* });
};
// All rules below are helpers.
// Defines the wrapper `$name` type, as well as its relevant
implementations (`Debug`,
// `Default`, `BitOr`, and conversion to the value type) and field accessor
methods.
- (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* })
=> {
+ (@core $vis:vis $name:ident $storage:ty $(, $comment:literal)? {
$($fields:tt)* }) => {
$(
#[doc=$comment]
)?
#[repr(transparent)]
#[derive(Clone, Copy)]
- pub(crate) struct $name($storage);
+ $vis struct $name($storage);
impl ::core::ops::BitOr for $name {
type Output = Self;
@@ -115,14 +118,14 @@ fn from(val: $storage) -> Self {
}
}
- bitfield!(@fields_dispatcher $name $storage { $($fields)* });
+ bitfield!(@fields_dispatcher $vis $name $storage { $($fields)* });
};
// Captures the fields and passes them to all the implementers that require
field information.
//
// Used to simplify the matching rules for implementers, so they don't
need to match the entire
// complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $name:ident $storage:ty {
+ (@fields_dispatcher $vis:vis $name:ident $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -131,7 +134,7 @@ fn from(val: $storage) -> Self {
)*
}
) => {
- bitfield!(@field_accessors $name $storage {
+ bitfield!(@field_accessors $vis $name $storage {
$(
$hi:$lo $field as $type
$(?=> $try_into_type)?
@@ -146,7 +149,7 @@ fn from(val: $storage) -> Self {
// Defines all the field getter/setter methods for `$name`.
(
- @field_accessors $name:ident $storage:ty {
+ @field_accessors $vis:vis $name:ident $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -161,8 +164,14 @@ fn from(val: $storage) -> Self {
#[allow(dead_code)]
impl $name {
+ /// Returns the raw underlying value
+ #[inline(always)]
+ $vis fn raw(&self) -> $storage {
+ self.0
+ }
+
$(
- bitfield!(@field_accessor $name $storage, $hi:$lo $field as $type
+ bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as
$type
$(?=> $try_into_type)?
$(=> $into_type)?
$(, $comment)?
@@ -196,11 +205,11 @@ impl $name {
// Catches fields defined as `bool` and convert them into a boolean value.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
bool => $into_type:ty
+ @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as bool => $into_type:ty
$(, $comment:literal)?;
) => {
bitfield!(
- @leaf_accessor $name $storage, $hi:$lo $field
+ @leaf_accessor $vis $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(if f != 0 { true } else { false }) }
$into_type => $into_type $(, $comment)?;
);
@@ -208,17 +217,17 @@ impl $name {
// Shortcut for fields defined as `bool` without the `=>` syntax.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
bool $(, $comment:literal)?;
+ @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as bool $(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $storage, $hi:$lo $field as bool =>
bool $(, $comment)?;);
+ bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as bool
=> bool $(, $comment)?;);
};
// Catches the `?=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
$type:tt ?=> $try_into_type:ty
+ @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as $type:tt ?=> $try_into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $storage, $hi:$lo $field
+ bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
{ |f| <$try_into_type>::try_from(f as $type) } $try_into_type
=>
::core::result::Result<
$try_into_type,
@@ -229,24 +238,24 @@ impl $name {
// Catches the `=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
$type:tt => $into_type:ty
+ @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as $type:tt => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $storage, $hi:$lo $field
+ bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(f as $type) } $into_type =>
$into_type $(, $comment)?;);
};
// Shortcut for non-boolean fields defined without the `=>` or `?=>`
syntax.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as
$type:tt
+ @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as $type:tt
$(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $storage, $hi:$lo $field as $type =>
$type $(, $comment)?;);
+ bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as $type
=> $type $(, $comment)?;);
};
// Generates the accessor methods for a single field.
(
- @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
+ @leaf_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident
{ $process:expr } $to_type:ty => $res_type:ty $(,
$comment:literal)?;
) => {
::kernel::macros::paste!(
@@ -269,7 +278,7 @@ impl $name {
#[doc=$comment]
)?
#[inline(always)]
- pub(crate) fn $field(self) -> $res_type {
+ $vis fn $field(self) -> $res_type {
::kernel::macros::paste!(
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
@@ -285,7 +294,7 @@ pub(crate) fn $field(self) -> $res_type {
#[doc=$comment]
)?
#[inline(always)]
- pub(crate) fn [<set_ $field>](mut self, value: $to_type) ->
Self {
+ $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
let value = (<$storage>::from(value) << SHIFT) &
MASK;
diff --git a/drivers/gpu/nova-core/regs/macros.rs
b/drivers/gpu/nova-core/regs/macros.rs
index d34c7f37fb93..6a4f3271beb3 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -276,25 +276,25 @@ pub(crate) trait RegisterBase<T> {
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } )
=> {
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own
fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } )
=> {
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? {
$($fields:tt)* } ) => {
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its
own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? {
$($fields:tt)* }) => {
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -305,7 +305,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -326,7 +326,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride
] ]);
};
@@ -348,7 +348,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx *
$alias::STRIDE ] );
};
@@ -357,7 +357,7 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array
alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? {
$($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
--
2.34.1
Joel Fernandes
2025-Sep-20 18:23 UTC
[PATCH v4 6/6] rust: bitfield: Use 'as' operator for setter type conversion
The bitfield macro's setter currently uses the From trait for type
conversion, which is overly restrictive and prevents use cases such as
narrowing conversions (e.g., u32 storage size to u8 field size) which
aren't supported by From.
Replace 'from' with 'as' in the setter implementation to support
this.
Suggested-by: Yury Norov <yury.norov at gmail.com>
Signed-off-by: Joel Fernandes <joel at joelfernandes.org>
---
rust/kernel/bits/bitfield.rs | 46 +++++++++++++++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/bits/bitfield.rs b/rust/kernel/bits/bitfield.rs
index 99e443580b36..6342352891fa 100644
--- a/rust/kernel/bits/bitfield.rs
+++ b/rust/kernel/bits/bitfield.rs
@@ -300,7 +300,7 @@ impl $name {
$vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = (<$storage>::from(value) << SHIFT) &
MASK;
+ let value = ((value as $storage) << SHIFT) & MASK;
self.0 = (self.0 & !MASK) | value;
self
@@ -452,6 +452,15 @@ struct TestPartialBits: u8 {
}
}
+ // For testing wide field types on narrow storage
+ bitfield! {
+ struct TestWideFields: u8 {
+ 3:0 nibble as u32;
+ 7:4 high_nibble as u32;
+ 7:0 full as u64;
+ }
+ }
+
#[test]
fn test_single_bits() {
let mut pte = TestPageTableEntry::default();
@@ -689,4 +698,39 @@ fn test_partial_bitfield() {
bf2 = bf2.set_state(0x55);
assert_eq!(bf2.state(), 0x1);
}
+
+ #[test]
+ fn test_wide_field_types() {
+ let mut wf = TestWideFields::default();
+
+ wf = wf.set_nibble(0x0000000F_u32);
+ assert_eq!(wf.nibble(), 0x0000000F_u32);
+
+ wf = wf.set_high_nibble(0x00000007_u32);
+ assert_eq!(wf.high_nibble(), 0x00000007_u32);
+
+ wf = wf.set_nibble(0xDEADBEEF_u32);
+ assert_eq!(wf.nibble(), 0x0000000F_u32);
+
+ wf = wf.set_high_nibble(0xCAFEBABE_u32);
+ assert_eq!(wf.high_nibble(), 0x0000000E_u32);
+
+ wf = wf.set_full(0xDEADBEEFCAFEBABE_u64);
+ assert_eq!(wf.full(), 0xBE_u64);
+
+ assert_eq!(wf.raw(), 0xBE_u8);
+
+ wf = TestWideFields::default()
+ .set_nibble(0x5_u32)
+ .set_high_nibble(0xA_u32);
+ assert_eq!(wf.raw(), 0xA5_u8);
+ assert_eq!(wf.nibble(), 0x5_u32);
+ assert_eq!(wf.high_nibble(), 0xA_u32);
+
+ // Test builder pattern
+ let wf2 = TestWideFields::default()
+ .set_nibble(0x12345678_u32) // truncated to 0x8
+ .set_high_nibble(0x9ABCDEF0_u32); // truncated to 0x0
+ assert_eq!(wf2.raw(), 0x08_u8);
+ }
}
--
2.34.1
Add KUNIT tests to make sure the macro is working correctly.
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
rust/kernel/bits/bitfield.rs | 349 +++++++++++++++++++++++++++++++++++
1 file changed, 349 insertions(+)
diff --git a/rust/kernel/bits/bitfield.rs b/rust/kernel/bits/bitfield.rs
index 6089f3cdbd1b..99e443580b36 100644
--- a/rust/kernel/bits/bitfield.rs
+++ b/rust/kernel/bits/bitfield.rs
@@ -341,3 +341,352 @@ fn default() -> Self {
}
};
}
+
+#[::kernel::macros::kunit_tests(kernel_bitfield)]
+mod tests {
+ use core::convert::TryFrom;
+
+ // Enum types for testing => and ?=> conversions
+ #[derive(Debug, Clone, Copy, PartialEq)]
+ enum MemoryType {
+ Unmapped = 0,
+ Normal = 1,
+ Device = 2,
+ Reserved = 3,
+ }
+
+ impl Default for MemoryType {
+ fn default() -> Self {
+ MemoryType::Unmapped
+ }
+ }
+
+ impl TryFrom<u8> for MemoryType {
+ type Error = u8;
+ fn try_from(value: u8) -> Result<Self, Self::Error> {
+ match value {
+ 0 => Ok(MemoryType::Unmapped),
+ 1 => Ok(MemoryType::Normal),
+ 2 => Ok(MemoryType::Device),
+ 3 => Ok(MemoryType::Reserved),
+ _ => Err(value),
+ }
+ }
+ }
+
+ impl From<MemoryType> for u64 {
+ fn from(mt: MemoryType) -> u64 {
+ mt as u64
+ }
+ }
+
+ #[derive(Debug, Clone, Copy, PartialEq)]
+ enum Priority {
+ Low = 0,
+ Medium = 1,
+ High = 2,
+ Critical = 3,
+ }
+
+ impl Default for Priority {
+ fn default() -> Self {
+ Priority::Low
+ }
+ }
+
+ impl From<u8> for Priority {
+ fn from(value: u8) -> Self {
+ match value & 0x3 {
+ 0 => Priority::Low,
+ 1 => Priority::Medium,
+ 2 => Priority::High,
+ _ => Priority::Critical,
+ }
+ }
+ }
+
+ impl From<Priority> for u16 {
+ fn from(p: Priority) -> u16 {
+ p as u16
+ }
+ }
+
+ bitfield! {
+ struct TestPageTableEntry: u64 {
+ 0:0 present as bool;
+ 1:1 writable as bool;
+ 11:9 available as u8;
+ 13:12 mem_type as u8 ?=> MemoryType;
+ 17:14 extended_type as u8 ?=> MemoryType; // For testing
failures
+ 51:12 pfn as u64;
+ 51:12 pfn_overlap as u64;
+ 61:52 available2 as u16;
+ }
+ }
+
+ bitfield! {
+ struct TestControlRegister: u16 {
+ 0:0 enable as bool;
+ 3:1 mode as u8;
+ 5:4 priority as u8 => Priority;
+ 7:4 priority_nibble as u8;
+ 15:8 channel as u8;
+ }
+ }
+
+ bitfield! {
+ struct TestStatusRegister: u8 {
+ 0:0 ready as bool;
+ 1:1 error as bool;
+ 3:2 state as u8;
+ 7:4 reserved as u8;
+ 7:0 full_byte as u8; // For entire register
+ }
+ }
+
+ bitfield! {
+ struct TestPartialBits: u8 {
+ 0:0 ready as bool;
+ 1:1 error as bool;
+ 3:2 state as u8;
+ }
+ }
+
+ #[test]
+ fn test_single_bits() {
+ let mut pte = TestPageTableEntry::default();
+
+ assert!(!pte.present());
+ assert!(!pte.writable());
+
+ pte = pte.set_present(true);
+ assert!(pte.present());
+
+ pte = pte.set_writable(true);
+ assert!(pte.writable());
+
+ pte = pte.set_writable(false);
+ assert!(!pte.writable());
+
+ assert_eq!(pte.available(), 0);
+ pte = pte.set_available(0x5);
+ assert_eq!(pte.available(), 0x5);
+ }
+
+ #[test]
+ fn test_range_fields() {
+ let mut pte = TestPageTableEntry::default();
+
+ pte = pte.set_pfn(0x123456);
+ assert_eq!(pte.pfn(), 0x123456);
+ // Test overlapping field reads same value
+ assert_eq!(pte.pfn_overlap(), 0x123456);
+
+ pte = pte.set_available(0x7);
+ assert_eq!(pte.available(), 0x7);
+
+ pte = pte.set_available2(0x3FF);
+ assert_eq!(pte.available2(), 0x3FF);
+
+ // Test TryFrom with ?=> for MemoryType
+ pte = pte.set_mem_type(MemoryType::Device);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Device));
+
+ pte = pte.set_mem_type(MemoryType::Normal);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Normal));
+
+ // Test all valid values for mem_type
+ pte = pte.set_mem_type(MemoryType::Reserved); // Valid value: 3
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+
+ // Test failure case using extended_type field which has 4 bits (0-15)
+ // MemoryType only handles 0-3, so values 4-15 should return Err
+ let mut raw = pte.raw();
+ // Set bits 17:14 to 7 (invalid for MemoryType)
+ raw = (raw & !::kernel::bits::genmask_u64(14..=17)) | (0x7 <<
14);
+ let invalid_pte = TestPageTableEntry::from(raw);
+ // Should return Err with the invalid value
+ assert_eq!(invalid_pte.extended_type(), Err(0x7));
+
+ // Test a valid value after testing invalid to ensure both cases work
+ // Set bits 17:14 to 2 (valid: Device)
+ raw = (raw & !::kernel::bits::genmask_u64(14..=17)) | (0x2 <<
14);
+ let valid_pte = TestPageTableEntry::from(raw);
+ assert_eq!(valid_pte.extended_type(), Ok(MemoryType::Device));
+
+ let max_pfn = ::kernel::bits::genmask_u64(0..=39);
+ pte = pte.set_pfn(max_pfn);
+ assert_eq!(pte.pfn(), max_pfn);
+ assert_eq!(pte.pfn_overlap(), max_pfn);
+ }
+
+ #[test]
+ fn test_builder_pattern() {
+ let pte = TestPageTableEntry::default()
+ .set_present(true)
+ .set_writable(true)
+ .set_available(0x7)
+ .set_pfn(0xABCDEF)
+ .set_mem_type(MemoryType::Reserved)
+ .set_available2(0x3FF);
+
+ assert!(pte.present());
+ assert!(pte.writable());
+ assert_eq!(pte.available(), 0x7);
+ assert_eq!(pte.pfn(), 0xABCDEF);
+ assert_eq!(pte.pfn_overlap(), 0xABCDEF);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+ assert_eq!(pte.available2(), 0x3FF);
+ }
+
+ #[test]
+ fn test_raw_operations() {
+ let raw_value = 0x3FF0000003123E03u64;
+
+ // Test using ::from() syntax
+ let pte = TestPageTableEntry::from(raw_value);
+ assert_eq!(pte.raw(), raw_value);
+
+ assert!(pte.present());
+ assert!(pte.writable());
+ assert_eq!(pte.available(), 0x7);
+ assert_eq!(pte.pfn(), 0x3123);
+ assert_eq!(pte.pfn_overlap(), 0x3123);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+ assert_eq!(pte.available2(), 0x3FF);
+
+ // Test using direct constructor syntax TestStruct(value)
+ let pte2 = TestPageTableEntry(raw_value);
+ assert_eq!(pte2.raw(), raw_value);
+ }
+
+ #[test]
+ fn test_u16_bitfield() {
+ let mut ctrl = TestControlRegister::default();
+
+ assert!(!ctrl.enable());
+ assert_eq!(ctrl.mode(), 0);
+ assert_eq!(ctrl.priority(), Priority::Low);
+ assert_eq!(ctrl.priority_nibble(), 0);
+ assert_eq!(ctrl.channel(), 0);
+
+ ctrl = ctrl.set_enable(true);
+ assert!(ctrl.enable());
+
+ ctrl = ctrl.set_mode(0x5);
+ assert_eq!(ctrl.mode(), 0x5);
+
+ // Test From conversion with =>
+ ctrl = ctrl.set_priority(Priority::High);
+ assert_eq!(ctrl.priority(), Priority::High);
+ assert_eq!(ctrl.priority_nibble(), 0x2); // High = 2 in bits 5:4
+
+ ctrl = ctrl.set_channel(0xAB);
+ assert_eq!(ctrl.channel(), 0xAB);
+
+ // Test overlapping fields
+ ctrl = ctrl.set_priority_nibble(0xF);
+ assert_eq!(ctrl.priority_nibble(), 0xF);
+ assert_eq!(ctrl.priority(), Priority::Critical); // bits 5:4 = 0x3
+
+ let ctrl2 = TestControlRegister::default()
+ .set_enable(true)
+ .set_mode(0x3)
+ .set_priority(Priority::Medium)
+ .set_channel(0x42);
+
+ assert!(ctrl2.enable());
+ assert_eq!(ctrl2.mode(), 0x3);
+ assert_eq!(ctrl2.priority(), Priority::Medium);
+ assert_eq!(ctrl2.channel(), 0x42);
+
+ let raw_value: u16 = 0x4217;
+ let ctrl3 = TestControlRegister::from(raw_value);
+ assert_eq!(ctrl3.raw(), raw_value);
+ assert!(ctrl3.enable());
+ assert_eq!(ctrl3.priority(), Priority::Medium);
+ assert_eq!(ctrl3.priority_nibble(), 0x1);
+ assert_eq!(ctrl3.channel(), 0x42);
+ }
+
+ #[test]
+ fn test_u8_bitfield() {
+ let mut status = TestStatusRegister::default();
+
+ assert!(!status.ready());
+ assert!(!status.error());
+ assert_eq!(status.state(), 0);
+ assert_eq!(status.reserved(), 0);
+ assert_eq!(status.full_byte(), 0);
+
+ status = status.set_ready(true);
+ assert!(status.ready());
+ assert_eq!(status.full_byte(), 0x01);
+
+ status = status.set_error(true);
+ assert!(status.error());
+ assert_eq!(status.full_byte(), 0x03);
+
+ status = status.set_state(0x3);
+ assert_eq!(status.state(), 0x3);
+ assert_eq!(status.full_byte(), 0x0F);
+
+ status = status.set_reserved(0xA);
+ assert_eq!(status.reserved(), 0xA);
+ assert_eq!(status.full_byte(), 0xAF);
+
+ // Test overlapping field
+ status = status.set_full_byte(0x55);
+ assert_eq!(status.full_byte(), 0x55);
+ assert!(status.ready());
+ assert!(!status.error());
+ assert_eq!(status.state(), 0x1);
+ assert_eq!(status.reserved(), 0x5);
+
+ let status2 = TestStatusRegister::default()
+ .set_ready(true)
+ .set_state(0x2)
+ .set_reserved(0x5);
+
+ assert!(status2.ready());
+ assert!(!status2.error());
+ assert_eq!(status2.state(), 0x2);
+ assert_eq!(status2.reserved(), 0x5);
+ assert_eq!(status2.full_byte(), 0x59);
+
+ let raw_value: u8 = 0x59;
+ let status3 = TestStatusRegister::from(raw_value);
+ assert_eq!(status3.raw(), raw_value);
+ assert!(status3.ready());
+ assert!(!status3.error());
+ assert_eq!(status3.state(), 0x2);
+ assert_eq!(status3.reserved(), 0x5);
+ assert_eq!(status3.full_byte(), 0x59);
+
+ let status4 = TestStatusRegister::from(0xFF);
+ assert!(status4.ready());
+ assert!(status4.error());
+ assert_eq!(status4.state(), 0x3);
+ assert_eq!(status4.reserved(), 0xF);
+ assert_eq!(status4.full_byte(), 0xFF);
+ }
+
+ #[test]
+ fn test_partial_bitfield() {
+ // Test creating a bitfield from a runtime value that has bits set
+ // beyond what the defined fields can handle.
+ let raw_value: u8 = 0xff; // All bits set
+ let bf = TestPartialBits::from(raw_value);
+ assert_eq!(bf.raw(), 0xff);
+
+ // Test individual field extraction from the runtime value
+ assert!(bf.ready());
+ assert!(bf.error());
+ assert_eq!(bf.state(), 3); // bits 3:2 are both set
+
+ // Test overflow of setters
+ let mut bf2 = TestPartialBits::default();
+ bf2 = bf2.set_state(0x55);
+ assert_eq!(bf2.state(), 0x1);
+ }
+}
--
2.34.1
Joel Fernandes
2025-Sep-20 18:23 UTC
[PATCH v4 4/6] rust: Move register and bitfield macros out of Nova
Out of broad need for these macros in Rust, move them out. Several folks
have shown interest (Nova, Tyr GPU drivers).
bitfield moved into bits modules - defines bitfields in Rust structs similar to
C.
register moved into io module - defines hardware registers and accessors.
Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev>
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
MAINTAINERS | 1 +
drivers/gpu/nova-core/falcon.rs | 2 +-
drivers/gpu/nova-core/falcon/gsp.rs | 4 +-
drivers/gpu/nova-core/falcon/sec2.rs | 2 +-
drivers/gpu/nova-core/nova_core.rs | 3 --
drivers/gpu/nova-core/regs.rs | 6 +--
rust/kernel/bits.rs | 2 +
.../kernel/bits}/bitfield.rs | 27 ++++++-----
rust/kernel/io.rs | 1 +
.../macros.rs => rust/kernel/io/register.rs | 46 ++++++++++---------
10 files changed, 51 insertions(+), 43 deletions(-)
rename {drivers/gpu/nova-core => rust/kernel/bits}/bitfield.rs (91%)
rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/io/register.rs
(93%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 841b76234045..a94af8607b6b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4379,6 +4379,7 @@ F: include/asm-generic/bitops.h
F: include/linux/bitops.h
F: lib/test_bitops.c
F: tools/*/bitops*
+F: rust/kernel/bits*
BITOPS API BINDINGS [RUST]
M: Yury Norov <yury.norov at gmail.com>
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 37e6298195e4..a15fa98c8614 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -6,6 +6,7 @@
use hal::FalconHal;
use kernel::device;
use kernel::dma::DmaAddress;
+use kernel::io::register::RegisterBase;
use kernel::prelude::*;
use kernel::sync::aref::ARef;
use kernel::time::Delta;
@@ -14,7 +15,6 @@
use crate::driver::Bar0;
use crate::gpu::Chipset;
use crate::regs;
-use crate::regs::macros::RegisterBase;
use crate::util;
pub(crate) mod gsp;
diff --git a/drivers/gpu/nova-core/falcon/gsp.rs
b/drivers/gpu/nova-core/falcon/gsp.rs
index f17599cb49fa..cd4960e997c8 100644
--- a/drivers/gpu/nova-core/falcon/gsp.rs
+++ b/drivers/gpu/nova-core/falcon/gsp.rs
@@ -1,9 +1,11 @@
// SPDX-License-Identifier: GPL-2.0
+use kernel::io::register::RegisterBase;
+
use crate::{
driver::Bar0,
falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase},
- regs::{self, macros::RegisterBase},
+ regs::self,
};
/// Type specifying the `Gsp` falcon engine. Cannot be instantiated.
diff --git a/drivers/gpu/nova-core/falcon/sec2.rs
b/drivers/gpu/nova-core/falcon/sec2.rs
index 815786c8480d..81717868a8a8 100644
--- a/drivers/gpu/nova-core/falcon/sec2.rs
+++ b/drivers/gpu/nova-core/falcon/sec2.rs
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
use crate::falcon::{FalconEngine, PFalcon2Base, PFalconBase};
-use crate::regs::macros::RegisterBase;
+use kernel::io::register::RegisterBase;
/// Type specifying the `Sec2` falcon engine. Cannot be instantiated.
pub(crate) struct Sec2(());
diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
index 112277c7921e..fffcaee2249f 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,9 +2,6 @@
//! Nova Core GPU Driver
-#[macro_use]
-mod bitfield;
-
mod dma;
mod driver;
mod falcon;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..1f08e6d4045a 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -4,15 +4,13 @@
// but are mapped to types.
#![allow(non_camel_case_types)]
-#[macro_use]
-pub(crate) mod macros;
-
use crate::falcon::{
DmaTrfCmdSize, FalconCoreRev, FalconCoreRevSubversion, FalconFbifMemType,
FalconFbifTarget,
FalconModSelAlgo, FalconSecurityModel, PFalcon2Base, PFalconBase,
PeregrineCoreSelect,
};
use crate::gpu::{Architecture, Chipset};
use kernel::prelude::*;
+use kernel::register;
// PMC
@@ -331,6 +329,7 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
pub(crate) mod gm107 {
// FUSE
+ use kernel::register;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 {
0:0 display_disabled as bool;
@@ -339,6 +338,7 @@ pub(crate) mod gm107 {
pub(crate) mod ga100 {
// FUSE
+ use kernel::register;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 {
0:0 display_disabled as bool;
diff --git a/rust/kernel/bits.rs b/rust/kernel/bits.rs
index 553d50265883..590a77d99ad7 100644
--- a/rust/kernel/bits.rs
+++ b/rust/kernel/bits.rs
@@ -201,3 +201,5 @@ pub const fn [<genmask_ $ty>](range:
RangeInclusive<u32>) -> $ty {
/// assert_eq!(genmask_u8(0..=7), u8::MAX);
/// ```
);
+
+pub mod bitfield;
diff --git a/drivers/gpu/nova-core/bitfield.rs b/rust/kernel/bits/bitfield.rs
similarity index 91%
rename from drivers/gpu/nova-core/bitfield.rs
rename to rust/kernel/bits/bitfield.rs
index 9c022fc2bd6a..6089f3cdbd1b 100644
--- a/drivers/gpu/nova-core/bitfield.rs
+++ b/rust/kernel/bits/bitfield.rs
@@ -80,10 +80,13 @@
//! - `as <type> ?=> <try_into_type>` calls
`<try_into_type>`'s `TryFrom::<<type>>` implementation
//! and returns the result. This is useful with fields for which not all
values are valid.
//!
+
+/// bitfield macro definition
+#[macro_export]
macro_rules! bitfield {
// Main entry point - defines the bitfield struct with fields
($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? {
$($fields:tt)* }) => {
- bitfield!(@core $vis $name $storage $(, $comment)? { $($fields)* });
+ ::kernel::bitfield!(@core $vis $name $storage $(, $comment)? {
$($fields)* });
};
// All rules below are helpers.
@@ -118,7 +121,7 @@ fn from(val: $storage) -> Self {
}
}
- bitfield!(@fields_dispatcher $vis $name $storage { $($fields)* });
+ ::kernel::bitfield!(@fields_dispatcher $vis $name $storage {
$($fields)* });
};
// Captures the fields and passes them to all the implementers that require
field information.
@@ -134,7 +137,7 @@ fn from(val: $storage) -> Self {
)*
}
) => {
- bitfield!(@field_accessors $vis $name $storage {
+ ::kernel::bitfield!(@field_accessors $vis $name $storage {
$(
$hi:$lo $field as $type
$(?=> $try_into_type)?
@@ -143,8 +146,8 @@ fn from(val: $storage) -> Self {
;
)*
});
- bitfield!(@debug $name { $($field;)* });
- bitfield!(@default $name { $($field;)* });
+ ::kernel::bitfield!(@debug $name { $($field;)* });
+ ::kernel::bitfield!(@default $name { $($field;)* });
};
// Defines all the field getter/setter methods for `$name`.
@@ -159,7 +162,7 @@ fn from(val: $storage) -> Self {
}
) => {
$(
- bitfield!(@check_field_bounds $hi:$lo $field as $type);
+ ::kernel::bitfield!(@check_field_bounds $hi:$lo $field as $type);
)*
#[allow(dead_code)]
@@ -171,7 +174,7 @@ impl $name {
}
$(
- bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as
$type
+ ::kernel::bitfield!(@field_accessor $vis $name $storage, $hi:$lo
$field as $type
$(?=> $try_into_type)?
$(=> $into_type)?
$(, $comment)?
@@ -208,7 +211,7 @@ impl $name {
@field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as bool => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(
+ ::kernel::bitfield!(
@leaf_accessor $vis $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(if f != 0 { true } else { false }) }
$into_type => $into_type $(, $comment)?;
@@ -219,7 +222,7 @@ impl $name {
(
@field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as bool $(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as bool
=> bool $(, $comment)?;);
+ ::kernel::bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field
as bool => bool $(, $comment)?;);
};
// Catches the `?=>` syntax for non-boolean fields.
@@ -227,7 +230,7 @@ impl $name {
@field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as $type:tt ?=> $try_into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
+ ::kernel::bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
{ |f| <$try_into_type>::try_from(f as $type) } $try_into_type
=>
::core::result::Result<
$try_into_type,
@@ -241,7 +244,7 @@ impl $name {
@field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as $type:tt => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
+ ::kernel::bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(f as $type) } $into_type =>
$into_type $(, $comment)?;);
};
@@ -250,7 +253,7 @@ impl $name {
@field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt
$field:ident as $type:tt
$(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as $type
=> $type $(, $comment)?;);
+ ::kernel::bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field
as $type => $type $(, $comment)?;);
};
// Generates the accessor methods for a single field.
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index ee182b0b5452..da1384fd9ab6 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -9,6 +9,7 @@
pub mod mem;
pub mod poll;
+pub mod register;
pub mod resource;
pub use resource::Resource;
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/rust/kernel/io/register.rs
similarity index 93%
rename from drivers/gpu/nova-core/regs/macros.rs
rename to rust/kernel/io/register.rs
index 6a4f3271beb3..088a8590db92 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/rust/kernel/io/register.rs
@@ -17,7 +17,8 @@
/// 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(crate) trait RegisterBase<T> {
+pub trait RegisterBase<T> {
+ /// The base address for the register.
const BASE: usize;
}
@@ -273,28 +274,29 @@ pub(crate) trait RegisterBase<T> {
/// # Ok(())
/// # }
/// ```
+#[macro_export]
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } )
=> {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own
fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } )
=> {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? {
$($fields:tt)* } ) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its
own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? {
$($fields:tt)* }) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -305,7 +307,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -326,7 +328,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride
] ]);
};
@@ -348,7 +350,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx *
$alias::STRIDE ] );
};
@@ -357,7 +359,7 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array
alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? {
$($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* }
);
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? {
$($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
@@ -414,12 +416,12 @@ pub(crate) fn read<const SIZE: usize, T, B>(
base: &B,
) -> Self where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
let value = io.read32(
- <B as
crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as
::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
);
Self(value)
@@ -435,13 +437,13 @@ pub(crate) fn write<const SIZE: usize, T, B>(
base: &B,
) where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
io.write32(
self.0,
- <B as
crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as
::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
);
}
@@ -455,7 +457,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
f: F,
) where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base));
@@ -600,11 +602,11 @@ pub(crate) fn read<const SIZE: usize, T, B>(
idx: usize,
) -> Self where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as
crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as
::kernel::io::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
let value = io.read32(offset);
@@ -622,11 +624,11 @@ pub(crate) fn write<const SIZE: usize, T, B>(
idx: usize
) where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as
crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as
::kernel::io::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
io.write32(self.0, offset);
@@ -643,7 +645,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
f: F,
) where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base, idx));
@@ -662,7 +664,7 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
idx: usize,
) -> ::kernel::error::Result<Self> where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(Self::read(io, base, idx))
@@ -684,7 +686,7 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
idx: usize,
) -> ::kernel::error::Result where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(self.write(io, base, idx))
@@ -707,7 +709,7 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
f: F,
) -> ::kernel::error::Result where
T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
--
2.34.1
Greg KH
2025-Sep-21 09:36 UTC
[PATCH v4 1/6] nova-core: bitfield: Move bitfield-specific code from register! into new macro
On Sat, Sep 20, 2025 at 02:22:27PM -0400, Joel Fernandes wrote:> The bitfield-specific into new macro. This will be used to define > structs with bitfields, similar to C language. > > Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev> > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> > --- > drivers/gpu/nova-core/bitfield.rs | 314 +++++++++++++++++++++++++++ > drivers/gpu/nova-core/nova_core.rs | 3 + > drivers/gpu/nova-core/regs/macros.rs | 259 +--------------------- > 3 files changed, 327 insertions(+), 249 deletions(-) > create mode 100644 drivers/gpu/nova-core/bitfield.rs > > diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs > new file mode 100644 > index 000000000000..ba6b7caa05d9 > --- /dev/null > +++ b/drivers/gpu/nova-core/bitfield.rs > @@ -0,0 +1,314 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Bitfield library for Rust structures > +//! > +//! Support for defining bitfields in Rust structures. Also used by the [`register!`] macro. > +//! > +//! # Syntax > +//! > +//! ```rust > +//! #[derive(Debug, Clone, Copy)] > +//! enum Mode { > +//! Low = 0, > +//! High = 1, > +//! Auto = 2, > +//! } > +//! > +//! impl TryFrom<u8> for Mode { > +//! type Error = u8; > +//! fn try_from(value: u8) -> Result<Self, Self::Error> { > +//! match value { > +//! 0 => Ok(Mode::Low), > +//! 1 => Ok(Mode::High), > +//! 2 => Ok(Mode::Auto), > +//! _ => Err(value), > +//! } > +//! } > +//! } > +//! > +//! impl From<Mode> for u32 { > +//! fn from(mode: Mode) -> u32 { > +//! mode as u32 > +//! } > +//! } > +//! > +//! #[derive(Debug, Clone, Copy)] > +//! enum State { > +//! Inactive = 0, > +//! Active = 1, > +//! } > +//! > +//! impl From<bool> for State { > +//! fn from(value: bool) -> Self { > +//! if value { State::Active } else { State::Inactive } > +//! } > +//! } > +//! > +//! impl From<State> for u32 { > +//! fn from(state: State) -> u32 { > +//! state as u32 > +//! } > +//! } > +//! > +//! bitfield! { > +//! struct ControlReg { > +//! 3:0 mode as u8 ?=> Mode; > +//! 7 state as bool => State; > +//! } > +//! }As discussed at the conference this week, I do object to this as it will allow the same mistakes to happen that we used to do in the kernel for a long time before the regmap() api happened, along with GENMASK(). The issue is that you are going to want to take these bitfields as part of a larger structure, and attempt to "lay it over" a chunk of memory that came from, or is going to, hardware. When that happens, all of the endian issues of mis-matches between hardware and cpus come into play, which is not able to be properly expressed here at all, unless you attempt to either resolve it all later on in something like the regmap api, or you have #ifdef stuff to attempt to capture all of the possible combinations and deal with it at build time (which is strongly never recommended, but is what we used to do in previous decades.) Your example code using this is nice, and it shows how to set up, and query these bits, but that's not anything anyone actually does in the kernel, what they want to do is read/write from hardware with this. So, how does that work? Where does this "drop down" to the native bus/memory transactions and swizzle the bits properly to work correctly? And where does this allow us to define things like BIT(2) for values? (ok, that's kind of not the point of this patch series, but it will come up over time...) Ideally, this would map to our existing regmap api, which does handle all of this properly, but I know that's not usually used by PCI drivers like where this code is coming from, as they "just assume" endian formats are all little and can get away with it due to the limited nature of different hardware types for their hardware. Also, a larger meta-comment, why doesn't rust have bit types? Why does everyone either have to roll their own or rely on an external crate? Is anyone working to provide native bit support to the language? I'm sure the embedded people would love it as I imagine it's what they reach for first when using the language on their hardware. thanks, greg k-h
Alexandre Courbot
2025-Sep-29 06:16 UTC
[PATCH v4 1/6] nova-core: bitfield: Move bitfield-specific code from register! into new macro
On Sun Sep 21, 2025 at 3:22 AM JST, Joel Fernandes wrote:> The bitfield-specific into new macro. This will be used to define > structs with bitfields, similar to C language. > > Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev> > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>Very clean. One nit remains below, in any case: Reviewed-by: Alexandre Courbot <acourbot at nvidia.com>> --- > drivers/gpu/nova-core/bitfield.rs | 314 +++++++++++++++++++++++++++ > drivers/gpu/nova-core/nova_core.rs | 3 + > drivers/gpu/nova-core/regs/macros.rs | 259 +--------------------- > 3 files changed, 327 insertions(+), 249 deletions(-) > create mode 100644 drivers/gpu/nova-core/bitfield.rs > > diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs > new file mode 100644 > index 000000000000..ba6b7caa05d9 > --- /dev/null > +++ b/drivers/gpu/nova-core/bitfield.rs > @@ -0,0 +1,314 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Bitfield library for Rust structures > +//! > +//! Support for defining bitfields in Rust structures. Also used by the [`register!`] macro. > +//! > +//! # Syntax > +//! > +//! ```rust > +//! #[derive(Debug, Clone, Copy)] > +//! enum Mode { > +//! Low = 0, > +//! High = 1, > +//! Auto = 2, > +//! } > +//! > +//! impl TryFrom<u8> for Mode { > +//! type Error = u8; > +//! fn try_from(value: u8) -> Result<Self, Self::Error> { > +//! match value { > +//! 0 => Ok(Mode::Low), > +//! 1 => Ok(Mode::High), > +//! 2 => Ok(Mode::Auto), > +//! _ => Err(value), > +//! } > +//! } > +//! } > +//! > +//! impl From<Mode> for u32 { > +//! fn from(mode: Mode) -> u32 { > +//! mode as u32 > +//! } > +//! }Jesung's `TryFrom` and `Into` derive macros [1] would be greatly useful here, I hope we can merge them soon. [1] https://lore.kernel.org/all/cover.1755235180.git.y.j3ms.n at gmail.com/> +//! > +//! #[derive(Debug, Clone, Copy)] > +//! enum State { > +//! Inactive = 0, > +//! Active = 1, > +//! } > +//! > +//! impl From<bool> for State { > +//! fn from(value: bool) -> Self { > +//! if value { State::Active } else { State::Inactive } > +//! } > +//! } > +//! > +//! impl From<State> for u32 { > +//! fn from(state: State) -> u32 { > +//! state as u32 > +//! } > +//! } > +//! > +//! bitfield! { > +//! struct ControlReg { > +//! 3:0 mode as u8 ?=> Mode; > +//! 7 state as bool => State; > +//! } > +//! } > +//! ``` > +//! > +//! This generates a struct with: > +//! - Field accessors: `mode()`, `state()`, etc. > +//! - Field setters: `set_mode()`, `set_state()`, etc. (supports chaining with builder pattern). > +//! - Debug and Default implementations > +//! > +//! The field setters can be used with the builder pattern, example: > +//! ControlReg::default().set_mode(mode).set_state(state);Missing code block for the example?
Alexandre Courbot
2025-Sep-29 06:22 UTC
[PATCH v4 2/6] nova-core: bitfield: Add support for different storage widths
On Sun Sep 21, 2025 at 3:22 AM JST, Joel Fernandes wrote:> Previously, bitfields were hardcoded to use u32 as the underlying > storage type. Add support for different storage types (u8, u16, u32, > u64) to the bitfield macro. > > New syntax is: struct Name: <type ex., u32> { ... } > > Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev> > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> > --- > drivers/gpu/nova-core/bitfield.rs | 71 +++++++++++++++++----------- > drivers/gpu/nova-core/regs/macros.rs | 16 +++---- > 2 files changed, 52 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs > index ba6b7caa05d9..687ef234be75 100644 > --- a/drivers/gpu/nova-core/bitfield.rs > +++ b/drivers/gpu/nova-core/bitfield.rs > @@ -51,7 +51,7 @@ > //! } > //! > //! bitfield! { > -//! struct ControlReg { > +//! struct ControlReg: u32 {Haven't we agreed in [1] to align the type definition syntax to that of an actual Rust struct? E.g. `struct ControlReg(u32)`? [1] https://lore.kernel.org/all/3814d6b7-7551-4e8c-b78a-4ac6236eb406 at nvidia.com/
Alexandre Courbot
2025-Sep-29 06:28 UTC
[PATCH v4 3/6] nova-core: bitfield: Add support for custom visiblity
On Sun Sep 21, 2025 at 3:22 AM JST, Joel Fernandes wrote:> Add support for custom visiblity to allow for users to control visibility > of the structure and helpers. > > Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev> > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>Just one comment below; otherwise Reviewed-by: Alexandre Courbot <acourbot at nvidia.com> <snip>> @@ -161,8 +164,14 @@ fn from(val: $storage) -> Self { > > #[allow(dead_code)] > impl $name { > + /// Returns the raw underlying value > + #[inline(always)] > + $vis fn raw(&self) -> $storage { > + self.0 > + } > +Why does this new method suddenly appears in this patch?
Alexandre Courbot
2025-Sep-29 06:30 UTC
[PATCH v4 4/6] rust: Move register and bitfield macros out of Nova
On Sun Sep 21, 2025 at 3:22 AM JST, Joel Fernandes wrote:> Out of broad need for these macros in Rust, move them out. Several folks > have shown interest (Nova, Tyr GPU drivers). > > bitfield moved into bits modules - defines bitfields in Rust structs similar to C. > register moved into io module - defines hardware registers and accessors. > > Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev> > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>Very clean move! Reviewed-by: Alexandre Courbot <acourbot at nvidia.com>
Alexandre Courbot
2025-Sep-29 06:47 UTC
[PATCH v4 6/6] rust: bitfield: Use 'as' operator for setter type conversion
On Sun Sep 21, 2025 at 3:22 AM JST, Joel Fernandes wrote:> The bitfield macro's setter currently uses the From trait for type > conversion, which is overly restrictive and prevents use cases such as > narrowing conversions (e.g., u32 storage size to u8 field size) which > aren't supported by From. > > Replace 'from' with 'as' in the setter implementation to support this. > > Suggested-by: Yury Norov <yury.norov at gmail.com>Can you add a `Link: ` tag to the discussion for context? But I am not really convinced this is needed or desirable at all. Where would it make sense to define a field that is larger that its containing type? This looks like it can introduce confusion or errors. It's already not ideal that we can pass values that would be truncated; but this makes it worse. Anyway, if we decide to keep this, I think you want to remove the +//! Note that the compiler will error out if the size of the setter's arg exceeds the +//! struct's storage size. bit that was introduced in patch 2.