[PATCH 0/3] rust: dma: add the single-buffer streaming DMA API

[PATCH 0/3] rust: dma: add the single-buffer streaming DMA API

[Maurice Hieronymus]

The Rust DMA abstraction covers `dma_alloc_coherent()` only. The streaming
half (`dma_map_single()`) is missing.

A streaming mapping is a temporary lease on memory the caller already owns:
between map and unmap the buffer belongs to the device, and the CPU may only
touch it in between a `dma_sync_single_for_cpu()` /
`dma_sync_single_for_device()` pair. In C that protocol is left to the driver
author, and getting it wrong is silent data corruption on non-coherent
platforms. It is a borrow handover, so it can be expressed in the type
system:

    let mut dma = Streaming::new(dev, buf, DataDirection::Bidirectional)?;

    *dma.for_cpu() = 42;

    let dma = dma.submit();

    // Program `dma.dma_handle()` into the device and wait for the transfer.

    // SAFETY: the transfer has been waited for.
    let mut dma = unsafe { dma.complete() };

    assert_eq!(*dma.for_cpu(), 42);

`submit()` consumes the `Streaming` and returns a `StreamingInFlight`, the
only source of the `DmaAddress`. It owns the buffer, so the contents are
unreachable while a transfer may be in flight, no matter where the driver
stores the address. That is what makes `for_cpu()` safe. Whether the device
has finished cannot be checked by any abstraction, so `complete()` is the
single `unsafe` operation. Dropping a `StreamingInFlight` without
`complete()` leaks the mapping and the storage, with a warning: safe code
cannot prove the device is done, so it is not allowed to unmap or free
memory the device may still be using.

Patch 1 adds `ContiguousBuffer`, describing storage `dma_map_single()`
accepts: a single physically contiguous region in the kernel's linear
mapping. Patch 2 adds the mapping itself. Patch 3 converts nova-core's
`GspFwWprMeta`, a streaming workload written against the coherent API.

One thing to note
=================

`Streaming` borrows a `&'a Device<Bound>` where `Coherent` and
`SGTable<Owned<P>>` both hold an `ARef<Device>`. The DMA API may only be
called while a driver is bound, and `Drop` unmaps, so a refcount does not
express what the mapping needs [1]. The two existing types predate the
`'bound` driver-core infrastructure; converting them is not part of this
series.

Testing
=======

Build-tested on x86_64 with `CLIPPY=1` and `rustfmtcheck`; the kernel crate
doctests, including the new `Streaming` ones, pass under virtme-ng.

Patch 3 is compile-tested only, I have no NVIDIA hardware; it touches the
GSP boot path on all supported chipsets, so a Tested-by would be very
welcome. However, I've tested it on my Rust EDU Driver locally [2] while
using swiotlb=force to emulate bounce buffers on x86.

[1] https://lore.kernel.org/all/20250306160907.GF354511@nvidia.com/
[2] https://lore.kernel.org/rust-for-linux/20260620-b4-rust-pci-edu-driver-v2-0-6fd6684f2c14@mailbox.org/

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
Maurice Hieronymus (3):
      rust: dma: add ContiguousBuffer trait for streaming DMA storage
      rust: dma: add abstraction for the single-buffer streaming DMA API
      gpu: nova-core: gsp: map the WPR meta for streaming DMA

 drivers/gpu/nova-core/firmware/booter.rs |  11 +-
 drivers/gpu/nova-core/gsp/boot.rs        |  20 +-
 drivers/gpu/nova-core/gsp/hal.rs         |   4 +-
 drivers/gpu/nova-core/gsp/hal/gh100.rs   |   4 +-
 drivers/gpu/nova-core/gsp/hal/tu102.rs   |   4 +-
 rust/helpers/dma.c                       |  35 +++
 rust/kernel/dma.rs                       | 449 +++++++++++++++++++++++++++++++
 7 files changed, 515 insertions(+), 12 deletions(-)
---
base-commit: dc01dfb37b34beeefcfe1c3055364d41a4070c7e
change-id: 20260719-dma-streaming-9c505a8760cb

Best regards,
-- 
Maurice Hieronymus <mhi@mailbox.org>

[PATCH 1/3] rust: dma: add ContiguousBuffer trait for streaming DMA storage

[Maurice Hieronymus]

The streaming DMA API (`dma_map_single()`) does not allocate, it maps a
buffer the caller already owns. Not every allocation qualifies: the
buffer must be a single physically contiguous region in the kernel's
linear mapping, which rules out `vmalloc()`ed memory and the stack.

Add `ContiguousBuffer`, an unsafe trait describing that requirement, and
implement it for `KBox<T>`, whose storage comes from `kmalloc()`.

The trait hands out owned storage rather than a borrowed slice, so the
mapping added in the next patch can take ownership and guarantee that no
other CPU-side reference exists while the device owns the buffer. `Data`
is bounded by `FromBytes` and `AsBytes` because the device may write an
arbitrary byte pattern into the region and may read it, so it must not
contain uninitialized padding.

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 rust/kernel/dma.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 200def84fb69..8a8af5ab7feb 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -564,6 +564,63 @@ fn from(value: CoherentBox<T>) -> Self {
     }
 }
 
+/// Backing storage that can be passed to the single-buffer streaming DMA API.
+///
+/// # Safety
+///
+/// Implementers must guarantee that, for as long as `Self` is alive and not mutated:
+///
+/// * [`ptr`](Self::ptr) returns a pointer to the start of a single, physically contiguous region
+///   of [`size`](Self::size) bytes, and [`data`](Self::data) refers to exactly that region.
+/// * The region lives in the kernel's linear mapping, i.e. it is neither `vmalloc()`ed nor stack
+///   memory, both of which `dma_map_single()` rejects.
+/// * The region is DMA-safe in the sense of the [DMA API howto].
+///
+/// [DMA API howto]: srctree/Documentation/core-api/dma-api-howto.rst
+pub unsafe trait ContiguousBuffer {
+    /// The CPU-side view of the region.
+    ///
+    /// [`FromBytes`] because the device may write an arbitrary byte pattern into the region,
+    /// [`AsBytes`] because it may read the region, which must therefore have no uninitialized
+    /// padding.
+    type Data: ?Sized + FromBytes + AsBytes;
+
+    /// Returns a pointer to the start of the region.
+    fn ptr(&mut self) -> *mut c_void;
+
+    /// Returns the size of the region in bytes.
+    fn size(&self) -> usize;
+
+    /// Returns a mutable reference to the region.
+    fn data(&mut self) -> &mut Self::Data;
+}
+
+// SAFETY: `KBox` allocates via `kmalloc()`, which returns a single physically contiguous,
+// DMA-safe region in the kernel's linear mapping. All three methods describe that allocation.
+unsafe impl<T: FromBytes + AsBytes> ContiguousBuffer for KBox<T> {
+    type Data = T;
+
+    fn ptr(&mut self) -> *mut c_void {
+        let ptr = &raw mut **self;
+        ptr.cast()
+    }
+
+    fn size(&self) -> usize {
+        const {
+            assert!(
+                core::mem::size_of::<T>() > 0,
+                "It doesn't make sense to map a ZST for DMA"
+            );
+        }
+
+        core::mem::size_of_val(&**self)
+    }
+
+    fn data(&mut self) -> &mut Self::Data {
+        self
+    }
+}
+
 /// An abstraction of the `dma_alloc_coherent` API.
 ///
 /// This is an abstraction around the `dma_alloc_coherent` API which is used to allocate and map

-- 
2.54.0

Re: [PATCH 1/3] rust: dma: add ContiguousBuffer trait for streaming DMA storage

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `size()` method creates a shared reference to potentially mutating DMA memory, causing Undefined Behavior.
- [Medium] `ContiguousBuffer` implementation for `KBox<T>` implicitly requires `T: Sized`, preventing the mapping of dynamically sized buffers.
- [Low] Missing `#[inline]` annotations on small trait methods in an abstraction.
--

--- Patch [1]: [PATCH 1/3] rust: dma: add ContiguousBuffer trait for streaming DMA storage ---
commit 9463792a46877c9a52666dde64c8d1a545e696f7
Author: Maurice Hieronymus <mhi@mailbox.org>

rust: dma: add ContiguousBuffer trait for streaming DMA storage

This commit introduces the ContiguousBuffer trait to describe backing storage
that can be mapped via the streaming DMA API. It also implements this trait
for KBox<T> to guarantee physically contiguous memory allocated via kmalloc().

> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 200def84fb69e..8a8af5ab7feb8 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs

[ ... ]

> +// SAFETY: `KBox` allocates via `kmalloc()`, which returns a single physically contiguous,
> +// DMA-safe region in the kernel's linear mapping. All three methods describe that allocation.
> +unsafe impl<T: FromBytes + AsBytes> ContiguousBuffer for KBox<T> {

[Severity: Medium]
Does this implementation unintentionally exclude dynamically sized buffers
like KBox<[u8]>?

Since T implicitly requires Sized here, it restricts the implementation to
statically sized types, despite the ContiguousBuffer trait allowing
type Data: ?Sized. Should this include the ?Sized bound?

> +    type Data = T;
> +
> +    fn ptr(&mut self) -> *mut c_void {

[Severity: Low]
This isn't a bug, but according to the Rust subsystem guidelines, should
small methods in abstractions, such as ptr(), size(), and data(), be
annotated with #[inline]?

> +        let ptr = &raw mut **self;
> +        ptr.cast()
> +    }
> +
> +    fn size(&self) -> usize {
> +        const {
> +            assert!(
> +                core::mem::size_of::<T>() > 0,
> +                "It doesn't make sense to map a ZST for DMA"
> +            );
> +        }
> +
> +        core::mem::size_of_val(&**self)

[Severity: High]
Does this expression create a shared reference to potentially mutating DMA
memory?

Calling core::mem::size_of_val(&**self) implicitly calls deref() and creates
a shared reference (&T) to the underlying allocation. If the device is
actively modifying the buffer during a DMA operation, could materializing
this shared reference violate Rust's aliasing rules and cause Undefined
Behavior?

Since T is implicitly Sized in this implementation, can this just use
core::mem::size_of::<T>() to evaluate the size purely at compile time without
creating hazardous references?

> +    }
> +
> +    fn data(&mut self) -> &mut Self::Data {
> +        self
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-dma-streaming-v1-0-03974c86b141@mailbox.org?part=1

[PATCH 2/3] rust: dma: add abstraction for the single-buffer streaming DMA API

[Maurice Hieronymus]

Add `Streaming`, a safe abstraction around `dma_map_single_attrs()`.

Between map and unmap the buffer belongs to the device, and the CPU may
only access it in between a `dma_sync_single_for_cpu()` /
`dma_sync_single_for_device()` pair. The types encode that protocol:

- `Streaming` owns the backing storage, so no other CPU-side reference
  to the region exists.
- `submit()` consumes it and returns a `StreamingInFlight`, the only
  source of the `DmaAddress`. It owns the buffer, so the contents stay
  unreachable while a transfer may be in flight, no matter where the
  driver stores the address.
- `for_cpu()` is therefore safe: no transfer can have been started from
  a `Streaming`. It syncs for the CPU, returns a guard dereferencing to
  the contents, and syncs for the device again on drop.
- `complete()` turns a `StreamingInFlight` back into a `Streaming`.
  Whether the device has finished cannot be checked by any abstraction,
  so this is the one `unsafe` operation.

Dropping a `StreamingInFlight` leaks the mapping and the backing
storage, with a warning. A transfer may still be in flight: freeing the
storage would leave the device writing through a dangling handle, and
even unmapping could recycle a SWIOTLB bounce slot mid-transfer.
Reclaiming either takes the assertion only `complete()` can make, so an
early `?` return between `submit()` and `complete()` costs a leak
instead of a device-side use-after-free.

The mapping is torn down on drop of a `Streaming`, or by
`into_inner()`, which returns the backing storage. Unmapping already
hands the buffer back to the CPU, so `into_inner()` skips a
synchronization nothing would consume.

The `'a` lifetime binds the mapping to a `Device<Bound>`: the DMA API
may only be called while a driver is bound, and `Drop` unmaps.

`DataDirection::None` (a `BUG_ON()` in the DMA core) and empty buffers
(not representable by `dma_map_single()`) are rejected with `EINVAL`.
So are `DMA_ATTR_SKIP_CPU_SYNC`, which disables the implicit CPU cache
maintenance the type invariants are built on with no way to compensate
through this API, and `DMA_ATTR_MMIO`, which describes memory a
`ContiguousBuffer` cannot represent.

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 rust/helpers/dma.c |  35 +++++
 rust/kernel/dma.rs | 392 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 427 insertions(+)

diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
index 9fbeb507b08c..7e1e5c67431a 100644
--- a/rust/helpers/dma.c
+++ b/rust/helpers/dma.c
@@ -49,3 +49,38 @@ __rust_helper void rust_helper_dma_set_max_seg_size(struct device *dev,
 {
 	dma_set_max_seg_size(dev, size);
 }
+
+__rust_helper dma_addr_t rust_helper_dma_map_single_attrs(struct device *dev,
+							  void *ptr, size_t size,
+							  enum dma_data_direction dir,
+							  unsigned long attrs)
+{
+	return dma_map_single_attrs(dev, ptr, size, dir, attrs);
+}
+
+__rust_helper void rust_helper_dma_unmap_single_attrs(struct device *dev,
+						      dma_addr_t addr, size_t size,
+						      enum dma_data_direction dir,
+						      unsigned long attrs)
+{
+	dma_unmap_single_attrs(dev, addr, size, dir, attrs);
+}
+
+__rust_helper int rust_helper_dma_mapping_error(struct device *dev, dma_addr_t addr)
+{
+	return dma_mapping_error(dev, addr);
+}
+
+__rust_helper void rust_helper_dma_sync_single_for_cpu(struct device *dev,
+						       dma_addr_t addr, size_t size,
+						       enum dma_data_direction dir)
+{
+	dma_sync_single_for_cpu(dev, addr, size, dir);
+}
+
+__rust_helper void rust_helper_dma_sync_single_for_device(struct device *dev,
+							  dma_addr_t addr, size_t size,
+							  enum dma_data_direction dir)
+{
+	dma_sync_single_for_device(dev, addr, size, dir);
+}
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 8a8af5ab7feb..cbaf30a2de86 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -24,6 +24,7 @@
     uaccess::UserSliceWriter,
 };
 use core::{
+    mem::ManuallyDrop,
     ops::{
         Deref,
         DerefMut, //
@@ -345,6 +346,14 @@ const fn const_cast(val: bindings::dma_data_direction) -> u32 {
         // is within the representable range of `u32`.
         wide_val as u32
     }
+
+    /// Returns whether this direction may be passed to a mapping or synchronization primitive.
+    ///
+    /// Equivalent to `valid_dma_direction()`; [`Self::None`] is a debugging aid the DMA core
+    /// rejects with a `BUG_ON()`.
+    const fn is_valid(self) -> bool {
+        !matches!(self, Self::None)
+    }
 }
 
 impl From<DataDirection> for bindings::dma_data_direction {
@@ -621,6 +630,389 @@ fn data(&mut self) -> &mut Self::Data {
     }
 }
 
+/// An abstraction of the `dma_map_single` API.
+///
+/// Unlike [`Coherent`], a streaming mapping is a temporary lease on memory the caller already
+/// owns: between mapping and unmapping the buffer belongs to the device, and the CPU may only
+/// access it in between a `dma_sync_single_for_cpu()` / `dma_sync_single_for_device()` pair.
+///
+/// [`Streaming`] owns the backing storage and is one of two states: [`submit`](Self::submit)
+/// consumes it and returns a [`StreamingInFlight`], the only source of the [`DmaAddress`];
+/// [`complete`](StreamingInFlight::complete) turns that back into a [`Streaming`], whose
+/// [`for_cpu`](Self::for_cpu) yields a [`StreamingCpuGuard`] dereferencing to the contents.
+///
+/// The mapping is torn down on drop of a [`Streaming`], or by [`into_inner`](Self::into_inner),
+/// which returns the backing storage. Dropping a [`StreamingInFlight`] instead leaks the mapping
+/// and the storage: safe code cannot prove the device is done, so it cannot be allowed to reclaim
+/// either.
+///
+/// The `'a` lifetime keeps the device bound for the life of the mapping.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::device::{Bound, Device};
+/// use kernel::dma::{
+///     DataDirection,
+///     Streaming, //
+/// };
+///
+/// # fn test(dev: &Device<Bound>) -> Result {
+/// let buf = KBox::new(0u64, GFP_KERNEL)?;
+/// let mut dma = Streaming::new(dev, buf, DataDirection::Bidirectional)?;
+///
+/// // The CPU prepares the buffer, and hands it back to the device by dropping the guard.
+/// *dma.for_cpu() = 42;
+///
+/// // Hand the buffer to the device; `dma` is consumed, so its contents are now unreachable.
+/// let dma = dma.submit();
+///
+/// // Program `dma.dma_handle()` and `dma.size()` into the device.
+///
+/// // SAFETY: For the sake of the example, assume the transfer has been waited for.
+/// let mut dma = unsafe { dma.complete() };
+///
+/// assert_eq!(*dma.for_cpu(), 42);
+/// # Ok::<(), Error>(()) }
+/// ```
+///
+/// # Invariants
+///
+/// * `dma_addr` denotes a live mapping of `container` for the lifetime of the instance, and
+///   `container`, `direction` and `dma_attrs` are unchanged since it was established.
+/// * `direction` is not [`DataDirection::None`].
+/// * The buffer is synchronized for the device whenever no [`StreamingCpuGuard`] borrowed from
+///   this instance is alive.
+pub struct Streaming<'a, C: ContiguousBuffer> {
+    container: C,
+    direction: DataDirection,
+    dma_addr: DmaAddress,
+    dma_attrs: Attrs,
+    dev: &'a device::Device<Bound>,
+}
+
+impl<'a, C: ContiguousBuffer> Streaming<'a, C> {
+    /// Maps `container` for streaming DMA in `direction`.
+    ///
+    /// Ownership of `container` is moved into the returned [`Streaming`]; the buffer belongs to
+    /// the device until [`for_cpu`](Self::for_cpu) is called.
+    ///
+    /// Returns [`EINVAL`] for an empty buffer, for [`DataDirection::None`], or if `dma_attrs`
+    /// contains [`DMA_ATTR_SKIP_CPU_SYNC`](attrs::DMA_ATTR_SKIP_CPU_SYNC) or
+    /// [`DMA_ATTR_MMIO`](attrs::DMA_ATTR_MMIO).
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::device::{Bound, Device};
+    /// use kernel::dma::{
+    ///     attrs::*,
+    ///     DataDirection,
+    ///     Streaming, //
+    /// };
+    ///
+    /// # fn test(dev: &Device<Bound>) -> Result {
+    /// let buf = KBox::new(0u64, GFP_KERNEL)?;
+    /// let dma = Streaming::new_with_attrs(
+    ///     dev,
+    ///     buf,
+    ///     DataDirection::ToDevice,
+    ///     DMA_ATTR_WEAK_ORDERING,
+    /// )?;
+    /// # Ok::<(), Error>(()) }
+    /// ```
+    pub fn new_with_attrs(
+        dev: &'a device::Device<Bound>,
+        mut container: C,
+        direction: DataDirection,
+        dma_attrs: Attrs,
+    ) -> Result<Self> {
+        // The DMA core `BUG_ON()`s on `DMA_NONE`, bail early.
+        if !direction.is_valid() {
+            return Err(EINVAL);
+        }
+
+        // The type invariants are built on the implicit CPU cache maintenance performed by
+        // `dma_map_single_attrs()` and `dma_unmap_single_attrs()`; `DMA_ATTR_SKIP_CPU_SYNC`
+        // disables it, with no way to compensate through this API. `DMA_ATTR_MMIO` describes
+        // memory a `ContiguousBuffer` cannot represent.
+        if dma_attrs.contains(attrs::DMA_ATTR_SKIP_CPU_SYNC)
+            || dma_attrs.contains(attrs::DMA_ATTR_MMIO)
+        {
+            return Err(EINVAL);
+        }
+
+        let size = container.size();
+
+        // `dma_map_single_attrs` cannot handle zero-length mappings, bail early.
+        if size == 0 {
+            return Err(EINVAL);
+        }
+
+        // SAFETY:
+        // - Device pointer is guaranteed as valid by the type invariant on `Device`.
+        // - By the safety requirements of `ContiguousBuffer`, `container.ptr()` points to a single
+        //   physically contiguous region of `size` bytes in the kernel's linear mapping.
+        // - `container` is moved into `Self` below, so the region stays alive and at a stable
+        //   address until the mapping is torn down in `Drop`.
+        let dma_addr = unsafe {
+            bindings::dma_map_single_attrs(
+                dev.as_raw(),
+                container.ptr(),
+                size,
+                direction.into(),
+                dma_attrs.as_raw(),
+            )
+        };
+
+        // SAFETY: Device pointer is valid per the above, and `dma_addr` was just returned by
+        // `dma_map_single_attrs()` for this device.
+        to_result(unsafe { bindings::dma_mapping_error(dev.as_raw(), dma_addr) })?;
+
+        // INVARIANT:
+        // - The mapping was just established with these exact parameters, none of which is
+        //   mutated afterwards.
+        // - `direction` was checked above.
+        Ok(Streaming {
+            container,
+            direction,
+            dma_addr,
+            dma_attrs,
+            dev,
+        })
+    }
+
+    /// Performs the same functionality as [`Streaming::new_with_attrs`], except the `dma_attrs`
+    /// is 0 by default.
+    #[inline]
+    pub fn new(
+        dev: &'a device::Device<Bound>,
+        container: C,
+        direction: DataDirection,
+    ) -> Result<Self> {
+        Self::new_with_attrs(dev, container, direction, Attrs(0))
+    }
+
+    /// Returns the size of the mapping in bytes.
+    #[inline]
+    pub fn size(&self) -> usize {
+        self.container.size()
+    }
+
+    /// Returns the direction this buffer was mapped with.
+    #[inline]
+    pub fn direction(&self) -> DataDirection {
+        self.direction
+    }
+
+    /// Hands the buffer to the device.
+    ///
+    /// This performs no synchronization: by the type invariants the buffer is already
+    /// synchronized for the device.
+    #[inline]
+    pub fn submit(self) -> StreamingInFlight<'a, C> {
+        StreamingInFlight(ManuallyDrop::new(self))
+    }
+
+    /// Transfers ownership of the buffer back to the CPU and returns a guard granting access to
+    /// its contents.
+    ///
+    /// Dropping the guard transfers ownership back to the device. If the buffer is not handed to
+    /// the device again, prefer [`into_inner`](Self::into_inner), which unmaps instead.
+    pub fn for_cpu(&mut self) -> StreamingCpuGuard<'_, C::Data> {
+        let dev = self.dev;
+        let dma_addr = self.dma_addr;
+        let direction = self.direction;
+        let size = self.container.size();
+
+        // SAFETY: By the type invariants, `dev` is bound and `dma_addr` denotes a live mapping of
+        // `size` bytes established with `direction`, which is the range synced here.
+        unsafe {
+            bindings::dma_sync_single_for_cpu(dev.as_raw(), dma_addr, size, direction.into())
+        };
+
+        // INVARIANT: The buffer is now owned by the CPU, and dropping the guard hands it back.
+        StreamingCpuGuard {
+            data: self.container.data(),
+            dev,
+            dma_addr,
+            size,
+            direction,
+        }
+    }
+
+    /// Tears the mapping down and returns the backing storage.
+    ///
+    /// Unmapping transfers ownership of the buffer back to the CPU, so no separate
+    /// [`for_cpu`](Self::for_cpu) is needed.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::device::{Bound, Device};
+    /// use kernel::dma::{
+    ///     DataDirection,
+    ///     Streaming, //
+    /// };
+    ///
+    /// # fn test(dev: &Device<Bound>) -> Result {
+    /// let dma = Streaming::new(
+    ///     dev,
+    ///     KBox::new(0u64, GFP_KERNEL)?,
+    ///     DataDirection::FromDevice,
+    /// )?
+    /// .submit();
+    ///
+    /// // Program `dma.dma_handle()` into the device.
+    ///
+    /// // SAFETY: For the sake of the example, assume the transfer has been waited for.
+    /// let dma = unsafe { dma.complete() };
+    ///
+    /// // Take the buffer back; the mapping is gone once this returns.
+    /// let buf: KBox<u64> = dma.into_inner();
+    /// # Ok::<(), Error>(()) }
+    /// ```
+    pub fn into_inner(self) -> C {
+        let mut this = ManuallyDrop::new(self);
+
+        this.unmap();
+
+        // SAFETY: `this` is wrapped in a `ManuallyDrop`, so `Streaming::drop()` never runs and
+        // `this.container` is never read again. The remaining fields are all `Copy`.
+        unsafe { core::ptr::read(&this.container) }
+    }
+
+    /// Tears the mapping down.
+    ///
+    /// Shared by [`Drop`] and [`into_inner`](Self::into_inner), both of which run it exactly once.
+    fn unmap(&mut self) {
+        // SAFETY: By the type invariants, `self.dev` is bound and the mapping is still live, with
+        // exactly the address, size, direction and attributes it was created with. Both callers
+        // run this at most once, so the mapping cannot be torn down twice.
+        unsafe {
+            bindings::dma_unmap_single_attrs(
+                self.dev.as_raw(),
+                self.dma_addr,
+                self.container.size(),
+                self.direction.into(),
+                self.dma_attrs.as_raw(),
+            )
+        };
+    }
+}
+
+impl<C: ContiguousBuffer> Drop for Streaming<'_, C> {
+    fn drop(&mut self) {
+        self.unmap();
+    }
+}
+
+/// A [`Streaming`] mapping whose [`DmaAddress`] has been handed out.
+///
+/// Returned by [`Streaming::submit`]. It owns the buffer, so the contents are unreachable while
+/// it exists. [`complete`](Self::complete) is the only way back: it is the caller's assertion
+/// that the device has finished, which nothing else can establish. Dropping this instead leaks
+/// the mapping and the backing storage, since reclaiming either while the device may still
+/// access the buffer would be a use-after-free.
+pub struct StreamingInFlight<'a, C: ContiguousBuffer>(ManuallyDrop<Streaming<'a, C>>);
+
+impl<'a, C: ContiguousBuffer> StreamingInFlight<'a, C> {
+    /// Returns the DMA address to program into the device.
+    #[inline]
+    pub fn dma_handle(&self) -> DmaAddress {
+        self.0.dma_addr
+    }
+
+    /// Returns the size of the mapping in bytes.
+    #[inline]
+    pub fn size(&self) -> usize {
+        self.0.size()
+    }
+
+    /// Returns the direction this buffer was mapped with.
+    #[inline]
+    pub fn direction(&self) -> DataDirection {
+        self.0.direction()
+    }
+
+    /// Takes the buffer back from the device.
+    ///
+    /// This performs no synchronization; [`Streaming::for_cpu`] does that.
+    ///
+    /// # Safety
+    ///
+    /// The device must have finished accessing the buffer.
+    #[inline]
+    pub unsafe fn complete(self) -> Streaming<'a, C> {
+        let mut this = ManuallyDrop::new(self);
+
+        // SAFETY: `this` is wrapped in a `ManuallyDrop`, so `StreamingInFlight::drop()` never
+        // runs and `this.0` is never touched again.
+        unsafe { ManuallyDrop::take(&mut this.0) }
+    }
+}
+
+impl<C: ContiguousBuffer> Drop for StreamingInFlight<'_, C> {
+    fn drop(&mut self) {
+        // A transfer may still be in flight: freeing the storage would leave the device writing
+        // through a dangling handle, and unmapping could recycle a SWIOTLB bounce slot
+        // mid-transfer. Reclaiming either requires the assertion only `complete()` can make, so
+        // leak both.
+        dev_warn!(
+            self.0.dev,
+            "StreamingInFlight dropped without complete(); leaking the mapping and its storage\n"
+        );
+    }
+}
+
+/// A guard granting the CPU access to the contents of a [`Streaming`] buffer.
+///
+/// Returned by [`Streaming::for_cpu`]. Dropping it issues a `dma_sync_single_for_device()`, which
+/// hands the buffer back to the device.
+///
+/// # Invariants
+///
+/// * `dev`, `dma_addr`, `size` and `direction` describe the live mapping of the [`Streaming`] this
+///   guard borrows, and are unchanged for the lifetime of the guard.
+/// * `data` refers to exactly the mapped region.
+pub struct StreamingCpuGuard<'a, T: ?Sized> {
+    data: &'a mut T,
+    dev: &'a device::Device<Bound>,
+    dma_addr: DmaAddress,
+    size: usize,
+    direction: DataDirection,
+}
+
+impl<T: ?Sized> Drop for StreamingCpuGuard<'_, T> {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, `self.dev` is bound and `self.dma_addr` denotes a live
+        // mapping of `self.size` bytes established with `self.direction`, which is the range
+        // synced here.
+        unsafe {
+            bindings::dma_sync_single_for_device(
+                self.dev.as_raw(),
+                self.dma_addr,
+                self.size,
+                self.direction.into(),
+            )
+        };
+    }
+}
+
+impl<T: ?Sized> Deref for StreamingCpuGuard<'_, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        self.data
+    }
+}
+
+impl<T: ?Sized> DerefMut for StreamingCpuGuard<'_, T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        self.data
+    }
+}
+
 /// An abstraction of the `dma_alloc_coherent` API.
 ///
 /// This is an abstraction around the `dma_alloc_coherent` API which is used to allocate and map

-- 
2.54.0

Re: [PATCH 2/3] rust: dma: add abstraction for the single-buffer streaming DMA API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] StreamingCpuGuard unconditionally implements DerefMut, allowing safe Rust code to mutate DMA_FROM_DEVICE buffers.
- [High] Safe Rust code can bypass dma_sync_single_for_device by leaking StreamingCpuGuard, causing a hardware data race.
- [Medium] Panic in ContiguousBuffer::data() leaves the DMA buffer synced for the CPU without creating a guard.
- [Low] Missing #[inline] annotations on small/forwarding methods in abstractions.
--

--- Patch [2]: [PATCH 2/3] rust: dma: add abstraction for the single-buffer streaming DMA API ---
commit 5deae1f6c89c43e53f36e40c370843d948aa3422
Author: Maurice Hieronymus <mhi@mailbox.org>

rust: dma: add abstraction for the single-buffer streaming DMA API

Add Streaming, a safe abstraction around dma_map_single_attrs(). Between
map and unmap the buffer belongs to the device, and the CPU may only access
it in between a dma_sync_single_for_cpu() / dma_sync_single_for_device()
pair.

> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 8a8af5ab7feb8..cbaf30a2de864 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -621,6 +630,389 @@ fn data(&mut self) -> &mut Self::Data {

[ ... ]

> +    pub fn for_cpu(&mut self) -> StreamingCpuGuard<'_, C::Data> {
> +        let dev = self.dev;
> +        let dma_addr = self.dma_addr;
> +        let direction = self.direction;
> +        let size = self.container.size();
> +
> +        // SAFETY: By the type invariants, `dev` is bound and `dma_addr` denotes a live mapping of
> +        // `size` bytes established with `direction`, which is the range synced here.
> +        unsafe {
> +            bindings::dma_sync_single_for_cpu(dev.as_raw(), dma_addr, size, direction.into())
> +        };
> +
> +        // INVARIANT: The buffer is now owned by the CPU, and dropping the guard hands it back.
> +        StreamingCpuGuard {
> +            data: self.container.data(),

[Severity: Medium]
If the custom ContiguousBuffer::data() implementation panics, does this
leave the DMA buffer synced for the CPU without creating the guard? While
the kernel currently uses panic=abort, this sequence mutates global state
(the DMA cache mapping) before a potentially panicking safe trait method
call, which could violate panic safety guarantees. Could data() be
evaluated before calling dma_sync_single_for_cpu()?

> +            dev,
> +            dma_addr,
> +            size,
> +            direction,
> +        }
> +    }

[ ... ]

> +    fn unmap(&mut self) {

[Severity: Low]
This isn't a bug, but according to the Rust kernel coding guidelines,
functions in abstractions that are small or directly forward to a C
binding should be annotated with #[inline]. Should this method have
an #[inline] annotation?

> +        // SAFETY: By the type invariants, `self.dev` is bound and the mapping is still live, with
> +        // exactly the address, size, direction and attributes it was created with. Both callers
> +        // run this at most once, so the mapping cannot be torn down twice.
> +        unsafe {
> +            bindings::dma_unmap_single_attrs(
> +                self.dev.as_raw(),
> +                self.dma_addr,
> +                self.container.size(),
> +                self.direction.into(),
> +                self.dma_attrs.as_raw(),
> +            )
> +        };
> +    }

[ ... ]

> +impl<T: ?Sized> Drop for StreamingCpuGuard<'_, T> {
> +    fn drop(&mut self) {

[Severity: High]
Does relying on Drop to run dma_sync_single_for_device() allow safe Rust
code to bypass the synchronization? If a driver calls for_cpu() and then
passes the guard to core::mem::forget(), the mutable borrow ends without
Drop ever being called. The driver could then safely call submit(),
causing the device to interact with an unsynchronized buffer and breaking
the explicit type invariant.

> +        // SAFETY: By the type invariants, `self.dev` is bound and `self.dma_addr` denotes a live
> +        // mapping of `self.size` bytes established with `self.direction`, which is the range
> +        // synced here.
> +        unsafe {
> +            bindings::dma_sync_single_for_device(
> +                self.dev.as_raw(),
> +                self.dma_addr,
> +                self.size,
> +                self.direction.into(),
> +            )
> +        };
> +    }
> +}
> +
> +impl<T: ?Sized> Deref for StreamingCpuGuard<'_, T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {

[Severity: Low]
This isn't a bug, but this small trait implementation in an abstraction
might benefit from an #[inline] annotation to follow the Rust kernel
coding guidelines.

> +        self.data
> +    }
> +}
> +
> +impl<T: ?Sized> DerefMut for StreamingCpuGuard<'_, T> {
> +    fn deref_mut(&mut self) -> &mut Self::Target {

[Severity: Low]
This isn't a bug, but this trait implementation could also use an
#[inline] annotation.

[Severity: High]
Does this unconditional DerefMut implementation permit safe code to mutate
buffers mapped with DataDirection::FromDevice? According to the DMA API,
memory mapped with DMA_FROM_DEVICE must be treated as read-only by the CPU
to avoid dirtying cache lines, which could asynchronously evict and
overwrite the device's DMA writes in physical memory.

> +        self.data
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-dma-streaming-v1-0-03974c86b141@mailbox.org?part=2

Re: [PATCH 2/3] rust: dma: add abstraction for the single-buffer streaming DMA API

[Robin Murphy]

On 2026-08-05 10:54 pm, Maurice Hieronymus wrote:
> Add `Streaming`, a safe abstraction around `dma_map_single_attrs()`.
> 
> Between map and unmap the buffer belongs to the device, and the CPU may
> only access it in between a `dma_sync_single_for_cpu()` /
> `dma_sync_single_for_device()` pair. The types encode that protocol:
> 
> - `Streaming` owns the backing storage, so no other CPU-side reference
>    to the region exists.
> - `submit()` consumes it and returns a `StreamingInFlight`, the only
>    source of the `DmaAddress`. It owns the buffer, so the contents stay
>    unreachable while a transfer may be in flight, no matter where the
>    driver stores the address.
> - `for_cpu()` is therefore safe: no transfer can have been started from
>    a `Streaming`. It syncs for the CPU, returns a guard dereferencing to
>    the contents, and syncs for the device again on drop.
> - `complete()` turns a `StreamingInFlight` back into a `Streaming`.
>    Whether the device has finished cannot be checked by any abstraction,
>    so this is the one `unsafe` operation.

So while the for_cpu() operation itself is the one which could actually 
break coherency and corrupt the buffer contents if the device is still 
writing, we attribute that to the caller having to have erroneously 
declared complete() already in order to be able to do so. But when used 
correctly, we can cycle through complete()/for_cpu()/submit() to recycle 
the same buffer and mapping for multiple transfers within the lifetime 
of the Streaming object. Took me a moment to get it, but this seems like 
a pretty OK design to me.

> Dropping a `StreamingInFlight` leaks the mapping and the backing
> storage, with a warning. A transfer may still be in flight: freeing the
> storage would leave the device writing through a dangling handle, and
> even unmapping could recycle a SWIOTLB bounce slot mid-transfer.
> Reclaiming either takes the assertion only `complete()` can make, so an
> early `?` return between `submit()` and `complete()` costs a leak
> instead of a device-side use-after-free.
> 
> The mapping is torn down on drop of a `Streaming`, or by
> `into_inner()`, which returns the backing storage. Unmapping already
> hands the buffer back to the CPU, so `into_inner()` skips a
> synchronization nothing would consume.
> 
> The `'a` lifetime binds the mapping to a `Device<Bound>`: the DMA API
> may only be called while a driver is bound, and `Drop` unmaps.
> 
> `DataDirection::None` (a `BUG_ON()` in the DMA core) and empty buffers
> (not representable by `dma_map_single()`) are rejected with `EINVAL`.
> So are `DMA_ATTR_SKIP_CPU_SYNC`, which disables the implicit CPU cache
> maintenance the type invariants are built on with no way to compensate
> through this API, and `DMA_ATTR_MMIO`, which describes memory a
> `ContiguousBuffer` cannot represent.

Yeah, SKIP_CPU_SYNC is a little tricky as it effectively has two 
different use-cases - one is publishing the same buffer to multiple 
devices, which I guess might best be encapsulated as some new variant to 
be created from an existing Streaming object and a different device, 
while the other is effectively still the same model as here, just with 
the object starting in, and/or being torn down directly from, the 
for_cpu state.

Indeed we can safely say that we should never need to accept MMIO here 
though - dma_map_resource() can have its own Rust abstraction if and 
when anyone wants that, and the P2P dma-buf business would also be its 
own whole other issue anyway.

Thanks,
Robin.

> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
> ---
>   rust/helpers/dma.c |  35 +++++
>   rust/kernel/dma.rs | 392 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 427 insertions(+)
> 
> diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
> index 9fbeb507b08c..7e1e5c67431a 100644
> --- a/rust/helpers/dma.c
> +++ b/rust/helpers/dma.c
> @@ -49,3 +49,38 @@ __rust_helper void rust_helper_dma_set_max_seg_size(struct device *dev,
>   {
>   	dma_set_max_seg_size(dev, size);
>   }
> +
> +__rust_helper dma_addr_t rust_helper_dma_map_single_attrs(struct device *dev,
> +							  void *ptr, size_t size,
> +							  enum dma_data_direction dir,
> +							  unsigned long attrs)
> +{
> +	return dma_map_single_attrs(dev, ptr, size, dir, attrs);
> +}
> +
> +__rust_helper void rust_helper_dma_unmap_single_attrs(struct device *dev,
> +						      dma_addr_t addr, size_t size,
> +						      enum dma_data_direction dir,
> +						      unsigned long attrs)
> +{
> +	dma_unmap_single_attrs(dev, addr, size, dir, attrs);
> +}
> +
> +__rust_helper int rust_helper_dma_mapping_error(struct device *dev, dma_addr_t addr)
> +{
> +	return dma_mapping_error(dev, addr);
> +}
> +
> +__rust_helper void rust_helper_dma_sync_single_for_cpu(struct device *dev,
> +						       dma_addr_t addr, size_t size,
> +						       enum dma_data_direction dir)
> +{
> +	dma_sync_single_for_cpu(dev, addr, size, dir);
> +}
> +
> +__rust_helper void rust_helper_dma_sync_single_for_device(struct device *dev,
> +							  dma_addr_t addr, size_t size,
> +							  enum dma_data_direction dir)
> +{
> +	dma_sync_single_for_device(dev, addr, size, dir);
> +}
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 8a8af5ab7feb..cbaf30a2de86 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -24,6 +24,7 @@
>       uaccess::UserSliceWriter,
>   };
>   use core::{
> +    mem::ManuallyDrop,
>       ops::{
>           Deref,
>           DerefMut, //
> @@ -345,6 +346,14 @@ const fn const_cast(val: bindings::dma_data_direction) -> u32 {
>           // is within the representable range of `u32`.
>           wide_val as u32
>       }
> +
> +    /// Returns whether this direction may be passed to a mapping or synchronization primitive.
> +    ///
> +    /// Equivalent to `valid_dma_direction()`; [`Self::None`] is a debugging aid the DMA core
> +    /// rejects with a `BUG_ON()`.
> +    const fn is_valid(self) -> bool {
> +        !matches!(self, Self::None)
> +    }
>   }
>   
>   impl From<DataDirection> for bindings::dma_data_direction {
> @@ -621,6 +630,389 @@ fn data(&mut self) -> &mut Self::Data {
>       }
>   }
>   
> +/// An abstraction of the `dma_map_single` API.
> +///
> +/// Unlike [`Coherent`], a streaming mapping is a temporary lease on memory the caller already
> +/// owns: between mapping and unmapping the buffer belongs to the device, and the CPU may only
> +/// access it in between a `dma_sync_single_for_cpu()` / `dma_sync_single_for_device()` pair.
> +///
> +/// [`Streaming`] owns the backing storage and is one of two states: [`submit`](Self::submit)
> +/// consumes it and returns a [`StreamingInFlight`], the only source of the [`DmaAddress`];
> +/// [`complete`](StreamingInFlight::complete) turns that back into a [`Streaming`], whose
> +/// [`for_cpu`](Self::for_cpu) yields a [`StreamingCpuGuard`] dereferencing to the contents.
> +///
> +/// The mapping is torn down on drop of a [`Streaming`], or by [`into_inner`](Self::into_inner),
> +/// which returns the backing storage. Dropping a [`StreamingInFlight`] instead leaks the mapping
> +/// and the storage: safe code cannot prove the device is done, so it cannot be allowed to reclaim
> +/// either.
> +///
> +/// The `'a` lifetime keeps the device bound for the life of the mapping.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # use kernel::device::{Bound, Device};
> +/// use kernel::dma::{
> +///     DataDirection,
> +///     Streaming, //
> +/// };
> +///
> +/// # fn test(dev: &Device<Bound>) -> Result {
> +/// let buf = KBox::new(0u64, GFP_KERNEL)?;
> +/// let mut dma = Streaming::new(dev, buf, DataDirection::Bidirectional)?;
> +///
> +/// // The CPU prepares the buffer, and hands it back to the device by dropping the guard.
> +/// *dma.for_cpu() = 42;
> +///
> +/// // Hand the buffer to the device; `dma` is consumed, so its contents are now unreachable.
> +/// let dma = dma.submit();
> +///
> +/// // Program `dma.dma_handle()` and `dma.size()` into the device.
> +///
> +/// // SAFETY: For the sake of the example, assume the transfer has been waited for.
> +/// let mut dma = unsafe { dma.complete() };
> +///
> +/// assert_eq!(*dma.for_cpu(), 42);
> +/// # Ok::<(), Error>(()) }
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// * `dma_addr` denotes a live mapping of `container` for the lifetime of the instance, and
> +///   `container`, `direction` and `dma_attrs` are unchanged since it was established.
> +/// * `direction` is not [`DataDirection::None`].
> +/// * The buffer is synchronized for the device whenever no [`StreamingCpuGuard`] borrowed from
> +///   this instance is alive.
> +pub struct Streaming<'a, C: ContiguousBuffer> {
> +    container: C,
> +    direction: DataDirection,
> +    dma_addr: DmaAddress,
> +    dma_attrs: Attrs,
> +    dev: &'a device::Device<Bound>,
> +}
> +
> +impl<'a, C: ContiguousBuffer> Streaming<'a, C> {
> +    /// Maps `container` for streaming DMA in `direction`.
> +    ///
> +    /// Ownership of `container` is moved into the returned [`Streaming`]; the buffer belongs to
> +    /// the device until [`for_cpu`](Self::for_cpu) is called.
> +    ///
> +    /// Returns [`EINVAL`] for an empty buffer, for [`DataDirection::None`], or if `dma_attrs`
> +    /// contains [`DMA_ATTR_SKIP_CPU_SYNC`](attrs::DMA_ATTR_SKIP_CPU_SYNC) or
> +    /// [`DMA_ATTR_MMIO`](attrs::DMA_ATTR_MMIO).
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::device::{Bound, Device};
> +    /// use kernel::dma::{
> +    ///     attrs::*,
> +    ///     DataDirection,
> +    ///     Streaming, //
> +    /// };
> +    ///
> +    /// # fn test(dev: &Device<Bound>) -> Result {
> +    /// let buf = KBox::new(0u64, GFP_KERNEL)?;
> +    /// let dma = Streaming::new_with_attrs(
> +    ///     dev,
> +    ///     buf,
> +    ///     DataDirection::ToDevice,
> +    ///     DMA_ATTR_WEAK_ORDERING,
> +    /// )?;
> +    /// # Ok::<(), Error>(()) }
> +    /// ```
> +    pub fn new_with_attrs(
> +        dev: &'a device::Device<Bound>,
> +        mut container: C,
> +        direction: DataDirection,
> +        dma_attrs: Attrs,
> +    ) -> Result<Self> {
> +        // The DMA core `BUG_ON()`s on `DMA_NONE`, bail early.
> +        if !direction.is_valid() {
> +            return Err(EINVAL);
> +        }
> +
> +        // The type invariants are built on the implicit CPU cache maintenance performed by
> +        // `dma_map_single_attrs()` and `dma_unmap_single_attrs()`; `DMA_ATTR_SKIP_CPU_SYNC`
> +        // disables it, with no way to compensate through this API. `DMA_ATTR_MMIO` describes
> +        // memory a `ContiguousBuffer` cannot represent.
> +        if dma_attrs.contains(attrs::DMA_ATTR_SKIP_CPU_SYNC)
> +            || dma_attrs.contains(attrs::DMA_ATTR_MMIO)
> +        {
> +            return Err(EINVAL);
> +        }
> +
> +        let size = container.size();
> +
> +        // `dma_map_single_attrs` cannot handle zero-length mappings, bail early.
> +        if size == 0 {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY:
> +        // - Device pointer is guaranteed as valid by the type invariant on `Device`.
> +        // - By the safety requirements of `ContiguousBuffer`, `container.ptr()` points to a single
> +        //   physically contiguous region of `size` bytes in the kernel's linear mapping.
> +        // - `container` is moved into `Self` below, so the region stays alive and at a stable
> +        //   address until the mapping is torn down in `Drop`.
> +        let dma_addr = unsafe {
> +            bindings::dma_map_single_attrs(
> +                dev.as_raw(),
> +                container.ptr(),
> +                size,
> +                direction.into(),
> +                dma_attrs.as_raw(),
> +            )
> +        };
> +
> +        // SAFETY: Device pointer is valid per the above, and `dma_addr` was just returned by
> +        // `dma_map_single_attrs()` for this device.
> +        to_result(unsafe { bindings::dma_mapping_error(dev.as_raw(), dma_addr) })?;
> +
> +        // INVARIANT:
> +        // - The mapping was just established with these exact parameters, none of which is
> +        //   mutated afterwards.
> +        // - `direction` was checked above.
> +        Ok(Streaming {
> +            container,
> +            direction,
> +            dma_addr,
> +            dma_attrs,
> +            dev,
> +        })
> +    }
> +
> +    /// Performs the same functionality as [`Streaming::new_with_attrs`], except the `dma_attrs`
> +    /// is 0 by default.
> +    #[inline]
> +    pub fn new(
> +        dev: &'a device::Device<Bound>,
> +        container: C,
> +        direction: DataDirection,
> +    ) -> Result<Self> {
> +        Self::new_with_attrs(dev, container, direction, Attrs(0))
> +    }
> +
> +    /// Returns the size of the mapping in bytes.
> +    #[inline]
> +    pub fn size(&self) -> usize {
> +        self.container.size()
> +    }
> +
> +    /// Returns the direction this buffer was mapped with.
> +    #[inline]
> +    pub fn direction(&self) -> DataDirection {
> +        self.direction
> +    }
> +
> +    /// Hands the buffer to the device.
> +    ///
> +    /// This performs no synchronization: by the type invariants the buffer is already
> +    /// synchronized for the device.
> +    #[inline]
> +    pub fn submit(self) -> StreamingInFlight<'a, C> {
> +        StreamingInFlight(ManuallyDrop::new(self))
> +    }
> +
> +    /// Transfers ownership of the buffer back to the CPU and returns a guard granting access to
> +    /// its contents.
> +    ///
> +    /// Dropping the guard transfers ownership back to the device. If the buffer is not handed to
> +    /// the device again, prefer [`into_inner`](Self::into_inner), which unmaps instead.
> +    pub fn for_cpu(&mut self) -> StreamingCpuGuard<'_, C::Data> {
> +        let dev = self.dev;
> +        let dma_addr = self.dma_addr;
> +        let direction = self.direction;
> +        let size = self.container.size();
> +
> +        // SAFETY: By the type invariants, `dev` is bound and `dma_addr` denotes a live mapping of
> +        // `size` bytes established with `direction`, which is the range synced here.
> +        unsafe {
> +            bindings::dma_sync_single_for_cpu(dev.as_raw(), dma_addr, size, direction.into())
> +        };
> +
> +        // INVARIANT: The buffer is now owned by the CPU, and dropping the guard hands it back.
> +        StreamingCpuGuard {
> +            data: self.container.data(),
> +            dev,
> +            dma_addr,
> +            size,
> +            direction,
> +        }
> +    }
> +
> +    /// Tears the mapping down and returns the backing storage.
> +    ///
> +    /// Unmapping transfers ownership of the buffer back to the CPU, so no separate
> +    /// [`for_cpu`](Self::for_cpu) is needed.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::device::{Bound, Device};
> +    /// use kernel::dma::{
> +    ///     DataDirection,
> +    ///     Streaming, //
> +    /// };
> +    ///
> +    /// # fn test(dev: &Device<Bound>) -> Result {
> +    /// let dma = Streaming::new(
> +    ///     dev,
> +    ///     KBox::new(0u64, GFP_KERNEL)?,
> +    ///     DataDirection::FromDevice,
> +    /// )?
> +    /// .submit();
> +    ///
> +    /// // Program `dma.dma_handle()` into the device.
> +    ///
> +    /// // SAFETY: For the sake of the example, assume the transfer has been waited for.
> +    /// let dma = unsafe { dma.complete() };
> +    ///
> +    /// // Take the buffer back; the mapping is gone once this returns.
> +    /// let buf: KBox<u64> = dma.into_inner();
> +    /// # Ok::<(), Error>(()) }
> +    /// ```
> +    pub fn into_inner(self) -> C {
> +        let mut this = ManuallyDrop::new(self);
> +
> +        this.unmap();
> +
> +        // SAFETY: `this` is wrapped in a `ManuallyDrop`, so `Streaming::drop()` never runs and
> +        // `this.container` is never read again. The remaining fields are all `Copy`.
> +        unsafe { core::ptr::read(&this.container) }
> +    }
> +
> +    /// Tears the mapping down.
> +    ///
> +    /// Shared by [`Drop`] and [`into_inner`](Self::into_inner), both of which run it exactly once.
> +    fn unmap(&mut self) {
> +        // SAFETY: By the type invariants, `self.dev` is bound and the mapping is still live, with
> +        // exactly the address, size, direction and attributes it was created with. Both callers
> +        // run this at most once, so the mapping cannot be torn down twice.
> +        unsafe {
> +            bindings::dma_unmap_single_attrs(
> +                self.dev.as_raw(),
> +                self.dma_addr,
> +                self.container.size(),
> +                self.direction.into(),
> +                self.dma_attrs.as_raw(),
> +            )
> +        };
> +    }
> +}
> +
> +impl<C: ContiguousBuffer> Drop for Streaming<'_, C> {
> +    fn drop(&mut self) {
> +        self.unmap();
> +    }
> +}
> +
> +/// A [`Streaming`] mapping whose [`DmaAddress`] has been handed out.
> +///
> +/// Returned by [`Streaming::submit`]. It owns the buffer, so the contents are unreachable while
> +/// it exists. [`complete`](Self::complete) is the only way back: it is the caller's assertion
> +/// that the device has finished, which nothing else can establish. Dropping this instead leaks
> +/// the mapping and the backing storage, since reclaiming either while the device may still
> +/// access the buffer would be a use-after-free.
> +pub struct StreamingInFlight<'a, C: ContiguousBuffer>(ManuallyDrop<Streaming<'a, C>>);
> +
> +impl<'a, C: ContiguousBuffer> StreamingInFlight<'a, C> {
> +    /// Returns the DMA address to program into the device.
> +    #[inline]
> +    pub fn dma_handle(&self) -> DmaAddress {
> +        self.0.dma_addr
> +    }
> +
> +    /// Returns the size of the mapping in bytes.
> +    #[inline]
> +    pub fn size(&self) -> usize {
> +        self.0.size()
> +    }
> +
> +    /// Returns the direction this buffer was mapped with.
> +    #[inline]
> +    pub fn direction(&self) -> DataDirection {
> +        self.0.direction()
> +    }
> +
> +    /// Takes the buffer back from the device.
> +    ///
> +    /// This performs no synchronization; [`Streaming::for_cpu`] does that.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The device must have finished accessing the buffer.
> +    #[inline]
> +    pub unsafe fn complete(self) -> Streaming<'a, C> {
> +        let mut this = ManuallyDrop::new(self);
> +
> +        // SAFETY: `this` is wrapped in a `ManuallyDrop`, so `StreamingInFlight::drop()` never
> +        // runs and `this.0` is never touched again.
> +        unsafe { ManuallyDrop::take(&mut this.0) }
> +    }
> +}
> +
> +impl<C: ContiguousBuffer> Drop for StreamingInFlight<'_, C> {
> +    fn drop(&mut self) {
> +        // A transfer may still be in flight: freeing the storage would leave the device writing
> +        // through a dangling handle, and unmapping could recycle a SWIOTLB bounce slot
> +        // mid-transfer. Reclaiming either requires the assertion only `complete()` can make, so
> +        // leak both.
> +        dev_warn!(
> +            self.0.dev,
> +            "StreamingInFlight dropped without complete(); leaking the mapping and its storage\n"
> +        );
> +    }
> +}
> +
> +/// A guard granting the CPU access to the contents of a [`Streaming`] buffer.
> +///
> +/// Returned by [`Streaming::for_cpu`]. Dropping it issues a `dma_sync_single_for_device()`, which
> +/// hands the buffer back to the device.
> +///
> +/// # Invariants
> +///
> +/// * `dev`, `dma_addr`, `size` and `direction` describe the live mapping of the [`Streaming`] this
> +///   guard borrows, and are unchanged for the lifetime of the guard.
> +/// * `data` refers to exactly the mapped region.
> +pub struct StreamingCpuGuard<'a, T: ?Sized> {
> +    data: &'a mut T,
> +    dev: &'a device::Device<Bound>,
> +    dma_addr: DmaAddress,
> +    size: usize,
> +    direction: DataDirection,
> +}
> +
> +impl<T: ?Sized> Drop for StreamingCpuGuard<'_, T> {
> +    fn drop(&mut self) {
> +        // SAFETY: By the type invariants, `self.dev` is bound and `self.dma_addr` denotes a live
> +        // mapping of `self.size` bytes established with `self.direction`, which is the range
> +        // synced here.
> +        unsafe {
> +            bindings::dma_sync_single_for_device(
> +                self.dev.as_raw(),
> +                self.dma_addr,
> +                self.size,
> +                self.direction.into(),
> +            )
> +        };
> +    }
> +}
> +
> +impl<T: ?Sized> Deref for StreamingCpuGuard<'_, T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        self.data
> +    }
> +}
> +
> +impl<T: ?Sized> DerefMut for StreamingCpuGuard<'_, T> {
> +    fn deref_mut(&mut self) -> &mut Self::Target {
> +        self.data
> +    }
> +}
> +
>   /// An abstraction of the `dma_alloc_coherent` API.
>   ///
>   /// This is an abstraction around the `dma_alloc_coherent` API which is used to allocate and map
> 

[PATCH 3/3] gpu: nova-core: gsp: map the WPR meta for streaming DMA

[Maurice Hieronymus]

`GspFwWprMeta` is filled in once during boot, read out of system memory
by the booter, and dropped when `Gsp::boot()` returns. That is a
streaming transfer, so a coherent allocation buys nothing.

Map a `KBox<GspFwWprMeta>` instead. The mapping is submitted right
after initialization, which makes the contents unreachable for the
duration of the chipset-specific boot sequence and lets `dma_handle()`
be read through a shared reference.

`complete()` is called as soon as `hal.boot()` succeeds, the earliest
point the device is provably done with the metadata: on Tu102 the
Booter-load falcon has halted, on GH100 GSP-FMC has released the
lockdown. If `hal.boot()` fails instead, no such proof exists --
`Gsp::unload()` deliberately carries on past failed steps, so the
falcon may still be reading the buffer -- and `wpr_meta` drops in
flight, trading a one-off leak for a device-side use-after-free.

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 drivers/gpu/nova-core/firmware/booter.rs | 11 +++++++----
 drivers/gpu/nova-core/gsp/boot.rs        | 20 ++++++++++++++++++--
 drivers/gpu/nova-core/gsp/hal.rs         |  4 ++--
 drivers/gpu/nova-core/gsp/hal/gh100.rs   |  4 ++--
 drivers/gpu/nova-core/gsp/hal/tu102.rs   |  4 ++--
 5 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index d9313ac361af..780c7702a777 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -9,9 +9,12 @@
 
 use kernel::{
     device,
-    dma::Coherent,
+    dma::StreamingInFlight,
     prelude::*,
-    transmute::FromBytes, //
+    transmute::{
+        AsBytes,
+        FromBytes, //
+    },
 };
 
 use crate::{
@@ -402,12 +405,12 @@ pub(crate) fn new(
     ///
     /// Resets SEC2, loads this firmware image, then boots with the WPR metadata
     /// address passed via the SEC2 mailboxes.
-    pub(crate) fn run<T>(
+    pub(crate) fn run<T: FromBytes + AsBytes>(
         &self,
         dev: &device::Device<device::Bound>,
         bar: Bar0<'_>,
         sec2_falcon: &Falcon<Sec2>,
-        wpr_meta: &Coherent<T>,
+        wpr_meta: &StreamingInFlight<'_, KBox<T>>,
     ) -> Result {
         sec2_falcon.reset(bar)?;
         sec2_falcon.load(dev, bar, self)?;
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 8afb62d689cb..300ebf4e843d 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -4,7 +4,10 @@
 use kernel::{
     bits,
     device,
-    dma::Coherent,
+    dma::{
+        DataDirection,
+        Streaming, //
+    },
     io::poll::read_poll_timeout,
     pci,
     prelude::*,
@@ -117,7 +120,12 @@ pub(crate) fn boot(
         let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
         dev_dbg!(dev, "{:#x?}\n", fb_layout);
 
-        let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
+        let wpr_meta = Streaming::new(
+            dev,
+            KBox::init(GspFwWprMeta::new(&gsp_fw, &fb_layout), GFP_KERNEL)?,
+            DataDirection::ToDevice,
+        )?
+        .submit();
 
         // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
         let unload_guard = hal.boot(
@@ -131,6 +139,14 @@ pub(crate) fn boot(
             sec2_falcon,
         )?;
 
+        // The chipset-specific boot sequence only succeeds once the device is done reading the
+        // WPR metadata: on Tu102 the Booter-load falcon has halted, on GH100 GSP-FMC has released
+        // the lockdown. If it fails instead, `wpr_meta` drops in flight and leaks, as the falcon
+        // may still be reading the buffer.
+        //
+        // SAFETY: Per the above, the device has finished accessing the buffer.
+        let _ = unsafe { wpr_meta.complete() };
+
         gsp_falcon.write_os_version(bar, gsp_fw.bootloader.app_version);
 
         // Poll for RISC-V to become active before continuing.
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index 04f004856c60..09a523e3a180 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -8,7 +8,7 @@
 
 use kernel::{
     device,
-    dma::Coherent, //
+    dma::StreamingInFlight, //
 };
 
 use crate::{
@@ -61,7 +61,7 @@ fn boot<'a>(
         bar: Bar0<'a>,
         chipset: Chipset,
         fb_layout: &FbLayout,
-        wpr_meta: &Coherent<GspFwWprMeta>,
+        wpr_meta: &StreamingInFlight<'_, KBox<GspFwWprMeta>>,
         gsp_falcon: &'a Falcon<GspEngine>,
         sec2_falcon: &'a Falcon<Sec2>,
     ) -> Result<BootUnloadGuard<'a>>;
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index 98f5ce197d13..67a79c54a739 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -5,7 +5,7 @@
 
 use kernel::{
     device,
-    dma::Coherent,
+    dma::StreamingInFlight,
     io::poll::read_poll_timeout,
     time::Delta, //
 };
@@ -156,7 +156,7 @@ fn boot<'a>(
         bar: Bar0<'a>,
         chipset: Chipset,
         fb_layout: &FbLayout,
-        wpr_meta: &Coherent<GspFwWprMeta>,
+        wpr_meta: &StreamingInFlight<'_, KBox<GspFwWprMeta>>,
         gsp_falcon: &'a Falcon<GspEngine>,
         sec2_falcon: &'a Falcon<Sec2>,
     ) -> Result<BootUnloadGuard<'a>> {
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index 2f6301af7113..7d906de6d8bf 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -5,7 +5,7 @@
 
 use kernel::{
     device,
-    dma::Coherent,
+    dma::StreamingInFlight,
     io::Io, //
 };
 
@@ -262,7 +262,7 @@ fn boot<'a>(
         bar: Bar0<'a>,
         chipset: Chipset,
         fb_layout: &FbLayout,
-        wpr_meta: &Coherent<GspFwWprMeta>,
+        wpr_meta: &StreamingInFlight<'_, KBox<GspFwWprMeta>>,
         gsp_falcon: &'a Falcon<GspEngine>,
         sec2_falcon: &'a Falcon<Sec2>,
     ) -> Result<BootUnloadGuard<'a>> {

-- 
2.54.0

Re: [PATCH 3/3] gpu: nova-core: gsp: map the WPR meta for streaming DMA

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unnecessary memory and DMA mapping leak on early failure paths in `hal.boot()`.
--

--- Patch [3]: [PATCH 3/3] gpu: nova-core: gsp: map the WPR meta for streaming DMA ---
commit 3c4ae11167b244d18dae6296df30ee5e35af6eb1
Author: Maurice Hieronymus <mhi@mailbox.org>

gpu: nova-core: gsp: map the WPR meta for streaming DMA

This commit changes the WPR metadata mapping from a coherent allocation
to a streaming DMA transfer. It intentionally drops the mapping in flight
without completing it if the boot sequence fails, to prevent a device-side
use-after-free.

> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index 8afb62d689cb4..300ebf4e843d6 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
[ ... ]
> @@ -131,6 +139,14 @@ pub(crate) fn boot(
>              sec2_falcon,
>          )?;
>  
> +        // The chipset-specific boot sequence only succeeds once the device is done reading the
> +        // WPR metadata: on Tu102 the Booter-load falcon has halted, on GH100 GSP-FMC has released
> +        // the lockdown. If it fails instead, `wpr_meta` drops in flight and leaks, as the falcon
> +        // may still be reading the buffer.
> +        //
> +        // SAFETY: Per the above, the device has finished accessing the buffer.
> +        let _ = unsafe { wpr_meta.complete() };

[Severity: Medium]
The patch intentionally drops wpr_meta in flight if hal.boot() returns an
error to avoid a device-side use-after-free. However, does this unconditionally
leak the memory and DMA mapping even on early failures where hardware access
is structurally impossible?

For example, looking at Gh100::boot() in drivers/gpu/nova-core/gsp/hal/gh100.rs,
an error can occur early:

Gh100::boot() {
    ...
    let fsp_fw = FspFirmware::new(dev, chipset, FIRMWARE_VERSION)?;
    ...
}

This happens before the device is given the DMA handle.

Similarly in drivers/gpu/nova-core/gsp/hal/tu102.rs:

Tu102::boot() {
    ...
    let bios = Vbios::new(dev, bar)?;
    ...
}

This also happens before the hardware is given the DMA handle via
BooterFirmware::run().

Because hal.boot() only borrows wpr_meta by reference, Gsp::boot() cannot
distinguish whether the error occurred before or after the DMA address was
handed to the hardware. Consequently, it leaks the buffer on any error.

Could the design be adjusted to only leak the buffer if the error happens
after the hardware actually receives the DMA handle?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-dma-streaming-v1-0-03974c86b141@mailbox.org?part=3