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

Daniel Almeida <[email protected]>
Newsgroups org.kernel.vger.linux-pwm,dev.linux.lists.driver-core,dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>

> On 6 Jul 2026, at 09:44, Gary Guo <[email protected]> wrote:
> 
> Implement `Mmio` as view type and convert `RelaxedMmio` to view type as
> well. I/O implementations of `MmioOwned` are changed to delegate to the
> `Mmio` view type.
> 
> All existing users of `MmioOwned` in the documentation which do not
> actually reflect the owning semantics is converted.
> 
> Reviewed-by: Alexandre Courbot <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
> ---
> rust/kernel/io.rs          | 176 ++++++++++++++++++++++++++++++++++-----------
> rust/kernel/io/poll.rs     |  10 +--
> rust/kernel/io/register.rs |  24 +++----
> 3 files changed, 153 insertions(+), 57 deletions(-)
> 
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index f93be7f78069..8110b49aa430 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -4,6 +4,10 @@
> //!
> //! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
> 
> +use core::{
> +    marker::PhantomData, //
> +};
> +
> use crate::{
>     bindings,
>     prelude::*,
> @@ -537,10 +541,11 @@ fn write64(self, value: u64, offset: usize)
>     /// ```no_run
>     /// use kernel::io::{
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
> -    /// fn do_reads(io: &MmioOwned) -> Result {
> +    /// fn do_reads(io: Mmio<'_, Region>) -> Result {
>     ///     // 32-bit read from address `0x10`.
>     ///     let v: u32 = io.try_read(0x10)?;
>     ///
> @@ -571,10 +576,11 @@ fn try_read<T, L>(self, location: L) -> Result<T>
>     /// ```no_run
>     /// use kernel::io::{
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
> -    /// fn do_writes(io: &MmioOwned) -> Result {
> +    /// fn do_writes(io: Mmio<'_, Region>) -> Result {
>     ///     // 32-bit write of value `1` at address `0x10`.
>     ///     io.try_write(0x10, 1u32)?;
>     ///
> @@ -609,7 +615,8 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
>     /// use kernel::io::{
>     ///     register,
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
>     /// register! {
> @@ -625,7 +632,7 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
>     ///     }
>     /// }
>     ///
> -    /// fn do_write_reg(io: &MmioOwned) -> Result {
> +    /// fn do_write_reg(io: Mmio<'_, Region>) -> Result {
>     ///
>     ///     io.try_write_reg(VERSION::new(1, 0))
>     /// }
> @@ -654,10 +661,11 @@ fn try_write_reg<T, L, V>(self, value: V) -> Result
>     /// ```no_run
>     /// use kernel::io::{
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
> -    /// fn do_update(io: &MmioOwned<0x1000>) -> Result {
> +    /// fn do_update(io: Mmio<'_, Region<0x1000>>) -> Result {
>     ///     io.try_update(0x10, |v: u32| {
>     ///         v + 1
>     ///     })
> @@ -691,10 +699,11 @@ fn try_update<T, L, F>(self, location: L, f: F) -> Result
>     /// ```no_run
>     /// use kernel::io::{
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
> -    /// fn do_reads(io: &MmioOwned<0x1000>) {
> +    /// fn do_reads(io: Mmio<'_, Region<0x1000>>) {
>     ///     // 32-bit read from address `0x10`.
>     ///     let v: u32 = io.read(0x10);
>     ///
> @@ -723,10 +732,11 @@ fn read<T, L>(self, location: L) -> T
>     /// ```no_run
>     /// use kernel::io::{
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
> -    /// fn do_writes(io: &MmioOwned<0x1000>) {
> +    /// fn do_writes(io: Mmio<'_, Region<0x1000>>) {
>     ///     // 32-bit write of value `1` at address `0x10`.
>     ///     io.write(0x10, 1u32);
>     ///
> @@ -757,7 +767,8 @@ fn write<T, L>(self, location: L, value: T)
>     /// use kernel::io::{
>     ///     register,
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
>     /// register! {
> @@ -773,7 +784,7 @@ fn write<T, L>(self, location: L, value: T)
>     ///     }
>     /// }
>     ///
> -    /// fn do_write_reg(io: &MmioOwned<0x1000>) {
> +    /// fn do_write_reg(io: Mmio<'_, Region<0x1000>>) {
>     ///     io.write_reg(VERSION::new(1, 0));
>     /// }
>     /// ```
> @@ -801,10 +812,11 @@ fn write_reg<T, L, V>(self, value: V)
>     /// ```no_run
>     /// use kernel::io::{
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     /// };
>     ///
> -    /// fn do_update(io: &MmioOwned<0x1000>) {
> +    /// fn do_update(io: Mmio<'_, Region<0x1000>>) {
>     ///     io.update(0x10, |v: u32| {
>     ///         v + 1
>     ///     })
> @@ -828,16 +840,72 @@ fn update<T, L, F>(self, location: L, f: F)
>     }
> }
> 
> +/// 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 a valid and aligned memory-mapped I/O region while `'a` is alive.
> +    #[inline]
> +    pub unsafe fn from_raw(raw: MmioRaw<T>) -> Self {
> +        // INVARIANT: Per safety requirement.
> +        Self {
> +            ptr: raw.ptr,
> +            phantom: PhantomData,
> +        }
> +    }
> +}
> +
> +// SAFETY: `Mmio<'_, T>` is conceptually `&T` but in I/O memory.
> +unsafe impl<T: ?Sized + Sync> Send for Mmio<'_, T> {}
> +
> +// SAFETY: `Mmio<'_, T>` is conceptually `&T` but in I/O memory.
> +unsafe impl<T: ?Sized + Sync> Sync for Mmio<'_, T> {}
> +
> +impl<T: ?Sized + KnownSize> Io for Mmio<'_, T> {
> +    type Target = T;
> +
> +    #[inline]
> +    fn addr(self) -> usize {
> +        self.ptr.addr()
> +    }
> +
> +    #[inline]
> +    fn maxsize(self) -> usize {
> +        KnownSize::size(self.ptr)
> +    }
> +}
> +
> /// Implements [`IoCapable`] on `$mmio` for `$ty` using `$read_fn` and `$write_fn`.
> macro_rules! impl_mmio_io_capable {
>     ($mmio:ident, $(#[$attr:meta])* $ty:ty, $read_fn:ident, $write_fn:ident) => {
>         $(#[$attr])*
> -        impl<const SIZE: usize> IoCapable<$ty> for &$mmio<SIZE> {
> +        impl<T: ?Sized> IoCapable<$ty> for $mmio<'_, T> {
> +            #[inline]
>             unsafe fn io_read(self, address: usize) -> $ty {
>                 // SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
>                 unsafe { bindings::$read_fn(address as *const c_void) }
>             }
> 
> +            #[inline]
>             unsafe fn io_write(self, value: $ty, address: usize) {
>                 // SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
>                 unsafe { bindings::$write_fn(value, address as *mut c_void) }
> @@ -847,17 +915,12 @@ unsafe fn io_write(self, value: $ty, address: usize) {
> }
> 
> // MMIO regions support 8, 16, and 32-bit accesses.
> -impl_mmio_io_capable!(MmioOwned, u8, readb, writeb);
> -impl_mmio_io_capable!(MmioOwned, u16, readw, writew);
> -impl_mmio_io_capable!(MmioOwned, u32, readl, writel);
> +impl_mmio_io_capable!(Mmio, u8, readb, writeb);
> +impl_mmio_io_capable!(Mmio, u16, readw, writew);
> +impl_mmio_io_capable!(Mmio, u32, readl, writel);
> // MMIO regions on 64-bit systems also support 64-bit accesses.
> -impl_mmio_io_capable!(
> -    MmioOwned,
> -    #[cfg(CONFIG_64BIT)]
> -    u64,
> -    readq,
> -    writeq
> -);
> +#[cfg(CONFIG_64BIT)]
> +impl_mmio_io_capable!(Mmio, u64, readq, writeq);
> 
> impl<'a, const SIZE: usize> Io for &'a MmioOwned<SIZE> {
>     type Target = Region<SIZE>;
> @@ -875,6 +938,23 @@ fn maxsize(self) -> usize {
>     }
> }
> 
> +impl<'a, const SIZE: usize, T> IoCapable<T> for &'a MmioOwned<SIZE>
> +where
> +    Mmio<'a, Region<SIZE>>: IoCapable<T>,
> +{
> +    #[inline]
> +    unsafe fn io_read(self, address: usize) -> T {
> +        // SAFETY: Per safety requirement.
> +        unsafe { self.as_view().io_read(address) }
> +    }
> +
> +    #[inline]
> +    unsafe fn io_write(self, value: T, address: usize) {
> +        // SAFETY: Per safety requirement.
> +        unsafe { self.as_view().io_write(value, address) }
> +    }
> +}
> +
> impl<const SIZE: usize> MmioOwned<SIZE> {
>     /// Converts an `MmioRaw` into an `MmioOwned` instance, providing the accessors to the MMIO
>     /// mapping.
> @@ -887,19 +967,33 @@ pub unsafe fn from_raw(raw: &MmioRaw<Region<SIZE>>) -> &Self {
>         // SAFETY: `MmioOwned` is a transparent wrapper around `MmioRaw`.
>         unsafe { &*core::ptr::from_ref(raw).cast() }
>     }
> +
> +    /// Return a view that covers the full region.
> +    #[inline]
> +    pub fn as_view(&self) -> Mmio<'_, Region<SIZE>> {
> +        // SAFETY: `Mmio` has same invariant as `MmioOwned`.
> +        unsafe { Mmio::from_raw(self.0) }
> +    }
> }
> 
> -/// [`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.
> +pub struct RelaxedMmio<'a, T: ?Sized>(Mmio<'a, T>);
> 
> -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
> +    }
> +}
> +
> +impl<T: ?Sized + KnownSize> Io for RelaxedMmio<'_, T> {
> +    type Target = T;
> 
>     #[inline]
>     fn addr(self) -> usize {
> @@ -912,8 +1006,8 @@ fn maxsize(self) -> usize {
>     }
> }
> 
> -impl<const SIZE: usize> MmioOwned<SIZE> {
> -    /// Returns a [`RelaxedMmio`] reference that performs relaxed I/O operations.
> +impl<'a, T: ?Sized> Mmio<'a, T> {
> +    /// Returns a [`RelaxedMmio`] that performs relaxed I/O operations.
>     ///
>     /// Relaxed accessors do not provide ordering guarantees with respect to DMA or memory accesses
>     /// and can be used when such ordering is not required.
> @@ -923,20 +1017,20 @@ impl<const SIZE: usize> MmioOwned<SIZE> {
>     /// ```no_run
>     /// use kernel::io::{
>     ///     Io,
> -    ///     MmioOwned,
> +    ///     Mmio,
> +    ///     Region,
>     ///     RelaxedMmio,
>     /// };
>     ///
> -    /// fn do_io(io: &MmioOwned<0x100>) {
> +    /// fn do_io(io: Mmio<'_, Region<0x100>>) {
>     ///     // The access is performed using `readl_relaxed` instead of `readl`.
>     ///     let v = io.relaxed().read32(0x10);
>     /// }
>     ///
>     /// ```
> -    pub fn relaxed(&self) -> &RelaxedMmio<SIZE> {
> -        // SAFETY: `RelaxedMmio` is `#[repr(transparent)]` over `MmioOwned`, so `MmioOwned<SIZE>`
> -        // and `RelaxedMmio<SIZE>` have identical layout.
> -        unsafe { core::mem::transmute(self) }
> +    #[inline]
> +    pub fn relaxed(self) -> RelaxedMmio<'a, T> {
> +        RelaxedMmio(self)
>     }
> }
> 
> diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
> index 79828a8006b5..d75f2fcf46f2 100644
> --- a/rust/kernel/io/poll.rs
> +++ b/rust/kernel/io/poll.rs
> @@ -47,14 +47,15 @@
> /// ```no_run
> /// use kernel::io::{
> ///     Io,
> -///     MmioOwned,
> +///     Mmio,
> +///     Region,
> ///     poll::read_poll_timeout, //
> /// };
> /// use kernel::time::Delta;
> ///
> /// const HW_READY: u16 = 0x01;
> ///
> -/// fn wait_for_hardware<const SIZE: usize>(io: &MmioOwned<SIZE>) -> Result {
> +/// fn wait_for_hardware<const SIZE: usize>(io: Mmio<'_, Region<SIZE>>) -> Result {
> ///     read_poll_timeout(
> ///         // The `op` closure reads the value of a specific status register.
> ///         || io.try_read16(0x1000),
> @@ -134,14 +135,15 @@ pub fn read_poll_timeout<Op, Cond, T>(
> /// ```no_run
> /// use kernel::io::{
> ///     Io,
> -///     MmioOwned,
> +///     Mmio,
> +///     Region,
> ///     poll::read_poll_timeout_atomic, //
> /// };
> /// use kernel::time::Delta;
> ///
> /// const HW_READY: u16 = 0x01;
> ///
> -/// fn wait_for_hardware<const SIZE: usize>(io: &MmioOwned<SIZE>) -> Result {
> +/// fn wait_for_hardware<const SIZE: usize>(io: Mmio<'_, Region<SIZE>>) -> Result {
> ///     read_poll_timeout_atomic(
> ///         // The `op` closure reads the value of a specific status register.
> ///         || io.try_read16(0x1000),
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index fd3bf6220f95..6cb07fc92cc3 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
> @@ -58,7 +58,7 @@
> //!     },
> //!     num::Bounded,
> //! };
> -//! # use kernel::io::MmioOwned;
> +//! # use kernel::io::{Mmio, Region};
> //! # register! {
> //! #     pub BOOT_0(u32) @ 0x00000100 {
> //! #         15:8 vendor_id;
> @@ -66,7 +66,7 @@
> //! #         3:0 minor_revision;
> //! #     }
> //! # }
> -//! # fn test(io: &MmioOwned<0x1000>) {
> +//! # fn test(io: Mmio<'_, Region<0x1000>>) {
> //! # fn obtain_vendor_id() -> u8 { 0xff }
> //!
> //! // Read from the register's defined offset (0x100).
> @@ -446,7 +446,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///         Io,
> ///     },
> /// };
> -/// # use kernel::io::MmioOwned;
> +/// # use kernel::io::{Mmio, Region};
> ///
> /// register! {
> ///     FIXED_REG(u32) @ 0x100 {
> @@ -455,7 +455,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///     }
> /// }
> ///
> -/// # fn test(io: &MmioOwned<0x1000>) {
> +/// # fn test(io: Mmio<'_, Region<0x1000>>) {
> /// let val = io.read(FIXED_REG);
> ///
> /// // Write from an already-existing value.
> @@ -559,7 +559,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///         Io,
> ///     },
> /// };
> -/// # use kernel::io::MmioOwned;
> +/// # use kernel::io::{Mmio, Region};
> ///
> /// // Type used to identify the base.
> /// pub struct CpuCtlBase;
> @@ -584,7 +584,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///     }
> /// }
> ///
> -/// # fn test(io: MmioOwned<0x1000>) {
> +/// # fn test(io: Mmio<'_, Region<0x1000>>) {
> /// // Read the status of `Cpu0`.
> /// let cpu0_started = io.read(CPU_CTL::of::<Cpu0>());
> ///
> @@ -601,7 +601,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///     }
> /// }
> ///
> -/// # fn test2(io: MmioOwned<0x1000>) {
> +/// # fn test2(io: Mmio<'_, Region<0x1000>>) {
> /// // Start the aliased `CPU0`, leaving its other fields untouched.
> /// io.update(CPU_CTL_ALIAS::of::<Cpu0>(), |r| r.with_alias_start(true));
> /// # }
> @@ -638,7 +638,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///         Io,
> ///     },
> /// };
> -/// # use kernel::io::MmioOwned;
> +/// # use kernel::io::{Mmio, Region};
> /// # fn get_scratch_idx() -> usize {
> /// #   0x15
> /// # }
> @@ -651,7 +651,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///     }
> /// }
> ///
> -/// # fn test(io: &MmioOwned<0x1000>)
> +/// # fn test(io: Mmio<'_, Region<0x1000>>)
> /// #     -> Result<(), Error>{
> /// // Read scratch register 0, i.e. I/O address `0x80`.
> /// let scratch_0 = io.read(SCRATCH::at(0)).value();
> @@ -724,7 +724,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///         Io,
> ///     },
> /// };
> -/// # use kernel::io::MmioOwned;
> +/// # use kernel::io::{Mmio, Region};
> /// # fn get_scratch_idx() -> usize {
> /// #   0x15
> /// # }
> @@ -752,7 +752,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///     }
> /// }
> ///
> -/// # fn test(io: &MmioOwned<0x1000>) -> Result<(), Error> {
> +/// # fn test(io: Mmio<'_, Region<0x1000>>) -> Result<(), Error> {
> /// // Read scratch register 0 of CPU0.
> /// let scratch = io.read(CPU_SCRATCH::of::<Cpu0>().at(0));
> ///
> @@ -794,7 +794,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> ///     }
> /// }
> ///
> -/// # fn test2(io: &MmioOwned<0x1000>) -> Result<(), Error> {
> +/// # fn test2(io: Mmio<'_, Region<0x1000>>) -> Result<(), Error> {
> /// let cpu0_status = io.read(CPU_FIRMWARE_STATUS::of::<Cpu0>()).status();
> /// # Ok(())
> /// # }
> 
> -- 
> 2.54.0
> 


Ran the tests again, they’re passing.

Tested-by: Daniel Almeida <[email protected]>
Reviewed-by: Daniel Almeida <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.