Re: [PATCH v4 09/20] rust: io: use view types instead of addresses for `Io`

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

On Fri Jun 12, 2026 at 1:28 AM JST, Gary Guo wrote:
> Currently, `io_read` and `io_write` methods require the exact type of `Io`
> plus an address. This means that they need to be monomorphized for each
> different `Io` instance. This also means that multiple I/O implementors for
> the same I/O kind needs to duplicate implementation (e.g. `Mmio` and
> `MmioOwned`).
>
> Create a new `IoBackend` trait and define these operations on it instead.
> The operations are just going to receive a view type and operate on them.
> This has the additional advantage that the invariants can be moved from the
> trait (and guaranteed via `unsafe`) to type invariants on the canonical
> view types of the backends, so `io_read` and `io_write` can be safe.
>
> Note that view type is needed; addresses are insufficient in this
> designk, as they do not carry sufficient information. For example,

typo: design

> `ConfigSpace` needs `&pci::Device` in addition to the address.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  rust/kernel/io.rs     | 345 ++++++++++++++++++++++++++------------------------
>  rust/kernel/pci/io.rs |  70 ++++++----
>  2 files changed, 224 insertions(+), 191 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 3ac8b396f5a7..e422a5ff2a5e 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -244,6 +244,38 @@ const fn offset_valid<U>(base: usize, offset: usize, size: usize) -> bool {
>      }
>  }
>  
> +/// I/O backends.
> +///
> +/// This is an abstract representation to be implemented by arbitrary I/O
> +/// backends (e.g. MMIO, PCI config space, etc.).
> +///
> +/// The base trait only defines the projection operations; which I/O methods are available depends
> +/// on which [`IoCapable<T>`] traits are implemented for the type. For example, for MMIO regions,
> +/// all widths (u8, u16, u32, and u64 on 64-bit systems) are typically supported. For PCI
> +/// configuration space, u8, u16, and u32 are supported but u64 is not.
> +///
> +/// This trait is separate from the `Io` trait as multiple different I/O types may share the same
> +/// operation.
> +pub trait IoBackend {
> +    /// View type for this I/O backend.
> +    type View<'a, T: ?Sized + KnownSize>: Io<'a, Backend = Self, Target = T>;
> +
> +    /// Convert a `view` to a raw pointer for projection.
> +    fn as_ptr<'a, T: ?Sized + KnownSize>(view: Self::View<'a, T>) -> *mut T;

Same as the previous patch, this pointer is not necessarily
dereferencable (e.g. for `pci::ConfigSpace`). This should probably be
mentioned, or maybe we can use a newtype to prevent dereferencing?

> +
> +    /// Project `view` to its subregion indicated by `ptr`.
> +    ///
> +    /// If input `view` is valid, returned view must also be valid.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must be a projection of `Self::as_ptr(view)`.
> +    unsafe fn project_view<'a, T: ?Sized + KnownSize, U: ?Sized + KnownSize>(
> +        view: Self::View<'a, T>,
> +        ptr: *mut U,
> +    ) -> Self::View<'a, U>;
> +}
> +
>  /// Trait indicating that an I/O backend supports operations of a certain type and providing an
>  /// implementation for these operations.
>  ///
> @@ -252,22 +284,12 @@ const fn offset_valid<U>(base: usize, offset: usize, size: usize) -> bool {
>  /// For example, a PCI configuration space may implement `IoCapable<u8>`, `IoCapable<u16>`,
>  /// and `IoCapable<u32>`, but not `IoCapable<u64>`, while an MMIO region on a 64-bit
>  /// system might implement all four.
> -pub trait IoCapable<T> {
> -    /// Performs an I/O read of type `T` at `address` and returns the result.
> -    ///
> -    /// # Safety
> -    ///
> -    /// - The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`.
> -    /// - `address` must be aligned.
> -    unsafe fn io_read(self, address: usize) -> T;
> +pub trait IoCapable<T>: IoBackend {
> +    /// Performs an I/O read of type `T` at `view` and returns the result.
> +    fn io_read<'a>(view: Self::View<'a, T>) -> T;
>  
> -    /// Performs an I/O write of `value` at `address`.
> -    ///
> -    /// # Safety
> -    ///
> -    /// - The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`.
> -    /// - `address` must be aligned.
> -    unsafe fn io_write(self, value: T, address: usize);
> +    /// Performs an I/O write of `value` at `view`.
> +    fn io_write<'a>(view: Self::View<'a, T>, value: T);
>  }
>  
>  /// Describes a given I/O location: its offset, width, and type to convert the raw value from and
> @@ -319,56 +341,54 @@ 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.
>  ///
> -/// This is an abstract representation to be implemented by arbitrary I/O
> -/// backends (e.g. MMIO, PCI config space, etc.).
> -///
>  /// The [`Io`] trait provides:
> -/// - Base address and size information
> +/// - 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 which [`IoCapable<T>`] traits
> -/// are implemented for the type.
> -///
> -/// # Examples
> -///
> -/// For MMIO regions, all widths (u8, u16, u32, and u64 on 64-bit systems) are typically
> -/// supported. For PCI configuration space, u8, u16, and u32 are supported but u64 is not.
> -pub trait Io: Copy {
> +/// Which I/O methods are available depends on the associated [`IoBackend`] implementation.
> +pub trait Io<'a>: Copy {
> +    /// Type that defines all I/O operations.
> +    type Backend: IoBackend;
> +
>      /// Type of this I/O region. For untyped regions, [`Region`] can be used.
>      type Target: ?Sized + KnownSize;
>  
> -    /// Returns the base address of this mapping.
> -    fn addr(self) -> usize;
> -
> -    /// Returns the maximum size of this mapping.
> -    fn maxsize(self) -> usize;
> +    /// Return a view that covers the full region.
> +    fn as_view(self) -> <Self::Backend as IoBackend>::View<'a, Self::Target>;
>  
> -    /// Returns the absolute I/O address for a given `offset`,
> -    /// performing compile-time bound checks.
> +    /// Returns a view for a given `offset`, performing compile-time bound checks.
>      // Always inline to optimize out error path of `build_assert`.
>      #[inline(always)]
> -    fn io_addr_assert<U>(self, offset: usize) -> usize {
> -        // We cannot check alignment with `offset_valid` using `self.addr()`. So set 0 for it and
> +    fn io_addr_assert<U>(self, offset: usize) -> <Self::Backend as IoBackend>::View<'a, U> {

Since this doesn't return an address anymore, should it be renamed?

> +        // We cannot check alignment with `offset_valid` using `ptr.addr()`. So set 0 for it and
>          // ensure alignment by checking that the alignment of `U` is smaller or equal to the
>          // alignment of `Self::Target`.
>          const_assert!(Alignment::of::<U>().as_usize() <= Self::Target::MIN_ALIGN.as_usize());
>          build_assert!(offset_valid::<U>(0, offset, Self::Target::MIN_SIZE));
>  
> -        self.addr() + offset
> +        let view = self.as_view();
> +        let ptr = Self::Backend::as_ptr(view);
> +        let projected_ptr = ptr.cast::<U>().wrapping_byte_add(offset);
> +        // SAFETY: `offset_valid` checks for size and alignment and therefore `projected_ptr` is a
> +        // valid projection.
> +        unsafe { Self::Backend::project_view(view, projected_ptr) }
>      }
>  
> -    /// Returns the absolute I/O address for a given `offset`,
> -    /// performing runtime bound checks.
> +    /// Returns a view for a given `offset`, performing runtime bound checks.
>      #[inline]
> -    fn io_addr<U>(self, offset: usize) -> Result<usize> {
> -        if !offset_valid::<U>(self.addr(), offset, self.maxsize()) {
> +    fn io_addr<U>(self, offset: usize) -> Result<<Self::Backend as IoBackend>::View<'a, U>> {

Same here.

And potentially, a more serious issue: `io_addr_assert` and `io_addr`
remain part of `Io`, which is a public trait. They only verify size and
alignment for `U`, not whether a projection of `U` at `offset` is
structurally valid. AFAICT this remains that way by the end of the
series, so users are able to call `io_addr*` to create and use invalid
projections.

Moving `io_addr*` out of the trait and into local helpers should be
enough to close that loophole.

Also, (and not entirely sure of it because I haven't completely wrapped
my head around the issue yet), we might need to seal or otherwise
restrict `IoLoc` so external code cannot create arbitrary
implementations that allow invalid projections.