Re: [PATCH v6 04/20] rust: io: implement `Io` on reference types instead
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: > > Currently, `Io` is implemented on owned I/O objects (e.g. `Bar`). This is > going to change with I/O projections, as then `Io` needs to work both for > owned objects and views of them. Views are themselves reference-like > (however they obviously cannot be references, because they belong to a > different address space). > > To facilitate the change, change `Io` to be implemented on reference types > for the owned I/O objects, and make methods take `self` instead of `&self`. > When I/O views are implemented, we can then naturally implement `Io` for > these objects. > > Reviewed-by: Alexandre Courbot <[email protected]> > Signed-off-by: Gary Guo <[email protected]> > --- > rust/kernel/io.rs | 82 ++++++++++++++++++++++++++------------------------- > rust/kernel/pci/io.rs | 12 ++++---- > 2 files changed, 48 insertions(+), 46 deletions(-) > > diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs > index 87141eb07056..9f060dd29182 100644 > --- a/rust/kernel/io.rs > +++ b/rust/kernel/io.rs > @@ -223,7 +223,7 @@ pub trait IoCapable<T> { > /// > /// - 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; > + unsafe fn io_read(self, address: usize) -> T; > > /// Performs an I/O write of `value` at `address`. > /// > @@ -231,7 +231,7 @@ pub trait IoCapable<T> { > /// > /// - 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); > + unsafe fn io_write(self, value: T, address: usize); > } > > /// Describes a given I/O location: its offset, width, and type to convert the raw value from and > @@ -294,25 +294,27 @@ fn offset(self) -> usize { > /// Which I/O methods are available depends on which [`IoCapable<T>`] traits > /// are implemented for the type. > /// > +/// This should be implemented on cheaply copyable handles, such as references or view types. > +/// > /// # 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 { > +pub trait Io: Copy { > /// 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; > + fn addr(self) -> usize; > > /// Returns the maximum size of this mapping. > - fn maxsize(&self) -> usize; > + fn maxsize(self) -> usize; > > /// Returns the absolute I/O address 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 { > + 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 > // ensure alignment by checking that the alignment of `U` is smaller or equal to the > // alignment of `Self::Target`. > @@ -325,7 +327,7 @@ fn io_addr_assert<U>(&self, offset: usize) -> usize { > /// Returns the absolute I/O address for a given `offset`, > /// performing runtime bound checks. > #[inline] > - fn io_addr<U>(&self, offset: usize) -> Result<usize> { > + fn io_addr<U>(self, offset: usize) -> Result<usize> { > if !offset_valid::<U>(self.addr(), offset, self.maxsize()) { > return Err(EINVAL); > } > @@ -337,7 +339,7 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> { > > /// Fallible 8-bit read with runtime bounds check. > #[inline(always)] > - fn try_read8(&self, offset: usize) -> Result<u8> > + fn try_read8(self, offset: usize) -> Result<u8> > where > usize: IoLoc<Self::Target, u8, IoType = u8>, > Self: IoCapable<u8>, > @@ -347,7 +349,7 @@ fn try_read8(&self, offset: usize) -> Result<u8> > > /// Fallible 16-bit read with runtime bounds check. > #[inline(always)] > - fn try_read16(&self, offset: usize) -> Result<u16> > + fn try_read16(self, offset: usize) -> Result<u16> > where > usize: IoLoc<Self::Target, u16, IoType = u16>, > Self: IoCapable<u16>, > @@ -357,7 +359,7 @@ fn try_read16(&self, offset: usize) -> Result<u16> > > /// Fallible 32-bit read with runtime bounds check. > #[inline(always)] > - fn try_read32(&self, offset: usize) -> Result<u32> > + fn try_read32(self, offset: usize) -> Result<u32> > where > usize: IoLoc<Self::Target, u32, IoType = u32>, > Self: IoCapable<u32>, > @@ -367,7 +369,7 @@ fn try_read32(&self, offset: usize) -> Result<u32> > > /// Fallible 64-bit read with runtime bounds check. > #[inline(always)] > - fn try_read64(&self, offset: usize) -> Result<u64> > + fn try_read64(self, offset: usize) -> Result<u64> > where > usize: IoLoc<Self::Target, u64, IoType = u64>, > Self: IoCapable<u64>, > @@ -377,7 +379,7 @@ fn try_read64(&self, offset: usize) -> Result<u64> > > /// Fallible 8-bit write with runtime bounds check. > #[inline(always)] > - fn try_write8(&self, value: u8, offset: usize) -> Result > + fn try_write8(self, value: u8, offset: usize) -> Result > where > usize: IoLoc<Self::Target, u8, IoType = u8>, > Self: IoCapable<u8>, > @@ -387,7 +389,7 @@ fn try_write8(&self, value: u8, offset: usize) -> Result > > /// Fallible 16-bit write with runtime bounds check. > #[inline(always)] > - fn try_write16(&self, value: u16, offset: usize) -> Result > + fn try_write16(self, value: u16, offset: usize) -> Result > where > usize: IoLoc<Self::Target, u16, IoType = u16>, > Self: IoCapable<u16>, > @@ -397,7 +399,7 @@ fn try_write16(&self, value: u16, offset: usize) -> Result > > /// Fallible 32-bit write with runtime bounds check. > #[inline(always)] > - fn try_write32(&self, value: u32, offset: usize) -> Result > + fn try_write32(self, value: u32, offset: usize) -> Result > where > usize: IoLoc<Self::Target, u32, IoType = u32>, > Self: IoCapable<u32>, > @@ -407,7 +409,7 @@ fn try_write32(&self, value: u32, offset: usize) -> Result > > /// Fallible 64-bit write with runtime bounds check. > #[inline(always)] > - fn try_write64(&self, value: u64, offset: usize) -> Result > + fn try_write64(self, value: u64, offset: usize) -> Result > where > usize: IoLoc<Self::Target, u64, IoType = u64>, > Self: IoCapable<u64>, > @@ -417,7 +419,7 @@ fn try_write64(&self, value: u64, offset: usize) -> Result > > /// Infallible 8-bit read with compile-time bounds check. > #[inline(always)] > - fn read8(&self, offset: usize) -> u8 > + fn read8(self, offset: usize) -> u8 > where > usize: IoLoc<Self::Target, u8, IoType = u8>, > Self: IoCapable<u8>, > @@ -427,7 +429,7 @@ fn read8(&self, offset: usize) -> u8 > > /// Infallible 16-bit read with compile-time bounds check. > #[inline(always)] > - fn read16(&self, offset: usize) -> u16 > + fn read16(self, offset: usize) -> u16 > where > usize: IoLoc<Self::Target, u16, IoType = u16>, > Self: IoCapable<u16>, > @@ -437,7 +439,7 @@ fn read16(&self, offset: usize) -> u16 > > /// Infallible 32-bit read with compile-time bounds check. > #[inline(always)] > - fn read32(&self, offset: usize) -> u32 > + fn read32(self, offset: usize) -> u32 > where > usize: IoLoc<Self::Target, u32, IoType = u32>, > Self: IoCapable<u32>, > @@ -447,7 +449,7 @@ fn read32(&self, offset: usize) -> u32 > > /// Infallible 64-bit read with compile-time bounds check. > #[inline(always)] > - fn read64(&self, offset: usize) -> u64 > + fn read64(self, offset: usize) -> u64 > where > usize: IoLoc<Self::Target, u64, IoType = u64>, > Self: IoCapable<u64>, > @@ -457,7 +459,7 @@ fn read64(&self, offset: usize) -> u64 > > /// Infallible 8-bit write with compile-time bounds check. > #[inline(always)] > - fn write8(&self, value: u8, offset: usize) > + fn write8(self, value: u8, offset: usize) > where > usize: IoLoc<Self::Target, u8, IoType = u8>, > Self: IoCapable<u8>, > @@ -467,7 +469,7 @@ fn write8(&self, value: u8, offset: usize) > > /// Infallible 16-bit write with compile-time bounds check. > #[inline(always)] > - fn write16(&self, value: u16, offset: usize) > + fn write16(self, value: u16, offset: usize) > where > usize: IoLoc<Self::Target, u16, IoType = u16>, > Self: IoCapable<u16>, > @@ -477,7 +479,7 @@ fn write16(&self, value: u16, offset: usize) > > /// Infallible 32-bit write with compile-time bounds check. > #[inline(always)] > - fn write32(&self, value: u32, offset: usize) > + fn write32(self, value: u32, offset: usize) > where > usize: IoLoc<Self::Target, u32, IoType = u32>, > Self: IoCapable<u32>, > @@ -487,7 +489,7 @@ fn write32(&self, value: u32, offset: usize) > > /// Infallible 64-bit write with compile-time bounds check. > #[inline(always)] > - fn write64(&self, value: u64, offset: usize) > + fn write64(self, value: u64, offset: usize) > where > usize: IoLoc<Self::Target, u64, IoType = u64>, > Self: IoCapable<u64>, > @@ -518,7 +520,7 @@ fn write64(&self, value: u64, offset: usize) > /// } > /// ``` > #[inline(always)] > - fn try_read<T, L>(&self, location: L) -> Result<T> > + fn try_read<T, L>(self, location: L) -> Result<T> > where > L: IoLoc<Self::Target, T>, > Self: IoCapable<L::IoType>, > @@ -552,7 +554,7 @@ fn try_read<T, L>(&self, location: L) -> Result<T> > /// } > /// ``` > #[inline(always)] > - fn try_write<T, L>(&self, location: L, value: T) -> Result > + fn try_write<T, L>(self, location: L, value: T) -> Result > where > L: IoLoc<Self::Target, T>, > Self: IoCapable<L::IoType>, > @@ -598,7 +600,7 @@ fn try_write<T, L>(&self, location: L, value: T) -> Result > /// } > /// ``` > #[inline(always)] > - fn try_write_reg<T, L, V>(&self, value: V) -> Result > + fn try_write_reg<T, L, V>(self, value: V) -> Result > where > L: IoLoc<Self::Target, T>, > V: LocatedRegister<Self::Target, Location = L, Value = T>, > @@ -631,7 +633,7 @@ fn try_write_reg<T, L, V>(&self, value: V) -> Result > /// } > /// ``` > #[inline(always)] > - fn try_update<T, L, F>(&self, location: L, f: F) -> Result > + fn try_update<T, L, F>(self, location: L, f: F) -> Result > where > L: IoLoc<Self::Target, T>, > Self: IoCapable<L::IoType>, > @@ -670,7 +672,7 @@ fn try_update<T, L, F>(&self, location: L, f: F) -> Result > /// } > /// ``` > #[inline(always)] > - fn read<T, L>(&self, location: L) -> T > + fn read<T, L>(self, location: L) -> T > where > L: IoLoc<Self::Target, T>, > Self: IoCapable<L::IoType>, > @@ -702,7 +704,7 @@ fn read<T, L>(&self, location: L) -> T > /// } > /// ``` > #[inline(always)] > - fn write<T, L>(&self, location: L, value: T) > + fn write<T, L>(self, location: L, value: T) > where > L: IoLoc<Self::Target, T>, > Self: IoCapable<L::IoType>, > @@ -745,7 +747,7 @@ fn write<T, L>(&self, location: L, value: T) > /// } > /// ``` > #[inline(always)] > - fn write_reg<T, L, V>(&self, value: V) > + fn write_reg<T, L, V>(self, value: V) > where > L: IoLoc<Self::Target, T>, > V: LocatedRegister<Self::Target, Location = L, Value = T>, > @@ -778,7 +780,7 @@ fn write_reg<T, L, V>(&self, value: V) > /// } > /// ``` > #[inline(always)] > - fn update<T, L, F>(&self, location: L, f: F) > + fn update<T, L, F>(self, location: L, f: F) > where > L: IoLoc<Self::Target, T>, > Self: IoCapable<L::IoType>, > @@ -799,13 +801,13 @@ fn update<T, L, F>(&self, location: L, f: F) > 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> { > - unsafe fn io_read(&self, address: usize) -> $ty { > + impl<const SIZE: usize> IoCapable<$ty> for &$mmio<SIZE> { > + 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) } > } > > - unsafe fn io_write(&self, value: $ty, address: usize) { > + 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) } > } > @@ -826,18 +828,18 @@ unsafe fn io_write(&self, value: $ty, address: usize) { > writeq > ); > > -impl<const SIZE: usize> Io for Mmio<SIZE> { > +impl<'a, const SIZE: usize> Io for &'a Mmio<SIZE> { > type Target = Region<SIZE>; > > /// Returns the base address of this mapping. > #[inline] > - fn addr(&self) -> usize { > + fn addr(self) -> usize { > self.0.addr() > } > > /// Returns the maximum size of this mapping. > #[inline] > - fn maxsize(&self) -> usize { > + fn maxsize(self) -> usize { > self.0.maxsize() > } > } > @@ -864,16 +866,16 @@ pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self { > #[repr(transparent)] > pub struct RelaxedMmio<const SIZE: usize = 0>(Mmio<SIZE>); > > -impl<const SIZE: usize> Io for RelaxedMmio<SIZE> { > +impl<'a, const SIZE: usize> Io for &'a RelaxedMmio<SIZE> { > type Target = Region<SIZE>; > > #[inline] > - fn addr(&self) -> usize { > + fn addr(self) -> usize { > self.0.addr() > } > > #[inline] > - fn maxsize(&self) -> usize { > + fn maxsize(self) -> usize { > self.0.maxsize() > } > } > diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs > index b4996aa059d8..505305cd9b86 100644 > --- a/rust/kernel/pci/io.rs > +++ b/rust/kernel/pci/io.rs > @@ -79,8 +79,8 @@ pub struct ConfigSpace<'a, S: ?Sized + ConfigSpaceKind = Extended> { > /// Implements [`IoCapable`] on [`ConfigSpace`] for `$ty` using `$read_fn` and `$write_fn`. > macro_rules! impl_config_space_io_capable { > ($ty:ty, $read_fn:ident, $write_fn:ident) => { > - impl<'a, S: ?Sized + ConfigSpaceKind> IoCapable<$ty> for ConfigSpace<'a, S> { > - unsafe fn io_read(&self, address: usize) -> $ty { > + impl<'a, S: ?Sized + ConfigSpaceKind> IoCapable<$ty> for &ConfigSpace<'a, S> { > + unsafe fn io_read(self, address: usize) -> $ty { > let mut val: $ty = 0; > > // Return value from C function is ignored in infallible accessors. > @@ -94,7 +94,7 @@ unsafe fn io_read(&self, address: usize) -> $ty { > val > } > > - unsafe fn io_write(&self, value: $ty, address: usize) { > + unsafe fn io_write(self, value: $ty, address: usize) { > // Return value from C function is ignored in infallible accessors. > let _ret = > // SAFETY: By the type invariant `self.pdev` is a valid address. > @@ -112,18 +112,18 @@ unsafe fn io_write(&self, value: $ty, address: usize) { > 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, S: ?Sized + ConfigSpaceKind> Io for ConfigSpace<'a, S> { > +impl<'a, S: ?Sized + ConfigSpaceKind> Io for &ConfigSpace<'a, S> { > type Target = S; > > /// Returns the base address of the I/O region. It is always 0 for configuration space. > #[inline] > - fn addr(&self) -> usize { > + fn addr(self) -> usize { > 0 > } > > /// Returns the maximum size of the configuration space. > #[inline] > - fn maxsize(&self) -> usize { > + fn maxsize(self) -> usize { > self.pdev.cfg_size().into_raw() > } > } > > -- > 2.54.0 > > Reviewed-by: Daniel Almeida <[email protected]>