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.0On 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
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
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
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
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.
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.
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
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.
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
© 2016 - 2026 Red Hat, Inc.