Re: [PATCH 6/8] rust: io: add `read_val` and `write_val` function on I/O view

[Andreas Hindborg <a.hindborg@kernel.org>]

"Gary Guo" <gary@kernel.org> writes:

> From: Gary Guo <gary@garyguo.net>
>
> When a view is narrowed to just a primitive, these functions provide a way
> to access them using the `IoCapable` trait. This is used to provide
> `io_read!` and `io_write!` macros, which are generalized version of current
> `dma_read!` and `dma_write!` macro (that works on `Coherent` only, but not
> subview of them, or other I/O backends).
>
> For DMA coherent objects, `IoCapable` is only implemented for atomically
> accessible primitives; this is because `read_volatile`/`write_volatile` can
> behave undesirably for aggregates; LLVM may turn them to multiple
> instructions to access parts and assemble, or could combine them to a
> single instruction.
>
> The ability to read/write aggregates (when atomicity is of no concern),
> could be implemented with copying primitives (e.g. memcpy_{from,to}io) in
> the future.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  rust/kernel/dma.rs | 40 +++++++++++++++++++
>  rust/kernel/io.rs  | 95 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index ae2939abc166..fcdf85f5ed50 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -877,6 +877,46 @@ fn as_ptr(&self) -> *mut Self::Type {
>      }
>  }
>
> +/// Implements [`IoCapable`] on `Coherent` for `$ty` using `read_volatile` and `write_volatile`.
> +macro_rules! impl_coherent_io_capable {
> +    ($(#[$attr:meta])* $ty:ty) => {
> +        $(#[$attr])*
> +        impl<T: ?Sized + KnownSize> IoCapable<$ty> for Coherent<T> {
> +            #[inline]
> +            unsafe fn io_read(&self, address: *mut $ty) -> $ty {
> +                // SAFETY:
> +                // - By the safety precondition, the address is within bounds of the allocation and
> +                //   aligned.
> +                // - Using read_volatile() here so that race with hardware is well-defined.
> +                // - Using read_volatile() here is not sound if it races with other CPU per Rust
> +                //   rules, but this is allowed per LKMM.

You are not covering how we satisfy "Reading from src must produce a
properly initialized value of type T." I assume you need a bound on `T: FromBytes`?


Best regards,
Andreas Hindborg