Re: [PATCH v6 12/20] rust: io: move `Io` methods to extension trait

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

> On 6 Jul 2026, at 09:44, Gary Guo <[email protected]> wrote:
> 
> `Io` trait now has a single required method with many more provided
> methods. Provided methods may want to rely on their implementations to not
> be arbitrarily overridden by implementers for correctness or soundness.
> A good example is the `size` method, it may be relied by unsafe code and
> thus must be consistent with the metadata obtained from `as_ptr`.
> 
> Thus, create a new trait to host `size` method, extract existing provided
> methods to the new trait, and provide a blanket implementation. This
> pattern is used extensively in userspace Rust libraries e.g. `tokio` where
> `AsyncRead` has minimum methods and `AsyncReadExt` is what users mostly
> interact with.
> 
> To avoid changing all user imports, the base trait is renamed to `IoBase`
> and the newly added trait takes the existing `Io` name.
> 
> Reviewed-by: Alexandre Courbot <[email protected]>
> Suggested-by: Danilo Krummrich <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
> ---
> rust/kernel/devres.rs |  3 ++-
> rust/kernel/io.rs     | 34 ++++++++++++++++++++++++----------
> rust/kernel/io/mem.rs |  6 +++---
> rust/kernel/pci/io.rs |  6 +++---
> 4 files changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 3545ffc5345d..6e0b845b229b 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -68,6 +68,7 @@ struct Inner<T> {
> ///     devres::Devres,
> ///     io::{
> ///         Io,
> +///         IoBase,
> ///         Mmio,
> ///         MmioRaw,
> ///         MmioBackend,
> @@ -105,7 +106,7 @@ struct Inner<T> {
> ///     }
> /// }
> ///
> -/// impl<'a, const SIZE: usize> Io<'a> for &'a IoMem<SIZE> {
> +/// impl<'a, const SIZE: usize> IoBase<'a> for &'a IoMem<SIZE> {
> ///    type Backend = MmioBackend;
> ///    type Target = Region<SIZE>;
> ///
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index c7533d605678..ab970e8b2e78 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -223,7 +223,7 @@ fn io_view<'a, IO: Io<'a>, U>(
> /// operation.
> pub trait IoBackend {
>     /// View type for this I/O backend.
> -    type View<'a, T: ?Sized + KnownSize>: Io<'a, Backend = Self, Target = T>;
> +    type View<'a, T: ?Sized + KnownSize>: IoBase<'a, Backend = Self, Target = T>;
> 
>     /// Convert a `view` to a raw pointer for projection.
>     ///
> @@ -309,15 +309,12 @@ fn offset(self) -> usize {
> /// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
> /// can perform I/O operations on regions of memory.
> ///
> -/// The [`Io`] trait provides:
> -/// - Method to convert into [`IoBackend::View`].
> -/// - Helper methods for offset validation and address calculation
> -/// - Fallible (runtime checked) accessors for different data widths
> -///
> -/// Which I/O methods are available depends on the associated [`IoBackend`] implementation.
> +/// This trait defines which backend shall be used for I/O operations and provides a method to
> +/// convert into [`IoBackend::View`]. Users should use the [`Io`] trait which provides the actual
> +/// methods to perform I/O operations.
> ///
> /// This should be implemented on cheaply copyable handles, such as references or view types.
> -pub trait Io<'a>: Copy {
> +pub trait IoBase<'a>: Copy {
>     /// Type that defines all I/O operations.
>     type Backend: IoBackend;
> 
> @@ -326,6 +323,21 @@ pub trait Io<'a>: Copy {
> 
>     /// Return a view that covers the full region.
>     fn as_view(self) -> <Self::Backend as IoBackend>::View<'a, Self::Target>;
> +}
> +
> +/// Extension trait to provide I/O operation methods to types that implement [`IoBase`].
> +///
> +/// This trait provides:
> +/// - Helper methods for offset validation and address calculation
> +/// - Fallible (runtime checked) accessors for different data widths
> +///
> +/// Which I/O methods are available depends on the associated [`IoBackend`] implementation.
> +pub trait Io<'a>: IoBase<'a> {
> +    /// Returns the size of this I/O region.
> +    #[inline]
> +    fn size(self) -> usize {
> +        KnownSize::size(Self::Backend::as_ptr(self.as_view()))
> +    }
> 
>     /// Fallible 8-bit read with runtime bounds check.
>     #[inline(always)]
> @@ -779,6 +791,8 @@ fn update<T, L, F>(self, location: L, f: F)
>     }
> }
> 

IIUC this is what makes the methods non-overridable?

If so, consider leaving a comment? i.e.:

// Provided methods may want to rely on their implementations to not be
// arbitrarily overridden by implementers for correctness or soundness, thus,
// enforce it through this blanket implementation

Makes it easier to maintain later IMHO.

> +impl<'a, T: IoBase<'a>> Io<'a> for T {}
> +
> /// A view of memory-mapped I/O region.
> ///
> /// # Invariant
> @@ -819,7 +833,7 @@ 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<'a, T: ?Sized + KnownSize> Io<'a> for Mmio<'a, T> {
> +impl<'a, T: ?Sized + KnownSize> IoBase<'a> for Mmio<'a, T> {
>     type Backend = MmioBackend;
>     type Target = T;
> 
> @@ -920,7 +934,7 @@ unsafe fn project_view<'a, T: ?Sized + KnownSize, U: ?Sized + KnownSize>(
>     }
> }
> 
> -impl<'a, T: ?Sized + KnownSize> Io<'a> for RelaxedMmio<'a, T> {
> +impl<'a, T: ?Sized + KnownSize> IoBase<'a> for RelaxedMmio<'a, T> {
>     type Backend = RelaxedMmioBackend;
>     type Target = T;
> 
> diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
> index d9b3189d09b4..e95b769ebe47 100644
> --- a/rust/kernel/io/mem.rs
> +++ b/rust/kernel/io/mem.rs
> @@ -14,7 +14,7 @@
>             Region,
>             Resource, //
>         },
> -        Io,
> +        IoBase,
>         Mmio,
>         MmioBackend,
>         MmioRaw, //
> @@ -210,7 +210,7 @@ pub fn into_devres(self) -> Result<Devres<ExclusiveIoMem<'static, SIZE>>> {
>     }
> }
> 
> -impl<'a, const SIZE: usize> Io<'a> for &'a ExclusiveIoMem<'_, SIZE> {
> +impl<'a, const SIZE: usize> IoBase<'a> for &'a ExclusiveIoMem<'_, SIZE> {
>     type Backend = MmioBackend;
>     type Target = super::Region<SIZE>;
> 
> @@ -292,7 +292,7 @@ fn drop(&mut self) {
>     }
> }
> 
> -impl<'a, const SIZE: usize> Io<'a> for &'a IoMem<'_, SIZE> {
> +impl<'a, const SIZE: usize> IoBase<'a> for &'a IoMem<'_, SIZE> {
>     type Backend = MmioBackend;
>     type Target = super::Region<SIZE>;
> 
> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index 4be33ecb4192..4d1d0afdc491 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs
> @@ -8,8 +8,8 @@
>     device,
>     devres::Devres,
>     io::{
> -        Io,
>         IoBackend,
> +        IoBase,
>         IoCapable,
>         Mmio,
>         MmioBackend,
> @@ -144,7 +144,7 @@ fn io_write(view: ConfigSpace<'_, $ty>, value: $ty) {
> impl_config_space_io_capable!(u16, pci_read_config_word, pci_write_config_word);
> impl_config_space_io_capable!(u32, pci_read_config_dword, pci_write_config_dword);
> 
> -impl<'a, T: ?Sized + KnownSize> Io<'a> for ConfigSpace<'a, T> {
> +impl<'a, T: ?Sized + KnownSize> IoBase<'a> for ConfigSpace<'a, T> {
>     type Backend = ConfigSpaceBackend;
>     type Target = T;
> 
> @@ -267,7 +267,7 @@ fn drop(&mut self) {
>     }
> }
> 
> -impl<'a, const SIZE: usize> Io<'a> for &'a Bar<'_, SIZE> {
> +impl<'a, const SIZE: usize> IoBase<'a> for &'a Bar<'_, SIZE> {
>     type Backend = MmioBackend;
>     type Target = crate::io::Region<SIZE>;
> 
> 
> -- 
> 2.54.0
> 

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.