Joel Fernandes
2025-Aug-24 14:00 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
Add a minimal bitfield library for defining in Rust structures (called
bitstruct), similar in concept to bit fields in C structs. This will be used
for defining page table entries and other structures in nova-core.
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++
drivers/gpu/nova-core/nova_core.rs | 1 +
2 files changed, 150 insertions(+)
create mode 100644 drivers/gpu/nova-core/bitstruct.rs
diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
new file mode 100644
index 000000000000..661a75da0a9c
--- /dev/null
+++ b/drivers/gpu/nova-core/bitstruct.rs
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// bitstruct.rs ? C-style library for bitfield-packed Rust structures
+//
+// A library that provides support for defining bit fields in Rust
+// structures to circumvent lack of native language support for this.
+//
+// Similar usage syntax to the register! macro.
+
+use kernel::prelude::*;
+
+/// Macro for defining bitfield-packed structures in Rust.
+/// The size of the underlying storage type is specified with #[repr(TYPE)].
+///
+/// # Example (just for illustration)
+/// ```rust
+/// bitstruct! {
+/// #[repr(u64)]
+/// pub struct PageTableEntry {
+/// 0:0 present as bool,
+/// 1:1 writable as bool,
+/// 11:9 available as u8,
+/// 51:12 pfn as u64,
+/// 62:52 available2 as u16,
+/// 63:63 nx as bool,
+/// }
+/// }
+/// ```
+///
+/// This generates a struct with methods:
+/// - Constructor: `default()` sets all bits to zero.
+/// - Field accessors: `present()`, `pfn()`, etc.
+/// - Field setters: `set_present()`, `set_pfn()`, etc.
+/// - Builder methods: `with_present()`, `with_pfn()`, etc.
+/// - Raw conversion: `from_raw()`, `into_raw()`
+#[allow(unused_macros)]
+macro_rules! bitstruct {
+ (
+ #[repr($storage:ty)]
+ $vis:vis struct $name:ident {
+ $(
+ $hi:literal : $lo:literal $field:ident as $field_type:tt
+ ),* $(,)?
+ }
+ ) => {
+ #[repr(transparent)]
+ #[derive(Copy, Clone, Default)]
+ $vis struct $name($storage);
+
+ impl $name {
+ /// Create from raw value
+ #[inline(always)]
+ $vis const fn from_raw(val: $storage) -> Self {
+ Self(val)
+ }
+
+ /// Get raw value
+ #[inline(always)]
+ $vis const fn into_raw(self) -> $storage {
+ self.0
+ }
+ }
+
+ impl core::fmt::Debug for $name {
+ fn fmt(&self, f: &mut core::fmt::Formatter<'_>)
-> core::fmt::Result {
+ write!(f, "{}({:#x})", stringify!($name), self.0)
+ }
+ }
+
+ // Generate all field methods
+ $(
+ bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field as
$field_type);
+ )*
+ };
+}
+
+/// Helper to calculate mask for bit fields
+#[allow(unused_macros)]
+macro_rules! bitstruct_mask {
+ ($hi:literal, $lo:literal, $storage:ty) => {{
+ let width = ($hi - $lo + 1) as usize;
+ let storage_bits = 8 * core::mem::size_of::<$storage>();
+ if width >= storage_bits {
+ <$storage>::MAX
+ } else {
+ ((1 as $storage) << width) - 1
+ }
+ }};
+}
+
+#[allow(unused_macros)]
+macro_rules! bitstruct_field_impl {
+ ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident as $field_type:tt) => {
+ impl $struct_name {
+ #[inline(always)]
+ $vis const fn $field(&self) -> $field_type {
+ let field_val = (self.0 >> $lo) &
bitstruct_mask!($hi, $lo, $storage);
+ bitstruct_cast_value!(field_val, $field_type)
+ }
+ }
+ bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo, $field,
$field_type);
+ };
+}
+
+/// Helper macro to convert extracted value to target type
+///
+/// Special handling for bool types is required because the `as` keyword
+/// cannot be used to convert to bool in Rust. For bool fields, we check
+/// if the extracted value is non-zero. For all other types, we use the
+/// standard `as` conversion.
+#[allow(unused_macros)]
+macro_rules! bitstruct_cast_value {
+ ($field_val:expr, bool) => {
+ $field_val != 0
+ };
+ ($field_val:expr, $field_type:tt) => {
+ $field_val as $field_type
+ };
+}
+
+#[allow(unused_macros)]
+macro_rules! bitstruct_write_bits {
+ ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{
+ let mask = bitstruct_mask!($hi, $lo, $storage);
+ ($raw & !(mask << $lo)) | ((($val as $storage) & mask)
<< $lo)
+ }};
+}
+
+#[allow(unused_macros)]
+macro_rules! bitstruct_make_setters {
+ ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal,
$field:ident, $field_type:tt) => {
+ ::kernel::macros::paste! {
+ impl $struct_name {
+ #[inline(always)]
+ #[allow(dead_code)]
+ $vis fn [<set_ $field>](&mut self, val: $field_type)
{
+ self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
+ }
+
+ #[inline(always)]
+ #[allow(dead_code)]
+ $vis const fn [<with_ $field>](mut self, val:
$field_type) -> Self {
+ self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val,
$storage);
+ self
+ }
+ }
+ }
+ };
+}
diff --git a/drivers/gpu/nova-core/nova_core.rs
b/drivers/gpu/nova-core/nova_core.rs
index cb2bbb30cba1..54505cad4a73 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,7 @@
//! Nova Core GPU Driver
+mod bitstruct;
mod dma;
mod driver;
mod falcon;
--
2.34.1
Add KUNIT tests to make sure the macros are working correctly.
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
---
drivers/gpu/nova-core/bitstruct.rs | 198 +++++++++++++++++++++++++++++
1 file changed, 198 insertions(+)
diff --git a/drivers/gpu/nova-core/bitstruct.rs
b/drivers/gpu/nova-core/bitstruct.rs
index 661a75da0a9c..7ce2f4ecfbba 100644
--- a/drivers/gpu/nova-core/bitstruct.rs
+++ b/drivers/gpu/nova-core/bitstruct.rs
@@ -147,3 +147,201 @@ impl $struct_name {
}
};
}
+
+#[kunit_tests(nova_core_bitstruct)]
+mod kunit_tests {
+ // These are dummy structures just for testing (not real hardware).
+ bitstruct! {
+ #[repr(u64)]
+ struct TestPageTableEntry {
+ 0:0 present as bool,
+ 1:1 writable as bool,
+ 11:9 available as u8,
+ 51:12 pfn as u64,
+ 61:52 available2 as u16,
+ }
+ }
+
+ bitstruct! {
+ #[repr(u16)]
+ struct TestControlRegister {
+ 0:0 enable as bool,
+ 3:1 mode as u8,
+ 7:4 priority as u8,
+ 15:8 channel as u8,
+ }
+ }
+
+ bitstruct! {
+ #[repr(u8)]
+ struct TestStatusRegister {
+ 0:0 ready as bool,
+ 1:1 error as bool,
+ 3:2 state as u8,
+ 7:4 reserved as u8,
+ }
+ }
+
+ #[test]
+ fn test_single_bits() {
+ let mut pte = TestPageTableEntry::default();
+
+ // Test initial state of boolean fields - expected false
+ assert!(!pte.present());
+ assert!(!pte.writable());
+
+ pte.set_present(true);
+ assert!(pte.present());
+
+ pte.set_writable(true);
+ assert!(pte.writable());
+
+ pte.set_writable(false);
+ assert!(!pte.writable());
+
+ // Test available field (3 bits)
+ assert_eq!(pte.available(), 0);
+ pte.set_available(0x5);
+ assert_eq!(pte.available(), 0x5);
+ }
+
+ #[test]
+ fn test_range_fields() {
+ let mut pte = TestPageTableEntry::default();
+
+ pte.set_pfn(0x123456);
+ assert_eq!(pte.pfn(), 0x123456);
+
+ pte.set_available(0x7);
+ assert_eq!(pte.available(), 0x7);
+
+ pte.set_available2(0x3FF);
+ assert_eq!(pte.available2(), 0x3FF);
+
+ let max_pfn = (1u64 << 40) - 1; // 40 bits for pfn
+ pte.set_pfn(max_pfn);
+ assert_eq!(pte.pfn(), max_pfn);
+ }
+
+ #[test]
+ fn test_builder_pattern() {
+ let pte = TestPageTableEntry::default()
+ .with_present(true)
+ .with_writable(true)
+ .with_available(0x7)
+ .with_pfn(0xABCDEF)
+ .with_available2(0x3FF);
+
+ assert!(pte.present());
+ assert!(pte.writable());
+ assert_eq!(pte.available(), 0x7);
+ assert_eq!(pte.pfn(), 0xABCDEF);
+ assert_eq!(pte.available2(), 0x3FF);
+ }
+
+ #[test]
+ fn test_raw_operations() {
+ let raw_value = 0x3FF0000000123E03u64;
+ let pte = TestPageTableEntry::from_raw(raw_value);
+ assert_eq!(pte.into_raw(), raw_value);
+
+ assert!(pte.present()); // bit 0
+ assert!(pte.writable()); // bit 1
+ assert_eq!(pte.available(), 0x7); // bits 9-11: 0xE00 >> 9 = 0x7
+ assert_eq!(pte.pfn(), 0x123); // bits 12-51: 0x123 << 12
+ assert_eq!(pte.available2(), 0x3FF); // bits 52-61: 0x3FF << 52
+ }
+
+ #[test]
+ fn test_u16_bitstruct() {
+ let mut ctrl = TestControlRegister::default();
+
+ // Test initial state
+ assert!(!ctrl.enable());
+ assert_eq!(ctrl.mode(), 0);
+ assert_eq!(ctrl.priority(), 0);
+ assert_eq!(ctrl.channel(), 0);
+
+ ctrl.set_enable(true);
+ assert!(ctrl.enable());
+
+ ctrl.set_mode(0x5);
+ assert_eq!(ctrl.mode(), 0x5);
+
+ ctrl.set_priority(0xF);
+ assert_eq!(ctrl.priority(), 0xF);
+
+ ctrl.set_channel(0xAB);
+ assert_eq!(ctrl.channel(), 0xAB);
+
+ // Test u16 builder pattern
+ let ctrl2 = TestControlRegister::default()
+ .with_enable(true)
+ .with_mode(0x3)
+ .with_priority(0x8)
+ .with_channel(0x42);
+
+ assert!(ctrl2.enable());
+ assert_eq!(ctrl2.mode(), 0x3);
+ assert_eq!(ctrl2.priority(), 0x8);
+ assert_eq!(ctrl2.channel(), 0x42);
+
+ // Test u16 raw operations
+ let raw_value: u16 = 0x4281;
+ let ctrl3 = TestControlRegister::from_raw(raw_value);
+ assert_eq!(ctrl3.into_raw(), raw_value);
+ assert!(ctrl3.enable());
+ assert_eq!(ctrl3.priority(), 0x8);
+ assert_eq!(ctrl3.channel(), 0x42);
+ }
+
+ #[test]
+ fn test_u8_bitstruct() {
+ let mut status = TestStatusRegister::default();
+
+ // Test initial state
+ assert!(!status.ready());
+ assert!(!status.error());
+ assert_eq!(status.state(), 0);
+ assert_eq!(status.reserved(), 0);
+
+ status.set_ready(true);
+ assert!(status.ready());
+
+ status.set_error(true);
+ assert!(status.error());
+
+ status.set_state(0x3);
+ assert_eq!(status.state(), 0x3);
+
+ status.set_reserved(0xA);
+ assert_eq!(status.reserved(), 0xA);
+
+ // Test u8 builder pattern
+ let status2 = TestStatusRegister::default()
+ .with_ready(true)
+ .with_state(0x2)
+ .with_reserved(0x5);
+
+ assert!(status2.ready());
+ assert!(!status2.error());
+ assert_eq!(status2.state(), 0x2);
+ assert_eq!(status2.reserved(), 0x5);
+
+ // Test u8 raw operations
+ let raw_value: u8 = 0x59;
+ let status3 = TestStatusRegister::from_raw(raw_value);
+ assert_eq!(status3.into_raw(), raw_value);
+ assert!(status3.ready());
+ assert!(!status3.error());
+ assert_eq!(status3.state(), 0x2);
+ assert_eq!(status3.reserved(), 0x5);
+
+ // Test max raw value - all bits set
+ let status4 = TestStatusRegister::from_raw(0xFF);
+ assert!(status4.ready());
+ assert!(status4.error());
+ assert_eq!(status4.state(), 0x3);
+ assert_eq!(status4.reserved(), 0xF);
+ }
+}
--
2.34.1
John Hubbard
2025-Aug-24 17:37 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
+Cc: bitmap maintainer/reviewers: Yury Norov, Rasmus Villemoes On 8/24/25 6:59 AM, Joel Fernandes wrote:> Add a minimal bitfield library for defining in Rust structures (called > bitstruct), similar in concept to bit fields in C structs. This will be used > for defining page table entries and other structures in nova-core. > > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> > --- > drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++ > drivers/gpu/nova-core/nova_core.rs | 1 + > 2 files changed, 150 insertions(+) > create mode 100644 drivers/gpu/nova-core/bitstruct.rs > > diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs > new file mode 100644 > index 000000000000..661a75da0a9c > --- /dev/null > +++ b/drivers/gpu/nova-core/bitstruct.rs > @@ -0,0 +1,149 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// bitstruct.rs ? C-style library for bitfield-packed Rust structures > +// > +// A library that provides support for defining bit fields in Rust > +// structures to circumvent lack of native language support for this. > +// > +// Similar usage syntax to the register! macro. > + > +use kernel::prelude::*; > + > +/// Macro for defining bitfield-packed structures in Rust. > +/// The size of the underlying storage type is specified with #[repr(TYPE)]. > +/// > +/// # Example (just for illustration) > +/// ```rust > +/// bitstruct! { > +/// #[repr(u64)] > +/// pub struct PageTableEntry { > +/// 0:0 present as bool, > +/// 1:1 writable as bool, > +/// 11:9 available as u8, > +/// 51:12 pfn as u64, > +/// 62:52 available2 as u16, > +/// 63:63 nx as bool, > +/// } > +/// } > +/// ``` > +/// > +/// This generates a struct with methods: > +/// - Constructor: `default()` sets all bits to zero. > +/// - Field accessors: `present()`, `pfn()`, etc. > +/// - Field setters: `set_present()`, `set_pfn()`, etc. > +/// - Builder methods: `with_present()`, `with_pfn()`, etc. > +/// - Raw conversion: `from_raw()`, `into_raw()` > +#[allow(unused_macros)] > +macro_rules! bitstruct { > + ( > + #[repr($storage:ty)] > + $vis:vis struct $name:ident { > + $( > + $hi:literal : $lo:literal $field:ident as $field_type:tt > + ),* $(,)? > + } > + ) => { > + #[repr(transparent)] > + #[derive(Copy, Clone, Default)] > + $vis struct $name($storage); > + > + impl $name { > + /// Create from raw value > + #[inline(always)] > + $vis const fn from_raw(val: $storage) -> Self { > + Self(val) > + } > + > + /// Get raw value > + #[inline(always)] > + $vis const fn into_raw(self) -> $storage { > + self.0 > + } > + } > + > + impl core::fmt::Debug for $name { > + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { > + write!(f, "{}({:#x})", stringify!($name), self.0) > + } > + } > + > + // Generate all field methods > + $( > + bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field as $field_type); > + )* > + }; > +} > + > +/// Helper to calculate mask for bit fields > +#[allow(unused_macros)] > +macro_rules! bitstruct_mask { > + ($hi:literal, $lo:literal, $storage:ty) => {{ > + let width = ($hi - $lo + 1) as usize; > + let storage_bits = 8 * core::mem::size_of::<$storage>(); > + if width >= storage_bits { > + <$storage>::MAX > + } else { > + ((1 as $storage) << width) - 1 > + } > + }}; > +} > + > +#[allow(unused_macros)] > +macro_rules! bitstruct_field_impl { > + ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal, $field:ident as $field_type:tt) => { > + impl $struct_name { > + #[inline(always)] > + $vis const fn $field(&self) -> $field_type { > + let field_val = (self.0 >> $lo) & bitstruct_mask!($hi, $lo, $storage); > + bitstruct_cast_value!(field_val, $field_type) > + } > + } > + bitstruct_make_setters!($vis, $struct_name, $storage, $hi, $lo, $field, $field_type); > + }; > +} > + > +/// Helper macro to convert extracted value to target type > +/// > +/// Special handling for bool types is required because the `as` keyword > +/// cannot be used to convert to bool in Rust. For bool fields, we check > +/// if the extracted value is non-zero. For all other types, we use the > +/// standard `as` conversion. > +#[allow(unused_macros)] > +macro_rules! bitstruct_cast_value { > + ($field_val:expr, bool) => { > + $field_val != 0 > + }; > + ($field_val:expr, $field_type:tt) => { > + $field_val as $field_type > + }; > +} > + > +#[allow(unused_macros)] > +macro_rules! bitstruct_write_bits { > + ($raw:expr, $hi:literal, $lo:literal, $val:expr, $storage:ty) => {{ > + let mask = bitstruct_mask!($hi, $lo, $storage); > + ($raw & !(mask << $lo)) | ((($val as $storage) & mask) << $lo) > + }}; > +} > + > +#[allow(unused_macros)] > +macro_rules! bitstruct_make_setters { > + ($vis:vis, $struct_name:ident, $storage:ty, $hi:literal, $lo:literal, $field:ident, $field_type:tt) => { > + ::kernel::macros::paste! { > + impl $struct_name { > + #[inline(always)] > + #[allow(dead_code)] > + $vis fn [<set_ $field>](&mut self, val: $field_type) { > + self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val, $storage); > + } > + > + #[inline(always)] > + #[allow(dead_code)] > + $vis const fn [<with_ $field>](mut self, val: $field_type) -> Self { > + self.0 = bitstruct_write_bits!(self.0, $hi, $lo, val, $storage); > + self > + } > + } > + } > + }; > +} > diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs > index cb2bbb30cba1..54505cad4a73 100644 > --- a/drivers/gpu/nova-core/nova_core.rs > +++ b/drivers/gpu/nova-core/nova_core.rs > @@ -2,6 +2,7 @@ > > //! Nova Core GPU Driver > > +mod bitstruct; > mod dma; > mod driver; > mod falcon;thanks, -- John Hubbard
Danilo Krummrich
2025-Aug-25 10:46 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
On 8/24/25 3:59 PM, Joel Fernandes wrote:> Add a minimal bitfield library for defining in Rust structures (called > bitstruct), similar in concept to bit fields in C structs. This will be used > for defining page table entries and other structures in nova-core. > > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> > --- > drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++ > drivers/gpu/nova-core/nova_core.rs | 1 + > 2 files changed, 150 insertions(+) > create mode 100644 drivers/gpu/nova-core/bitstruct.rsI 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
Alexandre Courbot
2025-Aug-25 11:07 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
Hi Joel, On Sun Aug 24, 2025 at 10:59 PM JST, Joel Fernandes wrote:> Add a minimal bitfield library for defining in Rust structures (called > bitstruct), similar in concept to bit fields in C structs. This will be used > for defining page table entries and other structures in nova-core.This is basically a rewrite (with some improvements, and some missing features) of the part of the `register!` macro that deals with bitfields. We planned to extract it into its own macro, and the split is already effective in its internal rules, so I'd suggest to just move the relevant rules into a new macro (as it will give you a couple useful features, like automatic conversion to enum types), and then apply your improvements on top of it. Otherwise we will end up with two implementations of the same thing, for no good justification IMHO. We were also planning to move the `register!` macro into the kernel crate this cycle so Tyr can use it, but this changes the plan a bit. Actually it is helpful, since your version implements one thing that we needed in the `register!` macro before moving it: visibility specifiers. So I would do things in this order: 1. Extract the bitfield-related code from the `register!` macro into its own macro (in nova-core), and make `register!` call into it, 2. Add support for visibility specifiers and non-u32 types in your macro and `register!`, 3. Move both macros to the kernel crate (hopefully in time for the next merge window so Tyr can make use of them). A few more comments inline.> > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> > --- > drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++ > drivers/gpu/nova-core/nova_core.rs | 1 + > 2 files changed, 150 insertions(+) > create mode 100644 drivers/gpu/nova-core/bitstruct.rs > > diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs > new file mode 100644 > index 000000000000..661a75da0a9c > --- /dev/null > +++ b/drivers/gpu/nova-core/bitstruct.rsI 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.
Daniel Almeida
2025-Sep-03 13:29 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
Hi Joel,> On 24 Aug 2025, at 10:59, Joel Fernandes <joelagnelf at nvidia.com> wrote: > > Add a minimal bitfield library for defining in Rust structures (called > bitstruct), similar in concept to bit fields in C structs. This will be used > for defining page table entries and other structures in nova-core. > > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> > --- > drivers/gpu/nova-core/bitstruct.rs | 149 +++++++++++++++++++++++++++++ > drivers/gpu/nova-core/nova_core.rs | 1 + > 2 files changed, 150 insertions(+) > create mode 100644 drivers/gpu/nova-core/bitstruct.rs > > diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs > new file mode 100644 > index 000000000000..661a75da0a9c > --- /dev/null > +++ b/drivers/gpu/nova-core/bitstruct.rs > @@ -0,0 +1,149 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// bitstruct.rs ? C-style library for bitfield-packed Rust structures > +// > +// A library that provides support for defining bit fields in Rust > +// structures to circumvent lack of native language support for this. > +// > +// Similar usage syntax to the register! macro. > + > +use kernel::prelude::*; > + > +/// Macro for defining bitfield-packed structures in Rust. > +/// The size of the underlying storage type is specified with #[repr(TYPE)]. > +/// > +/// # Example (just for illustration) > +/// ```rust > +/// bitstruct! { > +/// #[repr(u64)] > +/// pub struct PageTableEntry { > +/// 0:0 present as bool, > +/// 1:1 writable as bool, > +/// 11:9 available as u8, > +/// 51:12 pfn as u64, > +/// 62:52 available2 as u16, > +/// 63:63 nx as bool, > +/// } > +/// } > +/// ``` > +/// > +/// This generates a struct with methods: > +/// - Constructor: `default()` sets all bits to zero. > +/// - Field accessors: `present()`, `pfn()`, etc. > +/// - Field setters: `set_present()`, `set_pfn()`, etc. > +/// - Builder methods: `with_present()`, `with_pfn()`, etc.I think this could use a short example highlighting the builder pattern. It may be initially unclear that the methods can be chained, even though the word ?builder? is being used.> +/// - Raw conversion: `from_raw()`, `into_raw()` > +#[allow(unused_macros)] > +macro_rules! bitstruct { > + ( > + #[repr($storage:ty)] > + $vis:vis struct $name:ident { > + $( > + $hi:literal : $lo:literal $field:ident as $field_type:tt > + ),* $(,)? > + } > + ) => { > + #[repr(transparent)] > + #[derive(Copy, Clone, Default)] > + $vis struct $name($storage); > + > + impl $name { > + /// Create from raw value > + #[inline(always)] > + $vis const fn from_raw(val: $storage) -> Self { > + Self(val) > + } > + > + /// Get raw value > + #[inline(always)] > + $vis const fn into_raw(self) -> $storage { > + self.0 > + } > + } > + > + impl core::fmt::Debug for $name { > + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { > + write!(f, "{}({:#x})", stringify!($name), self.0) > + } > + } > + > + // Generate all field methods > + $( > + bitstruct_field_impl!($vis, $name, $storage, $hi, $lo, $field as $field_type); > + )* > + }; > +} > + > +/// Helper to calculate mask for bit fields > +#[allow(unused_macros)] > +macro_rules! bitstruct_mask { > + ($hi:literal, $lo:literal, $storage:ty) => {{ > + let width = ($hi - $lo + 1) as usize; > + let storage_bits = 8 * core::mem::size_of::<$storage>(); > + if width >= storage_bits { > + <$storage>::MAX > + } else { > + ((1 as $storage) << width) - 1Can?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