Joel Fernandes
2025-Oct-09 21:33 UTC
[PATCH RFC v2 2/3] rust: kernel: add bounded integer types
Hi Alex, Great effort, thanks. I replied with few comments below. Since the patch is large, it would be great if could be possibly split. Maybe the From primitives deserve a separate patch. On Thu, Oct 09, 2025 at 09:37:09PM +0900, Alexandre Courbot wrote:> Add the BoundedInt type, which restricts the number of bits allowed to > be used in a given integer value. This is useful to carry guarantees > when setting bitfields. > > Alongside this type, many `From` and `TryFrom` implementations are > provided to reduce friction when using with regular integer types. Proxy > implementations of common integer traits are also provided. > > Signed-off-by: Alexandre Courbot <acourbot at nvidia.com> > --- > rust/kernel/lib.rs | 1 + > rust/kernel/num.rs | 499 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 500 insertions(+) > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index fcffc3988a90..21c1f452ee6a 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -101,6 +101,7 @@ > pub mod mm; > #[cfg(CONFIG_NET)] > pub mod net; > +pub mod num; > pub mod of; > #[cfg(CONFIG_PM_OPP)] > pub mod opp; > diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs > new file mode 100644 > index 000000000000..b2aad95ce51c > --- /dev/null > +++ b/rust/kernel/num.rs > @@ -0,0 +1,499 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Numerical types for the kernel. > + > +use kernel::prelude::*; > + > +/// Integer type for which only the bits `0..NUM_BITS` are valid. > +/// > +/// # Invariants > +/// > +/// Stored values are represented with at most `NUM_BITS` bits. > +#[repr(transparent)] > +#[derive(Clone, Copy, Debug, Default, Hash)] > +pub struct BoundedInt<T, const NUM_BITS: u32>(T); > + > +/// Returns `true` if `$value` can be represented with at most `$NUM_BITS` on `$type`. > +macro_rules! is_in_bounds { > + ($value:expr, $type:ty, $num_bits:expr) => {{ > + let v = $value; > + v & <$type as Boundable<NUM_BITS>>::MASK == v > + }}; > +} > + > +/// Trait for primitive integer types that can be used with `BoundedInt`. > +pub trait Boundable<const NUM_BITS: u32> > +where > + Self: Sized + Copy + core::ops::BitAnd<Output = Self> + core::cmp::PartialEq, > + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>, > +{ > + /// Mask of the valid bits for this type. > + const MASK: Self; > + > + /// Returns `true` if `value` can be represented with at most `NUM_BITS`. > + /// > + /// TODO: post-RFC: replace this with a left-shift followed by right-shift operation. This will > + /// allow us to handle signed values as well. > + fn is_in_bounds(value: Self) -> bool { > + is_in_bounds!(value, Self, NUM_BITS) > + } > +} > + > +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u8 { > + const MASK: u8 = crate::bits::genmask_u8(0..=(NUM_BITS - 1)); > +} > + > +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u16 { > + const MASK: u16 = crate::bits::genmask_u16(0..=(NUM_BITS - 1)); > +} > + > +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u32 { > + const MASK: u32 = crate::bits::genmask_u32(0..=(NUM_BITS - 1)); > +} > + > +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u64 { > + const MASK: u64 = crate::bits::genmask_u64(0..=(NUM_BITS - 1)); > +} > + > +impl<T, const NUM_BITS: u32> BoundedInt<T, NUM_BITS> > +where > + T: Boundable<NUM_BITS>, > +{ > + /// Checks that `value` is valid for this type at compile-time and build a new value. > + /// > + /// This relies on [`build_assert!`] to perform validation at compile-time. If `value` cannot > + /// be inferred to be in bounds at compile-time, use the fallible [`Self::try_new`] instead. > + /// > + /// When possible, use one of the `new_const` methods instead of this method as it statically > + /// validates `value` instead of relying on the compiler's optimizations.This sounds like, users might use the less-optimal API first with the same build_assert issues we had with the IO accessors, since new() sounds very obvious. How about the following naming? new::<VALUE>() // Primary constructor for constants using const generics. try_new(value) // Keep as-is for fallible runtime new_from_expr(value) // For compile-time validated runtime values If new::<VALUE>() does not work for the user, the compiler will fail. Or, new_from_expr() could be from_value(), Ok with either naming or a better name.> + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::BoundedInt; > + /// > + /// # fn some_number() -> u32 { 0xffffffff } > + /// > + /// assert_eq!(BoundedInt::<u8, 1>::new(1).get(), 1); > + /// assert_eq!(BoundedInt::<u16, 8>::new(0xff).get(), 0xff); > + /// > + /// // Triggers a build error as `0x1ff` doesn't fit into 8 bits. > + /// // assert_eq!(BoundedInt::<u32, 8>::new(0x1ff).get(), 0x1ff); > + /// > + /// let v: u32 = some_number(); > + /// // Triggers a build error as `v` cannot be asserted to fit within 4 bits... > + /// // let _ = BoundedInt::<u32, 4>::new(v); > + /// // ... but this works as the compiler can assert the range from the mask. > + /// let _ = BoundedInt::<u32, 4>::new(v & 0xf); > + /// ``` > + pub fn new(value: T) -> Self { > + crate::build_assert!( > + T::is_in_bounds(value), > + "Provided parameter is larger than maximal supported value" > + ); > + > + Self(value) > + } > + > + /// Attempts to convert `value` into a value bounded by `NUM_BITS`. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::BoundedInt; > + /// > + /// assert_eq!(BoundedInt::<u8, 1>::try_new(1).map(|v| v.get()), Ok(1)); > + /// assert_eq!(BoundedInt::<u16, 8>::try_new(0xff).map(|v| v.get()), Ok(0xff)); > + /// > + /// // `0x1ff` doesn't fit into 8 bits. > + /// assert_eq!(BoundedInt::<u32, 8>::try_new(0x1ff), Err(EOVERFLOW)); > + /// ``` > + pub fn try_new(value: T) -> Result<Self> { > + if !T::is_in_bounds(value) { > + Err(EOVERFLOW) > + } else { > + Ok(Self(value)) > + } > + } > + > + /// Returns the contained value as a primitive type. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::BoundedInt; > + /// > + /// let v = BoundedInt::<u32, 4>::new_const::<7>(); > + /// assert_eq!(v.get(), 7u32); > + /// ``` > + pub fn get(self) -> T { > + if !T::is_in_bounds(self.0) { > + // SAFETY: Per the invariants, `self.0` cannot have bits set outside of `MASK`, so > + // this block will > + // never be reached. > + unsafe { core::hint::unreachable_unchecked() } > + }Does this if block help the compiler generate better code? I wonder if code gen could be checked to confirm the rationale.> + self.0 > + } > + > + /// Increase the number of bits usable for `self`. > + /// > + /// This operation cannot fail. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::BoundedInt; > + /// > + /// let v = BoundedInt::<u32, 4>::new_const::<7>(); > + /// let larger_v = v.enlarge::<12>(); > + /// // The contained values are equal even though `larger_v` has a bigger capacity. > + /// assert_eq!(larger_v, v); > + /// ``` > + pub const fn enlarge<const NEW_NUM_BITS: u32>(self) -> BoundedInt<T, NEW_NUM_BITS> > + where > + T: Boundable<NEW_NUM_BITS>, > + T: Copy,Boundable already implies copy so T: Copy is redundant.> + { > + build_assert!(NEW_NUM_BITS >= NUM_BITS); > + > + // INVARIANT: the value did fit within `NUM_BITS`, so it will all the more fit within > + // `NEW_NUM_BITS` which is larger. > + BoundedInt(self.0) > + } > + > + /// Shrink the number of bits usable for `self`. > + /// > + /// Returns `EOVERFLOW` if the value of `self` cannot be represented within `NEW_NUM_BITS`. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::BoundedInt; > + /// > + /// let v = BoundedInt::<u32, 12>::new_const::<7>(); > + /// let smaller_v = v.shrink::<4>()?; > + /// // The contained values are equal even though `smaller_v` has a smaller capacity. > + /// assert_eq!(smaller_v, v); > + /// > + /// # Ok::<(), Error>(()) > + /// ``` > + pub fn shrink<const NEW_NUM_BITS: u32>(self) -> Result<BoundedInt<T, NEW_NUM_BITS>> > + where > + T: Boundable<NEW_NUM_BITS>, > + T: Copy,Here too. [...]> +impl_const_new!(u8 u16 u32 u64); > + > +/// Declares a new `$trait` and implements it for all bounded types represented using `$num_bits`. > +/// > +/// This is used to declare properties as traits that we can use for later implementations. > +macro_rules! impl_size_rule { > + ($trait:ident, $($num_bits:literal)*) => { > + trait $trait {} > + > + $( > + impl<T> $trait for BoundedInt<T, $num_bits> where T: Boundable<$num_bits> {} > + )* > + }; > +} > + > +// Bounds that are larger than a `u64`. > +impl_size_rule!(LargerThanU64, 64); > + > +// Bounds that are larger than a `u32`. > +impl_size_rule!(LargerThanU32, > + 32 33 34 35 36 37 38 39If num_bits == 32 (number of bits), how could BoundedInt<T, 32> a LargerThanU32? It should be AtleastU32 or something. Or the above list should start from 33. Only a >= 33-bit wide integer can be LargerThanU32. Thanks.
Alexandre Courbot
2025-Oct-10 08:49 UTC
[PATCH RFC v2 2/3] rust: kernel: add bounded integer types
On Fri Oct 10, 2025 at 6:33 AM JST, Joel Fernandes wrote:> Hi Alex, > > Great effort, thanks. I replied with few comments below. Since the patch is > large, it would be great if could be possibly split. Maybe the From > primitives deserve a separate patch.I'm all for smaller patches when it makes reviewing easier, but in this case all it would achieve is making the second patch append code right after the next. :) Is there a benefit in doing so?> > On Thu, Oct 09, 2025 at 09:37:09PM +0900, Alexandre Courbot wrote: >> Add the BoundedInt type, which restricts the number of bits allowed to >> be used in a given integer value. This is useful to carry guarantees >> when setting bitfields. >> >> Alongside this type, many `From` and `TryFrom` implementations are >> provided to reduce friction when using with regular integer types. Proxy >> implementations of common integer traits are also provided. >> >> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com> >> --- >> rust/kernel/lib.rs | 1 + >> rust/kernel/num.rs | 499 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 500 insertions(+) >> >> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs >> index fcffc3988a90..21c1f452ee6a 100644 >> --- a/rust/kernel/lib.rs >> +++ b/rust/kernel/lib.rs >> @@ -101,6 +101,7 @@ >> pub mod mm; >> #[cfg(CONFIG_NET)] >> pub mod net; >> +pub mod num; >> pub mod of; >> #[cfg(CONFIG_PM_OPP)] >> pub mod opp; >> diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs >> new file mode 100644 >> index 000000000000..b2aad95ce51c >> --- /dev/null >> +++ b/rust/kernel/num.rs >> @@ -0,0 +1,499 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +//! Numerical types for the kernel. >> + >> +use kernel::prelude::*; >> + >> +/// Integer type for which only the bits `0..NUM_BITS` are valid. >> +/// >> +/// # Invariants >> +/// >> +/// Stored values are represented with at most `NUM_BITS` bits. >> +#[repr(transparent)] >> +#[derive(Clone, Copy, Debug, Default, Hash)] >> +pub struct BoundedInt<T, const NUM_BITS: u32>(T); >> + >> +/// Returns `true` if `$value` can be represented with at most `$NUM_BITS` on `$type`. >> +macro_rules! is_in_bounds { >> + ($value:expr, $type:ty, $num_bits:expr) => {{ >> + let v = $value; >> + v & <$type as Boundable<NUM_BITS>>::MASK == v >> + }}; >> +} >> + >> +/// Trait for primitive integer types that can be used with `BoundedInt`. >> +pub trait Boundable<const NUM_BITS: u32> >> +where >> + Self: Sized + Copy + core::ops::BitAnd<Output = Self> + core::cmp::PartialEq, >> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>, >> +{ >> + /// Mask of the valid bits for this type. >> + const MASK: Self; >> + >> + /// Returns `true` if `value` can be represented with at most `NUM_BITS`. >> + /// >> + /// TODO: post-RFC: replace this with a left-shift followed by right-shift operation. This will >> + /// allow us to handle signed values as well. >> + fn is_in_bounds(value: Self) -> bool { >> + is_in_bounds!(value, Self, NUM_BITS) >> + } >> +} >> + >> +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u8 { >> + const MASK: u8 = crate::bits::genmask_u8(0..=(NUM_BITS - 1)); >> +} >> + >> +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u16 { >> + const MASK: u16 = crate::bits::genmask_u16(0..=(NUM_BITS - 1)); >> +} >> + >> +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u32 { >> + const MASK: u32 = crate::bits::genmask_u32(0..=(NUM_BITS - 1)); >> +} >> + >> +impl<const NUM_BITS: u32> Boundable<NUM_BITS> for u64 { >> + const MASK: u64 = crate::bits::genmask_u64(0..=(NUM_BITS - 1)); >> +} >> + >> +impl<T, const NUM_BITS: u32> BoundedInt<T, NUM_BITS> >> +where >> + T: Boundable<NUM_BITS>, >> +{ >> + /// Checks that `value` is valid for this type at compile-time and build a new value. >> + /// >> + /// This relies on [`build_assert!`] to perform validation at compile-time. If `value` cannot >> + /// be inferred to be in bounds at compile-time, use the fallible [`Self::try_new`] instead. >> + /// >> + /// When possible, use one of the `new_const` methods instead of this method as it statically >> + /// validates `value` instead of relying on the compiler's optimizations. > > This sounds like, users might use the less-optimal API first with the same > build_assert issues we had with the IO accessors, since new() sounds very obvious. > How about the following naming? > > new::<VALUE>() // Primary constructor for constants using const generics. > try_new(value) // Keep as-is for fallible runtime > new_from_expr(value) // For compile-time validated runtime values > > If new::<VALUE>() does not work for the user, the compiler will fail. > > Or, new_from_expr() could be from_value(), Ok with either naming or a better name.Agreed, the preferred method should appear first. IIRC Alice also made a similar suggestion about v1 during the DRM sync, sorry for not picking it up.> >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::BoundedInt; >> + /// >> + /// # fn some_number() -> u32 { 0xffffffff } >> + /// >> + /// assert_eq!(BoundedInt::<u8, 1>::new(1).get(), 1); >> + /// assert_eq!(BoundedInt::<u16, 8>::new(0xff).get(), 0xff); >> + /// >> + /// // Triggers a build error as `0x1ff` doesn't fit into 8 bits. >> + /// // assert_eq!(BoundedInt::<u32, 8>::new(0x1ff).get(), 0x1ff); >> + /// >> + /// let v: u32 = some_number(); >> + /// // Triggers a build error as `v` cannot be asserted to fit within 4 bits... >> + /// // let _ = BoundedInt::<u32, 4>::new(v); >> + /// // ... but this works as the compiler can assert the range from the mask. >> + /// let _ = BoundedInt::<u32, 4>::new(v & 0xf); >> + /// ``` >> + pub fn new(value: T) -> Self { >> + crate::build_assert!( >> + T::is_in_bounds(value), >> + "Provided parameter is larger than maximal supported value" >> + ); >> + >> + Self(value) >> + } >> + >> + /// Attempts to convert `value` into a value bounded by `NUM_BITS`. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::BoundedInt; >> + /// >> + /// assert_eq!(BoundedInt::<u8, 1>::try_new(1).map(|v| v.get()), Ok(1)); >> + /// assert_eq!(BoundedInt::<u16, 8>::try_new(0xff).map(|v| v.get()), Ok(0xff)); >> + /// >> + /// // `0x1ff` doesn't fit into 8 bits. >> + /// assert_eq!(BoundedInt::<u32, 8>::try_new(0x1ff), Err(EOVERFLOW)); >> + /// ``` >> + pub fn try_new(value: T) -> Result<Self> { >> + if !T::is_in_bounds(value) { >> + Err(EOVERFLOW) >> + } else { >> + Ok(Self(value)) >> + } >> + } >> + >> + /// Returns the contained value as a primitive type. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::BoundedInt; >> + /// >> + /// let v = BoundedInt::<u32, 4>::new_const::<7>(); >> + /// assert_eq!(v.get(), 7u32); >> + /// ``` >> + pub fn get(self) -> T { >> + if !T::is_in_bounds(self.0) { >> + // SAFETY: Per the invariants, `self.0` cannot have bits set outside of `MASK`, so >> + // this block will >> + // never be reached. >> + unsafe { core::hint::unreachable_unchecked() } >> + } > > Does this if block help the compiler generate better code? I wonder if code > gen could be checked to confirm the rationale.Benno suggested that it would on a different patch: https://lore.kernel.org/rust-for-linux/DBL1ZGZCSJF3.29HNS9BSN89C6 at kernel.org/ OTOH as shown in patch 3/3, this doesn't exempt us from handling impossible values when using this in a match expression...> >> + self.0 >> + } >> + >> + /// Increase the number of bits usable for `self`. >> + /// >> + /// This operation cannot fail. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::BoundedInt; >> + /// >> + /// let v = BoundedInt::<u32, 4>::new_const::<7>(); >> + /// let larger_v = v.enlarge::<12>(); >> + /// // The contained values are equal even though `larger_v` has a bigger capacity. >> + /// assert_eq!(larger_v, v); >> + /// ``` >> + pub const fn enlarge<const NEW_NUM_BITS: u32>(self) -> BoundedInt<T, NEW_NUM_BITS> >> + where >> + T: Boundable<NEW_NUM_BITS>, >> + T: Copy, > > Boundable already implies copy so T: Copy is redundant.Thanks. I need to do a thorough review of all the contraints as I've changed them quite a bit as the implementation matured.> >> + { >> + build_assert!(NEW_NUM_BITS >= NUM_BITS); >> + >> + // INVARIANT: the value did fit within `NUM_BITS`, so it will all the more fit within >> + // `NEW_NUM_BITS` which is larger. >> + BoundedInt(self.0) >> + } >> + >> + /// Shrink the number of bits usable for `self`. >> + /// >> + /// Returns `EOVERFLOW` if the value of `self` cannot be represented within `NEW_NUM_BITS`. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::BoundedInt; >> + /// >> + /// let v = BoundedInt::<u32, 12>::new_const::<7>(); >> + /// let smaller_v = v.shrink::<4>()?; >> + /// // The contained values are equal even though `smaller_v` has a smaller capacity. >> + /// assert_eq!(smaller_v, v); >> + /// >> + /// # Ok::<(), Error>(()) >> + /// ``` >> + pub fn shrink<const NEW_NUM_BITS: u32>(self) -> Result<BoundedInt<T, NEW_NUM_BITS>> >> + where >> + T: Boundable<NEW_NUM_BITS>, >> + T: Copy, > > Here too. > > [...] >> +impl_const_new!(u8 u16 u32 u64); >> + >> +/// Declares a new `$trait` and implements it for all bounded types represented using `$num_bits`. >> +/// >> +/// This is used to declare properties as traits that we can use for later implementations. >> +macro_rules! impl_size_rule { >> + ($trait:ident, $($num_bits:literal)*) => { >> + trait $trait {} >> + >> + $( >> + impl<T> $trait for BoundedInt<T, $num_bits> where T: Boundable<$num_bits> {} >> + )* >> + }; >> +} >> + >> +// Bounds that are larger than a `u64`. >> +impl_size_rule!(LargerThanU64, 64); >> + >> +// Bounds that are larger than a `u32`. >> +impl_size_rule!(LargerThanU32, >> + 32 33 34 35 36 37 38 39 > > If num_bits == 32 (number of bits), how could BoundedInt<T, 32> a > LargerThanU32? It should be AtleastU32 or something. > > Or the above list should start from 33. Only a >= 33-bit wide integer can be > LargerThanU32.The name is a bit ambiguous indeed. An accurate one would be `LargerOrEqualThanU32`, but `AtLeastU32` should also work.