On Fri Mar 20, 2026 at 7:45 PM GMT, Danilo Krummrich wrote: > Currently, dma::Coherent cannot safely provide (mutable) access to its > underlying memory because the memory might be concurrently accessed by a > DMA device. This makes it difficult to safely initialize the memory > before handing it over to the hardware. > > Introduce dma::CoherentBox, a type that encapsulates a dma::Coherent > before its DMA address is exposed to the device. dma::CoherentBox can > guarantee exclusive access to the inner dma::Coherent and implement > Deref and DerefMut. > > Once the memory is properly initialized, dma::CoherentBox can be > converted into a regular dma::Coherent. > > Reviewed-by: Alice Ryhl <[email protected]> > Signed-off-by: Danilo Krummrich <[email protected]>
Reviewed-by: Gary Guo <[email protected]> See some nits below: > --- > rust/kernel/dma.rs | 154 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 153 insertions(+), 1 deletion(-) > > diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs > index db645b01bdd0..cefb54f0424a 100644 > --- a/rust/kernel/dma.rs > +++ b/rust/kernel/dma.rs > @@ -20,7 +20,13 @@ > FromBytes, // > }, // > }; > -use core::ptr::NonNull; > +use core::{ > + ops::{ > + Deref, > + DerefMut, // > + }, > + ptr::NonNull, // > +}; > > /// DMA address type. > /// > @@ -352,6 +358,152 @@ fn from(direction: DataDirection) -> Self { > } > } > > +/// CPU-owned DMA allocation that can be converted into a device-shared > [`Coherent`] object. > +/// > +/// Unlike [`Coherent`], a [`CoherentBox`] is guaranteed to be fully owned > by the CPU -- its DMA > +/// address is not exposed and it cannot be accessed by a device. This means > it can safely be used > +/// like a normal boxed allocation (e.g. direct reads, writes, and mutable > slices are all safe). > +/// > +/// A typical use is to allocate a [`CoherentBox`], populate it with normal > CPU access, and then > +/// convert it into a [`Coherent`] object to share it with the device. > +/// > +/// # Examples > +/// > +/// `CoherentBox<T>`: > +/// > +/// ``` > +/// # use kernel::device::{ > +/// # Bound, > +/// # Device, > +/// # }; > +/// use kernel::dma::{attrs::*, > +/// Coherent, > +/// CoherentBox, > +/// }; > +/// > +/// # fn test(dev: &Device<Bound>) -> Result { > +/// let mut dmem: CoherentBox<u64> = CoherentBox::zeroed(dev, GFP_KERNEL)?; > +/// *dmem = 42; > +/// let dmem: Coherent<u64> = dmem.into(); > +/// # Ok::<(), Error>(()) } > +/// ``` > +/// > +/// `CoherentBox<[T]>`: > +/// > +/// > +/// ``` > +/// # use kernel::device::{ > +/// # Bound, > +/// # Device, > +/// # }; > +/// use kernel::dma::{attrs::*, > +/// Coherent, > +/// CoherentBox, > +/// }; > +/// > +/// # fn test(dev: &Device<Bound>) -> Result { > +/// let mut dmem: CoherentBox<[u64]> = CoherentBox::zeroed_slice(dev, 4, > GFP_KERNEL)?; > +/// dmem.fill(42); > +/// let dmem: Coherent<[u64]> = dmem.into(); > +/// # Ok::<(), Error>(()) } > +/// ``` > +pub struct CoherentBox<T: AsBytes + FromBytes + KnownSize + > ?Sized>(Coherent<T>); Similar to the changes I've made to `Coherent`, this can also just have `KnownSize + ?Sized` bound on the struct, and only have those bounds on the constructor. This saves a quite bit of duplication where everything needs to say `T: AsBytes + FromBytes`. Should be something fix-up-able on apply, or something left for future cleanup. > + > +impl<T: AsBytes + FromBytes> CoherentBox<[T]> { this and ... > + /// [`CoherentBox`] variant of [`Coherent::zeroed_slice_with_attrs`]. > + #[inline] > + pub fn zeroed_slice_with_attrs( > + dev: &device::Device<Bound>, > + count: usize, > + gfp_flags: kernel::alloc::Flags, > + dma_attrs: Attrs, > + ) -> Result<Self> { > + Coherent::zeroed_slice_with_attrs(dev, count, gfp_flags, > dma_attrs).map(Self) > + } > + > + /// Same as [CoherentBox::zeroed_slice_with_attrs], but with > `dma::Attrs(0)`. > + #[inline] > + pub fn zeroed_slice( > + dev: &device::Device<Bound>, > + count: usize, > + gfp_flags: kernel::alloc::Flags, > + ) -> Result<Self> { > + Self::zeroed_slice_with_attrs(dev, count, gfp_flags, Attrs(0)) > + } > + > + /// Initializes the element at `i` using the given initializer. > + /// > + /// Returns `EINVAL` if `i` is out of bounds. > + pub fn init_at<E>(&mut self, i: usize, init: impl Init<T, E>) -> Result > + where > + Error: From<E>, > + { > + if i >= self.0.len() { > + return Err(EINVAL); > + } > + > + let ptr = &raw mut self[i]; > + > + // SAFETY: > + // - `ptr` is valid, properly aligned, and within this allocation. > + // - `T: AsBytes + FromBytes` guarantees all bit patterns are valid, > so partial writes on > + // error cannot leave the element in an invalid state. > + // - The DMA address has not been exposed yet, so there is no > concurrent device access. > + unsafe { init.__init(ptr)? }; > + > + Ok(()) > + } > +} > + > +impl<T: AsBytes + FromBytes> CoherentBox<T> { this should be the only two places that need `AsBytes + FromBytes`. Best, Gary > + /// Same as [`CoherentBox::zeroed_slice_with_attrs`], but for a single > element. > + #[inline] > + pub fn zeroed_with_attrs( > + dev: &device::Device<Bound>, > + gfp_flags: kernel::alloc::Flags, > + dma_attrs: Attrs, > + ) -> Result<Self> { > + Coherent::zeroed_with_attrs(dev, gfp_flags, dma_attrs).map(Self) > + } > + > + /// Same as [`CoherentBox::zeroed_slice`], but for a single element. > + #[inline] > + pub fn zeroed(dev: &device::Device<Bound>, gfp_flags: > kernel::alloc::Flags) -> Result<Self> { > + Self::zeroed_with_attrs(dev, gfp_flags, Attrs(0)) > + } > +} > + > +impl<T: AsBytes + FromBytes + KnownSize + ?Sized> Deref for CoherentBox<T> { > + type Target = T; > + > + #[inline] > + fn deref(&self) -> &Self::Target { > + // SAFETY: > + // - We have not exposed the DMA address yet, so there can't be any > concurrent access by a > + // device. > + // - We have exclusive access to `self.0`. > + unsafe { self.0.as_ref() } > + } > +} > + > +impl<T: AsBytes + FromBytes + KnownSize + ?Sized> DerefMut for > CoherentBox<T> { > + #[inline] > + fn deref_mut(&mut self) -> &mut Self::Target { > + // SAFETY: > + // - We have not exposed the DMA address yet, so there can't be any > concurrent access by a > + // device. > + // - We have exclusive access to `self.0`. > + unsafe { self.0.as_mut() } > + } > +} > + > +impl<T: AsBytes + FromBytes + KnownSize + ?Sized> From<CoherentBox<T>> for > Coherent<T> { > + #[inline] > + fn from(value: CoherentBox<T>) -> Self { > + value.0 > + } > +} > + > /// 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
