Re: [PATCH v4 07/20] rust: io: implement `Mmio` as view type

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

On Fri Jun 12, 2026 at 1:28 AM JST, Gary Guo wrote:
<...>
> +/// A view of memory-mapped I/O region.
> +///
> +/// # Invariant
> +///
> +/// `ptr` points to a valid and aligned memory-mapped I/O region for the duration lifetime `'a`.
> +pub struct Mmio<'a, T: ?Sized> {
> +    ptr: *mut T,
> +    phantom: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Copy for Mmio<'_, T> {}
> +impl<T: ?Sized> Clone for Mmio<'_, T> {
> +    #[inline]
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +impl<'a, T: ?Sized> Mmio<'a, T> {
> +    /// Create a `Mmio`, providing the accessors to the MMIO mapping.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `raw` represents an valid and aligned memory-mapped I/O region while `'a` is alive.

typo: "a valid".

<...>
> -/// [`MmioOwned`] wrapper using relaxed accessors.
> +/// [`Mmio`] but using relaxed accessors.
>  ///
>  /// This type provides an implementation of [`Io`] that uses relaxed I/O MMIO operands instead of
>  /// the regular ones.
>  ///
> -/// See [`MmioOwned::relaxed`] for a usage example.
> -#[repr(transparent)]
> -pub struct RelaxedMmio<const SIZE: usize = 0>(MmioOwned<SIZE>);
> +/// See [`Mmio::relaxed`] for a usage example.
> +///
> +/// # Invariant
> +///
> +/// `ptr` points to a valid and aligned memory-mapped I/O region for the duration lifetime `'a`.
> +pub struct RelaxedMmio<'a, T: ?Sized> {
> +    ptr: *mut T,
> +    phantom: PhantomData<&'a ()>,
> +}

Is there a reason for not just declaring `RelaxedMmio` as

    #[repr(transparent)]
    pub struct RelaxedMmio<'a, T: ?Sized>(Mmio<'a, T>);

similarly to what the original code did with `MmioOwned`?

I tried locally and could build. This avoids declaring `Mmio` and
`RelaxedMmio` as basically identical types, and lets us remove the
explicit `Send` and `Sync` implementations. IIUC you can also reduce or
even remove the invariant section as it is enforced by `Mmio`.

>  
> -impl<'a, const SIZE: usize> Io for &'a RelaxedMmio<SIZE> {
> -    type Target = Region<SIZE>;
> +impl<T: ?Sized> Copy for RelaxedMmio<'_, T> {}
> +impl<T: ?Sized> Clone for RelaxedMmio<'_, T> {
> +    #[inline]
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +// SAFETY: `RelaxedMmio<'_, T>` is conceptually `&T` but in I/O memory.
> +unsafe impl<T: ?Sized + Sync> Send for RelaxedMmio<'_, T> {}
> +
> +// SAFETY: `RelaxedMmio<'_, T>` is conceptually `&T` but in I/O memory.
> +unsafe impl<T: ?Sized + Sync> Sync for RelaxedMmio<'_, T> {}
> +
> +impl<T: ?Sized + KnownSize> Io for RelaxedMmio<'_, T> {
> +    type Target = T;
>  
>      #[inline]
>      fn addr(self) -> usize {
> -        self.0.addr()
> +        self.ptr.addr()
>      }
>  
>      #[inline]
>      fn maxsize(self) -> usize {
> -        self.0.maxsize()
> +        KnownSize::size(self.ptr)
>      }
>  }
>  
> -impl<const SIZE: usize> MmioOwned<SIZE> {
> +impl<'a, T: ?Sized> Mmio<'a, T> {
>      /// Returns a [`RelaxedMmio`] reference that performs relaxed I/O operations.

The `RelaxedMmio` is now returned by value, so this comment should
probably be updated.