After some discussions on the list [1] and the Rust libs team [2], this
patchset undergoes two major changes:
- The `PowerOfTwo` type is replaced by `Alignment`, which is heavily
inspired by the nightly type of the same name in the standard library
[3].
- The `last_set_bit` function is dropped, with the recommendation to use
the standard library's `checked_ilog2` which does essentially the same
thing.
The upstream `Alignment` is more constrained than the `PowerOfTwo` of
the last revision: it uses `usize` internally instead of a generic
value, and does not provide `align_down` or `align_up` methods.
These two shortcomings come together very nicely to gift us with a nice
headache: we need to align values potentially larger than `usize`, thus
need to make `align_down` and `align_up` generic. The generic parameter
needs to be constrained on the operations used to perform the alignment
(e.g. `BitAnd`, `Not`, etc) and there is one essential operation for
which no trait exists in the standard library: `checked_add`. Thus the
first patch of this series introduces a trait for it in the `num` module
and implements it for all integer types. I suspect we will need
something alongside these lines for other purposes anyway, and probably
other traits too.
This generic nature also restricts these methods to being non-const,
unfortunately. I have tried to implement them as macros instead, but
quickly hit a wall due to the inability to convert `Alignment`'s `usize`
into the type of the value to align.
So here it is, not perfect but the need for a standard way to align is
starting to become more pressing.
[1] https://lore.kernel.org/rust-for-linux/DBTGVEJQOUDM.OTGZ6PXLB9JV at
nvidia.com/T/#m09e068ecadf5b41099d4c6c55e13fbb3a98c5839
[2] https://github.com/rust-lang/libs-team/issues/631
[3] https://doc.rust-lang.org/std/ptr/struct.Alignment.html
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
Changes in v2:
- Fix indentation of paste! in impl_last_set_bit.
- Link to v1: https://lore.kernel.org/r/20250620-num-v1-0-7ec3d3fb06c9 at
nvidia.com
Changes since split from the nova-core series:
- Rename `fls` to `last_set_bit`,
- Generate per-type doctests,
- Add invariants section to `PowerOfTwo`.
- Do not use reference to `self` in `PowerOfTwo` methods since it
implements `Copy`,
- Use #[derive] where possible instead of implementing traits
manually,
- Remove `Deref` and `Borrow` implementations.
---
Alexandre Courbot (4):
rust: add `CheckedAdd` trait
rust: add `Alignment` type
gpu: nova-core: use Alignment for alignment-related operations
gpu: nova-core: use `checked_ilog2` to emulate `fls`
Documentation/gpu/nova/core/todo.rst | 15 ---
drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 +-
drivers/gpu/nova-core/fb.rs | 6 +-
drivers/gpu/nova-core/firmware/fwsec.rs | 11 +-
drivers/gpu/nova-core/vbios.rs | 4 +-
rust/kernel/lib.rs | 2 +
rust/kernel/num.rs | 28 ++++
rust/kernel/ptr.rs | 213 ++++++++++++++++++++++++++++++
8 files changed, 255 insertions(+), 28 deletions(-)
---
base-commit: 14ae91a81ec8fa0bc23170d4aa16dd2a20d54105
change-id: 20250620-num-9420281c02c7
Best regards,
--
Alexandre Courbot <acourbot at nvidia.com>
Rust provides traits for standard arithmetic and logic operations, but
in the context of the kernel we often need to consider overflows. The
checked Rust arithmetic methods are unfortunately not behind a trait,
which makes them unavailable to generic code.
As a start, add the `CheckedAdd` trait providing the `checked_add`
operation and implement it for all integer types. Its name and location
are inspired by the user-space `num` crate.
This trait is to be first used by the `Alignment` type.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
rust/kernel/lib.rs | 1 +
rust/kernel/num.rs | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index
6b4774b2b1c37f4da1866e993be6230bc6715841..2955f65da1278dd4cba1e4272ff178b8211a892c
100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -89,6 +89,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
0000000000000000000000000000000000000000..c81bb046078b70c321dd52aa9c2b5518be49d249
--- /dev/null
+++ b/rust/kernel/num.rs
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Numerical and binary utilities for primitive types.
+
+use core::ops::Add;
+
+/// Trait for performing a checked addition that returns `None` if the
operation would overflow.
+///
+/// This trait exists in order to represent scalar types already having a
`checked_add` method in
+/// generic code.
+pub trait CheckedAdd: Sized + Add<Self, Output = Self> {
+ /// Computes `self + rhs`, returning `None` if an overflow would occur.
+ fn checked_add(self, rhs: Self) -> Option<Self>;
+}
+
+macro_rules! impl_checked_add {
+ ($($t:ty),*) => {
+ $(
+ impl CheckedAdd for $t {
+ fn checked_add(self, rhs: Self) -> Option<Self> {
+ self.checked_add(rhs)
+ }
+ }
+ )*
+ };
+}
+
+impl_checked_add!(u8, u16, u32, u64, usize, i8, i16, i32, i64, isize);
--
2.50.1
Alignment operations are very common in the kernel. Since they are
always performed using a power of two value, enforcing this invariant
through a dedicated type leads to less bugs and can lead to improved
generated code.
Introduce the `Alignment` type, inspired by the nightly Rust feature of
the same name. It provides the same interface as its upstream namesake,
while extending it with `align_up` and `align_down` operations that are
usable on any integer type.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
rust/kernel/lib.rs | 1 +
rust/kernel/ptr.rs | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 214 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index
2955f65da1278dd4cba1e4272ff178b8211a892c..0e66b55cde66ee1b274862cd78ad465a572dc5d9
100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -100,6 +100,7 @@
pub mod platform;
pub mod prelude;
pub mod print;
+pub mod ptr;
pub mod rbtree;
pub mod revocable;
pub mod security;
diff --git a/rust/kernel/ptr.rs b/rust/kernel/ptr.rs
new file mode 100644
index
0000000000000000000000000000000000000000..6d941db58944619ea5b05676af06981a3ceaaca8
--- /dev/null
+++ b/rust/kernel/ptr.rs
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Types and functions to work with pointers and addresses.
+
+use core::fmt::Debug;
+use core::num::NonZero;
+use core::ops::{BitAnd, Not};
+
+use crate::build_assert;
+use crate::num::CheckedAdd;
+
+/// Type representing an alignment, which is always a power of two.
+///
+/// It be used to validate that a given value is a valid alignment, and to
perform masking and
+/// align down/up operations. The alignment operations are done using the
[`align_up!`] and
+/// [`align_down!`] macros.
+///
+/// Heavily inspired by the [`Alignment`] nightly feature from the Rust
standard library, and
+/// hopefully to be eventually replaced by it.
+///
+/// [`Alignment`]: https://github.com/rust-lang/rust/issues/102070
+///
+/// # Invariants
+///
+/// An alignment is always a power of two.
+#[repr(transparent)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct Alignment(NonZero<usize>);
+
+impl Alignment {
+ /// Validates that `align` is a power of two at build-time, and returns an
[`Alignment`] of the
+ /// same value.
+ ///
+ /// A build error is triggered if `align` cannot be asserted to be a power
of two.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// let v = Alignment::new(16);
+ /// assert_eq!(v.as_usize(), 16);
+ /// ```
+ #[inline(always)]
+ pub const fn new(align: usize) -> Self {
+ build_assert!(align.is_power_of_two());
+
+ // INVARIANT: `align` is a power of two.
+ // SAFETY: `align` is a power of two, and thus non-zero.
+ Self(unsafe { NonZero::new_unchecked(align) })
+ }
+
+ /// Validates that `align` is a power of two at runtime, and returns an
+ /// [`Alignment`] of the same value.
+ ///
+ /// [`None`] is returned if `align` was not a power of two.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// assert_eq!(Alignment::new_checked(16), Some(Alignment::new(16)));
+ /// assert_eq!(Alignment::new_checked(15), None);
+ /// assert_eq!(Alignment::new_checked(1), Some(Alignment::new(1)));
+ /// assert_eq!(Alignment::new_checked(0), None);
+ /// ```
+ #[inline(always)]
+ pub const fn new_checked(align: usize) -> Option<Self> {
+ if align.is_power_of_two() {
+ // INVARIANT: `align` is a power of two.
+ // SAFETY: `align` is a power of two, and thus non-zero.
+ Some(Self(unsafe { NonZero::new_unchecked(align) }))
+ } else {
+ None
+ }
+ }
+
+ /// Returns the alignment of `T`.
+ #[inline(always)]
+ pub const fn of<T>() -> Self {
+ // INVARIANT: `align_of` always returns a power of 2.
+ Self(unsafe { NonZero::new_unchecked(align_of::<T>()) })
+ }
+
+ /// Returns the base-2 logarithm of the alignment.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// assert_eq!(Alignment::of::<u8>().log2(), 0);
+ /// assert_eq!(Alignment::new(16).log2(), 4);
+ /// ```
+ #[inline(always)]
+ pub const fn log2(self) -> u32 {
+ self.0.ilog2()
+ }
+
+ /// Returns this alignment as a [`NonZero`].
+ ///
+ /// It is guaranteed to be a power of two.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// assert_eq!(Alignment::new(16).as_nonzero().get(), 16);
+ /// ```
+ #[inline(always)]
+ pub const fn as_nonzero(self) -> NonZero<usize> {
+ if !self.0.is_power_of_two() {
+ // SAFETY: per the invariants, `self.0` is always a power of two so
this block will
+ // never be reached.
+ unsafe { core::hint::unreachable_unchecked() }
+ }
+ self.0
+ }
+
+ /// Returns this alignment as a `usize`.
+ ///
+ /// It is guaranteed to be a power of two.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// assert_eq!(Alignment::new(16).as_usize(), 16);
+ /// ```
+ #[inline(always)]
+ pub const fn as_usize(self) -> usize {
+ self.as_nonzero().get()
+ }
+
+ /// Returns the mask corresponding to `self.as_usize() - 1`.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// assert_eq!(Alignment::new(0x10).mask(), 0xf);
+ /// ```
+ #[inline(always)]
+ pub const fn mask(self) -> usize {
+ // INVARIANT: `self.as_usize()` is guaranteed to be a power of two
(i.e. non-zero), thus
+ // `1` can safely be substracted from it.
+ self.as_usize() - 1
+ }
+
+ /// Aligns `value` down to this alignment.
+ ///
+ /// If the alignment contained in `self` is too large for `T`, then `0` is
returned, which
+ /// is correct as it is also the result that would have been returned if it
did.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// assert_eq!(Alignment::new(0x10).align_down(0x2f), 0x20);
+ /// assert_eq!(Alignment::new(0x10).align_down(0x30), 0x30);
+ /// assert_eq!(Alignment::new(0x1000).align_down(0xf0u8), 0x0);
+ /// ```
+ #[inline(always)]
+ pub fn align_down<T>(self, value: T) -> T
+ where
+ T: TryFrom<usize> + BitAnd<Output = T> + Not<Output =
T> + Default,
+ {
+ T::try_from(self.mask())
+ .map(|mask| value & !mask)
+ .unwrap_or(T::default())
+ }
+
+ /// Aligns `value` up to this alignment, returning `None` if aligning
pushes the result above
+ /// the limits of `value`'s type.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::Alignment;
+ ///
+ /// assert_eq!(Alignment::new(0x10).align_up(0x4f), Some(0x50));
+ /// assert_eq!(Alignment::new(0x10).align_up(0x40), Some(0x40));
+ /// assert_eq!(Alignment::new(0x10).align_up(0x0), Some(0x0));
+ /// assert_eq!(Alignment::new(0x10).align_up(u8::MAX), None);
+ /// assert_eq!(Alignment::new(0x100).align_up(0x10u8), None);
+ /// assert_eq!(Alignment::new(0x100).align_up(0x0u8), Some(0x0));
+ /// ```
+ #[inline(always)]
+ pub fn align_up<T>(self, value: T) -> Option<T>
+ where
+ T: TryFrom<usize>
+ + BitAnd<Output = T>
+ + Not<Output = T>
+ + Default
+ + PartialEq
+ + Copy
+ + CheckedAdd,
+ {
+ let aligned_down = self.align_down(value);
+ if value == aligned_down {
+ Some(aligned_down)
+ } else {
+ T::try_from(self.as_usize())
+ .ok()
+ .and_then(|align| aligned_down.checked_add(align))
+ }
+ }
+}
--
2.50.1
Alexandre Courbot
2025-Aug-04 11:45 UTC
[PATCH v2 3/4] gpu: nova-core: use Alignment for alignment-related operations
Make use of the newly-available `Alignment` type and remove the
corresponding TODO item.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
Documentation/gpu/nova/core/todo.rst | 1 -
drivers/gpu/nova-core/fb.rs | 6 +++---
drivers/gpu/nova-core/firmware/fwsec.rs | 11 +++++------
drivers/gpu/nova-core/vbios.rs | 4 ++--
4 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/Documentation/gpu/nova/core/todo.rst
b/Documentation/gpu/nova/core/todo.rst
index
894a1e9c3741a43ad4eb76d24a9486862999874e..8fdb5bced3460a3971699df79ffa2c69f84b2735
100644
--- a/Documentation/gpu/nova/core/todo.rst
+++ b/Documentation/gpu/nova/core/todo.rst
@@ -147,7 +147,6 @@ Numerical operations [NUMM]
Nova uses integer operations that are not part of the standard library (or not
implemented in an optimized way for the kernel). These include:
-- Aligning up and down to a power of two,
- The "Find Last Set Bit" (`fls` function of the C part of the
kernel)
operation.
diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index
4a702525fff4f394b75fcf54145ba78e34a1a539..ebeffc1b1feccc5c4dc08135d8447ec185a3068a
100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -3,6 +3,7 @@
use core::ops::Range;
use kernel::prelude::*;
+use kernel::ptr::Alignment;
use kernel::sizes::*;
use kernel::types::ARef;
use kernel::{dev_warn, device};
@@ -130,10 +131,9 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0) ->
Result<Self> {
};
let frts = {
- const FRTS_DOWN_ALIGN: u64 = SZ_128K as u64;
+ const FRTS_DOWN_ALIGN: Alignment = Alignment::new(SZ_128K);
const FRTS_SIZE: u64 = SZ_1M as u64;
- // TODO[NUMM]: replace with `align_down` once it lands.
- let frts_base = (vga_workspace.start & !(FRTS_DOWN_ALIGN - 1))
- FRTS_SIZE;
+ let frts_base = FRTS_DOWN_ALIGN.align_down(vga_workspace.start) -
FRTS_SIZE;
frts_base..frts_base + FRTS_SIZE
};
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs
b/drivers/gpu/nova-core/firmware/fwsec.rs
index
0dff3cfa90afee0cd4c3348023c8bfd7edccdb29..add8cea7bbecc98238d418d0fe6f235c6b850d58
100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -16,6 +16,7 @@
use kernel::device::{self, Device};
use kernel::prelude::*;
+use kernel::ptr::Alignment;
use kernel::transmute::FromBytes;
use crate::dma::DmaObject;
@@ -203,7 +204,7 @@ pub(crate) struct FwsecFirmware {
}
// We need to load full DMEM pages.
-const DMEM_LOAD_SIZE_ALIGN: u32 = 256;
+const DMEM_LOAD_SIZE_ALIGN: usize = 256;
impl FalconLoadParams for FwsecFirmware {
fn imem_load_params(&self) -> FalconLoadTarget {
@@ -218,11 +219,9 @@ fn dmem_load_params(&self) -> FalconLoadTarget {
FalconLoadTarget {
src_start: self.desc.imem_load_size,
dst_start: self.desc.dmem_phys_base,
- // TODO[NUMM]: replace with `align_up` once it lands.
- len: self
- .desc
- .dmem_load_size
- .next_multiple_of(DMEM_LOAD_SIZE_ALIGN),
+ len: Alignment::new(DMEM_LOAD_SIZE_ALIGN)
+ .align_up(self.desc.dmem_load_size)
+ .unwrap_or(u32::MAX),
}
}
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index
5b5d9f38cbb3a6b1c374c1e0eee2509eb8d5660c..3bc24ce26b9d4d1948313f808986211facb0e8d0
100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -10,6 +10,7 @@
use kernel::error::Result;
use kernel::pci;
use kernel::prelude::*;
+use kernel::ptr::Alignment;
/// The offset of the VBIOS ROM in the BAR0 space.
const ROM_OFFSET: usize = 0x300000;
@@ -177,8 +178,7 @@ fn next(&mut self) -> Option<Self::Item> {
// Advance to next image (aligned to 512 bytes).
self.current_offset += image_size;
- // TODO[NUMM]: replace with `align_up` once it lands.
- self.current_offset = self.current_offset.next_multiple_of(512);
+ self.current_offset =
Alignment::new(512).align_up(self.current_offset)?;
Some(Ok(full_image))
}
--
2.50.1
Alexandre Courbot
2025-Aug-04 11:45 UTC
[PATCH v2 4/4] gpu: nova-core: use `checked_ilog2` to emulate `fls`
Rust's `checked_ilog2` is in effect equivalent to the C `fls` operation,
with the exception that its result is zero-indexed. This means we don't
have a good basis to introduce an equivalent of `fls` on our own.
Convert the relevant Nova code to use `checked_ilog2`, and remove the
corresponding TODO item.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
Documentation/gpu/nova/core/todo.rst | 14 --------------
drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 ++--
2 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/Documentation/gpu/nova/core/todo.rst
b/Documentation/gpu/nova/core/todo.rst
index
8fdb5bced3460a3971699df79ffa2c69f84b2735..01dfa858d11fe377c345b463742c13c37878e334
100644
--- a/Documentation/gpu/nova/core/todo.rst
+++ b/Documentation/gpu/nova/core/todo.rst
@@ -141,20 +141,6 @@ Features desired before this happens:
| Complexity: Advanced
| Contact: Alexandre Courbot
-Numerical operations [NUMM]
----------------------------
-
-Nova uses integer operations that are not part of the standard library (or not
-implemented in an optimized way for the kernel). These include:
-
-- The "Find Last Set Bit" (`fls` function of the C part of the
kernel)
- operation.
-
-A `num` core kernel module is being designed to provide these operations.
-
-| Complexity: Intermediate
-| Contact: Alexandre Courbot
-
Delay / Sleep abstractions [DLAY]
---------------------------------
diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs
b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index
52c33d3f22a8e920742b45940c346c47fdc70e93..430a511aa1f85477690e78cdc1104f0e0097b0e4
100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -69,8 +69,8 @@ fn signature_reg_fuse_version_ga102(
let reg_fuse_version bar.read32(reg_fuse_base + ((ucode_id - 1) as
usize * core::mem::size_of::<u32>()));
- // TODO[NUMM]: replace with `last_set_bit` once it lands.
- Ok(u32::BITS - reg_fuse_version.leading_zeros())
+ // The original C code performs a `fls`; this is equivalent.
+ Ok(reg_fuse_version.checked_ilog2().map_or(0, |v| v + 1))
}
fn program_brom_ga102<E: FalconEngine>(bar: &Bar0, params:
&FalconBromParams) -> Result {
--
2.50.1