A very common pattern is to create a block of coherent memory with the
content of an already-existing slice of bytes (e.g. a loaded firmware
blob).

`CoherentBox` makes this easier, but still implies a potentially
panicking operation with `copy_from_slice` that requires a `PANIC`
comment.

Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
and `CoherentBox` to turn this into a trivial one-step operation.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 rust/kernel/dma.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 6d2bec52806b..a5cc993c919e 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -453,6 +453,62 @@ pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result
 
         Ok(())
     }
+
+    /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
+    /// copy of its contents.
+    ///
+    /// This is the [`CoherentBox`] variant of [`Coherent::from_slice_with_attrs`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use core::ops::Deref;
+    ///
+    /// # use kernel::device::{Bound, Device};
+    /// use kernel::dma::{
+    ///     attrs::*,
+    ///     CoherentBox
+    /// };
+    ///
+    /// # fn test(dev: &Device<Bound>) -> Result {
+    /// let data = [0u8, 1u8, 2u8, 3u8];
+    /// let c: CoherentBox<[u8]> =
+    ///     CoherentBox::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
+    ///
+    /// assert_eq!(c.deref(), &data);
+    /// # Ok::<(), Error>(()) }
+    /// ```
+    pub fn from_slice_with_attrs(
+        dev: &device::Device<Bound>,
+        data: &[T],
+        gfp_flags: kernel::alloc::Flags,
+        dma_attrs: Attrs,
+    ) -> Result<Self>
+    where
+        T: Copy,
+    {
+        Coherent::<T>::alloc_slice_with_attrs(dev, data.len(), gfp_flags, dma_attrs)
+            .map(Self)
+            .map(|mut slice| {
+                // PANIC: `slice` was created with length `data.len()`.
+                slice.copy_from_slice(data);
+                slice
+            })
+    }
+
+    /// Performs the same functionality as [`CoherentBox::from_slice_with_attrs`], except the
+    /// `dma_attrs` is 0 by default.
+    #[inline]
+    pub fn from_slice(
+        dev: &device::Device<Bound>,
+        data: &[T],
+        gfp_flags: kernel::alloc::Flags,
+    ) -> Result<Self>
+    where
+        T: Copy,
+    {
+        Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
+    }
 }
 
 impl<T: AsBytes + FromBytes> CoherentBox<T> {
@@ -827,6 +883,52 @@ pub fn zeroed_slice(
     ) -> Result<Coherent<[T]>> {
         Self::zeroed_slice_with_attrs(dev, len, gfp_flags, Attrs(0))
     }
+
+    /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
+    /// copy of its contents.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::device::{Bound, Device};
+    /// use kernel::dma::{
+    ///     attrs::*,
+    ///     Coherent
+    /// };
+    ///
+    /// # fn test(dev: &Device<Bound>) -> Result {
+    /// let data = [0u8, 1u8, 2u8, 3u8];
+    /// // `c` has the same content as `data`.
+    /// let c: Coherent<[u8]> =
+    ///     Coherent::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
+    ///
+    /// # Ok::<(), Error>(()) }
+    /// ```
+    pub fn from_slice_with_attrs(
+        dev: &device::Device<Bound>,
+        data: &[T],
+        gfp_flags: kernel::alloc::Flags,
+        dma_attrs: Attrs,
+    ) -> Result<Coherent<[T]>>
+    where
+        T: Copy,
+    {
+        CoherentBox::from_slice_with_attrs(dev, data, gfp_flags, dma_attrs).map(Into::into)
+    }
+
+    /// Performs the same functionality as [`Coherent::from_slice_with_attrs`], except the
+    /// `dma_attrs` is 0 by default.
+    #[inline]
+    pub fn from_slice(
+        dev: &device::Device<Bound>,
+        data: &[T],
+        gfp_flags: kernel::alloc::Flags,
+    ) -> Result<Coherent<[T]>>
+    where
+        T: Copy,
+    {
+        Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
+    }
 }
 
 impl<T> Coherent<[T]> {

-- 
2.53.0

"Alexandre Courbot" <acourbot@nvidia.com> writes:

> A very common pattern is to create a block of coherent memory with the
> content of an already-existing slice of bytes (e.g. a loaded firmware
> blob).
>
> `CoherentBox` makes this easier, but still implies a potentially
> panicking operation with `copy_from_slice` that requires a `PANIC`
> comment.
>
> Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
> and `CoherentBox` to turn this into a trivial one-step operation.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

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

I like `?` more than `.map()` as well, but it's fine to me either way.


Best regards,
Andreas Hindborg

On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
> A very common pattern is to create a block of coherent memory with the
> content of an already-existing slice of bytes (e.g. a loaded firmware
> blob).
>
> `CoherentBox` makes this easier, but still implies a potentially
> panicking operation with `copy_from_slice` that requires a `PANIC`
> comment.
>
> Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
> and `CoherentBox` to turn this into a trivial one-step operation.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  rust/kernel/dma.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 6d2bec52806b..a5cc993c919e 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -453,6 +453,62 @@ pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result
>  
>          Ok(())
>      }
> +
> +    /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
> +    /// copy of its contents.
> +    ///
> +    /// This is the [`CoherentBox`] variant of [`Coherent::from_slice_with_attrs`].
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use core::ops::Deref;
> +    ///
> +    /// # use kernel::device::{Bound, Device};
> +    /// use kernel::dma::{
> +    ///     attrs::*,
> +    ///     CoherentBox
> +    /// };
> +    ///
> +    /// # fn test(dev: &Device<Bound>) -> Result {
> +    /// let data = [0u8, 1u8, 2u8, 3u8];
> +    /// let c: CoherentBox<[u8]> =
> +    ///     CoherentBox::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
> +    ///
> +    /// assert_eq!(c.deref(), &data);
> +    /// # Ok::<(), Error>(()) }
> +    /// ```
> +    pub fn from_slice_with_attrs(
> +        dev: &device::Device<Bound>,
> +        data: &[T],
> +        gfp_flags: kernel::alloc::Flags,
> +        dma_attrs: Attrs,
> +    ) -> Result<Self>
> +    where
> +        T: Copy,
> +    {
> +        Coherent::<T>::alloc_slice_with_attrs(dev, data.len(), gfp_flags, dma_attrs)
> +            .map(Self)

I'd rather just use `?` and not use map.

> +            .map(|mut slice| {
> +                // PANIC: `slice` was created with length `data.len()`.
> +                slice.copy_from_slice(data);
> +                slice
> +            })
> +    }
> +
> +    /// Performs the same functionality as [`CoherentBox::from_slice_with_attrs`], except the
> +    /// `dma_attrs` is 0 by default.
> +    #[inline]
> +    pub fn from_slice(
> +        dev: &device::Device<Bound>,
> +        data: &[T],
> +        gfp_flags: kernel::alloc::Flags,
> +    ) -> Result<Self>
> +    where
> +        T: Copy,
> +    {
> +        Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
> +    }
>  }
>  
>  impl<T: AsBytes + FromBytes> CoherentBox<T> {
> @@ -827,6 +883,52 @@ pub fn zeroed_slice(
>      ) -> Result<Coherent<[T]>> {
>          Self::zeroed_slice_with_attrs(dev, len, gfp_flags, Attrs(0))
>      }
> +
> +    /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
> +    /// copy of its contents.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::device::{Bound, Device};
> +    /// use kernel::dma::{
> +    ///     attrs::*,
> +    ///     Coherent
> +    /// };
> +    ///
> +    /// # fn test(dev: &Device<Bound>) -> Result {
> +    /// let data = [0u8, 1u8, 2u8, 3u8];
> +    /// // `c` has the same content as `data`.
> +    /// let c: Coherent<[u8]> =
> +    ///     Coherent::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
> +    ///
> +    /// # Ok::<(), Error>(()) }
> +    /// ```
> +    pub fn from_slice_with_attrs(
> +        dev: &device::Device<Bound>,
> +        data: &[T],
> +        gfp_flags: kernel::alloc::Flags,
> +        dma_attrs: Attrs,
> +    ) -> Result<Coherent<[T]>>
> +    where
> +        T: Copy,
> +    {
> +        CoherentBox::from_slice_with_attrs(dev, data, gfp_flags, dma_attrs).map(Into::into)

This function can be inline as it's just wrapping another.

Best,
Gary

> +    }
> +
> +    /// Performs the same functionality as [`Coherent::from_slice_with_attrs`], except the
> +    /// `dma_attrs` is 0 by default.
> +    #[inline]
> +    pub fn from_slice(
> +        dev: &device::Device<Bound>,
> +        data: &[T],
> +        gfp_flags: kernel::alloc::Flags,
> +    ) -> Result<Coherent<[T]>>
> +    where
> +        T: Copy,
> +    {
> +        Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
> +    }
>  }
>  
>  impl<T> Coherent<[T]> {

On Tue Mar 24, 2026 at 1:55 AM JST, Gary Guo wrote:
> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>> A very common pattern is to create a block of coherent memory with the
>> content of an already-existing slice of bytes (e.g. a loaded firmware
>> blob).
>>
>> `CoherentBox` makes this easier, but still implies a potentially
>> panicking operation with `copy_from_slice` that requires a `PANIC`
>> comment.
>>
>> Add `from_slice_with_attrs` and `from_slice` methods to both `Coherent`
>> and `CoherentBox` to turn this into a trivial one-step operation.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  rust/kernel/dma.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 102 insertions(+)
>>
>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>> index 6d2bec52806b..a5cc993c919e 100644
>> --- a/rust/kernel/dma.rs
>> +++ b/rust/kernel/dma.rs
>> @@ -453,6 +453,62 @@ pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result
>>  
>>          Ok(())
>>      }
>> +
>> +    /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
>> +    /// copy of its contents.
>> +    ///
>> +    /// This is the [`CoherentBox`] variant of [`Coherent::from_slice_with_attrs`].
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// use core::ops::Deref;
>> +    ///
>> +    /// # use kernel::device::{Bound, Device};
>> +    /// use kernel::dma::{
>> +    ///     attrs::*,
>> +    ///     CoherentBox
>> +    /// };
>> +    ///
>> +    /// # fn test(dev: &Device<Bound>) -> Result {
>> +    /// let data = [0u8, 1u8, 2u8, 3u8];
>> +    /// let c: CoherentBox<[u8]> =
>> +    ///     CoherentBox::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
>> +    ///
>> +    /// assert_eq!(c.deref(), &data);
>> +    /// # Ok::<(), Error>(()) }
>> +    /// ```
>> +    pub fn from_slice_with_attrs(
>> +        dev: &device::Device<Bound>,
>> +        data: &[T],
>> +        gfp_flags: kernel::alloc::Flags,
>> +        dma_attrs: Attrs,
>> +    ) -> Result<Self>
>> +    where
>> +        T: Copy,
>> +    {
>> +        Coherent::<T>::alloc_slice_with_attrs(dev, data.len(), gfp_flags, dma_attrs)
>> +            .map(Self)
>
> I'd rather just use `?` and not use map.

Then it looks like this:

    let mut slice = Self(Coherent::<T>::alloc_slice_with_attrs(
        dev,
        data.len(),
        gfp_flags,
        dma_attrs,
    )?);

    // PANIC: `slice` was created with length `data.len()`.
    slice.copy_from_slice(data);

    Ok(slice)

FWIW I find using `map` more elegant, but I've made the change for v2
nonetheless.

>
>> +            .map(|mut slice| {
>> +                // PANIC: `slice` was created with length `data.len()`.
>> +                slice.copy_from_slice(data);
>> +                slice
>> +            })
>> +    }
>> +
>> +    /// Performs the same functionality as [`CoherentBox::from_slice_with_attrs`], except the
>> +    /// `dma_attrs` is 0 by default.
>> +    #[inline]
>> +    pub fn from_slice(
>> +        dev: &device::Device<Bound>,
>> +        data: &[T],
>> +        gfp_flags: kernel::alloc::Flags,
>> +    ) -> Result<Self>
>> +    where
>> +        T: Copy,
>> +    {
>> +        Self::from_slice_with_attrs(dev, data, gfp_flags, Attrs(0))
>> +    }
>>  }
>>  
>>  impl<T: AsBytes + FromBytes> CoherentBox<T> {
>> @@ -827,6 +883,52 @@ pub fn zeroed_slice(
>>      ) -> Result<Coherent<[T]>> {
>>          Self::zeroed_slice_with_attrs(dev, len, gfp_flags, Attrs(0))
>>      }
>> +
>> +    /// Allocates a region of coherent memory of the same size as `data` and initializes it with a
>> +    /// copy of its contents.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// # use kernel::device::{Bound, Device};
>> +    /// use kernel::dma::{
>> +    ///     attrs::*,
>> +    ///     Coherent
>> +    /// };
>> +    ///
>> +    /// # fn test(dev: &Device<Bound>) -> Result {
>> +    /// let data = [0u8, 1u8, 2u8, 3u8];
>> +    /// // `c` has the same content as `data`.
>> +    /// let c: Coherent<[u8]> =
>> +    ///     Coherent::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
>> +    ///
>> +    /// # Ok::<(), Error>(()) }
>> +    /// ```
>> +    pub fn from_slice_with_attrs(
>> +        dev: &device::Device<Bound>,
>> +        data: &[T],
>> +        gfp_flags: kernel::alloc::Flags,
>> +        dma_attrs: Attrs,
>> +    ) -> Result<Coherent<[T]>>
>> +    where
>> +        T: Copy,
>> +    {
>> +        CoherentBox::from_slice_with_attrs(dev, data, gfp_flags, dma_attrs).map(Into::into)
>
> This function can be inline as it's just wrapping another.

Indeed, thanks!

On Thu, Mar 26, 2026 at 3:59 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Then it looks like this:
>
>     let mut slice = Self(Coherent::<T>::alloc_slice_with_attrs(
>         dev,
>         data.len(),
>         gfp_flags,
>         dma_attrs,
>     )?);
>
>     // PANIC: `slice` was created with length `data.len()`.
>     slice.copy_from_slice(data);
>
>     Ok(slice)

Hmm... I think I prefer this version because (conceptually) it has
less calls, closures and conditionals (i.e. 2 `map`s vs. 1 `?`). Of
course, it should compile to the same thing.

In general, I think removing wrapper types early is better than
carrying them around. It also seems closer spiritually to the early
return style of the kernel in the C side.

For this case, I think both are understandable anyway.

Cheers,
Miguel

On 3/26/26 3:59 PM, Alexandre Courbot wrote:
> Then it looks like this:
> 
>     let mut slice = Self(Coherent::<T>::alloc_slice_with_attrs(
>         dev,
>         data.len(),
>         gfp_flags,
>         dma_attrs,
>     )?);
> 
>     // PANIC: `slice` was created with length `data.len()`.
>     slice.copy_from_slice(data);
> 
>     Ok(slice)
> 
> FWIW I find using `map` more elegant, but I've made the change for v2
> nonetheless.
I also prefer map() in this case.