[PATCH v3 0/2] rust, nova-core: add DeviceSize trait for SZ_* constants

[PATCH v3 0/2] rust, nova-core: add DeviceSize trait for SZ_* constants

[John Hubbard]

Changes in v3:
  * Dropped the Alignment::from_u64() patch.

  * Reworked define_sizes! macro to accept target types as arguments
    (define_sizes!(u32, u64, usize)) using recursive macro arms,
    instead of hardcoding impls for u32 and u64. Overflow checking
    uses u128 casts so the same assert works for any ("reasonable")
    target type.

  * Added usize to the DeviceSize implementations, so the trait
    covers u32, u64, and usize.

  * Hex values for each constant are now proper doc-comments
    (forwarded via macro meta), instead of inline comments.

  * Rebased onto latest drm-rust-next.

Changes in v2:
  * Replaced flat SZ_*_U64 constants with a DeviceSize trait that
    provides SZ_* as associated constants on u32 and u64.

  * A define_sizes! macro generates everything from one list of names.

  * Added Alignment::from_u64() so alignment values can be constructed
    from DeviceSize constants without falling back to usize variants.

  * Rebased onto drm-rust-next. No longer depends on the Blackwell
    patchset.

v1 is here:
    https://lore.kernel.org/20260310023145.120037-1-jhubbard@nvidia.com

John Hubbard (2):
  rust: sizes: add DeviceSize trait for device address space constants
  gpu: nova-core: use DeviceSize trait for u64 size constants

 drivers/gpu/nova-core/fb.rs     |  21 ++--
 drivers/gpu/nova-core/gsp/fw.rs |  15 ++-
 drivers/gpu/nova-core/regs.rs   |   7 +-
 rust/kernel/sizes.rs            | 166 +++++++++++++++++++++++---------
 4 files changed, 141 insertions(+), 68 deletions(-)


base-commit: 7c50d748b4a635bc39802ea3f6b120e66b1b9067
-- 
2.53.0

[PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[John Hubbard]

The SZ_* constants are usize, matching the CPU pointer width. But
device address spaces have their own widths (32-bit MMIO windows,
64-bit GPU framebuffers, etc.), so drivers end up casting these
constants with SZ_1M as u64 or helper functions. This adds
boilerplate with no safety benefit.

Add a DeviceSize trait with associated SZ_* constants, implemented
for u32, u64, and usize. With the trait in scope, callers write
u64::SZ_1M or u32::SZ_4K to get the constant in their device's
native width. All SZ_* values fit in a u32, so every implementation
is lossless. Each impl has a const assert to catch any future
constant that would overflow.

A define_sizes! macro generates everything from a single internal
list of names. The macro takes the target types as arguments, so
adding a new target type requires changing only the call site.

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Link: https://lore.kernel.org/all/DGB9G697GSWO.3VBFGU5MKFPMR@kernel.org/
Link: https://lore.kernel.org/all/DGHI8WRKBQS9.38910L6FIIZTE@kernel.org/
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 rust/kernel/sizes.rs | 166 +++++++++++++++++++++++++++++++------------
 1 file changed, 122 insertions(+), 44 deletions(-)

diff --git a/rust/kernel/sizes.rs b/rust/kernel/sizes.rs
index 661e680d9330..cbf0a6c0a757 100644
--- a/rust/kernel/sizes.rs
+++ b/rust/kernel/sizes.rs
@@ -3,48 +3,126 @@
 //! Commonly used sizes.
 //!
 //! C headers: [`include/linux/sizes.h`](srctree/include/linux/sizes.h).
+//!
+//! The top-level `SZ_*` constants are [`usize`]-typed, for use in kernel page
+//! arithmetic and similar CPU-side work.
+//!
+//! The [`DeviceSize`] trait provides the same constants as associated constants
+//! on [`u32`], [`u64`], and [`usize`], for use in device address spaces where
+//! the address width depends on the hardware. Device drivers frequently need
+//! these constants as [`u64`] (or [`u32`]) rather than [`usize`], because
+//! device address spaces are sized independently of the CPU pointer width.
+//!
+//! # Examples
+//!
+//! ```
+//! use kernel::sizes::{DeviceSize, SZ_1M};
+//!
+//! // usize constant (CPU-side)
+//! let num_pages_in_1m: usize = SZ_1M / kernel::page::PAGE_SIZE;
+//!
+//! // Device-side constant via the trait
+//! let heap_size: u64 = 14 * u64::SZ_1M;
+//! let small: u32 = u32::SZ_4K;
+//! ```
+
+macro_rules! define_sizes {
+    ($($type:ty),* $(,)?) => {
+        define_sizes!(@internal [$($type),*]
+            /// 0x0000_0400
+            SZ_1K,
+            /// 0x0000_0800
+            SZ_2K,
+            /// 0x0000_1000
+            SZ_4K,
+            /// 0x0000_2000
+            SZ_8K,
+            /// 0x0000_4000
+            SZ_16K,
+            /// 0x0000_8000
+            SZ_32K,
+            /// 0x0001_0000
+            SZ_64K,
+            /// 0x0002_0000
+            SZ_128K,
+            /// 0x0004_0000
+            SZ_256K,
+            /// 0x0008_0000
+            SZ_512K,
+            /// 0x0010_0000
+            SZ_1M,
+            /// 0x0020_0000
+            SZ_2M,
+            /// 0x0040_0000
+            SZ_4M,
+            /// 0x0080_0000
+            SZ_8M,
+            /// 0x0100_0000
+            SZ_16M,
+            /// 0x0200_0000
+            SZ_32M,
+            /// 0x0400_0000
+            SZ_64M,
+            /// 0x0800_0000
+            SZ_128M,
+            /// 0x1000_0000
+            SZ_256M,
+            /// 0x2000_0000
+            SZ_512M,
+            /// 0x4000_0000
+            SZ_1G,
+            /// 0x8000_0000
+            SZ_2G,
+        );
+    };
+
+    (@internal [$($type:ty),*] $($names_and_metas:tt)*) => {
+        define_sizes!(@consts_and_trait $($names_and_metas)*);
+        define_sizes!(@impls [$($type),*] $($names_and_metas)*);
+    };
+
+    (@consts_and_trait $($(#[$meta:meta])* $name:ident,)*) => {
+        $(
+            $(#[$meta])*
+            pub const $name: usize = bindings::$name as usize;
+        )*
+
+        /// Size constants for device address spaces.
+        ///
+        /// Implemented for [`u32`], [`u64`], and [`usize`] so drivers can
+        /// choose the width that matches their hardware. All `SZ_*` values fit
+        /// in a [`u32`], so all implementations are lossless.
+        ///
+        /// # Examples
+        ///
+        /// ```
+        /// use kernel::sizes::DeviceSize;
+        ///
+        /// let gpu_heap: u64 = 14 * u64::SZ_1M;
+        /// let mmio_window: u32 = u32::SZ_16M;
+        /// ```
+        pub trait DeviceSize {
+            $(
+                $(#[$meta])*
+                const $name: Self;
+            )*
+        }
+    };
+
+    (@impls [] $($(#[$meta:meta])* $name:ident,)*) => {};
+
+    (@impls [$first:ty $(, $rest:ty)*] $($(#[$meta:meta])* $name:ident,)*) => {
+        impl DeviceSize for $first {
+            $(
+                const $name: Self = {
+                    assert!((self::$name as u128) <= (<$first>::MAX as u128));
+                    self::$name as $first
+                };
+            )*
+        }
+
+        define_sizes!(@impls [$($rest),*] $($(#[$meta])* $name,)*);
+    };
+}
 
-/// 0x00000400
-pub const SZ_1K: usize = bindings::SZ_1K as usize;
-/// 0x00000800
-pub const SZ_2K: usize = bindings::SZ_2K as usize;
-/// 0x00001000
-pub const SZ_4K: usize = bindings::SZ_4K as usize;
-/// 0x00002000
-pub const SZ_8K: usize = bindings::SZ_8K as usize;
-/// 0x00004000
-pub const SZ_16K: usize = bindings::SZ_16K as usize;
-/// 0x00008000
-pub const SZ_32K: usize = bindings::SZ_32K as usize;
-/// 0x00010000
-pub const SZ_64K: usize = bindings::SZ_64K as usize;
-/// 0x00020000
-pub const SZ_128K: usize = bindings::SZ_128K as usize;
-/// 0x00040000
-pub const SZ_256K: usize = bindings::SZ_256K as usize;
-/// 0x00080000
-pub const SZ_512K: usize = bindings::SZ_512K as usize;
-/// 0x00100000
-pub const SZ_1M: usize = bindings::SZ_1M as usize;
-/// 0x00200000
-pub const SZ_2M: usize = bindings::SZ_2M as usize;
-/// 0x00400000
-pub const SZ_4M: usize = bindings::SZ_4M as usize;
-/// 0x00800000
-pub const SZ_8M: usize = bindings::SZ_8M as usize;
-/// 0x01000000
-pub const SZ_16M: usize = bindings::SZ_16M as usize;
-/// 0x02000000
-pub const SZ_32M: usize = bindings::SZ_32M as usize;
-/// 0x04000000
-pub const SZ_64M: usize = bindings::SZ_64M as usize;
-/// 0x08000000
-pub const SZ_128M: usize = bindings::SZ_128M as usize;
-/// 0x10000000
-pub const SZ_256M: usize = bindings::SZ_256M as usize;
-/// 0x20000000
-pub const SZ_512M: usize = bindings::SZ_512M as usize;
-/// 0x40000000
-pub const SZ_1G: usize = bindings::SZ_1G as usize;
-/// 0x80000000
-pub const SZ_2G: usize = bindings::SZ_2G as usize;
+define_sizes!(u32, u64, usize);
-- 
2.53.0

[PATCH v3 2/2] gpu: nova-core: use DeviceSize trait for u64 size constants

[John Hubbard]

Replace manual usize-to-u64 conversions of SZ_* constants with the
DeviceSize trait's associated constants on u64. With the DeviceSize
trait in scope, u64::SZ_1M replaces usize_as_u64(SZ_1M) and similar.

This removes several now-unused imports: usize_as_u64, FromSafeCast,
and individual SZ_* type-level constants.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs     | 21 +++++++++------------
 drivers/gpu/nova-core/gsp/fw.rs | 15 +++++++--------
 drivers/gpu/nova-core/regs.rs   |  7 +++----
 3 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index bdd5eed760e1..5c304fc03467 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -24,11 +24,8 @@
     firmware::gsp::GspFirmware,
     gpu::Chipset,
     gsp,
-    num::{
-        usize_as_u64,
-        FromSafeCast, //
-    },
-    regs,
+    num::FromSafeCast,
+    regs, //
 };
 
 mod hal;
@@ -127,8 +124,8 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if f.alternate() {
             let size = self.len();
 
-            if size < usize_as_u64(SZ_1M) {
-                let size_kib = size / usize_as_u64(SZ_1K);
+            if size < u64::SZ_1M {
+                let size_kib = size / u64::SZ_1K;
                 f.write_fmt(fmt!(
                     "{:#x}..{:#x} ({} KiB)",
                     self.0.start,
@@ -136,7 +133,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                     size_kib
                 ))
             } else {
-                let size_mib = size / usize_as_u64(SZ_1M);
+                let size_mib = size / u64::SZ_1M;
                 f.write_fmt(fmt!(
                     "{:#x}..{:#x} ({} MiB)",
                     self.0.start,
@@ -186,7 +183,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
 
         let vga_workspace = {
             let vga_base = {
-                const NV_PRAMIN_SIZE: u64 = usize_as_u64(SZ_1M);
+                const NV_PRAMIN_SIZE: u64 = u64::SZ_1M;
                 let base = fb.end - NV_PRAMIN_SIZE;
 
                 if hal.supports_display(bar) {
@@ -196,7 +193,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
                     {
                         Some(addr) => {
                             if addr < base {
-                                const VBIOS_WORKSPACE_SIZE: u64 = usize_as_u64(SZ_128K);
+                                const VBIOS_WORKSPACE_SIZE: u64 = u64::SZ_128K;
 
                                 // Point workspace address to end of framebuffer.
                                 fb.end - VBIOS_WORKSPACE_SIZE
@@ -216,7 +213,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
 
         let frts = {
             const FRTS_DOWN_ALIGN: Alignment = Alignment::new::<SZ_128K>();
-            const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
+            const FRTS_SIZE: u64 = u64::SZ_1M;
             let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
 
             FbRange(frts_base..frts_base + FRTS_SIZE)
@@ -256,7 +253,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
         };
 
         let heap = {
-            const HEAP_SIZE: u64 = usize_as_u64(SZ_1M);
+            const HEAP_SIZE: u64 = u64::SZ_1M;
 
             FbRange(wpr2.start - HEAP_SIZE..wpr2.start)
         };
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 0c8a74f0e8ac..5745aa50b00c 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -17,8 +17,8 @@
         KnownSize, //
     },
     sizes::{
-        SZ_128K,
-        SZ_1M, //
+        DeviceSize,
+        SZ_128K, //
     },
     transmute::{
         AsBytes,
@@ -123,7 +123,7 @@ fn client_alloc_size() -> u64 {
     /// Returns the amount of memory to reserve for management purposes for a framebuffer of size
     /// `fb_size`.
     fn management_overhead(fb_size: u64) -> u64 {
-        let fb_size_gb = fb_size.div_ceil(u64::from_safe_cast(kernel::sizes::SZ_1G));
+        let fb_size_gb = fb_size.div_ceil(u64::SZ_1G);
 
         u64::from(bindings::GSP_FW_HEAP_PARAM_SIZE_PER_GB_FB)
             .saturating_mul(fb_size_gb)
@@ -145,9 +145,8 @@ impl LibosParams {
     const LIBOS2: LibosParams = LibosParams {
         carveout_size: num::u32_as_u64(bindings::GSP_FW_HEAP_PARAM_OS_SIZE_LIBOS2),
         allowed_heap_size: num::u32_as_u64(bindings::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MIN_MB)
-            * num::usize_as_u64(SZ_1M)
-            ..num::u32_as_u64(bindings::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MAX_MB)
-                * num::usize_as_u64(SZ_1M),
+            * u64::SZ_1M
+            ..num::u32_as_u64(bindings::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MAX_MB) * u64::SZ_1M,
     };
 
     /// Version 3 of the GSP LIBOS (GA102+)
@@ -155,9 +154,9 @@ impl LibosParams {
         carveout_size: num::u32_as_u64(bindings::GSP_FW_HEAP_PARAM_OS_SIZE_LIBOS3_BAREMETAL),
         allowed_heap_size: num::u32_as_u64(
             bindings::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS3_BAREMETAL_MIN_MB,
-        ) * num::usize_as_u64(SZ_1M)
+        ) * u64::SZ_1M
             ..num::u32_as_u64(bindings::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS3_BAREMETAL_MAX_MB)
-                * num::usize_as_u64(SZ_1M),
+                * u64::SZ_1M,
     };
 
     /// Returns the libos parameters corresponding to `chipset`.
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 2f171a4ff9ba..28d69e33f7b1 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -7,6 +7,7 @@
         Io, //
     },
     prelude::*,
+    sizes::DeviceSize,
     time, //
 };
 
@@ -30,7 +31,6 @@
         Architecture,
         Chipset, //
     },
-    num::FromSafeCast,
 };
 
 // PMC
@@ -150,8 +150,7 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
 impl NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE {
     /// Returns the usable framebuffer size, in bytes.
     pub(crate) fn usable_fb_size(self) -> u64 {
-        let size = (u64::from(self.lower_mag()) << u64::from(self.lower_scale()))
-            * u64::from_safe_cast(kernel::sizes::SZ_1M);
+        let size = (u64::from(self.lower_mag()) << u64::from(self.lower_scale())) * u64::SZ_1M;
 
         if self.ecc_mode_enabled() {
             // Remove the amount of memory reserved for ECC (one per 16 units).
@@ -241,7 +240,7 @@ pub(crate) fn completed(self) -> bool {
 impl NV_USABLE_FB_SIZE_IN_MB {
     /// Returns the usable framebuffer size, in bytes.
     pub(crate) fn usable_fb_size(self) -> u64 {
-        u64::from(self.value()) * u64::from_safe_cast(kernel::sizes::SZ_1M)
+        u64::from(self.value()) * u64::SZ_1M
     }
 }
 
-- 
2.53.0

Re: [PATCH v3 0/2] rust, nova-core: add DeviceSize trait for SZ_* constants

[Eliot Courtney]

On Wed Apr 1, 2026 at 7:43 AM JST, John Hubbard wrote:
> Changes in v3:
>   * Dropped the Alignment::from_u64() patch.
>
>   * Reworked define_sizes! macro to accept target types as arguments
>     (define_sizes!(u32, u64, usize)) using recursive macro arms,
>     instead of hardcoding impls for u32 and u64. Overflow checking
>     uses u128 casts so the same assert works for any ("reasonable")
>     target type.
>
>   * Added usize to the DeviceSize implementations, so the trait
>     covers u32, u64, and usize.
>
>   * Hex values for each constant are now proper doc-comments
>     (forwarded via macro meta), instead of inline comments.
>
>   * Rebased onto latest drm-rust-next.
>
> Changes in v2:
>   * Replaced flat SZ_*_U64 constants with a DeviceSize trait that
>     provides SZ_* as associated constants on u32 and u64.
>
>   * A define_sizes! macro generates everything from one list of names.
>
>   * Added Alignment::from_u64() so alignment values can be constructed
>     from DeviceSize constants without falling back to usize variants.
>
>   * Rebased onto drm-rust-next. No longer depends on the Blackwell
>     patchset.
>
> v1 is here:
>     https://lore.kernel.org/20260310023145.120037-1-jhubbard@nvidia.com
>
> John Hubbard (2):
>   rust: sizes: add DeviceSize trait for device address space constants
>   gpu: nova-core: use DeviceSize trait for u64 size constants
>
>  drivers/gpu/nova-core/fb.rs     |  21 ++--
>  drivers/gpu/nova-core/gsp/fw.rs |  15 ++-
>  drivers/gpu/nova-core/regs.rs   |   7 +-
>  rust/kernel/sizes.rs            | 166 +++++++++++++++++++++++---------
>  4 files changed, 141 insertions(+), 68 deletions(-)
>
>
> base-commit: 7c50d748b4a635bc39802ea3f6b120e66b1b9067

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[Alice Ryhl]

On Tue, Mar 31, 2026 at 03:43:18PM -0700, John Hubbard wrote:
> The SZ_* constants are usize, matching the CPU pointer width. But
> device address spaces have their own widths (32-bit MMIO windows,
> 64-bit GPU framebuffers, etc.), so drivers end up casting these
> constants with SZ_1M as u64 or helper functions. This adds
> boilerplate with no safety benefit.
> 
> Add a DeviceSize trait with associated SZ_* constants, implemented
> for u32, u64, and usize. With the trait in scope, callers write
> u64::SZ_1M or u32::SZ_4K to get the constant in their device's
> native width. All SZ_* values fit in a u32, so every implementation
> is lossless. Each impl has a const assert to catch any future
> constant that would overflow.
> 
> A define_sizes! macro generates everything from a single internal
> list of names. The macro takes the target types as arguments, so
> adding a new target type requires changing only the call site.
> 
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Link: https://lore.kernel.org/all/DGB9G697GSWO.3VBFGU5MKFPMR@kernel.org/
> Link: https://lore.kernel.org/all/DGHI8WRKBQS9.38910L6FIIZTE@kernel.org/
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>

The name `DeviceSize` seems overly specific to the use-case you had. It
also makes it sound like something you would implement *for* the device
type rather than for the integer type. Why not name it something more
generic such as SizeConstants?

Alice

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[Miguel Ojeda]

On Wed, Apr 1, 2026 at 12:43 AM John Hubbard <jhubbard@nvidia.com> wrote:
>
> +//! // usize constant (CPU-side)

Some nits below -- possibly for apply time.

Markdown and period:

    //! // `usize` constant (CPU-side).

> +//! let num_pages_in_1m: usize = SZ_1M / kernel::page::PAGE_SIZE;

Perhaps import `PAGE_SIZE` since we are doing imports above anyway?

> +//! // Device-side constant via the trait

Period at the end.

> +//! let heap_size: u64 = 14 * u64::SZ_1M;
> +//! let small: u32 = u32::SZ_4K;

I guess the resulting types here are to make it clearer; or do we
expect people to write the sizes for this? i.e. since part of the
advantage of using the constants is that you get them already typed, I
wonder if we should write the code as we would normally expect it to
be written.

Also, I always consider in examples whether to add `assert_eq!`s, but
I am ambivalent for this case.

> +            /// 0x0000_0400
> +            SZ_1K,

This is consistent with the other constants, but I wonder if there was
a reason to not do something like:

    /// `0x0000_0400`.

Anyway, all these is minor or can be fixed on apply.

These constants are meant to be useful soon, so we could do the same
as with the other patch, i.e. I could pick it already it depending on
the rest of the discussion (especially if we confirm that the user
patch is what we want).

Thanks!

Cheers,
Miguel

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[John Hubbard]

On 4/1/26 2:46 AM, Alice Ryhl wrote:
> On Tue, Mar 31, 2026 at 03:43:18PM -0700, John Hubbard wrote:
>> The SZ_* constants are usize, matching the CPU pointer width. But
>> device address spaces have their own widths (32-bit MMIO windows,
>> 64-bit GPU framebuffers, etc.), so drivers end up casting these
>> constants with SZ_1M as u64 or helper functions. This adds
>> boilerplate with no safety benefit.
>>
>> Add a DeviceSize trait with associated SZ_* constants, implemented
>> for u32, u64, and usize. With the trait in scope, callers write
>> u64::SZ_1M or u32::SZ_4K to get the constant in their device's
>> native width. All SZ_* values fit in a u32, so every implementation
>> is lossless. Each impl has a const assert to catch any future
>> constant that would overflow.
>>
>> A define_sizes! macro generates everything from a single internal
>> list of names. The macro takes the target types as arguments, so
>> adding a new target type requires changing only the call site.
>>
>> Suggested-by: Danilo Krummrich <dakr@kernel.org>
>> Link: https://lore.kernel.org/all/DGB9G697GSWO.3VBFGU5MKFPMR@kernel.org/
>> Link: https://lore.kernel.org/all/DGHI8WRKBQS9.38910L6FIIZTE@kernel.org/
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> 
> The name `DeviceSize` seems overly specific to the use-case you had. It

Yes, actually this name has been worrying me from the start. Because
it is not necessary to tie it, conceptually, to devices at all.

> also makes it sound like something you would implement *for* the device
> type rather than for the integer type. Why not name it something more
> generic such as SizeConstants?

Yes, thanks, I do think SizeConstants is more accurate.

I'm inclined to make that change, unless someone else tells me not
to...let's see.


thanks,
-- 
John Hubbard

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[John Hubbard]

On 4/1/26 3:16 AM, Miguel Ojeda wrote:
> On Wed, Apr 1, 2026 at 12:43 AM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> +//! // usize constant (CPU-side)
> 
> Some nits below -- possibly for apply time.

I've fixed all of them in a pending v4, which I think I'll need
to post in order to rename the trait, as per the other thread with
Alice.

Sorry that these slipped in, there are three separate systems
that should have caught them, each of which requires a bit of
maintenance:

* My brain: still hasn't emotionally absorbed these rules yet,
especially the backtick thing. Work in progress... :)

* checkpatch.pl: it's still delighted to bless all of these
patches with a "has no obvious style problems and is ready for
submission, hooray!".

I could update checkpatch, but...I wonder if it's really
the right tool. I forget what we decided lately, but I vaguely
recall wanting the Rust toolchain to help.

* Local AI reviewer tool(s): this at least I can quickly fix: I've
added a few lines to the rules.

thanks,
-- 
John Hubbard
> 
> Markdown and period:
> 
>     //! // `usize` constant (CPU-side).
> 
>> +//! let num_pages_in_1m: usize = SZ_1M / kernel::page::PAGE_SIZE;
> 
> Perhaps import `PAGE_SIZE` since we are doing imports above anyway?
> 
>> +//! // Device-side constant via the trait
> 
> Period at the end.
> 
>> +//! let heap_size: u64 = 14 * u64::SZ_1M;
>> +//! let small: u32 = u32::SZ_4K;
> 
> I guess the resulting types here are to make it clearer; or do we
> expect people to write the sizes for this? i.e. since part of the
> advantage of using the constants is that you get them already typed, I
> wonder if we should write the code as we would normally expect it to
> be written.
> 
> Also, I always consider in examples whether to add `assert_eq!`s, but
> I am ambivalent for this case.
> 
>> +            /// 0x0000_0400
>> +            SZ_1K,
> 
> This is consistent with the other constants, but I wonder if there was
> a reason to not do something like:
> 
>     /// `0x0000_0400`.
> 
> Anyway, all these is minor or can be fixed on apply.
> 
> These constants are meant to be useful soon, so we could do the same
> as with the other patch, i.e. I could pick it already it depending on
> the rest of the discussion (especially if we confirm that the user
> patch is what we want).
> 
> Thanks!
> 
> Cheers,
> Miguel

Re: [PATCH v3 0/2] rust, nova-core: add DeviceSize trait for SZ_* constants

[John Hubbard]

On 4/1/26 12:01 AM, Eliot Courtney wrote:
> On Wed Apr 1, 2026 at 7:43 AM JST, John Hubbard wrote:
>> Changes in v3:
>>   * Dropped the Alignment::from_u64() patch.
>>
>>   * Reworked define_sizes! macro to accept target types as arguments
>>     (define_sizes!(u32, u64, usize)) using recursive macro arms,
>>     instead of hardcoding impls for u32 and u64. Overflow checking
>>     uses u128 casts so the same assert works for any ("reasonable")
>>     target type.
>>
>>   * Added usize to the DeviceSize implementations, so the trait
>>     covers u32, u64, and usize.
>>
>>   * Hex values for each constant are now proper doc-comments
>>     (forwarded via macro meta), instead of inline comments.
>>
>>   * Rebased onto latest drm-rust-next.
>>
>> Changes in v2:
>>   * Replaced flat SZ_*_U64 constants with a DeviceSize trait that
>>     provides SZ_* as associated constants on u32 and u64.
>>
>>   * A define_sizes! macro generates everything from one list of names.
>>
>>   * Added Alignment::from_u64() so alignment values can be constructed
>>     from DeviceSize constants without falling back to usize variants.
>>
>>   * Rebased onto drm-rust-next. No longer depends on the Blackwell
>>     patchset.
>>
>> v1 is here:
>>     https://lore.kernel.org/20260310023145.120037-1-jhubbard@nvidia.com
>>
>> John Hubbard (2):
>>   rust: sizes: add DeviceSize trait for device address space constants
>>   gpu: nova-core: use DeviceSize trait for u64 size constants
>>
>>  drivers/gpu/nova-core/fb.rs     |  21 ++--
>>  drivers/gpu/nova-core/gsp/fw.rs |  15 ++-
>>  drivers/gpu/nova-core/regs.rs   |   7 +-
>>  rust/kernel/sizes.rs            | 166 +++++++++++++++++++++++---------
>>  4 files changed, 141 insertions(+), 68 deletions(-)
>>
>>
>> base-commit: 7c50d748b4a635bc39802ea3f6b120e66b1b9067
> 
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
> 

Thanks for the review! 

thanks,
-- 
John Hubbard

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[Danilo Krummrich]

On Wed Apr 1, 2026 at 10:22 PM CEST, John Hubbard wrote:
> On 4/1/26 2:46 AM, Alice Ryhl wrote:
>> On Tue, Mar 31, 2026 at 03:43:18PM -0700, John Hubbard wrote:
>>> The SZ_* constants are usize, matching the CPU pointer width. But
>>> device address spaces have their own widths (32-bit MMIO windows,
>>> 64-bit GPU framebuffers, etc.), so drivers end up casting these
>>> constants with SZ_1M as u64 or helper functions. This adds
>>> boilerplate with no safety benefit.
>>>
>>> Add a DeviceSize trait with associated SZ_* constants, implemented
>>> for u32, u64, and usize. With the trait in scope, callers write
>>> u64::SZ_1M or u32::SZ_4K to get the constant in their device's
>>> native width. All SZ_* values fit in a u32, so every implementation
>>> is lossless. Each impl has a const assert to catch any future
>>> constant that would overflow.
>>>
>>> A define_sizes! macro generates everything from a single internal
>>> list of names. The macro takes the target types as arguments, so
>>> adding a new target type requires changing only the call site.
>>>
>>> Suggested-by: Danilo Krummrich <dakr@kernel.org>
>>> Link: https://lore.kernel.org/all/DGB9G697GSWO.3VBFGU5MKFPMR@kernel.org/
>>> Link: https://lore.kernel.org/all/DGHI8WRKBQS9.38910L6FIIZTE@kernel.org/
>>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> 
>> The name `DeviceSize` seems overly specific to the use-case you had. It
>
> Yes, actually this name has been worrying me from the start. Because
> it is not necessary to tie it, conceptually, to devices at all.
>
>> also makes it sound like something you would implement *for* the device
>> type rather than for the integer type. Why not name it something more
>> generic such as SizeConstants?
>
> Yes, thanks, I do think SizeConstants is more accurate.
>
> I'm inclined to make that change, unless someone else tells me not
> to...let's see.

I think I brought up the name DeviceSize when I proposed this.

The reason is that when I proposed this I was thinking of it as a marker trait
for "complex" types around u32, u64, etc. that we can use in DRM APIs (or any
other device centric API) though generics.

For instance, instead of GpuVm<T>::vm_start() -> u64, it could be
GpuVm<T, V: DeviceSize>::vm_start() -> V.

Another example would be buddy::Block<V: DeviceSize>::offset() -> V instead of
buddy::Block::offset() -> u64.

This way you can interact with the API with the actual device native size.

That said, I'm perfectly fine renaming this to something else; the "device
semantics" comes from the API using the type, not the type itself.

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[John Hubbard]

On 4/1/26 2:20 PM, Danilo Krummrich wrote:
> On Wed Apr 1, 2026 at 10:22 PM CEST, John Hubbard wrote:
>> On 4/1/26 2:46 AM, Alice Ryhl wrote:
>>> On Tue, Mar 31, 2026 at 03:43:18PM -0700, John Hubbard wrote:
...
>>> The name `DeviceSize` seems overly specific to the use-case you had. It
>>
>> Yes, actually this name has been worrying me from the start. Because
>> it is not necessary to tie it, conceptually, to devices at all.
>>
>>> also makes it sound like something you would implement *for* the device
>>> type rather than for the integer type. Why not name it something more
>>> generic such as SizeConstants?
>>
>> Yes, thanks, I do think SizeConstants is more accurate.
>>
>> I'm inclined to make that change, unless someone else tells me not
>> to...let's see.
> 
> I think I brought up the name DeviceSize when I proposed this.

I let this patchset linger so long in my backlog that I forgot the
trait's naming provenance. :)

> 
> The reason is that when I proposed this I was thinking of it as a marker trait
> for "complex" types around u32, u64, etc. that we can use in DRM APIs (or any
> other device centric API) though generics.
> 
> For instance, instead of GpuVm<T>::vm_start() -> u64, it could be
> GpuVm<T, V: DeviceSize>::vm_start() -> V.
> 
> Another example would be buddy::Block<V: DeviceSize>::offset() -> V instead of
> buddy::Block::offset() -> u64.
> 
> This way you can interact with the API with the actual device native size.

Yes. But it's an uneasy fit, somehow. I can (again) see the idea, but
it's also true that SizeConstants is something that is generally
desirable elsewhere, too.

> 
> That said, I'm perfectly fine renaming this to something else; the "device

Excellent!

> semantics" comes from the API using the type, not the type itself.

That's exactly where I landed, too. And of course, when naming APIs,
it's best to pick names that describe what the API provides, rather
than to (often imperfectly) predict what current and future callers
may do. 


thanks,
-- 
John Hubbard

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[Alexandre Courbot]

On Thu Apr 2, 2026 at 6:20 AM JST, Danilo Krummrich wrote:
> On Wed Apr 1, 2026 at 10:22 PM CEST, John Hubbard wrote:
>> On 4/1/26 2:46 AM, Alice Ryhl wrote:
>>> On Tue, Mar 31, 2026 at 03:43:18PM -0700, John Hubbard wrote:
>>>> The SZ_* constants are usize, matching the CPU pointer width. But
>>>> device address spaces have their own widths (32-bit MMIO windows,
>>>> 64-bit GPU framebuffers, etc.), so drivers end up casting these
>>>> constants with SZ_1M as u64 or helper functions. This adds
>>>> boilerplate with no safety benefit.
>>>>
>>>> Add a DeviceSize trait with associated SZ_* constants, implemented
>>>> for u32, u64, and usize. With the trait in scope, callers write
>>>> u64::SZ_1M or u32::SZ_4K to get the constant in their device's
>>>> native width. All SZ_* values fit in a u32, so every implementation
>>>> is lossless. Each impl has a const assert to catch any future
>>>> constant that would overflow.
>>>>
>>>> A define_sizes! macro generates everything from a single internal
>>>> list of names. The macro takes the target types as arguments, so
>>>> adding a new target type requires changing only the call site.
>>>>
>>>> Suggested-by: Danilo Krummrich <dakr@kernel.org>
>>>> Link: https://lore.kernel.org/all/DGB9G697GSWO.3VBFGU5MKFPMR@kernel.org/
>>>> Link: https://lore.kernel.org/all/DGHI8WRKBQS9.38910L6FIIZTE@kernel.org/
>>>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>>> 
>>> The name `DeviceSize` seems overly specific to the use-case you had. It
>>
>> Yes, actually this name has been worrying me from the start. Because
>> it is not necessary to tie it, conceptually, to devices at all.
>>
>>> also makes it sound like something you would implement *for* the device
>>> type rather than for the integer type. Why not name it something more
>>> generic such as SizeConstants?
>>
>> Yes, thanks, I do think SizeConstants is more accurate.
>>
>> I'm inclined to make that change, unless someone else tells me not
>> to...let's see.
>
> I think I brought up the name DeviceSize when I proposed this.
>
> The reason is that when I proposed this I was thinking of it as a marker trait
> for "complex" types around u32, u64, etc. that we can use in DRM APIs (or any
> other device centric API) though generics.
>
> For instance, instead of GpuVm<T>::vm_start() -> u64, it could be
> GpuVm<T, V: DeviceSize>::vm_start() -> V.

With the proposed naming this becomes `GpuVm<T, V: SizeConstants>`. Why
not just name it `Size`? Sure it's a very common word, but we have the
module to scope the name properly.

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[John Hubbard]

On 4/1/26 6:42 PM, Alexandre Courbot wrote:
> On Thu Apr 2, 2026 at 6:20 AM JST, Danilo Krummrich wrote:
>> On Wed Apr 1, 2026 at 10:22 PM CEST, John Hubbard wrote:
>>> On 4/1/26 2:46 AM, Alice Ryhl wrote:
...
>> The reason is that when I proposed this I was thinking of it as a marker trait
>> for "complex" types around u32, u64, etc. that we can use in DRM APIs (or any
>> other device centric API) though generics.
>>
>> For instance, instead of GpuVm<T>::vm_start() -> u64, it could be
>> GpuVm<T, V: DeviceSize>::vm_start() -> V.
> 
> With the proposed naming this becomes `GpuVm<T, V: SizeConstants>`. Why
> not just name it `Size`? Sure it's a very common word, but we have the
> module to scope the name properly.

I was waiting to see if anyone else weighed in.

SizeConstants accurately describes what this trait provides. Size is
too general. Again, I think it's best to name things for what they
are or what they provide. And then, if they look odd in some use case,
that's a hint to consider if that use case is precisely the best way
to compose what you want to do.

Anyway, I'm pretty sold on SizeConstants, so I'm hoping to stay with
that, are you OK with it?

thanks,
-- 
John Hubbard

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[Alexandre Courbot]

On Fri Apr 3, 2026 at 10:36 AM JST, John Hubbard wrote:
> On 4/1/26 6:42 PM, Alexandre Courbot wrote:
>> On Thu Apr 2, 2026 at 6:20 AM JST, Danilo Krummrich wrote:
>>> On Wed Apr 1, 2026 at 10:22 PM CEST, John Hubbard wrote:
>>>> On 4/1/26 2:46 AM, Alice Ryhl wrote:
> ...
>>> The reason is that when I proposed this I was thinking of it as a marker trait
>>> for "complex" types around u32, u64, etc. that we can use in DRM APIs (or any
>>> other device centric API) though generics.
>>>
>>> For instance, instead of GpuVm<T>::vm_start() -> u64, it could be
>>> GpuVm<T, V: DeviceSize>::vm_start() -> V.
>> 
>> With the proposed naming this becomes `GpuVm<T, V: SizeConstants>`. Why
>> not just name it `Size`? Sure it's a very common word, but we have the
>> module to scope the name properly.
>
> I was waiting to see if anyone else weighed in.
>
> SizeConstants accurately describes what this trait provides. Size is
> too general. Again, I think it's best to name things for what they
> are or what they provide. And then, if they look odd in some use case,
> that's a hint to consider if that use case is precisely the best way
> to compose what you want to do.
>
> Anyway, I'm pretty sold on SizeConstants, so I'm hoping to stay with
> that, are you OK with it?

Yes, as mentioned on my v4 review. I agree `SizeConstants` describes
what this trait provides currently - but was also thinking that this
trait might grow into depending on some arithmetic operations to allow
generic code to be written for anything that is a size in the general
sense.

That being said, we can also write said generic code by depending on the
required ops explicitly, so limiting the trait to providing constants
doesn't really limit us.

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[Gary Guo]

On Fri Apr 3, 2026 at 9:21 AM BST, Alexandre Courbot wrote:
> On Fri Apr 3, 2026 at 10:36 AM JST, John Hubbard wrote:
>> On 4/1/26 6:42 PM, Alexandre Courbot wrote:
>>> On Thu Apr 2, 2026 at 6:20 AM JST, Danilo Krummrich wrote:
>>>> On Wed Apr 1, 2026 at 10:22 PM CEST, John Hubbard wrote:
>>>>> On 4/1/26 2:46 AM, Alice Ryhl wrote:
>> ...
>>>> The reason is that when I proposed this I was thinking of it as a marker trait
>>>> for "complex" types around u32, u64, etc. that we can use in DRM APIs (or any
>>>> other device centric API) though generics.
>>>>
>>>> For instance, instead of GpuVm<T>::vm_start() -> u64, it could be
>>>> GpuVm<T, V: DeviceSize>::vm_start() -> V.
>>> 
>>> With the proposed naming this becomes `GpuVm<T, V: SizeConstants>`. Why
>>> not just name it `Size`? Sure it's a very common word, but we have the
>>> module to scope the name properly.
>>
>> I was waiting to see if anyone else weighed in.
>>
>> SizeConstants accurately describes what this trait provides. Size is
>> too general. Again, I think it's best to name things for what they
>> are or what they provide. And then, if they look odd in some use case,
>> that's a hint to consider if that use case is precisely the best way
>> to compose what you want to do.
>>
>> Anyway, I'm pretty sold on SizeConstants, so I'm hoping to stay with
>> that, are you OK with it?
>
> Yes, as mentioned on my v4 review. I agree `SizeConstants` describes
> what this trait provides currently - but was also thinking that this
> trait might grow into depending on some arithmetic operations to allow
> generic code to be written for anything that is a size in the general
> sense.

We can always rename it when we need to grow the trait. We can leave
bikeshedding to the future.

Best,
Gary

>
> That being said, we can also write said generic code by depending on the
> required ops explicitly, so limiting the trait to providing constants
> doesn't really limit us.

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[Danilo Krummrich]

On Fri Apr 3, 2026 at 3:36 AM CEST, John Hubbard wrote:
> On 4/1/26 6:42 PM, Alexandre Courbot wrote:
>> On Thu Apr 2, 2026 at 6:20 AM JST, Danilo Krummrich wrote:
>>> On Wed Apr 1, 2026 at 10:22 PM CEST, John Hubbard wrote:
>>>> On 4/1/26 2:46 AM, Alice Ryhl wrote:
> ...
>>> The reason is that when I proposed this I was thinking of it as a marker trait
>>> for "complex" types around u32, u64, etc. that we can use in DRM APIs (or any
>>> other device centric API) though generics.
>>>
>>> For instance, instead of GpuVm<T>::vm_start() -> u64, it could be
>>> GpuVm<T, V: DeviceSize>::vm_start() -> V.
>> 
>> With the proposed naming this becomes `GpuVm<T, V: SizeConstants>`. Why
>> not just name it `Size`? Sure it's a very common word, but we have the
>> module to scope the name properly.
>
> I was waiting to see if anyone else weighed in.
>
> SizeConstants accurately describes what this trait provides. Size is
> too general. Again, I think it's best to name things for what they
> are or what they provide. And then, if they look odd in some use case,
> that's a hint to consider if that use case is precisely the best way
> to compose what you want to do.
>
> Anyway, I'm pretty sold on SizeConstants, so I'm hoping to stay with
> that, are you OK with it?

As mentioned no complaints from my side, but I think it would be good to add a
TODO for creating those type wrappers; the name can be revisited in this context
or a separate marker trait introduced.

Re: [PATCH v3 1/2] rust: sizes: add DeviceSize trait for device address space constants

[John Hubbard]

On 4/3/26 6:07 AM, Danilo Krummrich wrote:
...
>> Anyway, I'm pretty sold on SizeConstants, so I'm hoping to stay with
>> that, are you OK with it?
> 
> As mentioned no complaints from my side, but I think it would be good to add a
> TODO for creating those type wrappers; the name can be revisited in this context
> or a separate marker trait introduced.

OK. It's a little too "future design idea" for a code TODO, but
it fits nicely into Documentation/gpu/nova/core/todo.rst, so I've
added a PATCH 3/3 for that.

thanks,
-- 
John Hubbard