NVIDIA GPU driver infrastructure
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Gary Guo" <gary@garyguo.net>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Onur Özkan" <work@onurozkan.dev>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	driver-core@lists.linux.dev, rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org, nova-gpu@lists.linux.dev,
	dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 02/16] rust: io: add `IoRepr` trait
Date: Wed, 12 Aug 2026 13:51:21 +0900	[thread overview]
Message-ID: <DKMP1HAAG2E2.26IBNW8ICEREP@nvidia.com> (raw)
In-Reply-To: <DKL837PVU38R.3OONERSO8RMOW@garyguo.net>

On Mon Aug 10, 2026 at 8:21 PM JST, Gary Guo wrote:
> On Mon Aug 10, 2026 at 10:30 AM BST, Alexandre Courbot wrote:
>> On Thu Aug 6, 2026 at 1:35 AM JST, Gary Guo wrote:
>>> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
>>> index adfc555de7d0..71c6180ed745 100644
>>> --- a/rust/kernel/io.rs
>>> +++ b/rust/kernel/io.rs
>>> @@ -276,6 +276,91 @@ pub trait IoCapable<T>: IoBackend {
>>>      fn io_write<'a>(view: Self::View<'a, T>, value: T);
>>>  }
>>>  
>>> +/// Safe transmute that performs size check on monomorphization-time.
>>> +///
>>> +/// Can be considered as generic version of [`zerocopy::transmute!`] macro but using the unstable
>>> +/// `core::mem::transmute_neo` instead of [`core::mem::transmute`].
>>> +#[inline(always)] // This is a no-op.
>>> +fn transmute_neo<Src: IntoBytes, Dst: FromBytes>(val: Src) -> Dst {
>>> +    const_assert!(size_of::<Src>() == size_of::<Dst>());
>>> +
>>> +    // SAFETY: `Src: IntoBytes` and `Dst: FromBytes` and we've checked size is the same.
>>> +    unsafe { core::mem::transmute_copy(&core::mem::ManuallyDrop::new(val)) }
>>> +}
>>
>> This is universally useful, so let's move this to the `transmute` module?
>
> I think we can just have a `kernel::mem` and put it there so it's consistent
> with std naming. The `transmute` module just contains two types that are going
> away.

Yes, that would work as well and is probably an even better location.

>
> Ideally we have this function in `zerocopy`. But my understanding is that this
> depends on inline const which is stable since 1.79 but zerocopy's MSRV is 1.56.
>
>>
>>> +
>>> +/// Trait indicating the underlying primitive types to be used for I/O operations.
>>> +///
>>> +/// Implementing trait allows arbitrary types to be used for I/O operations, not just raw
>>> +/// primitives.
>>> +///
>>> +/// The layout of the type and the underlying primitive must match; this is enforced via const
>>> +/// assertions when I/O methods are used, as the type system cannot represent this.
>>> +/// [`IoRepr::from_repr`] and [`IoRepr::into_expr`] can be overridden for conversions, however it
>>> +/// should be noted that they are only invoked on value read/write operations and are not invoked
>>> +/// on byte operations such as [`Io::copy_read`].
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// ```
>>> +/// # use kernel::io::*;
>>> +/// #[repr(transparent)]
>>> +/// #[derive(FromBytes, IntoBytes)]
>>> +/// pub struct MyNewType(u32);
>>> +///
>>> +/// impl IoRepr for MyNewType {
>>> +///     type Repr = u32;
>>> +/// }
>>> +///
>>> +/// #[repr(C)]
>>> +/// pub struct MyStruct {
>>> +///     raw: u32,
>>> +///     new_type: MyNewType,
>>> +/// }
>>> +///
>>> +/// # fn test(mmio: Mmio<'_, MyStruct>) {
>>> +/// // let mmio: Mmio<'_, MyStruct>;
>>> +/// let val: u32 = io_read!(mmio, .raw); // Raw primitive read
>>> +/// io_write!(mmio, .raw, val);          // Raw primitve write
>>> +/// let val: MyNewType = io_read!(mmio, .new_type); // Read via `IoRepr`.
>>> +/// io_write!(mmio, .new_type, val);                // Write via `IoRepr`.
>>> +/// # }
>>> +/// ```
>>> +pub trait IoRepr: FromBytes + IntoBytes + Sized {
>>> +    /// The backing I/O capable type.
>>> +    type Repr: FromBytes + IntoBytes;
>>> +
>>> +    /// Convert from [`IoRepr::Repr`] to `Self`.
>>> +    #[inline(always)]
>>> +    fn from_repr(repr: Self::Repr) -> Self {
>>> +        transmute_neo(repr)
>>> +    }
>>> +
>>> +    /// Convert from `Self` to [`IoRepr::Repr`].
>>> +    #[inline(always)]
>>> +    fn into_repr(this: Self) -> Self::Repr {
>>> +        transmute_neo(this)
>>> +    }
>>> +}
>>> +
>>> +macro_rules! impl_io_repr {
>>> +    ($($ty:ty => $backing:ty,)*) => {
>>> +        $(impl IoRepr for $ty {
>>> +            type Repr = $backing;
>>> +        })*
>>> +    };
>>> +}
>>> +
>>> +impl_io_repr! {
>>> +    u8 => u8,
>>> +    u16 => u16,
>>> +    u32 => u32,
>>> +    u64 => u64,
>>> +    i8 => u8,
>>> +    i16 => u16,
>>> +    i32 => u32,
>>> +    i64 => u64,
>>> +}
>>
>> ... and `IoRepr` and its implementations for primitive types should also
>> be part of `transmute` IMHO (after being renamed to e.g. `Repr`), for
>> there is nothing I/O exclusive to it. It just indicates that one type
>> can be represented by another, a property that is again useful outside
>> of I/O.
>
> I tried to be a little bit forward looking in designing this, so I ended up
> with a design that provides conversion functions instead of just transmutation.
> If we're moving it we should probably just get rid of these and just require a
> transmutability with the raw repr.

If that works for you then that would simplify things a bit yes, and we
can think about hypotheticals when we need them.

>
> Note that there is `AtomicType` which has similar (but not equivalent
> requirement). `AtomicType` requires a round-trip transmutability only, and
> `IoRepr` needs both directions. So `repr(C)` enums (that is not used up all its
> variant reprs) can be `AtomicType` but not `IoRepr`.
>
>>
>> That way `bitfield` gets a dependency on `transmute` rather than `io`,
>> which doesn't break layering.
>
> We can also avoid breaking layering by requiring user to need to specify
> `#[derive(IoRepr)]` when declaring bitfield.

Yup, that sounds like a good solution to the problem as well.

<...>
>> These appear to repeat quite a bit of [1] which is already in
>> `driver-core-next`. You'll want to rebase, I think the only difference
>> between the two is the removal of $storage from @io_base.
>
> I'm thinking of moving this entire thing to syn :)

Heh, I guess that's the next logical step. It would also avoid the (for
now harmless) recursion introduced by the token muncher of this series.

  reply	other threads:[~2026-08-12  4:51 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 16:35 [PATCH v2 00/16] rust: io: support register projections and remove relative registers Gary Guo
2026-08-05 16:35 ` [PATCH v2 01/16] rust: io: add static `cast()` method for views Gary Guo
2026-08-10  9:29   ` Alexandre Courbot
2026-08-05 16:35 ` [PATCH v2 02/16] rust: io: add `IoRepr` trait Gary Guo
2026-08-10  9:30   ` Alexandre Courbot
2026-08-10 11:21     ` Gary Guo
2026-08-12  4:51       ` Alexandre Courbot [this message]
2026-08-05 16:35 ` [PATCH v2 03/16] rust: io: support register projections Gary Guo
2026-08-10  9:30   ` Alexandre Courbot
2026-08-10 11:23     ` Gary Guo
2026-08-05 16:35 ` [PATCH v2 04/16] rust: io: register: handle one register at a time Gary Guo
2026-08-10  9:30   ` Alexandre Courbot
2026-08-05 16:35 ` [PATCH v2 05/16] rust: io: register extract offset computation to helper rules Gary Guo
2026-08-10  9:31   ` Alexandre Courbot
2026-08-05 16:35 ` [PATCH v2 06/16] rust: io: register: allow explicit base type specification Gary Guo
2026-08-10  9:32   ` Alexandre Courbot
2026-08-05 16:35 ` [PATCH v2 07/16] gpu: nova-core: specify base type for registers Gary Guo
2026-08-05 16:35 ` [PATCH v2 08/16] drm/tyr: " Gary Guo
2026-08-05 16:59   ` Gary Guo
2026-08-05 16:35 ` [PATCH v2 09/16] samples: rust: pci: " Gary Guo
2026-08-05 16:35 ` [PATCH v2 10/16] rust: io: register: make register have a typed base Gary Guo
2026-08-05 16:35 ` [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield Gary Guo
2026-08-05 16:35 ` [PATCH v2 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers Gary Guo
2026-08-05 16:35 ` [PATCH v2 13/16] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
2026-08-05 16:35 ` [PATCH v2 14/16] rust: io: register: remove relative registers Gary Guo
2026-08-05 16:35 ` [PATCH v2 15/16] rust: io: register: remove `Register` trait and cleanup macro Gary Guo
2026-08-05 16:35 ` [PATCH v2 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKMP1HAAG2E2.26IBNW8ICEREP@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=driver-core@lists.linux.dev \
    --cc=gary@garyguo.net \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox