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

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

[Joel Fernandes]

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@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

[PATCH 2/2] nova-core: Add KUNIT tests for bitstruct

[Joel Fernandes]

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

Signed-off-by: Joel Fernandes <joelagnelf@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

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

[John Hubbard]

+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@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

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

[Boqun Feng]

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@nvidia.com>

FWIW, Joel, I only received patch 1/2.

Regards,
Boqun

[..]

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

[Joel Fernandes]

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@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
> 
> [..]

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

[Alexandre Courbot]

On Mon Aug 25, 2025 at 1:16 PM JST, Joel Fernandes wrote:
>
>
> 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@nvidia.com>
>> 
>> FWIW, Joel, I only received patch 1/2.
>
> Oh. Good catch, will CC you on 2/2 for the next time.

I believe the reason for that is that patch 1/2 has been send to
the rust-for-linux list, but not 2/2 despite it changing the Rust code
as well.

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

[Danilo Krummrich]

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@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

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

[Alexandre Courbot]

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@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.

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

[Joel Fernandes]

Hi Alex,

On 8/25/2025 7:07 AM, Alexandre Courbot wrote:
> 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.
> 

Ok, all these sound good to me.

>>
>> Signed-off-by: Joel Fernandes <joelagnelf@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?

I'd say its own since it is related to structures and keeps the file smaller.

>> @@ -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. :)

Ah true, already fixed.

> 
>> +
>> +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).

Perhaps future add-on enhancement to have both syntax? I'd like to initially
keep H:L and stabilize the code first, what do you think?

> 
>> +///     }
>> +/// }
>> +/// ```
>> +///
>> +/// 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

Sure, done.

> 
>> +        )*
>> +    };
>> +}
>> +
>> +/// 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?
> 

Maybe, I'll look into it, thanks.

>> +#[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.
Ack, done.
 - Joel

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

[Alexandre Courbot]

On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
<snip>
>>> +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).
>
> Perhaps future add-on enhancement to have both syntax? I'd like to initially
> keep H:L and stabilize the code first, what do you think?

Let's have the discussion with the other stakeholders (Daniel?). I think
in Nova we want to keep the `H:L` syntax, as it matches what the OpenRM
headers do (so Nova would have its own `register` macro that calls into
the common one, tweaking things as it needs). But in the kernel crate we
should use something intuitive for everyone.

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

[Danilo Krummrich]

On Thu Sep 4, 2025 at 5:16 AM CEST, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
> <snip>
>>>> +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).
>>
>> Perhaps future add-on enhancement to have both syntax? I'd like to initially
>> keep H:L and stabilize the code first, what do you think?
>
> Let's have the discussion with the other stakeholders (Daniel?). I think
> in Nova we want to keep the `H:L` syntax, as it matches what the OpenRM
> headers do (so Nova would have its own `register` macro that calls into
> the common one, tweaking things as it needs). But in the kernel crate we
> should use something intuitive for everyone.

I don't care too much about whether it's gonna be H:L or L:H [1], but I do care
about being consistent throughout the kernel. Let's not start the practice of
twisting kernel APIs to NV_* specific APIs that differ from what people are used
to work with in the kernel.

[1] If it's gonna be H:L, I think we should also list things in reverse order,
    i.e.:

	pub struct PageTableEntry {
	    63:63     nx          as bool,
	    62:52     available2  as u16,
	    51:12     pfn         as u64,
	    11:9      available   as u8,
	    1:1       writable    as bool,
	    0:0       present     as bool,
	}

This is also what would be my preferred style for the kernel in general.

- Danilo

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

[Alexandre Courbot]

On Thu Sep 4, 2025 at 4:16 PM JST, Danilo Krummrich wrote:
> On Thu Sep 4, 2025 at 5:16 AM CEST, Alexandre Courbot wrote:
>> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
>> <snip>
>>>>> +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).
>>>
>>> Perhaps future add-on enhancement to have both syntax? I'd like to initially
>>> keep H:L and stabilize the code first, what do you think?
>>
>> Let's have the discussion with the other stakeholders (Daniel?). I think
>> in Nova we want to keep the `H:L` syntax, as it matches what the OpenRM
>> headers do (so Nova would have its own `register` macro that calls into
>> the common one, tweaking things as it needs). But in the kernel crate we
>> should use something intuitive for everyone.
>
> I don't care too much about whether it's gonna be H:L or L:H [1], but I do care
> about being consistent throughout the kernel. Let's not start the practice of
> twisting kernel APIs to NV_* specific APIs that differ from what people are used
> to work with in the kernel.
>
> [1] If it's gonna be H:L, I think we should also list things in reverse order,
>     i.e.:
>
> 	pub struct PageTableEntry {
> 	    63:63     nx          as bool,
> 	    62:52     available2  as u16,
> 	    51:12     pfn         as u64,
> 	    11:9      available   as u8,
> 	    1:1       writable    as bool,
> 	    0:0       present     as bool,
> 	}
>
> This is also what would be my preferred style for the kernel in general.

Sorry for the confusion. The discussion was whether to keep using the
`H:L` syntax of the current macro, or use Rust's inclusive ranges syntax
(i.e. `L..=H`), as the `genmask_*` macros currently do.

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

[John Hubbard]

On 9/4/25 4:06 AM, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 4:16 PM JST, Danilo Krummrich wrote:
>> On Thu Sep 4, 2025 at 5:16 AM CEST, Alexandre Courbot wrote:
>>> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
>>> <snip>
>> 	pub struct PageTableEntry {
>> 	    63:63     nx          as bool,
>> 	    62:52     available2  as u16,
>> 	    51:12     pfn         as u64,
>> 	    11:9      available   as u8,
>> 	    1:1       writable    as bool,
>> 	    0:0       present     as bool,
>> 	}
>>
>> This is also what would be my preferred style for the kernel in general.
> 
> Sorry for the confusion. The discussion was whether to keep using the
> `H:L` syntax of the current macro, or use Rust's inclusive ranges syntax
> (i.e. `L..=H`), as the `genmask_*` macros currently do.
> 

The H:L (for example "11:9 available as u8", above) is elegant and readable.

The Rust native syntax "L..=H", much less so.

For this part of the kernel, dealing specifically with bits, feel pretty
strongly that we should go with "H:L".


thanks,
-- 
John Hubbard

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

[Alexandre Courbot]

On Sat Sep 6, 2025 at 6:29 AM JST, John Hubbard wrote:
> On 9/4/25 4:06 AM, Alexandre Courbot wrote:
>> On Thu Sep 4, 2025 at 4:16 PM JST, Danilo Krummrich wrote:
>>> On Thu Sep 4, 2025 at 5:16 AM CEST, Alexandre Courbot wrote:
>>>> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
>>>> <snip>
>>> 	pub struct PageTableEntry {
>>> 	    63:63     nx          as bool,
>>> 	    62:52     available2  as u16,
>>> 	    51:12     pfn         as u64,
>>> 	    11:9      available   as u8,
>>> 	    1:1       writable    as bool,
>>> 	    0:0       present     as bool,
>>> 	}
>>>
>>> This is also what would be my preferred style for the kernel in general.
>> 
>> Sorry for the confusion. The discussion was whether to keep using the
>> `H:L` syntax of the current macro, or use Rust's inclusive ranges syntax
>> (i.e. `L..=H`), as the `genmask_*` macros currently do.
>> 
>
> The H:L (for example "11:9 available as u8", above) is elegant and readable.
>
> The Rust native syntax "L..=H", much less so.
>
> For this part of the kernel, dealing specifically with bits, feel pretty
> strongly that we should go with "H:L".

We discussed that bit during the Rust DRM meeting, and the consensus was
indeed to go with `H:L` for these macros as this is the syntax typically
used in fields definitions.

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

[Joel Fernandes]

> On Sep 4, 2025, at 3:16 AM, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> On Thu Sep 4, 2025 at 5:16 AM CEST, Alexandre Courbot wrote:
>> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
>> <snip>
>>>>> +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).
>>> 
>>> Perhaps future add-on enhancement to have both syntax? I'd like to initially
>>> keep H:L and stabilize the code first, what do you think?
>> 
>> Let's have the discussion with the other stakeholders (Daniel?). I think
>> in Nova we want to keep the `H:L` syntax, as it matches what the OpenRM
>> headers do (so Nova would have its own `register` macro that calls into
>> the common one, tweaking things as it needs). But in the kernel crate we
>> should use something intuitive for everyone.
> 
> I don't care too much about whether it's gonna be H:L or L:H [1], but I do care
> about being consistent throughout the kernel. Let's not start the practice of
> twisting kernel APIs to NV_* specific APIs that differ from what people are used
> to work with in the kernel.
> 
> [1] If it's gonna be H:L, I think we should also list things in reverse order,
>    i.e.:
> 
>    pub struct PageTableEntry {
>        63:63     nx          as bool,
>        62:52     available2  as u16,
>        51:12     pfn         as u64,
>        11:9      available   as u8,
>        1:1       writable    as bool,
>        0:0       present     as bool,
>    }
> 
> This is also what would be my preferred style for the kernel in general.

Sure, that only means I have to change the example code since the
macro itself already supports that. I can do that.

Thanks.





> 
> - Danilo

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

[Daniel Almeida]

> On 4 Sep 2025, at 00:16, Alexandre Courbot <acourbot@nvidia.com> wrote:
> 
> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
> <snip>
>>>> +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).
>> 
>> Perhaps future add-on enhancement to have both syntax? I'd like to initially
>> keep H:L and stabilize the code first, what do you think?
> 
> Let's have the discussion with the other stakeholders (Daniel?). I think
> in Nova we want to keep the `H:L` syntax, as it matches what the OpenRM
> headers do (so Nova would have its own `register` macro that calls into
> the common one, tweaking things as it needs). But in the kernel crate we
> should use something intuitive for everyone.
> 

I don’t specifically care which syntax is used. We will adapt to it.

— Daniel

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

[Joel Fernandes]

> On Sep 3, 2025, at 11:16 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> 
> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
> <snip>
>>>> +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).
>> 
>> Perhaps future add-on enhancement to have both syntax? I'd like to initially
>> keep H:L and stabilize the code first, what do you think?
> 
> Let's have the discussion with the other stakeholders (Daniel?). I think
> in Nova we want to keep the `H:L` syntax, as it matches what the OpenRM
> headers do (so Nova would have its own `register` macro that calls into
> the common one, tweaking things as it needs). But in the kernel crate we
> should use something intuitive for everyone.

I do not think we should have a nova only register macro using an external
register macro. We should have just one outside-nova macro, and can
support both syntaxes with it if needed.

Though, to be honest, I am thinking only supporting H:L is more than
enough initially and others on the thread are also Ok with it.
We can always add support for the alternate syntax as well if needed, in the future.

Thanks.

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

[Elle Rhumsaa]

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@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@weathered-steel.dev>

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

[Joel Fernandes]

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@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@weathered-steel.dev>
Thanks! Since the patches are a bit new, kindly review again.
 - Joel

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

[Daniel Almeida]

Hi Joel,

> On 24 Aug 2025, at 10:59, Joel Fernandes <joelagnelf@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@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

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

[Joel Fernandes]

On 9/3/2025 9:29 AM, Daniel Almeida wrote:
> Hi Joel,
> 
>> On 24 Aug 2025, at 10:59, Joel Fernandes <joelagnelf@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@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.

Sure, added, thanks!

> 
>> +/// - 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?

Good idea, will change.

>> +    }};
>> +}
>> +
>> +#[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 :)

Glad to hear! I will send a v2 out today or tomorrow, thanks.

 - Joel

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

[Yury Norov]

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@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
> 

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

[Joel Fernandes]

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@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@nvidia.com/

thanks,

 - Joel