Re: [PATCH v5 14/20] rust: io: add I/O backend for system memory with volatile access

["Alexandre Courbot" <acourbot@nvidia.com>]

On Fri Jun 26, 2026 at 11:45 PM JST, Gary Guo wrote:
> From: Laura Nao <laura.nao@collabora.com>
>
> Add `SysMem`, an `Io` trait implementation for kernel virtual address
> ranges. It uses volatile accessors to provide safe access to shared
> memory that may be concurrently accessed by external hardware. Implement
> `IoCapable` for `u8`, `u16`, `u32`, and `u64` (for 64-bit system).
>
> This can be used for instead of `Coherent` for cases where a different

typo: "can be used instead of".

> layer takes care of mapping the system memory to the device (e.g. dma-buf
> or GPUVM).
>
> Signed-off-by: Laura Nao <laura.nao@collabora.com>
> [ Rebased and adapted on top of I/O rework. - Gary ]
> Co-developed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  rust/kernel/io.rs | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 122 insertions(+)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 96962498af77..5c06785facea 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -1034,6 +1034,128 @@ pub fn relaxed(self) -> RelaxedMmio<'a, T> {
>  #[cfg(CONFIG_64BIT)]
>  impl_mmio_io_capable!(RelaxedMmioBackend, u64, readq_relaxed, writeq_relaxed);
>  
> +/// I/O Backend for system memory.
> +pub struct SysMemBackend;
> +
> +impl IoBackend for SysMemBackend {
> +    type View<'a, T: ?Sized + KnownSize> = SysMem<'a, T>;
> +
> +    #[inline]
> +    fn as_ptr<'a, T: ?Sized + KnownSize>(view: Self::View<'a, T>) -> *mut T {
> +        view.ptr
> +    }
> +
> +    #[inline]
> +    unsafe fn project_view<'a, T: ?Sized + KnownSize, U: ?Sized + KnownSize>(
> +        _view: Self::View<'a, T>,
> +        ptr: *mut U,
> +    ) -> Self::View<'a, U> {
> +        // INVARIANT: Per safety requirement, `ptr` is projection from `view`, so it is also a valid
> +        // kernel accessible memory region.
> +        SysMem {
> +            ptr,
> +            phantom: PhantomData,
> +        }
> +    }
> +}
> +
> +/// Implements [`IoCapable`] on `SysMemBackend` for `$ty` using `read_volatile` and
> +/// `write_volatile`.
> +macro_rules! impl_sysmem_io_capable {
> +    ($ty:ty) => {
> +        impl IoCapable<$ty> for SysMemBackend {
> +            #[inline]
> +            fn io_read(view: SysMem<'_, $ty>) -> $ty {
> +                // SAFETY:
> +                // - Per type invariant, `ptr` is valid 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.
> +                // - The macro is only used on primitives so all bit patterns are valid.
> +                unsafe { view.ptr.read_volatile() }
> +            }
> +
> +            #[inline]
> +            fn io_write(view: SysMem<'_, $ty>, value: $ty) {
> +                // SAFETY:
> +                // - Per type invariant, `ptr` is valid and aligned.
> +                // - Using write_volatile() here so that race with hardware is well-defined.
> +                // - Using write_volatile() here is not sound if it races with other CPU per Rust
> +                //   rules, but this is allowed per LKMM.
> +                unsafe { view.ptr.write_volatile(value) }
> +            }
> +        }
> +    };
> +}
> +
> +impl_sysmem_io_capable!(u8);
> +impl_sysmem_io_capable!(u16);
> +impl_sysmem_io_capable!(u32);
> +#[cfg(CONFIG_64BIT)]
> +impl_sysmem_io_capable!(u64);
> +
> +/// System memory region.

Maybe "A view of a system memory region" for consistency with the other
view types.

Follow-up idea: `SysMem` and `IoSysMap` could maybe be moved into their
own module to de-clutter `io.rs` a bit.

> +///
> +/// Provides `Io` trait implementation for kernel virtual address ranges,
> +/// using volatile read/write to safely access shared memory that may be
> +/// concurrently accessed by external hardware.
> +///
> +/// # Invariants
> +///
> +/// `self.ptr.addr() .. self.ptr.addr() + KnownSize::size(self.ptr)` is valid and aligned kernel
> +/// accessible memory region for the lifetime `'a`.
> +pub struct SysMem<'a, T: ?Sized> {
> +    ptr: *mut T,
> +    phantom: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Copy for SysMem<'_, T> {}
> +impl<T: ?Sized> Clone for SysMem<'_, T> {
> +    #[inline]
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +// SAFETY: `SysMem<'_, T>` is conceptually `&T` but in I/O memory.

IIUC `SysMem` is not I/O memory. :)

> +unsafe impl<T: ?Sized + Sync> Send for SysMem<'_, T> {}
> +
> +// SAFETY: `SysMem<'_, T>` is conceptually `&T` but in I/O memory.

Same here.

With these considered,

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>