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

From: Alexandre Courbot

Date: Wed Apr 01 2026 - 21:42:38 EST


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@xxxxxxxxxx>
>>>> Link: https://lore.kernel.org/all/DGB9G697GSWO.3VBFGU5MKFPMR@xxxxxxxxxx/
>>>> Link: https://lore.kernel.org/all/DGHI8WRKBQS9.38910L6FIIZTE@xxxxxxxxxx/
>>>> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
>>>
>>> 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.