Message-ID: <20251009213353.GA2326866@joelbox2>
Date: Thu, 9 Oct 2025 17:33:53 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Danilo Krummrich <dakr@...nel.org>, Yury Norov <yury.norov@...il.com>,
	Jesung Yang <y.j3ms.n@...il.com>, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	nouveau@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org
Subject: Re: [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@...dia.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 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.

Thanks.

Powered by blists - more mailing lists