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

"Alexandre Courbot" <[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 Fri Jun 26, 2026 at 11:45 PM JST, Gary Guo wrote:
> From: Laura Nao <[email protected]>
>
> 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 <[email protected]>
> [ Rebased and adapted on top of I/O rework. - Gary ]
> Co-developed-by: Gary Guo <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
> ---
>  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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.