Thanks to the feedback received on the last revision, I feel like we are
converging.
The ACP for `ffs`/`fls` equivalents having been accepted [1], I am
dropping any work related to them from this patchset which is now
exclusively focused on alignment features.
The first patch makes two additions:
- Bring an equivalent of the nightly upstream `Alignment` type [2] with
an identical public interface,
- Add a new `Alignable` extension trait that provides a way to align
values up or down according to an `Alignment`, and implement it on
unsigned integer types.
The second patch makes use of these in the Nova driver.
[3] is recommended as a dependency, a small cleanup preventing an unwrap
upon applying this series.
[1] https://github.com/rust-lang/libs-team/issues/631
[2] https://doc.rust-lang.org/std/ptr/struct.Alignment.html
[3]
https://lore.kernel.org/rust-for-linux/20250808-falcondma_256b-v1-1-15f911d89ffd
at nvidia.com/
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
Changes in v3:
- Move `align_down` and `align_up` into their own `Alignable` trait.
(thanks Miguel!)
- Fix `Alignment::mask` implementation to match upstream's.
- Add missing `SAFETY:` comments.
- Improve grammar on a few hard-to-parse comments.
- Link to v2: https://lore.kernel.org/r/20250804-num-v2-0-a96b9ca6eb02 at
nvidia.com
Changes in v2:
- Remove `last_checked_bit` (use Rust's `checked_ilog2` instead).
- Port Rust nightly `Alignment` type and extend it with `align_down` and
`align_up` methods.
- 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 (2):
rust: add `Alignment` type
gpu: nova-core: use Alignment for alignment-related operations
Documentation/gpu/nova/core/todo.rst | 1 -
drivers/gpu/nova-core/fb.rs | 6 +-
drivers/gpu/nova-core/vbios.rs | 4 +-
rust/kernel/lib.rs | 1 +
rust/kernel/ptr.rs | 218 +++++++++++++++++++++++++++++++++++
5 files changed, 224 insertions(+), 6 deletions(-)
---
base-commit: 062b3e4a1f880f104a8d4b90b767788786aa7b78
change-id: 20250620-num-9420281c02c7
prerequisite-message-id: <20250808-falcondma_256b-v1-1-15f911d89ffd at
nvidia.com>
prerequisite-patch-id: 2439f5f9b560ee4867716f0018b5326dcd72cda3
Best regards,
--
Alexandre Courbot <acourbot at nvidia.com>
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 fewer bugs and can improve the
generated code.
Introduce the `Alignment` type, inspired by the nightly Rust type of the
same name and providing the same interface, and a new `Alignable` trait
allowing unsigned integers to be aligned up or down.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
rust/kernel/lib.rs | 1 +
rust/kernel/ptr.rs | 218 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 219 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index
ed53169e795c0badf548025a57f946fa18bc73e3..65586b8bd3f7ddef5f398b12796716fc192c507e
100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -110,6 +110,7 @@
pub mod platform;
pub mod prelude;
pub mod print;
+pub mod ptr;
pub mod rbtree;
pub mod regulator;
pub mod revocable;
diff --git a/rust/kernel/ptr.rs b/rust/kernel/ptr.rs
new file mode 100644
index
0000000000000000000000000000000000000000..b2b3ea18bb2782c8ccb69f2ccce3fd217879c7b5
--- /dev/null
+++ b/rust/kernel/ptr.rs
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Types and functions to work with pointers and addresses.
+
+use core::fmt::Debug;
+use core::num::NonZero;
+
+use crate::build_assert;
+
+/// Type representing an alignment, which is always a power of two.
+///
+/// It is used to validate that a given value is a valid alignment, and to
perform masking and
+/// alignment operations.
+///
+/// TODO: Temporary substitute for the [`Alignment`] nightly type from the
standard library, and 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` is 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 {
+ Self::new(align_of::<T>())
+ }
+
+ /// 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 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> {
+ // Allow the compiler to know that the value is indeed a power of two.
This can help
+ // optimize some operations down the line, like e.g. replacing
divisions by bit shifts.
+ 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 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 the mask for this alignment.
+ ///
+ /// This is equivalent 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 {
+ // No underflow can occur as the alignment is guaranteed to be a power
of two, and thus is
+ // non-zero.
+ !(self.as_usize() - 1)
+ }
+}
+
+/// Trait for items that can be aligned against an [`Alignment`].
+pub trait Alignable: Sized {
+ /// Aligns `self` down to `alignment`.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::{Alignable, Alignment};
+ ///
+ /// assert_eq!(0x2f_usize.align_down(Alignment::new(0x10)), 0x20);
+ /// assert_eq!(0x30usize.align_down(Alignment::new(0x10)), 0x30);
+ /// assert_eq!(0xf0u8.align_down(Alignment::new(0x1000)), 0x0);
+ /// ```
+ fn align_down(self, alignment: Alignment) -> Self;
+
+ /// Aligns `self` up to `alignment`, returning `None` if aligning would
result in an overflow.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::ptr::{Alignable, Alignment};
+ ///
+ /// assert_eq!(0x4fusize.align_up(Alignment::new(0x10)), Some(0x50));
+ /// assert_eq!(0x40usize.align_up(Alignment::new(0x10)), Some(0x40));
+ /// assert_eq!(0x0usize.align_up(Alignment::new(0x10)), Some(0x0));
+ /// assert_eq!(u8::MAX.align_up(Alignment::new(0x10)), None);
+ /// assert_eq!(0x10u8.align_up(Alignment::new(0x100)), None);
+ /// assert_eq!(0x0u8.align_up(Alignment::new(0x100)), Some(0x0));
+ /// ```
+ fn align_up(self, alignment: Alignment) -> Option<Self>;
+}
+
+/// Implement [`Alignable`] for unsigned integer types.
+macro_rules! impl_alignable_uint {
+ ($($t:ty),*) => {
+ $(
+ impl Alignable for $t {
+ #[inline(always)]
+ fn align_down(self, alignment: Alignment) -> Self {
+ // The operands of `&` need to be of the same type so
convert the alignment to
+ // `Self`. This means we need to compute the mask ourselves.
+
::core::num::NonZero::<Self>::try_from(alignment.as_nonzero())
+ .map(|align| self & !(align.get() - 1))
+ // An alignment larger than `Self` always aligns down to
`0`.
+ .unwrap_or(0)
+ }
+
+ #[inline(always)]
+ fn align_up(self, alignment: Alignment) -> Option<Self> {
+ let aligned_down = self.align_down(alignment);
+ if self == aligned_down {
+ Some(aligned_down)
+ } else {
+ Self::try_from(alignment.as_usize())
+ .ok()
+ .and_then(|align| aligned_down.checked_add(align))
+ }
+ }
+ }
+ )*
+ };
+}
+
+impl_alignable_uint!(u8, u16, u32, u64, usize);
--
2.50.1
Alexandre Courbot
2025-Aug-12 04:23 UTC
[PATCH v3 2/2] 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/vbios.rs | 4 ++--
3 files changed, 5 insertions(+), 6 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..344cb5d2d919c456212d60d7210667de8041f812
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::{Alignable, 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 = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) -
FRTS_SIZE;
frts_base..frts_base + FRTS_SIZE
};
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index
5b5d9f38cbb3a6b1c374c1e0eee2509eb8d5660c..680d4d961593d7cb88bdd58c6c1dbf77b51b6d7f
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::{Alignable, 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 =
self.current_offset.align_up(Alignment::new(512))?;
Some(Ok(full_image))
}
--
2.50.1