Messages in this thread |  | | Date | Thu, 26 Mar 2026 14:35:28 +0000 | | From | "Gary Guo" <> | | Subject | Re: [PATCH 3/8] rust: io: use pointer types instead of address |
| |
On Thu Mar 26, 2026 at 2:20 PM GMT, Andreas Hindborg wrote: > "Gary Guo" <gary@kernel.org> writes: > >> From: Gary Guo <gary@garyguo.net> >> >> This carries the size information with the pointer type and metadata, makes >> it possible to use I/O projections and paves the way for IO view types. >> >> With this change, minimum size information becomes available through types; >> so `KnownSize::MIN_SIZE` can be used and `IoKnownSize` trait is no longer >> necessary. The trait is kept for compatibility and can be removed when >> users stop using it for bounds. >> >> PCI config space uses only offsets and not pointers like MMIO; for this >> null pointers (with proper size metadata) is used. This is okay as I/O >> trait impl and I/O projections can operate on invalid pointers, and for PCI >> config space we will only use address info and ignore the provenance. >> >> The current safety comment on `io_read`/`io_write` does not cover the topic >> about alignment, although this is guaranteed by checks in `Io`. Add it so >> it can be relied on by implementor of `IoCapable`. >> >> Signed-off-by: Gary Guo <gary@garyguo.net> >> --- > > <cut> > >> +impl<T: Io + ?Sized> IoKnownSize for T {} >> >> /// Implements [`IoCapable`] on `$mmio` for `$ty` using `$read_fn` and `$write_fn`. >> macro_rules! impl_mmio_io_capable { >> ($mmio:ident, $(#[$attr:meta])* $ty:ty, $read_fn:ident, $write_fn:ident) => { >> $(#[$attr])* >> impl<T: ?Sized> IoCapable<$ty> for $mmio<T> { >> - unsafe fn io_read(&self, address: usize) -> $ty { >> + unsafe fn io_read(&self, address: *mut $ty) -> $ty { >> // SAFETY: By the trait invariant `address` is a valid address for MMIO operations. >> unsafe { bindings::$read_fn(address as *const c_void) } > > Should we change this to a `cast()`?
This is a `*mut T` -> `*const c_void` cast which needs `cast_const().cast::<c_void>` which I think is less readable than just `as`.
It's also existing code that I haven't touched in this commit.
> > Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org> > > Side question: Can you explain a situation where `IoLoc::offset` is not identity?
By identity, you mean the impl of `IoLoc for usize`? `IoLoc` is also implemented for registers generated by `register!()` macro.
Best, Gary
|  |