Re: [PATCH 1/7] rust: dma: add from-slice constructors for Coherent and CoherentBox

[Alexandre Courbot]

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@xxxxxxxxxx>
>> ---
>>  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!