Re: [PATCH v2 02/16] rust: io: add `IoRepr` trait

["Alexandre Courbot" <acourbot@nvidia.com>]

On Thu Aug 6, 2026 at 1:35 AM JST, Gary Guo wrote:
> For types that are layout-compatible with an I/O capable type, we would
> want the ability to use them directly for I/O operations. E.g.
>
>     bitfield! {
>         pub struct Foo(u32) {
>             ...
>         }
>     }
>
>     #[repr(C)]
>     struct Bar {
>         foo: Foo,
>     }
>
>     let mmio: Mmio<'_, Bar> = ...;
>     io_read!(mmio, .foo)
>
> Currently this feature is available from `register!()` macro but not
> otherwise available with `io_read!`, `io_write!`. Support this by adding a
> `IoRepr` type to denote the underlying I/O type to use for a specific type.
>
> This makes the `IoLoc::IoType` and `Register::Storage` redundant; thus
> remove them; also convert register methods to use the `read_val` and
> `write_val` instead.

Ah that's nice, I was a bit bothered that `IoType` and `Storage`
basically defined the same thing.

>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  rust/kernel/bitfield.rs    |   5 ++
>  rust/kernel/io.rs          | 203 ++++++++++++++++++++++++++++++++-------------
>  rust/kernel/io/register.rs |  59 +++++--------
>  3 files changed, 170 insertions(+), 97 deletions(-)
>
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index 35ede53f2b8e..4bc92c62da06 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -308,6 +308,7 @@ macro_rules! bitfield {
>          $(#[$attr])*
>          #[repr(transparent)]
>          #[derive(Clone, Copy, PartialEq, Eq)]
> +        #[derive($crate::prelude::FromBytes, $crate::prelude::IntoBytes)]
>          $vis struct $name {
>              inner: $storage,
>          }
> @@ -346,6 +347,10 @@ fn from(val: $storage) -> $name {
>                  Self::from_raw(val)
>              }
>          }
> +
> +        impl $crate::io::IoRepr for $name {
> +            type Repr = $storage;
> +        }

This introduces a dependency from `bitfield` to `io`, which looks like
inconsistent layering. But thankfully there is an easy fix - see below.

>      };
>  
>      // Definitions requiring knowledge of individual fields: private and public field accessors,
> 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?

> +
> +/// 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.

That way `bitfield` gets a dependency on `transmute` rather than `io`,
which doesn't break layering.

In order to avoid `Repr::Repr` we can also rename the associated type to
`Raw` and update the method names accordingly to `from_raw`/`into_raw` -
which would have allowed us to remove the `bitfield` methods of the same
name if they weren't needed in const context! But at least it makes
things align nicely.

<...>
> @@ -831,8 +816,8 @@ macro_rules! register {
>              { $($fields:tt)* }
>      ) => {
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
> -        $crate::register!(@io_base $name($storage) @ $offset);
> -        $crate::register!(@io_fixed $(#[$attr])* $vis $name($storage));
> +        $crate::register!(@io_base $name @ $offset);
> +        $crate::register!(@io_fixed $(#[$attr])* $vis $name);
>      };
>  
>      // Creates an alias register of fixed offset register `alias` with its own fields.
> @@ -842,10 +827,10 @@ macro_rules! register {
>      ) => {
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
>          $crate::register!(
> -            @io_base $name($storage) @
> +            @io_base $name @
>              <$alias as $crate::io::register::Register>::OFFSET
>          );
> -        $crate::register!(@io_fixed $(#[$attr])* $vis $name($storage));
> +        $crate::register!(@io_fixed $(#[$attr])* $vis $name);
>      };
>  
>      // Creates a register at a relative offset from a base address provider.
> @@ -854,8 +839,8 @@ macro_rules! register {
>              { $($fields:tt)* }
>      ) => {
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
> -        $crate::register!(@io_base $name($storage) @ $offset);
> -        $crate::register!(@io_relative $vis $name($storage) @ $base);
> +        $crate::register!(@io_base $name @ $offset);
> +        $crate::register!(@io_relative $vis $name @ $base);
>      };
>  
>      // Creates an alias register of relative offset register `alias` with its own fields.
> @@ -865,9 +850,9 @@ macro_rules! register {
>      ) => {
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
>          $crate::register!(
> -            @io_base $name($storage) @ <$alias as $crate::io::register::Register>::OFFSET
> +            @io_base $name @ <$alias as $crate::io::register::Register>::OFFSET
>          );
> -        $crate::register!(@io_relative $vis $name($storage) @ $base);
> +        $crate::register!(@io_relative $vis $name @ $base);
>      };
>  
>      // Creates an array of registers at a fixed offset of the MMIO space.
> @@ -878,8 +863,8 @@ macro_rules! register {
>          $crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
>  
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
> -        $crate::register!(@io_base $name($storage) @ $offset);
> -        $crate::register!(@io_array $vis $name($storage) [ $size, stride = $stride ]);
> +        $crate::register!(@io_base $name @ $offset);
> +        $crate::register!(@io_array $vis $name [ $size, stride = $stride ]);
>      };
>  
>      // Shortcut for contiguous array of registers (stride == size of element).
> @@ -904,11 +889,11 @@ macro_rules! register {
>  
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
>          $crate::register!(
> -            @io_base $name($storage) @
> +            @io_base $name @
>              <$alias as $crate::io::register::Register>::OFFSET
>                  + $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
>          );
> -        $crate::register!(@io_fixed $(#[$attr])* $vis $name($storage));
> +        $crate::register!(@io_fixed $(#[$attr])* $vis $name);
>      };
>  
>      // Creates an array of registers at a relative offset from a base address provider.
> @@ -920,9 +905,9 @@ macro_rules! register {
>          $crate::build_assert::static_assert!(::core::mem::size_of::<$storage>() <= $stride);
>  
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
> -        $crate::register!(@io_base $name($storage) @ $offset);
> +        $crate::register!(@io_base $name @ $offset);
>          $crate::register!(
> -            @io_relative_array $vis $name($storage) [ $size, stride = $stride ] @ $base + $offset
> +            @io_relative_array $vis $name [ $size, stride = $stride ] @ $base + $offset
>          );
>      };
>  
> @@ -949,11 +934,11 @@ macro_rules! register {
>  
>          $crate::register!(@bitfield $(#[$attr])* $vis struct $name($storage) { $($fields)* });
>          $crate::register!(
> -            @io_base $name($storage) @
> +            @io_base $name @
>                  <$alias as $crate::io::register::Register>::OFFSET +
>                  $idx * <$alias as $crate::io::register::RegisterArray>::STRIDE
>          );
> -        $crate::register!(@io_relative $vis $name($storage) @ $base);
> +        $crate::register!(@io_relative $vis $name @ $base);
>      };
>  
>      // Generates the bitfield for the register.
> @@ -970,16 +955,14 @@ macro_rules! register {
>      };
>  
>      // Implementations shared by all registers types.
> -    (@io_base $name:ident($storage:ty) @ $offset:expr) => {
> +    (@io_base $name:ident @ $offset:expr) => {
>          impl $crate::io::register::Register for $name {
> -            type Storage = $storage;
> -
>              const OFFSET: usize = $offset;
>          }
>      };
>  
>      // Implementations of fixed registers.
> -    (@io_fixed $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)) => {
> +    (@io_fixed $(#[$attr:meta])* $vis:vis $name:ident) => {
>          impl $crate::io::register::FixedRegister for $name {}
>  
>          $(#[$attr])*
> @@ -988,7 +971,7 @@ impl $crate::io::register::FixedRegister for $name {}
>      };
>  
>      // Implementations of relative registers.
> -    (@io_relative $vis:vis $name:ident ($storage:ty) @ $base:ident) => {
> +    (@io_relative $vis:vis $name:ident @ $base:ident) => {
>          impl $crate::io::register::WithBase for $name {
>              type BaseFamily = $base;
>          }
> @@ -997,7 +980,7 @@ impl $crate::io::register::RelativeRegister for $name {}
>      };
>  
>      // Implementations of register arrays.
> -    (@io_array $vis:vis $name:ident ($storage:ty) [ $size:expr, stride = $stride:expr ]) => {
> +    (@io_array $vis:vis $name:ident [ $size:expr, stride = $stride:expr ]) => {
>          impl $crate::io::register::Array for $name {}
>  
>          impl $crate::io::register::RegisterArray for $name {
> @@ -1008,7 +991,7 @@ impl $crate::io::register::RegisterArray for $name {
>  
>      // Implementations of relative array registers.
>      (
> -        @io_relative_array $vis:vis $name:ident ($storage:ty) [ $size:expr, stride = $stride:expr ]
> +        @io_relative_array $vis:vis $name:ident [ $size:expr, stride = $stride:expr ]
>              @ $base:ident + $offset:literal
>      ) => {
>          impl $crate::io::register::WithBase for $name {

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.

[1] https://lore.kernel.org/all/20260724-registers_fix-v2-2-a0fb58b02185@nvidia.com/