Nouveau - Sep 2025 - [PATCH 1/2] nova-core: Add a library for bitfields in Rust structs

Add KUNIT tests to make sure the macros are working correctly.

Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
 drivers/gpu/nova-core/bitstruct.rs | 198 +++++++++++++++++++++++++++++
 1 file changed, 198 insertions(+)

diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
index 661a75da0a9c..7ce2f4ecfbba 100644
--- a/drivers/gpu/nova-core/bitstruct.rs
+++ b/drivers/gpu/nova-core/bitstruct.rs
@@ -147,3 +147,201 @@ impl $struct_name {
         }
     };
 }
+
+#[kunit_tests(nova_core_bitstruct)]
+mod kunit_tests {
+    // These are dummy structures just for testing (not real hardware).
+    bitstruct! {
+        #[repr(u64)]
+        struct TestPageTableEntry {
+            0:0       present     as bool,
+            1:1       writable    as bool,
+            11:9      available   as u8,
+            51:12     pfn         as u64,
+            61:52     available2  as u16,
+        }
+    }
+
+    bitstruct! {
+        #[repr(u16)]
+        struct TestControlRegister {
+            0:0       enable      as bool,
+            3:1       mode        as u8,
+            7:4       priority    as u8,
+            15:8      channel     as u8,
+        }
+    }
+
+    bitstruct! {
+        #[repr(u8)]
+        struct TestStatusRegister {
+            0:0       ready       as bool,
+            1:1       error       as bool,
+            3:2       state       as u8,
+            7:4       reserved    as u8,
+        }
+    }
+
+    #[test]
+    fn test_single_bits() {
+        let mut pte = TestPageTableEntry::default();
+
+        // Test initial state of boolean fields - expected false
+        assert!(!pte.present());
+        assert!(!pte.writable());
+
+        pte.set_present(true);
+        assert!(pte.present());
+
+        pte.set_writable(true);
+        assert!(pte.writable());
+
+        pte.set_writable(false);
+        assert!(!pte.writable());
+
+        // Test available field (3 bits)
+        assert_eq!(pte.available(), 0);
+        pte.set_available(0x5);
+        assert_eq!(pte.available(), 0x5);
+    }
+
+    #[test]
+    fn test_range_fields() {
+        let mut pte = TestPageTableEntry::default();
+
+        pte.set_pfn(0x123456);
+        assert_eq!(pte.pfn(), 0x123456);
+
+        pte.set_available(0x7);
+        assert_eq!(pte.available(), 0x7);
+
+        pte.set_available2(0x3FF);
+        assert_eq!(pte.available2(), 0x3FF);
+
+        let max_pfn = (1u64 << 40) - 1; // 40 bits for pfn
+        pte.set_pfn(max_pfn);
+        assert_eq!(pte.pfn(), max_pfn);
+    }
+
+    #[test]
+    fn test_builder_pattern() {
+        let pte = TestPageTableEntry::default()
+            .with_present(true)
+            .with_writable(true)
+            .with_available(0x7)
+            .with_pfn(0xABCDEF)
+            .with_available2(0x3FF);
+
+        assert!(pte.present());
+        assert!(pte.writable());
+        assert_eq!(pte.available(), 0x7);
+        assert_eq!(pte.pfn(), 0xABCDEF);
+        assert_eq!(pte.available2(), 0x3FF);
+    }
+
+    #[test]
+    fn test_raw_operations() {
+        let raw_value = 0x3FF0000000123E03u64;
+        let pte = TestPageTableEntry::from_raw(raw_value);
+        assert_eq!(pte.into_raw(), raw_value);
+
+        assert!(pte.present()); // bit 0
+        assert!(pte.writable()); // bit 1
+        assert_eq!(pte.available(), 0x7); // bits 9-11: 0xE00 >> 9 = 0x7
+        assert_eq!(pte.pfn(), 0x123); // bits 12-51: 0x123 << 12
+        assert_eq!(pte.available2(), 0x3FF); // bits 52-61: 0x3FF << 52
+    }
+
+    #[test]
+    fn test_u16_bitstruct() {
+        let mut ctrl = TestControlRegister::default();
+
+        // Test initial state
+        assert!(!ctrl.enable());
+        assert_eq!(ctrl.mode(), 0);
+        assert_eq!(ctrl.priority(), 0);
+        assert_eq!(ctrl.channel(), 0);
+
+        ctrl.set_enable(true);
+        assert!(ctrl.enable());
+
+        ctrl.set_mode(0x5);
+        assert_eq!(ctrl.mode(), 0x5);
+
+        ctrl.set_priority(0xF);
+        assert_eq!(ctrl.priority(), 0xF);
+
+        ctrl.set_channel(0xAB);
+        assert_eq!(ctrl.channel(), 0xAB);
+
+        // Test u16 builder pattern
+        let ctrl2 = TestControlRegister::default()
+            .with_enable(true)
+            .with_mode(0x3)
+            .with_priority(0x8)
+            .with_channel(0x42);
+
+        assert!(ctrl2.enable());
+        assert_eq!(ctrl2.mode(), 0x3);
+        assert_eq!(ctrl2.priority(), 0x8);
+        assert_eq!(ctrl2.channel(), 0x42);
+
+        // Test u16 raw operations
+        let raw_value: u16 = 0x4281;
+        let ctrl3 = TestControlRegister::from_raw(raw_value);
+        assert_eq!(ctrl3.into_raw(), raw_value);
+        assert!(ctrl3.enable());
+        assert_eq!(ctrl3.priority(), 0x8);
+        assert_eq!(ctrl3.channel(), 0x42);
+    }
+
+    #[test]
+    fn test_u8_bitstruct() {
+        let mut status = TestStatusRegister::default();
+
+        // Test initial state
+        assert!(!status.ready());
+        assert!(!status.error());
+        assert_eq!(status.state(), 0);
+        assert_eq!(status.reserved(), 0);
+
+        status.set_ready(true);
+        assert!(status.ready());
+
+        status.set_error(true);
+        assert!(status.error());
+
+        status.set_state(0x3);
+        assert_eq!(status.state(), 0x3);
+
+        status.set_reserved(0xA);
+        assert_eq!(status.reserved(), 0xA);
+
+        // Test u8 builder pattern
+        let status2 = TestStatusRegister::default()
+            .with_ready(true)
+            .with_state(0x2)
+            .with_reserved(0x5);
+
+        assert!(status2.ready());
+        assert!(!status2.error());
+        assert_eq!(status2.state(), 0x2);
+        assert_eq!(status2.reserved(), 0x5);
+
+        // Test u8 raw operations
+        let raw_value: u8 = 0x59;
+        let status3 = TestStatusRegister::from_raw(raw_value);
+        assert_eq!(status3.into_raw(), raw_value);
+        assert!(status3.ready());
+        assert!(!status3.error());
+        assert_eq!(status3.state(), 0x2);
+        assert_eq!(status3.reserved(), 0x5);
+
+        // Test max raw value - all bits set
+        let status4 = TestStatusRegister::from_raw(0xFF);
+        assert!(status4.ready());
+        assert!(status4.error());
+        assert_eq!(status4.state(), 0x3);
+        assert_eq!(status4.reserved(), 0xF);
+    }
+}
-- 
2.34.1

Add a minimal bitfield library for defining in Rust structures (called
bitstruct), similar in concept to bit fields in C structs. This will be used
for defining page table entries and other structures in nova-core.

Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
 drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
 drivers/gpu/nova-core/nova_core.rs |   1 +
 2 files changed, 150 insertions(+)
 create mode 100644 drivers/gpu/nova-core/bitstruct.rs

diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
new file mode 100644
index 000000000000..661a75da0a9c
--- /dev/null
+++ b/drivers/gpu/nova-core/bitstruct.rs
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// bitstruct.rs ? C-style library for bitfield-packed Rust structures
+//
+// A library that provides support for defining bit fields in Rust
+// structures to circumvent lack of native language support for this.
+//
+// Similar usage syntax to the register! macro.
+
+use kernel::prelude::*;
+
+/// Macro for defining bitfield-packed structures in Rust.
+/// The size of the underlying storage type is specified with #[repr(TYPE)].
+///
+/// # Example (just for illustration)
+/// ```rust
+/// bitstruct! {
+///     #[repr(u64)]
+///     pub struct PageTableEntry {
+///         0:0       present     as bool,
+///         1:1       writable    as bool,
+///         11:9      available   as u8,
+///         51:12     pfn         as u64,
+///         62:52     available2  as u16,
+///         63:63     nx          as bool,
+///     }
+/// }
+/// ```
+///
+/// This generates a struct with methods:
+/// - Constructor: `default()` sets all bits to zero.
+/// - Field accessors: `present()`, `pfn()`, etc.
+/// - Field setters: `set_present()`, `set_pfn()`, etc.
+/// - Builder methods: `with_present()`, `with_pfn()`, etc.
+/// - Raw conversion: `from_raw()`, `into_raw()`
+#[allow(unused_macros)]
+macro_rules! bitstruct {
+    (
+        #[repr($storage:ty)]
+        $vis:vis struct $name:ident {
+            $(
+                $hi:literal : $lo:literal $field:ident as $field_type:tt
+            ),* $(,)?
+        }
+    ) => {
+        #[repr(transparent)]
+        #[derive(Copy, Clone, Default)]
+        $vis struct $name($storage);
+
+        impl $name {
+            /// Create from raw value
+            #[inline(always)]
+            $vis const fn from_raw(val: $storage) -> Self {
+                Self(val)
+            }
+
+            /// Get raw value
+            #[inline(always)]
+            $vis const fn into_raw(self) -> $storage {
+                self.0
+            }
+        }
+
+        impl core::fmt::Debug for $name {
+            fn fmt(&self, f: &mut core::fmt::Formatter<'_>)
-> core::fmt::Result {
+                write!(f, "{}({:#x})", stringify!($name), self.0)
+            }
+        }
+
+        // Generate all field methods
+        $(
+            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field as
$field_type);
+        )*
+    };
+}
+
+/// Helper to calculate mask for bit fields
+#[allow(unused_macros)]
+macro_rules! bitstruct_mask {
+    ($hi:literal, $lo:literal, $storage:ty) => {{
+        let width = ($hi - $lo + 1) as usize;
+        let storage_bits = 8 * core::mem::size_of::<$storage>();
+        if width >= storage_bits {
+            <$storage>::MAX
+        } else {
+            ((1 as $storage) << width) - 1
+        }
+    }};
+}
+
+#[allow(unused_macros)]
+macro_rules! bitstruct_field_impl {
+    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident as $field_type:tt) => {
+        impl $struct_name {
+            #[inline(always)]
+            $vis const fn $field(&self) -> $field_type {
+                let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
+                bitstruct_cast_value!(field_val, $field_type)
+            }
+        }
+        bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo, $field,
$field_type);
+    };
+}
+
+/// Helper macro to convert extracted value to target type
+///
+/// Special handling for bool types is required because the `as` keyword
+/// cannot be used to convert to bool in Rust. For bool fields, we check
+/// if the extracted value is non-zero. For all other types, we use the
+/// standard `as` conversion.
+#[allow(unused_macros)]
+macro_rules! bitstruct_cast_value {
+    ($field_val:expr, bool) => {
+        $field_val != 0
+    };
+    ($field_val:expr, $field_type:tt) => {
+        $field_val as $field_type
+    };
+}
+
+#[allow(unused_macros)]
+macro_rules! bitstruct_write_bits {
+    ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{
+        let mask = bitstruct_mask!($hi, $lo, $storage);
+        ($raw & !(mask << $lo)) | ((($val as $storage) & mask)
<< $lo)
+    }};
+}
+
+#[allow(unused_macros)]
+macro_rules! bitstruct_make_setters {
+    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident, $field_type:tt) => {
+        ::kernel::macros::paste! {
+            impl $struct_name {
+                #[inline(always)]
+                #[allow(dead_code)]
+                $vis fn [<set_ $field>](&mut self, val: $field_type)
{
+                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
+                }
+
+                #[inline(always)]
+                #[allow(dead_code)]
+                $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
+                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
+                    self
+                }
+            }
+        }
+    };
+}
diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
index cb2bbb30cba1..54505cad4a73 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,7 @@
 
 //! Nova Core GPU Driver
 
+mod bitstruct;
 mod dma;
 mod driver;
 mod falcon;
-- 
2.34.1

+Cc: bitmap maintainer/reviewers: Yury Norov, Rasmus Villemoes

On 8/24/25 6:59 AM, Joel Fernandes wrote:
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs. This will be
used
> for defining page table entries and other structures in nova-core.
> 
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> ---
>   drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
>   drivers/gpu/nova-core/nova_core.rs |   1 +
>   2 files changed, 150 insertions(+)
>   create mode 100644 drivers/gpu/nova-core/bitstruct.rs
> 
> diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
> new file mode 100644
> index 000000000000..661a75da0a9c
> --- /dev/null
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// bitstruct.rs ? C-style library for bitfield-packed Rust structures
> +//
> +// A library that provides support for defining bit fields in Rust
> +// structures to circumvent lack of native language support for this.
> +//
> +// Similar usage syntax to the register! macro.
> +
> +use kernel::prelude::*;
> +
> +/// Macro for defining bitfield-packed structures in Rust.
> +/// The size of the underlying storage type is specified with
#[repr(TYPE)].
> +///
> +/// # Example (just for illustration)
> +/// ```rust
> +/// bitstruct! {
> +///     #[repr(u64)]
> +///     pub struct PageTableEntry {
> +///         0:0       present     as bool,
> +///         1:1       writable    as bool,
> +///         11:9      available   as u8,
> +///         51:12     pfn         as u64,
> +///         62:52     available2  as u16,
> +///         63:63     nx          as bool,
> +///     }
> +/// }
> +/// ```
> +///
> +/// This generates a struct with methods:
> +/// - Constructor: `default()` sets all bits to zero.
> +/// - Field accessors: `present()`, `pfn()`, etc.
> +/// - Field setters: `set_present()`, `set_pfn()`, etc.
> +/// - Builder methods: `with_present()`, `with_pfn()`, etc.
> +/// - Raw conversion: `from_raw()`, `into_raw()`
> +#[allow(unused_macros)]
> +macro_rules! bitstruct {
> +    (
> +        #[repr($storage:ty)]
> +        $vis:vis struct $name:ident {
> +            $(
> +                $hi:literal : $lo:literal $field:ident as $field_type:tt
> +            ),* $(,)?
> +        }
> +    ) => {
> +        #[repr(transparent)]
> +        #[derive(Copy, Clone, Default)]
> +        $vis struct $name($storage);
> +
> +        impl $name {
> +            /// Create from raw value
> +            #[inline(always)]
> +            $vis const fn from_raw(val: $storage) -> Self {
> +                Self(val)
> +            }
> +
> +            /// Get raw value
> +            #[inline(always)]
> +            $vis const fn into_raw(self) -> $storage {
> +                self.0
> +            }
> +        }
> +
> +        impl core::fmt::Debug for $name {
> +            fn fmt(&self, f: &mut
core::fmt::Formatter<'_>) -> core::fmt::Result {
> +                write!(f, "{}({:#x})", stringify!($name),
self.0)
> +            }
> +        }
> +
> +        // Generate all field methods
> +        $(
> +            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field
as $field_type);
> +        )*
> +    };
> +}
> +
> +/// Helper to calculate mask for bit fields
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_mask {
> +    ($hi:literal, $lo:literal, $storage:ty) => {{
> +        let width = ($hi - $lo + 1) as usize;
> +        let storage_bits = 8 * core::mem::size_of::<$storage>();
> +        if width >= storage_bits {
> +            <$storage>::MAX
> +        } else {
> +            ((1 as $storage) << width) - 1
> +        }
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_field_impl {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident as $field_type:tt) => {
> +        impl $struct_name {
> +            #[inline(always)]
> +            $vis const fn $field(&self) -> $field_type {
> +                let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
> +                bitstruct_cast_value!(field_val, $field_type)
> +            }
> +        }
> +        bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo,
$field, $field_type);
> +    };
> +}
> +
> +/// Helper macro to convert extracted value to target type
> +///
> +/// Special handling for bool types is required because the `as` keyword
> +/// cannot be used to convert to bool in Rust. For bool fields, we check
> +/// if the extracted value is non-zero. For all other types, we use the
> +/// standard `as` conversion.
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_cast_value {
> +    ($field_val:expr, bool) => {
> +        $field_val != 0
> +    };
> +    ($field_val:expr, $field_type:tt) => {
> +        $field_val as $field_type
> +    };
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_write_bits {
> +    ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{
> +        let mask = bitstruct_mask!($hi, $lo, $storage);
> +        ($raw & !(mask << $lo)) | ((($val as $storage) &
mask) << $lo)
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_make_setters {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident, $field_type:tt) => {
> +        ::kernel::macros::paste! {
> +            impl $struct_name {
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis fn [<set_ $field>](&mut self, val:
$field_type) {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                }
> +
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                    self
> +                }
> +            }
> +        }
> +    };
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
> index cb2bbb30cba1..54505cad4a73 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,6 +2,7 @@
>   
>   //! Nova Core GPU Driver
>   
> +mod bitstruct;
>   mod dma;
>   mod driver;
>   mod falcon;
thanks,
-- 
John Hubbard

On Sun, Aug 24, 2025 at 09:59:52AM -0400, Joel Fernandes
wrote:
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs. This will be
used
> for defining page table entries and other structures in nova-core.
> 
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
FWIW, Joel, I only received patch 1/2.

Regards,
Boqun

[..]

On 8/25/2025 9:44 AM, Boqun Feng wrote:
> On Sun, Aug 24, 2025 at 09:59:52AM -0400, Joel Fernandes wrote:
>> Add a minimal bitfield library for defining in Rust structures (called
>> bitstruct), similar in concept to bit fields in C structs. This will be
used
>> for defining page table entries and other structures in nova-core.
>>
>> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> 
> FWIW, Joel, I only received patch 1/2.
Oh. Good catch, will CC you on 2/2 for the next time.

thanks,

 - Joel

> 
> Regards,
> Boqun
> 
> [..]

On 8/24/25 3:59 PM, Joel Fernandes wrote:
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs. This will be
used
> for defining page table entries and other structures in nova-core.
> 
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> ---
>   drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
>   drivers/gpu/nova-core/nova_core.rs |   1 +
>   2 files changed, 150 insertions(+)
>   create mode 100644 drivers/gpu/nova-core/bitstruct.rs
I think this is much simpler than the register!() macro that we decided to
experiment with and work out within nova-core before making it available as
generic infrastructure.

So, probably this should go under rust/kernel/ directly.

- Danilo

Hi Joel,

On Sun Aug 24, 2025 at 10:59 PM JST, Joel Fernandes
wrote:
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs. This will be
used
> for defining page table entries and other structures in nova-core.
This is basically a rewrite (with some improvements, and some missing
features) of the part of the `register!` macro that deals with
bitfields. We planned to extract it into its own macro, and the split is
already effective in its internal rules, so I'd suggest to just move the
relevant rules into a new macro (as it will give you a couple useful
features, like automatic conversion to enum types), and then apply your
improvements on top of it. Otherwise we will end up with two
implementations of the same thing, for no good justification IMHO.

We were also planning to move the `register!` macro into the kernel
crate this cycle so Tyr can use it, but this changes the plan a bit.
Actually it is helpful, since your version implements one thing that we
needed in the `register!` macro before moving it: visibility specifiers.
So I would do things in this order:

1. Extract the bitfield-related code from the `register!` macro into its
   own macro (in nova-core), and make `register!` call into it,
2. Add support for visibility specifiers and non-u32 types in your macro and
   `register!`,
3. Move both macros to the kernel crate (hopefully in time for the next
   merge window so Tyr can make use of them).

A few more comments inline.
>
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> ---
>  drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
>  drivers/gpu/nova-core/nova_core.rs |   1 +
>  2 files changed, 150 insertions(+)
>  create mode 100644 drivers/gpu/nova-core/bitstruct.rs
>
> diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
> new file mode 100644
> index 000000000000..661a75da0a9c
> --- /dev/null
> +++ b/drivers/gpu/nova-core/bitstruct.rs
I wonder whether this should go under the existing `bits.rs`, or be its
own module?
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// bitstruct.rs ? C-style library for bitfield-packed Rust structures
> +//
> +// A library that provides support for defining bit fields in Rust
> +// structures to circumvent lack of native language support for this.
> +//
> +// Similar usage syntax to the register! macro.
Eventually the `register!` macro is the one that should reference this
(simpler) one, so let's make it the reference. :)
> +
> +use kernel::prelude::*;
> +
> +/// Macro for defining bitfield-packed structures in Rust.
> +/// The size of the underlying storage type is specified with
#[repr(TYPE)].
> +///
> +/// # Example (just for illustration)
> +/// ```rust
> +/// bitstruct! {
> +///     #[repr(u64)]
> +///     pub struct PageTableEntry {
> +///         0:0       present     as bool,
> +///         1:1       writable    as bool,
> +///         11:9      available   as u8,
> +///         51:12     pfn         as u64,
> +///         62:52     available2  as u16,
> +///         63:63     nx          as bool,
A note on syntax: for nova-core, we may want to use the `H:L` notation,
as this is what OpenRM uses, but in the larger kernel we might want to
use inclusive ranges (`L..=H`) as it will look more natural in Rust
code (and is the notation the `bits` module already uses).
> +///     }
> +/// }
> +/// ```
> +///
> +/// This generates a struct with methods:
> +/// - Constructor: `default()` sets all bits to zero.
> +/// - Field accessors: `present()`, `pfn()`, etc.
> +/// - Field setters: `set_present()`, `set_pfn()`, etc.
> +/// - Builder methods: `with_present()`, `with_pfn()`, etc.
> +/// - Raw conversion: `from_raw()`, `into_raw()`
> +#[allow(unused_macros)]
> +macro_rules! bitstruct {
> +    (
> +        #[repr($storage:ty)]
> +        $vis:vis struct $name:ident {
> +            $(
> +                $hi:literal : $lo:literal $field:ident as $field_type:tt
> +            ),* $(,)?
> +        }
> +    ) => {
> +        #[repr(transparent)]
> +        #[derive(Copy, Clone, Default)]
> +        $vis struct $name($storage);
> +
> +        impl $name {
> +            /// Create from raw value
> +            #[inline(always)]
> +            $vis const fn from_raw(val: $storage) -> Self {
> +                Self(val)
> +            }
> +
> +            /// Get raw value
> +            #[inline(always)]
> +            $vis const fn into_raw(self) -> $storage {
> +                self.0
> +            }
> +        }
> +
> +        impl core::fmt::Debug for $name {
> +            fn fmt(&self, f: &mut
core::fmt::Formatter<'_>) -> core::fmt::Result {
> +                write!(f, "{}({:#x})", stringify!($name),
self.0)
> +            }
> +        }
> +
> +        // Generate all field methods
> +        $(
> +            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field
as $field_type);
Maybe use internal rules [1] instead of a private macro (that cannot be so
private :))

[1] https://lukaswirth.dev/tlborm/decl-macros/patterns/internal-rules.html
> +        )*
> +    };
> +}
> +
> +/// Helper to calculate mask for bit fields
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_mask {
> +    ($hi:literal, $lo:literal, $storage:ty) => {{
> +        let width = ($hi - $lo + 1) as usize;
> +        let storage_bits = 8 * core::mem::size_of::<$storage>();
> +        if width >= storage_bits {
> +            <$storage>::MAX
> +        } else {
> +            ((1 as $storage) << width) - 1
> +        }
> +    }};
> +}
Is there a way to leverage the `genmask_*` functions of the `bits` module?
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_field_impl {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident as $field_type:tt) => {
> +        impl $struct_name {
> +            #[inline(always)]
> +            $vis const fn $field(&self) -> $field_type {
> +                let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
> +                bitstruct_cast_value!(field_val, $field_type)
> +            }
> +        }
> +        bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo,
$field, $field_type);
> +    };
> +}
> +
> +/// Helper macro to convert extracted value to target type
> +///
> +/// Special handling for bool types is required because the `as` keyword
> +/// cannot be used to convert to bool in Rust. For bool fields, we check
> +/// if the extracted value is non-zero. For all other types, we use the
> +/// standard `as` conversion.
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_cast_value {
> +    ($field_val:expr, bool) => {
> +        $field_val != 0
> +    };
> +    ($field_val:expr, $field_type:tt) => {
> +        $field_val as $field_type
> +    };
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_write_bits {
> +    ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{
> +        let mask = bitstruct_mask!($hi, $lo, $storage);
> +        ($raw & !(mask << $lo)) | ((($val as $storage) &
mask) << $lo)
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_make_setters {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident, $field_type:tt) => {
> +        ::kernel::macros::paste! {
> +            impl $struct_name {
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis fn [<set_ $field>](&mut self, val:
$field_type) {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                }
> +
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                    self
> +                }
> +            }
> +        }
> +    };
> +}
Yep, I think you definitely want to put these into internal rules - see
how `register!` does it, actually it should be easy to extract these
rules only and implement your improvements on top.

On Sun, Aug 24, 2025 at 09:59:52AM -0400, Joel Fernandes
wrote:
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs. This will be
used
> for defining page table entries and other structures in nova-core.
> 
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> ---
>  drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
>  drivers/gpu/nova-core/nova_core.rs |   1 +
>  2 files changed, 150 insertions(+)
>  create mode 100644 drivers/gpu/nova-core/bitstruct.rs
> 
> diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
> new file mode 100644
> index 000000000000..661a75da0a9c
> --- /dev/null
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// bitstruct.rs ? C-style library for bitfield-packed Rust structures
> +//
> +// A library that provides support for defining bit fields in Rust
> +// structures to circumvent lack of native language support for this.
> +//
> +// Similar usage syntax to the register! macro.
> +
> +use kernel::prelude::*;
> +
> +/// Macro for defining bitfield-packed structures in Rust.
> +/// The size of the underlying storage type is specified with
#[repr(TYPE)].
> +///
> +/// # Example (just for illustration)
> +/// ```rust
> +/// bitstruct! {
> +///     #[repr(u64)]
> +///     pub struct PageTableEntry {
> +///         0:0       present     as bool,
> +///         1:1       writable    as bool,
> +///         11:9      available   as u8,
> +///         51:12     pfn         as u64,
> +///         62:52     available2  as u16,
> +///         63:63     nx          as bool,
> +///     }
> +/// }
> +/// ```
> +///
> +/// This generates a struct with methods:
> +/// - Constructor: `default()` sets all bits to zero.
> +/// - Field accessors: `present()`, `pfn()`, etc.
> +/// - Field setters: `set_present()`, `set_pfn()`, etc.
> +/// - Builder methods: `with_present()`, `with_pfn()`, etc.
> +/// - Raw conversion: `from_raw()`, `into_raw()`
> +#[allow(unused_macros)]
> +macro_rules! bitstruct {
> +    (
> +        #[repr($storage:ty)]
> +        $vis:vis struct $name:ident {
> +            $(
> +                $hi:literal : $lo:literal $field:ident as $field_type:tt
> +            ),* $(,)?
> +        }
> +    ) => {
> +        #[repr(transparent)]
> +        #[derive(Copy, Clone, Default)]
> +        $vis struct $name($storage);
> +
> +        impl $name {
> +            /// Create from raw value
> +            #[inline(always)]
> +            $vis const fn from_raw(val: $storage) -> Self {
> +                Self(val)
> +            }
> +
> +            /// Get raw value
> +            #[inline(always)]
> +            $vis const fn into_raw(self) -> $storage {
> +                self.0
> +            }
> +        }
> +
> +        impl core::fmt::Debug for $name {
> +            fn fmt(&self, f: &mut
core::fmt::Formatter<'_>) -> core::fmt::Result {
> +                write!(f, "{}({:#x})", stringify!($name),
self.0)
> +            }
> +        }
> +
> +        // Generate all field methods
> +        $(
> +            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field
as $field_type);
> +        )*
> +    };
> +}
> +
> +/// Helper to calculate mask for bit fields
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_mask {
> +    ($hi:literal, $lo:literal, $storage:ty) => {{
> +        let width = ($hi - $lo + 1) as usize;
> +        let storage_bits = 8 * core::mem::size_of::<$storage>();
> +        if width >= storage_bits {
> +            <$storage>::MAX
> +        } else {
> +            ((1 as $storage) << width) - 1
> +        }
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_field_impl {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident as $field_type:tt) => {
> +        impl $struct_name {
> +            #[inline(always)]
> +            $vis const fn $field(&self) -> $field_type {
> +                let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
> +                bitstruct_cast_value!(field_val, $field_type)
> +            }
> +        }
> +        bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo,
$field, $field_type);
> +    };
> +}
> +
> +/// Helper macro to convert extracted value to target type
> +///
> +/// Special handling for bool types is required because the `as` keyword
> +/// cannot be used to convert to bool in Rust. For bool fields, we check
> +/// if the extracted value is non-zero. For all other types, we use the
> +/// standard `as` conversion.
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_cast_value {
> +    ($field_val:expr, bool) => {
> +        $field_val != 0
> +    };
> +    ($field_val:expr, $field_type:tt) => {
> +        $field_val as $field_type
> +    };
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_write_bits {
> +    ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{
> +        let mask = bitstruct_mask!($hi, $lo, $storage);
> +        ($raw & !(mask << $lo)) | ((($val as $storage) &
mask) << $lo)
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_make_setters {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident, $field_type:tt) => {
> +        ::kernel::macros::paste! {
> +            impl $struct_name {
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis fn [<set_ $field>](&mut self, val:
$field_type) {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                }
> +
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                    self
> +                }
> +            }
> +        }
> +    };
> +}
This is awesome. Is there a place for this to live outside of
`nova-core`? I would think this would be extremely useful as a general
helper for bitfield struct definitions.
> diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
> index cb2bbb30cba1..54505cad4a73 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,6 +2,7 @@
>  
>  //! Nova Core GPU Driver
>  
> +mod bitstruct;
>  mod dma;
>  mod driver;
>  mod falcon;
> -- 
> 2.34.1
Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev>

Hi Joel,
> On 24 Aug 2025, at 10:59, Joel Fernandes <joelagnelf at nvidia.com>
wrote:
> 
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs. This will be
used
> for defining page table entries and other structures in nova-core.
> 
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> ---
> drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
> drivers/gpu/nova-core/nova_core.rs |   1 +
> 2 files changed, 150 insertions(+)
> create mode 100644 drivers/gpu/nova-core/bitstruct.rs
> 
> diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
> new file mode 100644
> index 000000000000..661a75da0a9c
> --- /dev/null
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// bitstruct.rs ? C-style library for bitfield-packed Rust structures
> +//
> +// A library that provides support for defining bit fields in Rust
> +// structures to circumvent lack of native language support for this.
> +//
> +// Similar usage syntax to the register! macro.
> +
> +use kernel::prelude::*;
> +
> +/// Macro for defining bitfield-packed structures in Rust.
> +/// The size of the underlying storage type is specified with
#[repr(TYPE)].
> +///
> +/// # Example (just for illustration)
> +/// ```rust
> +/// bitstruct! {
> +///     #[repr(u64)]
> +///     pub struct PageTableEntry {
> +///         0:0       present     as bool,
> +///         1:1       writable    as bool,
> +///         11:9      available   as u8,
> +///         51:12     pfn         as u64,
> +///         62:52     available2  as u16,
> +///         63:63     nx          as bool,
> +///     }
> +/// }
> +/// ```
> +///
> +/// This generates a struct with methods:
> +/// - Constructor: `default()` sets all bits to zero.
> +/// - Field accessors: `present()`, `pfn()`, etc.
> +/// - Field setters: `set_present()`, `set_pfn()`, etc.
> +/// - Builder methods: `with_present()`, `with_pfn()`, etc.
I think this could use a short example highlighting the builder pattern. It may
be initially unclear that the methods can be chained, even though the word
?builder? is being used.
> +/// - Raw conversion: `from_raw()`, `into_raw()`
> +#[allow(unused_macros)]
> +macro_rules! bitstruct {
> +    (
> +        #[repr($storage:ty)]
> +        $vis:vis struct $name:ident {
> +            $(
> +                $hi:literal : $lo:literal $field:ident as $field_type:tt
> +            ),* $(,)?
> +        }
> +    ) => {
> +        #[repr(transparent)]
> +        #[derive(Copy, Clone, Default)]
> +        $vis struct $name($storage);
> +
> +        impl $name {
> +            /// Create from raw value
> +            #[inline(always)]
> +            $vis const fn from_raw(val: $storage) -> Self {
> +                Self(val)
> +            }
> +
> +            /// Get raw value
> +            #[inline(always)]
> +            $vis const fn into_raw(self) -> $storage {
> +                self.0
> +            }
> +        }
> +
> +        impl core::fmt::Debug for $name {
> +            fn fmt(&self, f: &mut
core::fmt::Formatter<'_>) -> core::fmt::Result {
> +                write!(f, "{}({:#x})", stringify!($name),
self.0)
> +            }
> +        }
> +
> +        // Generate all field methods
> +        $(
> +            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field
as $field_type);
> +        )*
> +    };
> +}
> +
> +/// Helper to calculate mask for bit fields
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_mask {
> +    ($hi:literal, $lo:literal, $storage:ty) => {{
> +        let width = ($hi - $lo + 1) as usize;
> +        let storage_bits = 8 * core::mem::size_of::<$storage>();
> +        if width >= storage_bits {
> +            <$storage>::MAX
> +        } else {
> +            ((1 as $storage) << width) - 1
Can?t we have a build_assert here instead?
> +        }
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_field_impl {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident as $field_type:tt) => {
> +        impl $struct_name {
> +            #[inline(always)]
> +            $vis const fn $field(&self) -> $field_type {
> +                let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
> +                bitstruct_cast_value!(field_val, $field_type)
> +            }
> +        }
> +        bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo,
$field, $field_type);
> +    };
> +}
> +
> +/// Helper macro to convert extracted value to target type
> +///
> +/// Special handling for bool types is required because the `as` keyword
> +/// cannot be used to convert to bool in Rust. For bool fields, we check
> +/// if the extracted value is non-zero. For all other types, we use the
> +/// standard `as` conversion.
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_cast_value {
> +    ($field_val:expr, bool) => {
> +        $field_val != 0
> +    };
> +    ($field_val:expr, $field_type:tt) => {
> +        $field_val as $field_type
> +    };
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_write_bits {
> +    ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{
> +        let mask = bitstruct_mask!($hi, $lo, $storage);
> +        ($raw & !(mask << $lo)) | ((($val as $storage) &
mask) << $lo)
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_make_setters {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident, $field_type:tt) => {
> +        ::kernel::macros::paste! {
> +            impl $struct_name {
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis fn [<set_ $field>](&mut self, val:
$field_type) {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                }
> +
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                    self
> +                }
> +            }
> +        }
> +    };
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
> index cb2bbb30cba1..54505cad4a73 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,6 +2,7 @@
> 
> //! Nova Core GPU Driver
> 
> +mod bitstruct;
> mod dma;
> mod driver;
> mod falcon;
> -- 
> 2.34.1
> 
> 
The code itself looks good. Thanks for doing this work, it will be useful for
Tyr :)

? Daniel

On 8/25/2025 7:20 PM, Elle Rhumsaa wrote:
> On Sun, Aug 24, 2025 at 09:59:52AM -0400, Joel Fernandes wrote:
>> Add a minimal bitfield library for defining in Rust structures (called
>> bitstruct), similar in concept to bit fields in C structs. This will be
used
>> for defining page table entries and other structures in nova-core.
>>
>> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
>> ---
>>  drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
>>  drivers/gpu/nova-core/nova_core.rs |   1 +
>>  2 files changed, 150 insertions(+)
>>  create mode 100644 drivers/gpu/nova-core/bitstruct.rs
>>
>> diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
>> new file mode 100644
>> index 000000000000..661a75da0a9c
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/bitstruct.rs
>> @@ -0,0 +1,149 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +//
>> +// bitstruct.rs ? C-style library for bitfield-packed Rust structures
>> +//
>> +// A library that provides support for defining bit fields in Rust
>> +// structures to circumvent lack of native language support for this.
>> +//
>> +// Similar usage syntax to the register! macro.
>> +
>> +use kernel::prelude::*;
>> +
>> +/// Macro for defining bitfield-packed structures in Rust.
>> +/// The size of the underlying storage type is specified with
#[repr(TYPE)].
>> +///
>> +/// # Example (just for illustration)
>> +/// ```rust
>> +/// bitstruct! {
>> +///     #[repr(u64)]
>> +///     pub struct PageTableEntry {
>> +///         0:0       present     as bool,
>> +///         1:1       writable    as bool,
>> +///         11:9      available   as u8,
>> +///         51:12     pfn         as u64,
>> +///         62:52     available2  as u16,
>> +///         63:63     nx          as bool,
>> +///     }
>> +/// }
>> +/// ```
>> +///
>> +/// This generates a struct with methods:
>> +/// - Constructor: `default()` sets all bits to zero.
>> +/// - Field accessors: `present()`, `pfn()`, etc.
>> +/// - Field setters: `set_present()`, `set_pfn()`, etc.
>> +/// - Builder methods: `with_present()`, `with_pfn()`, etc.
>> +/// - Raw conversion: `from_raw()`, `into_raw()`
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct {
>> +    (
>> +        #[repr($storage:ty)]
>> +        $vis:vis struct $name:ident {
>> +            $(
>> +                $hi:literal : $lo:literal $field:ident as
$field_type:tt
>> +            ),* $(,)?
>> +        }
>> +    ) => {
>> +        #[repr(transparent)]
>> +        #[derive(Copy, Clone, Default)]
>> +        $vis struct $name($storage);
>> +
>> +        impl $name {
>> +            /// Create from raw value
>> +            #[inline(always)]
>> +            $vis const fn from_raw(val: $storage) -> Self {
>> +                Self(val)
>> +            }
>> +
>> +            /// Get raw value
>> +            #[inline(always)]
>> +            $vis const fn into_raw(self) -> $storage {
>> +                self.0
>> +            }
>> +        }
>> +
>> +        impl core::fmt::Debug for $name {
>> +            fn fmt(&self, f: &mut
core::fmt::Formatter<'_>) -> core::fmt::Result {
>> +                write!(f, "{}({:#x})", stringify!($name),
self.0)
>> +            }
>> +        }
>> +
>> +        // Generate all field methods
>> +        $(
>> +            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo,
$field as $field_type);
>> +        )*
>> +    };
>> +}
>> +
>> +/// Helper to calculate mask for bit fields
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct_mask {
>> +    ($hi:literal, $lo:literal, $storage:ty) => {{
>> +        let width = ($hi - $lo + 1) as usize;
>> +        let storage_bits = 8 * core::mem::size_of::<$storage>();
>> +        if width >= storage_bits {
>> +            <$storage>::MAX
>> +        } else {
>> +            ((1 as $storage) << width) - 1
>> +        }
>> +    }};
>> +}
>> +
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct_field_impl {
>> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal,
$lo:literal, $field:ident as $field_type:tt) => {
>> +        impl $struct_name {
>> +            #[inline(always)]
>> +            $vis const fn $field(&self) -> $field_type {
>> +                let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
>> +                bitstruct_cast_value!(field_val, $field_type)
>> +            }
>> +        }
>> +        bitstruct_make_setters!($vis, $struct_name, $storage, $hi,
$lo, $field, $field_type);
>> +    };
>> +}
>> +
>> +/// Helper macro to convert extracted value to target type
>> +///
>> +/// Special handling for bool types is required because the `as`
keyword
>> +/// cannot be used to convert to bool in Rust. For bool fields, we
check
>> +/// if the extracted value is non-zero. For all other types, we use
the
>> +/// standard `as` conversion.
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct_cast_value {
>> +    ($field_val:expr, bool) => {
>> +        $field_val != 0
>> +    };
>> +    ($field_val:expr, $field_type:tt) => {
>> +        $field_val as $field_type
>> +    };
>> +}
>> +
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct_write_bits {
>> +    ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty)
=> {{
>> +        let mask = bitstruct_mask!($hi, $lo, $storage);
>> +        ($raw & !(mask << $lo)) | ((($val as $storage) &
mask) << $lo)
>> +    }};
>> +}
>> +
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct_make_setters {
>> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal,
$lo:literal, $field:ident, $field_type:tt) => {
>> +        ::kernel::macros::paste! {
>> +            impl $struct_name {
>> +                #[inline(always)]
>> +                #[allow(dead_code)]
>> +                $vis fn [<set_ $field>](&mut self, val:
$field_type) {
>> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo,
val, $storage);
>> +                }
>> +
>> +                #[inline(always)]
>> +                #[allow(dead_code)]
>> +                $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
>> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo,
val, $storage);
>> +                    self
>> +                }
>> +            }
>> +        }
>> +    };
>> +}
> 
> This is awesome. Is there a place for this to live outside of
> `nova-core`? I would think this would be extremely useful as a general
> helper for bitfield struct definitions.
About to send v2 which moves it. :)
> 
>> diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
>> index cb2bbb30cba1..54505cad4a73 100644
>> --- a/drivers/gpu/nova-core/nova_core.rs
>> +++ b/drivers/gpu/nova-core/nova_core.rs
>> @@ -2,6 +2,7 @@
>>  
>>  //! Nova Core GPU Driver
>>  
>> +mod bitstruct;
>>  mod dma;
>>  mod driver;
>>  mod falcon;
>> -- 
>> 2.34.1
> 
> Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev>
Thanks! Since the patches are a bit new, kindly review again.
 - Joel

Hi Joel,

(Thanks to John for referencing this.)

On Sun, Aug 24, 2025 at 09:59:52AM -0400, Joel Fernandes
wrote:
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs.
So maybe name it bitfield? 
> This will be used
> for defining page table entries and other structures in nova-core.
I think this is understatement, and this will find a broader use. :)
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
I agree with the others that this bitstruct is worth to live in core
directory. I just merged bitmap wrapper in rust/kernel/bitmap.rs, and
I think this one should go in rust/kernel/bitstruct.rs (or bitfield.rs?).

Can you please consider this change for v2, and also add the new file in
BITOPS API record in MAINTAINERS?

A couple nits inline.

Thanks,
Yury
> ---
>  drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
>  drivers/gpu/nova-core/nova_core.rs |   1 +
>  2 files changed, 150 insertions(+)
>  create mode 100644 drivers/gpu/nova-core/bitstruct.rs
> 
> diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
> new file mode 100644
> index 000000000000..661a75da0a9c
> --- /dev/null
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// bitstruct.rs ? C-style library for bitfield-packed Rust structures
> +//
> +// A library that provides support for defining bit fields in Rust
> +// structures to circumvent lack of native language support for this.
> +//
> +// Similar usage syntax to the register! macro.
> +
> +use kernel::prelude::*;
> +
> +/// Macro for defining bitfield-packed structures in Rust.
> +/// The size of the underlying storage type is specified with
#[repr(TYPE)].
> +///
> +/// # Example (just for illustration)
> +/// ```rust
> +/// bitstruct! {
> +///     #[repr(u64)]
> +///     pub struct PageTableEntry {
> +///         0:0       present     as bool,
> +///         1:1       writable    as bool,
> +///         11:9      available   as u8,
> +///         51:12     pfn         as u64,
> +///         62:52     available2  as u16,
> +///         63:63     nx          as bool,
> +///     }
> +/// }
Is it possible to create overlapping fields? Should we allow that?
(I guess yes.) Does your machinery handle it correctly now?

If the answer is yes, can you add a test for it?
> +/// ```
> +///
> +/// This generates a struct with methods:
> +/// - Constructor: `default()` sets all bits to zero.
> +/// - Field accessors: `present()`, `pfn()`, etc.
> +/// - Field setters: `set_present()`, `set_pfn()`, etc.
> +/// - Builder methods: `with_present()`, `with_pfn()`, etc.
> +/// - Raw conversion: `from_raw()`, `into_raw()`
> +#[allow(unused_macros)]
> +macro_rules! bitstruct {
> +    (
> +        #[repr($storage:ty)]
> +        $vis:vis struct $name:ident {
> +            $(
> +                $hi:literal : $lo:literal $field:ident as $field_type:tt
> +            ),* $(,)?
> +        }
> +    ) => {
> +        #[repr(transparent)]
> +        #[derive(Copy, Clone, Default)]
> +        $vis struct $name($storage);
> +
> +        impl $name {
> +            /// Create from raw value
> +            #[inline(always)]
> +            $vis const fn from_raw(val: $storage) -> Self {
> +                Self(val)
> +            }
> +
> +            /// Get raw value
> +            #[inline(always)]
> +            $vis const fn into_raw(self) -> $storage {
> +                self.0
> +            }
> +        }
> +
> +        impl core::fmt::Debug for $name {
> +            fn fmt(&self, f: &mut
core::fmt::Formatter<'_>) -> core::fmt::Result {
> +                write!(f, "{}({:#x})", stringify!($name),
self.0)
> +            }
> +        }
> +
> +        // Generate all field methods
> +        $(
> +            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field
as $field_type);
> +        )*
> +    };
> +}
> +
> +/// Helper to calculate mask for bit fields
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_mask {
> +    ($hi:literal, $lo:literal, $storage:ty) => {{
> +        let width = ($hi - $lo + 1) as usize;
> +        let storage_bits = 8 * core::mem::size_of::<$storage>();
Does this '8' mean BITS_PER_BYTE? If so, we've got BITS_PER_TYPE()
macro. Can
you use it here?
> +        if width >= storage_bits {
> +            <$storage>::MAX
This is an attempt to make an out-of-boundary access. Maybe print a
warning or similar? 

I actually think that if user wants to make an out-of-boundary access,
the best thing we can do is to keep the memory untouched. So, maybe
return None here, or 0, and make sure that the upper code doesn't
access it?
> +        } else {
> +            ((1 as $storage) << width) - 1
> +        }
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_field_impl {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident as $field_type:tt) => {
> +        impl $struct_name {
> +            #[inline(always)]
> +            $vis const fn $field(&self) -> $field_type {
> +                let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
> +                bitstruct_cast_value!(field_val, $field_type)
> +            }
> +        }
> +        bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo,
$field, $field_type);
> +    };
> +}
> +
> +/// Helper macro to convert extracted value to target type
> +///
> +/// Special handling for bool types is required because the `as` keyword
> +/// cannot be used to convert to bool in Rust. For bool fields, we check
> +/// if the extracted value is non-zero. For all other types, we use the
> +/// standard `as` conversion.
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_cast_value {
> +    ($field_val:expr, bool) => {
> +        $field_val != 0
> +    };
> +    ($field_val:expr, $field_type:tt) => {
> +        $field_val as $field_type
> +    };
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_write_bits {
> +    ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{
> +        let mask = bitstruct_mask!($hi, $lo, $storage);
> +        ($raw & !(mask << $lo)) | ((($val as $storage) &
mask) << $lo)
> +    }};
> +}
> +
> +#[allow(unused_macros)]
> +macro_rules! bitstruct_make_setters {
> +    ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident, $field_type:tt) => {
> +        ::kernel::macros::paste! {
> +            impl $struct_name {
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis fn [<set_ $field>](&mut self, val:
$field_type) {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                }
> +
> +                #[inline(always)]
> +                #[allow(dead_code)]
> +                $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
> +                    self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
> +                    self
> +                }
> +            }
> +        }
> +    };
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
> index cb2bbb30cba1..54505cad4a73 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,6 +2,7 @@
>  
>  //! Nova Core GPU Driver
>  
> +mod bitstruct;
>  mod dma;
>  mod driver;
>  mod falcon;
> -- 
> 2.34.1
>

On 9/4/2025 5:35 PM, Yury Norov wrote:
> Hi Joel,
> 
> (Thanks to John for referencing this.)
> 
> On Sun, Aug 24, 2025 at 09:59:52AM -0400, Joel Fernandes wrote:
>> Add a minimal bitfield library for defining in Rust structures (called
>> bitstruct), similar in concept to bit fields in C structs.
> 
> So maybe name it bitfield?
Sure, that sounds good to me. I wanted the name "struct" in it, but I
guess
since a caller is expected to pass "struct" in the invocation, it
should be
good, in fact better because "bitfield" adds more information, so
thank you!
> 
>> This will be used
>> for defining page table entries and other structures in nova-core.
> 
> I think this is understatement, and this will find a broader use. :)
Thanks :)
> 
>> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> 
> I agree with the others that this bitstruct is worth to live in core
> directory. I just merged bitmap wrapper in rust/kernel/bitmap.rs, and
> I think this one should go in rust/kernel/bitstruct.rs (or bitfield.rs?).
Sure, bitfield.rs sounds good to me.
> 
> Can you please consider this change for v2, and also add the new file in
> BITOPS API record in MAINTAINERS?
Yes.

[...]>> +/// # Example (just for illustration)
>> +/// ```rust
>> +/// bitstruct! {
>> +///     #[repr(u64)]
>> +///     pub struct PageTableEntry {
>> +///         0:0       present     as bool,
>> +///         1:1       writable    as bool,
>> +///         11:9      available   as u8,
>> +///         51:12     pfn         as u64,
>> +///         62:52     available2  as u16,
>> +///         63:63     nx          as bool,
>> +///     }
>> +/// }
> 
> Is it possible to create overlapping fields? Should we allow that?
> (I guess yes.) Does your machinery handle it correctly now?
> 
> If the answer is yes, can you add a test for it?
It is possible, but it is an unintended side effect. It is not the primary use
case though, but also there is no harm in doing it if the user needs it. I will
add a test case, I will include this in the K-Unit test in v3.
> 
>> +/// ```
>> +///
>> +/// This generates a struct with methods:
>> +/// - Constructor: `default()` sets all bits to zero.
>> +/// - Field accessors: `present()`, `pfn()`, etc.
>> +/// - Field setters: `set_present()`, `set_pfn()`, etc.
>> +/// - Builder methods: `with_present()`, `with_pfn()`, etc.
>> +/// - Raw conversion: `from_raw()`, `into_raw()`
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct {
>> +    (
>> +        #[repr($storage:ty)]
>> +        $vis:vis struct $name:ident {
>> +            $(
>> +                $hi:literal : $lo:literal $field:ident as
$field_type:tt
>> +            ),* $(,)?
>> +        }
>> +    ) => {
>> +        #[repr(transparent)]
>> +        #[derive(Copy, Clone, Default)]
>> +        $vis struct $name($storage);
>> +
>> +        impl $name {
>> +            /// Create from raw value
>> +            #[inline(always)]
>> +            $vis const fn from_raw(val: $storage) -> Self {
>> +                Self(val)
>> +            }
>> +
>> +            /// Get raw value
>> +            #[inline(always)]
>> +            $vis const fn into_raw(self) -> $storage {
>> +                self.0
>> +            }
>> +        }
>> +
>> +        impl core::fmt::Debug for $name {
>> +            fn fmt(&self, f: &mut
core::fmt::Formatter<'_>) -> core::fmt::Result {
>> +                write!(f, "{}({:#x})", stringify!($name),
self.0)
>> +            }
>> +        }
>> +
>> +        // Generate all field methods
>> +        $(
>> +            bitstruct_field_impl!($vis, $name, $storage, $hi, $lo,
$field as $field_type);
>> +        )*
>> +    };
>> +}
>> +
>> +/// Helper to calculate mask for bit fields
>> +#[allow(unused_macros)]
>> +macro_rules! bitstruct_mask {
>> +    ($hi:literal, $lo:literal, $storage:ty) => {{
>> +        let width = ($hi - $lo + 1) as usize;
>> +        let storage_bits = 8 * core::mem::size_of::<$storage>();
> 
> Does this '8' mean BITS_PER_BYTE? If so, we've got
BITS_PER_TYPE() macro. Can
> you use it here?
> 
>> +        if width >= storage_bits {
>> +            <$storage>::MAX
> 
> This is an attempt to make an out-of-boundary access. Maybe print a
> warning or similar? 
Only width > storage_bits is out-of-boundary. Also in v2 [1] I completely
replaced this part with genmask, so we are no longer calculating WIDTH.

+        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,
+                _ => <$storage>::MAX
+            }
+        };

I could add a build_assert here though to make sure size_of($storage) > ($hi
-
$lo), but I think genmask already has a build_assert:
                build_assert!(n < <$ty>::BITS);
so it may not be needed.

[1] https://lore.kernel.org/all/20250903215428.1296517-3-joelagnelf at
nvidia.com/

thanks,

 - Joel