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
Alexandre Courbot
2025-Sep-04 11:06 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
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.
Joel Fernandes
2025-Sep-04 11:33 UTC
[PATCH 1/2] nova-core: Add a library for bitfields in Rust structs
> On Sep 4, 2025, at 3:16?AM, Danilo Krummrich <dakr at 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