Re: [PATCH v6 RESEND 4/7] rust: io: factor common I/O helpers into Io trait

[Zhi Wang <zhiw@nvidia.com>]

On Fri, 14 Nov 2025 12:58:31 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Mon, Nov 10, 2025 at 10:41:16PM +0200, Zhi Wang wrote:
> > The previous Io<SIZE> type combined both the generic I/O access
> > helpers and MMIO implementation details in a single struct.
> > 
> > To establish a cleaner layering between the I/O interface and its
> > concrete backends, paving the way for supporting additional I/O
> > mechanisms in the future, Io<SIZE> need to be factored.
> > 
> > Factor the common helpers into a new Io trait, and move the
> > MMIO-specific logic into a dedicated Mmio<SIZE> type implementing
> > that trait. Rename the IoRaw to MmioRaw and update the bus MMIO
> > implementations to use MmioRaw.
> > 
> > No functional change intended.
> > 
> > Cc: Alexandre Courbot <acourbot@nvidia.com>
> > Cc: Bjorn Helgaas <helgaas@kernel.org>
> > Cc: Danilo Krummrich <dakr@kernel.org>
> > Cc: John Hubbard <jhubbard@nvidia.com>
> > Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> 
> This defines three traits:
> 
> * Io
> * IoInfallible: Io
> * IoFallible: Io
> 
> This particular split says that there are going to be cases where we
> implement IoInfallible only, cases where we implement IoFallible only,
> and maybe cases where we implement both.
> 
> And the distiction between them is whether the bounds check is runtime
> or compile-time.
> 
> But this doesn't make much sense to me. Surely any Io resource that
> can provide compile-time checked io can also provide runtime-checked
> io, so maybe IoFallible should extend IoInfallible?
> 
> And why are these separate traits at all? Why not support both
> compile-time and runtime-checked IO always?
>

Hi Alice:

Thanks for comments. I did have a version that PCI configuration space
only have fallible accessors because I thought the device can be
unplugged or a VF might fail its FLR and get unresponsive, so the driver
may need to check the return all the time. And Danilo's comments were
let's have the infallible accessors for PCI configuration space and add
them later if some driver needs it. [1]

I am open to either options. like have both or having infallibles first
and fallibles later.

> I noticed also that the trait does not have methods for 64-bit writes,
> and that these are left as inherent methods on Mmio.
> 
> The traits that would make sense to me are these:
> 
> * Io
> * Io64: Io
> 

Hehe. I had the same idea here [2]:

> Io trait - Main trait + 32-bit access
>  | 
>  | -- Common address/bound checks
>  |
>  |	(accessor traits)
>  | -- Io Fallible trait - (MMIO backend implements)
>  | -- Io Infallible trait - (MMIO/ConfigSpace backend implements this)
>  |
>  | -- Io64 trait - For backend supports 64 bit access
> 	   |      (accessor traits)
>          | -- Io64 Faillable trait (MMIO backend implements this)
> 	   | -- Io64 Infallible trait (MMIO backend implements this)

I am struggling with how many IO backends actually need 64bit read/write
other than MMIO backend. E.g. SPI, I2C. The conclusion we had so far was
we can add it at any time when someone need it. If we think this is
necessary, I can add it in the next spin.

[1] https://lore.kernel.org/all/DE00WIVOSYC2.2CAGWUYWE6FZ@kernel.org/
[2]
https://lore.kernel.org/all/e7a75899-be93-4f0f-9c9f-0d63d03f4806@kernel.org/

> where Io provides everything the three traits you have now provides,
> and Io64 provides the 64-bit operations. That way, everything needs to
> support operations of various sizes with both compile-time and
> runtime-checked bounds, but types may opt-in to providing 64-bit ops.
> 
> Thoughts?
> 
> Alice