* [PATCH 0/2] rust: add BitInt type and use in Nova's bitfield macro
@ 2025-10-31 13:39 Alexandre Courbot
2025-10-31 13:39 ` [PATCH 1/2] rust: add BitInt integer wrapping type Alexandre Courbot
2025-10-31 13:39 ` [PATCH FOR REFERENCE 2/2] gpu: nova-core: use BitInt for bitfields Alexandre Courbot
0 siblings, 2 replies; 22+ messages in thread
From: Alexandre Courbot @ 2025-10-31 13:39 UTC (permalink / raw)
To: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Yury Norov, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: linux-kernel, rust-for-linux, Alexandre Courbot
I think this code is mature enough to be moved out of RFC, thanks for
all the feedback so far.
This series provides `BitInt`, a wrapper type for primitive integers
that guarantees that only a given number of bits are used to represent
values. This is particularly useful when working with bitfields, as the
guarantee that a given value fits within the number of assigned bits can
be enforced by the type system, saving cumbersome runtime checks, or
(worse) stripping data when bits are silently dropped.
Don't worry about the size; half of the code is actually rustdoc. :)
Example use (from the rustdoc):
use kernel::num::BitInt;
// An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
// The value `15` is statically validated to fit within the specified number of bits.
let v = BitInt::<u8, 4>::new::<15>();
assert_eq!(v.get(), 15);
let v = BitInt::<i8, 4>::new::<-8>();
assert_eq!(v.get(), -8);
// This doesn't build: a `u8` is smaller than the requested 9 bits.
// let _ = BitInt::<u8, 9>::new::<10>();
// This also doesn't build: the requested value doesn't fit within the requested bits.
// let _ = BitInt::<i8, 4>::new::<8>();
// Values can also be validated at runtime. This succeeds because `15` can be represented
// with 4 bits.
assert!(BitInt::<u8, 4>::try_new(15).is_some());
// This fails because `16` cannot be represented with 4 bits.
assert!(BitInt::<u8, 4>::try_new(16).is_none());
// Non-constant expressions can also be validated at build-time thanks to compiler
// optimizations. This should be used as a last resort though.
let v = BitInt::<u8, 4>::from_expr(15);
// Common operations are supported against the backing type.
assert_eq!(v + 5, 20);
assert_eq!(v / 3, 5);
// The backing type can be changed while preserving the number of bits used for representation.
assert_eq!(v.cast::<u32>(), BitInt::<u32, 4>::new::<15>());
// We can safely extend the number of bits...
assert_eq!(v.extend::<5>(), BitInt::<u8, 5>::new::<15>());
// ... but reducing the number of bits fails here as the value doesn't fit anymore.
assert_eq!(v.try_shrink::<3>(), None);
// Conversion into primitive types is dependent on the number of useful bits, not the backing
// type.
//
// Event though its backing type is `u32`, this `BitInt` only uses 8 bits and thus can safely
// be converted to a `u8`.
assert_eq!(u8::from(BitInt::<u32, 8>::new::<128>()), 128u8);
// The same applies to signed values.
assert_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
// This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
// contained value).
// let _ = u8::from(BitInt::<u32, 10>::new::<10>());
// Conversely, `BitInt` types large enough to contain a given primitive type can be created
// directly from it:
//
// This `BitInt` has 8 bits, so it can represent any `u8`.
let _ = BitInt::<u32, 8>::from(128u8);
// This `BitInt` has 8 bits, so it can represent any `i8`.
let _ = BitInt::<i32, 8>::from(127i8);
// This is not allowed, as this 6-bit `BitInt` does not have enough capacity to represent a
// `u8` (regardless of the passed value).
// let _ = BitInt::<u32, 6>::from(10u8);
// A `u8` can be converted to an `i16` if we allow at least 9 bits to be used.
let _ = BitInt::<i16, 9>::from(255u8);
// ... but 8 bits aren't enough as the MSB is used for the sign, so this doesn't build.
// let _ = BitInt::<i16, 8>::from(255u8);
There are still a few things I hope to simplify: notably the many
implementations from and to primitive types. I have added an basic
`Integer` trait to allow writing generic code against integer types,
which simplifies things a bit, but it is still a bit too complex to my
taste. Suggestions are welcome.
The first use of this will be to represent bitfields in Nova register
types to guarantee that no data is ever stripped when manipulating them.
This should eventually allow the `bitfield` and `register` macros to
move out of Nova and into the kernel crate.
The second patch is here to illustrate the use of this module; it is not
intended to be merged this cycle as it would likely result in big merge
conflicts with the drm tree.
This series applies on top of drm-rust-next for the needs of the second
patch. The first patch should apply cleanly on rust-next. A branch with
this series and its dependencies is available here:
https://github.com/Gnurou/linux/tree/b4/bounded_ints
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v1:
- Rebase on top of `drm-rust-next`.
- Rename `BoundedInt` to `BitInt` (thanks Yury!).
- Add lots of documentation.
- Use shifts to validate bounds, as it works with both unsigned and
signed types contrary to the mask method.
- Support signed types (albeit with some bugs).
- Use `new` for the const static constructor and make it the preferred
way to build values (thanks Alice and Joel!).
- Rename `LargerThanX` into `AtLeastX` (thanks Joel!).
- Support basic arithmetic operations (+, -, etc.) against the backing
type.
- Add `Deref` implementation as alternative to the `get` method.
- Write correct `Default` implementation for bitfields.
- Make the new bitfield accessors `inline(always)`.
- Update bitfield documentation to the new usage.
- Link to RFC v2: https://lore.kernel.org/r/20251009-bounded_ints-v2-0-ff3d7fee3ffd@nvidia.com
Changes in RFC v2:
- Thorough implementation of `BoundedInt`.
- nova-core fully adapted to use `BoundedInt`.
- Link to RFC v1: https://lore.kernel.org/r/20251002-bounded_ints-v1-0-dd60f5804ea4@nvidia.com
---
Alexandre Courbot (2):
rust: add BitInt integer wrapping type
[FOR REFERENCE] gpu: nova-core: use BitInt for bitfields
drivers/gpu/nova-core/bitfield.rs | 366 ++++++------
drivers/gpu/nova-core/falcon.rs | 134 ++---
drivers/gpu/nova-core/falcon/hal/ga102.rs | 5 +-
drivers/gpu/nova-core/fb/hal/ga100.rs | 3 +-
drivers/gpu/nova-core/gpu.rs | 9 +-
drivers/gpu/nova-core/regs.rs | 139 ++---
rust/kernel/lib.rs | 1 +
rust/kernel/num.rs | 75 +++
rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++
9 files changed, 1325 insertions(+), 303 deletions(-)
---
base-commit: 9a3c2f8a4f84960a48c056d0da88de3d09e6d622
change-id: 20251001-bounded_ints-1d0457d9ae26
Best regards,
--
Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/2] rust: add BitInt integer wrapping type
2025-10-31 13:39 [PATCH 0/2] rust: add BitInt type and use in Nova's bitfield macro Alexandre Courbot
@ 2025-10-31 13:39 ` Alexandre Courbot
2025-11-02 5:46 ` Alexandre Courbot
2025-11-03 2:17 ` Yury Norov
2025-10-31 13:39 ` [PATCH FOR REFERENCE 2/2] gpu: nova-core: use BitInt for bitfields Alexandre Courbot
1 sibling, 2 replies; 22+ messages in thread
From: Alexandre Courbot @ 2025-10-31 13:39 UTC (permalink / raw)
To: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Yury Norov, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: linux-kernel, rust-for-linux, Alexandre Courbot
Add the `BitInt` type, which is an integer on which the number of bits
allowed to be used is restricted, capping its maximal value below that
of primitive type is wraps.
This is useful to e.g. enforce guarantees when working with bit fields.
Alongside this type, provide many `From` and `TryFrom` implementations
are 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 | 75 ++++
rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 972 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3dd7bebe7888..235d0d8b1eff 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -109,6 +109,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..4e81879d88f7
--- /dev/null
+++ b/rust/kernel/num.rs
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Numerical features for the kernel.
+
+pub mod bitint;
+pub use bitint::*;
+
+/// Type used to designate unsigned primitive types.
+pub struct Unsigned;
+
+/// Type used to designate signed primitive types.
+pub struct Signed;
+
+/// Trait describing properties of integer types.
+pub trait Integer {
+ /// Whether this type is [`Signed`] or [`Unsigned`].
+ type Signedness;
+
+ /// Number of bits used for value representation.
+ const BITS: u32;
+}
+
+impl Integer for bool {
+ type Signedness = Unsigned;
+
+ const BITS: u32 = 1;
+}
+
+impl Integer for u8 {
+ type Signedness = Unsigned;
+
+ const BITS: u32 = u8::BITS;
+}
+
+impl Integer for u16 {
+ type Signedness = Unsigned;
+
+ const BITS: u32 = u16::BITS;
+}
+
+impl Integer for u32 {
+ type Signedness = Unsigned;
+
+ const BITS: u32 = u32::BITS;
+}
+
+impl Integer for u64 {
+ type Signedness = Unsigned;
+
+ const BITS: u32 = u64::BITS;
+}
+
+impl Integer for i8 {
+ type Signedness = Signed;
+
+ const BITS: u32 = i8::BITS;
+}
+
+impl Integer for i16 {
+ type Signedness = Signed;
+
+ const BITS: u32 = i16::BITS;
+}
+
+impl Integer for i32 {
+ type Signedness = Signed;
+
+ const BITS: u32 = i32::BITS;
+}
+
+impl Integer for i64 {
+ type Signedness = Signed;
+
+ const BITS: u32 = i64::BITS;
+}
diff --git a/rust/kernel/num/bitint.rs b/rust/kernel/num/bitint.rs
new file mode 100644
index 000000000000..5af1447347a8
--- /dev/null
+++ b/rust/kernel/num/bitint.rs
@@ -0,0 +1,896 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! [`BitInt`], a primitive integer type with a limited set of bits usable to represent values.
+
+use core::ops::Deref;
+
+use kernel::num::{Integer, Signed, Unsigned};
+use kernel::prelude::*;
+
+/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
+///
+/// Can be used in const context.
+macro_rules! fits_within {
+ ($value:expr, $type:ty, $num_bits:expr) => {{
+ // Any attempt to create a `BitInt` with more bits used for representation than its backing
+ // type (i.e. create an invalid `BitInt`) must be aborted at build time.
+ build_assert!(
+ <$type>::BITS >= $num_bits,
+ "Number of bits requested for representation is larger than backing type."
+ );
+
+ let shift: u32 = <$type>::BITS - $num_bits;
+ let v = $value;
+
+ // The value fits within `NUM_BITS` if shifting it left by the number of unused bits, then
+ // right by the same number, doesn't change the value.
+ //
+ // This method has the benefit of working with both unsigned and signed integers.
+ (v << shift) >> shift == v
+ }};
+}
+
+/// Trait for primitive integer types that can be used to back a [`BitInt`].
+///
+/// This is mostly used to lock all the operations we need for [`BitInt`] in a single trait.
+pub trait Boundable
+where
+ Self: Integer
+ + Sized
+ + Copy
+ + core::ops::Shl<u32, Output = Self>
+ + core::ops::Shr<u32, Output = Self>
+ + core::cmp::PartialEq,
+ Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
+ Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
+{
+ /// Returns `true` if `value` can be represented with at most `NUM_BITS` on `T`.
+ fn fits_within(value: Self, num_bits: u32) -> bool {
+ fits_within!(value, Self, num_bits)
+ }
+}
+
+/// Inplement `Boundable` for all integers types.
+impl<T> Boundable for T
+where
+ T: Integer
+ + Sized
+ + Copy
+ + core::ops::Shl<u32, Output = Self>
+ + core::ops::Shr<u32, Output = Self>
+ + core::cmp::PartialEq,
+ Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
+ Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
+{
+}
+
+/// Integer type for which only the bits `0..NUM_BITS` are valid.
+///
+/// # Invariants
+///
+/// Stored values are represented with at most `NUM_BITS` bits.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// // An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
+/// // The value `15` is statically validated to fit within the specified number of bits.
+/// let v = BitInt::<u8, 4>::new::<15>();
+/// assert_eq!(v.get(), 15);
+///
+/// let v = BitInt::<i8, 4>::new::<-8>();
+/// assert_eq!(v.get(), -8);
+///
+/// // This doesn't build: a `u8` is smaller than the requested 9 bits.
+/// // let _ = BitInt::<u8, 9>::new::<10>();
+///
+/// // This also doesn't build: the requested value doesn't fit within the requested bits.
+/// // let _ = BitInt::<i8, 4>::new::<8>();
+///
+/// // Values can also be validated at runtime. This succeeds because `15` can be represented
+/// // with 4 bits.
+/// assert!(BitInt::<u8, 4>::try_new(15).is_some());
+/// // This fails because `16` cannot be represented with 4 bits.
+/// assert!(BitInt::<u8, 4>::try_new(16).is_none());
+///
+/// // Non-constant expressions can also be validated at build-time thanks to compiler
+/// // optimizations. This should be used as a last resort though.
+/// let v = BitInt::<u8, 4>::from_expr(15);
+///
+/// // Common operations are supported against the backing type.
+/// assert_eq!(v + 5, 20);
+/// assert_eq!(v / 3, 5);
+///
+/// // The backing type can be changed while preserving the number of bits used for representation.
+/// assert_eq!(v.cast::<u32>(), BitInt::<u32, 4>::new::<15>());
+///
+/// // We can safely extend the number of bits...
+/// assert_eq!(v.extend::<5>(), BitInt::<u8, 5>::new::<15>());
+/// // ... but reducing the number of bits fails here as the value doesn't fit anymore.
+/// assert_eq!(v.try_shrink::<3>(), None);
+///
+/// // Conversion into primitive types is dependent on the number of useful bits, not the backing
+/// // type.
+/// //
+/// // Event though its backing type is `u32`, this `BitInt` only uses 8 bits and thus can safely
+/// // be converted to a `u8`.
+/// assert_eq!(u8::from(BitInt::<u32, 8>::new::<128>()), 128u8);
+///
+/// // The same applies to signed values.
+/// assert_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
+///
+/// // This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
+/// // contained value).
+/// // let _ = u8::from(BitInt::<u32, 10>::new::<10>());
+///
+/// // Conversely, `BitInt` types large enough to contain a given primitive type can be created
+/// // directly from it:
+/// //
+/// // This `BitInt` has 8 bits, so it can represent any `u8`.
+/// let _ = BitInt::<u32, 8>::from(128u8);
+///
+/// // This `BitInt` has 8 bits, so it can represent any `i8`.
+/// let _ = BitInt::<i32, 8>::from(127i8);
+///
+/// // This is not allowed, as this 6-bit `BitInt` does not have enough capacity to represent a
+/// // `u8` (regardless of the passed value).
+/// // let _ = BitInt::<u32, 6>::from(10u8);
+///
+/// // A `u8` can be converted to an `i16` if we allow at least 9 bits to be used.
+/// let _ = BitInt::<i16, 9>::from(255u8);
+/// // ... but 8 bits aren't enough as the MSB is used for the sign, so this doesn't build.
+/// // let _ = BitInt::<i16, 8>::from(255u8);
+/// ```
+#[repr(transparent)]
+#[derive(Clone, Copy, Debug, Default, Hash)]
+pub struct BitInt<T: Boundable, const NUM_BITS: u32>(T);
+
+/// Validating the value as a const expression cannot be done as a regular method, as the
+/// arithmetic expressions we rely on to check the bounds are not const. Thus, implement
+/// [`BitInt::new`] using a macro.
+macro_rules! impl_const_new {
+ ($($type:ty)*) => {
+ $(
+ impl<const NUM_BITS: u32> BitInt<$type, NUM_BITS> {
+ /// Creates a [`BitInt`] for the constant `VALUE`.
+ ///
+ /// Fails at build time if `VALUE` cannot be represented with `NUM_BITS`.
+ ///
+ /// This method should be used preferred to [`Self::from_expr`] whenever possible.
+ ///
+ /// # Examples
+ /// ```
+ /// use kernel::num::BitInt;
+ ///
+ #[doc = ::core::concat!(
+ "let v = BitInt::<",
+ ::core::stringify!($type),
+ ", 4>::new::<7>();")]
+ /// assert_eq!(v.get(), 7);
+ /// ```
+ pub const fn new<const VALUE: $type>() -> Self {
+ build_assert!(
+ fits_within!(VALUE, $type, NUM_BITS),
+ "Requested value larger than maximal representable value."
+ );
+
+ // INVARIANT: `fits_within` confirmed that `value` can be represented within
+ // `NUM_BITS`.
+ Self(VALUE)
+ }
+ }
+ )*
+ };
+}
+
+impl_const_new!(u8 u16 u32 u64);
+impl_const_new!(i8 i16 i32 i64);
+
+impl<T, const NUM_BITS: u32> BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+{
+ /// Attempts to convert `value` into a `BitInt` using `NUM_BITS`.
+ ///
+ /// Returns [`None`] if `value` doesn't fit within `NUM_BITS`.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::num::BitInt;
+ ///
+ /// assert_eq!(BitInt::<u8, 1>::try_new(1).as_deref(), Some(&1));
+ /// assert_eq!(BitInt::<u16, 8>::try_new(0xff).as_deref(), Some(&0xff));
+ ///
+ /// // `0x1ff` doesn't fit into 8 bits.
+ /// assert_eq!(BitInt::<u32, 8>::try_new(0x1ff), None);
+ ///
+ /// // Signed types can also be used.
+ /// assert_eq!(BitInt::<i8, 4>::try_new(-2).as_deref(), Some(&-2));
+ /// assert_eq!(BitInt::<i8, 4>::try_new(7).as_deref(), Some(&7));
+ ///
+ /// // `8` doesn't fit into 4 signed bits.
+ /// assert_eq!(BitInt::<i8, 4>::try_new(8), None);
+ ///
+ /// // `-9` doesn't fit into 4 signed bits.
+ /// assert_eq!(BitInt::<i8, 4>::try_new(-9), None);
+ /// ```
+ pub fn try_new(value: T) -> Option<Self> {
+ if !T::fits_within(value, NUM_BITS) {
+ None
+ } else {
+ // INVARIANT: `fits_within` confirmed that `value` can be represented within `NUM_BITS`.
+ Some(Self(value))
+ }
+ }
+
+ /// Checks that `expr` 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 `expr` cannot
+ /// be inferred to be in bounds at compile-time, use the fallible [`Self::try_new`] instead.
+ ///
+ /// When possible, use one of the [`Self::new`] constructors instead of this one as it
+ /// statically validates `expr` instead of relying on the compiler's optimizations.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::num::BitInt;
+ ///
+ /// # fn some_number() -> u32 { 0xffffffff }
+ ///
+ /// assert_eq!(BitInt::<u8, 1>::from_expr(1).get(), 1);
+ /// assert_eq!(BitInt::<u16, 8>::from_expr(0xff).get(), 0xff);
+ ///
+ /// // Triggers a build error as `0x1ff` doesn't fit into 8 bits.
+ /// // assert_eq!(BitInt::<u32, 8>::from_expr(0x1ff).get(), 0x1ff);
+ ///
+ /// // Signed types can also be used.
+ /// assert_eq!(BitInt::<i8, 4>::from_expr(-2).get(), -2);
+ /// assert_eq!(BitInt::<i8, 4>::from_expr(7).get(), 7);
+ ///
+ /// // Triggers a build error as `8` cannot be represented with 4 signed bits.
+ /// // assert_eq!(BitInt::<i8, 4>::from_expr(8).get(), 8);
+ ///
+ /// // Triggers a build error as `-9` cannot be represented with 4 signed bits.
+ /// // assert_eq!(BitInt::<i8, 4>::from_expr(-9).get(), -9);
+ ///
+ /// let v: u32 = some_number();
+ /// // Triggers a build error as `v` cannot be asserted to fit within 4 bits...
+ /// // let _ = BitInt::<u32, 4>::from_expr(v);
+ /// // ... but this works as the compiler can assert the range from the mask.
+ /// let _ = BitInt::<u32, 4>::from_expr(v & 0xf);
+ /// ```
+ pub fn from_expr(expr: T) -> Self {
+ crate::build_assert!(
+ T::fits_within(expr, NUM_BITS),
+ "Requested value larger than maximal representable value."
+ );
+
+ // INVARIANT: `fits_within` confirmed that `expr` can be represented within `NUM_BITS`.
+ Self(expr)
+ }
+
+ /// Returns the contained value as the backing type.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::num::BitInt;
+ ///
+ /// let v = BitInt::<u32, 4>::new::<7>();
+ /// assert_eq!(v.get(), 7u32);
+ /// ```
+ pub fn get(self) -> T {
+ *self.deref()
+ }
+
+ /// Increase the number of bits usable for `self`.
+ ///
+ /// This operation cannot fail.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::num::BitInt;
+ ///
+ /// let v = BitInt::<u32, 4>::new::<7>();
+ /// let larger_v = v.extend::<12>();
+ /// // The contained values are equal even though `larger_v` has a bigger capacity.
+ /// assert_eq!(larger_v, v);
+ /// ```
+ pub const fn extend<const NEW_NUM_BITS: u32>(self) -> BitInt<T, NEW_NUM_BITS> {
+ build_assert!(
+ NEW_NUM_BITS >= NUM_BITS,
+ "Requested number of bits is less than the current representation."
+ );
+
+ // INVARIANT: the value did fit within `NUM_BITS`, so it will all the more fit within
+ // the larger `NEW_NUM_BITS`.
+ BitInt(self.0)
+ }
+
+ /// Shrink the number of bits usable for `self`.
+ ///
+ /// Returns [`None`] if the value of `self` cannot be represented within `NEW_NUM_BITS`.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::num::BitInt;
+ ///
+ /// let v = BitInt::<u32, 12>::new::<7>();
+ /// let smaller_v = v.try_shrink::<4>().ok_or(EOVERFLOW)?;
+ /// // The contained values are equal even though `smaller_v` has a smaller capacity.
+ /// assert_eq!(smaller_v, v);
+ ///
+ /// # Ok::<(), Error>(())
+ /// ```
+ pub fn try_shrink<const NEW_NUM_BITS: u32>(self) -> Option<BitInt<T, NEW_NUM_BITS>> {
+ BitInt::<T, NEW_NUM_BITS>::try_new(self.get())
+ }
+
+ /// Casts `self` into a [`BitInt`] backed by a different storage type, but using the same
+ /// number of bits for value representation.
+ ///
+ /// This method cannot fail as the number of bits used for representation doesn't change.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::num::BitInt;
+ ///
+ /// let v = BitInt::<i32, 4>::new::<-3>();
+ /// let u: BitInt<i8, _> = v.cast();
+ /// // The contained values are equal even though `u` has a smaller storage type.
+ /// assert_eq!(i32::from(u.get()), v.get());
+ ///
+ /// let v = BitInt::<u8, 7>::new::<127>();
+ /// // This doesn't build: signedness cannot be safely changed when casting.
+ /// // let _: BitInt<i8, _> = v.cast();
+ /// ```
+ pub fn cast<U>(self) -> BitInt<U, NUM_BITS>
+ where
+ U: TryFrom<T> + Boundable,
+ T: Integer,
+ // Only allow casting from types of same signedness.
+ U: Integer<Signedness = T::Signedness>,
+ {
+ // Make sure that the target type can contain the required number of bits.
+ build_assert!(U::BITS >= NUM_BITS, "Cast to incompatible target type.");
+
+ // SAFETY: the contained value is represented using `NUM_BITS`, `U` is larger than
+ // `NUM_BITS`, and `U` and `T` have the same sign, hence this conversion cannot fail.
+ let value = unsafe { U::try_from(self.0).unwrap_unchecked() };
+
+ // INVARIANT: although the storage type has changed, the value is still represented within
+ // `NUM_BITS`, and with the same signedness.
+ BitInt(value)
+ }
+}
+
+impl<T, const NUM_BITS: u32> Deref for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+{
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ if !T::fits_within(self.0, NUM_BITS) {
+ // SAFETY: Per the `BitInt` invariants, `fits_within` can never return `false`.
+ unsafe { core::hint::unreachable_unchecked() }
+ }
+ &self.0
+ }
+}
+
+/// Trait similar to [`TryInto`] but for `BitInt`, to avoid conflicting implementations errors.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::{BitInt, TryIntoBitInt};
+///
+/// // Succeed because `128` fits into 8 bits.
+/// assert_eq!(128u32.try_into_bitint(), Some(BitInt::<u16, 8>::new::<128>()));
+/// // Fails because `128` doesn't fits into 6 bits.
+/// assert_eq!(TryIntoBitInt::<u16, 6>::try_into_bitint(128u32), None);
+/// ```
+pub trait TryIntoBitInt<T: Boundable, const NUM_BITS: u32> {
+ /// Attempts to convert `self` into a [`BitInt`] using `NUM_BITS`.
+ fn try_into_bitint(self) -> Option<BitInt<T, NUM_BITS>>;
+}
+
+/// Any value can be attempted to be converted into a [`BitInt`] of any size.
+impl<T, U, const NUM_BITS: u32> TryIntoBitInt<T, NUM_BITS> for U
+where
+ T: Boundable,
+ U: TryInto<T>,
+{
+ fn try_into_bitint(self) -> Option<BitInt<T, NUM_BITS>> {
+ self.try_into().ok().and_then(BitInt::try_new)
+ }
+}
+
+/// Allows comparison between [`BitInt`]s, even if their number of valid bits differ.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v1 = BitInt::<u32, 8>::new::<15>();
+/// let v2 = BitInt::<u32, 4>::new::<15>();
+/// assert_eq!(v1, v2);
+/// ```
+impl<T, U, const NUM_BITS: u32, const NUM_BITS_U: u32> PartialEq<BitInt<U, NUM_BITS_U>>
+ for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ U: Boundable,
+ T: PartialEq<U>,
+{
+ fn eq(&self, other: &BitInt<U, NUM_BITS_U>) -> bool {
+ self.get() == other.get()
+ }
+}
+
+impl<T, const NUM_BITS: u32> Eq for BitInt<T, NUM_BITS> where T: Boundable {}
+
+/// Allows partial ordering between [`BitInt`]s, even if their number of valid bits differ.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v1 = BitInt::<u32, 8>::new::<4>();
+/// let v2 = BitInt::<u32, 4>::new::<15>();
+/// assert!(v1 < v2);
+/// ```
+impl<T, U, const NUM_BITS: u32, const NUM_BITS_U: u32> PartialOrd<BitInt<U, NUM_BITS_U>>
+ for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ U: Boundable,
+ T: PartialOrd<U>,
+{
+ fn partial_cmp(&self, other: &BitInt<U, NUM_BITS_U>) -> Option<core::cmp::Ordering> {
+ self.get().partial_cmp(&other.get())
+ }
+}
+
+/// Allows ordering between [`BitInt`]s.
+///
+/// # Examples
+///
+/// ```
+/// use core::cmp::Ordering;
+/// use kernel::num::BitInt;
+///
+/// let v1 = BitInt::<u32, 8>::new::<4>();
+/// let v2 = BitInt::<u32, 8>::new::<15>();
+/// assert_eq!(v1.cmp(&v2), Ordering::Less);
+/// ```
+impl<T, const NUM_BITS: u32> Ord for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: Ord,
+{
+ fn cmp(&self, other: &Self) -> core::cmp::Ordering {
+ self.get().cmp(&other.get())
+ }
+}
+
+/// Allows comparison between a [`BitInt`] and its backing type.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v = BitInt::<u32, 8>::new::<15>();
+/// assert_eq!(v, 15);
+/// ```
+impl<T, const NUM_BITS: u32> PartialEq<T> for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: PartialEq,
+{
+ fn eq(&self, other: &T) -> bool {
+ self.get() == *other
+ }
+}
+
+/// Allows partial ordering between a [`BitInt`] and its backing type.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v = BitInt::<u32, 8>::new::<4>();
+/// assert!(v < 15);
+/// ```
+impl<T, const NUM_BITS: u32> PartialOrd<T> for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: PartialOrd,
+{
+ fn partial_cmp(&self, other: &T) -> Option<core::cmp::Ordering> {
+ self.get().partial_cmp(other)
+ }
+}
+
+/// Allows adding a [`BitInt`] to its backing type.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v = BitInt::<u32, 4>::new::<15>();
+/// assert_eq!(v + 5, 20);
+/// ```
+impl<T, const NUM_BITS: u32> core::ops::Add<T> for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::ops::Add<Output = T>,
+{
+ type Output = T;
+
+ fn add(self, rhs: T) -> Self::Output {
+ *self + rhs
+ }
+}
+
+/// Allows subtracting a [`BitInt`] to its backing type.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v = BitInt::<u32, 4>::new::<15>();
+/// assert_eq!(v - 5, 10);
+/// ```
+impl<T, const NUM_BITS: u32> core::ops::Sub<T> for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::ops::Sub<Output = T>,
+{
+ type Output = T;
+
+ fn sub(self, rhs: T) -> Self::Output {
+ *self - rhs
+ }
+}
+
+/// Allows multiplying a [`BitInt`] to its backing type.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v = BitInt::<u32, 4>::new::<15>();
+/// assert_eq!(v * 10, 150);
+/// ```
+impl<T, const NUM_BITS: u32> core::ops::Mul<T> for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::ops::Mul<Output = T>,
+{
+ type Output = T;
+
+ fn mul(self, rhs: T) -> Self::Output {
+ *self * rhs
+ }
+}
+
+/// Allows dividing a [`BitInt`] to its backing type.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v = BitInt::<u32, 4>::new::<15>();
+/// assert_eq!(v / 3, 5);
+/// ```
+impl<T, const NUM_BITS: u32> core::ops::Div<T> for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::ops::Div<Output = T>,
+{
+ type Output = T;
+
+ fn div(self, rhs: T) -> Self::Output {
+ *self / rhs
+ }
+}
+
+/// Allows getting a remainder of a [`BitInt`] when divided by its backing type.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// let v = BitInt::<u32, 4>::new::<15>();
+/// assert_eq!(v % 10, 5);
+/// ```
+impl<T, const NUM_BITS: u32> core::ops::Rem<T> for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::ops::Rem<Output = T>,
+{
+ type Output = T;
+
+ fn rem(self, rhs: T) -> Self::Output {
+ *self % rhs
+ }
+}
+
+// TODO: implement more of std::ops.
+
+impl<T, const NUM_BITS: u32> core::fmt::Display for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::fmt::Display,
+{
+ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+ self.0.fmt(f)
+ }
+}
+
+impl<T, const NUM_BITS: u32> core::fmt::LowerHex for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::fmt::LowerHex,
+{
+ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+ self.0.fmt(f)
+ }
+}
+
+impl<T, const NUM_BITS: u32> core::fmt::UpperHex for BitInt<T, NUM_BITS>
+where
+ T: Boundable,
+ T: core::fmt::UpperHex,
+{
+ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+ self.0.fmt(f)
+ }
+}
+
+/// Declares a new `$trait` and implements it for all [`BitInt`] types represented using
+/// `$num_bits`.
+///
+/// This is used to declare properties as traits that we can use in later implementations.
+macro_rules! impl_size_rule {
+ ($trait:ident, $($num_bits:literal)*) => {
+ trait $trait {}
+
+ $(
+ impl<T> $trait for BitInt<T, $num_bits> where T: Boundable {}
+ )*
+ };
+
+ // Variant declaring and implementing two traits, for signed and unsigned integers.
+ ($u_trait:ident, $i_trait:ident, $($num_bits:literal)*) => {
+ trait $u_trait {}
+ trait $i_trait {}
+
+ $(
+ impl<T> $u_trait for BitInt<T, $num_bits>
+ where
+ T: Boundable + Integer<Signedness = Unsigned>
+ {
+ }
+
+ impl<T> $i_trait for BitInt<T, $num_bits>
+ where
+ T: Boundable + Integer<Signedness = Signed>
+ {
+ }
+ )*
+ };
+}
+
+/// Implementations for infallibly converting a primitive type into a `BitInt` that can contain it.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// // This `BitInt` has 8 bits, so it can represent any `u8`.
+/// let _ = BitInt::<u32, 8>::from(128u8);
+///
+/// // This `BitInt` has 8 bits, so it can represent any `i8`.
+/// let _ = BitInt::<i32, 8>::from(127i8);
+///
+/// // This is not allowed, as this 6-bit `BitInt` does not have enough capacity to represent a
+/// // `u8` (regardless of the passed value).
+/// // let _ = BitInt::<u32, 6>::from(10u8);
+///
+/// // A `u8` can be converted to an `i16` if we allow at least 9 bits to be used.
+/// let _ = BitInt::<i16, 9>::from(255u8);
+/// // ... but 8 bits aren't enough as the MSB is used for the sign, so this doesn't build.
+/// // let _ = BitInt::<i16, 8>::from(255u8);
+/// ```
+mod atleast_impls {
+ use super::*;
+
+ // Number of bits at least as large as `64`.
+ impl_size_rule!(AtLeast64Bits, 64);
+
+ // Number of bits at least as large as `32`.
+ impl_size_rule!(AtLeast32Bits,
+ 32 33 34 35 36 37 38 39
+ 40 41 42 43 44 45 46 47
+ 48 49 50 51 52 53 54 55
+ 56 57 58 59 60 61 62 63
+ );
+ // Anything larger than `64` is also larger than `32`.
+ impl<T> AtLeast32Bits for T where T: AtLeast64Bits {}
+
+ // Number of bits at least as large as `16`.
+ impl_size_rule!(AtLeast16Bits,
+ 16 17 18 19 20 21 22 23
+ 24 25 26 27 28 29 30 31
+ );
+ // Anything larger than `32` is also larger than `16`.
+ impl<T> AtLeast16Bits for T where T: AtLeast32Bits {}
+
+ // Number of bits at least as large as a `8`.
+ impl_size_rule!(AtLeast8Bits, 8 9 10 11 12 13 14 15);
+ // Anything larger than `16` is also larger than `8`.
+ impl<T> AtLeast8Bits for T where T: AtLeast16Bits {}
+
+ // Number of bits at least as large as `1`.
+ impl_size_rule!(AtLeast1Bit, 1 2 3 4 5 6 7);
+ // Anything larger than `8` is also larger than `1`.
+ impl<T> AtLeast1Bit for T where T: AtLeast8Bits {}
+
+ /// Generates `From` implementations from a primitive type into a [`BitInt`] that is
+ /// guaranteed to being able to contain it.
+ macro_rules! impl_from_primitive {
+ ($($type:ty => $trait:ident),*) => {
+ $(
+ impl<T, const NUM_BITS: u32> From<$type> for BitInt<T, NUM_BITS>
+ where
+ Self: $trait,
+ T: From<$type> + Boundable,
+ {
+ fn from(value: $type) -> Self {
+ build_assert!(
+ T::BITS >= NUM_BITS,
+ "Number of bits requested for representation is larger than backing type."
+ );
+ // INVARIANT: the impl constraints guarantee that the source type is smaller
+ // than `NUM_BITS`, and the `build_assert` above that the backing type can
+ // contain `NUM_BITS`.
+ Self(T::from(value))
+ }
+ }
+ )*
+ }
+ }
+
+ impl_from_primitive!(
+ bool => AtLeast1Bit,
+ u8 => AtLeast8Bits,
+ i8 => AtLeast8Bits,
+ u16 => AtLeast16Bits,
+ i16 => AtLeast16Bits,
+ u32 => AtLeast32Bits,
+ i32 => AtLeast32Bits,
+ u64 =>AtLeast64Bits,
+ i64 =>AtLeast64Bits
+ );
+}
+
+/// Implementations for infallibly converting a [`BitInt`] into a primitive type that can contain
+/// it.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::num::BitInt;
+///
+/// // Event though its backing type is `u32`, this `BitInt` only uses 8 bits and thus can safely
+/// // be converted to a `u8`.
+/// assert_eq!(u8::from(BitInt::<u32, 8>::new::<128>()), 128u8);
+///
+/// // The same applies to signed values.
+/// assert_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
+///
+/// // This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
+/// // contained value).
+/// // let _ = u8::from(BitInt::<u32, 10>::new::<10>());
+/// ```
+mod fits_impls {
+ use super::*;
+
+ // Number of bits that can be converted to a `bool`.
+ impl_size_rule!(IntoBool, 1);
+
+ // Number of bits that can be converted to a `u8`.
+ impl_size_rule!(IntoU8, IntoI8, 2 3 4 5 6 7 8);
+ // Anything that fits into `1` bit also fits into `8`.
+ impl<T> IntoU8 for T where T: IntoBool {}
+
+ // Number of bits that can be converted to a `u16`.
+ impl_size_rule!(IntoU16, IntoI16, 9 10 11 12 13 14 15 16);
+ // Anything that fits into `8` bits also fits into `16`.
+ impl<T> IntoU16 for T where T: IntoU8 {}
+ impl<T> IntoI16 for T where T: IntoI8 {}
+
+ // Number of bits that can be converted into a `u32`.
+ impl_size_rule!(IntoU32, IntoI32,
+ 17 18 19 20 21 22 23 24
+ 25 26 27 28 29 30 31 32
+ );
+ // Anything that fits into `16` bits also fits into `32`.
+ impl<T> IntoU32 for T where T: IntoU16 {}
+ impl<T> IntoI32 for T where T: IntoI16 {}
+
+ // Number of bits that can be converted into a `u64`.
+ impl_size_rule!(IntoU64, IntoI64,
+ 33 34 35 36 37 38 39 40
+ 41 42 43 44 45 46 47 48
+ 49 50 51 52 53 54 55 56
+ 57 58 59 60 61 62 63 64
+ );
+ // Anything that fits into `32` bits also fits into `64`.
+ impl<T> IntoU64 for T where T: IntoU32 {}
+ impl<T> IntoI64 for T where T: IntoI32 {}
+
+ /// Generates [`From`] implementations from a [`BitInt`] into a primitive type that is
+ /// guaranteed to contain it.
+ macro_rules! impl_into_primitive {
+ ($($trait:ident => $type:ty),*) => {
+ $(
+ impl<T, const NUM_BITS: u32> From<BitInt<T, NUM_BITS>> for $type
+ where
+ T: Boundable,
+ BitInt<T, NUM_BITS>: $trait
+ {
+ fn from(value: BitInt<T, NUM_BITS>) -> $type {
+ // SAFETY: the impl constraints ensure that `NUM_BITS` can be converted into
+ // the destination type, so this conversion cannot fail.
+ unsafe { value.get().try_into().unwrap_unchecked() }
+ }
+ }
+ )*
+ }
+ }
+
+ impl_into_primitive!(
+ IntoU8 => u8,
+ IntoI8 => i8,
+ IntoU16 => u16,
+ IntoI16 => i16,
+ IntoU32 => u32,
+ IntoI32 => i32,
+ IntoU64 => u64,
+ IntoI64 => i64
+ );
+
+ // Conversion to boolean must be handled separately as it does not have a [`TryFrom`]
+ // implementation from integers.
+ impl<T> From<BitInt<T, 1>> for bool
+ where
+ T: Boundable,
+ BitInt<T, 1>: IntoBool,
+ T: PartialEq + Zeroable,
+ {
+ fn from(value: BitInt<T, 1>) -> Self {
+ value.get() != Zeroable::zeroed()
+ }
+ }
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH FOR REFERENCE 2/2] gpu: nova-core: use BitInt for bitfields
2025-10-31 13:39 [PATCH 0/2] rust: add BitInt type and use in Nova's bitfield macro Alexandre Courbot
2025-10-31 13:39 ` [PATCH 1/2] rust: add BitInt integer wrapping type Alexandre Courbot
@ 2025-10-31 13:39 ` Alexandre Courbot
1 sibling, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2025-10-31 13:39 UTC (permalink / raw)
To: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Yury Norov, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: linux-kernel, rust-for-linux, Alexandre Courbot
Use BitInt with the bitfield!() and register!() macros and adapt the
nova-core code accordingly.
This makes it impossible to trim values when setting a register field,
because either the value of the field has been inferred at compile-time
to fit within the bounds of the field, or the user has been forced to
check at runtime that it does indeed fit.
The use of BitInt actually simplifies register fields definitions,
as they don't need an intermediate storage type (the "as ..." part of
fields definitions). Instead, the internal storage type for each field
is now the bounded integer of its width in bits, which can optionally be
converted to another type that implements `From`` or `TryFrom`` for that
bounded integer type.
This means that something like
register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
3:3 status_valid as bool,
31:8 addr as u32,
});
Now becomes
register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
3:3 status_valid => bool,
31:8 addr,
});
(here `status_valid` is infallibly converted to a bool for convenience
and to remain compatible with the previous semantics)
The field setter/getters are also simplified. If a field has no target
type, then its setter expects any type that implements `Into` to the
field's bounded integer type. Due to the many `From` implementations for
primitive types, this means that most calls can be left unchanged. If
the caller passes a value that is potentially larger than the field's
capacity, it must use the `try_` variant of the setter, which returns an
error if the value cannot be converted at runtime.
For fields that use `=>` to convert to another type, both setter and
getter are always infallible.
For fields that use `?=>` to fallibly convert to another type, only the
getter needs to be fallible as the setter always provide valid values by
design.
Outside of the register macro, the biggest changes occur in `falcon.rs`,
which defines many enums for fields - their conversion implementations
need to be changed from the original primitive type of the field to the
new corresponding bounded int type. Hopefully the TryFrom/Into derive
macros [1] can take care of implementing these, but it will need to be
adapted to support bounded integers... :/
But overall, I am rather happy at how simple it was to convert the whole
of nova-core to this.
Note: This RFC uses nova-core's register!() macro for practical
purposes, but the hope is to move this patch on top of the bitfield
macro after it is split out [2].
[1]
https://lore.kernel.org/rust-for-linux/cover.1755235180.git.y.j3ms.n@gmail.com/
[2]
https://lore.kernel.org/rust-for-linux/20251003154748.1687160-1-joelagnelf@nvidia.com/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/bitfield.rs | 366 ++++++++++++++++--------------
drivers/gpu/nova-core/falcon.rs | 134 ++++++-----
drivers/gpu/nova-core/falcon/hal/ga102.rs | 5 +-
drivers/gpu/nova-core/fb/hal/ga100.rs | 3 +-
drivers/gpu/nova-core/gpu.rs | 9 +-
drivers/gpu/nova-core/regs.rs | 139 ++++++------
6 files changed, 353 insertions(+), 303 deletions(-)
diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs
index 16e143658c51..c75d95ef1ae9 100644
--- a/drivers/gpu/nova-core/bitfield.rs
+++ b/drivers/gpu/nova-core/bitfield.rs
@@ -19,21 +19,21 @@
/// Auto = 2,
/// }
///
-/// impl TryFrom<u8> for Mode {
-/// type Error = u8;
-/// fn try_from(value: u8) -> Result<Self, Self::Error> {
-/// match value {
+/// impl TryFrom<BitInt<u32, 4>> for Mode {
+/// type Error = u32;
+/// fn try_from(value: BitInt<u32, 4>) -> Result<Self, Self::Error> {
+/// match *value {
/// 0 => Ok(Mode::Low),
/// 1 => Ok(Mode::High),
/// 2 => Ok(Mode::Auto),
-/// _ => Err(value),
+/// value => Err(value),
/// }
/// }
/// }
///
-/// impl From<Mode> for u8 {
-/// fn from(mode: Mode) -> u8 {
-/// mode as u8
+/// impl From<Mode> for BitInt<u32, 4> {
+/// fn from(mode: Mode) -> BitInt<u32, 4> {
+/// BitInt::from_expr(mode as u32)
/// }
/// }
///
@@ -44,25 +44,29 @@
/// Active = 1,
/// }
///
-/// impl From<bool> for State {
-/// fn from(value: bool) -> Self {
-/// if value { State::Active } else { State::Inactive }
+/// impl From<BitInt<u32, 1>> for State {
+/// fn from(value: BitInt<u32, 1>) -> Self {
+/// if bool::from(value) {
+/// State::Active
+/// } else {
+/// State::Inactive
+/// }
/// }
/// }
///
-/// impl From<State> for bool {
-/// fn from(state: State) -> bool {
+/// impl From<State> for BitInt<u32, 1> {
+/// fn from(state: State) -> BitInt<u32, 1> {
/// match state {
-/// State::Inactive => false,
-/// State::Active => true,
+/// State::Inactive => false.into(),
+/// State::Active => true.into(),
/// }
/// }
/// }
///
/// bitfield! {
/// pub struct ControlReg(u32) {
-/// 7:7 state as bool => State;
-/// 3:0 mode as u8 ?=> Mode;
+/// 7:7 state => State;
+/// 3:0 mode ?=> Mode;
/// }
/// }
/// ```
@@ -112,12 +116,9 @@ fn from(val: $name) -> $storage {
bitfield!(@fields_dispatcher $vis $name $storage { $($fields)* });
};
- // Captures the fields and passes them to all the implementers that require field information.
- //
- // Used to simplify the matching rules for implementers, so they don't need to match the entire
- // complex fields rule even though they only make use of part of it.
+ // Dispatch fields depending on the syntax used.
(@fields_dispatcher $vis:vis $name:ident $storage:ty {
- $($hi:tt:$lo:tt $field:ident as $type:tt
+ $($hi:tt:$lo:tt $field:ident
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
$(, $comment:literal)?
@@ -125,173 +126,208 @@ fn from(val: $name) -> $storage {
)*
}
) => {
- bitfield!(@field_accessors $vis $name $storage {
- $(
- $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- )*
- });
- bitfield!(@debug $name { $($field;)* });
- bitfield!(@default $name { $($field;)* });
- };
-
- // Defines all the field getter/setter methods for `$name`.
- (
- @field_accessors $vis:vis $name:ident $storage:ty {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- $(
- bitfield!(@check_field_bounds $hi:$lo $field as $type);
- )*
-
#[allow(dead_code)]
impl $name {
- $(
- bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- );
- )*
+ $(
+ bitfield!(@private_field_accessors $name $storage : $hi:$lo $field);
+ bitfield!(@public_field_accessors $vis $name $storage : $hi:$lo $field
+ $(?=> $try_into_type)?
+ $(=> $into_type)?
+ $(, $comment)?
+ );
+ )*
}
+
+ bitfield!(@debug $name { $($field;)* });
+ bitfield!(@default $name { $($field;)* });
+
};
- // Boolean fields must have `$hi == $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert!(
- $hi == $lo,
- concat!("boolean field `", stringify!($field), "` covers more than one bit")
- );
- };
- };
-
- // Non-boolean fields must have `$hi >= $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert!(
- $hi >= $lo,
- concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB")
- );
- };
- };
-
- // Catches fields defined as `bool` and convert them into a boolean value.
(
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
- => $into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(
- @leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$into_type>::from(f != 0) }
- bool $into_type => $into_type $(, $comment)?;
- );
- };
-
- // Shortcut for fields defined as `bool` without the `=>` syntax.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
- $(, $comment:literal)?;
- ) => {
- bitfield!(
- @field_accessor $vis $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;
- );
- };
-
- // Catches the `?=>` syntax for non-boolean fields.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- ?=> $try_into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$try_into_type>::try_from(f as $type) } $type $try_into_type =>
- ::core::result::Result<
- $try_into_type,
- <$try_into_type as ::core::convert::TryFrom<$type>>::Error
- >
- $(, $comment)?;);
- };
-
- // Catches the `=>` syntax for non-boolean fields.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- => $into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$into_type>::from(f as $type) } $type $into_type => $into_type $(, $comment)?;);
- };
-
- // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- $(, $comment:literal)?;
- ) => {
- bitfield!(
- @field_accessor $vis $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;
- );
- };
-
- // Generates the accessor methods for a single field.
- (
- @leaf_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
- { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
+ @private_field_accessors $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
) => {
::kernel::macros::paste!(
const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
- const [<$field:upper _MASK>]: $storage = {
- // Generate mask for shifting
- match ::core::mem::size_of::<$storage>() {
- 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
- 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
- 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
- 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
- _ => ::kernel::build_error!("Unsupported storage type size")
- }
- };
+ const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
const [<$field:upper _SHIFT>]: u32 = $lo;
);
+ ::kernel::macros::paste!(
+ fn [<$field _internal>](self) ->
+ ::kernel::num::BitInt<$storage, { $hi + 1 - $lo }> {
+ const MASK: u32 = $name::[<$field:upper _MASK>];
+ const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
+
+ let field = ((self.0 & MASK) >> SHIFT);
+
+ ::kernel::num::BitInt::<$storage, { $hi + 1 - $lo }>::from_expr(field)
+ }
+
+ fn [<set_ $field _internal>](
+ mut self,
+ value: ::kernel::num::BitInt<$storage, { $hi + 1 - $lo }>,
+ ) -> Self
+ {
+ const MASK: u32 = $name::[<$field:upper _MASK>];
+ const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
+
+ let value = (value.get() << SHIFT) & MASK;
+ self.0 = (self.0 & !MASK) | value;
+
+ self
+ }
+
+ fn [<try_set_ $field _internal>]<T>(
+ mut self,
+ value: T,
+ ) -> ::kernel::error::Result<Self>
+ where T: ::kernel::num::TryIntoBitInt<$storage, { $hi + 1 - $lo }>,
+ {
+ const MASK: u32 = $name::[<$field:upper _MASK>];
+ const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
+
+ let value = (
+ value.try_into_bitint().ok_or(::kernel::error::code::EOVERFLOW)?.get() << SHIFT
+ ) & MASK;
+
+ self.0 = (self.0 & !MASK) | value;
+
+ Ok(self)
+ }
+ );
+ };
+
+ // Generates the public accessors for fields infallibly (`=>`) converted to a type.
+ (
+ @public_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+ => $into_type:ty $(, $comment:literal)?
+ ) => {
+ ::kernel::macros::paste!(
+
$(
#[doc="Returns the value of this field:"]
#[doc=$comment]
)?
#[inline(always)]
- $vis fn $field(self) -> $res_type {
- ::kernel::macros::paste!(
- const MASK: $storage = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- );
- let field = ((self.0 & MASK) >> SHIFT);
-
- $process(field)
+ $vis fn $field(self) -> $into_type
+ {
+ self.[<$field _internal>]().into()
}
- ::kernel::macros::paste!(
$(
#[doc="Sets the value of this field:"]
#[doc=$comment]
)?
#[inline(always)]
- $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
- const MASK: $storage = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = ($storage::from($prim_type::from(value)) << SHIFT) & MASK;
- self.0 = (self.0 & !MASK) | value;
-
- self
+ $vis fn [<set_ $field>](self, value: $into_type) -> Self
+ {
+ self.[<set_ $field _internal>](value.into())
}
+
+ /// Private method, for use in the [`Default`] implementation.
+ fn [<$field _default>]() -> $into_type {
+ Default::default()
+ }
+
+ );
+ };
+
+ // Generates the public accessors for fields fallibly (`?=>`) converted to a type.
+ (
+ @public_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+ ?=> $try_into_type:ty $(, $comment:literal)?
+ ) => {
+ ::kernel::macros::paste!(
+
+ $(
+ #[doc="Returns the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ $vis fn $field(self) ->
+ Result<
+ $try_into_type,
+ <$try_into_type as ::core::convert::TryFrom<
+ ::kernel::num::BitInt<$storage, { $hi + 1 - $lo }>
+ >>::Error
+ >
+ {
+ self.[<$field _internal>]().try_into()
+ }
+
+ $(
+ #[doc="Sets the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ $vis fn [<set_ $field>](self, value: $try_into_type) -> Self
+ {
+ self.[<set_ $field _internal>](value.into())
+ }
+
+ /// Private method, for use in the [`Default`] implementation.
+ fn [<$field _default>]() -> $try_into_type {
+ Default::default()
+ }
+
+ );
+ };
+
+ // Generates the public accessors for fields not converted to a type.
+ (
+ @public_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+ $(, $comment:literal)?
+ ) => {
+ ::kernel::macros::paste!(
+
+ $(
+ #[doc="Returns the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ $vis fn $field(self) ->
+ ::kernel::num::BitInt<$storage, { $hi + 1 - $lo }>
+ {
+ self.[<$field _internal>]()
+ }
+
+ $(
+ #[doc="Sets the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ $vis fn [<set_ $field>]<T>(
+ self,
+ value: T,
+ ) -> Self
+ where T: Into<::kernel::num::BitInt<$storage, { $hi + 1 - $lo }>>,
+ {
+ self.[<set_ $field _internal>](value.into())
+ }
+
+ $(
+ #[doc="Attempts to set the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ $vis fn [<try_set_ $field>]<T>(
+ self,
+ value: T,
+ ) -> ::kernel::error::Result<Self>
+ where T: ::kernel::num::TryIntoBitInt<$storage, { $hi + 1 - $lo }>,
+ {
+ Ok(
+ self.[<set_ $field _internal>](
+ value.try_into_bitint().ok_or(::kernel::error::code::EOVERFLOW)?
+ )
+ )
+ }
+
+ /// Private method, for use in the [`Default`] implementation.
+ fn [<$field _default>]() -> ::kernel::num::BitInt<$storage, { $hi + 1 - $lo }> {
+ Default::default()
+ }
+
);
};
@@ -319,7 +355,7 @@ fn default() -> Self {
::kernel::macros::paste!(
$(
- value.[<set_ $field>](Default::default());
+ value.[<set_ $field>](Self::[<$field _default>]());
)*
);
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index fb3561cc9746..7d85a01ea06e 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -7,6 +7,7 @@
use kernel::device;
use kernel::dma::DmaAddress;
use kernel::io::poll::read_poll_timeout;
+use kernel::num::{self, BitInt};
use kernel::prelude::*;
use kernel::sync::aref::ARef;
use kernel::time::delay::fsleep;
@@ -23,11 +24,14 @@
pub(crate) mod sec2;
// TODO[FPRI]: Replace with `ToPrimitive`.
-macro_rules! impl_from_enum_to_u8 {
- ($enum_type:ty) => {
- impl From<$enum_type> for u8 {
+macro_rules! impl_from_enum_to_bounded {
+ ($enum_type:ty, $length:literal) => {
+ impl<T> From<$enum_type> for BitInt<T, $length>
+ where
+ T: From<u8> + num::Boundable,
+ {
fn from(value: $enum_type) -> Self {
- value as u8
+ BitInt::from_expr(T::from(value as u8))
}
}
};
@@ -47,16 +51,19 @@ pub(crate) enum FalconCoreRev {
Rev6 = 6,
Rev7 = 7,
}
-impl_from_enum_to_u8!(FalconCoreRev);
+impl_from_enum_to_bounded!(FalconCoreRev, 4);
// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconCoreRev {
+impl<T> TryFrom<BitInt<T, 4>> for FalconCoreRev
+where
+ T: num::Boundable + num::Integer<Signedness = num::Unsigned>,
+{
type Error = Error;
- fn try_from(value: u8) -> Result<Self> {
+ fn try_from(value: BitInt<T, 4>) -> Result<Self> {
use FalconCoreRev::*;
- let rev = match value {
+ let rev = match u8::from(value) {
1 => Rev1,
2 => Rev2,
3 => Rev3,
@@ -82,24 +89,25 @@ pub(crate) enum FalconCoreRevSubversion {
Subversion2 = 2,
Subversion3 = 3,
}
-impl_from_enum_to_u8!(FalconCoreRevSubversion);
+impl_from_enum_to_bounded!(FalconCoreRevSubversion, 2);
// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconCoreRevSubversion {
- type Error = Error;
-
- fn try_from(value: u8) -> Result<Self> {
+impl<T> From<BitInt<T, 2>> for FalconCoreRevSubversion
+where
+ T: num::Boundable + num::Integer<Signedness = num::Unsigned>,
+{
+ fn from(value: BitInt<T, 2>) -> Self {
use FalconCoreRevSubversion::*;
- let sub_version = match value & 0b11 {
+ match u8::from(value) {
0 => Subversion0,
1 => Subversion1,
2 => Subversion2,
3 => Subversion3,
- _ => return Err(EINVAL),
- };
-
- Ok(sub_version)
+ // TODO: somehow the compiler cannot infer that `value` cannot be > 3. Find a way to
+ // handle this gracefully, or switch back to fallible ops.
+ _ => panic!(),
+ }
}
}
@@ -126,16 +134,19 @@ pub(crate) enum FalconSecurityModel {
/// Also known as High-Secure, Privilege Level 3 or PL3.
Heavy = 3,
}
-impl_from_enum_to_u8!(FalconSecurityModel);
+impl_from_enum_to_bounded!(FalconSecurityModel, 2);
// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconSecurityModel {
+impl<T> TryFrom<BitInt<T, 2>> for FalconSecurityModel
+where
+ T: num::Boundable + num::Integer<Signedness = num::Unsigned>,
+{
type Error = Error;
- fn try_from(value: u8) -> Result<Self> {
+ fn try_from(value: BitInt<T, 2>) -> Result<Self> {
use FalconSecurityModel::*;
- let sec_model = match value {
+ let sec_model = match u8::from(value) {
0 => None,
2 => Light,
3 => Heavy,
@@ -158,14 +169,17 @@ pub(crate) enum FalconModSelAlgo {
#[default]
Rsa3k = 1,
}
-impl_from_enum_to_u8!(FalconModSelAlgo);
+impl_from_enum_to_bounded!(FalconModSelAlgo, 8);
// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconModSelAlgo {
+impl<T> TryFrom<BitInt<T, 8>> for FalconModSelAlgo
+where
+ T: num::Boundable + num::Integer<Signedness = num::Unsigned>,
+{
type Error = Error;
- fn try_from(value: u8) -> Result<Self> {
- match value {
+ fn try_from(value: BitInt<T, 8>) -> Result<Self> {
+ match u8::from(value) {
1 => Ok(FalconModSelAlgo::Rsa3k),
_ => Err(EINVAL),
}
@@ -180,14 +194,17 @@ pub(crate) enum DmaTrfCmdSize {
#[default]
Size256B = 0x6,
}
-impl_from_enum_to_u8!(DmaTrfCmdSize);
+impl_from_enum_to_bounded!(DmaTrfCmdSize, 3);
// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for DmaTrfCmdSize {
+impl<T> TryFrom<BitInt<T, 3>> for DmaTrfCmdSize
+where
+ T: num::Boundable + num::Integer<Signedness = num::Unsigned>,
+{
type Error = Error;
- fn try_from(value: u8) -> Result<Self> {
- match value {
+ fn try_from(value: BitInt<T, 3>) -> Result<Self> {
+ match u8::from(value) {
0x6 => Ok(Self::Size256B),
_ => Err(EINVAL),
}
@@ -203,25 +220,20 @@ pub(crate) enum PeregrineCoreSelect {
/// RISC-V core is active.
Riscv = 1,
}
+impl_from_enum_to_bounded!(PeregrineCoreSelect, 1);
-impl From<bool> for PeregrineCoreSelect {
- fn from(value: bool) -> Self {
- match value {
+impl<T> From<BitInt<T, 1>> for PeregrineCoreSelect
+where
+ T: num::Boundable + Zeroable,
+{
+ fn from(value: BitInt<T, 1>) -> Self {
+ match bool::from(value) {
false => PeregrineCoreSelect::Falcon,
true => PeregrineCoreSelect::Riscv,
}
}
}
-impl From<PeregrineCoreSelect> for bool {
- fn from(value: PeregrineCoreSelect) -> Self {
- match value {
- PeregrineCoreSelect::Falcon => false,
- PeregrineCoreSelect::Riscv => true,
- }
- }
-}
-
/// Different types of memory present in a falcon core.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum FalconMem {
@@ -245,14 +257,17 @@ pub(crate) enum FalconFbifTarget {
/// Non-coherent system memory (System DRAM).
NoncoherentSysmem = 2,
}
-impl_from_enum_to_u8!(FalconFbifTarget);
+impl_from_enum_to_bounded!(FalconFbifTarget, 2);
// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconFbifTarget {
+impl<T> TryFrom<BitInt<T, 2>> for FalconFbifTarget
+where
+ T: num::Boundable + num::Integer<Signedness = num::Unsigned>,
+{
type Error = Error;
- fn try_from(value: u8) -> Result<Self> {
- let res = match value {
+ fn try_from(value: BitInt<T, 2>) -> Result<Self> {
+ let res = match u8::from(value) {
0 => Self::LocalFb,
1 => Self::CoherentSysmem,
2 => Self::NoncoherentSysmem,
@@ -272,26 +287,21 @@ pub(crate) enum FalconFbifMemType {
/// Physical memory addresses.
Physical = 1,
}
+impl_from_enum_to_bounded!(FalconFbifMemType, 1);
/// Conversion from a single-bit register field.
-impl From<bool> for FalconFbifMemType {
- fn from(value: bool) -> Self {
- match value {
+impl<T> From<BitInt<T, 1>> for FalconFbifMemType
+where
+ T: num::Boundable + Zeroable,
+{
+ fn from(value: BitInt<T, 1>) -> Self {
+ match bool::from(value) {
false => Self::Virtual,
true => Self::Physical,
}
}
}
-impl From<FalconFbifMemType> for bool {
- fn from(value: FalconFbifMemType) -> Self {
- match value {
- FalconFbifMemType::Virtual => false,
- FalconFbifMemType::Physical => true,
- }
- }
-}
-
/// Type used to represent the `PFALCON` registers address base for a given falcon engine.
pub(crate) struct PFalconBase(());
@@ -414,7 +424,7 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
self.reset_wait_mem_scrubbing(bar)?;
regs::NV_PFALCON_FALCON_RM::default()
- .set_value(regs::NV_PMC_BOOT_0::read(bar).into())
+ .set_value(u32::from(regs::NV_PMC_BOOT_0::read(bar)))
.write(bar, &E::ID);
Ok(())
@@ -481,18 +491,18 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
.set_base((dma_start >> 8) as u32)
.write(bar, &E::ID);
regs::NV_PFALCON_FALCON_DMATRFBASE1::default()
- .set_base((dma_start >> 40) as u16)
+ .try_set_base(dma_start >> 40)?
.write(bar, &E::ID);
let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::default()
.set_size(DmaTrfCmdSize::Size256B)
.set_imem(target_mem == FalconMem::Imem)
- .set_sec(if sec { 1 } else { 0 });
+ .set_sec(BitInt::from_expr(if sec { 1 } else { 0 }));
for pos in (0..num_transfers).map(|i| i * DMA_LEN) {
// Perform a transfer of size `DMA_LEN`.
regs::NV_PFALCON_FALCON_DMATRFMOFFS::default()
- .set_offs(load_offsets.dst_start + pos)
+ .try_set_offs(load_offsets.dst_start + pos)?
.write(bar, &E::ID);
regs::NV_PFALCON_FALCON_DMATRFFBOFFS::default()
.set_offs(src_start + pos)
diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index afed353b24d2..c43e48823eff 100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -51,7 +51,7 @@ fn signature_reg_fuse_version_ga102(
// `ucode_idx` is guaranteed to be in the range [0..15], making the `read` calls provable valid
// at build-time.
- let reg_fuse_version = if engine_id_mask & 0x0001 != 0 {
+ let reg_fuse_version: u16 = if engine_id_mask & 0x0001 != 0 {
regs::NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION::read(bar, ucode_idx).data()
} else if engine_id_mask & 0x0004 != 0 {
regs::NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION::read(bar, ucode_idx).data()
@@ -60,7 +60,8 @@ fn signature_reg_fuse_version_ga102(
} else {
dev_err!(dev, "unexpected engine_id_mask {:#x}", engine_id_mask);
return Err(EINVAL);
- };
+ }
+ .into();
// TODO[NUMM]: replace with `last_set_bit` once it lands.
Ok(u16::BITS - reg_fuse_version.leading_zeros())
diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
index 871c42bf033a..5b55ca8aaddb 100644
--- a/drivers/gpu/nova-core/fb/hal/ga100.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
@@ -2,6 +2,7 @@
struct Ga100;
+use kernel::num::BitInt;
use kernel::prelude::*;
use crate::driver::Bar0;
@@ -18,7 +19,7 @@ pub(super) fn read_sysmem_flush_page_ga100(bar: &Bar0) -> u64 {
pub(super) fn write_sysmem_flush_page_ga100(bar: &Bar0, addr: u64) {
regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI::default()
- .set_adr_63_40((addr >> FLUSH_SYSMEM_ADDR_SHIFT_HI) as u32)
+ .set_adr_63_40(BitInt::from_expr(addr >> FLUSH_SYSMEM_ADDR_SHIFT_HI).cast())
.write(bar);
regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::default()
.set_adr_39_08((addr >> FLUSH_SYSMEM_ADDR_SHIFT) as u32)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 9d182bffe8b4..2db3e48ea59f 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+use kernel::num::BitInt;
use kernel::{device, devres::Devres, error::code::*, fmt, pci, prelude::*, sync::Arc};
use crate::driver::Bar0;
@@ -130,15 +131,15 @@ fn try_from(value: u8) -> Result<Self> {
}
pub(crate) struct Revision {
- major: u8,
- minor: u8,
+ major: BitInt<u8, 4>,
+ minor: BitInt<u8, 4>,
}
impl Revision {
fn from_boot0(boot0: regs::NV_PMC_BOOT_0) -> Self {
Self {
- major: boot0.major_revision(),
- minor: boot0.minor_revision(),
+ major: boot0.major_revision().cast(),
+ minor: boot0.minor_revision().cast(),
}
}
}
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..1542d72e4a65 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -17,18 +17,19 @@
// PMC
register!(NV_PMC_BOOT_0 @ 0x00000000, "Basic revision information about the GPU" {
- 3:0 minor_revision as u8, "Minor revision of the chip";
- 7:4 major_revision as u8, "Major revision of the chip";
- 8:8 architecture_1 as u8, "MSB of the architecture";
- 23:20 implementation as u8, "Implementation version of the architecture";
- 28:24 architecture_0 as u8, "Lower bits of the architecture";
+ 3:0 minor_revision, "Minor revision of the chip";
+ 7:4 major_revision, "Major revision of the chip";
+ 8:8 architecture_1, "MSB of the architecture";
+ 23:20 implementation, "Implementation version of the architecture";
+ 28:24 architecture_0, "Lower bits of the architecture";
});
impl NV_PMC_BOOT_0 {
/// Combines `architecture_0` and `architecture_1` to obtain the architecture of the chip.
pub(crate) fn architecture(self) -> Result<Architecture> {
Architecture::try_from(
- self.architecture_0() | (self.architecture_1() << Self::ARCHITECTURE_0_RANGE.len()),
+ u8::from(self.architecture_0())
+ | (u8::from(self.architecture_1()) << Self::ARCHITECTURE_0_RANGE.len()),
)
}
@@ -49,7 +50,7 @@ pub(crate) fn chipset(self) -> Result<Chipset> {
register!(NV_PBUS_SW_SCRATCH_0E_FRTS_ERR => NV_PBUS_SW_SCRATCH[0xe],
"scratch register 0xe used as FRTS firmware error code" {
- 31:16 frts_err_code as u16;
+ 31:16 frts_err_code;
});
// PFB
@@ -58,17 +59,17 @@ pub(crate) fn chipset(self) -> Result<Chipset> {
// GPU to perform sysmembar operations (see `fb::SysmemFlush`).
register!(NV_PFB_NISO_FLUSH_SYSMEM_ADDR @ 0x00100c10 {
- 31:0 adr_39_08 as u32;
+ 31:0 adr_39_08;
});
register!(NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI @ 0x00100c40 {
- 23:0 adr_63_40 as u32;
+ 23:0 adr_63_40;
});
register!(NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE @ 0x00100ce0 {
- 3:0 lower_scale as u8;
- 9:4 lower_mag as u8;
- 30:30 ecc_mode_enabled as bool;
+ 3:0 lower_scale;
+ 9:4 lower_mag;
+ 30:30 ecc_mode_enabled => bool;
});
impl NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE {
@@ -87,7 +88,7 @@ pub(crate) fn usable_fb_size(self) -> u64 {
}
register!(NV_PFB_PRI_MMU_WPR2_ADDR_LO@0x001fa824 {
- 31:4 lo_val as u32, "Bits 12..40 of the lower (inclusive) bound of the WPR2 region";
+ 31:4 lo_val, "Bits 12..40 of the lower (inclusive) bound of the WPR2 region";
});
impl NV_PFB_PRI_MMU_WPR2_ADDR_LO {
@@ -98,7 +99,7 @@ pub(crate) fn lower_bound(self) -> u64 {
}
register!(NV_PFB_PRI_MMU_WPR2_ADDR_HI@0x001fa828 {
- 31:4 hi_val as u32, "Bits 12..40 of the higher (exclusive) bound of the WPR2 region";
+ 31:4 hi_val, "Bits 12..40 of the higher (exclusive) bound of the WPR2 region";
});
impl NV_PFB_PRI_MMU_WPR2_ADDR_HI {
@@ -123,7 +124,7 @@ pub(crate) fn higher_bound(self) -> u64 {
// `PGC6_AON_SECURE_SCRATCH_GROUP_05` register (which it needs to read GFW_BOOT).
register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK @ 0x00118128,
"Privilege level mask register" {
- 0:0 read_protection_level0 as bool, "Set after FWSEC lowers its protection level";
+ 0:0 read_protection_level0 => bool, "Set after FWSEC lowers its protection level";
});
// OpenRM defines this as a register array, but doesn't specify its size and only uses its first
@@ -133,7 +134,7 @@ pub(crate) fn higher_bound(self) -> u64 {
register!(
NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_0_GFW_BOOT => NV_PGC6_AON_SECURE_SCRATCH_GROUP_05[0],
"Scratch group 05 register 0 used as GFW boot progress indicator" {
- 7:0 progress as u8, "Progress of GFW boot (0xff means completed)";
+ 7:0 progress, "Progress of GFW boot (0xff means completed)";
}
);
@@ -145,13 +146,13 @@ pub(crate) fn completed(self) -> bool {
}
register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_42 @ 0x001183a4 {
- 31:0 value as u32;
+ 31:0 value;
});
register!(
NV_USABLE_FB_SIZE_IN_MB => NV_PGC6_AON_SECURE_SCRATCH_GROUP_42,
"Scratch group 42 register used as framebuffer size" {
- 31:0 value as u32, "Usable framebuffer size, in megabytes";
+ 31:0 value, "Usable framebuffer size, in megabytes";
}
);
@@ -165,8 +166,8 @@ pub(crate) fn usable_fb_size(self) -> u64 {
// PDISP
register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
- 3:3 status_valid as bool, "Set if the `addr` field is valid";
- 31:8 addr as u32, "VGA workspace base address divided by 0x10000";
+ 3:3 status_valid => bool, "Set if the `addr` field is valid";
+ 31:8 addr, "VGA workspace base address divided by 0x10000";
});
impl NV_PDISP_VGA_WORKSPACE_BASE {
@@ -185,40 +186,40 @@ pub(crate) fn vga_workspace_addr(self) -> Option<u64> {
pub(crate) const NV_FUSE_OPT_FPF_SIZE: usize = 16;
register!(NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION @ 0x00824100[NV_FUSE_OPT_FPF_SIZE] {
- 15:0 data as u16;
+ 15:0 data;
});
register!(NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION @ 0x00824140[NV_FUSE_OPT_FPF_SIZE] {
- 15:0 data as u16;
+ 15:0 data;
});
register!(NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION @ 0x008241c0[NV_FUSE_OPT_FPF_SIZE] {
- 15:0 data as u16;
+ 15:0 data;
});
// PFALCON
register!(NV_PFALCON_FALCON_IRQSCLR @ PFalconBase[0x00000004] {
- 4:4 halt as bool;
- 6:6 swgen0 as bool;
+ 4:4 halt => bool;
+ 6:6 swgen0 => bool;
});
register!(NV_PFALCON_FALCON_MAILBOX0 @ PFalconBase[0x00000040] {
- 31:0 value as u32;
+ 31:0 value => u32;
});
register!(NV_PFALCON_FALCON_MAILBOX1 @ PFalconBase[0x00000044] {
- 31:0 value as u32;
+ 31:0 value => u32;
});
register!(NV_PFALCON_FALCON_RM @ PFalconBase[0x00000084] {
- 31:0 value as u32;
+ 31:0 value => u32;
});
register!(NV_PFALCON_FALCON_HWCFG2 @ PFalconBase[0x000000f4] {
- 10:10 riscv as bool;
- 12:12 mem_scrubbing as bool, "Set to 0 after memory scrubbing is completed";
- 31:31 reset_ready as bool, "Signal indicating that reset is completed (GA102+)";
+ 10:10 riscv => bool;
+ 12:12 mem_scrubbing => bool, "Set to 0 after memory scrubbing is completed";
+ 31:31 reset_ready => bool, "Signal indicating that reset is completed (GA102+)";
});
impl NV_PFALCON_FALCON_HWCFG2 {
@@ -229,101 +230,101 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
}
register!(NV_PFALCON_FALCON_CPUCTL @ PFalconBase[0x00000100] {
- 1:1 startcpu as bool;
- 4:4 halted as bool;
- 6:6 alias_en as bool;
+ 1:1 startcpu => bool;
+ 4:4 halted => bool;
+ 6:6 alias_en => bool;
});
register!(NV_PFALCON_FALCON_BOOTVEC @ PFalconBase[0x00000104] {
- 31:0 value as u32;
+ 31:0 value => u32;
});
register!(NV_PFALCON_FALCON_DMACTL @ PFalconBase[0x0000010c] {
- 0:0 require_ctx as bool;
- 1:1 dmem_scrubbing as bool;
- 2:2 imem_scrubbing as bool;
- 6:3 dmaq_num as u8;
- 7:7 secure_stat as bool;
+ 0:0 require_ctx => bool;
+ 1:1 dmem_scrubbing => bool;
+ 2:2 imem_scrubbing => bool;
+ 6:3 dmaq_num;
+ 7:7 secure_stat => bool;
});
register!(NV_PFALCON_FALCON_DMATRFBASE @ PFalconBase[0x00000110] {
- 31:0 base as u32;
+ 31:0 base => u32;
});
register!(NV_PFALCON_FALCON_DMATRFMOFFS @ PFalconBase[0x00000114] {
- 23:0 offs as u32;
+ 23:0 offs;
});
register!(NV_PFALCON_FALCON_DMATRFCMD @ PFalconBase[0x00000118] {
- 0:0 full as bool;
- 1:1 idle as bool;
- 3:2 sec as u8;
- 4:4 imem as bool;
- 5:5 is_write as bool;
- 10:8 size as u8 ?=> DmaTrfCmdSize;
- 14:12 ctxdma as u8;
- 16:16 set_dmtag as u8;
+ 0:0 full => bool;
+ 1:1 idle => bool;
+ 3:2 sec;
+ 4:4 imem => bool;
+ 5:5 is_write => bool;
+ 10:8 size ?=> DmaTrfCmdSize;
+ 14:12 ctxdma;
+ 16:16 set_dmtag;
});
register!(NV_PFALCON_FALCON_DMATRFFBOFFS @ PFalconBase[0x0000011c] {
- 31:0 offs as u32;
+ 31:0 offs => u32;
});
register!(NV_PFALCON_FALCON_DMATRFBASE1 @ PFalconBase[0x00000128] {
- 8:0 base as u16;
+ 8:0 base;
});
register!(NV_PFALCON_FALCON_HWCFG1 @ PFalconBase[0x0000012c] {
- 3:0 core_rev as u8 ?=> FalconCoreRev, "Core revision";
- 5:4 security_model as u8 ?=> FalconSecurityModel, "Security model";
- 7:6 core_rev_subversion as u8 ?=> FalconCoreRevSubversion, "Core revision subversion";
+ 3:0 core_rev ?=> FalconCoreRev, "Core revision";
+ 5:4 security_model ?=> FalconSecurityModel, "Security model";
+ 7:6 core_rev_subversion => FalconCoreRevSubversion, "Core revision subversion";
});
register!(NV_PFALCON_FALCON_CPUCTL_ALIAS @ PFalconBase[0x00000130] {
- 1:1 startcpu as bool;
+ 1:1 startcpu => bool;
});
// Actually known as `NV_PSEC_FALCON_ENGINE` and `NV_PGSP_FALCON_ENGINE` depending on the falcon
// instance.
register!(NV_PFALCON_FALCON_ENGINE @ PFalconBase[0x000003c0] {
- 0:0 reset as bool;
+ 0:0 reset => bool;
});
register!(NV_PFALCON_FBIF_TRANSCFG @ PFalconBase[0x00000600[8]] {
- 1:0 target as u8 ?=> FalconFbifTarget;
- 2:2 mem_type as bool => FalconFbifMemType;
+ 1:0 target ?=> FalconFbifTarget;
+ 2:2 mem_type => FalconFbifMemType;
});
register!(NV_PFALCON_FBIF_CTL @ PFalconBase[0x00000624] {
- 7:7 allow_phys_no_ctx as bool;
+ 7:7 allow_phys_no_ctx => bool;
});
/* PFALCON2 */
register!(NV_PFALCON2_FALCON_MOD_SEL @ PFalcon2Base[0x00000180] {
- 7:0 algo as u8 ?=> FalconModSelAlgo;
+ 7:0 algo ?=> FalconModSelAlgo;
});
register!(NV_PFALCON2_FALCON_BROM_CURR_UCODE_ID @ PFalcon2Base[0x00000198] {
- 7:0 ucode_id as u8;
+ 7:0 ucode_id => u8;
});
register!(NV_PFALCON2_FALCON_BROM_ENGIDMASK @ PFalcon2Base[0x0000019c] {
- 31:0 value as u32;
+ 31:0 value => u32;
});
// OpenRM defines this as a register array, but doesn't specify its size and only uses its first
// element. Be conservative until we know the actual size or need to use more registers.
register!(NV_PFALCON2_FALCON_BROM_PARAADDR @ PFalcon2Base[0x00000210[1]] {
- 31:0 value as u32;
+ 31:0 value => u32;
});
// PRISCV
register!(NV_PRISCV_RISCV_BCR_CTRL @ PFalconBase[0x00001668] {
- 0:0 valid as bool;
- 4:4 core_select as bool => PeregrineCoreSelect;
- 8:8 br_fetch as bool;
+ 0:0 valid => bool;
+ 4:4 core_select => PeregrineCoreSelect;
+ 8:8 br_fetch => bool;
});
// The modules below provide registers that are not identical on all supported chips. They should
@@ -333,7 +334,7 @@ pub(crate) mod gm107 {
// FUSE
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 {
- 0:0 display_disabled as bool;
+ 0:0 display_disabled => bool;
});
}
@@ -341,6 +342,6 @@ pub(crate) mod ga100 {
// FUSE
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 {
- 0:0 display_disabled as bool;
+ 0:0 display_disabled => bool;
});
}
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-10-31 13:39 ` [PATCH 1/2] rust: add BitInt integer wrapping type Alexandre Courbot
@ 2025-11-02 5:46 ` Alexandre Courbot
2025-11-03 2:17 ` Yury Norov
1 sibling, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2025-11-02 5:46 UTC (permalink / raw)
To: Alexandre Courbot, Alice Ryhl, Danilo Krummrich, Miguel Ojeda,
Joel Fernandes, Yury Norov, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: linux-kernel, rust-for-linux
Quick self-review as I've found a couple of points that can be improved.
On Fri Oct 31, 2025 at 10:39 PM JST, Alexandre Courbot wrote:
<snip>
> diff --git a/rust/kernel/num/bitint.rs b/rust/kernel/num/bitint.rs
> new file mode 100644
> index 000000000000..5af1447347a8
> --- /dev/null
> +++ b/rust/kernel/num/bitint.rs
> @@ -0,0 +1,896 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! [`BitInt`], a primitive integer type with a limited set of bits usable to represent values.
> +
> +use core::ops::Deref;
> +
> +use kernel::num::{Integer, Signed, Unsigned};
> +use kernel::prelude::*;
> +
> +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
> +///
> +/// Can be used in const context.
> +macro_rules! fits_within {
> + ($value:expr, $type:ty, $num_bits:expr) => {{
> + // Any attempt to create a `BitInt` with more bits used for representation than its backing
> + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
> + build_assert!(
> + <$type>::BITS >= $num_bits,
> + "Number of bits requested for representation is larger than backing type."
> + );
All these similar invariant checks sprinkled around the code are very
error-prone. A better way to enforce them is to have a private
constructor that is called by any path that creates a new `BitInt`, and
have these invariants enforced in a const block:
const fn __new(value: T) -> Self {
// Enforce the type invariants.
const {
// `NUM_BITS` cannot be zero.
build_assert!(NUM_BITS != 0);
// The backing type is at least as large as `NUM_BITS`.
build_assert!(NUM_BITS <= T::BITS);
}
Self(value)
}
This means we do this in one place instead of 3 or 4, and since the
invariants are enforced in const context we get a decent error message
should they fail.
<snip>
> +mod atleast_impls {
> + use super::*;
> +
> + // Number of bits at least as large as `64`.
> + impl_size_rule!(AtLeast64Bits, 64);
> +
> + // Number of bits at least as large as `32`.
> + impl_size_rule!(AtLeast32Bits,
> + 32 33 34 35 36 37 38 39
> + 40 41 42 43 44 45 46 47
> + 48 49 50 51 52 53 54 55
> + 56 57 58 59 60 61 62 63
> + );
> + // Anything larger than `64` is also larger than `32`.
> + impl<T> AtLeast32Bits for T where T: AtLeast64Bits {}
> +
> + // Number of bits at least as large as `16`.
> + impl_size_rule!(AtLeast16Bits,
> + 16 17 18 19 20 21 22 23
> + 24 25 26 27 28 29 30 31
> + );
> + // Anything larger than `32` is also larger than `16`.
> + impl<T> AtLeast16Bits for T where T: AtLeast32Bits {}
> +
> + // Number of bits at least as large as a `8`.
> + impl_size_rule!(AtLeast8Bits, 8 9 10 11 12 13 14 15);
> + // Anything larger than `16` is also larger than `8`.
> + impl<T> AtLeast8Bits for T where T: AtLeast16Bits {}
> +
> + // Number of bits at least as large as `1`.
> + impl_size_rule!(AtLeast1Bit, 1 2 3 4 5 6 7);
> + // Anything larger than `8` is also larger than `1`.
> + impl<T> AtLeast1Bit for T where T: AtLeast8Bits {}
> +
> + /// Generates `From` implementations from a primitive type into a [`BitInt`] that is
> + /// guaranteed to being able to contain it.
> + macro_rules! impl_from_primitive {
> + ($($type:ty => $trait:ident),*) => {
> + $(
> + impl<T, const NUM_BITS: u32> From<$type> for BitInt<T, NUM_BITS>
> + where
> + Self: $trait,
> + T: From<$type> + Boundable,
> + {
> + fn from(value: $type) -> Self {
> + build_assert!(
> + T::BITS >= NUM_BITS,
> + "Number of bits requested for representation is larger than backing type."
> + );
> + // INVARIANT: the impl constraints guarantee that the source type is smaller
> + // than `NUM_BITS`, and the `build_assert` above that the backing type can
> + // contain `NUM_BITS`.
> + Self(T::from(value))
> + }
> + }
> + )*
> + }
> + }
> +
> + impl_from_primitive!(
> + bool => AtLeast1Bit,
> + u8 => AtLeast8Bits,
> + i8 => AtLeast8Bits,
> + u16 => AtLeast16Bits,
> + i16 => AtLeast16Bits,
> + u32 => AtLeast32Bits,
> + i32 => AtLeast32Bits,
> + u64 =>AtLeast64Bits,
> + i64 =>AtLeast64Bits
> + );
I dislike this part so much, especially since it could be done much more
cleanly if only we had `generic_const_exprs`. But for now we have to
work our way around with macros.
Still, this can be simplified significantly by replacing the
`AtLeast8Bits`, `AtLeast16Bits`, ... traits with a single
`AtLeastXBits<const N: usize>`.
By doing so, the cryptic
Self: $trait,
constraint can become
Self: AtLeastXBits<{ <$type as Integer>::BITS as usize }>,
which is much more explicit about its purpose, and we can remove the
`$trait` argument from the macro.
what more, we can even write a constraint like
Self: AtLeastXBits<{ <$type as Integer>::BITS as usize + 1 }>,
And support conversion from unsigned primitive types to signed `BitInt`s
with at least one more bit.
The same simplification can also be done for the `IntoXX` traits below.
I'll submit a v2 soonish with these improvements and some more minor
fixes.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-10-31 13:39 ` [PATCH 1/2] rust: add BitInt integer wrapping type Alexandre Courbot
2025-11-02 5:46 ` Alexandre Courbot
@ 2025-11-03 2:17 ` Yury Norov
2025-11-03 10:47 ` Alice Ryhl
` (2 more replies)
1 sibling, 3 replies; 22+ messages in thread
From: Yury Norov @ 2025-11-03 2:17 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Fri, Oct 31, 2025 at 10:39:57PM +0900, Alexandre Courbot wrote:
> Add the `BitInt` type, which is an integer on which the number of bits
> allowed to be used is restricted, capping its maximal value below that
> of primitive type is wraps.
>
> This is useful to e.g. enforce guarantees when working with bit fields.
>
> Alongside this type, provide many `From` and `TryFrom` implementations
> are 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 | 75 ++++
> rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 972 insertions(+)
...
> +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
> +///
> +/// Can be used in const context.
> +macro_rules! fits_within {
> + ($value:expr, $type:ty, $num_bits:expr) => {{
> + // Any attempt to create a `BitInt` with more bits used for representation than its backing
> + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
> + build_assert!(
> + <$type>::BITS >= $num_bits,
> + "Number of bits requested for representation is larger than backing type."
> + );
> +
> + let shift: u32 = <$type>::BITS - $num_bits;
> + let v = $value;
> +
> + // The value fits within `NUM_BITS` if shifting it left by the number of unused bits, then
> + // right by the same number, doesn't change the value.
> + //
> + // This method has the benefit of working with both unsigned and signed integers.
> + (v << shift) >> shift == v
In C it doesn't work:
int c = 0x7fffffff;
printf("%x\t%x\n", c, (c << 1) >> 1); // 7fffffff ffffffff
Neither in rust:
let c: i32 = 0x7fffffff;
println!("{} {}", c, (c << 1) >> 1); // 2147483647 -1
Or I misunderstand something?
> + }};
> +}
> +
> +/// Trait for primitive integer types that can be used to back a [`BitInt`].
> +///
> +/// This is mostly used to lock all the operations we need for [`BitInt`] in a single trait.
> +pub trait Boundable
> +where
> + Self: Integer
> + + Sized
> + + Copy
> + + core::ops::Shl<u32, Output = Self>
> + + core::ops::Shr<u32, Output = Self>
> + + core::cmp::PartialEq,
> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
> +{
> + /// Returns `true` if `value` can be represented with at most `NUM_BITS` on `T`.
> + fn fits_within(value: Self, num_bits: u32) -> bool {
> + fits_within!(value, Self, num_bits)
> + }
> +}
> +
> +/// Inplement `Boundable` for all integers types.
> +impl<T> Boundable for T
> +where
> + T: Integer
> + + Sized
> + + Copy
> + + core::ops::Shl<u32, Output = Self>
> + + core::ops::Shr<u32, Output = Self>
> + + core::cmp::PartialEq,
> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
> +{
> +}
> +
> +/// Integer type for which only the bits `0..NUM_BITS` are valid.
> +///
> +/// # Invariants
> +///
> +/// Stored values are represented with at most `NUM_BITS` bits.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::num::BitInt;
> +///
> +/// // An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
> +/// // The value `15` is statically validated to fit within the specified number of bits.
> +/// let v = BitInt::<u8, 4>::new::<15>();
What for do you make user to declare the storage explicitly? From
end-user perspective, declaring 4-bit variable must imply the most
suitable storage... C version of the same doesn't allow user to select
the storage as well:
_BitInt(4) x = 8;
I can't think out any useful usecase for this... I believe that the
optimal storage must be chosen by implementation. And it may even be
different for different arches.
> +/// assert_eq!(v.get(), 15);
> +///
> +/// let v = BitInt::<i8, 4>::new::<-8>();
> +/// assert_eq!(v.get(), -8);
> +///
> +/// // This doesn't build: a `u8` is smaller than the requested 9 bits.
> +/// // let _ = BitInt::<u8, 9>::new::<10>();
> +///
> +/// // This also doesn't build: the requested value doesn't fit within the requested bits.
> +/// // let _ = BitInt::<i8, 4>::new::<8>();
> +///
> +/// // Values can also be validated at runtime. This succeeds because `15` can be represented
> +/// // with 4 bits.
> +/// assert!(BitInt::<u8, 4>::try_new(15).is_some());
> +/// // This fails because `16` cannot be represented with 4 bits.
> +/// assert!(BitInt::<u8, 4>::try_new(16).is_none());
Nice! Maybe .is_overflow() instead of .is_none(), so that user will
know that the variable contains truncated value. Just like C does.
Can you please print the exact error that user will get on compile-
and runtime? How big is the cost of runtime test for overflow? If it
is indeed nonzero, can you consider making the runtime part
configurable?
> +/// // Non-constant expressions can also be validated at build-time thanks to compiler
> +/// // optimizations. This should be used as a last resort though.
> +/// let v = BitInt::<u8, 4>::from_expr(15);
Not sure I understand that. Can you confirm my understanding?
1. For compile-time initialization I use BitInt::<i8, 4>::new::<8>();
2. For compile- or runtime initialization: BitInt::<i8, 4>::from_expr(val);
3. For runtime-only initialization: BitInt::<i8, 4>::try_new(val);
In this scheme #3 looks excessive...
> +/// // Common operations are supported against the backing type.
> +/// assert_eq!(v + 5, 20);
> +/// assert_eq!(v / 3, 5);
No, v + 5 == 20 for a different reason. There's nothing about 'backing
storage' here.
v + 5 should be 20 because addition implies typecasting to the wider
type. In this case, 20 is numeral, or int, and BitInt(4) + int == int.
I tried C23, and it works exactly like that:
unsigned _BitInt(4) x = 15;
printf("%d\n", x + 5); // 20
printf("%d\n", x / 3); // 5
printf("%d\n", x + (unsigned _BitInt(4))5); // 4
x += 5;
printf("%d\n", x); // 4
Rust _must_ do the same thing to at least be arithmetically
compatible to the big brother.
It makes me more confident that this 'backing storage' concept
brings nothing but confusion.
> +/// // The backing type can be changed while preserving the number of bits used for representation.
> +/// assert_eq!(v.cast::<u32>(), BitInt::<u32, 4>::new::<15>());
> +///
> +/// // We can safely extend the number of bits...
> +/// assert_eq!(v.extend::<5>(), BitInt::<u8, 5>::new::<15>());
> +/// // ... but reducing the number of bits fails here as the value doesn't fit anymore.
> +/// assert_eq!(v.try_shrink::<3>(), None);
Not sure what for you need this. If I need to 'extend', I just assign
the value to a variable:
BitInt(3) a = 3;
BitInt(10) b;
int c;
b = a + 123; // extend
c = b; // another extend
How would this 'extend' and 'shrink' work with arrays of BitInts?
> +/// // Conversion into primitive types is dependent on the number of useful bits, not the backing
> +/// // type.
> +/// //
> +/// // Event though its backing type is `u32`, this `BitInt` only uses 8 bits and thus can safely
> +/// // be converted to a `u8`.
> +/// assert_eq!(u8::from(BitInt::<u32, 8>::new::<128>()), 128u8);
'Backing type' is useless here too.
> +/// // The same applies to signed values.
> +/// asserkkt_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
> +///
> +/// // This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
> +/// // contained value).
> +/// // let _ = u8::from(BitInt::<u32, 10>::new::<10>());
If I explicitly typecast from a wider type, please just let me do
that. In the above examples you show that .is_some() and .is_none()
can help user to check for overflow if needed.
Otherwise, user will hack your protection by just converting
BitInt(8) to u32, and then to BitInt(10).
Thanks,
Yury
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 2:17 ` Yury Norov
@ 2025-11-03 10:47 ` Alice Ryhl
2025-11-03 14:31 ` Alexandre Courbot
2025-11-03 13:42 ` Alexandre Courbot
2025-11-03 14:10 ` Miguel Ojeda
2 siblings, 1 reply; 22+ messages in thread
From: Alice Ryhl @ 2025-11-03 10:47 UTC (permalink / raw)
To: Yury Norov
Cc: Alexandre Courbot, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Sun, Nov 02, 2025 at 09:17:35PM -0500, Yury Norov wrote:
> On Fri, Oct 31, 2025 at 10:39:57PM +0900, Alexandre Courbot wrote:
> > Add the `BitInt` type, which is an integer on which the number of bits
> > allowed to be used is restricted, capping its maximal value below that
> > of primitive type is wraps.
> >
> > This is useful to e.g. enforce guarantees when working with bit fields.
> >
> > Alongside this type, provide many `From` and `TryFrom` implementations
> > are 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 | 75 ++++
> > rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 972 insertions(+)
>
> ...
>
> > +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
> > +///
> > +/// Can be used in const context.
> > +macro_rules! fits_within {
> > + ($value:expr, $type:ty, $num_bits:expr) => {{
> > + // Any attempt to create a `BitInt` with more bits used for representation than its backing
> > + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
> > + build_assert!(
> > + <$type>::BITS >= $num_bits,
> > + "Number of bits requested for representation is larger than backing type."
> > + );
> > +
> > + let shift: u32 = <$type>::BITS - $num_bits;
> > + let v = $value;
> > +
> > + // The value fits within `NUM_BITS` if shifting it left by the number of unused bits, then
> > + // right by the same number, doesn't change the value.
> > + //
> > + // This method has the benefit of working with both unsigned and signed integers.
> > + (v << shift) >> shift == v
>
> In C it doesn't work:
>
> int c = 0x7fffffff;
> printf("%x\t%x\n", c, (c << 1) >> 1); // 7fffffff ffffffff
>
> Neither in rust:
>
> let c: i32 = 0x7fffffff;
> println!("{} {}", c, (c << 1) >> 1); // 2147483647 -1
>
> Or I misunderstand something?
I think we should probably just define a BitInt::MIN and BitInt::MAX
constant and have this code check MIN <= v && v <= MAX.
> > + }};
> > +}
> > +
> > +/// Trait for primitive integer types that can be used to back a [`BitInt`].
> > +///
> > +/// This is mostly used to lock all the operations we need for [`BitInt`] in a single trait.
> > +pub trait Boundable
> > +where
> > + Self: Integer
> > + + Sized
> > + + Copy
> > + + core::ops::Shl<u32, Output = Self>
> > + + core::ops::Shr<u32, Output = Self>
> > + + core::cmp::PartialEq,
> > + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
> > + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
> > +{
> > + /// Returns `true` if `value` can be represented with at most `NUM_BITS` on `T`.
> > + fn fits_within(value: Self, num_bits: u32) -> bool {
> > + fits_within!(value, Self, num_bits)
> > + }
> > +}
> > +
> > +/// Inplement `Boundable` for all integers types.
> > +impl<T> Boundable for T
> > +where
> > + T: Integer
> > + + Sized
> > + + Copy
> > + + core::ops::Shl<u32, Output = Self>
> > + + core::ops::Shr<u32, Output = Self>
> > + + core::cmp::PartialEq,
> > + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
> > + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
> > +{
> > +}
> > +
> > +/// Integer type for which only the bits `0..NUM_BITS` are valid.
> > +///
> > +/// # Invariants
> > +///
> > +/// Stored values are represented with at most `NUM_BITS` bits.
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::num::BitInt;
> > +///
> > +/// // An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
> > +/// // The value `15` is statically validated to fit within the specified number of bits.
> > +/// let v = BitInt::<u8, 4>::new::<15>();
>
> What for do you make user to declare the storage explicitly? From
> end-user perspective, declaring 4-bit variable must imply the most
> suitable storage... C version of the same doesn't allow user to select
> the storage as well:
>
> _BitInt(4) x = 8;
>
> I can't think out any useful usecase for this... I believe that the
> optimal storage must be chosen by implementation. And it may even be
> different for different arches.
It's more complex to not specify the backing storage explicitly, but I
also think it would be nice to be able to avoid it.
> > +/// assert_eq!(v.get(), 15);
> > +///
> > +/// let v = BitInt::<i8, 4>::new::<-8>();
> > +/// assert_eq!(v.get(), -8);
> > +///
> > +/// // This doesn't build: a `u8` is smaller than the requested 9 bits.
> > +/// // let _ = BitInt::<u8, 9>::new::<10>();
> > +///
> > +/// // This also doesn't build: the requested value doesn't fit within the requested bits.
> > +/// // let _ = BitInt::<i8, 4>::new::<8>();
> > +///
> > +/// // Values can also be validated at runtime. This succeeds because `15` can be represented
> > +/// // with 4 bits.
> > +/// assert!(BitInt::<u8, 4>::try_new(15).is_some());
> > +/// // This fails because `16` cannot be represented with 4 bits.
> > +/// assert!(BitInt::<u8, 4>::try_new(16).is_none());
>
> Nice! Maybe .is_overflow() instead of .is_none(), so that user will
> know that the variable contains truncated value. Just like C does.
>
> Can you please print the exact error that user will get on compile-
> and runtime? How big is the cost of runtime test for overflow? If it
> is indeed nonzero, can you consider making the runtime part
> configurable?
This uses the stdlib type Option<_> that either contains a value
(Some(_)) or does not None, and .is_none() is a stdlib provided type so
we cannot configure its name.
> > +/// // Non-constant expressions can also be validated at build-time thanks to compiler
> > +/// // optimizations. This should be used as a last resort though.
> > +/// let v = BitInt::<u8, 4>::from_expr(15);
>
> Not sure I understand that. Can you confirm my understanding?
>
> 1. For compile-time initialization I use BitInt::<i8, 4>::new::<8>();
> 2. For compile- or runtime initialization: BitInt::<i8, 4>::from_expr(val);
> 3. For runtime-only initialization: BitInt::<i8, 4>::try_new(val);
>
> In this scheme #3 looks excessive...
I'm not sure we should have `from_expr` at all. What it does is emit an
if condition checking whether the value is in-bounds containing a call
to a function that does not exist, so that if the optimizer prove the
range check at compile-time, then there is a linker error.
What is the use-case for from_expr?
> > +/// // Common operations are supported against the backing type.
> > +/// assert_eq!(v + 5, 20);
> > +/// assert_eq!(v / 3, 5);
>
> No, v + 5 == 20 for a different reason. There's nothing about 'backing
> storage' here.
>
> v + 5 should be 20 because addition implies typecasting to the wider
> type. In this case, 20 is numeral, or int, and BitInt(4) + int == int.
>
> I tried C23, and it works exactly like that:
>
> unsigned _BitInt(4) x = 15;
>
> printf("%d\n", x + 5); // 20
> printf("%d\n", x / 3); // 5
> printf("%d\n", x + (unsigned _BitInt(4))5); // 4
> x += 5;
> printf("%d\n", x); // 4
>
> Rust _must_ do the same thing to at least be arithmetically
> compatible to the big brother.
>
> It makes me more confident that this 'backing storage' concept
> brings nothing but confusion.
Rust has no such thing as automatic type casting between different
integer types at all. All that happened here is that BitInt has an
implementation of BitInt<T> + T and defined the output type to be T. As
far as I can tell, that's the only implementation of addition defined
for BitInt.
> > +/// // The backing type can be changed while preserving the number of bits used for representation.
> > +/// assert_eq!(v.cast::<u32>(), BitInt::<u32, 4>::new::<15>());
> > +///
> > +/// // We can safely extend the number of bits...
> > +/// assert_eq!(v.extend::<5>(), BitInt::<u8, 5>::new::<15>());
> > +/// // ... but reducing the number of bits fails here as the value doesn't fit anymore.
> > +/// assert_eq!(v.try_shrink::<3>(), None);
>
> Not sure what for you need this. If I need to 'extend', I just assign
> the value to a variable:
>
> BitInt(3) a = 3;
> BitInt(10) b;
> int c;
>
> b = a + 123; // extend
> c = b; // another extend
Rust has no such things as automatic type casting between different
integer types. Explicit casts are always required - also with normal
integer types.
> How would this 'extend' and 'shrink' work with arrays of BitInts?
We would need a separate method for the array case.
> > +/// // The same applies to signed values.
> > +/// asserkkt_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
> > +///
> > +/// // This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
> > +/// // contained value).
> > +/// // let _ = u8::from(BitInt::<u32, 10>::new::<10>());
>
> If I explicitly typecast from a wider type, please just let me do
> that. In the above examples you show that .is_some() and .is_none()
> can help user to check for overflow if needed.
>
> Otherwise, user will hack your protection by just converting
> BitInt(8) to u32, and then to BitInt(10).
It is Rust convention that u8::from(..) must only compile if the
conversion is lossless. If you want to check overflow instead, you use
the different conversion method that exists for that purpose.
Alice
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 2:17 ` Yury Norov
2025-11-03 10:47 ` Alice Ryhl
@ 2025-11-03 13:42 ` Alexandre Courbot
2025-11-03 13:57 ` Alexandre Courbot
2025-11-03 19:36 ` Yury Norov
2025-11-03 14:10 ` Miguel Ojeda
2 siblings, 2 replies; 22+ messages in thread
From: Alexandre Courbot @ 2025-11-03 13:42 UTC (permalink / raw)
To: Yury Norov, Alexandre Courbot
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
Hi Yury,
On Mon Nov 3, 2025 at 11:17 AM JST, Yury Norov wrote:
> On Fri, Oct 31, 2025 at 10:39:57PM +0900, Alexandre Courbot wrote:
>> Add the `BitInt` type, which is an integer on which the number of bits
>> allowed to be used is restricted, capping its maximal value below that
>> of primitive type is wraps.
>>
>> This is useful to e.g. enforce guarantees when working with bit fields.
>>
>> Alongside this type, provide many `From` and `TryFrom` implementations
>> are 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 | 75 ++++
>> rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 972 insertions(+)
>
> ...
>
>> +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
>> +///
>> +/// Can be used in const context.
>> +macro_rules! fits_within {
>> + ($value:expr, $type:ty, $num_bits:expr) => {{
>> + // Any attempt to create a `BitInt` with more bits used for representation than its backing
>> + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
>> + build_assert!(
>> + <$type>::BITS >= $num_bits,
>> + "Number of bits requested for representation is larger than backing type."
>> + );
>> +
>> + let shift: u32 = <$type>::BITS - $num_bits;
>> + let v = $value;
>> +
>> + // The value fits within `NUM_BITS` if shifting it left by the number of unused bits, then
>> + // right by the same number, doesn't change the value.
>> + //
>> + // This method has the benefit of working with both unsigned and signed integers.
>> + (v << shift) >> shift == v
>
> In C it doesn't work:
>
> int c = 0x7fffffff;
> printf("%x\t%x\n", c, (c << 1) >> 1); // 7fffffff ffffffff
>
> Neither in rust:
>
> let c: i32 = 0x7fffffff;
> println!("{} {}", c, (c << 1) >> 1); // 2147483647 -1
>
> Or I misunderstand something?
For a short while I though this was indeed not working, to my despair as
this looked like an elegant solution.
But then I considered why we are doing that shift by 1 in the first
place: that would be because we are intent on using only 31 of the 32
bits of the `0x7fffffff` value, in order to encode a signed number.
And the positive `0x7fffffff` value cannot be encoded as a signed
integer of 31 bits, as after ignoring the MSB the next bit (which would
be the sign bit) is `1`. So the post-shift value differs from the
original one, and the creation of a `BitInt<i32, 31>` for that value
fails, which is working as intended.
If OTOH we did the same operation for a `BitInt<u32, 31>`, the
non-signed nature of the value would make the shift-and-back operation
produce the same value as the initial one - allowing the creation of the
`BitInt`, which again is correct because `0x7fffffff` can be
represented as an unsigned value using 31 bits.
I have tested both cases successfully - so this way of validating still
looks correct to me.
>
>> + }};
>> +}
>> +
>> +/// Trait for primitive integer types that can be used to back a [`BitInt`].
>> +///
>> +/// This is mostly used to lock all the operations we need for [`BitInt`] in a single trait.
>> +pub trait Boundable
>> +where
>> + Self: Integer
>> + + Sized
>> + + Copy
>> + + core::ops::Shl<u32, Output = Self>
>> + + core::ops::Shr<u32, Output = Self>
>> + + core::cmp::PartialEq,
>> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
>> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
>> +{
>> + /// Returns `true` if `value` can be represented with at most `NUM_BITS` on `T`.
>> + fn fits_within(value: Self, num_bits: u32) -> bool {
>> + fits_within!(value, Self, num_bits)
>> + }
>> +}
>> +
>> +/// Inplement `Boundable` for all integers types.
>> +impl<T> Boundable for T
>> +where
>> + T: Integer
>> + + Sized
>> + + Copy
>> + + core::ops::Shl<u32, Output = Self>
>> + + core::ops::Shr<u32, Output = Self>
>> + + core::cmp::PartialEq,
>> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
>> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
>> +{
>> +}
>> +
>> +/// Integer type for which only the bits `0..NUM_BITS` are valid.
>> +///
>> +/// # Invariants
>> +///
>> +/// Stored values are represented with at most `NUM_BITS` bits.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// use kernel::num::BitInt;
>> +///
>> +/// // An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
>> +/// // The value `15` is statically validated to fit within the specified number of bits.
>> +/// let v = BitInt::<u8, 4>::new::<15>();
>
> What for do you make user to declare the storage explicitly? From
> end-user perspective, declaring 4-bit variable must imply the most
> suitable storage... C version of the same doesn't allow user to select
> the storage as well:
>
> _BitInt(4) x = 8;
>
> I can't think out any useful usecase for this... I believe that the
> optimal storage must be chosen by implementation. And it may even be
> different for different arches.
Alice provided a good justification in her reply, but let me elaborate a
bit more.
C allows itself to implicitly cast values when performing operations.
For instance:
signed char a = -128;
unsigned int b = 120;
unsigned short c = b + a;
printf("%d\n", c);
This absolutely not confusing program is perfectly valid and prints
`65528`.
For the equivalent code in Rust:
let a = -128i8;
let b = 120u32;
let c: u16 = b + a;
I don't even bother printing the result because these 3 lines alone
produce 3 build errors. Rust doesn't let you perform operations on
primitive integers that are not exactly the same type.
So that's the first reason for explicitly passing a type: so you can
perform the operations you want with your `BitInt` without jumping
through cast hoops. This is particularly important to use it with
bitfields, which is the primary goal.
Another reason is that even if you don't care about the size of the
storage, you should care about its signedness, which is part of the
type. I played a bit with C's `_Bitint`, and managed to produce this
wonder:
_BitInt(8) v = -1;
printf("%d\n", v);
This programs prints `255`, even though I used the signed "%d"
formatter? Maybe that's because I should make my `_BitInt` explicitly
signed?
signed _BitInt(8) v = -1;
printf("%d\n", v);
Nope. Still `255`. Go figure. ¯\_(ツ)_/¯
You cannot do that with our implementation. You either specify
let v = BitInt::<i8, 8>::new::<-1>();
and get a proper, signed `-1` value that prints properly, or do
let v = BitInt::<u8, 8>::new::<-1>();
and get the build error you should get. No ambiguity, no surprises.
>
>> +/// assert_eq!(v.get(), 15);
>> +///
>> +/// let v = BitInt::<i8, 4>::new::<-8>();
>> +/// assert_eq!(v.get(), -8);
>> +///
>> +/// // This doesn't build: a `u8` is smaller than the requested 9 bits.
>> +/// // let _ = BitInt::<u8, 9>::new::<10>();
>> +///
>> +/// // This also doesn't build: the requested value doesn't fit within the requested bits.
>> +/// // let _ = BitInt::<i8, 4>::new::<8>();
>> +///
>> +/// // Values can also be validated at runtime. This succeeds because `15` can be represented
>> +/// // with 4 bits.
>> +/// assert!(BitInt::<u8, 4>::try_new(15).is_some());
>> +/// // This fails because `16` cannot be represented with 4 bits.
>> +/// assert!(BitInt::<u8, 4>::try_new(16).is_none());
>
> Nice! Maybe .is_overflow() instead of .is_none(), so that user will
> know that the variable contains truncated value. Just like C does.
>
> Can you please print the exact error that user will get on compile-
> and runtime? How big is the cost of runtime test for overflow? If it
> is indeed nonzero, can you consider making the runtime part
> configurable?
`is_none()` comes from the `Option` type, so we cannot change its name
and it's the idiomatic way to do anyway.
The runtime cost for overflow is the double-shift and comparison with
the initial value.
>
>> +/// // Non-constant expressions can also be validated at build-time thanks to compiler
>> +/// // optimizations. This should be used as a last resort though.
>> +/// let v = BitInt::<u8, 4>::from_expr(15);
>
> Not sure I understand that. Can you confirm my understanding?
>
> 1. For compile-time initialization I use BitInt::<i8, 4>::new::<8>();
> 2. For compile- or runtime initialization: BitInt::<i8, 4>::from_expr(val);
> 3. For runtime-only initialization: BitInt::<i8, 4>::try_new(val);
>
> In this scheme #3 looks excessive...
This example was not very good. The v2 features a better one:
let v = BitInt::<u32, 4>::from_expr(some_number() & 0xf);
Here assume that `some_number()` returns a random value. You cannot use
the static `new` constructor, as the exact value is not statically
known, so the alternative would be to use `try_new` and check for an
overflow error you know cannot happen because the value is masked with
`0xf` and thus will fit the 4 bits.
`from_expr` allows you to create this `BitInt` infallibly, by relying on
the compiler being smart enough to infer from the mask operation that
the value will indeed satify the constraints of the `BitInt`, and throws
a linker error if it couldn't. If the program builds, there is no need
for error checking and no runtime validation.
>
>> +/// // Common operations are supported against the backing type.
>> +/// assert_eq!(v + 5, 20);
>> +/// assert_eq!(v / 3, 5);
>
> No, v + 5 == 20 for a different reason. There's nothing about 'backing
> storage' here.
>
> v + 5 should be 20 because addition implies typecasting to the wider
> type. In this case, 20 is numeral, or int, and BitInt(4) + int == int.
>
> I tried C23, and it works exactly like that:
>
> unsigned _BitInt(4) x = 15;
>
> printf("%d\n", x + 5); // 20
> printf("%d\n", x / 3); // 5
> printf("%d\n", x + (unsigned _BitInt(4))5); // 4
> x += 5;
> printf("%d\n", x); // 4
>
> Rust _must_ do the same thing to at least be arithmetically
> compatible to the big brother.
Rust doesn't do implicit casts. I do agree that "backing storage" is not
the best choice of words though.
>
> It makes me more confident that this 'backing storage' concept
> brings nothing but confusion.
>
>> +/// // The backing type can be changed while preserving the number of bits used for representation.
>> +/// assert_eq!(v.cast::<u32>(), BitInt::<u32, 4>::new::<15>());
>> +///
>> +/// // We can safely extend the number of bits...
>> +/// assert_eq!(v.extend::<5>(), BitInt::<u8, 5>::new::<15>());
>> +/// // ... but reducing the number of bits fails here as the value doesn't fit anymore.
>> +/// assert_eq!(v.try_shrink::<3>(), None);
>
> Not sure what for you need this. If I need to 'extend', I just assign
> the value to a variable:
>
> BitInt(3) a = 3;
> BitInt(10) b;
> int c;
>
> b = a + 123; // extend
> c = b; // another extend
>
> How would this 'extend' and 'shrink' work with arrays of BitInts?
I think this discussion would be more productive if we don't rely on
examples in a language that is irrelevant for the current patch. :)
Rust does not allow overloading the `=` operator, so these implicit
conversions from one type to another cannot be performed. Having
dedicated methods is an idiomatic way to do this according to my
experience - an alternative would be to have `From` implementations, but
doing this elegantly would require one language feature (generic const
expressions) that is still not stable.
>
>> +/// // Conversion into primitive types is dependent on the number of useful bits, not the backing
>> +/// // type.
>> +/// //
>> +/// // Event though its backing type is `u32`, this `BitInt` only uses 8 bits and thus can safely
>> +/// // be converted to a `u8`.
>> +/// assert_eq!(u8::from(BitInt::<u32, 8>::new::<128>()), 128u8);
>
> 'Backing type' is useless here too.
>
>> +/// // The same applies to signed values.
>> +/// asserkkt_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
>> +///
>> +/// // This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
>> +/// // contained value).
>> +/// // let _ = u8::from(BitInt::<u32, 10>::new::<10>());
>
> If I explicitly typecast from a wider type, please just let me do
> that. In the above examples you show that .is_some() and .is_none()
> can help user to check for overflow if needed.
>
> Otherwise, user will hack your protection by just converting
> BitInt(8) to u32, and then to BitInt(10).
Doing so would require validating that the value in the `u32` can fit
within the 10-bit `BitInt` one way or the other, so the protection
cannot be bypassed that way.
After comparing this implementation with C's `_BitInt`, I have also come
to a more fundamental divergence between the two.
The C `_BitInt` is used to express numbers with an arbitrary number of
bits - which could be less than a primitive type, but also *more* - for
instance a `_BitInt(4094)` is a valid thing!
Which is really cool, but also not something we need or want in the
kernel. Our purposes here is strictly to limit the width of existing
primitive types to provide extra guarantees about the code. And even if
we wanted to mimic the C `_BitInt`, we simply couldn't without compiler
support as literal values larger than a primitive type cannot even be
expressed.
So although I liked the `BitInt` name, that makes it quite a bit
misleading for our type as users could think that they will have an
equivalent to the C namesake, while the purpose and use is different.
The original `BoundedInt` name was a more accurate fit IMHO, but I hope
we can find something shorter.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 13:42 ` Alexandre Courbot
@ 2025-11-03 13:57 ` Alexandre Courbot
2025-11-03 14:01 ` Danilo Krummrich
2025-11-03 19:36 ` Yury Norov
1 sibling, 1 reply; 22+ messages in thread
From: Alexandre Courbot @ 2025-11-03 13:57 UTC (permalink / raw)
To: Alexandre Courbot, Yury Norov
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Mon Nov 3, 2025 at 10:42 PM JST, Alexandre Courbot wrote:
<snip>
> After comparing this implementation with C's `_BitInt`, I have also come
> to a more fundamental divergence between the two.
>
> The C `_BitInt` is used to express numbers with an arbitrary number of
> bits - which could be less than a primitive type, but also *more* - for
> instance a `_BitInt(4094)` is a valid thing!
>
> Which is really cool, but also not something we need or want in the
> kernel. Our purposes here is strictly to limit the width of existing
> primitive types to provide extra guarantees about the code. And even if
> we wanted to mimic the C `_BitInt`, we simply couldn't without compiler
> support as literal values larger than a primitive type cannot even be
> expressed.
>
> So although I liked the `BitInt` name, that makes it quite a bit
> misleading for our type as users could think that they will have an
> equivalent to the C namesake, while the purpose and use is different.
>
> The original `BoundedInt` name was a more accurate fit IMHO, but I hope
> we can find something shorter.
Actually - the core library names similar wrapping types `NonZero` or
`Wrapping` - not `NonZeroInt` or `WrappingInt`. So this type could just
be called `Bounded`, as its generic parameter makes it clear what it
sets the bounds of anyway.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 13:57 ` Alexandre Courbot
@ 2025-11-03 14:01 ` Danilo Krummrich
0 siblings, 0 replies; 22+ messages in thread
From: Danilo Krummrich @ 2025-11-03 14:01 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Yury Norov, Alice Ryhl, Miguel Ojeda, Joel Fernandes, Jesung Yang,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, linux-kernel, rust-for-linux
On 11/3/25 2:57 PM, Alexandre Courbot wrote:
> Actually - the core library names similar wrapping types `NonZero` or
> `Wrapping` - not `NonZeroInt` or `WrappingInt`. So this type could just
> be called `Bounded`, as its generic parameter makes it clear what it
> sets the bounds of anyway.
+1
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 2:17 ` Yury Norov
2025-11-03 10:47 ` Alice Ryhl
2025-11-03 13:42 ` Alexandre Courbot
@ 2025-11-03 14:10 ` Miguel Ojeda
2025-11-03 14:26 ` Yury Norov
2 siblings, 1 reply; 22+ messages in thread
From: Miguel Ojeda @ 2025-11-03 14:10 UTC (permalink / raw)
To: Yury Norov
Cc: Alexandre Courbot, Alice Ryhl, Danilo Krummrich, Miguel Ojeda,
Joel Fernandes, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, linux-kernel, rust-for-linux
On Mon, Nov 3, 2025 at 3:17 AM Yury Norov <yury.norov@gmail.com> wrote:
>
> Rust _must_ do the same thing to at least be arithmetically
> compatible to the big brother.
No, Rust doesn't need to do anything of the sort, sorry.
The point here is not to copy what C does, but to improve on it.
In particular, we definitely do not want to have anything like integer
promotions and arithmetic conversions from C.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 14:10 ` Miguel Ojeda
@ 2025-11-03 14:26 ` Yury Norov
2025-11-03 14:44 ` Alexandre Courbot
2025-11-03 14:54 ` Miguel Ojeda
0 siblings, 2 replies; 22+ messages in thread
From: Yury Norov @ 2025-11-03 14:26 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Alexandre Courbot, Alice Ryhl, Danilo Krummrich, Miguel Ojeda,
Joel Fernandes, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, linux-kernel, rust-for-linux
On Mon, Nov 03, 2025 at 03:10:01PM +0100, Miguel Ojeda wrote:
> On Mon, Nov 3, 2025 at 3:17 AM Yury Norov <yury.norov@gmail.com> wrote:
> >
> > Rust _must_ do the same thing to at least be arithmetically
> > compatible to the big brother.
>
> No, Rust doesn't need to do anything of the sort, sorry.
>
> The point here is not to copy what C does, but to improve on it.
>
> In particular, we definitely do not want to have anything like integer
> promotions and arithmetic conversions from C.
This is exactly what the patch does:
+/// let v = BitInt::<u8, 4>::from_expr(15);
+///
+/// // Common operations are supported against the backing type.
+/// assert_eq!(v + 5, 20);
+/// assert_eq!(v / 3, 5);
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 10:47 ` Alice Ryhl
@ 2025-11-03 14:31 ` Alexandre Courbot
0 siblings, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2025-11-03 14:31 UTC (permalink / raw)
To: Alice Ryhl, Yury Norov
Cc: Alexandre Courbot, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
Hi Alice,
On Mon Nov 3, 2025 at 7:47 PM JST, Alice Ryhl wrote:
> On Sun, Nov 02, 2025 at 09:17:35PM -0500, Yury Norov wrote:
>> On Fri, Oct 31, 2025 at 10:39:57PM +0900, Alexandre Courbot wrote:
>> > Add the `BitInt` type, which is an integer on which the number of bits
>> > allowed to be used is restricted, capping its maximal value below that
>> > of primitive type is wraps.
>> >
>> > This is useful to e.g. enforce guarantees when working with bit fields.
>> >
>> > Alongside this type, provide many `From` and `TryFrom` implementations
>> > are 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 | 75 ++++
>> > rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 972 insertions(+)
>>
>> ...
>>
>> > +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
>> > +///
>> > +/// Can be used in const context.
>> > +macro_rules! fits_within {
>> > + ($value:expr, $type:ty, $num_bits:expr) => {{
>> > + // Any attempt to create a `BitInt` with more bits used for representation than its backing
>> > + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
>> > + build_assert!(
>> > + <$type>::BITS >= $num_bits,
>> > + "Number of bits requested for representation is larger than backing type."
>> > + );
>> > +
>> > + let shift: u32 = <$type>::BITS - $num_bits;
>> > + let v = $value;
>> > +
>> > + // The value fits within `NUM_BITS` if shifting it left by the number of unused bits, then
>> > + // right by the same number, doesn't change the value.
>> > + //
>> > + // This method has the benefit of working with both unsigned and signed integers.
>> > + (v << shift) >> shift == v
>>
>> In C it doesn't work:
>>
>> int c = 0x7fffffff;
>> printf("%x\t%x\n", c, (c << 1) >> 1); // 7fffffff ffffffff
>>
>> Neither in rust:
>>
>> let c: i32 = 0x7fffffff;
>> println!("{} {}", c, (c << 1) >> 1); // 2147483647 -1
>>
>> Or I misunderstand something?
>
> I think we should probably just define a BitInt::MIN and BitInt::MAX
> constant and have this code check MIN <= v && v <= MAX.
Thankfully (and unless I am mistaken) it looks like this is actually
working as intended - please see my reply to Yury if you want to
confirm.
>
>> > + }};
>> > +}
>> > +
>> > +/// Trait for primitive integer types that can be used to back a [`BitInt`].
>> > +///
>> > +/// This is mostly used to lock all the operations we need for [`BitInt`] in a single trait.
>> > +pub trait Boundable
>> > +where
>> > + Self: Integer
>> > + + Sized
>> > + + Copy
>> > + + core::ops::Shl<u32, Output = Self>
>> > + + core::ops::Shr<u32, Output = Self>
>> > + + core::cmp::PartialEq,
>> > + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
>> > + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
>> > +{
>> > + /// Returns `true` if `value` can be represented with at most `NUM_BITS` on `T`.
>> > + fn fits_within(value: Self, num_bits: u32) -> bool {
>> > + fits_within!(value, Self, num_bits)
>> > + }
>> > +}
>> > +
>> > +/// Inplement `Boundable` for all integers types.
>> > +impl<T> Boundable for T
>> > +where
>> > + T: Integer
>> > + + Sized
>> > + + Copy
>> > + + core::ops::Shl<u32, Output = Self>
>> > + + core::ops::Shr<u32, Output = Self>
>> > + + core::cmp::PartialEq,
>> > + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
>> > + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
>> > +{
>> > +}
>> > +
>> > +/// Integer type for which only the bits `0..NUM_BITS` are valid.
>> > +///
>> > +/// # Invariants
>> > +///
>> > +/// Stored values are represented with at most `NUM_BITS` bits.
>> > +///
>> > +/// # Examples
>> > +///
>> > +/// ```
>> > +/// use kernel::num::BitInt;
>> > +///
>> > +/// // An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
>> > +/// // The value `15` is statically validated to fit within the specified number of bits.
>> > +/// let v = BitInt::<u8, 4>::new::<15>();
>>
>> What for do you make user to declare the storage explicitly? From
>> end-user perspective, declaring 4-bit variable must imply the most
>> suitable storage... C version of the same doesn't allow user to select
>> the storage as well:
>>
>> _BitInt(4) x = 8;
>>
>> I can't think out any useful usecase for this... I believe that the
>> optimal storage must be chosen by implementation. And it may even be
>> different for different arches.
>
> It's more complex to not specify the backing storage explicitly, but I
> also think it would be nice to be able to avoid it.
Indeed, it's not impossible to automatically assign a primitive type
from the number of requested bits, but it's a bit of a PITA without
generic constant expressions. And I don't think we can avoid explicitly
mentioning the signedness anyway, so why not give the full type.
Finally, this would also make arithmetic operations (or anything using
the `BitInt`'s value) more complex as one would need to cast to the type
they actually need to perform anything useful.
<snip>
>> > +/// // Non-constant expressions can also be validated at build-time thanks to compiler
>> > +/// // optimizations. This should be used as a last resort though.
>> > +/// let v = BitInt::<u8, 4>::from_expr(15);
>>
>> Not sure I understand that. Can you confirm my understanding?
>>
>> 1. For compile-time initialization I use BitInt::<i8, 4>::new::<8>();
>> 2. For compile- or runtime initialization: BitInt::<i8, 4>::from_expr(val);
>> 3. For runtime-only initialization: BitInt::<i8, 4>::try_new(val);
>>
>> In this scheme #3 looks excessive...
>
> I'm not sure we should have `from_expr` at all. What it does is emit an
> if condition checking whether the value is in-bounds containing a call
> to a function that does not exist, so that if the optimizer prove the
> range check at compile-time, then there is a linker error.
>
> What is the use-case for from_expr?
It is pretty fundamental to using this successfully with bitfields. For
instance, the getter method for a given field (which returns a `BitInt`)
works roughly as follows:
// Extract the field using its mask and shift.
let field = ((self.0 & MASK) >> SHIFT);
// Infallibly create the `BitInt` from `field` - the compiler can
// infer from the mask and shift above that the overflow check
// will never fail.
BitInt::<$storage, { $hi + 1 - $lo }>::from_expr(field)
Without this, we should need to resort to unsafe code and probably an
`unwrap_unchecked`.
Another example from the Nova code:
const FLUSH_SYSMEM_ADDR_SHIFT_HI: u32 = 40;
pub(super) fn write_sysmem_flush_page_ga100(bar: &Bar0, addr: u64) {
regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI::default()
.set_adr_63_40(BitInt::from_expr(addr >> FLUSH_SYSMEM_ADDR_SHIFT_HI).cast())
.write(bar);
}
The `adr_63_40` field is 24-bits long, and from the 40-bit shift
operation `from_expr` can infer that the result will fit and do without
a runtime check. I think that's pretty cool! :)
Also, this does not look very different from what e.g.
`Io::io_addr_assert` does.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 14:26 ` Yury Norov
@ 2025-11-03 14:44 ` Alexandre Courbot
2025-11-03 14:54 ` Miguel Ojeda
1 sibling, 0 replies; 22+ messages in thread
From: Alexandre Courbot @ 2025-11-03 14:44 UTC (permalink / raw)
To: Yury Norov, Miguel Ojeda
Cc: Alexandre Courbot, Alice Ryhl, Danilo Krummrich, Miguel Ojeda,
Joel Fernandes, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, linux-kernel, rust-for-linux
On Mon Nov 3, 2025 at 11:26 PM JST, Yury Norov wrote:
> On Mon, Nov 03, 2025 at 03:10:01PM +0100, Miguel Ojeda wrote:
>> On Mon, Nov 3, 2025 at 3:17 AM Yury Norov <yury.norov@gmail.com> wrote:
>> >
>> > Rust _must_ do the same thing to at least be arithmetically
>> > compatible to the big brother.
>>
>> No, Rust doesn't need to do anything of the sort, sorry.
>>
>> The point here is not to copy what C does, but to improve on it.
>>
>> In particular, we definitely do not want to have anything like integer
>> promotions and arithmetic conversions from C.
>
> This is exactly what the patch does:
>
> +/// let v = BitInt::<u8, 4>::from_expr(15);
> +///
> +/// // Common operations are supported against the backing type.
> +/// assert_eq!(v + 5, 20);
> +/// assert_eq!(v / 3, 5);
That's incorrect. `v` is backed by a u8, thus the RHS `5` is also a u8
(and so is the result, `20`).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 14:26 ` Yury Norov
2025-11-03 14:44 ` Alexandre Courbot
@ 2025-11-03 14:54 ` Miguel Ojeda
2025-11-03 19:43 ` Yury Norov
1 sibling, 1 reply; 22+ messages in thread
From: Miguel Ojeda @ 2025-11-03 14:54 UTC (permalink / raw)
To: Yury Norov
Cc: Alexandre Courbot, Alice Ryhl, Danilo Krummrich, Miguel Ojeda,
Joel Fernandes, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, linux-kernel, rust-for-linux
On Mon, Nov 3, 2025 at 3:26 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> This is exactly what the patch does:
No, there are no arithmetic conversions going on here in the sense of
C. It defines a particular operation for a set of types.
What you are seeing there is that literals, in Rust, do type
inference, and so the compiler picks a type:
https://doc.rust-lang.org/reference/expressions/literal-expr.html#r-expr.literal.int.infer
Thus if you do:
let v1 = BitInt::<u8, 4>::from_expr(15);
let v2 = BitInt::<u16, 4>::from_expr(15);
let i = 5;
assert_eq!(v1 + i, 20);
assert_eq!(v2 + i, 20);
That will not build, because `i` cannot have two types. But it will if
you comment one of the two asserts.
And if you do:
let v = BitInt::<u16, 4>::from_expr(15);
assert_eq!(v + 5u8, 20);
It will not build either -- there is not even "widening" going on from
`u8` to `u16` in this last example.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 13:42 ` Alexandre Courbot
2025-11-03 13:57 ` Alexandre Courbot
@ 2025-11-03 19:36 ` Yury Norov
2025-11-04 3:13 ` Alexandre Courbot
1 sibling, 1 reply; 22+ messages in thread
From: Yury Norov @ 2025-11-03 19:36 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Mon, Nov 03, 2025 at 10:42:13PM +0900, Alexandre Courbot wrote:
> Hi Yury,
>
> On Mon Nov 3, 2025 at 11:17 AM JST, Yury Norov wrote:
> > On Fri, Oct 31, 2025 at 10:39:57PM +0900, Alexandre Courbot wrote:
> >> Add the `BitInt` type, which is an integer on which the number of bits
> >> allowed to be used is restricted, capping its maximal value below that
> >> of primitive type is wraps.
> >>
> >> This is useful to e.g. enforce guarantees when working with bit fields.
> >>
> >> Alongside this type, provide many `From` and `TryFrom` implementations
> >> are 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 | 75 ++++
> >> rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 972 insertions(+)
> >
> > ...
> >
> >> +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
> >> +///
> >> +/// Can be used in const context.
> >> +macro_rules! fits_within {
> >> + ($value:expr, $type:ty, $num_bits:expr) => {{
> >> + // Any attempt to create a `BitInt` with more bits used for representation than its backing
> >> + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
> >> + build_assert!(
> >> + <$type>::BITS >= $num_bits,
> >> + "Number of bits requested for representation is larger than backing type."
> >> + );
> >> +
> >> + let shift: u32 = <$type>::BITS - $num_bits;
> >> + let v = $value;
> >> +
> >> + // The value fits within `NUM_BITS` if shifting it left by the number of unused bits, then
> >> + // right by the same number, doesn't change the value.
> >> + //
> >> + // This method has the benefit of working with both unsigned and signed integers.
> >> + (v << shift) >> shift == v
> >
> > In C it doesn't work:
> >
> > int c = 0x7fffffff;
> > printf("%x\t%x\n", c, (c << 1) >> 1); // 7fffffff ffffffff
> >
> > Neither in rust:
> >
> > let c: i32 = 0x7fffffff;
> > println!("{} {}", c, (c << 1) >> 1); // 2147483647 -1
> >
> > Or I misunderstand something?
>
> For a short while I though this was indeed not working, to my despair as
> this looked like an elegant solution.
No it does not. One suggested by Alice is better because it's more
straightforward.
> But then I considered why we are doing that shift by 1 in the first
> place: that would be because we are intent on using only 31 of the 32
> bits of the `0x7fffffff` value, in order to encode a signed number.
>
> And the positive `0x7fffffff` value cannot be encoded as a signed
> integer of 31 bits, as after ignoring the MSB the next bit (which would
> be the sign bit) is `1`. So the post-shift value differs from the
> original one, and the creation of a `BitInt<i32, 31>` for that value
> fails, which is working as intended.
>
> If OTOH we did the same operation for a `BitInt<u32, 31>`, the
> non-signed nature of the value would make the shift-and-back operation
> produce the same value as the initial one - allowing the creation of the
> `BitInt`, which again is correct because `0x7fffffff` can be
> represented as an unsigned value using 31 bits.
>
> I have tested both cases successfully - so this way of validating still
> looks correct to me.
I read this carefully more than once, but still can't understand. So
I gave your macro a try:
macro_rules! fits_within {
($value:expr, $BITS:expr, $num_bits:expr) => {{
let shift: u32 = $BITS - $num_bits;
let v = $value;
(v << shift) >> shift == v
}};
}
fn main() {
let a: i32 = -1;
let b: i32 = 0x7fffffff;
println!("{}", fits_within!(a, 32, 31)); // true
println!("{}", fits_within!(b, 32, 31)); // false
}
This is exactly opposite to what I naively expect: 32-bit '-1'
shouldn't fit into 31-bit storage, but 31-bit 0x7fffffff should.
And said aloud, my naive expectation is: fits_withing() must
return true iff typecasting from one type to another followed
by typecasting back to the original type would not change the
value.
Nevermind. Let's consider a more practical example: I read some
12-bit ADC with a C function:
int read_my_adc();
It gives me either some 12-bit value, or errno, like -EAGAIN. Let's
assume my ADC is differential, and bit 11 encodes sign of the output.
Now, I want to put it into a 12-bit variable. So:
let val = read_my_adc(); // -EAGAIN
fits_within!(val, i32, 12);
returns true. This is surely not what one would expect.
This is one more example:
let a: i64 = -1i64;
println!("{}", fits_within!(a, 32, 31));
Here you exceed even the 'backing storage', but still return OK.
> >> + }};
> >> +}
> >> +
> >> +/// Trait for primitive integer types that can be used to back a [`BitInt`].
> >> +///
> >> +/// This is mostly used to lock all the operations we need for [`BitInt`] in a single trait.
> >> +pub trait Boundable
> >> +where
> >> + Self: Integer
> >> + + Sized
> >> + + Copy
> >> + + core::ops::Shl<u32, Output = Self>
> >> + + core::ops::Shr<u32, Output = Self>
> >> + + core::cmp::PartialEq,
> >> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
> >> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
> >> +{
> >> + /// Returns `true` if `value` can be represented with at most `NUM_BITS` on `T`.
> >> + fn fits_within(value: Self, num_bits: u32) -> bool {
> >> + fits_within!(value, Self, num_bits)
> >> + }
> >> +}
> >> +
> >> +/// Inplement `Boundable` for all integers types.
> >> +impl<T> Boundable for T
> >> +where
> >> + T: Integer
> >> + + Sized
> >> + + Copy
> >> + + core::ops::Shl<u32, Output = Self>
> >> + + core::ops::Shr<u32, Output = Self>
> >> + + core::cmp::PartialEq,
> >> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
> >> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
> >> +{
> >> +}
> >> +
> >> +/// Integer type for which only the bits `0..NUM_BITS` are valid.
> >> +///
> >> +/// # Invariants
> >> +///
> >> +/// Stored values are represented with at most `NUM_BITS` bits.
> >> +///
> >> +/// # Examples
> >> +///
> >> +/// ```
> >> +/// use kernel::num::BitInt;
> >> +///
> >> +/// // An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
> >> +/// // The value `15` is statically validated to fit within the specified number of bits.
> >> +/// let v = BitInt::<u8, 4>::new::<15>();
> >
> > What for do you make user to declare the storage explicitly? From
> > end-user perspective, declaring 4-bit variable must imply the most
> > suitable storage... C version of the same doesn't allow user to select
> > the storage as well:
> >
> > _BitInt(4) x = 8;
> >
> > I can't think out any useful usecase for this... I believe that the
> > optimal storage must be chosen by implementation. And it may even be
> > different for different arches.
>
> Alice provided a good justification in her reply, but let me elaborate a
> bit more.
This is what Alice said:
It's more complex to not specify the backing storage explicitly, but I
also think it would be nice to be able to avoid it.
I agree with her. But I don't see any justification here for the
proposed approach.
TBH, I think it's not too complicated to realize that the best
backing storage for an N-bit value is 2^(ceil(log(N)))-bit type,
unless arch specifics comes in play.
I also think that you can bake first version based on 64-bit backing
storage, regardless of number of bits, so tailoring BitInt() would
be a future improvement.
> C allows itself to implicitly cast values when performing operations.
> For instance:
>
> signed char a = -128;
> unsigned int b = 120;
> unsigned short c = b + a;
>
> printf("%d\n", c);
>
> This absolutely not confusing program is perfectly valid and prints
> `65528`.
>
> For the equivalent code in Rust:
>
> let a = -128i8;
> let b = 120u32;
> let c: u16 = b + a;
>
> I don't even bother printing the result because these 3 lines alone
> produce 3 build errors. Rust doesn't let you perform operations on
> primitive integers that are not exactly the same type.
>
> So that's the first reason for explicitly passing a type: so you can
> perform the operations you want with your `BitInt` without jumping
> through cast hoops. This is particularly important to use it with
> bitfields, which is the primary goal.
OK, that's really a Rust innovation. So, if we follow the rule, I have
two and a half considerations:
1. It also means that BitInt(u8, 4) and BitInt(u8, 5) should be the
different types, and you shouldn't be able to do arithmetics with them
even if the backing types match. Can you confirm that?
2. Later in your patch you allow this:
let v = BitInt::<u32, 4>::new::<15>();
assert_eq!(v * 10, 150);
This one is really misleading. You allow me to multiply 4-bit number
by 10, which looks really C-like, and produce an extended result, just
like C does. But from what you say here, and Miguel in another email,
it breaks the spirit of Rust types safety.
2a. Here you allow to do:
let v = BitInt::<i32, 4>::new::<15>();
let n = -EINVAL;
assert_eq!(v + n, -7);
And this is not OK. To follow Rust way, you need to convert n to
BitInt(4), and then it would work as expected - making sure it's
impossible and throwing an overflow.
> Another reason is that even if you don't care about the size of the
> storage, you should care about its signedness, which is part of the
> type.
Just invent BitUint or unsigned BitInt, or teach the macro to
understand BitInt(i12) notation.
> I played a bit with C's `_Bitint`, and managed to produce this
> wonder:
>
> _BitInt(8) v = -1;
> printf("%d\n", v);
>
> This programs prints `255`, even though I used the signed "%d"
> formatter? Maybe that's because I should make my `_BitInt` explicitly
> signed?
>
> signed _BitInt(8) v = -1;
> printf("%d\n", v);
>
> Nope. Still `255`. Go figure. ¯\_(ツ)_/¯
Clang throws Wformat on this; and it is printf() issue, not the
_BitInt()s math bug.
> You cannot do that with our implementation. You either specify
>
> let v = BitInt::<i8, 8>::new::<-1>();
>
> and get a proper, signed `-1` value that prints properly, or do
>
> let v = BitInt::<u8, 8>::new::<-1>();
>
> and get the build error you should get. No ambiguity, no surprises.
>
> >
> >> +/// assert_eq!(v.get(), 15);
> >> +///
> >> +/// let v = BitInt::<i8, 4>::new::<-8>();
> >> +/// assert_eq!(v.get(), -8);
> >> +///
> >> +/// // This doesn't build: a `u8` is smaller than the requested 9 bits.
> >> +/// // let _ = BitInt::<u8, 9>::new::<10>();
> >> +///
> >> +/// // This also doesn't build: the requested value doesn't fit within the requested bits.
> >> +/// // let _ = BitInt::<i8, 4>::new::<8>();
> >> +///
> >> +/// // Values can also be validated at runtime. This succeeds because `15` can be represented
> >> +/// // with 4 bits.
> >> +/// assert!(BitInt::<u8, 4>::try_new(15).is_some());
> >> +/// // This fails because `16` cannot be represented with 4 bits.
> >> +/// assert!(BitInt::<u8, 4>::try_new(16).is_none());
> >
> > Nice! Maybe .is_overflow() instead of .is_none(), so that user will
> > know that the variable contains truncated value. Just like C does.
> >
> > Can you please print the exact error that user will get on compile-
> > and runtime? How big is the cost of runtime test for overflow? If it
> > is indeed nonzero, can you consider making the runtime part
> > configurable?
>
> `is_none()` comes from the `Option` type, so we cannot change its name
> and it's the idiomatic way to do anyway.
I understand. I suggested to implement this .is_overflow(), and have
.is_none() reserved for cases like assigning floating values to
unsigned type.
> The runtime cost for overflow is the double-shift and comparison with
> the initial value.
Are you sure that everyone is OK to pay this runtime cost, especially
after passing debug phase?
> >> +/// // Non-constant expressions can also be validated at build-time thanks to compiler
> >> +/// // optimizations. This should be used as a last resort though.
> >> +/// let v = BitInt::<u8, 4>::from_expr(15);
> >
> > Not sure I understand that. Can you confirm my understanding?
> >
> > 1. For compile-time initialization I use BitInt::<i8, 4>::new::<8>();
> > 2. For compile- or runtime initialization: BitInt::<i8, 4>::from_expr(val);
> > 3. For runtime-only initialization: BitInt::<i8, 4>::try_new(val);
> >
> > In this scheme #3 looks excessive...
>
> This example was not very good. The v2 features a better one:
>
> let v = BitInt::<u32, 4>::from_expr(some_number() & 0xf);
>
> Here assume that `some_number()` returns a random value. You cannot use
> the static `new` constructor, as the exact value is not statically
> known, so the alternative would be to use `try_new` and check for an
> overflow error you know cannot happen because the value is masked with
> `0xf` and thus will fit the 4 bits.
>
> `from_expr` allows you to create this `BitInt` infallibly, by relying on
> the compiler being smart enough to infer from the mask operation that
> the value will indeed satify the constraints of the `BitInt`, and throws
> a linker error if it couldn't. If the program builds, there is no need
> for error checking and no runtime validation.
So from_expr() only allows statically true expressions, like this one
some_number() & 0xf < 16.
And try_new() allows true runtime fallible ones.
Looks like OK... But this ::new<8>() syntax really looks overloaded.
> >> +/// // Common operations are supported against the backing type.
> >> +/// assert_eq!(v + 5, 20);
> >> +/// assert_eq!(v / 3, 5);
> >
> > No, v + 5 == 20 for a different reason. There's nothing about 'backing
> > storage' here.
> >
> > v + 5 should be 20 because addition implies typecasting to the wider
> > type. In this case, 20 is numeral, or int, and BitInt(4) + int == int.
> >
> > I tried C23, and it works exactly like that:
> >
> > unsigned _BitInt(4) x = 15;
> >
> > printf("%d\n", x + 5); // 20
> > printf("%d\n", x / 3); // 5
> > printf("%d\n", x + (unsigned _BitInt(4))5); // 4
> > x += 5;
> > printf("%d\n", x); // 4
> >
> > Rust _must_ do the same thing to at least be arithmetically
> > compatible to the big brother.
>
> Rust doesn't do implicit casts. I do agree that "backing storage" is not
> the best choice of words though.
In fact, in "assert_eq!(v + 5, 20)" the '5' is implicitly typecasted
to the backing storage type.
In other words, Rust doesn't prohibit typecasting. It just typecasts
to the most restricted type, instead than most relaxed one.
> > It makes me more confident that this 'backing storage' concept
> > brings nothing but confusion.
> >
> >> +/// // The backing type can be changed while preserving the number of bits used for representation.
> >> +/// assert_eq!(v.cast::<u32>(), BitInt::<u32, 4>::new::<15>());
> >> +///
> >> +/// // We can safely extend the number of bits...
> >> +/// assert_eq!(v.extend::<5>(), BitInt::<u8, 5>::new::<15>());
> >> +/// // ... but reducing the number of bits fails here as the value doesn't fit anymore.
> >> +/// assert_eq!(v.try_shrink::<3>(), None);
> >
> > Not sure what for you need this. If I need to 'extend', I just assign
> > the value to a variable:
> >
> > BitInt(3) a = 3;
> > BitInt(10) b;
> > int c;
> >
> > b = a + 123; // extend
> > c = b; // another extend
> >
> > How would this 'extend' and 'shrink' work with arrays of BitInts?
>
> I think this discussion would be more productive if we don't rely on
> examples in a language that is irrelevant for the current patch. :)
The language I rely on is called 'pseudo language', and it is perfectly
OK as soon as it helps me to describe my intention. If you want me to
start thinking in Rust, please make it more friendly. The following is
really a nightmare:
let a = BitInt::<u8, 3>::new::<3>();
let b = BitInt::<u16, 10>::new::<123>() + BitInt::<u16, 10>::try_from(a);
let c = BitInt::<u32, 32>::try_from(a) + BitInt::<u32, 32>::try_from(b);
> Rust does not allow overloading the `=` operator, so these implicit
> conversions from one type to another cannot be performed. Having
> dedicated methods is an idiomatic way to do this according to my
> experience - an alternative would be to have `From` implementations, but
> doing this elegantly would require one language feature (generic const
> expressions) that is still not stable.
>
> >
> >> +/// // Conversion into primitive types is dependent on the number of useful bits, not the backing
> >> +/// // type.
> >> +/// //
> >> +/// // Event though its backing type is `u32`, this `BitInt` only uses 8 bits and thus can safely
> >> +/// // be converted to a `u8`.
> >> +/// assert_eq!(u8::from(BitInt::<u32, 8>::new::<128>()), 128u8);
> >
> > 'Backing type' is useless here too.
> >
> >> +/// // The same applies to signed values.
> >> +/// asserkkt_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
> >> +///
> >> +/// // This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
> >> +/// // contained value).
> >> +/// // let _ = u8::from(BitInt::<u32, 10>::new::<10>());
> >
> > If I explicitly typecast from a wider type, please just let me do
> > that. In the above examples you show that .is_some() and .is_none()
> > can help user to check for overflow if needed.
> >
> > Otherwise, user will hack your protection by just converting
> > BitInt(8) to u32, and then to BitInt(10).
>
> Doing so would require validating that the value in the `u32` can fit
> within the 10-bit `BitInt` one way or the other, so the protection
> cannot be bypassed that way.
That's what I meant. You allow one way to initialize data, but
disallow another equivalent and more straightforward way.
> After comparing this implementation with C's `_BitInt`, I have also come
> to a more fundamental divergence between the two.
>
> The C `_BitInt` is used to express numbers with an arbitrary number of
> bits - which could be less than a primitive type, but also *more* - for
> instance a `_BitInt(4094)` is a valid thing!
>
> Which is really cool, but also not something we need or want in the
> kernel. Our purposes here is strictly to limit the width of existing
> primitive types to provide extra guarantees about the code. And even if
> we wanted to mimic the C `_BitInt`, we simply couldn't without compiler
> support as literal values larger than a primitive type cannot even be
> expressed.
>
> So although I liked the `BitInt` name, that makes it quite a bit
> misleading for our type as users could think that they will have an
> equivalent to the C namesake, while the purpose and use is different.
Great idea by the way. For the next iteration, can you please start with
a Documentation patch that discusses all that things.
What is the purpose and use of rust BitInts?
What is the difference with C, other than 64-bit limitation?
In my understanding, BitInt is purposed to be the fundamental type.
OK, you say we can't override the '=', and have to use the new() and
from() for initialization. But with that in mind, in every other
aspect BitInt(u32) should behave like u32, and BitInt(i12) should be i12.
C is really close to it.
Thanks,
Yury
> The original `BoundedInt` name was a more accurate fit IMHO, but I hope
> we can find something shorter.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 14:54 ` Miguel Ojeda
@ 2025-11-03 19:43 ` Yury Norov
2025-11-03 20:00 ` Yury Norov
0 siblings, 1 reply; 22+ messages in thread
From: Yury Norov @ 2025-11-03 19:43 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Alexandre Courbot, Alice Ryhl, Danilo Krummrich, Miguel Ojeda,
Joel Fernandes, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, linux-kernel, rust-for-linux
On Mon, Nov 03, 2025 at 03:54:08PM +0100, Miguel Ojeda wrote:
> On Mon, Nov 3, 2025 at 3:26 PM Yury Norov <yury.norov@gmail.com> wrote:
> >
> > This is exactly what the patch does:
>
> No, there are no arithmetic conversions going on here in the sense of
> C. It defines a particular operation for a set of types.
>
> What you are seeing there is that literals, in Rust, do type
> inference, and so the compiler picks a type:
>
> https://doc.rust-lang.org/reference/expressions/literal-expr.html#r-expr.literal.int.infer
>
> Thus if you do:
>
> let v1 = BitInt::<u8, 4>::from_expr(15);
> let v2 = BitInt::<u16, 4>::from_expr(15);
> let i = 5;
> assert_eq!(v1 + i, 20);
> assert_eq!(v2 + i, 20);
>
> That will not build, because `i` cannot have two types. But it will if
> you comment one of the two asserts.
>
> And if you do:
>
> let v = BitInt::<u16, 4>::from_expr(15);
> assert_eq!(v + 5u8, 20);
>
> It will not build either -- there is not even "widening" going on from
> `u8` to `u16` in this last example.
The current BitInt() allows this:
let v = BitInt::<u32, 4>::new::<15>();
assert_eq!(v * 10, 150);
It looks and feels like C integer promotion. If Rust doesn't like it,
we shouldn't allow such things with BitInt()s.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 19:43 ` Yury Norov
@ 2025-11-03 20:00 ` Yury Norov
0 siblings, 0 replies; 22+ messages in thread
From: Yury Norov @ 2025-11-03 20:00 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Alexandre Courbot, Alice Ryhl, Danilo Krummrich, Miguel Ojeda,
Joel Fernandes, Jesung Yang, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, linux-kernel, rust-for-linux
On Mon, Nov 03, 2025 at 02:43:04PM -0500, Yury Norov wrote:
> On Mon, Nov 03, 2025 at 03:54:08PM +0100, Miguel Ojeda wrote:
> > On Mon, Nov 3, 2025 at 3:26 PM Yury Norov <yury.norov@gmail.com> wrote:
> > >
> > > This is exactly what the patch does:
> >
> > No, there are no arithmetic conversions going on here in the sense of
> > C. It defines a particular operation for a set of types.
> >
> > What you are seeing there is that literals, in Rust, do type
> > inference, and so the compiler picks a type:
> >
> > https://doc.rust-lang.org/reference/expressions/literal-expr.html#r-expr.literal.int.infer
> >
> > Thus if you do:
> >
> > let v1 = BitInt::<u8, 4>::from_expr(15);
> > let v2 = BitInt::<u16, 4>::from_expr(15);
> > let i = 5;
> > assert_eq!(v1 + i, 20);
> > assert_eq!(v2 + i, 20);
> >
> > That will not build, because `i` cannot have two types. But it will if
> > you comment one of the two asserts.
> >
> > And if you do:
> >
> > let v = BitInt::<u16, 4>::from_expr(15);
> > assert_eq!(v + 5u8, 20);
> >
> > It will not build either -- there is not even "widening" going on from
> > `u8` to `u16` in this last example.
>
> The current BitInt() allows this:
>
> let v = BitInt::<u32, 4>::new::<15>();
> assert_eq!(v * 10, 150);
>
> It looks and feels like C integer promotion. If Rust doesn't like it,
> we shouldn't allow such things with BitInt()s.
Sorry, send an unfinished answer.
So,
let v = BitInt::<u16, 4>::from_expr(5);
assert_eq!(v + 5, 10);
is OK,
assert_eq!(v + 5u8, 10);
is a compile-time error, and
assert_eq!(v + 50, 55);
is OK, and in fact an integer promotion unwelcome in Rust.
This is not what I, as the user of BitInts(), would expect to see.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-03 19:36 ` Yury Norov
@ 2025-11-04 3:13 ` Alexandre Courbot
2025-11-04 19:30 ` Yury Norov
0 siblings, 1 reply; 22+ messages in thread
From: Alexandre Courbot @ 2025-11-04 3:13 UTC (permalink / raw)
To: Yury Norov, Alexandre Courbot
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Tue Nov 4, 2025 at 4:36 AM JST, Yury Norov wrote:
> On Mon, Nov 03, 2025 at 10:42:13PM +0900, Alexandre Courbot wrote:
>> Hi Yury,
>>
>> On Mon Nov 3, 2025 at 11:17 AM JST, Yury Norov wrote:
>> > On Fri, Oct 31, 2025 at 10:39:57PM +0900, Alexandre Courbot wrote:
>> >> Add the `BitInt` type, which is an integer on which the number of bits
>> >> allowed to be used is restricted, capping its maximal value below that
>> >> of primitive type is wraps.
>> >>
>> >> This is useful to e.g. enforce guarantees when working with bit fields.
>> >>
>> >> Alongside this type, provide many `From` and `TryFrom` implementations
>> >> are 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 | 75 ++++
>> >> rust/kernel/num/bitint.rs | 896 ++++++++++++++++++++++++++++++++++++++++++++++
>> >> 3 files changed, 972 insertions(+)
>> >
>> > ...
>> >
>> >> +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
>> >> +///
>> >> +/// Can be used in const context.
>> >> +macro_rules! fits_within {
>> >> + ($value:expr, $type:ty, $num_bits:expr) => {{
>> >> + // Any attempt to create a `BitInt` with more bits used for representation than its backing
>> >> + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
>> >> + build_assert!(
>> >> + <$type>::BITS >= $num_bits,
>> >> + "Number of bits requested for representation is larger than backing type."
>> >> + );
>> >> +
>> >> + let shift: u32 = <$type>::BITS - $num_bits;
>> >> + let v = $value;
>> >> +
>> >> + // The value fits within `NUM_BITS` if shifting it left by the number of unused bits, then
>> >> + // right by the same number, doesn't change the value.
>> >> + //
>> >> + // This method has the benefit of working with both unsigned and signed integers.
>> >> + (v << shift) >> shift == v
>> >
>> > In C it doesn't work:
>> >
>> > int c = 0x7fffffff;
>> > printf("%x\t%x\n", c, (c << 1) >> 1); // 7fffffff ffffffff
>> >
>> > Neither in rust:
>> >
>> > let c: i32 = 0x7fffffff;
>> > println!("{} {}", c, (c << 1) >> 1); // 2147483647 -1
>> >
>> > Or I misunderstand something?
>>
>> For a short while I though this was indeed not working, to my despair as
>> this looked like an elegant solution.
>
> No it does not. One suggested by Alice is better because it's more
> straightforward.
The benefit of the shift approach is that it makes it very clear to the
compiler that the MSBs will be unused, opening the way to related
optimizations. I'm worried this would not be carried as clearly if we
rely on value comparisons.
>
>> But then I considered why we are doing that shift by 1 in the first
>> place: that would be because we are intent on using only 31 of the 32
>> bits of the `0x7fffffff` value, in order to encode a signed number.
>>
>> And the positive `0x7fffffff` value cannot be encoded as a signed
>> integer of 31 bits, as after ignoring the MSB the next bit (which would
>> be the sign bit) is `1`. So the post-shift value differs from the
>> original one, and the creation of a `BitInt<i32, 31>` for that value
>> fails, which is working as intended.
>>
>> If OTOH we did the same operation for a `BitInt<u32, 31>`, the
>> non-signed nature of the value would make the shift-and-back operation
>> produce the same value as the initial one - allowing the creation of the
>> `BitInt`, which again is correct because `0x7fffffff` can be
>> represented as an unsigned value using 31 bits.
>>
>> I have tested both cases successfully - so this way of validating still
>> looks correct to me.
>
> I read this carefully more than once, but still can't understand. So
> I gave your macro a try:
>
> macro_rules! fits_within {
> ($value:expr, $BITS:expr, $num_bits:expr) => {{
> let shift: u32 = $BITS - $num_bits;
> let v = $value;
>
> (v << shift) >> shift == v
> }};
> }
>
> fn main() {
> let a: i32 = -1;
> let b: i32 = 0x7fffffff;
>
> println!("{}", fits_within!(a, 32, 31)); // true
> println!("{}", fits_within!(b, 32, 31)); // false
> }
>
> This is exactly opposite to what I naively expect: 32-bit '-1'
> shouldn't fit into 31-bit storage, but 31-bit 0x7fffffff should.
That still looks correct to me, but from your later example I understand
why you think it isn't. Let me explain my reasoning.
For `a`, you want to represent the signed value `-1` using 31 bits.
That's possible (just like you can represent it using 16 or 8 bits), so
`fits_within` succeeds.
For `b`, you want to represent the signed, positive value `0x7fffffff`
using 31 bits. To do that, the MSB or sign bit must be zero, but bit 30
is set to 1 in that value. Thus you need at 32 bits to represent it
properly, and `fits_within` fails.
But from your example below I think your expectation is that we should
consider bit 30 as the sign bit in this case? There could be a case for
doing that, but I think it is also essential that we are able to express
things like "create a 4-bits BitInt with the value -1" simply.
>
> And said aloud, my naive expectation is: fits_withing() must
> return true iff typecasting from one type to another followed
> by typecasting back to the original type would not change the
> value.
>
> Nevermind. Let's consider a more practical example: I read some
> 12-bit ADC with a C function:
>
> int read_my_adc();
>
> It gives me either some 12-bit value, or errno, like -EAGAIN. Let's
> assume my ADC is differential, and bit 11 encodes sign of the output.
> Now, I want to put it into a 12-bit variable. So:
>
> let val = read_my_adc(); // -EAGAIN
> fits_within!(val, i32, 12);
>
> returns true. This is surely not what one would expect.
In Rust `read_my_adc` would return a `Result`, so you cannot pass the
error value to `fits_within` to begin with.
Now if the problem is that `val`'s MSBs are zero and we still want to
interpret it as signed with bit 11 as the signed bit, that's a different
thing (and I better understand the different expectations we have about
`fits_within`). We could either normalize the value before turning it
into a bounded int, or have a dedicated constructor that only considers
the first 12 bits and extends the bit sign as appropriate. Normalizing
is simple ("if bit 11 is set then apply this mask on val"), so I'd
suggest that `read_my_adc` does that (and better, returns a
`Result<BitInt>`).
>
> This is one more example:
>
> let a: i64 = -1i64;
> println!("{}", fits_within!(a, 32, 31));
>
> Here you exceed even the 'backing storage', but still return OK.
>
>> >> + }};
>> >> +}
>> >> +
>> >> +/// Trait for primitive integer types that can be used to back a [`BitInt`].
>> >> +///
>> >> +/// This is mostly used to lock all the operations we need for [`BitInt`] in a single trait.
>> >> +pub trait Boundable
>> >> +where
>> >> + Self: Integer
>> >> + + Sized
>> >> + + Copy
>> >> + + core::ops::Shl<u32, Output = Self>
>> >> + + core::ops::Shr<u32, Output = Self>
>> >> + + core::cmp::PartialEq,
>> >> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
>> >> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
>> >> +{
>> >> + /// Returns `true` if `value` can be represented with at most `NUM_BITS` on `T`.
>> >> + fn fits_within(value: Self, num_bits: u32) -> bool {
>> >> + fits_within!(value, Self, num_bits)
>> >> + }
>> >> +}
>> >> +
>> >> +/// Inplement `Boundable` for all integers types.
>> >> +impl<T> Boundable for T
>> >> +where
>> >> + T: Integer
>> >> + + Sized
>> >> + + Copy
>> >> + + core::ops::Shl<u32, Output = Self>
>> >> + + core::ops::Shr<u32, Output = Self>
>> >> + + core::cmp::PartialEq,
>> >> + Self: TryInto<u8> + TryInto<u16> + TryInto<u32> + TryInto<u64>,
>> >> + Self: TryInto<i8> + TryInto<i16> + TryInto<i32> + TryInto<i64>,
>> >> +{
>> >> +}
>> >> +
>> >> +/// Integer type for which only the bits `0..NUM_BITS` are valid.
>> >> +///
>> >> +/// # Invariants
>> >> +///
>> >> +/// Stored values are represented with at most `NUM_BITS` bits.
>> >> +///
>> >> +/// # Examples
>> >> +///
>> >> +/// ```
>> >> +/// use kernel::num::BitInt;
>> >> +///
>> >> +/// // An unsigned 8-bit integer, of which only the 4 LSBs can ever be set.
>> >> +/// // The value `15` is statically validated to fit within the specified number of bits.
>> >> +/// let v = BitInt::<u8, 4>::new::<15>();
>> >
>> > What for do you make user to declare the storage explicitly? From
>> > end-user perspective, declaring 4-bit variable must imply the most
>> > suitable storage... C version of the same doesn't allow user to select
>> > the storage as well:
>> >
>> > _BitInt(4) x = 8;
>> >
>> > I can't think out any useful usecase for this... I believe that the
>> > optimal storage must be chosen by implementation. And it may even be
>> > different for different arches.
>>
>> Alice provided a good justification in her reply, but let me elaborate a
>> bit more.
>
> This is what Alice said:
>
> It's more complex to not specify the backing storage explicitly, but I
> also think it would be nice to be able to avoid it.
>
> I agree with her. But I don't see any justification here for the
> proposed approach.
>
> TBH, I think it's not too complicated to realize that the best
> backing storage for an N-bit value is 2^(ceil(log(N)))-bit type,
> unless arch specifics comes in play.
>
> I also think that you can bake first version based on 64-bit backing
> storage, regardless of number of bits, so tailoring BitInt() would
> be a future improvement.
I don't think I can explain it better than I already did. If we don't
let users control the size of their values, it will make it very
cumbersome to perform actual work with them.
>
>> C allows itself to implicitly cast values when performing operations.
>> For instance:
>>
>> signed char a = -128;
>> unsigned int b = 120;
>> unsigned short c = b + a;
>>
>> printf("%d\n", c);
>>
>> This absolutely not confusing program is perfectly valid and prints
>> `65528`.
>>
>> For the equivalent code in Rust:
>>
>> let a = -128i8;
>> let b = 120u32;
>> let c: u16 = b + a;
>>
>> I don't even bother printing the result because these 3 lines alone
>> produce 3 build errors. Rust doesn't let you perform operations on
>> primitive integers that are not exactly the same type.
>>
>> So that's the first reason for explicitly passing a type: so you can
>> perform the operations you want with your `BitInt` without jumping
>> through cast hoops. This is particularly important to use it with
>> bitfields, which is the primary goal.
>
> OK, that's really a Rust innovation. So, if we follow the rule, I have
> two and a half considerations:
>
> 1. It also means that BitInt(u8, 4) and BitInt(u8, 5) should be the
> different types, and you shouldn't be able to do arithmetics with them
> even if the backing types match. Can you confirm that?
That's true as of v2. But we could probably make it work, and make the
return type be the backing type. Actually I got the following to work:
let v = BitInt::<u8, 4>::new::<15>();
let u = BitInt::<u8, 5>::new::<30>();
assert_eq!(v + 5, 20);
assert_eq!(v + u, 45);
And I'd say it probably does make sense to support this, since `BitInt`
is really just a transparent shell around a primitive type that
restricts its allowed values. That's all the more reason to specify the
type being wrapped, otherwise you wouldn't be able to e.g. add a
`BitInt<4>` to a `BitInt<17>` as their automatically-infered backing
types would differ.
>
> 2. Later in your patch you allow this:
>
> let v = BitInt::<u32, 4>::new::<15>();
> assert_eq!(v * 10, 150);
>
> This one is really misleading. You allow me to multiply 4-bit number
> by 10, which looks really C-like, and produce an extended result, just
> like C does. But from what you say here, and Miguel in another email,
> it breaks the spirit of Rust types safety.
We don't produce an extended result here. We multiply a u32 with another
u32, and get a u32 as a result. It does make sense when (repeating
myself, but this is the fundamental point) you consider `BitInt` as a
transparent shell around a primitive type that restricts its possible
values.
>
> 2a. Here you allow to do:
>
> let v = BitInt::<i32, 4>::new::<15>();
> let n = -EINVAL;
> assert_eq!(v + n, -7);
>
> And this is not OK. To follow Rust way, you need to convert n to
> BitInt(4), and then it would work as expected - making sure it's
> impossible and throwing an overflow.
Have you tried building this code before telling me it is allowed? How
do you get `EINVAL` as an integer in Rust?
Error codes are their own type, `kernel::error::Error`, so you cannot
even contemplate adding them to an integer.
>
>> Another reason is that even if you don't care about the size of the
>> storage, you should care about its signedness, which is part of the
>> type.
>
> Just invent BitUint or unsigned BitInt, or teach the macro to
> understand BitInt(i12) notation.
>
>> I played a bit with C's `_Bitint`, and managed to produce this
>> wonder:
>>
>> _BitInt(8) v = -1;
>> printf("%d\n", v);
>>
>> This programs prints `255`, even though I used the signed "%d"
>> formatter? Maybe that's because I should make my `_BitInt` explicitly
>> signed?
>>
>> signed _BitInt(8) v = -1;
>> printf("%d\n", v);
>>
>> Nope. Still `255`. Go figure. ¯\_(ツ)_/¯
>
> Clang throws Wformat on this; and it is printf() issue, not the
> _BitInt()s math bug.
>
>> You cannot do that with our implementation. You either specify
>>
>> let v = BitInt::<i8, 8>::new::<-1>();
>>
>> and get a proper, signed `-1` value that prints properly, or do
>>
>> let v = BitInt::<u8, 8>::new::<-1>();
>>
>> and get the build error you should get. No ambiguity, no surprises.
>>
>> >
>> >> +/// assert_eq!(v.get(), 15);
>> >> +///
>> >> +/// let v = BitInt::<i8, 4>::new::<-8>();
>> >> +/// assert_eq!(v.get(), -8);
>> >> +///
>> >> +/// // This doesn't build: a `u8` is smaller than the requested 9 bits.
>> >> +/// // let _ = BitInt::<u8, 9>::new::<10>();
>> >> +///
>> >> +/// // This also doesn't build: the requested value doesn't fit within the requested bits.
>> >> +/// // let _ = BitInt::<i8, 4>::new::<8>();
>> >> +///
>> >> +/// // Values can also be validated at runtime. This succeeds because `15` can be represented
>> >> +/// // with 4 bits.
>> >> +/// assert!(BitInt::<u8, 4>::try_new(15).is_some());
>> >> +/// // This fails because `16` cannot be represented with 4 bits.
>> >> +/// assert!(BitInt::<u8, 4>::try_new(16).is_none());
>> >
>> > Nice! Maybe .is_overflow() instead of .is_none(), so that user will
>> > know that the variable contains truncated value. Just like C does.
>> >
>> > Can you please print the exact error that user will get on compile-
>> > and runtime? How big is the cost of runtime test for overflow? If it
>> > is indeed nonzero, can you consider making the runtime part
>> > configurable?
>>
>> `is_none()` comes from the `Option` type, so we cannot change its name
>> and it's the idiomatic way to do anyway.
>
> I understand. I suggested to implement this .is_overflow(), and have
> .is_none() reserved for cases like assigning floating values to
> unsigned type.
We cannot add a method to `Option`, we would have to implement our own
return type and we are not doing that. Returning an `Option` is the
idiomatic way to do in that case, that's how it is done in the standard
library [1], and everyone a bit familiar with Rust expects this pattern.
[1] https://doc.rust-lang.org/std/num/struct.NonZero.html#method.new
We also don't need to reserve anything for assigning floating values to
unsigned types, since you already get a build error if you try to assign
floating values to unsigned types.
>
>> The runtime cost for overflow is the double-shift and comparison with
>> the initial value.
>
> Are you sure that everyone is OK to pay this runtime cost, especially
> after passing debug phase?
`BitInt` guarantees the following invariant: stored values can be
represented within `N` bits, and the compiler and user can make
optimization decisions based on this invariant.
If you want to create a `BitInt`, you must guarantee that this invariant
is withheld for the value you want to wrap. If you can prove this
statically, you can use `new` or `from_expr` constructors, and there
will be no runtime check and thus no runtime cost.
If you cannot, then you must check the value at runtime - otherwise you
get undefined behavior.
Now we can also add a `new_unchecked` constructor that is marked
`unsafe` and bypass that check altogether if the need arises, but for
now the current family of constructors looks reasonable to me.
>
>> >> +/// // Non-constant expressions can also be validated at build-time thanks to compiler
>> >> +/// // optimizations. This should be used as a last resort though.
>> >> +/// let v = BitInt::<u8, 4>::from_expr(15);
>> >
>> > Not sure I understand that. Can you confirm my understanding?
>> >
>> > 1. For compile-time initialization I use BitInt::<i8, 4>::new::<8>();
>> > 2. For compile- or runtime initialization: BitInt::<i8, 4>::from_expr(val);
>> > 3. For runtime-only initialization: BitInt::<i8, 4>::try_new(val);
>> >
>> > In this scheme #3 looks excessive...
>>
>> This example was not very good. The v2 features a better one:
>>
>> let v = BitInt::<u32, 4>::from_expr(some_number() & 0xf);
>>
>> Here assume that `some_number()` returns a random value. You cannot use
>> the static `new` constructor, as the exact value is not statically
>> known, so the alternative would be to use `try_new` and check for an
>> overflow error you know cannot happen because the value is masked with
>> `0xf` and thus will fit the 4 bits.
>>
>> `from_expr` allows you to create this `BitInt` infallibly, by relying on
>> the compiler being smart enough to infer from the mask operation that
>> the value will indeed satify the constraints of the `BitInt`, and throws
>> a linker error if it couldn't. If the program builds, there is no need
>> for error checking and no runtime validation.
>
> So from_expr() only allows statically true expressions, like this one
>
> some_number() & 0xf < 16.
>
> And try_new() allows true runtime fallible ones.
>
> Looks like OK... But this ::new<8>() syntax really looks overloaded.
That's how Rust deals with constant parameters. FWIW, using
`from_expr(8)` should be strictly equivalent, but the `::<8>` syntax has
the benefit of being usable in `const` context.
>
>> >> +/// // Common operations are supported against the backing type.
>> >> +/// assert_eq!(v + 5, 20);
>> >> +/// assert_eq!(v / 3, 5);
>> >
>> > No, v + 5 == 20 for a different reason. There's nothing about 'backing
>> > storage' here.
>> >
>> > v + 5 should be 20 because addition implies typecasting to the wider
>> > type. In this case, 20 is numeral, or int, and BitInt(4) + int == int.
>> >
>> > I tried C23, and it works exactly like that:
>> >
>> > unsigned _BitInt(4) x = 15;
>> >
>> > printf("%d\n", x + 5); // 20
>> > printf("%d\n", x / 3); // 5
>> > printf("%d\n", x + (unsigned _BitInt(4))5); // 4
>> > x += 5;
>> > printf("%d\n", x); // 4
>> >
>> > Rust _must_ do the same thing to at least be arithmetically
>> > compatible to the big brother.
>>
>> Rust doesn't do implicit casts. I do agree that "backing storage" is not
>> the best choice of words though.
>
> In fact, in "assert_eq!(v + 5, 20)" the '5' is implicitly typecasted
> to the backing storage type.
>
> In other words, Rust doesn't prohibit typecasting. It just typecasts
> to the most restricted type, instead than most relaxed one.
>
>> > It makes me more confident that this 'backing storage' concept
>> > brings nothing but confusion.
`5` is never cast. It is created as the backing storage type of `v` from
the beginning.
>> >
>> >> +/// // The backing type can be changed while preserving the number of bits used for representation.
>> >> +/// assert_eq!(v.cast::<u32>(), BitInt::<u32, 4>::new::<15>());
>> >> +///
>> >> +/// // We can safely extend the number of bits...
>> >> +/// assert_eq!(v.extend::<5>(), BitInt::<u8, 5>::new::<15>());
>> >> +/// // ... but reducing the number of bits fails here as the value doesn't fit anymore.
>> >> +/// assert_eq!(v.try_shrink::<3>(), None);
>> >
>> > Not sure what for you need this. If I need to 'extend', I just assign
>> > the value to a variable:
>> >
>> > BitInt(3) a = 3;
>> > BitInt(10) b;
>> > int c;
>> >
>> > b = a + 123; // extend
>> > c = b; // another extend
>> >
>> > How would this 'extend' and 'shrink' work with arrays of BitInts?
>>
>> I think this discussion would be more productive if we don't rely on
>> examples in a language that is irrelevant for the current patch. :)
>
> The language I rely on is called 'pseudo language', and it is perfectly
> OK as soon as it helps me to describe my intention. If you want me to
> start thinking in Rust, please make it more friendly.
I am doing my best to make my explanations accessible.
To answer your question, your code above would translate to
something like:
let a = BitInt::<i8, 3>::new::<3>();
let b = BitInt::<i16, 10>::from_expr(a.cast::<i16>() + 123i16);
let c: i32 = b.into();
(let `let b` line works because `from_expr` can infer that the BitInt
invariant won't be broken - another way would be to use `try_new` if the
value in `a` was not known at build time).
Regarding arrays of BitInts, I guess you mean BitInts that are larger
than any primitive type? This is out of scope for this implementation.
>
> The following is really a nightmare:
>
> let a = BitInt::<u8, 3>::new::<3>();
> let b = BitInt::<u16, 10>::new::<123>() + BitInt::<u16, 10>::try_from(a);
>
> let c = BitInt::<u32, 32>::try_from(a) + BitInt::<u32, 32>::try_from(b);
It also wouldn't build. Here is the correct version (assuming the `+`
operator improvement discussed above is available):
let a = BitInt::<u8, 3>::new::<3>();
let b = BitInt::<u16, 10>::new::<123>() + a.cast::<u16>();
let c = a.cast::<u32>() + u32::from(b);
Note that `b` and `c` are regular `u16` and `u32`. Arithmetic operations
cannot guarantee that the BitInt invariant will be maintained, so the
result needs to be converted back if that's what one wants.
>
>> Rust does not allow overloading the `=` operator, so these implicit
>> conversions from one type to another cannot be performed. Having
>> dedicated methods is an idiomatic way to do this according to my
>> experience - an alternative would be to have `From` implementations, but
>> doing this elegantly would require one language feature (generic const
>> expressions) that is still not stable.
>>
>> >
>> >> +/// // Conversion into primitive types is dependent on the number of useful bits, not the backing
>> >> +/// // type.
>> >> +/// //
>> >> +/// // Event though its backing type is `u32`, this `BitInt` only uses 8 bits and thus can safely
>> >> +/// // be converted to a `u8`.
>> >> +/// assert_eq!(u8::from(BitInt::<u32, 8>::new::<128>()), 128u8);
>> >
>> > 'Backing type' is useless here too.
>> >
>> >> +/// // The same applies to signed values.
>> >> +/// asserkkt_eq!(i8::from(BitInt::<i32, 8>::new::<127>()), 127i8);
>> >> +///
>> >> +/// // This however is not allowed, as 10 bits won't fit into a `u8` (regardless of the actually
>> >> +/// // contained value).
>> >> +/// // let _ = u8::from(BitInt::<u32, 10>::new::<10>());
>> >
>> > If I explicitly typecast from a wider type, please just let me do
>> > that. In the above examples you show that .is_some() and .is_none()
>> > can help user to check for overflow if needed.
>> >
>> > Otherwise, user will hack your protection by just converting
>> > BitInt(8) to u32, and then to BitInt(10).
>>
>> Doing so would require validating that the value in the `u32` can fit
>> within the 10-bit `BitInt` one way or the other, so the protection
>> cannot be bypassed that way.
>
> That's what I meant. You allow one way to initialize data, but
> disallow another equivalent and more straightforward way.
I cannot make sense of what you want here. Can you write down what you
expect to be possible to do?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-04 3:13 ` Alexandre Courbot
@ 2025-11-04 19:30 ` Yury Norov
2025-11-04 20:21 ` Danilo Krummrich
2025-11-05 14:03 ` Alexandre Courbot
0 siblings, 2 replies; 22+ messages in thread
From: Yury Norov @ 2025-11-04 19:30 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Tue, Nov 04, 2025 at 12:13:26PM +0900, Alexandre Courbot wrote:
> On Tue Nov 4, 2025 at 4:36 AM JST, Yury Norov wrote:
> > On Mon, Nov 03, 2025 at 10:42:13PM +0900, Alexandre Courbot wrote:
> >> Hi Yury,
...
> let a = BitInt::<u8, 3>::new::<3>();
> let b = BitInt::<u16, 10>::new::<123>() + a.cast::<u16>();
>
> let c = a.cast::<u32>() + u32::from(b);
>
> Note that `b` and `c` are regular `u16` and `u32`. Arithmetic operations
> cannot guarantee that the BitInt invariant will be maintained, so the
> result needs to be converted back if that's what one wants.
What C does and what I proposed is to make BitInt a 1st class type,
just like basic integers. What you implement is really a bounded int.
Both have advantages. C-style BitInt() is a true type with all type
guarantees. And I like it more because it is a natural extension of
the existing integer scheme.
Your bounded ints are actually classical integers with some limitations.
It's not a primitive type in sense of C - it's an object. It also works
fine. You can easily extend it to arbitrary min and max limits; you can
expand it to floating types, and do whatever you can do with the objects.
BitInt(i32, -128, 255)
BitFloat(f32, -1, 1)
That's why you think that -1i32 fits into BitInt(i32, 4), and even
into BitInt(i8, 4), while I don't.
I don't know which would work better for bitfields. Most likely both
would work reasonably well. And if bitfield will carefully hide
internals, I hope most users will not care much.
You switched name to BitInt, but still think about it as an object,
and that brought all the confusion in my mind. Maybe switch back to
BoundedInt then to avoid this confusion? If you find it lengthy,
probably LimInt or simply Lint will be better for you.
Looking at how good rust macros work to implement bitfields, I thought
that they will be able to mimic native types just as well. But now it
seems like an arbitrary-length integer types requires support on
language side. Just like in C.
With that in mind, I think that bounded integers are a bit out of
scope of basic bit operations, and probably I'm not a right person
to maintain them neither in Rust, nor in C.
Please keep me in CC for next versions.
Thanks,
Yury
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-04 19:30 ` Yury Norov
@ 2025-11-04 20:21 ` Danilo Krummrich
2025-11-05 14:03 ` Alexandre Courbot
1 sibling, 0 replies; 22+ messages in thread
From: Danilo Krummrich @ 2025-11-04 20:21 UTC (permalink / raw)
To: Yury Norov
Cc: Alexandre Courbot, Alice Ryhl, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Tue Nov 4, 2025 at 8:30 PM CET, Yury Norov wrote:
> You switched name to BitInt, but still think about it as an object,
Note that in Rust even "real" primitives have a strong object character, they
have lots of methods, trait implementation and associated constants [1].
You can even implement your own custom traits for a primitive type in Rust.
[1] https://rust.docs.kernel.org/core/primitive.u32.html
> and that brought all the confusion in my mind. Maybe switch back to
> BoundedInt then to avoid this confusion? If you find it lengthy,
> probably LimInt or simply Lint will be better for you.
In another thread Alex proposed Bounded, because it follows the naming scheme of
other existing numeric types in Rust, e.g. NonZero [2].
[2] https://rust.docs.kernel.org/core/num/struct.NonZero.html
> Looking at how good rust macros work to implement bitfields, I thought
> that they will be able to mimic native types just as well.
I assume you mean primitive types. If so, I think there isn't too much we can't
do (e.g. literal syntax would be nice) with BitInt (or Bounded) that primitives
can do. Especially when we consider the strong object character of "real" Rust
primitives.
> With that in mind, I think that bounded integers are a bit out of
> scope of basic bit operations, and probably I'm not a right person
> to maintain them neither in Rust, nor in C.
I think it's fair to consider this kind of type as core Rust infrastructure.
Maybe a RUST [NUMERICS] entry would make sense? You could be a reviewer. :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-04 19:30 ` Yury Norov
2025-11-04 20:21 ` Danilo Krummrich
@ 2025-11-05 14:03 ` Alexandre Courbot
2025-11-05 15:45 ` Yury Norov
1 sibling, 1 reply; 22+ messages in thread
From: Alexandre Courbot @ 2025-11-05 14:03 UTC (permalink / raw)
To: Yury Norov, Alexandre Courbot
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Wed Nov 5, 2025 at 4:30 AM JST, Yury Norov wrote:
> On Tue, Nov 04, 2025 at 12:13:26PM +0900, Alexandre Courbot wrote:
>> On Tue Nov 4, 2025 at 4:36 AM JST, Yury Norov wrote:
>> > On Mon, Nov 03, 2025 at 10:42:13PM +0900, Alexandre Courbot wrote:
>> >> Hi Yury,
>
> ...
>
>> let a = BitInt::<u8, 3>::new::<3>();
>> let b = BitInt::<u16, 10>::new::<123>() + a.cast::<u16>();
>>
>> let c = a.cast::<u32>() + u32::from(b);
>>
>> Note that `b` and `c` are regular `u16` and `u32`. Arithmetic operations
>> cannot guarantee that the BitInt invariant will be maintained, so the
>> result needs to be converted back if that's what one wants.
>
> What C does and what I proposed is to make BitInt a 1st class type,
> just like basic integers. What you implement is really a bounded int.
>
> Both have advantages. C-style BitInt() is a true type with all type
> guarantees. And I like it more because it is a natural extension of
> the existing integer scheme.
Yeah, it's definitely different from what we are doing here. IIUC C's
_BitInt is implemented at the compiler level, here we are just a regular
Rust project, not touching the compiler at all.
>
> Your bounded ints are actually classical integers with some limitations.
That's a very accurate way to put it. It is not an unusual thing to do
though, there are several types in the standard library (e.g. `NonZero`)
that do exactly that. Putting limitations also means we get more
guarantees, which can remove unnecessary error handling and help the
compiler produce more optimized code.
> It's not a primitive type in sense of C - it's an object. It also works
> fine. You can easily extend it to arbitrary min and max limits; you can
> expand it to floating types, and do whatever you can do with the objects.
>
> BitInt(i32, -128, 255)
> BitFloat(f32, -1, 1)
>
> That's why you think that -1i32 fits into BitInt(i32, 4), and even
> into BitInt(i8, 4), while I don't.
>
> I don't know which would work better for bitfields. Most likely both
> would work reasonably well. And if bitfield will carefully hide
> internals, I hope most users will not care much.
>
> You switched name to BitInt, but still think about it as an object,
> and that brought all the confusion in my mind. Maybe switch back to
> BoundedInt then to avoid this confusion? If you find it lengthy,
> probably LimInt or simply Lint will be better for you.
It looks like we are going to settle with just `Bounded`, to follow the
naming pattern of similar types in the Rust standard library.
>
> Looking at how good rust macros work to implement bitfields, I thought
> that they will be able to mimic native types just as well. But now it
> seems like an arbitrary-length integer types requires support on
> language side. Just like in C.
If only to be able to express arbitrary-length immediate values, yes -
although I suspect macros could also help here.
But I believe the more fundamental question is: do we need
arbitrary-length (as in, larger than primitive types) integers in the
kernel, when Rust supports up to 128-bit primitives?
>
> With that in mind, I think that bounded integers are a bit out of
> scope of basic bit operations, and probably I'm not a right person
> to maintain them neither in Rust, nor in C.
>
> Please keep me in CC for next versions.
Will do.
Also jumping on Danilo's suggestion I will probably propose to add a
MAINTAINERS entry for this in the next revision (up to the Rust core
team to take it or not :)). Let me know if you want to be a reviewer,
that would be a good way to keep up with what happens here.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] rust: add BitInt integer wrapping type
2025-11-05 14:03 ` Alexandre Courbot
@ 2025-11-05 15:45 ` Yury Norov
0 siblings, 0 replies; 22+ messages in thread
From: Yury Norov @ 2025-11-05 15:45 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, Danilo Krummrich, Miguel Ojeda, Joel Fernandes,
Jesung Yang, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, linux-kernel,
rust-for-linux
On Wed, Nov 05, 2025 at 11:03:24PM +0900, Alexandre Courbot wrote:
> On Wed Nov 5, 2025 at 4:30 AM JST, Yury Norov wrote:
> > On Tue, Nov 04, 2025 at 12:13:26PM +0900, Alexandre Courbot wrote:
> >> On Tue Nov 4, 2025 at 4:36 AM JST, Yury Norov wrote:
> >> > On Mon, Nov 03, 2025 at 10:42:13PM +0900, Alexandre Courbot wrote:
> >> >> Hi Yury,
> >
> > ...
> >
> >> let a = BitInt::<u8, 3>::new::<3>();
> >> let b = BitInt::<u16, 10>::new::<123>() + a.cast::<u16>();
> >>
> >> let c = a.cast::<u32>() + u32::from(b);
> >>
> >> Note that `b` and `c` are regular `u16` and `u32`. Arithmetic operations
> >> cannot guarantee that the BitInt invariant will be maintained, so the
> >> result needs to be converted back if that's what one wants.
> >
> > What C does and what I proposed is to make BitInt a 1st class type,
> > just like basic integers. What you implement is really a bounded int.
> >
> > Both have advantages. C-style BitInt() is a true type with all type
> > guarantees. And I like it more because it is a natural extension of
> > the existing integer scheme.
>
> Yeah, it's definitely different from what we are doing here. IIUC C's
> _BitInt is implemented at the compiler level, here we are just a regular
> Rust project, not touching the compiler at all.
>
> >
> > Your bounded ints are actually classical integers with some limitations.
>
> That's a very accurate way to put it. It is not an unusual thing to do
> though, there are several types in the standard library (e.g. `NonZero`)
> that do exactly that. Putting limitations also means we get more
> guarantees, which can remove unnecessary error handling and help the
> compiler produce more optimized code.
>
> > It's not a primitive type in sense of C - it's an object. It also works
> > fine. You can easily extend it to arbitrary min and max limits; you can
> > expand it to floating types, and do whatever you can do with the objects.
> >
> > BitInt(i32, -128, 255)
> > BitFloat(f32, -1, 1)
> >
> > That's why you think that -1i32 fits into BitInt(i32, 4), and even
> > into BitInt(i8, 4), while I don't.
> >
> > I don't know which would work better for bitfields. Most likely both
> > would work reasonably well. And if bitfield will carefully hide
> > internals, I hope most users will not care much.
> >
> > You switched name to BitInt, but still think about it as an object,
> > and that brought all the confusion in my mind. Maybe switch back to
> > BoundedInt then to avoid this confusion? If you find it lengthy,
> > probably LimInt or simply Lint will be better for you.
>
> It looks like we are going to settle with just `Bounded`, to follow the
> naming pattern of similar types in the Rust standard library.
Yes, Danilo mentioned that, and I agree it's the best choice.
> > Looking at how good rust macros work to implement bitfields, I thought
> > that they will be able to mimic native types just as well. But now it
> > seems like an arbitrary-length integer types requires support on
> > language side. Just like in C.
>
> If only to be able to express arbitrary-length immediate values, yes -
> although I suspect macros could also help here.
>
> But I believe the more fundamental question is: do we need
> arbitrary-length (as in, larger than primitive types) integers in the
> kernel, when Rust supports up to 128-bit primitives?
Bitmaps are actually the arbitrary length integers. They support all
the logical operations, shifts, bitsearch, bitcounts, prints and more.
The fact that nobody's proposed arithmetics for bitmaps answers to your
question.
> > With that in mind, I think that bounded integers are a bit out of
> > scope of basic bit operations, and probably I'm not a right person
> > to maintain them neither in Rust, nor in C.
> >
> > Please keep me in CC for next versions.
>
> Will do.
>
> Also jumping on Danilo's suggestion I will probably propose to add a
> MAINTAINERS entry for this in the next revision (up to the Rust core
> team to take it or not :)). Let me know if you want to be a reviewer,
> that would be a good way to keep up with what happens here.
Sure, if you find it useful.
Thanks,
Yury
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-11-05 15:45 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-31 13:39 [PATCH 0/2] rust: add BitInt type and use in Nova's bitfield macro Alexandre Courbot
2025-10-31 13:39 ` [PATCH 1/2] rust: add BitInt integer wrapping type Alexandre Courbot
2025-11-02 5:46 ` Alexandre Courbot
2025-11-03 2:17 ` Yury Norov
2025-11-03 10:47 ` Alice Ryhl
2025-11-03 14:31 ` Alexandre Courbot
2025-11-03 13:42 ` Alexandre Courbot
2025-11-03 13:57 ` Alexandre Courbot
2025-11-03 14:01 ` Danilo Krummrich
2025-11-03 19:36 ` Yury Norov
2025-11-04 3:13 ` Alexandre Courbot
2025-11-04 19:30 ` Yury Norov
2025-11-04 20:21 ` Danilo Krummrich
2025-11-05 14:03 ` Alexandre Courbot
2025-11-05 15:45 ` Yury Norov
2025-11-03 14:10 ` Miguel Ojeda
2025-11-03 14:26 ` Yury Norov
2025-11-03 14:44 ` Alexandre Courbot
2025-11-03 14:54 ` Miguel Ojeda
2025-11-03 19:43 ` Yury Norov
2025-11-03 20:00 ` Yury Norov
2025-10-31 13:39 ` [PATCH FOR REFERENCE 2/2] gpu: nova-core: use BitInt for bitfields Alexandre Courbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).