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
Joel Fernandes
2025-Sep-04 11:32 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
> On Sep 3, 2025, at 11:16?PM, 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 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.