Re: [PATCH v5 19/20] rust: io: add copying methods

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

On Fri Jun 26, 2026 at 11:45 PM JST, Gary Guo wrote:
<...>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index aa82736253ac..b5ac3ac86bbd 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -5,7 +5,8 @@
>  //! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
>  
>  use core::{
> -    marker::PhantomData, //
> +    marker::PhantomData,
> +    mem::MaybeUninit, //
>  };
>  
>  use crate::{
> @@ -271,6 +272,61 @@ pub trait IoCapable<T>: IoBackend {
>      fn io_write<'a>(view: Self::View<'a, T>, value: T);
>  }
>  
> +/// Trait indicating that an I/O backend supports memory copy operations.
> +pub trait IoCopyable: IoBackend {
> +    /// Copy contents of `view` to `buffer`.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `buffer` is valid for volatile write for `view.size()` bytes.
> +    unsafe fn copy_from_io(view: Self::View<'_, [u8]>, buffer: *mut u8);

Should we also define whether overlapping regions are allowed or not? I
guess the shape of the API assumes that the regions don't overlap, but
it might be useful to state it clearly.

> +
> +    /// Copy `size` bytes from `buffer` to `address`.

There are no `size` or `address` parameters.

> +    ///
> +    /// # Safety
> +    ///
> +    /// - `buffer` is valid for volatile read for `view.size()` bytes.
> +    unsafe fn copy_to_io(view: Self::View<'_, [u8]>, buffer: *const u8);
> +
> +    /// Copy from `view` and return the value.
> +    #[inline]
> +    fn copy_read<T: FromBytes>(view: Self::View<'_, T>) -> T {
> +        // Project `self` to `[u8]`.
> +        let ptr = Self::as_ptr(view);
> +        // SAFETY: This is a identity projection.
> +        let slice_view = unsafe {
> +            Self::project_view(
> +                view,
> +                core::ptr::slice_from_raw_parts_mut::<u8>(ptr.cast(), size_of::<T>()),
> +            )
> +        };
> +
> +        let mut buf = MaybeUninit::<T>::uninit();
> +        // SAFETY: `buf.as_mut_ptr()` is valid for write for `size_of::<T>()` bytes.
> +        unsafe { Self::copy_from_io(slice_view, buf.as_mut_ptr().cast()) };
> +        // SAFETY: T: FromBytes` guarantee that all bit patterns are valid.

Missing `.

> +        unsafe { buf.assume_init() }
> +    }
> +
> +    /// Copy `value` to `view`.
> +    #[inline]
> +    fn copy_write<T: IntoBytes>(view: Self::View<'_, T>, value: T) {
> +        // Project `self` to `[u8]`.
> +        let ptr = Self::as_ptr(view);
> +        // SAFETY: This is a identity projection.
> +        let slice_view = unsafe {
> +            Self::project_view(
> +                view,
> +                core::ptr::slice_from_raw_parts_mut::<u8>(ptr.cast(), size_of::<T>()),
> +            )
> +        };
> +
> +        // SAFETY: `&raw const value` is valid for read for `size_of::<T>()` bytes.
> +        unsafe { Self::copy_to_io(slice_view, (&raw const value).cast()) };
> +        core::mem::forget(value);

Maybe we should mention in the doccomment that the destructor of `value`
is not run, neither by the end of this method nor at any point in the
future IIUC, because the `View` doesn't own the object that has been
copied into it. Or we can sidestep the problem by taking a `value: &T`.

> +    }
> +}
> +
>  /// Describes a given I/O location: its offset, width, and type to convert the raw value from and
>  /// into.
>  ///
> @@ -350,6 +406,24 @@ fn size(self) -> usize {
>          KnownSize::size(Self::Backend::as_ptr(self.as_view()))
>      }
>  
> +    /// Returns the length of the slice in number of elements.
> +    #[inline]
> +    fn len<T>(self) -> usize
> +    where
> +        Self: Io<'a, Target = [T]>,
> +    {
> +        Self::Backend::as_ptr(self.as_view()).len()
> +    }
> +
> +    /// Returns `true` if the slice has a length of 0.
> +    #[inline]
> +    fn is_empty<T>(self) -> bool
> +    where
> +        Self: Io<'a, Target = [T]>,
> +    {
> +        self.len() == 0
> +    }

nit: these two could have been introduced alongside `size` in patch 12,
or all 3 in the present patch. It's a bit inconsistent that `size` stays
unused all this time, while these two related ones are introduced right
when they become useful.

> +
>      /// Try to convert into a different typed I/O view.
>      ///
>      /// The target type must be of same or smaller size to current type, and the current view must
> @@ -437,6 +511,115 @@ fn write_val(self, value: Self::Target)
>          Self::Backend::io_write(self.as_view(), value)
>      }
>  
> +    /// Copy-read from I/O memory.
> +    ///
> +    /// This is equivalent to reading from the I/O memory with byte-wise copy, although the actual
> +    /// implementation might be more efficient. There is no atomicity guarantee. Note that for some
> +    /// backends (e.g. `Mmio`), this can read different value compared to [`read_val`] as
> +    /// byte-swapping is not performed.
> +    ///
> +    /// [`read_val`]: Io::read_val
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```no_run
> +    /// # use kernel::io::*;
> +    /// # fn test_copy_read(mmio: Mmio<'_, [u8; 6]>) {
> +    /// // let mmio: Mmio<'_, [u8; 6]>;
> +    /// let val: [u8; 6] = mmio.copy_read();
> +    /// # }
> +    /// ```
> +    #[inline]
> +    fn copy_read(self) -> Self::Target
> +    where
> +        Self::Backend: IoCopyable,
> +        Self::Target: Sized + FromBytes,
> +    {
> +        Self::Backend::copy_read(self.as_view())
> +    }
> +
> +    /// Copy-write to I/O memory.
> +    ///
> +    /// This is equivalent to writing to the I/O memory with byte-wise copy, although the actual
> +    /// implementation might be more efficient. There is no atomicity guarantee. Note that for some
> +    /// backends (e.g. `Mmio`), this can read different value compared to [`write_val`] as

"this can write" I presume (copy/paste omission?)

> +    /// byte-swapping is not performed.
> +    ///
> +    /// [`write_val`]: Io::write_val
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```no_run
> +    /// # use kernel::io::*;
> +    /// # fn test_copy_write(mmio: Mmio<'_, [u8; 6]>) {
> +    /// // let mmio: Mmio<'_, [u8; 6]>;
> +    /// mmio.copy_write([0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF]);
> +    /// # }
> +    /// ```
> +    #[inline]
> +    fn copy_write(self, value: Self::Target)
> +    where
> +        Self::Backend: IoCopyable,
> +        Self::Target: Sized + IntoBytes,
> +    {
> +        Self::Backend::copy_write(self.as_view(), value);
> +    }
> +
> +    /// Copy bytes from slice to I/O memory.

"from `data`" maybe.

> +    ///
> +    /// The length of `self` must be the same as `data`, similar to [`[u8]::copy_from_slice`].
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```no_run
> +    /// # use kernel::io::*;
> +    /// # fn test_copy_write(mmio: Mmio<'_, [u8]>) {
> +    /// // let mmio: Mmio<'_, [u8]>;
> +    /// mmio.copy_from_slice(&[0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF]);
> +    /// # }
> +    /// ```
> +    #[inline]
> +    fn copy_from_slice(self, data: &[u8])
> +    where
> +        Self::Backend: IoCopyable,
> +        Self: Io<'a, Target = [u8]>,
> +    {
> +        assert_eq!(self.len(), data.len());

I think this one deserves an entry in a `# Panics` section of the
doccomment (same for all other asserts).

> +
> +        // SAFETY: `data.as_ptr()` is valid for read for `self.size()` bytes.
> +        unsafe {
> +            Self::Backend::copy_to_io(self.as_view(), data.as_ptr());
> +        }
> +    }
> +
> +    /// Copy bytes from I/O memory to slice.

"to `data`".

> +    ///
> +    /// The length of `self` must be the same as `data`, similar to [`[u8]::copy_from_slice`].
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```no_run
> +    /// # use kernel::io::*;
> +    /// # fn test_copy_write(mmio: Mmio<'_, [u8]>) {
> +    /// // let mmio: Mmio<'_, [u8]>;
> +    /// let mut buf = [0; 6];
> +    /// mmio.copy_to_slice(&mut buf);
> +    /// # }
> +    /// ```
> +    #[inline]
> +    fn copy_to_slice(self, data: &mut [u8])
> +    where
> +        Self::Backend: IoCopyable,
> +        Self: Io<'a, Target = [u8]>,
> +    {
> +        assert_eq!(self.len(), data.len());
> +
> +        // SAFETY: `data.as_ptr()` is valid for write for `self.size()` bytes.

Should be `data.as_mut_ptr()`.