Re: [PATCH v2 11/11] rust: io: add copying methods

Andreas Hindborg
2026-04-28T13:22:54Z

Gary Guo <gary@garyguo.net> writes:

> One feature that was lost from the old `dma_read!()` and `dma_write!()`
> when moving to `io_read!()` and `io_write!()` was the ability to read/write
> a large structs. However, the semantics was unclear to begin with, as there
> was no guarantee about their atomicity even for structs that were small
> enough to fit in u32. Re-introduces the capability in the form of copying
> methods.
>
> dma_read!(foo, bar) -> io_project!(foo, bar).copy_read()
> dma_write!(foo, bar, baz) -> io_project!(foo, bar).copy_write(baz)
>
> The semantics for these are modelled after memcpy so user has clear
> expectation of lack of atomicity. As an additional benefit of this change,
> this now works for MMIO as well, which maps to `memcpy_{from,to}io`.
>
> For slices, which is unsized so the API above can't work, `copy_from_slice`
> and `copy_to_slice` were added to copy from/to normal memory, and
> `copy_from_io_slice` and `copy_to_io_slice` were added to copy from/to
> other `Io` regions. They're optimized if at least one end is mapped to
> system memory; if none are, the copy occurs with an intermediate stack
> buffer.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
> rust/kernel/dma.rs | 8 +-
> rust/kernel/io.rs | 231 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 238 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index bbdeb117c145..307f5769ca0a 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -16,7 +16,8 @@
> fs::file,
> io::{
> Io,
> - IoCapable, //
> + IoCapable,
> + IoCopyable, //
> },
> prelude::*,
> ptr::KnownSize,
> @@ -997,6 +998,11 @@ unsafe fn io_write(&self, value: $ty, address: *mut $ty) {
> u64
> );
>
> +// SAFETY: `Coherent` is mapped to CPU address space.
> +unsafe impl<T: ?Sized + KnownSize> IoCopyable for Coherent<T> {
> + const IS_MAPPED: bool = true;
> +}
> +
> impl<'a, B: ?Sized + KnownSize, T: ?Sized> crate::io::View<'a, Coherent, T> {
> /// Returns a DMA handle which may be given to the device as the DMA address base of
> /// the region.
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index efcd7e6741d7..0b1ed68c0f9b 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -4,6 +4,8 @@
> //!
> //! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
>
> +use core::mem::MaybeUninit;
> +
> use crate::{
> bindings,
> prelude::*,
> @@ -233,6 +235,55 @@ pub trait IoCapable<T> {
> unsafe fn io_write(&self, value: T, address: *mut T);
> }
>
> +/// Trait indicating that an I/O backend supports memory copy operations.
> +///
> +/// # Safety
> +///
> +/// If [`IS_MAPPED`] is overridden to true, it must be correct per documentation.
> +pub unsafe trait IoCopyable {
> + /// Whether the pointers for this I/O backend are in the CPU address space, and are coherently
> + /// mapped.
> + ///
> + /// When this is true, it means that memory can be accessed with byte-wise atomic memory copy.
> + const IS_MAPPED: bool = false;
> +
> + /// Copy `size` bytes from `address` to `buffer`.
> + ///
> + /// # Safety
> + ///
> + /// - The range `[address..address + size]` must be within the bounds of `Self`.

We should probably specify what "bounds of `Self`" means here. It's not
the bounds of `Self`, it is the bounds of the memory region `Self`
represents.

> + /// - `buffer` is valid for write for `size` bytes.
> + #[inline]
> + unsafe fn copy_from_io(&self, address: *mut u8, buffer: *mut u8, size: usize) {
> + const_assert!(Self::IS_MAPPED);
> +
> + // Use `bindings::memcpy` instead of copy_nonoverlapping for volatile.
> + // SAFETY:
> + // - `buffer` is valid for write for `size` bytes.
> + // - `IS_MAPPED` guarantees `address` is in CPU address space, with safety requirements
> + // `address` is valid for read for `size` bytes.
> + unsafe { bindings::memcpy(buffer.cast(), address.cast(), size) };
> + }

You could just leave out the default impl and implement each case for
`Coherent<_>` and `Mmio<_>`. Do you expect more implementers that can
share the default impl?

> +
> + /// Copy `size` bytes from `buffer` to `address`.
> + ///
> + /// # Safety
> + ///
> + /// - The range `[address..address + size]` must be within the bounds of `Self`.
> + /// - `buffer` is valid for read for `size` bytes.
> + #[inline]
> + unsafe fn copy_to_io(&self, address: *mut u8, buffer: *const u8, size: usize) {
> + const_assert!(Self::IS_MAPPED);
> +
> + // Use `bindings::memcpy` instead of copy_nonoverlapping for volatile.
> + // SAFETY:
> + // - `IS_MAPPED` guarantees `address` is in CPU address space, with safety requirements
> + // `address` is valid for write for `size` bytes.
> + // - `buffer` is valid for read for `size` bytes.
> + unsafe { bindings::memcpy(address.cast(), buffer.cast(), size) };
> + }
> +}
> +
> /// Describes a given I/O location: its offset, width, and type to convert the raw value from and
> /// into.
> ///
> @@ -841,6 +892,19 @@ unsafe fn io_write(&self, value: $ty, address: *mut $ty) {
> writeq
> );
>
> +// SAFETY: `IS_MAPPED` is not overridden.
> +unsafe impl<T: ?Sized + KnownSize> IoCopyable for Mmio<T> {
> + unsafe fn copy_from_io(&self, address: *mut u8, buffer: *mut u8, size: usize) {
> + // SAFETY: Per safety requirement.
> + unsafe { bindings::memcpy_fromio(buffer.cast(), address.cast(), size) };
> + }
> +
> + unsafe fn copy_to_io(&self, address: *mut u8, buffer: *const u8, size: usize) {
> + // SAFETY: Per safety requirement.
> + unsafe { bindings::memcpy_toio(address.cast(), buffer.cast(), size) };
> + }
> +}
> +
> impl<T: ?Sized + KnownSize> Io for Mmio<T> {
> type Type = T;
>
> @@ -1029,6 +1093,173 @@ pub fn write_val(&self, value: T) {
> }
> }
>
> +impl<'a, IO: ?Sized, T> View<'a, IO, [T]> {
> + /// Returns the length of the slice in number of elements.
> + #[inline]
> + pub fn len(self) -> usize {
> + self.as_ptr().len()
> + }
> +
> + /// Returns `true` if the slice has a length of 0.
> + #[inline]
> + pub fn is_empty(self) -> bool {
> + self.len() == 0
> + }
> +}
> +
> +impl<T, IO: ?Sized + Io + IoCopyable> View<'_, IO, T> {
> + /// Copy-read from I/O memory.
> + ///
> + /// There is no atomicity gurantee.

Please add examples for the public functions.

> + #[inline]
> + pub fn copy_read(self) -> T
> + where
> + T: FromBytes,
> + {
> + // Optimized path if I/O backend is CPU mapped.
> + if IO::IS_MAPPED {
> + // SAFETY:
> + // - `IS_MAPPED` guarantees `self.ptr` is in CPU address space, with type invariants
> + // `self.ptr` is valid for read for `size` bytes.
> + // - `T: FromBytes` guarantee that all bit patterns are valid.
> + // - 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.
> + return unsafe { self.ptr.read_volatile() };
> + }
> +
> + let mut buf = MaybeUninit::<T>::uninit();
> + // SAFETY:
> + // - Per type invariants.
> + // - `buf.as_mut_ptr()` is valid for write for `size_of::<T>()` bytes.
> + unsafe {
> + self.io
> + .copy_from_io(self.ptr.cast(), buf.as_mut_ptr().cast(), size_of::<T>())
> + };
> + // SAFETY: T: FromBytes` guarantee that all bit patterns are valid.
> + unsafe { buf.assume_init() }
> + }
> +
> + /// Write to I/O memory.
> + ///
> + /// There is no atomicity gurantee.
> + #[inline]
> + pub fn copy_write(self, value: T)
> + where
> + T: AsBytes,
> + {
> + // Optimized path if I/O backend is CPU mapped.
> + if IO::IS_MAPPED {
> + // SAFETY:
> + // - `IS_MAPPED` guarantees `self.ptr` is in CPU address space, with safety requirements
> + // `self.ptr` is valid for write for `size` bytes.
> + // - 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 { self.ptr.write_volatile(value) };
> + return;
> + }
> +
> + // SAFETY:
> + // - Per type invariants.
> + // - `&raw const value` is valid for read for `size_of::<T>()` bytes.
> + unsafe {
> + self.io
> + .copy_to_io(self.ptr.cast(), (&raw const value).cast(), size_of::<T>())
> + };
> + core::mem::forget(value);
> + }
> +}
> +
> +impl<IO: ?Sized + Io + IoCopyable> View<'_, IO, [u8]> {
> + /// Copy bytes from slice to I/O memory.
> + #[inline]
> + pub fn copy_from_slice(self, data: &[u8]) {
> + assert_eq!(self.len(), data.len());

Do you really want a panic here?

Best regards,
Andreas Hindborg