public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs
@ 2026-03-21 17:27 Danilo Krummrich
  2026-03-21 17:27 ` [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping Danilo Krummrich
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-03-21 17:27 UTC (permalink / raw)
  To: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, gary, bjorn3_gh, lossin, aliceryhl, tmgross
  Cc: driver-core, rust-for-linux, linux-kernel, Danilo Krummrich,
	stable

When DMA_ATTR_NO_KERNEL_MAPPING is passed to dma_alloc_attrs(), the
returned CPU address is not a pointer to the allocated memory but an
opaque handle (e.g. struct page *).

Coherent<T> (or CoherentAllocation<T> respectively) stores this value as
NonNull<T> and exposes methods that dereference it and even modify its
contents.

Remove the flag from the public attrs module such that drivers cannot
pass it to Coherent<T> (or CoherentAllocation<T> respectively) in the
first place.

Instead DMA_ATTR_NO_KERNEL_MAPPING can be supported with an additional
opaque type (e.g. CoherentHandle) which does not provide access to the
allocated memory.

Cc: stable@vger.kernel.org
Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/dma.rs | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 6d2bec52806b..9e0c9ff91cba 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -264,9 +264,6 @@ pub mod attrs {
     /// Specifies that writes to the mapping may be buffered to improve performance.
     pub const DMA_ATTR_WRITE_COMBINE: Attrs = Attrs(bindings::DMA_ATTR_WRITE_COMBINE);
 
-    /// Lets the platform to avoid creating a kernel virtual mapping for the allocated buffer.
-    pub const DMA_ATTR_NO_KERNEL_MAPPING: Attrs = Attrs(bindings::DMA_ATTR_NO_KERNEL_MAPPING);
-
     /// Allows platform code to skip synchronization of the CPU cache for the given buffer assuming
     /// that it has been already transferred to 'device' domain.
     pub const DMA_ATTR_SKIP_CPU_SYNC: Attrs = Attrs(bindings::DMA_ATTR_SKIP_CPU_SYNC);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-21 17:27 [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Danilo Krummrich
@ 2026-03-21 17:27 ` Danilo Krummrich
  2026-03-22 14:52   ` Alexandre Courbot
  2026-03-24 20:10   ` Gary Guo
  2026-03-21 18:42 ` [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Gary Guo
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-03-21 17:27 UTC (permalink / raw)
  To: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, gary, bjorn3_gh, lossin, aliceryhl, tmgross
  Cc: driver-core, rust-for-linux, linux-kernel, Danilo Krummrich

Add CoherentHandle, an opaque DMA allocation type for buffers that are
only ever accessed by hardware. Unlike Coherent<T>, it does not provide
CPU access to the allocated memory.

CoherentHandle implicitly sets DMA_ATTR_NO_KERNEL_MAPPING and stores the
value returned by dma_alloc_attrs() as an opaque handle
(NonNull<c_void>) rather than a typed pointer, since with this flag the
C API returns an opaque cookie (e.g. struct page *), not a CPU pointer
to the allocated memory.

Only the DMA bus address is exposed to drivers; the opaque handle is
used solely to free the allocation on drop.

This commit is for reference only; there is currently no in-tree user.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/dma.rs | 119 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 9e0c9ff91cba..fa30793c798d 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -1011,6 +1011,125 @@ fn drop(&mut self) {
 // can be sent to another thread.
 unsafe impl<T: KnownSize + Send + ?Sized> Send for Coherent<T> {}
 
+/// An opaque DMA allocation without a kernel virtual mapping.
+///
+/// Unlike [`Coherent`], a `CoherentHandle` does not provide CPU access to the allocated memory.
+/// The allocation is always performed with `DMA_ATTR_NO_KERNEL_MAPPING`, meaning no kernel
+/// virtual mapping is created for the buffer. The value returned by the C API as the CPU
+/// address is an opaque handle used only to free the allocation.
+///
+/// This is useful for buffers that are only ever accessed by hardware.
+///
+/// # Invariants
+///
+/// - `cpu_handle` holds the opaque handle returned by `dma_alloc_attrs` with
+///   `DMA_ATTR_NO_KERNEL_MAPPING` set, and is only valid for passing back to `dma_free_attrs`.
+/// - `dma_handle` is the corresponding bus address for device DMA.
+/// - `size` is the allocation size in bytes as passed to `dma_alloc_attrs`.
+/// - `dma_attrs` contains the attributes used for the allocation, always including
+///   `DMA_ATTR_NO_KERNEL_MAPPING`.
+pub struct CoherentHandle {
+    dev: ARef<device::Device>,
+    dma_handle: DmaAddress,
+    cpu_handle: NonNull<c_void>,
+    size: usize,
+    dma_attrs: Attrs,
+}
+
+impl CoherentHandle {
+    /// Allocates `size` bytes of coherent DMA memory without creating a kernel virtual mapping.
+    ///
+    /// Additional DMA attributes may be passed via `dma_attrs`; `DMA_ATTR_NO_KERNEL_MAPPING` is
+    /// always set implicitly.
+    ///
+    /// Returns `EINVAL` if `size` is zero, `ENOMEM` if the allocation fails.
+    pub fn alloc_with_attrs(
+        dev: &device::Device<Bound>,
+        size: usize,
+        gfp_flags: kernel::alloc::Flags,
+        dma_attrs: Attrs,
+    ) -> Result<Self> {
+        if size == 0 {
+            return Err(EINVAL);
+        }
+
+        let dma_attrs = dma_attrs | Attrs(bindings::DMA_ATTR_NO_KERNEL_MAPPING);
+        let mut dma_handle = 0;
+        // SAFETY: `dev.as_raw()` is valid by the type invariant on `device::Device`.
+        let cpu_handle = unsafe {
+            bindings::dma_alloc_attrs(
+                dev.as_raw(),
+                size,
+                &mut dma_handle,
+                gfp_flags.as_raw(),
+                dma_attrs.as_raw(),
+            )
+        };
+
+        let cpu_handle = NonNull::new(cpu_handle).ok_or(ENOMEM)?;
+
+        // INVARIANT: `cpu_handle` is the opaque handle from a successful `dma_alloc_attrs` call
+        // with `DMA_ATTR_NO_KERNEL_MAPPING`, `dma_handle` is the corresponding DMA address,
+        // and we hold a refcounted reference to the device.
+        Ok(Self {
+            dev: dev.into(),
+            dma_handle,
+            cpu_handle,
+            size,
+            dma_attrs,
+        })
+    }
+
+    /// Allocates `size` bytes of coherent DMA memory without creating a kernel virtual mapping.
+    #[inline]
+    pub fn alloc(
+        dev: &device::Device<Bound>,
+        size: usize,
+        gfp_flags: kernel::alloc::Flags,
+    ) -> Result<Self> {
+        Self::alloc_with_attrs(dev, size, gfp_flags, Attrs(0))
+    }
+
+    /// Returns the DMA handle for this allocation.
+    ///
+    /// This address can be programmed into device hardware for DMA access.
+    #[inline]
+    pub fn dma_handle(&self) -> DmaAddress {
+        self.dma_handle
+    }
+
+    /// Returns the size in bytes of this allocation.
+    #[inline]
+    pub fn size(&self) -> usize {
+        self.size
+    }
+}
+
+impl Drop for CoherentHandle {
+    fn drop(&mut self) {
+        // SAFETY: All values are valid by the type invariants on `CoherentHandle`.
+        // `cpu_handle` is the opaque handle from `dma_alloc_attrs` and is passed back unchanged.
+        unsafe {
+            bindings::dma_free_attrs(
+                self.dev.as_raw(),
+                self.size,
+                self.cpu_handle.as_ptr(),
+                self.dma_handle,
+                self.dma_attrs.as_raw(),
+            )
+        }
+    }
+}
+
+// SAFETY: `CoherentHandle` only holds a device reference, a DMA handle, an opaque CPU handle,
+// and a size. None of these are tied to a specific thread.
+unsafe impl Send for CoherentHandle {}
+
+// SAFETY: `CoherentHandle` provides no CPU access to the underlying allocation. The only
+// operations on `&CoherentHandle` are reading the DMA handle and size, both of which are
+// plain `Copy` values.
+unsafe impl Sync for CoherentHandle {}
+
 /// Reads a field of an item from an allocated region of structs.
 ///
 /// The syntax is of the form `kernel::dma_read!(dma, proj)` where `dma` is an expression evaluating
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs
  2026-03-21 17:27 [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Danilo Krummrich
  2026-03-21 17:27 ` [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping Danilo Krummrich
@ 2026-03-21 18:42 ` Gary Guo
  2026-03-22 11:07 ` Alice Ryhl
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Gary Guo @ 2026-03-21 18:42 UTC (permalink / raw)
  To: Danilo Krummrich, abdiel.janulgue, daniel.almeida, robin.murphy,
	a.hindborg, ojeda, boqun, gary, bjorn3_gh, lossin, aliceryhl,
	tmgross
  Cc: driver-core, rust-for-linux, linux-kernel, stable

On Sat Mar 21, 2026 at 5:27 PM GMT, Danilo Krummrich wrote:
> When DMA_ATTR_NO_KERNEL_MAPPING is passed to dma_alloc_attrs(), the
> returned CPU address is not a pointer to the allocated memory but an
> opaque handle (e.g. struct page *).
> 
> Coherent<T> (or CoherentAllocation<T> respectively) stores this value as
> NonNull<T> and exposes methods that dereference it and even modify its
> contents.
> 
> Remove the flag from the public attrs module such that drivers cannot
> pass it to Coherent<T> (or CoherentAllocation<T> respectively) in the
> first place.
> 
> Instead DMA_ATTR_NO_KERNEL_MAPPING can be supported with an additional
> opaque type (e.g. CoherentHandle) which does not provide access to the
> allocated memory.
> 
> Cc: stable@vger.kernel.org
> Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/dma.rs | 3 ---
>  1 file changed, 3 deletions(-)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs
  2026-03-21 17:27 [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Danilo Krummrich
  2026-03-21 17:27 ` [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping Danilo Krummrich
  2026-03-21 18:42 ` [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Gary Guo
@ 2026-03-22 11:07 ` Alice Ryhl
  2026-03-22 14:52 ` Alexandre Courbot
  2026-03-28 14:51 ` Alexandre Courbot
  4 siblings, 0 replies; 13+ messages in thread
From: Alice Ryhl @ 2026-03-22 11:07 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, gary, bjorn3_gh, lossin, tmgross, driver-core,
	rust-for-linux, linux-kernel, stable

On Sat, Mar 21, 2026 at 6:27 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> When DMA_ATTR_NO_KERNEL_MAPPING is passed to dma_alloc_attrs(), the
> returned CPU address is not a pointer to the allocated memory but an
> opaque handle (e.g. struct page *).
>
> Coherent<T> (or CoherentAllocation<T> respectively) stores this value as
> NonNull<T> and exposes methods that dereference it and even modify its
> contents.
>
> Remove the flag from the public attrs module such that drivers cannot
> pass it to Coherent<T> (or CoherentAllocation<T> respectively) in the
> first place.
>
> Instead DMA_ATTR_NO_KERNEL_MAPPING can be supported with an additional
> opaque type (e.g. CoherentHandle) which does not provide access to the
> allocated memory.
>
> Cc: stable@vger.kernel.org
> Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs
  2026-03-21 17:27 [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-03-22 11:07 ` Alice Ryhl
@ 2026-03-22 14:52 ` Alexandre Courbot
  2026-03-28 14:51 ` Alexandre Courbot
  4 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2026-03-22 14:52 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, gary, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel, stable

On Sun Mar 22, 2026 at 2:27 AM JST, Danilo Krummrich wrote:
> When DMA_ATTR_NO_KERNEL_MAPPING is passed to dma_alloc_attrs(), the
> returned CPU address is not a pointer to the allocated memory but an
> opaque handle (e.g. struct page *).
>
> Coherent<T> (or CoherentAllocation<T> respectively) stores this value as
> NonNull<T> and exposes methods that dereference it and even modify its
> contents.
>
> Remove the flag from the public attrs module such that drivers cannot
> pass it to Coherent<T> (or CoherentAllocation<T> respectively) in the
> first place.
>
> Instead DMA_ATTR_NO_KERNEL_MAPPING can be supported with an additional
> opaque type (e.g. CoherentHandle) which does not provide access to the
> allocated memory.
>
> Cc: stable@vger.kernel.org
> Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-21 17:27 ` [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping Danilo Krummrich
@ 2026-03-22 14:52   ` Alexandre Courbot
  2026-03-22 15:21     ` Danilo Krummrich
  2026-03-24 20:10   ` Gary Guo
  1 sibling, 1 reply; 13+ messages in thread
From: Alexandre Courbot @ 2026-03-22 14:52 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, gary, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel

On Sun Mar 22, 2026 at 2:27 AM JST, Danilo Krummrich wrote:
> Add CoherentHandle, an opaque DMA allocation type for buffers that are
> only ever accessed by hardware. Unlike Coherent<T>, it does not provide
> CPU access to the allocated memory.
>
> CoherentHandle implicitly sets DMA_ATTR_NO_KERNEL_MAPPING and stores the
> value returned by dma_alloc_attrs() as an opaque handle
> (NonNull<c_void>) rather than a typed pointer, since with this flag the
> C API returns an opaque cookie (e.g. struct page *), not a CPU pointer
> to the allocated memory.
>
> Only the DMA bus address is exposed to drivers; the opaque handle is
> used solely to free the allocation on drop.
>
> This commit is for reference only; there is currently no in-tree user.

nova-core's sysmem flush memory page would be a prime candidate to use
this, I'll add this patch as a dependency to [1] and use it.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

(one question below)

[1] https://lore.kernel.org/all/20260321-b4-nova-dma-removal-v1-0-5cf18a75ff64@nvidia.com/

>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/dma.rs | 119 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 9e0c9ff91cba..fa30793c798d 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -1011,6 +1011,125 @@ fn drop(&mut self) {
>  // can be sent to another thread.
>  unsafe impl<T: KnownSize + Send + ?Sized> Send for Coherent<T> {}
>  
> +/// An opaque DMA allocation without a kernel virtual mapping.
> +///
> +/// Unlike [`Coherent`], a `CoherentHandle` does not provide CPU access to the allocated memory.
> +/// The allocation is always performed with `DMA_ATTR_NO_KERNEL_MAPPING`, meaning no kernel
> +/// virtual mapping is created for the buffer. The value returned by the C API as the CPU
> +/// address is an opaque handle used only to free the allocation.
> +///
> +/// This is useful for buffers that are only ever accessed by hardware.
> +///
> +/// # Invariants
> +///
> +/// - `cpu_handle` holds the opaque handle returned by `dma_alloc_attrs` with
> +///   `DMA_ATTR_NO_KERNEL_MAPPING` set, and is only valid for passing back to `dma_free_attrs`.
> +/// - `dma_handle` is the corresponding bus address for device DMA.
> +/// - `size` is the allocation size in bytes as passed to `dma_alloc_attrs`.
> +/// - `dma_attrs` contains the attributes used for the allocation, always including
> +///   `DMA_ATTR_NO_KERNEL_MAPPING`.

Quick question for my erudition: I understand all the invariants are
referred to by `drop`, but some of them (`size` notably) really read
more like doccomments. Do we need to be that exhaustive every time we
call a C API?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-22 14:52   ` Alexandre Courbot
@ 2026-03-22 15:21     ` Danilo Krummrich
  0 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-03-22 15:21 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, gary, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel

On Sun Mar 22, 2026 at 3:52 PM CET, Alexandre Courbot wrote:
> On Sun Mar 22, 2026 at 2:27 AM JST, Danilo Krummrich wrote:
>> Add CoherentHandle, an opaque DMA allocation type for buffers that are
>> only ever accessed by hardware. Unlike Coherent<T>, it does not provide
>> CPU access to the allocated memory.
>>
>> CoherentHandle implicitly sets DMA_ATTR_NO_KERNEL_MAPPING and stores the
>> value returned by dma_alloc_attrs() as an opaque handle
>> (NonNull<c_void>) rather than a typed pointer, since with this flag the
>> C API returns an opaque cookie (e.g. struct page *), not a CPU pointer
>> to the allocated memory.
>>
>> Only the DMA bus address is exposed to drivers; the opaque handle is
>> used solely to free the allocation on drop.
>>
>> This commit is for reference only; there is currently no in-tree user.
>
> nova-core's sysmem flush memory page would be a prime candidate to use
> this, I'll add this patch as a dependency to [1] and use it.

Sure, please do.

> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>
> (one question below)
>
> [1] https://lore.kernel.org/all/20260321-b4-nova-dma-removal-v1-0-5cf18a75ff64@nvidia.com/
>
>>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>>  rust/kernel/dma.rs | 119 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 119 insertions(+)
>>
>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>> index 9e0c9ff91cba..fa30793c798d 100644
>> --- a/rust/kernel/dma.rs
>> +++ b/rust/kernel/dma.rs
>> @@ -1011,6 +1011,125 @@ fn drop(&mut self) {
>>  // can be sent to another thread.
>>  unsafe impl<T: KnownSize + Send + ?Sized> Send for Coherent<T> {}
>>  
>> +/// An opaque DMA allocation without a kernel virtual mapping.
>> +///
>> +/// Unlike [`Coherent`], a `CoherentHandle` does not provide CPU access to the allocated memory.
>> +/// The allocation is always performed with `DMA_ATTR_NO_KERNEL_MAPPING`, meaning no kernel
>> +/// virtual mapping is created for the buffer. The value returned by the C API as the CPU
>> +/// address is an opaque handle used only to free the allocation.
>> +///
>> +/// This is useful for buffers that are only ever accessed by hardware.
>> +///
>> +/// # Invariants
>> +///
>> +/// - `cpu_handle` holds the opaque handle returned by `dma_alloc_attrs` with
>> +///   `DMA_ATTR_NO_KERNEL_MAPPING` set, and is only valid for passing back to `dma_free_attrs`.
>> +/// - `dma_handle` is the corresponding bus address for device DMA.
>> +/// - `size` is the allocation size in bytes as passed to `dma_alloc_attrs`.
>> +/// - `dma_attrs` contains the attributes used for the allocation, always including
>> +///   `DMA_ATTR_NO_KERNEL_MAPPING`.
>
> Quick question for my erudition: I understand all the invariants are
> referred to by `drop`, but some of them (`size` notably) really read
> more like doccomments. Do we need to be that exhaustive every time we
> call a C API?

We have the same on dma::Coherent / dma::CoherentAllocation, as the destructor
relies on all those to be invariant.

I don't think we have to do this for every C API call, but in this case the
question is what the safety justification in the destructor would look like if
we'd drop those invariants.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-21 17:27 ` [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping Danilo Krummrich
  2026-03-22 14:52   ` Alexandre Courbot
@ 2026-03-24 20:10   ` Gary Guo
  2026-03-24 22:20     ` Danilo Krummrich
  1 sibling, 1 reply; 13+ messages in thread
From: Gary Guo @ 2026-03-24 20:10 UTC (permalink / raw)
  To: Danilo Krummrich, abdiel.janulgue, daniel.almeida, robin.murphy,
	a.hindborg, ojeda, boqun, gary, bjorn3_gh, lossin, aliceryhl,
	tmgross
  Cc: driver-core, rust-for-linux, linux-kernel

On Sat Mar 21, 2026 at 5:27 PM GMT, Danilo Krummrich wrote:
> Add CoherentHandle, an opaque DMA allocation type for buffers that are
> only ever accessed by hardware. Unlike Coherent<T>, it does not provide
> CPU access to the allocated memory.
>
> CoherentHandle implicitly sets DMA_ATTR_NO_KERNEL_MAPPING and stores the
> value returned by dma_alloc_attrs() as an opaque handle
> (NonNull<c_void>) rather than a typed pointer, since with this flag the
> C API returns an opaque cookie (e.g. struct page *), not a CPU pointer
> to the allocated memory.
>
> Only the DMA bus address is exposed to drivers; the opaque handle is
> used solely to free the allocation on drop.
>
> This commit is for reference only; there is currently no in-tree user.

Thinking about this a bit more, instead of creating new separate types, we can
probably just have multiple flavour/kind of `Coherent`, and we default to the
most common one.

Something like

pub struct Normal;
pub struct NoMapping;

struct Coherent<T: KnonwSize + ?Sized, Kind = Normal> {
    ...
}

Where for the common functions, they're implemented on `Coherent<T, Kind>` while
the ones that require CPU mapping have `impl Coherent<T>`.

The benefit is that there's no code duplication. Also, you could also implement
`Io` but leave out all `IoCapable` impls, so you may do I/O projections on
these and be able to get offseted DMA addresses without having to do manual
computation.

Best,
Gary

>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/dma.rs | 119 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 9e0c9ff91cba..fa30793c798d 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -1011,6 +1011,125 @@ fn drop(&mut self) {
>  // can be sent to another thread.
>  unsafe impl<T: KnownSize + Send + ?Sized> Send for Coherent<T> {}
>  
> +/// An opaque DMA allocation without a kernel virtual mapping.
> +///
> +/// Unlike [`Coherent`], a `CoherentHandle` does not provide CPU access to the allocated memory.
> +/// The allocation is always performed with `DMA_ATTR_NO_KERNEL_MAPPING`, meaning no kernel
> +/// virtual mapping is created for the buffer. The value returned by the C API as the CPU
> +/// address is an opaque handle used only to free the allocation.
> +///
> +/// This is useful for buffers that are only ever accessed by hardware.
> +///
> +/// # Invariants
> +///
> +/// - `cpu_handle` holds the opaque handle returned by `dma_alloc_attrs` with
> +///   `DMA_ATTR_NO_KERNEL_MAPPING` set, and is only valid for passing back to `dma_free_attrs`.
> +/// - `dma_handle` is the corresponding bus address for device DMA.
> +/// - `size` is the allocation size in bytes as passed to `dma_alloc_attrs`.
> +/// - `dma_attrs` contains the attributes used for the allocation, always including
> +///   `DMA_ATTR_NO_KERNEL_MAPPING`.
> +pub struct CoherentHandle {
> +    dev: ARef<device::Device>,
> +    dma_handle: DmaAddress,
> +    cpu_handle: NonNull<c_void>,
> +    size: usize,
> +    dma_attrs: Attrs,
> +}
> +


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-24 20:10   ` Gary Guo
@ 2026-03-24 22:20     ` Danilo Krummrich
  2026-03-25 13:09       ` Gary Guo
  0 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-03-24 22:20 UTC (permalink / raw)
  To: Gary Guo
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel

On Tue Mar 24, 2026 at 9:10 PM CET, Gary Guo wrote:
> On Sat Mar 21, 2026 at 5:27 PM GMT, Danilo Krummrich wrote:
>> Add CoherentHandle, an opaque DMA allocation type for buffers that are
>> only ever accessed by hardware. Unlike Coherent<T>, it does not provide
>> CPU access to the allocated memory.
>>
>> CoherentHandle implicitly sets DMA_ATTR_NO_KERNEL_MAPPING and stores the
>> value returned by dma_alloc_attrs() as an opaque handle
>> (NonNull<c_void>) rather than a typed pointer, since with this flag the
>> C API returns an opaque cookie (e.g. struct page *), not a CPU pointer
>> to the allocated memory.
>>
>> Only the DMA bus address is exposed to drivers; the opaque handle is
>> used solely to free the allocation on drop.
>>
>> This commit is for reference only; there is currently no in-tree user.
>
> Thinking about this a bit more, instead of creating new separate types, we can
> probably just have multiple flavour/kind of `Coherent`, and we default to the
> most common one.
>
> Something like
>
> pub struct Normal;
> pub struct NoMapping;
>
> struct Coherent<T: KnonwSize + ?Sized, Kind = Normal> {
>     ...
> }
>
> Where for the common functions, they're implemented on `Coherent<T, Kind>` while
> the ones that require CPU mapping have `impl Coherent<T>`.
>
> The benefit is that there's no code duplication.

This was my first thought as well.

But I found it a bit odd that users would still have to define T, then I thought
we could have a type alias, e.g.

	type CoherentHandle = Coherent<[u8], NoMapping>;

In this case the constructor would be CoherentHandle::zeroed_slice(), which I
find a bit odd as well.

Alternatively, we could have a const SIZE generic on CoherentHandle wrapping
Coherent<[u8; SIZE], NoMapping>.

Then we'd get CoherentHandle::<SIZE>::zeroed() (which I find much better), but
we would still have Coherent<T, NoMapping> publically available, which I still
find a bit odd.

And then I thought, it's probably not worth the additional complexity and a
separate CoherentHandle type is probably good enough.

> Also, you could also implement `Io` but leave out all `IoCapable` impls, so
> you may do I/O projections on these and be able to get offseted DMA addresses
> without having to do manual computation.

This is a reason for having Coherent<T, NoMapping>, but I'm not sure it is an
improvement in the first place.

If the memory is not mapped on the CPU side it is probably just an opaque
buffer. And I'm not sure there is a huge advantage in defining a structure
representing any relevant offsets and then do an I/O projection compared to just
having constants or an enum defining those offsets.

I also think there's not a huge difference in terms of robustness. In the former
case, if you mess up the offsets, the buffer size will be wrong (unless multiple
offsets are wrong and compensate each other) and the device will not properly
work on runtime.

In the latter case, you either also pass a wrong offset to the device, or you
get a compile-time error if the offset is out of bounds (which is probably even
better).

Also, consider the case where the offset into the opaque buffer is dynamic, then
a type and I/O projections won't help either.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-24 22:20     ` Danilo Krummrich
@ 2026-03-25 13:09       ` Gary Guo
  2026-03-25 17:18         ` Danilo Krummrich
  0 siblings, 1 reply; 13+ messages in thread
From: Gary Guo @ 2026-03-25 13:09 UTC (permalink / raw)
  To: Danilo Krummrich, Gary Guo
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel

On Tue Mar 24, 2026 at 10:20 PM GMT, Danilo Krummrich wrote:
> On Tue Mar 24, 2026 at 9:10 PM CET, Gary Guo wrote:
>> On Sat Mar 21, 2026 at 5:27 PM GMT, Danilo Krummrich wrote:
>>> Add CoherentHandle, an opaque DMA allocation type for buffers that are
>>> only ever accessed by hardware. Unlike Coherent<T>, it does not provide
>>> CPU access to the allocated memory.
>>>
>>> CoherentHandle implicitly sets DMA_ATTR_NO_KERNEL_MAPPING and stores the
>>> value returned by dma_alloc_attrs() as an opaque handle
>>> (NonNull<c_void>) rather than a typed pointer, since with this flag the
>>> C API returns an opaque cookie (e.g. struct page *), not a CPU pointer
>>> to the allocated memory.
>>>
>>> Only the DMA bus address is exposed to drivers; the opaque handle is
>>> used solely to free the allocation on drop.
>>>
>>> This commit is for reference only; there is currently no in-tree user.
>>
>> Thinking about this a bit more, instead of creating new separate types, we can
>> probably just have multiple flavour/kind of `Coherent`, and we default to the
>> most common one.
>>
>> Something like
>>
>> pub struct Normal;
>> pub struct NoMapping;
>>
>> struct Coherent<T: KnonwSize + ?Sized, Kind = Normal> {
>>     ...
>> }
>>
>> Where for the common functions, they're implemented on `Coherent<T, Kind>` while
>> the ones that require CPU mapping have `impl Coherent<T>`.
>>
>> The benefit is that there's no code duplication.
>
> This was my first thought as well.
>
> But I found it a bit odd that users would still have to define T, then I thought
> we could have a type alias, e.g.
>
> 	type CoherentHandle = Coherent<[u8], NoMapping>;
>
> In this case the constructor would be CoherentHandle::zeroed_slice(), which I
> find a bit odd as well.
>
> Alternatively, we could have a const SIZE generic on CoherentHandle wrapping
> Coherent<[u8; SIZE], NoMapping>.
>
> Then we'd get CoherentHandle::<SIZE>::zeroed() (which I find much better), but
> we would still have Coherent<T, NoMapping> publically available, which I still
> find a bit odd.

This only gives you fixed size, though. If you want the same type that supports
both a fixed size and dynamic size, then a generic that is either array/slice is
the way to go.

>
> And then I thought, it's probably not worth the additional complexity and a
> separate CoherentHandle type is probably good enough.
>
>> Also, you could also implement `Io` but leave out all `IoCapable` impls, so
>> you may do I/O projections on these and be able to get offseted DMA addresses
>> without having to do manual computation.
>
> This is a reason for having Coherent<T, NoMapping>, but I'm not sure it is an
> improvement in the first place.
>
> If the memory is not mapped on the CPU side it is probably just an opaque
> buffer. And I'm not sure there is a huge advantage in defining a structure
> representing any relevant offsets and then do an I/O projection compared to just
> having constants or an enum defining those offsets.
>
> I also think there's not a huge difference in terms of robustness. In the former
> case, if you mess up the offsets, the buffer size will be wrong (unless multiple
> offsets are wrong and compensate each other) and the device will not properly
> work on runtime.
>
> In the latter case, you either also pass a wrong offset to the device, or you
> get a compile-time error if the offset is out of bounds (which is probably even
> better).
>
> Also, consider the case where the offset into the opaque buffer is dynamic, then
> a type and I/O projections won't help either.

You still can use `io_project!(handle, [start..end]?)` to do the bounds
checking. But as you said, the benefit might be minimal.

Best,
Gary

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-25 13:09       ` Gary Guo
@ 2026-03-25 17:18         ` Danilo Krummrich
  2026-03-26 13:37           ` Gary Guo
  0 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-03-25 17:18 UTC (permalink / raw)
  To: Gary Guo
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel

On Wed Mar 25, 2026 at 2:09 PM CET, Gary Guo wrote:
> This only gives you fixed size, though. If you want the same type that supports
> both a fixed size and dynamic size, then a generic that is either array/slice is
> the way to go.

[...]

> You still can use `io_project!(handle, [start..end]?)` to do the bounds
> checking. But as you said, the benefit might be minimal.

I think we shouldn't overengineer this for now, and wait for actual use-cases
that require any of that.

The only thing we need right now in practice is an API that takes the size and
returns a handle to the buffer of this size exposing the DMA address that is
subsequently passed to the device without further modification.

So, if we want to re-use dma::Coherent<T>, we can just do this

	pub struct CoherentHandle(Coherent<[u8]>);
	
	impl CoherentHandle {
	    pub fn alloc_with_attrs(
	        dev: &device::Device<Bound>,
	        size: usize,
	        gfp_flags: kernel::alloc::Flags,
	        dma_attrs: Attrs,
	    ) -> Result<Self> {
	        let dma_attrs = dma_attrs | Attrs(bindings::DMA_ATTR_NO_KERNEL_MAPPING);
	        Coherent::<u8>::alloc_slice_with_attrs(dev, size, gfp_flags, dma_attrs).map(Self)
	    }
	
	    #[inline]
	    pub fn alloc(
	        dev: &device::Device<Bound>,
	        size: usize,
	        gfp_flags: kernel::alloc::Flags,
	    ) -> Result<Self> {
	        Self::alloc_with_attrs(dev, size, gfp_flags, Attrs(0))
	    }
	
	    #[inline]
	    pub fn dma_handle(&self) -> DmaAddress {
	        self.0.dma_handle()
	    }
	
	    #[inline]
	    pub fn size(&self) -> usize {
	        self.0.size()
	    }
	}

and add a guarantee that Coherent::alloc_with_attrs() and Coherent::drop() never
touch the cpu_ptr.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping
  2026-03-25 17:18         ` Danilo Krummrich
@ 2026-03-26 13:37           ` Gary Guo
  0 siblings, 0 replies; 13+ messages in thread
From: Gary Guo @ 2026-03-26 13:37 UTC (permalink / raw)
  To: Danilo Krummrich, Gary Guo
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel

On Wed Mar 25, 2026 at 5:18 PM GMT, Danilo Krummrich wrote:
> On Wed Mar 25, 2026 at 2:09 PM CET, Gary Guo wrote:
>> This only gives you fixed size, though. If you want the same type that supports
>> both a fixed size and dynamic size, then a generic that is either array/slice is
>> the way to go.
>
> [...]
>
>> You still can use `io_project!(handle, [start..end]?)` to do the bounds
>> checking. But as you said, the benefit might be minimal.
>
> I think we shouldn't overengineer this for now, and wait for actual use-cases
> that require any of that.
>
> The only thing we need right now in practice is an API that takes the size and
> returns a handle to the buffer of this size exposing the DMA address that is
> subsequently passed to the device without further modification.
>
> So, if we want to re-use dma::Coherent<T>, we can just do this
>
> 	pub struct CoherentHandle(Coherent<[u8]>);
> 	
> 	impl CoherentHandle {
> 	    pub fn alloc_with_attrs(
> 	        dev: &device::Device<Bound>,
> 	        size: usize,
> 	        gfp_flags: kernel::alloc::Flags,
> 	        dma_attrs: Attrs,
> 	    ) -> Result<Self> {
> 	        let dma_attrs = dma_attrs | Attrs(bindings::DMA_ATTR_NO_KERNEL_MAPPING);
> 	        Coherent::<u8>::alloc_slice_with_attrs(dev, size, gfp_flags, dma_attrs).map(Self)
> 	    }
> 	
> 	    #[inline]
> 	    pub fn alloc(
> 	        dev: &device::Device<Bound>,
> 	        size: usize,
> 	        gfp_flags: kernel::alloc::Flags,
> 	    ) -> Result<Self> {
> 	        Self::alloc_with_attrs(dev, size, gfp_flags, Attrs(0))
> 	    }
> 	
> 	    #[inline]
> 	    pub fn dma_handle(&self) -> DmaAddress {
> 	        self.0.dma_handle()
> 	    }
> 	
> 	    #[inline]
> 	    pub fn size(&self) -> usize {
> 	        self.0.size()
> 	    }
> 	}
>
> and add a guarantee that Coherent::alloc_with_attrs() and Coherent::drop() never
> touch the cpu_ptr.

Hmm, it sounds like we *could* actually have `Coherent` support `Drop` types and
have `drop()` run the destructor for it. We don't need to for `FromBytes +
AsBytes`, but I think we'd better not have that as a guarantee.

So the original approach that you have is better.

Best,
Gary

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs
  2026-03-21 17:27 [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Danilo Krummrich
                   ` (3 preceding siblings ...)
  2026-03-22 14:52 ` Alexandre Courbot
@ 2026-03-28 14:51 ` Alexandre Courbot
  4 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2026-03-28 14:51 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: abdiel.janulgue, daniel.almeida, robin.murphy, a.hindborg, ojeda,
	boqun, gary, bjorn3_gh, lossin, aliceryhl, tmgross, driver-core,
	rust-for-linux, linux-kernel, stable

On Sun Mar 22, 2026 at 2:27 AM JST, Danilo Krummrich wrote:
> When DMA_ATTR_NO_KERNEL_MAPPING is passed to dma_alloc_attrs(), the
> returned CPU address is not a pointer to the allocated memory but an
> opaque handle (e.g. struct page *).
>
> Coherent<T> (or CoherentAllocation<T> respectively) stores this value as
> NonNull<T> and exposes methods that dereference it and even modify its
> contents.
>
> Remove the flag from the public attrs module such that drivers cannot
> pass it to Coherent<T> (or CoherentAllocation<T> respectively) in the
> first place.
>
> Instead DMA_ATTR_NO_KERNEL_MAPPING can be supported with an additional
> opaque type (e.g. CoherentHandle) which does not provide access to the
> allocated memory.
>
> Cc: stable@vger.kernel.org
> Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

As agreed with Danilo [1], pushed the series to drm-rust-next - thanks!

[1] https://lore.kernel.org/all/f7e39810-bcf2-419e-af2a-2ae0c4d5ab67@kernel.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-03-28 14:51 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21 17:27 [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Danilo Krummrich
2026-03-21 17:27 ` [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping Danilo Krummrich
2026-03-22 14:52   ` Alexandre Courbot
2026-03-22 15:21     ` Danilo Krummrich
2026-03-24 20:10   ` Gary Guo
2026-03-24 22:20     ` Danilo Krummrich
2026-03-25 13:09       ` Gary Guo
2026-03-25 17:18         ` Danilo Krummrich
2026-03-26 13:37           ` Gary Guo
2026-03-21 18:42 ` [PATCH 1/2] rust: dma: remove DMA_ATTR_NO_KERNEL_MAPPING from public attrs Gary Guo
2026-03-22 11:07 ` Alice Ryhl
2026-03-22 14:52 ` Alexandre Courbot
2026-03-28 14:51 ` Alexandre Courbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox