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 <[email protected]> > --- > 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.
