* [PATCH v4 1/7] rust: extract `bitfield!` macro from `register!`
2026-05-27 12:51 [PATCH v4 0/7] rust: add `bitfield!` macro Alexandre Courbot
@ 2026-05-27 12:51 ` Alexandre Courbot
2026-05-27 12:51 ` [PATCH v4 2/7] rust: bitfield: inline private accessors Alexandre Courbot
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 12:51 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Alexandre Courbot, Yury Norov
Extract the bitfield-defining part of the `register!` macro into an
independent macro used to define bitfield types with bounds-checked
accessors.
Each field is represented as a `Bounded` of the appropriate bit width,
ensuring field values are never silently truncated.
Fields can optionally be converted to/from custom types, either fallibly
or infallibly.
Appropriate documentation is also added, and a MAINTAINERS entry created
for the new module.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Yury Norov <ynorov@nvidia.com>
---
MAINTAINERS | 7 +
rust/kernel/bitfield.rs | 546 ++++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
3 files changed, 554 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index c2c6d79275c6..d40e0c606893 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23422,6 +23422,13 @@ T: git https://github.com/Rust-for-Linux/linux.git alloc-next
F: rust/kernel/alloc.rs
F: rust/kernel/alloc/
+RUST [BITFIELD]
+M: Alexandre Courbot <acourbot@nvidia.com>
+R: Yury Norov <yury.norov@gmail.com>
+L: rust-for-linux@vger.kernel.org
+S: Maintained
+F: rust/kernel/bitfield.rs
+
RUST [INTEROP]
M: Joel Fernandes <joelagnelf@nvidia.com>
M: Alexandre Courbot <acourbot@nvidia.com>
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
new file mode 100644
index 000000000000..4083e7b7a307
--- /dev/null
+++ b/rust/kernel/bitfield.rs
@@ -0,0 +1,546 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Support for defining bitfields as Rust structures.
+//!
+//! The [`bitfield!`](kernel::bitfield!) macro declares integer types that are split into distinct
+//! bit fields of arbitrary length. Each field is typed using [`Bounded`](kernel::num::Bounded) to
+//! ensure values are properly validated and to avoid implicit data loss.
+//!
+//! # Example
+//!
+//! ```rust
+//! use kernel::bitfield;
+//! use kernel::num::Bounded;
+//!
+//! bitfield! {
+//! pub struct Rgb(u16) {
+//! 15:11 blue;
+//! 10:5 green;
+//! 4:0 red;
+//! }
+//! }
+//!
+//! // Valid value for the `blue` field.
+//! let blue = Bounded::<u16, 5>::new::<0x18>();
+//!
+//! // Setters can be chained. Values ranges are checked at compile-time.
+//! let color = Rgb::zeroed()
+//! // Compile-time bounds check of constant value.
+//! .with_const_red::<0x10>()
+//! .with_const_green::<0x1f>()
+//! // A `Bounded` can also be passed.
+//! .with_blue(blue);
+//!
+//! assert_eq!(color.red(), 0x10);
+//! assert_eq!(color.green(), 0x1f);
+//! assert_eq!(color.blue(), 0x18);
+//! assert_eq!(
+//! color.into_raw(),
+//! (0x18 << Rgb::BLUE_SHIFT) + (0x1f << Rgb::GREEN_SHIFT) + 0x10,
+//! );
+//!
+//! // Convert to/from the backing storage type.
+//! let raw: u16 = color.into();
+//! assert_eq!(Rgb::from(raw), color);
+//! ```
+//!
+//! # Syntax
+//!
+//! ```text
+//! bitfield! {
+//! #[attributes]
+//! // Documentation for `Name`.
+//! pub struct Name(storage_type) {
+//! // `field_1` documentation.
+//! hi:lo field_1;
+//! // `field_2` documentation.
+//! hi:lo field_2 => ConvertedType;
+//! // `field_3` documentation.
+//! hi:lo field_3 ?=> ConvertedType;
+//! ...
+//! }
+//! }
+//! ```
+//!
+//! - `storage_type`: The underlying unsigned integer type (`u8`, `u16`, `u32`, `u64`).
+//! Signed integer storage types are not supported.
+//! - `hi:lo`: Bit range (inclusive), where `hi >= lo`.
+//! - `=> Type`: Optional infallible conversion (see [below](#infallible-conversion-)).
+//! - `?=> Type`: Optional fallible conversion (see [below](#fallible-conversion-)).
+//! - Documentation strings and attributes are optional.
+//!
+//! # Generated code
+//!
+//! Each field is internally represented as a [`Bounded`] parameterized by its bit width. Field
+//! values can either be set/retrieved directly, or converted from/to another type.
+//!
+//! The use of `Bounded` for each field enforces bounds-checking (at build time or runtime) of every
+//! value assigned to a field. This ensures that data is never accidentally truncated.
+//!
+//! The macro generates the bitfield type, [`From`] and [`Into`] implementations for its storage
+//! type, as well as [`Debug`] and [`Zeroable`](pin_init::Zeroable) implementations.
+//!
+//! For each field, it also generates:
+//!
+//! - `field()`: Getter method for the field value.
+//! - `with_field(value)`: Infallible setter; the argument type must fit within the field's width.
+//! - `with_const_field::<VALUE>()`: `const` setter; the value is validated at compile time.
+//! Usually shorter to use than `with_field` for constant values as it doesn't require
+//! constructing a [`Bounded`].
+//! - `try_with_field(value)`: Fallible setter. Returns an error if the value is out of range.
+//! - `FIELD_MASK`, `FIELD_SHIFT`, `FIELD_RANGE`: Constants for manual bit manipulation.
+//!
+//! # Reserved names for field identifiers
+//!
+//! Field identifiers are used to generate methods and associated constants on the bitfield type.
+//! For a field named `field`, the macro may generate methods named `field`, `with_field`,
+//! `with_const_field`, `try_with_field`, `__field` and `__with_field`, as well as constants named
+//! `FIELD_MASK`, `FIELD_SHIFT` and `FIELD_RANGE`.
+//!
+//! Therefore, field identifiers must not use names that would collide with generated items for
+//! any field in the same bitfield. The following prefixes are thus reserved for field identifiers:
+//!
+//! - `with_`
+//! - `const_`
+//! - `try_with_`
+//! - `__`
+//!
+//! The field identifiers `from_raw`, `into_raw`, and `into` are also reserved.
+//!
+//! In addition, field identifiers should follow Rust `snake_case` conventions, since the associated
+//! constants are generated by uppercasing the field name.
+//!
+//! # Implicit conversions
+//!
+//! Types that fit entirely within a field's bit width can be used directly with setters. For
+//! example, `bool` works with single-bit fields, and `u8` works with 8-bit fields:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//!
+//! bitfield! {
+//! pub struct Flags(u32) {
+//! 15:8 byte_field;
+//! 0:0 flag;
+//! }
+//! }
+//!
+//! let flags = Flags::zeroed()
+//! .with_byte_field(0x42_u8)
+//! .with_flag(true);
+//!
+//! assert_eq!(flags.into_raw(), (0x42 << Flags::BYTE_FIELD_SHIFT) | 1);
+//! ```
+//!
+//! # Runtime bounds checking
+//!
+//! When a value is not known at compile time, use `try_with_field()` to check bounds at runtime:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//!
+//! bitfield! {
+//! pub struct Config(u8) {
+//! 3:0 nibble;
+//! }
+//! }
+//!
+//! fn set_nibble(config: Config, value: u8) -> Result<Config, Error> {
+//! // Returns `EOVERFLOW` if `value > 0xf`.
+//! config.try_with_nibble(value)
+//! }
+//! # Ok::<(), Error>(())
+//! ```
+//!
+//! # Type conversion
+//!
+//! Fields can be automatically converted to/from a custom type using `=>` (infallible) or `?=>`
+//! (fallible). The custom type must implement the appropriate `From` or `TryFrom` traits with
+//! `Bounded`.
+//!
+//! ## Infallible conversion (`=>`)
+//!
+//! Use this when all possible bit patterns of a field map to valid values:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//! use kernel::num::Bounded;
+//!
+//! #[derive(Debug, Clone, Copy, PartialEq)]
+//! enum Power {
+//! Off,
+//! On,
+//! }
+//!
+//! impl From<Bounded<u32, 1>> for Power {
+//! fn from(v: Bounded<u32, 1>) -> Self {
+//! match *v {
+//! 0 => Power::Off,
+//! _ => Power::On,
+//! }
+//! }
+//! }
+//!
+//! impl From<Power> for Bounded<u32, 1> {
+//! fn from(p: Power) -> Self {
+//! (p as u32 != 0).into()
+//! }
+//! }
+//!
+//! bitfield! {
+//! pub struct Control(u32) {
+//! 0:0 power => Power;
+//! }
+//! }
+//!
+//! let ctrl = Control::zeroed().with_power(Power::On);
+//! assert_eq!(ctrl.power(), Power::On);
+//! ```
+//!
+//! ## Fallible conversion (`?=>`)
+//!
+//! Use this when some bit patterns of a field are invalid. The getter returns a [`Result`]:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//! use kernel::num::Bounded;
+//!
+//! #[derive(Debug, Clone, Copy, PartialEq)]
+//! enum Mode {
+//! Low = 0,
+//! High = 1,
+//! Auto = 2,
+//! // 3 is invalid
+//! }
+//!
+//! impl TryFrom<Bounded<u32, 2>> for Mode {
+//! type Error = u32;
+//!
+//! fn try_from(v: Bounded<u32, 2>) -> Result<Self, u32> {
+//! match *v {
+//! 0 => Ok(Mode::Low),
+//! 1 => Ok(Mode::High),
+//! 2 => Ok(Mode::Auto),
+//! n => Err(n),
+//! }
+//! }
+//! }
+//!
+//! impl From<Mode> for Bounded<u32, 2> {
+//! fn from(m: Mode) -> Self {
+//! match m {
+//! Mode::Low => Bounded::<u32, _>::new::<0>(),
+//! Mode::High => Bounded::<u32, _>::new::<1>(),
+//! Mode::Auto => Bounded::<u32, _>::new::<2>(),
+//! }
+//! }
+//! }
+//!
+//! bitfield! {
+//! pub struct Config(u32) {
+//! 1:0 mode ?=> Mode;
+//! }
+//! }
+//!
+//! let cfg = Config::zeroed().with_mode(Mode::Auto);
+//! assert_eq!(cfg.mode(), Ok(Mode::Auto));
+//!
+//! // Invalid bit pattern returns an error.
+//! assert_eq!(Config::from(0b11).mode(), Err(3));
+//! ```
+//!
+//! # Bits outside of declared fields
+//!
+//! Bits of the storage type that are not part of any declared field are preserved by the setter
+//! methods, and can only be modified through `from_raw` or the [`From`] implementation from the
+//! storage type.
+//!
+//! ```rust
+//! use kernel::bitfield;
+//!
+//! bitfield! {
+//! pub struct Sparse(u8) {
+//! 7:6 high;
+//! // Bits 5:1 are not covered by any field.
+//! 0:0 low;
+//! }
+//! }
+//!
+//! // Set the gap bits via `from_raw`, then mutate the declared fields.
+//! let val = Sparse::from_raw(0b0010_1010)
+//! .with_const_high::<0b11>()
+//! .with_low(true);
+//!
+//! // Bits 5:1 are unchanged.
+//! assert_eq!(val.into_raw(), 0b1110_1011);
+//! ```
+//!
+//! # Signed field values
+//!
+//! Bitfield storage types are unsigned. Since field getter methods return a [`Bounded`] of the
+//! storage type, fields are also unsigned by default.
+//!
+//! If a field needs to encode a signed value, use a custom conversion type with `=>` or `?=>` to
+//! perform the sign interpretation explicitly.
+//!
+//! [`Bounded`]: kernel::num::Bounded
+
+/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
+///
+/// See the [`mod@kernel::bitfield`] module for full documentation and examples.
+#[macro_export]
+macro_rules! bitfield {
+ // Entry point defining the bitfield struct, its implementations and its field accessors.
+ (
+ $(#[$attr:meta])* $vis:vis struct $name:ident($storage:ty) { $($fields:tt)* }
+ ) => {
+ $crate::bitfield!(@core
+ #[allow(non_camel_case_types)]
+ $(#[$attr])* $vis $name $storage
+ );
+ $crate::bitfield!(@fields $vis $name $storage { $($fields)* });
+ };
+
+ // All rules below are helpers.
+
+ // Defines the wrapper `$name` type and its conversions from/to the storage type.
+ (@core $(#[$attr:meta])* $vis:vis $name:ident $storage:ty) => {
+ $(#[$attr])*
+ #[repr(transparent)]
+ #[derive(Clone, Copy, PartialEq, Eq)]
+ $vis struct $name {
+ inner: $storage,
+ }
+
+ #[allow(dead_code)]
+ impl $name {
+ /// Creates a bitfield from a raw value.
+ #[inline(always)]
+ $vis const fn from_raw(value: $storage) -> Self {
+ Self{ inner: value }
+ }
+
+ /// Turns this bitfield into its raw value.
+ ///
+ /// This is similar to the [`From`] implementation, but is shorter to invoke in
+ /// most cases.
+ #[inline(always)]
+ $vis const fn into_raw(self) -> $storage {
+ self.inner
+ }
+ }
+
+ // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
+ unsafe impl ::pin_init::Zeroable for $name {}
+
+ impl ::core::convert::From<$name> for $storage {
+ #[inline(always)]
+ fn from(val: $name) -> $storage {
+ val.into_raw()
+ }
+ }
+
+ impl ::core::convert::From<$storage> for $name {
+ #[inline(always)]
+ fn from(val: $storage) -> $name {
+ Self::from_raw(val)
+ }
+ }
+ };
+
+ // Definitions requiring knowledge of individual fields: private and public field accessors,
+ // and `Debug` implementation.
+ (@fields $vis:vis $name:ident $storage:ty {
+ $($(#[doc = $doc:expr])* $hi:literal:$lo:literal $field:ident
+ $(?=> $try_into_type:ty)?
+ $(=> $into_type:ty)?
+ ;
+ )*
+ }
+ ) => {
+ #[allow(dead_code)]
+ impl $name {
+ $(
+ $crate::bitfield!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
+ $crate::bitfield!(
+ @public_field_accessors $(#[doc = $doc])* $vis $name $storage : $hi:$lo $field
+ $(?=> $try_into_type)?
+ $(=> $into_type)?
+ );
+ )*
+ }
+
+ $crate::bitfield!(@debug $name { $($field;)* });
+ };
+
+ // Private field accessors working with the exact `Bounded` type for the field.
+ (
+ @private_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+ ) => {
+ ::kernel::macros::paste!(
+ $vis const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
+ $vis const [<$field:upper _MASK>]: $storage =
+ ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
+ $vis const [<$field:upper _SHIFT>]: u32 = $lo;
+ );
+
+ ::kernel::macros::paste!(
+ fn [<__ $field>](self) ->
+ ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }> {
+ // Left shift to align the field's MSB with the storage MSB.
+ const ALIGN_TOP: u32 = $storage::BITS - ($hi + 1);
+ // Right shift to move the top-aligned field to bit 0 of the storage.
+ const ALIGN_BOTTOM: u32 = ALIGN_TOP + $lo;
+
+ // Extract the field using two shifts. `Bounded::shr` produces the correctly-sized
+ // output type.
+ let val = ::kernel::num::Bounded::<$storage, { $storage::BITS }>::from(
+ self.inner << ALIGN_TOP
+ );
+ val.shr::<ALIGN_BOTTOM, { $hi + 1 - $lo } >()
+ }
+
+ const fn [<__with_ $field>](
+ mut self,
+ value: ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>,
+ ) -> Self
+ {
+ const MASK: $storage = <$name>::[<$field:upper _MASK>];
+ const SHIFT: u32 = <$name>::[<$field:upper _SHIFT>];
+
+ let value = value.get() << SHIFT;
+ self.inner = (self.inner & !MASK) | value;
+
+ self
+ }
+ );
+ };
+
+ // Public accessors for fields infallibly (`=>`) converted to a type.
+ (
+ @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
+ $hi:literal:$lo:literal $field:ident => $into_type:ty
+ ) => {
+ ::kernel::macros::paste!(
+
+ $(#[doc = $doc])*
+ #[doc = "Returns the value of this field."]
+ #[inline(always)]
+ $vis fn $field(self) -> $into_type
+ {
+ self.[<__ $field>]().into()
+ }
+
+ $(#[doc = $doc])*
+ #[doc = "Sets this field to the given `value`."]
+ #[inline(always)]
+ $vis fn [<with_ $field>](self, value: $into_type) -> Self
+ {
+ self.[<__with_ $field>](value.into())
+ }
+
+ );
+ };
+
+ // Public accessors for fields fallibly (`?=>`) converted to a type.
+ (
+ @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
+ $hi:tt:$lo:tt $field:ident ?=> $try_into_type:ty
+ ) => {
+ ::kernel::macros::paste!(
+
+ $(#[doc = $doc])*
+ #[doc = "Returns the value of this field."]
+ #[inline(always)]
+ $vis fn $field(self) ->
+ Result<
+ $try_into_type,
+ <$try_into_type as ::core::convert::TryFrom<
+ ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
+ >>::Error
+ >
+ {
+ self.[<__ $field>]().try_into()
+ }
+
+ $(#[doc = $doc])*
+ #[doc = "Sets this field to the given `value`."]
+ #[inline(always)]
+ $vis fn [<with_ $field>](self, value: $try_into_type) -> Self
+ {
+ self.[<__with_ $field>](value.into())
+ }
+
+ );
+ };
+
+ // Public accessors for fields not converted to a type.
+ (
+ @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
+ $hi:tt:$lo:tt $field:ident
+ ) => {
+ ::kernel::macros::paste!(
+
+ $(#[doc = $doc])*
+ #[doc = "Returns the value of this field."]
+ #[inline(always)]
+ $vis fn $field(self) ->
+ ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
+ {
+ self.[<__ $field>]()
+ }
+
+ $(#[doc = $doc])*
+ #[doc = "Sets this field to the compile-time constant `VALUE`."]
+ #[inline(always)]
+ $vis const fn [<with_const_ $field>]<const VALUE: $storage>(self) -> Self {
+ self.[<__with_ $field>](
+ ::kernel::num::Bounded::<$storage, { $hi + 1 - $lo }>::new::<VALUE>()
+ )
+ }
+
+ $(#[doc = $doc])*
+ #[doc = "Sets this field to the given `value`."]
+ #[inline(always)]
+ $vis fn [<with_ $field>]<T>(
+ self,
+ value: T,
+ ) -> Self
+ where T: Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
+ {
+ self.[<__with_ $field>](value.into())
+ }
+
+ $(#[doc = $doc])*
+ #[doc = "Tries to set this field to `value`, returning an error if it is out of range."]
+ #[inline(always)]
+ $vis fn [<try_with_ $field>]<T>(
+ self,
+ value: T,
+ ) -> ::kernel::error::Result<Self>
+ where T: ::kernel::num::TryIntoBounded<$storage, { $hi + 1 - $lo }>,
+ {
+ Ok(
+ self.[<__with_ $field>](
+ value.try_into_bounded().ok_or(::kernel::error::code::EOVERFLOW)?
+ )
+ )
+ }
+
+ );
+ };
+
+ // `Debug` implementation.
+ (@debug $name:ident { $($field:ident;)* }) => {
+ impl ::kernel::fmt::Debug for $name {
+ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
+ f.debug_struct(stringify!($name))
+ .field("<raw>", &::kernel::prelude::fmt!("{:#x}", self.inner))
+ $(
+ .field(stringify!($field), &self.$field())
+ )*
+ .finish()
+ }
+ }
+ };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b72b2fbe046d..9512af7156df 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -44,6 +44,7 @@
pub mod alloc;
#[cfg(CONFIG_AUXILIARY_BUS)]
pub mod auxiliary;
+pub mod bitfield;
pub mod bitmap;
pub mod bits;
#[cfg(CONFIG_BLOCK)]
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 2/7] rust: bitfield: inline private accessors
2026-05-27 12:51 [PATCH v4 0/7] rust: add `bitfield!` macro Alexandre Courbot
2026-05-27 12:51 ` [PATCH v4 1/7] rust: extract `bitfield!` macro from `register!` Alexandre Courbot
@ 2026-05-27 12:51 ` Alexandre Courbot
2026-05-27 12:51 ` [PATCH v4 3/7] rust: bitfield: fully qualify types in macro Alexandre Courbot
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 12:51 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Alexandre Courbot
All bitfield methods are inline, except these two ones. This is an
oversight, so inline them.
Reported-by: Gary Guo <gary@garyguo.net>
Closes: https://lore.kernel.org/all/DIFZRBF3LYHN.19Z6RBMECXDVM@garyguo.net/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/bitfield.rs | 2 ++
1 file changed, 2 insertions(+)
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index 4083e7b7a307..b3ad5fdbfb23 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -385,6 +385,7 @@ impl $name {
);
::kernel::macros::paste!(
+ #[inline(always)]
fn [<__ $field>](self) ->
::kernel::num::Bounded<$storage, { $hi + 1 - $lo }> {
// Left shift to align the field's MSB with the storage MSB.
@@ -400,6 +401,7 @@ fn [<__ $field>](self) ->
val.shr::<ALIGN_BOTTOM, { $hi + 1 - $lo } >()
}
+ #[inline(always)]
const fn [<__with_ $field>](
mut self,
value: ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>,
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 3/7] rust: bitfield: fully qualify types in macro
2026-05-27 12:51 [PATCH v4 0/7] rust: add `bitfield!` macro Alexandre Courbot
2026-05-27 12:51 ` [PATCH v4 1/7] rust: extract `bitfield!` macro from `register!` Alexandre Courbot
2026-05-27 12:51 ` [PATCH v4 2/7] rust: bitfield: inline private accessors Alexandre Courbot
@ 2026-05-27 12:51 ` Alexandre Courbot
2026-05-27 13:24 ` Miguel Ojeda
2026-05-27 12:51 ` [PATCH v4 4/7] rust: io: use the `bitfield!` macro in `register!` Alexandre Courbot
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 12:51 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Alexandre Courbot
These types were not fully qualified, which could cause issues if the
local module shadows them.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/bitfield.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index b3ad5fdbfb23..2498107979dc 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -455,7 +455,7 @@ const fn [<__with_ $field>](
#[doc = "Returns the value of this field."]
#[inline(always)]
$vis fn $field(self) ->
- Result<
+ ::core::result::Result<
$try_into_type,
<$try_into_type as ::core::convert::TryFrom<
::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
@@ -508,7 +508,7 @@ const fn [<__with_ $field>](
self,
value: T,
) -> Self
- where T: Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
+ where T: ::core::convert::Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
{
self.[<__with_ $field>](value.into())
}
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 3/7] rust: bitfield: fully qualify types in macro
2026-05-27 12:51 ` [PATCH v4 3/7] rust: bitfield: fully qualify types in macro Alexandre Courbot
@ 2026-05-27 13:24 ` Miguel Ojeda
2026-05-27 13:44 ` Alexandre Courbot
0 siblings, 1 reply; 12+ messages in thread
From: Miguel Ojeda @ 2026-05-27 13:24 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core
On Wed, May 27, 2026 at 2:52 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> These types were not fully qualified, which could cause issues if the
> local module shadows them.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Should we fixup this into #1? (on apply maybe?)
Similarly, for #2, I guess it may separate to give credit to Gary (?),
but typically we don't create extra commits if it is still not landed.
Or what am I missing? Perhaps they are intended to show the changelog (?).
(Either way, thanks for this!)
Cheers,
Miguel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 3/7] rust: bitfield: fully qualify types in macro
2026-05-27 13:24 ` Miguel Ojeda
@ 2026-05-27 13:44 ` Alexandre Courbot
2026-05-27 20:51 ` Yury Norov
0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 13:44 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core
On Wed May 27, 2026 at 10:24 PM JST, Miguel Ojeda wrote:
> On Wed, May 27, 2026 at 2:52 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> These types were not fully qualified, which could cause issues if the
>> local module shadows them.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Should we fixup this into #1? (on apply maybe?)
>
> Similarly, for #2, I guess it may separate to give credit to Gary (?),
> but typically we don't create extra commits if it is still not landed.
>
> Or what am I missing? Perhaps they are intended to show the changelog (?).
The intent was to make #1 a pure code move - except that, upon actual
inspection, it does not move the code but only extracts it, which weakens
that argument.
So yes, if you prefer it that way I agree that squashing #2 and #3 into
#1 (with a description of the changes in the commit log) should be fine.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 3/7] rust: bitfield: fully qualify types in macro
2026-05-27 13:44 ` Alexandre Courbot
@ 2026-05-27 20:51 ` Yury Norov
0 siblings, 0 replies; 12+ messages in thread
From: Yury Norov @ 2026-05-27 20:51 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Miguel Ojeda, Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core
On Wed, May 27, 2026 at 10:44:16PM +0900, Alexandre Courbot wrote:
> On Wed May 27, 2026 at 10:24 PM JST, Miguel Ojeda wrote:
> > On Wed, May 27, 2026 at 2:52 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
> >>
> >> These types were not fully qualified, which could cause issues if the
> >> local module shadows them.
> >>
> >> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >
> > Should we fixup this into #1? (on apply maybe?)
> >
> > Similarly, for #2, I guess it may separate to give credit to Gary (?),
> > but typically we don't create extra commits if it is still not landed.
> >
> > Or what am I missing? Perhaps they are intended to show the changelog (?).
>
> The intent was to make #1 a pure code move - except that, upon actual
> inspection, it does not move the code but only extracts it, which weakens
> that argument.
>
> So yes, if you prefer it that way I agree that squashing #2 and #3 into
> #1 (with a description of the changes in the commit log) should be fine.
Both ways are OK for me. If you decide to merge the patches, can you
explicitly list everything that is not a plain move in the change log?
Acked-by: Yury Norov <yury.norov@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 4/7] rust: io: use the `bitfield!` macro in `register!`
2026-05-27 12:51 [PATCH v4 0/7] rust: add `bitfield!` macro Alexandre Courbot
` (2 preceding siblings ...)
2026-05-27 12:51 ` [PATCH v4 3/7] rust: bitfield: fully qualify types in macro Alexandre Courbot
@ 2026-05-27 12:51 ` Alexandre Courbot
2026-05-27 12:51 ` [PATCH v4 5/7] gpu: nova-core: switch to kernel bitfield macro Alexandre Courbot
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 12:51 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Alexandre Courbot
Replace the local bitfield rules by the equivalent invocation of the
`bitfield!` macro.
No functional change should be introduced as the `bitfield!` macro has
been extracted from the rules of `register!`.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Yury Norov <yury.norov@gmail.com>
---
rust/kernel/io/register.rs | 246 +--------------------------------------------
1 file changed, 2 insertions(+), 244 deletions(-)
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index abc49926abfe..388647f28292 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -956,11 +956,10 @@ macro_rules! register {
(
@bitfield $(#[$attr:meta])* $vis:vis struct $name:ident($storage:ty) { $($fields:tt)* }
) => {
- $crate::register!(@bitfield_core
+ $crate::bitfield!(
#[allow(non_camel_case_types)]
- $(#[$attr])* $vis $name $storage
+ $(#[$attr])* $vis struct $name($storage) { $($fields)* }
);
- $crate::register!(@bitfield_fields $vis $name $storage { $($fields)* });
};
// Implementations shared by all registers types.
@@ -1016,245 +1015,4 @@ impl $crate::io::register::RegisterArray for $name {
impl $crate::io::register::RelativeRegisterArray for $name {}
};
-
- // Defines the wrapper `$name` type and its conversions from/to the storage type.
- (@bitfield_core $(#[$attr:meta])* $vis:vis $name:ident $storage:ty) => {
- $(#[$attr])*
- #[repr(transparent)]
- #[derive(Clone, Copy, PartialEq, Eq)]
- $vis struct $name {
- inner: $storage,
- }
-
- #[allow(dead_code)]
- impl $name {
- /// Creates a bitfield from a raw value.
- #[inline(always)]
- $vis const fn from_raw(value: $storage) -> Self {
- Self{ inner: value }
- }
-
- /// Turns this bitfield into its raw value.
- ///
- /// This is similar to the [`From`] implementation, but is shorter to invoke in
- /// most cases.
- #[inline(always)]
- $vis const fn into_raw(self) -> $storage {
- self.inner
- }
- }
-
- // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
- unsafe impl ::pin_init::Zeroable for $name {}
-
- impl ::core::convert::From<$name> for $storage {
- #[inline(always)]
- fn from(val: $name) -> $storage {
- val.into_raw()
- }
- }
-
- impl ::core::convert::From<$storage> for $name {
- #[inline(always)]
- fn from(val: $storage) -> $name {
- Self::from_raw(val)
- }
- }
- };
-
- // Definitions requiring knowledge of individual fields: private and public field accessors,
- // and `Debug` implementation.
- (@bitfield_fields $vis:vis $name:ident $storage:ty {
- $($(#[doc = $doc:expr])* $hi:literal:$lo:literal $field:ident
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- ;
- )*
- }
- ) => {
- #[allow(dead_code)]
- impl $name {
- $(
- $crate::register!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
- $crate::register!(
- @public_field_accessors $(#[doc = $doc])* $vis $name $storage : $hi:$lo $field
- $(?=> $try_into_type)?
- $(=> $into_type)?
- );
- )*
- }
-
- $crate::register!(@debug $name { $($field;)* });
- };
-
- // Private field accessors working with the exact `Bounded` type for the field.
- (
- @private_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
- ) => {
- ::kernel::macros::paste!(
- $vis const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
- $vis const [<$field:upper _MASK>]: $storage =
- ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
- $vis const [<$field:upper _SHIFT>]: u32 = $lo;
- );
-
- ::kernel::macros::paste!(
- fn [<__ $field>](self) ->
- ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }> {
- // Left shift to align the field's MSB with the storage MSB.
- const ALIGN_TOP: u32 = $storage::BITS - ($hi + 1);
- // Right shift to move the top-aligned field to bit 0 of the storage.
- const ALIGN_BOTTOM: u32 = ALIGN_TOP + $lo;
-
- // Extract the field using two shifts. `Bounded::shr` produces the correctly-sized
- // output type.
- let val = ::kernel::num::Bounded::<$storage, { $storage::BITS }>::from(
- self.inner << ALIGN_TOP
- );
- val.shr::<ALIGN_BOTTOM, { $hi + 1 - $lo } >()
- }
-
- const fn [<__with_ $field>](
- mut self,
- value: ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>,
- ) -> Self
- {
- const MASK: $storage = <$name>::[<$field:upper _MASK>];
- const SHIFT: u32 = <$name>::[<$field:upper _SHIFT>];
-
- let value = value.get() << SHIFT;
- self.inner = (self.inner & !MASK) | value;
-
- self
- }
- );
- };
-
- // Public accessors for fields infallibly (`=>`) converted to a type.
- (
- @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
- $hi:literal:$lo:literal $field:ident => $into_type:ty
- ) => {
- ::kernel::macros::paste!(
-
- $(#[doc = $doc])*
- #[doc = "Returns the value of this field."]
- #[inline(always)]
- $vis fn $field(self) -> $into_type
- {
- self.[<__ $field>]().into()
- }
-
- $(#[doc = $doc])*
- #[doc = "Sets this field to the given `value`."]
- #[inline(always)]
- $vis fn [<with_ $field>](self, value: $into_type) -> Self
- {
- self.[<__with_ $field>](value.into())
- }
-
- );
- };
-
- // Public accessors for fields fallibly (`?=>`) converted to a type.
- (
- @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
- $hi:tt:$lo:tt $field:ident ?=> $try_into_type:ty
- ) => {
- ::kernel::macros::paste!(
-
- $(#[doc = $doc])*
- #[doc = "Returns the value of this field."]
- #[inline(always)]
- $vis fn $field(self) ->
- Result<
- $try_into_type,
- <$try_into_type as ::core::convert::TryFrom<
- ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
- >>::Error
- >
- {
- self.[<__ $field>]().try_into()
- }
-
- $(#[doc = $doc])*
- #[doc = "Sets this field to the given `value`."]
- #[inline(always)]
- $vis fn [<with_ $field>](self, value: $try_into_type) -> Self
- {
- self.[<__with_ $field>](value.into())
- }
-
- );
- };
-
- // Public accessors for fields not converted to a type.
- (
- @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
- $hi:tt:$lo:tt $field:ident
- ) => {
- ::kernel::macros::paste!(
-
- $(#[doc = $doc])*
- #[doc = "Returns the value of this field."]
- #[inline(always)]
- $vis fn $field(self) ->
- ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
- {
- self.[<__ $field>]()
- }
-
- $(#[doc = $doc])*
- #[doc = "Sets this field to the compile-time constant `VALUE`."]
- #[inline(always)]
- $vis const fn [<with_const_ $field>]<const VALUE: $storage>(self) -> Self {
- self.[<__with_ $field>](
- ::kernel::num::Bounded::<$storage, { $hi + 1 - $lo }>::new::<VALUE>()
- )
- }
-
- $(#[doc = $doc])*
- #[doc = "Sets this field to the given `value`."]
- #[inline(always)]
- $vis fn [<with_ $field>]<T>(
- self,
- value: T,
- ) -> Self
- where T: Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
- {
- self.[<__with_ $field>](value.into())
- }
-
- $(#[doc = $doc])*
- #[doc = "Tries to set this field to `value`, returning an error if it is out of range."]
- #[inline(always)]
- $vis fn [<try_with_ $field>]<T>(
- self,
- value: T,
- ) -> ::kernel::error::Result<Self>
- where T: ::kernel::num::TryIntoBounded<$storage, { $hi + 1 - $lo }>,
- {
- Ok(
- self.[<__with_ $field>](
- value.try_into_bounded().ok_or(::kernel::error::code::EOVERFLOW)?
- )
- )
- }
-
- );
- };
-
- // `Debug` implementation.
- (@debug $name:ident { $($field:ident;)* }) => {
- impl ::kernel::fmt::Debug for $name {
- fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
- f.debug_struct(stringify!($name))
- .field("<raw>", &::kernel::prelude::fmt!("{:#x}", self.inner))
- $(
- .field(stringify!($field), &self.$field())
- )*
- .finish()
- }
- }
- };
}
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 5/7] gpu: nova-core: switch to kernel bitfield macro
2026-05-27 12:51 [PATCH v4 0/7] rust: add `bitfield!` macro Alexandre Courbot
` (3 preceding siblings ...)
2026-05-27 12:51 ` [PATCH v4 4/7] rust: io: use the `bitfield!` macro in `register!` Alexandre Courbot
@ 2026-05-27 12:51 ` Alexandre Courbot
2026-05-27 12:52 ` [PATCH v4 6/7] gpu: nova-core: remove the driver-local `bitfield!` macro Alexandre Courbot
2026-05-27 12:52 ` [PATCH v4 7/7] rust: bitfield: Add KUnit tests for bitfield Alexandre Courbot
6 siblings, 0 replies; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 12:51 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Alexandre Courbot
Replace uses of the Nova-internal `bitfield!` macro with the kernel one.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/fw.rs | 11 ++++++-----
drivers/gpu/nova-core/nova_core.rs | 3 ---
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 0c8a74f0e8ac..d54688904740 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -9,6 +9,7 @@
use core::ops::Range;
use kernel::{
+ bitfield,
dma::Coherent,
prelude::*,
ptr::{
@@ -728,8 +729,8 @@ unsafe impl AsBytes for MsgqRxHeader {}
bitfield! {
struct MsgHeaderVersion(u32) {
- 31:24 major as u8;
- 23:16 minor as u8;
+ 31:24 major;
+ 23:16 minor;
}
}
@@ -738,9 +739,9 @@ impl MsgHeaderVersion {
const MINOR_TOT: u8 = 0;
fn new() -> Self {
- Self::default()
- .set_major(Self::MAJOR_TOT)
- .set_minor(Self::MINOR_TOT)
+ Self::zeroed()
+ .with_major(Self::MAJOR_TOT)
+ .with_minor(Self::MINOR_TOT)
}
}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 04a1fa6b25f8..3a0c45481a92 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -10,9 +10,6 @@
InPlaceModule, //
};
-#[macro_use]
-mod bitfield;
-
mod driver;
mod falcon;
mod fb;
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 6/7] gpu: nova-core: remove the driver-local `bitfield!` macro
2026-05-27 12:51 [PATCH v4 0/7] rust: add `bitfield!` macro Alexandre Courbot
` (4 preceding siblings ...)
2026-05-27 12:51 ` [PATCH v4 5/7] gpu: nova-core: switch to kernel bitfield macro Alexandre Courbot
@ 2026-05-27 12:52 ` Alexandre Courbot
2026-05-27 12:52 ` [PATCH v4 7/7] rust: bitfield: Add KUnit tests for bitfield Alexandre Courbot
6 siblings, 0 replies; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 12:52 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Alexandre Courbot
This module is now orphaned code replaced by a kernel-global
implementation, so remove it.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/bitfield.rs | 329 --------------------------------------
1 file changed, 329 deletions(-)
diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs
deleted file mode 100644
index 02efdcf78d89..000000000000
--- a/drivers/gpu/nova-core/bitfield.rs
+++ /dev/null
@@ -1,329 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-//! Bitfield library for Rust structures
-//!
-//! Support for defining bitfields in Rust structures. Also used by the [`register!`] macro.
-
-/// Defines a struct with accessors to access bits within an inner unsigned integer.
-///
-/// # Syntax
-///
-/// ```rust
-/// use nova_core::bitfield;
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum Mode {
-/// #[default]
-/// 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 u8 {
-/// fn from(mode: Mode) -> u8 {
-/// mode as u8
-/// }
-/// }
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum State {
-/// #[default]
-/// 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 bool {
-/// fn from(state: State) -> bool {
-/// match state {
-/// State::Inactive => false,
-/// State::Active => true,
-/// }
-/// }
-/// }
-///
-/// bitfield! {
-/// pub struct ControlReg(u32) {
-/// 7:7 state as bool => State;
-/// 3:0 mode as u8 ?=> Mode;
-/// }
-/// }
-/// ```
-///
-/// 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.
-///
-/// 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`.
-///
-/// 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
- ($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`, and conversion to the value type) and field accessor methods.
- (@core $vis:vis $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
- $(
- #[doc=$comment]
- )?
- #[repr(transparent)]
- #[derive(Clone, Copy)]
- $vis struct $name($storage);
-
- impl ::core::convert::From<$name> for $storage {
- fn from(val: $name) -> $storage {
- val.0
- }
- }
-
- 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 $vis:vis $name:ident $storage:ty {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- bitfield!(@field_accessors $vis $name $storage {
- $(
- $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 $vis:vis $name:ident $storage:ty {
- $($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 $vis $name $storage, $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 $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
- => $into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(
- @leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$into_type>::from(f != 0) }
- bool $into_type => $into_type $(, $comment)?;
- );
- };
-
- // Shortcut for fields defined as `bool` without the `=>` syntax.
- (
- @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)?;
- );
- };
-
- // Catches the `?=>` syntax for non-boolean fields.
- (
- @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
- { |f| <$try_into_type>::try_from(f as $type) } $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 $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
- { |f| <$into_type>::from(f as $type) } $type $into_type => $into_type $(, $comment)?;);
- };
-
- // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
- (
- @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)?;
- );
- };
-
- // Generates the accessor methods for a single field.
- (
- @leaf_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
- { $process:expr } $prim_type:tt $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>]: $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 = $lo;
- );
-
- $(
- #[doc="Returns the value of this field:"]
- #[doc=$comment]
- )?
- #[inline(always)]
- $vis fn $field(self) -> $res_type {
- ::kernel::macros::paste!(
- const MASK: $storage = $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)]
- $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($prim_type::from(value)) << SHIFT) & MASK;
- self.0 = (self.0 & !MASK) | value;
-
- self
- }
- );
- };
-
- // Generates the `Debug` implementation for `$name`.
- (@debug $name:ident { $($field:ident;)* }) => {
- impl ::kernel::fmt::Debug for $name {
- fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
- f.debug_struct(stringify!($name))
- .field("<raw>", &::kernel::prelude::fmt!("{:#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 {
- let value = Self(Default::default());
-
- ::kernel::macros::paste!(
- $(
- let value = value.[<set_ $field>](Default::default());
- )*
- );
-
- value
- }
- }
- };
-}
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 7/7] rust: bitfield: Add KUnit tests for bitfield
2026-05-27 12:51 [PATCH v4 0/7] rust: add `bitfield!` macro Alexandre Courbot
` (5 preceding siblings ...)
2026-05-27 12:52 ` [PATCH v4 6/7] gpu: nova-core: remove the driver-local `bitfield!` macro Alexandre Courbot
@ 2026-05-27 12:52 ` Alexandre Courbot
2026-05-27 20:48 ` Yury Norov
6 siblings, 1 reply; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-27 12:52 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Alexandre Courbot, Joel Fernandes
From: Joel Fernandes <joelagnelf@nvidia.com>
Add KUNIT tests to make sure the macro is working correctly. The unit
tests are put behind the new `RUST_BITFIELD_KUNIT_TEST` Kconfig option.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
rust/kernel/Kconfig.test | 10 ++
rust/kernel/bitfield.rs | 315 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 325 insertions(+)
diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
index fc47614e6ec9..4fc6dc978101 100644
--- a/rust/kernel/Kconfig.test
+++ b/rust/kernel/Kconfig.test
@@ -73,4 +73,14 @@ config RUST_ATOMICS_KUNIT_TEST
If unsure, say N.
+config RUST_BITFIELD_KUNIT_TEST
+ bool "KUnit tests for the Rust `bitfield!` macro" if !KUNIT_ALL_TESTS
+ default KUNIT_ALL_TESTS
+ help
+ This option enables KUnit tests for the Rust `bitfield!` macro.
+ These are only for development and testing, not for regular
+ kernel use cases.
+
+ If unsure, say N.
+
endif
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index 2498107979dc..4720cdd23c74 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -546,3 +546,318 @@ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
}
};
}
+
+#[cfg(CONFIG_RUST_BITFIELD_KUNIT_TEST)]
+#[::kernel::macros::kunit_tests(kernel_bitfield)]
+mod tests {
+ use core::convert::TryFrom;
+
+ use pin_init::Zeroable;
+
+ use kernel::num::Bounded;
+
+ // Enum types for testing => and ?=> conversions
+ #[derive(Debug, Clone, Copy, PartialEq)]
+ enum MemoryType {
+ Unmapped = 0,
+ Normal = 1,
+ Device = 2,
+ Reserved = 3,
+ }
+
+ impl TryFrom<Bounded<u64, 4>> for MemoryType {
+ type Error = u64;
+ fn try_from(value: Bounded<u64, 4>) -> Result<Self, Self::Error> {
+ match value.get() {
+ 0 => Ok(MemoryType::Unmapped),
+ 1 => Ok(MemoryType::Normal),
+ 2 => Ok(MemoryType::Device),
+ 3 => Ok(MemoryType::Reserved),
+ _ => Err(value.get()),
+ }
+ }
+ }
+
+ impl From<MemoryType> for Bounded<u64, 4> {
+ fn from(mt: MemoryType) -> Bounded<u64, 4> {
+ Bounded::from_expr(mt as u64)
+ }
+ }
+
+ #[derive(Debug, Clone, Copy, PartialEq)]
+ enum Priority {
+ Low = 0,
+ Medium = 1,
+ High = 2,
+ Critical = 3,
+ }
+
+ impl From<Bounded<u16, 2>> for Priority {
+ fn from(value: Bounded<u16, 2>) -> Self {
+ match value & 0x3 {
+ 0 => Priority::Low,
+ 1 => Priority::Medium,
+ 2 => Priority::High,
+ _ => Priority::Critical,
+ }
+ }
+ }
+
+ impl From<Priority> for Bounded<u16, 2> {
+ fn from(p: Priority) -> Bounded<u16, 2> {
+ Bounded::from_expr(p as u16)
+ }
+ }
+
+ bitfield! {
+ struct TestPageTableEntry(u64) {
+ 61:52 available2;
+ 51:16 pfn;
+ 15:12 mem_type ?=> MemoryType;
+ 11:9 available;
+ 1:1 writable;
+ 0:0 present;
+ }
+ }
+
+ bitfield! {
+ struct TestControlRegister(u16) {
+ 15:8 channel;
+ 7:4 priority_nibble;
+ 5:4 priority => Priority;
+ 3:1 mode;
+ 0:0 enable;
+ }
+ }
+
+ bitfield! {
+ struct TestStatusRegister(u8) {
+ 7:0 full_byte; // For entire register
+ 7:4 reserved;
+ 3:2 state;
+ 1:1 error;
+ 0:0 ready;
+ }
+ }
+
+ #[test]
+ fn test_single_bits() {
+ let mut pte = TestPageTableEntry::zeroed();
+
+ assert!(!pte.present().into_bool());
+ assert!(!pte.writable().into_bool());
+ assert_eq!(u64::from(pte), 0x0);
+
+ pte = pte.with_present(true);
+ assert!(pte.present().into_bool());
+ assert_eq!(u64::from(pte), 0x1);
+
+ pte = pte.with_writable(true);
+ assert!(pte.writable().into_bool());
+ assert_eq!(u64::from(pte), 0x3);
+
+ pte = pte.with_writable(false);
+ assert!(!pte.writable().into_bool());
+ assert_eq!(u64::from(pte), 0x1);
+
+ assert_eq!(pte.available(), 0);
+ pte = pte.with_const_available::<0x5>();
+ assert_eq!(pte.available(), 0x5);
+ assert_eq!(u64::from(pte), 0xA01);
+ }
+
+ #[test]
+ fn test_range_fields() {
+ let mut pte = TestPageTableEntry::zeroed();
+ assert_eq!(u64::from(pte), 0x0);
+
+ pte = pte.with_const_pfn::<0x123456>();
+ assert_eq!(pte.pfn(), 0x123456);
+ assert_eq!(u64::from(pte), 0x1234560000);
+
+ pte = pte.with_const_available::<0x7>();
+ assert_eq!(pte.available(), 0x7);
+ assert_eq!(u64::from(pte), 0x1234560E00);
+
+ pte = pte.with_const_available2::<0x3FF>();
+ assert_eq!(pte.available2(), 0x3FF);
+ assert_eq!(u64::from(pte), 0x3FF0_0012_3456_0E00u64);
+
+ // Test TryFrom with ?=> for MemoryType
+ pte = pte.with_mem_type(MemoryType::Device);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Device));
+ assert_eq!(u64::from(pte), 0x3FF0_0012_3456_2E00u64);
+
+ pte = pte.with_mem_type(MemoryType::Normal);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Normal));
+ assert_eq!(u64::from(pte), 0x3FF0_0012_3456_1E00u64);
+
+ // Test all valid values for mem_type
+ pte = pte.with_mem_type(MemoryType::Reserved);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+ assert_eq!(u64::from(pte), 0x3FF0_0012_3456_3E00u64);
+
+ // Test failure case using mem_type field which has 4 bits (0-15)
+ // MemoryType only handles 0-3, so values 4-15 should return Err
+ let mut raw = pte.into_raw();
+ // Set bits 15:12 to 7 (invalid for MemoryType)
+ raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x7 << 12);
+ let invalid_pte = TestPageTableEntry::from_raw(raw);
+ // Should return Err with the invalid value
+ assert_eq!(invalid_pte.mem_type(), Err(0x7));
+
+ // Test a valid value after testing invalid to ensure both cases work
+ // Set bits 15:12 to 2 (valid: Device)
+ raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x2 << 12);
+ let valid_pte = TestPageTableEntry::from_raw(raw);
+ assert_eq!(valid_pte.mem_type(), Ok(MemoryType::Device));
+
+ const MAX_PFN: u64 = ::kernel::bits::genmask_u64(0..=35);
+ pte = pte.with_const_pfn::<{ MAX_PFN }>();
+ assert_eq!(pte.pfn(), MAX_PFN);
+ }
+
+ #[test]
+ fn test_builder_pattern() {
+ let pte = TestPageTableEntry::zeroed()
+ .with_present(true)
+ .with_writable(true)
+ .with_const_available::<0x7>()
+ .with_const_pfn::<0xABCDEF>()
+ .with_mem_type(MemoryType::Reserved)
+ .with_const_available2::<0x3FF>();
+
+ assert!(pte.present().into_bool());
+ assert!(pte.writable().into_bool());
+ assert_eq!(pte.available(), 0x7);
+ assert_eq!(pte.pfn(), 0xABCDEF);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+ assert_eq!(pte.available2(), 0x3FF);
+ }
+
+ #[test]
+ fn test_raw_operations() {
+ let raw_value = 0x3FF0000031233E03u64;
+
+ let pte = TestPageTableEntry::from_raw(raw_value);
+ assert_eq!(u64::from(pte), raw_value);
+
+ assert!(pte.present().into_bool());
+ assert!(pte.writable().into_bool());
+ assert_eq!(pte.available(), 0x7);
+ assert_eq!(pte.pfn(), 0x3123);
+ assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+ assert_eq!(pte.available2(), 0x3FF);
+ }
+
+ #[test]
+ fn test_u16_bitfield() {
+ let mut ctrl = TestControlRegister::zeroed();
+
+ assert!(!ctrl.enable().into_bool());
+ assert_eq!(ctrl.mode(), 0);
+ assert_eq!(ctrl.priority(), Priority::Low);
+ assert_eq!(ctrl.priority_nibble(), 0);
+ assert_eq!(ctrl.channel(), 0);
+
+ ctrl = ctrl.with_enable(true);
+ assert!(ctrl.enable().into_bool());
+
+ ctrl = ctrl.with_const_mode::<0x5>();
+ assert_eq!(ctrl.mode(), 0x5);
+
+ // Test From conversion with =>
+ ctrl = ctrl.with_priority(Priority::High);
+ assert_eq!(ctrl.priority(), Priority::High);
+ assert_eq!(ctrl.priority_nibble(), 0x2); // High = 2 in bits 5:4
+
+ ctrl = ctrl.with_channel(0xAB);
+ assert_eq!(ctrl.channel(), 0xAB);
+
+ // Test overlapping fields
+ ctrl = ctrl.with_const_priority_nibble::<0xF>();
+ assert_eq!(ctrl.priority_nibble(), 0xF);
+ assert_eq!(ctrl.priority(), Priority::Critical); // bits 5:4 = 0x3
+
+ let ctrl2 = TestControlRegister::zeroed()
+ .with_enable(true)
+ .with_const_mode::<0x3>()
+ .with_priority(Priority::Medium)
+ .with_channel(0x42);
+
+ assert!(ctrl2.enable().into_bool());
+ 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(raw_value);
+ assert_eq!(u16::from(ctrl3), raw_value);
+ assert!(ctrl3.enable().into_bool());
+ 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::zeroed();
+
+ assert!(!status.ready().into_bool());
+ assert!(!status.error().into_bool());
+ assert_eq!(status.state(), 0);
+ assert_eq!(status.reserved(), 0);
+ assert_eq!(status.full_byte(), 0);
+
+ status = status.with_ready(true);
+ assert!(status.ready().into_bool());
+ assert_eq!(status.full_byte(), 0x01);
+
+ status = status.with_error(true);
+ assert!(status.error().into_bool());
+ assert_eq!(status.full_byte(), 0x03);
+
+ status = status.with_const_state::<0x3>();
+ assert_eq!(status.state(), 0x3);
+ assert_eq!(status.full_byte(), 0x0F);
+
+ status = status.with_const_reserved::<0xA>();
+ assert_eq!(status.reserved(), 0xA);
+ assert_eq!(status.full_byte(), 0xAF);
+
+ // Test overlapping field
+ status = status.with_full_byte(0x55);
+ assert_eq!(status.full_byte(), 0x55);
+ assert!(status.ready().into_bool());
+ assert!(!status.error().into_bool());
+ assert_eq!(status.state(), 0x1);
+ assert_eq!(status.reserved(), 0x5);
+
+ let status2 = TestStatusRegister::zeroed()
+ .with_ready(true)
+ .with_const_state::<0x2>()
+ .with_const_reserved::<0x5>();
+
+ assert!(status2.ready().into_bool());
+ assert!(!status2.error().into_bool());
+ 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(raw_value);
+ assert_eq!(u8::from(status3), raw_value);
+ assert!(status3.ready().into_bool());
+ assert!(!status3.error().into_bool());
+ assert_eq!(status3.state(), 0x2);
+ assert_eq!(status3.reserved(), 0x5);
+ assert_eq!(status3.full_byte(), 0x59);
+
+ let status4 = TestStatusRegister::from_raw(0xFF);
+ assert!(status4.ready().into_bool());
+ assert!(status4.error().into_bool());
+ assert_eq!(status4.state(), 0x3);
+ assert_eq!(status4.reserved(), 0xF);
+ assert_eq!(status4.full_byte(), 0xFF);
+ }
+}
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 7/7] rust: bitfield: Add KUnit tests for bitfield
2026-05-27 12:52 ` [PATCH v4 7/7] rust: bitfield: Add KUnit tests for bitfield Alexandre Courbot
@ 2026-05-27 20:48 ` Yury Norov
0 siblings, 0 replies; 12+ messages in thread
From: Yury Norov @ 2026-05-27 20:48 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Yury Norov, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, David Airlie,
Simona Vetter, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kernel, rust-for-linux, nova-gpu,
driver-core, Joel Fernandes
On Wed, May 27, 2026 at 09:52:01PM +0900, Alexandre Courbot wrote:
> From: Joel Fernandes <joelagnelf@nvidia.com>
>
> Add KUNIT tests to make sure the macro is working correctly. The unit
> tests are put behind the new `RUST_BITFIELD_KUNIT_TEST` Kconfig option.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> rust/kernel/Kconfig.test | 10 ++
> rust/kernel/bitfield.rs | 315 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 325 insertions(+)
>
> diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
> index fc47614e6ec9..4fc6dc978101 100644
> --- a/rust/kernel/Kconfig.test
> +++ b/rust/kernel/Kconfig.test
> @@ -73,4 +73,14 @@ config RUST_ATOMICS_KUNIT_TEST
>
> If unsure, say N.
>
> +config RUST_BITFIELD_KUNIT_TEST
> + bool "KUnit tests for the Rust `bitfield!` macro" if !KUNIT_ALL_TESTS
> + default KUNIT_ALL_TESTS
> + help
> + This option enables KUnit tests for the Rust `bitfield!` macro.
> + These are only for development and testing, not for regular
> + kernel use cases.
> +
> + If unsure, say N.
> +
> endif
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index 2498107979dc..4720cdd23c74 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -546,3 +546,318 @@ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
> }
> };
> }
> +
> +#[cfg(CONFIG_RUST_BITFIELD_KUNIT_TEST)]
Here, can you also add a comment describing limitations, like
unsigned base types only, prohibited names, rules for unallocated
bits.
The motivation is simple: you demonstrate which behavior is intentional,
and which is implementation detail. So that, if someone changes the code,
he'd have to walk through the test to ensure consistency, and if that
doesn't hold - inspect the codebase for the corner cases.
> +#[::kernel::macros::kunit_tests(kernel_bitfield)]
> +mod tests {
> + use core::convert::TryFrom;
> +
> + use pin_init::Zeroable;
> +
> + use kernel::num::Bounded;
> +
> + // Enum types for testing => and ?=> conversions
> + #[derive(Debug, Clone, Copy, PartialEq)]
> + enum MemoryType {
> + Unmapped = 0,
> + Normal = 1,
> + Device = 2,
> + Reserved = 3,
> + }
> +
> + impl TryFrom<Bounded<u64, 4>> for MemoryType {
> + type Error = u64;
> + fn try_from(value: Bounded<u64, 4>) -> Result<Self, Self::Error> {
> + match value.get() {
> + 0 => Ok(MemoryType::Unmapped),
> + 1 => Ok(MemoryType::Normal),
> + 2 => Ok(MemoryType::Device),
> + 3 => Ok(MemoryType::Reserved),
> + _ => Err(value.get()),
Does this engine supports non-sequential enumeration, like:
0 => Ok(MemoryType::Unmapped),
1 => Ok(MemoryType::Normal),
2 => Err(value.get()),
3 => Ok(MemoryType::Reserved),
_ => Err(value.get()),
If so, can you add a test for it? (This is a real example from my
working place, FWIW.)
> + }
> + }
> + }
> +
> + impl From<MemoryType> for Bounded<u64, 4> {
> + fn from(mt: MemoryType) -> Bounded<u64, 4> {
> + Bounded::from_expr(mt as u64)
> + }
> + }
> +
> + #[derive(Debug, Clone, Copy, PartialEq)]
> + enum Priority {
> + Low = 0,
> + Medium = 1,
> + High = 2,
> + Critical = 3,
> + }
> +
> + impl From<Bounded<u16, 2>> for Priority {
> + fn from(value: Bounded<u16, 2>) -> Self {
> + match value & 0x3 {
> + 0 => Priority::Low,
> + 1 => Priority::Medium,
> + 2 => Priority::High,
> + _ => Priority::Critical,
> + }
> + }
> + }
> +
> + impl From<Priority> for Bounded<u16, 2> {
> + fn from(p: Priority) -> Bounded<u16, 2> {
> + Bounded::from_expr(p as u16)
> + }
> + }
> +
> + bitfield! {
> + struct TestPageTableEntry(u64) {
Can you add bit 63 in the list? The highest and lowest bits must be
covered in some test. Maybe create a separate subtest, if you don't
want to touch this one?
> + 61:52 available2;
> + 51:16 pfn;
> + 15:12 mem_type ?=> MemoryType;
> + 11:9 available;
> + 1:1 writable;
> + 0:0 present;
> + }
> + }
> +
> + bitfield! {
> + struct TestControlRegister(u16) {
For the purpose of testing, I think it's more clear to give names that
are meaningful in the testing context, like u16reg, u8reg, and so on.
Where is the 32-bit register by the way? Is it omitted on purpose?
> + 15:8 channel;
> + 7:4 priority_nibble;
> + 5:4 priority => Priority;
I understand, you want the test looking more realistic. But giving
'real' names may mislead and distract. Instead, can you name the
fields such that they underline the testing intention.
For example, this part may look like:
7:4 split_field;
7:6 split_field_hi;
5:4 split_field_lo => Priority;
So later in the code, you may add another trivially looking consistency
check:
assert!(split_field == (split_field_hi << 2) | split_field_lo)
> + 3:1 mode;
> + 0:0 enable;
> + }
> + }
> +
> + bitfield! {
> + struct TestStatusRegister(u8) {
> + 7:0 full_byte; // For entire register
Don't you have the .into() for it? Do you encourage to use that
placeholder instead of the method in some cases? Can you elaborate?
> + 7:4 reserved;
> + 3:2 state;
> + 1:1 error;
> + 0:0 ready;
> + }
> + }
> + #[test]
> + fn test_single_bits() {
> + let mut pte = TestPageTableEntry::zeroed();
> +
> + assert!(!pte.present().into_bool());
> + assert!(!pte.writable().into_bool());
> + assert_eq!(u64::from(pte), 0x0);
> +
> + pte = pte.with_present(true);
> + assert!(pte.present().into_bool());
> + assert_eq!(u64::from(pte), 0x1);
> +
> + pte = pte.with_writable(true);
> + assert!(pte.writable().into_bool());
> + assert_eq!(u64::from(pte), 0x3);
> +
> + pte = pte.with_writable(false);
> + assert!(!pte.writable().into_bool());
> + assert_eq!(u64::from(pte), 0x1);
> +
> + assert_eq!(pte.available(), 0);
> + pte = pte.with_const_available::<0x5>();
> + assert_eq!(pte.available(), 0x5);
> + assert_eq!(u64::from(pte), 0xA01);
> + }
> +
> + #[test]
> + fn test_range_fields() {
> + let mut pte = TestPageTableEntry::zeroed();
> + assert_eq!(u64::from(pte), 0x0);
> +
> + pte = pte.with_const_pfn::<0x123456>();
> + assert_eq!(pte.pfn(), 0x123456);
> + assert_eq!(u64::from(pte), 0x1234560000);
> +
> + pte = pte.with_const_available::<0x7>();
> + assert_eq!(pte.available(), 0x7);
> + assert_eq!(u64::from(pte), 0x1234560E00);
> +
> + pte = pte.with_const_available2::<0x3FF>();
> + assert_eq!(pte.available2(), 0x3FF);
> + assert_eq!(u64::from(pte), 0x3FF0_0012_3456_0E00u64);
> +
> + // Test TryFrom with ?=> for MemoryType
> + pte = pte.with_mem_type(MemoryType::Device);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Device));
> + assert_eq!(u64::from(pte), 0x3FF0_0012_3456_2E00u64);
> +
> + pte = pte.with_mem_type(MemoryType::Normal);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Normal));
> + assert_eq!(u64::from(pte), 0x3FF0_0012_3456_1E00u64);
> +
> + // Test all valid values for mem_type
> + pte = pte.with_mem_type(MemoryType::Reserved);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> + assert_eq!(u64::from(pte), 0x3FF0_0012_3456_3E00u64);
> +
> + // Test failure case using mem_type field which has 4 bits (0-15)
> + // MemoryType only handles 0-3, so values 4-15 should return Err
> + let mut raw = pte.into_raw();
> + // Set bits 15:12 to 7 (invalid for MemoryType)
> + raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x7 << 12);
> + let invalid_pte = TestPageTableEntry::from_raw(raw);
> + // Should return Err with the invalid value
> + assert_eq!(invalid_pte.mem_type(), Err(0x7));
> +
> + // Test a valid value after testing invalid to ensure both cases work
> + // Set bits 15:12 to 2 (valid: Device)
> + raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x2 << 12);
> + let valid_pte = TestPageTableEntry::from_raw(raw);
> + assert_eq!(valid_pte.mem_type(), Ok(MemoryType::Device));
> +
> + const MAX_PFN: u64 = ::kernel::bits::genmask_u64(0..=35);
> + pte = pte.with_const_pfn::<{ MAX_PFN }>();
> + assert_eq!(pte.pfn(), MAX_PFN);
> + }
> +
> + #[test]
> + fn test_builder_pattern() {
> + let pte = TestPageTableEntry::zeroed()
> + .with_present(true)
> + .with_writable(true)
> + .with_const_available::<0x7>()
> + .with_const_pfn::<0xABCDEF>()
> + .with_mem_type(MemoryType::Reserved)
> + .with_const_available2::<0x3FF>();
I can't find try_with in the test. Can you test it too?
Can I chain it like:
let bitfield = TestU64Entry::zeroes()
.try_with_present(val1)
.try_with_writable(val2)
...
Thanks,
Yury
> +
> + assert!(pte.present().into_bool());
> + assert!(pte.writable().into_bool());
> + assert_eq!(pte.available(), 0x7);
> + assert_eq!(pte.pfn(), 0xABCDEF);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> + assert_eq!(pte.available2(), 0x3FF);
> + }
> +
> + #[test]
> + fn test_raw_operations() {
> + let raw_value = 0x3FF0000031233E03u64;
> +
> + let pte = TestPageTableEntry::from_raw(raw_value);
> + assert_eq!(u64::from(pte), raw_value);
> +
> + assert!(pte.present().into_bool());
> + assert!(pte.writable().into_bool());
> + assert_eq!(pte.available(), 0x7);
> + assert_eq!(pte.pfn(), 0x3123);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> + assert_eq!(pte.available2(), 0x3FF);
> + }
> +
> + #[test]
> + fn test_u16_bitfield() {
> + let mut ctrl = TestControlRegister::zeroed();
> +
> + assert!(!ctrl.enable().into_bool());
> + assert_eq!(ctrl.mode(), 0);
> + assert_eq!(ctrl.priority(), Priority::Low);
> + assert_eq!(ctrl.priority_nibble(), 0);
> + assert_eq!(ctrl.channel(), 0);
> +
> + ctrl = ctrl.with_enable(true);
> + assert!(ctrl.enable().into_bool());
> +
> + ctrl = ctrl.with_const_mode::<0x5>();
> + assert_eq!(ctrl.mode(), 0x5);
> +
> + // Test From conversion with =>
> + ctrl = ctrl.with_priority(Priority::High);
> + assert_eq!(ctrl.priority(), Priority::High);
> + assert_eq!(ctrl.priority_nibble(), 0x2); // High = 2 in bits 5:4
> +
> + ctrl = ctrl.with_channel(0xAB);
> + assert_eq!(ctrl.channel(), 0xAB);
> +
> + // Test overlapping fields
> + ctrl = ctrl.with_const_priority_nibble::<0xF>();
> + assert_eq!(ctrl.priority_nibble(), 0xF);
> + assert_eq!(ctrl.priority(), Priority::Critical); // bits 5:4 = 0x3
> +
> + let ctrl2 = TestControlRegister::zeroed()
> + .with_enable(true)
> + .with_const_mode::<0x3>()
> + .with_priority(Priority::Medium)
> + .with_channel(0x42);
> +
> + assert!(ctrl2.enable().into_bool());
> + 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(raw_value);
> + assert_eq!(u16::from(ctrl3), raw_value);
> + assert!(ctrl3.enable().into_bool());
> + 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::zeroed();
> +
> + assert!(!status.ready().into_bool());
> + assert!(!status.error().into_bool());
> + assert_eq!(status.state(), 0);
> + assert_eq!(status.reserved(), 0);
> + assert_eq!(status.full_byte(), 0);
> +
> + status = status.with_ready(true);
> + assert!(status.ready().into_bool());
> + assert_eq!(status.full_byte(), 0x01);
> +
> + status = status.with_error(true);
> + assert!(status.error().into_bool());
> + assert_eq!(status.full_byte(), 0x03);
> +
> + status = status.with_const_state::<0x3>();
> + assert_eq!(status.state(), 0x3);
> + assert_eq!(status.full_byte(), 0x0F);
> +
> + status = status.with_const_reserved::<0xA>();
> + assert_eq!(status.reserved(), 0xA);
> + assert_eq!(status.full_byte(), 0xAF);
> +
> + // Test overlapping field
> + status = status.with_full_byte(0x55);
> + assert_eq!(status.full_byte(), 0x55);
> + assert!(status.ready().into_bool());
> + assert!(!status.error().into_bool());
> + assert_eq!(status.state(), 0x1);
> + assert_eq!(status.reserved(), 0x5);
> +
> + let status2 = TestStatusRegister::zeroed()
> + .with_ready(true)
> + .with_const_state::<0x2>()
> + .with_const_reserved::<0x5>();
> +
> + assert!(status2.ready().into_bool());
> + assert!(!status2.error().into_bool());
> + 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(raw_value);
> + assert_eq!(u8::from(status3), raw_value);
> + assert!(status3.ready().into_bool());
> + assert!(!status3.error().into_bool());
> + assert_eq!(status3.state(), 0x2);
> + assert_eq!(status3.reserved(), 0x5);
> + assert_eq!(status3.full_byte(), 0x59);
> +
> + let status4 = TestStatusRegister::from_raw(0xFF);
> + assert!(status4.ready().into_bool());
> + assert!(status4.error().into_bool());
> + assert_eq!(status4.state(), 0x3);
> + assert_eq!(status4.reserved(), 0xF);
> + assert_eq!(status4.full_byte(), 0xFF);
> + }
> +}
>
> --
> 2.54.0
^ permalink raw reply [flat|nested] 12+ messages in thread