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