dma::Coherent & dma::CoherentBox API

[PATCH v2 5/8] rust: dma: add Coherent:init() and Coherent::init_with_attrs()

Posted by Danilo Krummrich 2 weeks ago

Analogous to Coherent::zeroed() and Coherent::zeroed_with_attrs(), add
Coherent:init() and Coherent::init_with_attrs() which both take an impl
Init<T, E> argument initializing the DMA coherent memory.

Compared to CoherentInit, Coherent::init() is a one-shot constructor
that runs an Init closure and immediately exposes the DMA handle,
whereas CoherentInit is a multi-stage initializer that provides safe
&mut T access by withholding the DMA address until converted to
Coherent.

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

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index cefb54f0424a..6d2bec52806b 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -709,6 +709,44 @@ pub fn zeroed(dev: &device::Device<Bound>, gfp_flags: kernel::alloc::Flags) -> R
         Self::zeroed_with_attrs(dev, gfp_flags, Attrs(0))
     }
 
+    /// Same as [`Coherent::zeroed_with_attrs`], but instead of a zero-initialization the memory is
+    /// initialized with `init`.
+    pub fn init_with_attrs<E>(
+        dev: &device::Device<Bound>,
+        gfp_flags: kernel::alloc::Flags,
+        dma_attrs: Attrs,
+        init: impl Init<T, E>,
+    ) -> Result<Self>
+    where
+        Error: From<E>,
+    {
+        let dmem = Self::alloc_with_attrs(dev, gfp_flags, dma_attrs)?;
+        let ptr = dmem.as_mut_ptr();
+
+        // SAFETY:
+        // - `ptr` is valid, properly aligned, and points to exclusively owned memory.
+        // - If `__init` fails, `self` is dropped, which safely frees the underlying `Coherent`'s
+        //   DMA memory. `T: AsBytes + FromBytes` ensures there are no complex `Drop` requirements
+        //   we are bypassing.
+        unsafe { init.__init(ptr)? };
+
+        Ok(dmem)
+    }
+
+    /// Same as [`Coherent::zeroed`], but instead of a zero-initialization the memory is initialized
+    /// with `init`.
+    #[inline]
+    pub fn init<E>(
+        dev: &device::Device<Bound>,
+        gfp_flags: kernel::alloc::Flags,
+        init: impl Init<T, E>,
+    ) -> Result<Self>
+    where
+        Error: From<E>,
+    {
+        Self::init_with_attrs(dev, gfp_flags, Attrs(0), init)
+    }
+
     /// Allocates a region of `[T; len]` of coherent memory.
     fn alloc_slice_with_attrs(
         dev: &device::Device<Bound>,
-- 
2.53.0

Re: [PATCH v2 5/8] rust: dma: add Coherent:init() and Coherent::init_with_attrs()

Posted by Andreas Hindborg 1 week, 3 days ago

"Danilo Krummrich" <dakr@kernel.org> writes:

> Analogous to Coherent::zeroed() and Coherent::zeroed_with_attrs(), add
> Coherent:init() and Coherent::init_with_attrs() which both take an impl
> Init<T, E> argument initializing the DMA coherent memory.
>
> Compared to CoherentInit, Coherent::init() is a one-shot constructor
> that runs an Init closure and immediately exposes the DMA handle,
> whereas CoherentInit is a multi-stage initializer that provides safe
> &mut T access by withholding the DMA address until converted to
> Coherent.

You forgot to update this to CoherentBox

>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/dma.rs | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index cefb54f0424a..6d2bec52806b 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -709,6 +709,44 @@ pub fn zeroed(dev: &device::Device<Bound>, gfp_flags: kernel::alloc::Flags) -> R
>          Self::zeroed_with_attrs(dev, gfp_flags, Attrs(0))
>      }
>
> +    /// Same as [`Coherent::zeroed_with_attrs`], but instead of a zero-initialization the memory is
> +    /// initialized with `init`.
> +    pub fn init_with_attrs<E>(
> +        dev: &device::Device<Bound>,
> +        gfp_flags: kernel::alloc::Flags,
> +        dma_attrs: Attrs,
> +        init: impl Init<T, E>,
> +    ) -> Result<Self>
> +    where
> +        Error: From<E>,
> +    {
> +        let dmem = Self::alloc_with_attrs(dev, gfp_flags, dma_attrs)?;
> +        let ptr = dmem.as_mut_ptr();
> +
> +        // SAFETY:
> +        // - `ptr` is valid, properly aligned, and points to exclusively owned memory.
> +        // - If `__init` fails, `self` is dropped, which safely frees the underlying `Coherent`'s
> +        //   DMA memory. `T: AsBytes + FromBytes` ensures there are no complex `Drop` requirements
> +        //   we are bypassing.
> +        unsafe { init.__init(ptr)? };
> +
> +        Ok(dmem)
> +    }
> +
> +    /// Same as [`Coherent::zeroed`], but instead of a zero-initialization the memory is initialized
> +    /// with `init`.
> +    #[inline]
> +    pub fn init<E>(
> +        dev: &device::Device<Bound>,
> +        gfp_flags: kernel::alloc::Flags,
> +        init: impl Init<T, E>,
> +    ) -> Result<Self>
> +    where
> +        Error: From<E>,
> +    {
> +        Self::init_with_attrs(dev, gfp_flags, Attrs(0), init)
> +    }
> +

I think we are missing an array initializer for `Coherent<[T]>`.

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

Re: [PATCH v2 5/8] rust: dma: add Coherent:init() and Coherent::init_with_attrs()

Posted by Danilo Krummrich 1 week, 3 days ago

On Tue Mar 24, 2026 at 3:00 PM CET, Andreas Hindborg wrote:
>> +    /// Same as [`Coherent::zeroed`], but instead of a zero-initialization the memory is initialized
>> +    /// with `init`.
>> +    #[inline]
>> +    pub fn init<E>(
>> +        dev: &device::Device<Bound>,
>> +        gfp_flags: kernel::alloc::Flags,
>> +        init: impl Init<T, E>,
>> +    ) -> Result<Self>
>> +    where
>> +        Error: From<E>,
>> +    {
>> +        Self::init_with_attrs(dev, gfp_flags, Attrs(0), init)
>> +    }
>> +
>
> I think we are missing an array initializer for `Coherent<[T]>`.

This method is already compatible with arrays, T can be an array type itself,
e.g. T = [MyStruct; 5].

For instance, the diff in [1] should work.

> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>

Thanks! The patch series has been applied yesterday, and the branch it has been
applied to is immutable, so I can't add additional tags.

However, the Link: included in the patch still points to this conversation.

Also note that subsequent review is still valued; we can always send follow-up
patches if required.

Thanks,
Danilo

[1]

diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 129bb4b39c04..3d3ffc21ea77 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -22,6 +22,7 @@
 struct DmaSampleDriver {
     pdev: ARef<pci::Device>,
     ca: Coherent<[MyStruct]>,
+    ca_init: Coherent<[MyStruct; 5]>,
     #[pin]
     sgt: SGTable<Owned<VVec<u8>>>,
 }
@@ -76,6 +77,15 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
                 kernel::dma_write!(ca, [i]?, MyStruct::new(value.0, value.1));
             }

+            let ca_init: Coherent<[MyStruct; 5]> = Coherent::init(
+                pdev.as_ref(),
+                GFP_KERNEL,
+                pin_init::init_array_from_fn(|i| {
+                    let (h, b) = TEST_VALUES[i];
+                    MyStruct::new(h, b)
+                }),
+            )?;
+
             let size = 4 * page::PAGE_SIZE;
             let pages = VVec::with_capacity(size, GFP_KERNEL)?;

@@ -84,6 +94,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
             Ok(try_pin_init!(Self {
                 pdev: pdev.into(),
                 ca,
+                ca_init,
                 sgt <- sgt,
             }))
         })
@@ -98,6 +109,12 @@ fn check_dma(&self) -> Result {

             assert_eq!(val0, value.0);
             assert_eq!(val1, value.1);
+
+            let val0 = kernel::dma_read!(self.ca_init, [i]?.h);
+            let val1 = kernel::dma_read!(self.ca_init, [i]?.b);
+
+            assert_eq!(val0, value.0);
+            assert_eq!(val1, value.1);
         }

         Ok(())

Re: [PATCH v2 5/8] rust: dma: add Coherent:init() and Coherent::init_with_attrs()

Posted by Andreas Hindborg 1 week, 3 days ago

"Danilo Krummrich" <dakr@kernel.org> writes:

> On Tue Mar 24, 2026 at 3:00 PM CET, Andreas Hindborg wrote:
>>> +    /// Same as [`Coherent::zeroed`], but instead of a zero-initialization the memory is initialized
>>> +    /// with `init`.
>>> +    #[inline]
>>> +    pub fn init<E>(
>>> +        dev: &device::Device<Bound>,
>>> +        gfp_flags: kernel::alloc::Flags,
>>> +        init: impl Init<T, E>,
>>> +    ) -> Result<Self>
>>> +    where
>>> +        Error: From<E>,
>>> +    {
>>> +        Self::init_with_attrs(dev, gfp_flags, Attrs(0), init)
>>> +    }
>>> +
>>
>> I think we are missing an array initializer for `Coherent<[T]>`.
>
> This method is already compatible with arrays, T can be an array type itself,
> e.g. T = [MyStruct; 5].
>
> For instance, the diff in [1] should work.

Cool!

>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Thanks! The patch series has been applied yesterday, and the branch it has been
> applied to is immutable, so I can't add additional tags.

You guys move fast!

> However, the Link: included in the patch still points to this conversation.
>
> Also note that subsequent review is still valued; we can always send follow-up
> patches if required.

At any rate I can send the changes I suggested if they are deemed valid.


Best regards,
Andreas Hindborg

Re: [PATCH v2 5/8] rust: dma: add Coherent:init() and Coherent::init_with_attrs()

Posted by Gary Guo 2 weeks ago

On Fri Mar 20, 2026 at 7:45 PM GMT, Danilo Krummrich wrote:
> Analogous to Coherent::zeroed() and Coherent::zeroed_with_attrs(), add
> Coherent:init() and Coherent::init_with_attrs() which both take an impl
> Init<T, E> argument initializing the DMA coherent memory.
> 
> Compared to CoherentInit, Coherent::init() is a one-shot constructor
> that runs an Init closure and immediately exposes the DMA handle,
> whereas CoherentInit is a multi-stage initializer that provides safe
> &mut T access by withholding the DMA address until converted to
> Coherent.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

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

> ---
>  rust/kernel/dma.rs | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)

[PATCH v2 1/8] rust: dma: use "kernel vertical" style for imports
[PATCH v2 2/8] rust: dma: add generalized container for types other than slices
[PATCH v2 3/8] rust: dma: add zeroed constructor to `Coherent`
[PATCH v2 4/8] rust: dma: introduce dma::CoherentBox for memory initialization
[PATCH v2 5/8] rust: dma: add Coherent:init() and Coherent::init_with_attrs()
[PATCH v2 6/8] gpu: nova-core: use Coherent::init to initialize GspFwWprMeta
[PATCH v2 7/8] gpu: nova-core: convert Gsp::new() to use CoherentBox
[PATCH v2 8/8] gpu: nova-core: convert to new dma::Coherent API