Alexandre Courbot
2025-Sep-04 03:16 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
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.
Danilo Krummrich
2025-Sep-04 07:16 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
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
Daniel Almeida
2025-Sep-04 11:02 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
> On 4 Sep 2025, at 00:16, Alexandre Courbot <acourbot at 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