Re: [PATCH v8 06/10] rust: io: use generic read/write accessors for primitive accesses

[Gary Guo]

On Mon Mar 9, 2026 at 3:14 PM GMT, Alexandre Courbot wrote:
> By providing the required `IoLoc` implementations on `usize`, we can
> leverage the generic accessors and reduce the number of unsafe blocks in
> the module.
>
> This also allows us to directly call the generic `read/write/update`
> methods with primitive types, so add examples illustrating this.
>
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
>  rust/kernel/io.rs | 199 +++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 131 insertions(+), 68 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 1db6572f4a42..ed6fab001a39 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -197,6 +197,25 @@ pub trait IoLoc<T> {
>      fn offset(&self) -> usize;
>  }
>  
> +/// Implements [`IoLoc<$ty>`] for [`usize`], allowing to use `usize` as a parameter of
> +/// [`Io::read`] and [`Io::write`].
> +macro_rules! impl_usize_ioloc {
> +    ($($ty:ty),*) => {
> +        $(
> +            impl IoLoc<$ty> for usize {
> +                type IoType = $ty;
> +

#[inline(always)]

the fact that this is a pointer is somewhat uneasy to me. I wonder if Clippy
with its inlining tweak would cause optimisation failure here.

Could this be just `fn offset(self)`?

The rest LGTM.

Best,
Gary

> +                fn offset(&self) -> usize {
> +                    *self
> +                }
> +            }
> +        )*
> +    }
> +}
> +
> +// Provide the ability to read any primitive type from a [`usize`].
> +impl_usize_ioloc!(u8, u16, u32, u64);
> +
>  /// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
>  /// can perform I/O operations on regions of memory.
>  ///
> @@ -241,10 +260,7 @@ fn try_read8(&self, offset: usize) -> Result<u8>
>      where
>          Self: IoCapable<u8>,
>      {
> -        let address = self.io_addr::<u8>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        Ok(unsafe { self.io_read(address) })
> +        self.try_read(offset)
>      }
>  
>      /// Fallible 16-bit read with runtime bounds check.
> @@ -253,10 +269,7 @@ fn try_read16(&self, offset: usize) -> Result<u16>
>      where
>          Self: IoCapable<u16>,
>      {
> -        let address = self.io_addr::<u16>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        Ok(unsafe { self.io_read(address) })
> +        self.try_read(offset)
>      }
>  
>      /// Fallible 32-bit read with runtime bounds check.
> @@ -265,10 +278,7 @@ fn try_read32(&self, offset: usize) -> Result<u32>
>      where
>          Self: IoCapable<u32>,
>      {
> -        let address = self.io_addr::<u32>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        Ok(unsafe { self.io_read(address) })
> +        self.try_read(offset)
>      }
>  
>      /// Fallible 64-bit read with runtime bounds check.
> @@ -277,10 +287,7 @@ fn try_read64(&self, offset: usize) -> Result<u64>
>      where
>          Self: IoCapable<u64>,
>      {
> -        let address = self.io_addr::<u64>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        Ok(unsafe { self.io_read(address) })
> +        self.try_read(offset)
>      }
>  
>      /// Fallible 8-bit write with runtime bounds check.
> @@ -289,11 +296,7 @@ fn try_write8(&self, value: u8, offset: usize) -> Result
>      where
>          Self: IoCapable<u8>,
>      {
> -        let address = self.io_addr::<u8>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        unsafe { self.io_write(value, address) };
> -        Ok(())
> +        self.try_write(offset, value)
>      }
>  
>      /// Fallible 16-bit write with runtime bounds check.
> @@ -302,11 +305,7 @@ fn try_write16(&self, value: u16, offset: usize) -> Result
>      where
>          Self: IoCapable<u16>,
>      {
> -        let address = self.io_addr::<u16>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        unsafe { self.io_write(value, address) };
> -        Ok(())
> +        self.try_write(offset, value)
>      }
>  
>      /// Fallible 32-bit write with runtime bounds check.
> @@ -315,11 +314,7 @@ fn try_write32(&self, value: u32, offset: usize) -> Result
>      where
>          Self: IoCapable<u32>,
>      {
> -        let address = self.io_addr::<u32>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        unsafe { self.io_write(value, address) };
> -        Ok(())
> +        self.try_write(offset, value)
>      }
>  
>      /// Fallible 64-bit write with runtime bounds check.
> @@ -328,11 +323,7 @@ fn try_write64(&self, value: u64, offset: usize) -> Result
>      where
>          Self: IoCapable<u64>,
>      {
> -        let address = self.io_addr::<u64>(offset)?;
> -
> -        // SAFETY: `address` has been validated by `io_addr`.
> -        unsafe { self.io_write(value, address) };
> -        Ok(())
> +        self.try_write(offset, value)
>      }
>  
>      /// Infallible 8-bit read with compile-time bounds check.
> @@ -341,10 +332,7 @@ fn read8(&self, offset: usize) -> u8
>      where
>          Self: IoKnownSize + IoCapable<u8>,
>      {
> -        let address = self.io_addr_assert::<u8>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_read(address) }
> +        self.read(offset)
>      }
>  
>      /// Infallible 16-bit read with compile-time bounds check.
> @@ -353,10 +341,7 @@ fn read16(&self, offset: usize) -> u16
>      where
>          Self: IoKnownSize + IoCapable<u16>,
>      {
> -        let address = self.io_addr_assert::<u16>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_read(address) }
> +        self.read(offset)
>      }
>  
>      /// Infallible 32-bit read with compile-time bounds check.
> @@ -365,10 +350,7 @@ fn read32(&self, offset: usize) -> u32
>      where
>          Self: IoKnownSize + IoCapable<u32>,
>      {
> -        let address = self.io_addr_assert::<u32>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_read(address) }
> +        self.read(offset)
>      }
>  
>      /// Infallible 64-bit read with compile-time bounds check.
> @@ -377,10 +359,7 @@ fn read64(&self, offset: usize) -> u64
>      where
>          Self: IoKnownSize + IoCapable<u64>,
>      {
> -        let address = self.io_addr_assert::<u64>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_read(address) }
> +        self.read(offset)
>      }
>  
>      /// Infallible 8-bit write with compile-time bounds check.
> @@ -389,10 +368,7 @@ fn write8(&self, value: u8, offset: usize)
>      where
>          Self: IoKnownSize + IoCapable<u8>,
>      {
> -        let address = self.io_addr_assert::<u8>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_write(value, address) }
> +        self.write(offset, value)
>      }
>  
>      /// Infallible 16-bit write with compile-time bounds check.
> @@ -401,10 +377,7 @@ fn write16(&self, value: u16, offset: usize)
>      where
>          Self: IoKnownSize + IoCapable<u16>,
>      {
> -        let address = self.io_addr_assert::<u16>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_write(value, address) }
> +        self.write(offset, value)
>      }
>  
>      /// Infallible 32-bit write with compile-time bounds check.
> @@ -413,10 +386,7 @@ fn write32(&self, value: u32, offset: usize)
>      where
>          Self: IoKnownSize + IoCapable<u32>,
>      {
> -        let address = self.io_addr_assert::<u32>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_write(value, address) }
> +        self.write(offset, value)
>      }
>  
>      /// Infallible 64-bit write with compile-time bounds check.
> @@ -425,13 +395,28 @@ fn write64(&self, value: u64, offset: usize)
>      where
>          Self: IoKnownSize + IoCapable<u64>,
>      {
> -        let address = self.io_addr_assert::<u64>(offset);
> -
> -        // SAFETY: `address` has been validated by `io_addr_assert`.
> -        unsafe { self.io_write(value, address) }
> +        self.write(offset, value)
>      }
>  
>      /// Generic fallible read with runtime bounds check.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Read a primitive type from an I/O address:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_reads(io: &Mmio) -> Result {
> +    ///     // 32-bit read from address `0x10`.
> +    ///     let v: u32 = io.try_read(0x10)?;
> +    ///
> +    ///     // 8-bit read from address `0xfff`.
> +    ///     let v: u8 = io.try_read(0xfff)?;
> +    ///
> +    ///     Ok(())
> +    /// }
> +    /// ```
>      #[inline(always)]
>      fn try_read<T, L>(&self, location: L) -> Result<T>
>      where
> @@ -445,6 +430,24 @@ fn try_read<T, L>(&self, location: L) -> Result<T>
>      }
>  
>      /// Generic fallible write with runtime bounds check.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Write a primitive type to an I/O address:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_writes(io: &Mmio) -> Result {
> +    ///     // 32-bit write of value `1` at address `0x10`.
> +    ///     io.try_write(0x10, 1u32)?;
> +    ///
> +    ///     // 8-bit write of value `0xff` at address `0xfff`.
> +    ///     io.try_write(0xfff, 0xffu8)?;
> +    ///
> +    ///     Ok(())
> +    /// }
> +    /// ```
>      #[inline(always)]
>      fn try_write<T, L>(&self, location: L, value: T) -> Result
>      where
> @@ -464,6 +467,20 @@ fn try_write<T, L>(&self, location: L, value: T) -> Result
>      ///
>      /// Caution: this does not perform any synchronization. Race conditions can occur in case of
>      /// concurrent access.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Read the u32 value at address `0x10`, increment it, and store the updated value back:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_update(io: &Mmio<0x1000>) -> Result {
> +    ///     io.try_update(0x10, |v: u32| {
> +    ///         v + 1
> +    ///     })
> +    /// }
> +    /// ```
>      #[inline(always)]
>      fn try_update<T, L, F>(&self, location: L, f: F) -> Result
>      where
> @@ -484,6 +501,22 @@ fn try_update<T, L, F>(&self, location: L, f: F) -> Result
>      }
>  
>      /// Generic infallible read with compile-time bounds check.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Read a primitive type from an I/O address:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_reads(io: &Mmio<0x1000>) {
> +    ///     // 32-bit read from address `0x10`.
> +    ///     let v: u32 = io.read(0x10);
> +    ///
> +    ///     // 8-bit read from the top of the I/O space.
> +    ///     let v: u8 = io.read(0xfff);
> +    /// }
> +    /// ```
>      #[inline(always)]
>      fn read<T, L>(&self, location: L) -> T
>      where
> @@ -497,6 +530,22 @@ fn read<T, L>(&self, location: L) -> T
>      }
>  
>      /// Generic infallible write with compile-time bounds check.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Write a primitive type to an I/O address:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_writes(io: &Mmio<0x1000>) {
> +    ///     // 32-bit write of value `1` at address `0x10`.
> +    ///     io.write(0x10, 1u32);
> +    ///
> +    ///     // 8-bit write of value `0xff` at the top of the I/O space.
> +    ///     io.write(0xfff, 0xffu8);
> +    /// }
> +    /// ```
>      #[inline(always)]
>      fn write<T, L>(&self, location: L, value: T)
>      where
> @@ -514,6 +563,20 @@ fn write<T, L>(&self, location: L, value: T)
>      ///
>      /// Caution: this does not perform any synchronization. Race conditions can occur in case of
>      /// concurrent access.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Read the u32 value at address `0x10`, increment it, and store the updated value back:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_update(io: &Mmio<0x1000>) {
> +    ///     io.update(0x10, |v: u32| {
> +    ///         v + 1
> +    ///     })
> +    /// }
> +    /// ```
>      #[inline(always)]
>      fn update<T, L, F>(&self, location: L, f: F)
>      where