public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Dirk Behme" <dirk.behme@de.bosch.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Yury Norov" <yury.norov@gmail.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>, "Edwin Peer" <epeer@nvidia.com>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Steven Price" <steven.price@arm.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/6] rust: add `bitfield!` macro
Date: Tue, 20 Jan 2026 21:51:59 +0900	[thread overview]
Message-ID: <DFTFICFYMS05.1DC65YR0HDI0F@nvidia.com> (raw)
In-Reply-To: <f12b84b7-4972-47ee-a79e-61e8d1ad49b2@de.bosch.com>

Hi Dirk,

On Tue Jan 20, 2026 at 8:45 PM JST, Dirk Behme wrote:
> Hi Alexandre,
>
> On 20/01/2026 07:17, Alexandre Courbot wrote:
>> Add a macro for defining bitfield structs 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.
>> 
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>   rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   rust/kernel/lib.rs      |   1 +
>>   2 files changed, 504 insertions(+)
>> 
>> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
>> new file mode 100644
>> index 000000000000..2926ab802227
>> --- /dev/null
>> +++ b/rust/kernel/bitfield.rs
>> @@ -0,0 +1,503 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Support for defining bitfields as Rust structures.
>> +
>> +/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
>> +///
>> +/// # Example
>> +///
>> +/// ```rust
>> +/// use kernel::bitfield;
>> +/// use kernel::num::Bounded;
>> +///
>> +/// bitfield! {
>> +///     pub struct Rgb(u16) {
>> +///         15:11 blue;
>> +///         10:5 green;
>> +///         4:0 red;
>> +///     }
>> +/// }
>> +///
>> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> +/// let color = Rgb::default()
>> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>> +///
>> +/// assert_eq!(color.red(), 0x10);
>> +/// assert_eq!(color.green(), 0x1f);
>> +/// assert_eq!(color.blue(), 0x18);
>> +/// assert_eq!(
>> +///     color.as_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]
>> +///     pub struct Name(storage_type), "Struct documentation." {
>> +///         hi:lo field_1, "Field documentation.";
>> +///         hi:lo field_2 => ConvertedType, "Field documentation.";
>> +///         hi:lo field_3 ?=> ConvertedType, "Field documentation.";
>> +///         ...
>> +///     }
>> +/// }
>> +/// ```
>> +///
>> +/// - `storage_type`: The underlying integer type (`u8`, `u16`, `u32`, `u64`).
>> +/// - `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, and [`Default`] and [`Debug`] implementations.
>> +///
>> +/// For each field, it also generates:
>> +/// - `field()` - getter returning a [`Bounded`] (or converted type) for the field,
>> +/// - `set_field(value)` - setter with compile-time bounds checking,
>> +/// - `try_set_field(value)` - setter with runtime bounds checking (for fields without type
>> +///   conversion),
>> +/// - `FIELD_MASK`, `FIELD_SHIFT`, `FIELD_RANGE` - constants for manual bit manipulation.
>> +///
>> +/// # 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::default()
>> +///     .set_byte_field(0x42_u8)
>> +///     .set_flag(true);
>> +///
>> +/// assert_eq!(flags.as_raw(), (0x42 << Flags::BYTE_FIELD_SHIFT) | 1);
>> +/// ```
>> +///
>> +/// # Runtime bounds checking
>> +///
>> +/// When a value is not known at compile time, use `try_set_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_set_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 when all bit patterns map to valid values:
>> +///
>> +/// ```rust
>> +/// use kernel::bitfield;
>> +/// use kernel::num::Bounded;
>> +///
>> +/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
>> +/// enum Power {
>> +///     #[default]
>> +///     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::default().set_power(Power::On);
>> +/// assert_eq!(ctrl.power(), Power::On);
>> +/// ```
>> +///
>> +/// ## Fallible conversion (`?=>`)
>> +///
>> +/// Use when some bit patterns are invalid. The getter returns a [`Result`]:
>> +///
>> +/// ```rust
>> +/// use kernel::bitfield;
>> +/// use kernel::num::Bounded;
>> +///
>> +/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
>> +/// enum Mode {
>> +///     #[default]
>> +///     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::default().set_mode(Mode::Auto);
>> +/// assert_eq!(cfg.mode(), Ok(Mode::Auto));
>> +///
>> +/// // Invalid bit pattern returns an error.
>> +/// assert_eq!(Config::from(0b11).mode(), Err(3));
>> +/// ```
>> +///
>> +/// [`Bounded`]: kernel::num::Bounded
>> +#[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)
>> +            $(, $comment:literal)? { $($fields:tt)* }
>> +    ) => {
>> +        ::kernel::bitfield!(@core $(#[$attr])* $vis $name $storage $(, $comment)?);
>> +        ::kernel::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 $(, $comment:literal)?) => {
>> +        $(
>> +        #[doc=$comment]
>> +        )?
>> +        $(#[$attr])*
>> +        #[repr(transparent)]
>> +        #[derive(Clone, Copy, PartialEq, Eq)]
>> +        $vis struct $name($storage);
>> +
>> +        #[allow(dead_code)]
>> +        impl $name {
>> +            /// Returns the raw value of this bitfield.
>> +            ///
>> +            /// This is similar to the [`From`] implementation, but is shorter to invoke in
>> +            /// most cases.
>> +            $vis fn as_raw(self) -> $storage {
>> +                self.0
>> +            }
>> +        }
>> +
>> +        impl ::core::convert::From<$name> for $storage {
>> +            fn from(val: $name) -> $storage {
>> +                val.0
>> +            }
>> +        }
>> +
>> +        impl ::core::convert::From<$storage> for $name {
>> +            fn from(val: $storage) -> $name {
>> +                Self(val)
>> +            }
>> +        }
>> +    };
>> +
>> +    // Definitions requiring knowledge of individual fields: private and public field accessors,
>> +    // and `Debug` and `Default` implementations.
>> +    (@fields $vis:vis $name:ident $storage:ty {
>> +        $($hi:tt:$lo:tt $field:ident
>> +            $(?=> $try_into_type:ty)?
>> +            $(=> $into_type:ty)?
>> +            $(, $comment:literal)?
>> +        ;
>> +        )*
>> +    }
>> +    ) => {
>> +        #[allow(dead_code)]
>> +        impl $name {
>> +        $(
>> +        ::kernel::bitfield!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
>> +        ::kernel::bitfield!(@public_field_accessors $vis $name $storage : $hi:$lo $field
>> +            $(?=> $try_into_type)?
>> +            $(=> $into_type)?
>> +            $(, $comment)?
>> +        );
>> +        )*
>> +        }
>> +
>> +        ::kernel::bitfield!(@debug $name { $($field;)* });
>> +        ::kernel::bitfield!(@default $name { $($field;)* });
>> +    };
>> +
>> +    // Private field accessors working with the correct `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.0 << ALIGN_TOP
>> +            );
>> +            val.shr::<ALIGN_BOTTOM, _>()
>
> What have I missed?
>
> error[E0747]: type provided when a constant was expected
>     --> rust/kernel/bitfield.rs:318:37
>      |
> 318 |               val.shr::<ALIGN_BOTTOM, _>()
>      |                                       ^
> ...
> 566 | /     bitfield! {
> 567 | |         struct TestPageTableEntry(u64) {
> 568 | |             0:0       present;
> 569 | |             1:1       writable;
> ...   |
> 574 | |         }
> 575 | |     }
>      | |_____- in this macro invocation
>      |
>      = help: const arguments cannot yet be inferred with `_`
>      = note: this error originates in the macro `::kernel::bitfield` 
> which comes from the expansion of the macro `bitfield` (in Nightly 
> builds, run with -Z macro-backtrace for more info)
> help: add `#![feature(generic_arg_infer)]` to the crate attributes to enable
>     --> rust/kernel/lib.rs:59:1
>      |
> 59  + #![feature(generic_arg_infer)]
>
>
> $ rustc --version
> rustc 1.81.0 (eeb90cda1 2024-09-04)

Ah, I'm the one who missed something - this code builds fine with rustc
1.92, but on older versions the second generic parameter of `shr` cannot
be inferred and we need to specify the expected number of bits
explicitly. I will fix that in v2 as we need to support 1.78 anyway.

  parent reply	other threads:[~2026-01-20 12:52 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20  6:17 [PATCH 0/6] rust: add `bitfield!` and `register!` macros Alexandre Courbot
2026-01-20  6:17 ` [PATCH 1/6] rust: num: add `shr` and `shl` methods to `Bounded` Alexandre Courbot
2026-01-20  8:44   ` Alice Ryhl
2026-01-20 12:53     ` Alexandre Courbot
2026-01-20 16:12   ` kernel test robot
2026-01-21  8:15   ` Yury Norov
2026-01-21 10:10     ` Alice Ryhl
2026-01-20  6:17 ` [PATCH 2/6] rust: num: add `as_bool` method to `Bounded<_, 1>` Alexandre Courbot
2026-01-20  8:45   ` Alice Ryhl
2026-01-20  6:17 ` [PATCH 3/6] rust: add `bitfield!` macro Alexandre Courbot
2026-01-20 11:45   ` Dirk Behme
2026-01-20 12:37     ` Miguel Ojeda
2026-01-20 12:47       ` Dirk Behme
2026-01-20 13:08         ` Miguel Ojeda
2026-01-20 13:20           ` Alexandre Courbot
2026-01-20 21:02             ` Miguel Ojeda
2026-01-20 12:51     ` Alexandre Courbot [this message]
2026-01-21  9:16   ` Yury Norov
2026-01-26 13:35     ` Alexandre Courbot
2026-01-27  2:55       ` Yury Norov
2026-01-27  3:25         ` Joel Fernandes
2026-01-27  4:49           ` Yury Norov
2026-01-27 10:41             ` Alexandre Courbot
2026-01-27 10:55               ` Miguel Ojeda
2026-01-28  5:27               ` Yury Norov
2026-01-28 14:12                 ` Alexandre Courbot
2026-01-28 18:05                   ` Yury Norov
2026-01-29 13:40                     ` Alexandre Courbot
2026-01-29 15:12                       ` Miguel Ojeda
2026-01-27 11:00             ` Joel Fernandes
2026-01-27 15:02           ` Gary Guo
2026-01-28  1:23             ` Alexandre Courbot
2026-01-28  4:33               ` Yury Norov
2026-01-28 14:02                 ` Alexandre Courbot
2026-01-28 18:12                   ` Yury Norov
2026-01-27  9:57         ` Alexandre Courbot
2026-01-27 21:03           ` John Hubbard
2026-01-27 21:10             ` Gary Guo
2026-01-27 21:22               ` John Hubbard
2026-01-28  1:28               ` Alexandre Courbot
2026-01-28  1:41                 ` John Hubbard
2026-01-20  6:17 ` [PATCH 4/6] rust: bitfield: Add KUNIT tests for bitfield Alexandre Courbot
2026-01-20  6:17 ` [PATCH 5/6] rust: io: add `register!` macro Alexandre Courbot
2026-01-20  6:17 ` [PATCH FOR REFERENCE 6/6] gpu: nova-core: use the kernel `register!` and `bitfield!` macros Alexandre Courbot
2026-01-20 13:14 ` [PATCH 0/6] rust: add `bitfield!` and `register!` macros Miguel Ojeda
2026-01-20 13:38   ` Danilo Krummrich
2026-01-20 13:50     ` Miguel Ojeda
2026-01-20 14:18       ` Danilo Krummrich
2026-01-20 14:57         ` Miguel Ojeda
2026-01-20 15:27           ` Danilo Krummrich
2026-01-20 15:48             ` Miguel Ojeda
2026-01-20 20:01               ` Danilo Krummrich
2026-01-20 20:31                 ` Miguel Ojeda
2026-01-21  5:57                   ` Yury Norov
2026-01-21  6:55                     ` Alexandre Courbot
2026-01-26 14:03                     ` Joel Fernandes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DFTFICFYMS05.1DC65YR0HDI0F@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dirk.behme@de.bosch.com \
    --cc=ecourtney@nvidia.com \
    --cc=epeer@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=steven.price@arm.com \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox