Re: [PATCH v5 15/20] rust: io: implement a view type for `Coherent`

[Alexandre Courbot]

On Fri Jun 26, 2026 at 11:45 PM JST, Gary Guo wrote:
> Implement a `CoherentView` type which is a view of `Coherent`. To be able
> to give out DMA handles, the view type contains both CPU and DMA pointers,
> and the projection method projects both at once.
>
> Delegate most of the `Io` implementation to `SysMemBackend`. Provide a
> method to erase the DMA handle and give out a `SysMem` view, if the user
> does not need the `dma_handle`.
>
> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>

Of the Sashiko comments, the second one (adding exclusivity requirement
to the safety comment) looks actionable; with that:

Reviewed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>

(one nit below)

> ---
>  rust/kernel/dma.rs | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 135 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 200def84fb69..ab6504910e4f 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -14,14 +14,21 @@
>      },
>      error::to_result,
>      fs::file,
> +    io::{
> +        IoBackend,
> +        IoBase,
> +        IoCapable,
> +        SysMem,
> +        SysMemBackend, //
> +    },
>      prelude::*,
>      ptr::KnownSize,
>      sync::aref::ARef,
>      transmute::{
>          AsBytes,
>          FromBytes, //
> -    }, //
> -    uaccess::UserSliceWriter,
> +    },
> +    uaccess::UserSliceWriter, //
>  };
>  use core::{
>      ops::{
> @@ -1133,6 +1140,132 @@ unsafe impl Send for CoherentHandle {}
>  // plain `Copy` values.
>  unsafe impl Sync for CoherentHandle {}
>  
> +/// View type for `Coherent`.
> +///
> +/// This is same as [`SysMem`] but with additional information that allows handing out a DMA handle.
> +pub struct CoherentView<'a, T: ?Sized> {
> +    cpu_addr: SysMem<'a, T>,
> +    dma_handle: DmaAddress,
> +}
> +
> +impl<T: ?Sized> Copy for CoherentView<'_, T> {}
> +impl<T: ?Sized> Clone for CoherentView<'_, T> {
> +    #[inline]
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +impl<'a, T: ?Sized> CoherentView<'a, T> {
> +    /// Erase the DMA handle information and obtain a [`SysMem`] view of the same memory region.
> +    #[inline]
> +    pub fn as_sys_mem(self) -> SysMem<'a, T> {
> +        self.cpu_addr
> +    }
> +
> +    /// Returns a DMA handle which may be given to the device as the DMA address base of the region.
> +    #[inline]
> +    pub fn dma_handle(self) -> DmaAddress {
> +        self.dma_handle
> +    }

This is really cool - we are doing DMA handle arithmetic in Nova's
falcon DMA load methods, looking forward to replacing these with this.

> +
> +    /// Returns a reference to the data in the region.
> +    ///
> +    /// # Safety
> +    ///
> +    /// * Callers must ensure that the device does not read/write to/from memory while the returned
> +    ///   reference is live.
> +    /// * Callers must ensure that this call does not race with a write to the same region while
> +    ///   the returned reference is live.
> +    #[inline]
> +    pub unsafe fn as_ref(self) -> &'a T {
> +        // SAFETY: pointer is aligned and valid per type invariant. Aliasing rule is satisfied per
> +        // safety requirement.
> +        unsafe { &*self.cpu_addr.as_ptr() }
> +    }
> +
> +    /// Returns a mutable reference to the data in the region.
> +    ///
> +    /// # Safety
> +    ///
> +    /// * Callers must ensure that the device does not read/write to/from memory while the returned
> +    ///   reference is live.
> +    /// * Callers must ensure that this call does not race with a read or write to the same region
> +    ///   while the returned reference is live.
> +    #[inline]
> +    pub unsafe fn as_mut(self) -> &'a mut T {
> +        // SAFETY: pointer is aligned and valid per type invariant. Aliasing rule is satisfied per
> +        // safety requirement.
> +        unsafe { &mut *self.cpu_addr.as_ptr() }
> +    }
> +}
> +
> +/// `IoBackend` implementation for `Coherent`.
> +pub struct CoherentBackend;

nit: somehow I feel this should be named `CoherentIoBackend` for clarity.