Re: [PATCH RFC v2 2/3] rust: kernel: add bounded integer types

["Alexandre Courbot" <acourbot@nvidia.com>]

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@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@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.