[PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core

[PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core

[Alexandre Courbot]

nova-core's `DmaObject` type has been created to serve the same purpose
as `dma::Coherent`, with the addition of a handy constructor to create
an object from a slice of bytes, but without the flexibility of
`dma::Coherent` since `DmaObject` is limited to slices of bytes.

This series adds new constructors to `Coherent` and `CoherentBox` to
cover this (arguably common) use-case, and updates the nova-core code to
use them. This results in more consistent code overall, and allows us to
retire `DmaObject` and nova-core's `dma` module.

Depends on drm-rust-next as of 2026-03-21 + [1].

[1] https://lore.kernel.org/all/20260320194626.36263-1-dakr@kernel.org/

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Alexandre Courbot (7):
      rust: dma: add from-slice constructors for Coherent and CoherentBox
      gpu: nova-core: firmware: riscv: use dma::Coherent
      gpu: nova-core: firmware: fwsec: use dma::Coherent
      gpu: nova-core: falcon: use dma::Coherent
      gpu: nova-core: fb: use dma::Coherent
      gpu: nova-core: firmware: gsp: use dma::Coherent for signatures
      gpu: nova-core: firmware: gsp: use dma::Coherent for level0 table

 drivers/gpu/nova-core/dma.rs                       |  53 -----------
 drivers/gpu/nova-core/falcon.rs                    |   6 +-
 drivers/gpu/nova-core/fb.rs                        |   6 +-
 drivers/gpu/nova-core/firmware/fwsec/bootloader.rs |   6 +-
 drivers/gpu/nova-core/firmware/gsp.rs              |  27 +++---
 drivers/gpu/nova-core/firmware/riscv.rs            |   6 +-
 drivers/gpu/nova-core/nova_core.rs                 |   1 -
 rust/kernel/dma.rs                                 | 102 +++++++++++++++++++++
 8 files changed, 129 insertions(+), 78 deletions(-)
---
base-commit: 19761c783a46bc4fb2ba0ef22111f7dd27b441e6
change-id: 20260321-b4-nova-dma-removal-a7e88d4a6790

Best regards,
-- 
Alexandre Courbot <acourbot@nvidia.com>

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

[Alexandre Courbot]

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

[PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent

[Alexandre Courbot]

Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
index 14aad2f0ee8a..2afa7f36404e 100644
--- a/drivers/gpu/nova-core/firmware/riscv.rs
+++ b/drivers/gpu/nova-core/firmware/riscv.rs
@@ -5,13 +5,13 @@
 
 use kernel::{
     device,
+    dma::Coherent,
     firmware::Firmware,
     prelude::*,
     transmute::FromBytes, //
 };
 
 use crate::{
-    dma::DmaObject,
     firmware::BinFirmware,
     num::FromSafeCast, //
 };
@@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
     /// Application version.
     pub(crate) app_version: u32,
     /// Device-mapped firmware image.
-    pub(crate) ucode: DmaObject,
+    pub(crate) ucode: Coherent<[u8]>,
 }
 
 impl RiscvFirmware {
@@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
             let len = usize::from_safe_cast(bin_fw.hdr.data_size);
             let end = start.checked_add(len).ok_or(EINVAL)?;
 
-            DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
+            Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
         };
 
         Ok(Self {

-- 
2.53.0

[PATCH 3/7] gpu: nova-core: firmware: fwsec: use dma::Coherent

[Alexandre Courbot]

Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/firmware/fwsec/bootloader.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
index 342dba59b2f9..7c6bb0a871c1 100644
--- a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
@@ -12,6 +12,7 @@
         self,
         Device, //
     },
+    dma::Coherent,
     prelude::*,
     ptr::{
         Alignable,
@@ -25,7 +26,6 @@
 };
 
 use crate::{
-    dma::DmaObject,
     driver::Bar0,
     falcon::{
         self,
@@ -126,7 +126,7 @@ unsafe impl AsBytes for BootloaderDmemDescV2 {}
 /// operation.
 pub(crate) struct FwsecFirmwareWithBl {
     /// DMA object the bootloader will copy the firmware from.
-    _firmware_dma: DmaObject,
+    _firmware_dma: Coherent<[u8]>,
     /// Code of the bootloader to be loaded into non-secure IMEM.
     ucode: KVec<u8>,
     /// Descriptor to be loaded into DMEM for the bootloader to read.
@@ -208,7 +208,7 @@ pub(crate) fn new(
 
             (
                 align_padding,
-                DmaObject::from_data(dev, firmware_obj.as_slice())?,
+                Coherent::from_slice(dev, firmware_obj.as_slice(), GFP_KERNEL)?,
             )
         };
 

-- 
2.53.0

[PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent

[Alexandre Courbot]

Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/falcon.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 5bf8da8760bf..f6239c44dd80 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -10,6 +10,7 @@
         Device, //
     },
     dma::{
+        Coherent,
         DmaAddress,
         DmaMask, //
     },
@@ -20,7 +21,6 @@
 };
 
 use crate::{
-    dma::DmaObject,
     driver::Bar0,
     falcon::hal::LoadMethod,
     gpu::Chipset,
@@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
     fn dma_wr(
         &self,
         bar: &Bar0,
-        dma_obj: &DmaObject,
+        dma_obj: &Coherent<[u8]>,
         target_mem: FalconMem,
         load_offsets: FalconDmaLoadTarget,
     ) -> Result {
@@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
         fw: &F,
     ) -> Result {
         // Create DMA object with firmware content as the source of the DMA engine.
-        let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
+        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
 
         self.dma_reset(bar);
         regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {

-- 
2.53.0

[PATCH 5/7] gpu: nova-core: fb: use dma::Coherent

[Alexandre Courbot]

Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 6536d0035cb1..ba971a114a06 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -7,6 +7,7 @@
 
 use kernel::{
     device,
+    dma::Coherent,
     fmt,
     prelude::*,
     ptr::{
@@ -18,7 +19,6 @@
 };
 
 use crate::{
-    dma::DmaObject,
     driver::Bar0,
     firmware::gsp::GspFirmware,
     gpu::Chipset,
@@ -52,7 +52,7 @@ pub(crate) struct SysmemFlush {
     chipset: Chipset,
     device: ARef<device::Device>,
     /// Keep the page alive as long as we need it.
-    page: DmaObject,
+    page: Coherent<[u8]>,
 }
 
 impl SysmemFlush {
@@ -62,7 +62,7 @@ pub(crate) fn register(
         bar: &Bar0,
         chipset: Chipset,
     ) -> Result<Self> {
-        let page = DmaObject::new(dev, kernel::page::PAGE_SIZE)?;
+        let page = Coherent::zeroed_slice(dev, kernel::page::PAGE_SIZE, GFP_KERNEL)?;
 
         hal::fb_hal(chipset).write_sysmem_flush_page(bar, page.dma_handle())?;
 

-- 
2.53.0

[PATCH 6/7] gpu: nova-core: firmware: gsp: use dma::Coherent for signatures

[Alexandre Courbot]

Replace the nova-core local `DmaObject` with a `Coherent` that can
fulfill the same role.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/firmware/gsp.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 9488a626352f..1e0d545a74fe 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -3,6 +3,7 @@
 use kernel::{
     device,
     dma::{
+        Coherent,
         DataDirection,
         DmaAddress, //
     },
@@ -140,7 +141,7 @@ pub(crate) struct GspFirmware {
     /// Size in bytes of the firmware contained in [`Self::fw`].
     pub(crate) size: usize,
     /// Device-mapped GSP signatures matching the GPU's [`Chipset`].
-    pub(crate) signatures: DmaObject,
+    pub(crate) signatures: Coherent<[u8]>,
     /// GSP bootloader, verifies the GSP firmware before loading and running it.
     pub(crate) bootloader: RiscvFirmware,
 }
@@ -226,7 +227,7 @@ pub(crate) fn new<'a>(
 
                     elf::elf64_section(firmware.data(), sigs_section)
                         .ok_or(EINVAL)
-                        .and_then(|data| DmaObject::from_data(dev, data))?
+                        .and_then(|data| Coherent::from_slice(dev, data, GFP_KERNEL))?
                 },
                 bootloader: {
                     let bl = super::request_firmware(dev, chipset, "bootloader", ver)?;

-- 
2.53.0

[PATCH 7/7] gpu: nova-core: firmware: gsp: use dma::Coherent for level0 table

[Alexandre Courbot]

Replace the nova-core local `DmaObject` with a `CoherentBox` that can
fulfill the same role.

Since `CoherentBox` is more flexible than `DmaObject`, we can use the
native `u64` type for page table entries instead of messing with bytes.

The `dma` module becomes unused with that change, so remove it as well.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/dma.rs          | 53 -----------------------------------
 drivers/gpu/nova-core/firmware/gsp.rs | 22 ++++++++-------
 drivers/gpu/nova-core/nova_core.rs    |  1 -
 3 files changed, 12 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
deleted file mode 100644
index 3c19d5ffcfe8..000000000000
--- a/drivers/gpu/nova-core/dma.rs
+++ /dev/null
@@ -1,53 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-//! Simple DMA object wrapper.
-
-use core::ops::{
-    Deref,
-    DerefMut, //
-};
-
-use kernel::{
-    device,
-    dma::Coherent,
-    page::PAGE_SIZE,
-    prelude::*, //
-};
-
-pub(crate) struct DmaObject {
-    dma: Coherent<[u8]>,
-}
-
-impl DmaObject {
-    pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Self> {
-        let len = core::alloc::Layout::from_size_align(len, PAGE_SIZE)
-            .map_err(|_| EINVAL)?
-            .pad_to_align()
-            .size();
-        let dma = Coherent::zeroed_slice(dev, len, GFP_KERNEL)?;
-
-        Ok(Self { dma })
-    }
-
-    pub(crate) fn from_data(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self> {
-        let dma_obj = Self::new(dev, data.len())?;
-        // SAFETY: We have just allocated the DMA memory, we are the only users and
-        // we haven't made the device aware of the handle yet.
-        unsafe { dma_obj.as_mut()[..data.len()].copy_from_slice(data) };
-        Ok(dma_obj)
-    }
-}
-
-impl Deref for DmaObject {
-    type Target = Coherent<[u8]>;
-
-    fn deref(&self) -> &Self::Target {
-        &self.dma
-    }
-}
-
-impl DerefMut for DmaObject {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.dma
-    }
-}
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 1e0d545a74fe..86fb3f074195 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -4,10 +4,10 @@
     device,
     dma::{
         Coherent,
+        CoherentBox,
         DataDirection,
         DmaAddress, //
     },
-    kvec,
     prelude::*,
     scatterlist::{
         Owned,
@@ -16,7 +16,6 @@
 };
 
 use crate::{
-    dma::DmaObject,
     firmware::riscv::RiscvFirmware,
     gpu::{
         Architecture,
@@ -137,7 +136,7 @@ pub(crate) struct GspFirmware {
     #[pin]
     level1: SGTable<Owned<VVec<u8>>>,
     /// Level 0 page table (single 4KB page) with one entry: DMA address of first level 1 page.
-    level0: DmaObject,
+    level0: Coherent<[u64]>,
     /// Size in bytes of the firmware contained in [`Self::fw`].
     pub(crate) size: usize,
     /// Device-mapped GSP signatures matching the GPU's [`Chipset`].
@@ -198,17 +197,20 @@ pub(crate) fn new<'a>(
                     // Allocate the level 0 page table as a device-visible DMA object, and map the
                     // level 1 page table onto it.
 
-                    // Level 0 page table data.
-                    let mut level0_data = kvec![0u8; GSP_PAGE_SIZE]?;
-
                     // Fill level 1 page entry.
                     let level1_entry = level1.iter().next().ok_or(EINVAL)?;
                     let level1_entry_addr = level1_entry.dma_address();
-                    let dst = &mut level0_data[..size_of_val(&level1_entry_addr)];
-                    dst.copy_from_slice(&level1_entry_addr.to_le_bytes());
 
-                    // Turn the level0 page table into a [`DmaObject`].
-                    DmaObject::from_data(dev, &level0_data)?
+                    // Create level 0 page table data and fill its first entry with the level 1
+                    // table.
+                    let mut level0 = CoherentBox::<[u64]>::zeroed_slice(
+                        dev,
+                        GSP_PAGE_SIZE / size_of::<u64>(),
+                        GFP_KERNEL
+                    )?;
+                    level0[0] = level1_entry_addr.to_le();
+
+                    level0.into()
                 },
                 size,
                 signatures: {
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b5caf1044697..e0271e0c6335 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -5,7 +5,6 @@
 #[macro_use]
 mod bitfield;
 
-mod dma;
 mod driver;
 mod falcon;
 mod fb;

-- 
2.53.0

Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent

[Gary Guo]

On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
> Replace the nova-core local `DmaObject` with a `Coherent` that can
> fulfill the same role.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
> index 14aad2f0ee8a..2afa7f36404e 100644
> --- a/drivers/gpu/nova-core/firmware/riscv.rs
> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
> @@ -5,13 +5,13 @@
>  
>  use kernel::{
>      device,
> +    dma::Coherent,
>      firmware::Firmware,
>      prelude::*,
>      transmute::FromBytes, //
>  };
>  
>  use crate::{
> -    dma::DmaObject,
>      firmware::BinFirmware,
>      num::FromSafeCast, //
>  };
> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>      /// Application version.
>      pub(crate) app_version: u32,
>      /// Device-mapped firmware image.
> -    pub(crate) ucode: DmaObject,
> +    pub(crate) ucode: Coherent<[u8]>,
>  }
>  
>  impl RiscvFirmware {
> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>              let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>              let end = start.checked_add(len).ok_or(EINVAL)?;
>  
> -            DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
> +            Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?

`DmaObject` rounds the data up to be page-sized, while this new API doesn't.

It has impact on alignment, as the allocator aligns things to the largest
power-of-two exponent of the allocated size.

Best,
Gary

>          };
>  
>          Ok(Self {

Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent

[Alexandre Courbot]

On Sat Mar 21, 2026 at 11:58 PM JST, Gary Guo wrote:
> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>> fulfill the same role.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
>> index 14aad2f0ee8a..2afa7f36404e 100644
>> --- a/drivers/gpu/nova-core/firmware/riscv.rs
>> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
>> @@ -5,13 +5,13 @@
>>  
>>  use kernel::{
>>      device,
>> +    dma::Coherent,
>>      firmware::Firmware,
>>      prelude::*,
>>      transmute::FromBytes, //
>>  };
>>  
>>  use crate::{
>> -    dma::DmaObject,
>>      firmware::BinFirmware,
>>      num::FromSafeCast, //
>>  };
>> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>>      /// Application version.
>>      pub(crate) app_version: u32,
>>      /// Device-mapped firmware image.
>> -    pub(crate) ucode: DmaObject,
>> +    pub(crate) ucode: Coherent<[u8]>,
>>  }
>>  
>>  impl RiscvFirmware {
>> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>>              let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>>              let end = start.checked_add(len).ok_or(EINVAL)?;
>>  
>> -            DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
>> +            Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
>
> `DmaObject` rounds the data up to be page-sized, while this new API doesn't.
>
> It has impact on alignment, as the allocator aligns things to the largest
> power-of-two exponent of the allocated size.

Doesn't `dma_alloc_coherent` always allocate from the page pool and thus
returns page-aligned memory though? I'm not sure why `DmaObject` was
doing that but it was redundant I think.

Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent

[Gary Guo]

On Mon Mar 23, 2026 at 6:15 AM GMT, Alexandre Courbot wrote:
> On Sat Mar 21, 2026 at 11:58 PM JST, Gary Guo wrote:
>> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>> fulfill the same role.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>>  drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
>>> index 14aad2f0ee8a..2afa7f36404e 100644
>>> --- a/drivers/gpu/nova-core/firmware/riscv.rs
>>> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
>>> @@ -5,13 +5,13 @@
>>>  
>>>  use kernel::{
>>>      device,
>>> +    dma::Coherent,
>>>      firmware::Firmware,
>>>      prelude::*,
>>>      transmute::FromBytes, //
>>>  };
>>>  
>>>  use crate::{
>>> -    dma::DmaObject,
>>>      firmware::BinFirmware,
>>>      num::FromSafeCast, //
>>>  };
>>> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>>>      /// Application version.
>>>      pub(crate) app_version: u32,
>>>      /// Device-mapped firmware image.
>>> -    pub(crate) ucode: DmaObject,
>>> +    pub(crate) ucode: Coherent<[u8]>,
>>>  }
>>>  
>>>  impl RiscvFirmware {
>>> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>>>              let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>>>              let end = start.checked_add(len).ok_or(EINVAL)?;
>>>  
>>> -            DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
>>> +            Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
>>
>> `DmaObject` rounds the data up to be page-sized, while this new API doesn't.
>>
>> It has impact on alignment, as the allocator aligns things to the largest
>> power-of-two exponent of the allocated size.
>
> Doesn't `dma_alloc_coherent` always allocate from the page pool and thus
> returns page-aligned memory though?

Oh you're right, this is not from DMA pool, so allocations are page-aligned
indeed.

I brought this up because I got bite by this size adjustment behaviour
because in the `from_data` I initially put

    unsafe { dma_obj.as_mut().copy_from_slice(data) };

but that doesn't work due to the size being aligned up so dma_obj.len() !=
data.len().

But if this behaviour is not needed it does simplify things quite a bit.

> I'm not sure why `DmaObject` was doing that but it was redundant I think.

A question for yourself I guess? :-)

https://lore.kernel.org/all/20250619-nova-frts-v6-13-ecf41ef99252@nvidia.com/

Best,
Gary

Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent

[Alexandre Courbot]

On Mon Mar 23, 2026 at 10:05 PM JST, Gary Guo wrote:
> On Mon Mar 23, 2026 at 6:15 AM GMT, Alexandre Courbot wrote:
>> On Sat Mar 21, 2026 at 11:58 PM JST, Gary Guo wrote:
>>> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>>> fulfill the same role.
>>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> ---
>>>>  drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
>>>> index 14aad2f0ee8a..2afa7f36404e 100644
>>>> --- a/drivers/gpu/nova-core/firmware/riscv.rs
>>>> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
>>>> @@ -5,13 +5,13 @@
>>>>  
>>>>  use kernel::{
>>>>      device,
>>>> +    dma::Coherent,
>>>>      firmware::Firmware,
>>>>      prelude::*,
>>>>      transmute::FromBytes, //
>>>>  };
>>>>  
>>>>  use crate::{
>>>> -    dma::DmaObject,
>>>>      firmware::BinFirmware,
>>>>      num::FromSafeCast, //
>>>>  };
>>>> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>>>>      /// Application version.
>>>>      pub(crate) app_version: u32,
>>>>      /// Device-mapped firmware image.
>>>> -    pub(crate) ucode: DmaObject,
>>>> +    pub(crate) ucode: Coherent<[u8]>,
>>>>  }
>>>>  
>>>>  impl RiscvFirmware {
>>>> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>>>>              let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>>>>              let end = start.checked_add(len).ok_or(EINVAL)?;
>>>>  
>>>> -            DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
>>>> +            Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
>>>
>>> `DmaObject` rounds the data up to be page-sized, while this new API doesn't.
>>>
>>> It has impact on alignment, as the allocator aligns things to the largest
>>> power-of-two exponent of the allocated size.
>>
>> Doesn't `dma_alloc_coherent` always allocate from the page pool and thus
>> returns page-aligned memory though?
>
> Oh you're right, this is not from DMA pool, so allocations are page-aligned
> indeed.
>
> I brought this up because I got bite by this size adjustment behaviour
> because in the `from_data` I initially put
>
>     unsafe { dma_obj.as_mut().copy_from_slice(data) };
>
> but that doesn't work due to the size being aligned up so dma_obj.len() !=
> data.len().
>
> But if this behaviour is not needed it does simplify things quite a bit.

I was contemplating hardening the `Coherent` allocations by padding the
structs when there are specific alignment needs (in nova-core it's only
a couple of types affected), but then thought this was quite restrictive
and should be handled by an alignment parameter on `Coherent` directly -
and `Coherent` doesn't have such a parameter because the underlying C
API doesn't either, and the page alignment seems to be a property of the
API itself.

Maybe that point is worth mentioning in the documentation of `Coherent`?

>
>> I'm not sure why `DmaObject` was doing that but it was redundant I think.
>
> A question for yourself I guess? :-)
>
> https://lore.kernel.org/all/20250619-nova-frts-v6-13-ecf41ef99252@nvidia.com/

Ha, I was precisely wondering who was the lesser engineer who wrote that. :)

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

[Gary Guo]

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]> {

Re: [PATCH 0/7] rust: dma: add from-slice constructors and use them in nova-core

[Gary Guo]

On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
> nova-core's `DmaObject` type has been created to serve the same purpose
> as `dma::Coherent`, with the addition of a handy constructor to create
> an object from a slice of bytes, but without the flexibility of
> `dma::Coherent` since `DmaObject` is limited to slices of bytes.
>
> This series adds new constructors to `Coherent` and `CoherentBox` to
> cover this (arguably common) use-case, and updates the nova-core code to
> use them. This results in more consistent code overall, and allows us to
> retire `DmaObject` and nova-core's `dma` module.
>
> Depends on drm-rust-next as of 2026-03-21 + [1].
>
> [1] https://lore.kernel.org/all/20260320194626.36263-1-dakr@kernel.org/
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

Looks like a good cleanup. For the series (with nits fixed in first patch):

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

Best,
Gary

> ---
> Alexandre Courbot (7):
>       rust: dma: add from-slice constructors for Coherent and CoherentBox
>       gpu: nova-core: firmware: riscv: use dma::Coherent
>       gpu: nova-core: firmware: fwsec: use dma::Coherent
>       gpu: nova-core: falcon: use dma::Coherent
>       gpu: nova-core: fb: use dma::Coherent
>       gpu: nova-core: firmware: gsp: use dma::Coherent for signatures
>       gpu: nova-core: firmware: gsp: use dma::Coherent for level0 table
>
>  drivers/gpu/nova-core/dma.rs                       |  53 -----------
>  drivers/gpu/nova-core/falcon.rs                    |   6 +-
>  drivers/gpu/nova-core/fb.rs                        |   6 +-
>  drivers/gpu/nova-core/firmware/fwsec/bootloader.rs |   6 +-
>  drivers/gpu/nova-core/firmware/gsp.rs              |  27 +++---
>  drivers/gpu/nova-core/firmware/riscv.rs            |   6 +-
>  drivers/gpu/nova-core/nova_core.rs                 |   1 -
>  rust/kernel/dma.rs                                 | 102 +++++++++++++++++++++
>  8 files changed, 129 insertions(+), 78 deletions(-)
> ---
> base-commit: 19761c783a46bc4fb2ba0ef22111f7dd27b441e6
> change-id: 20260321-b4-nova-dma-removal-a7e88d4a6790
>
> Best regards,

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

[Andreas Hindborg]

"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

Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent

[Eliot Courtney]

On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
> Replace the nova-core local `DmaObject` with a `Coherent` that can
> fulfill the same role.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/falcon.rs | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 5bf8da8760bf..f6239c44dd80 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -10,6 +10,7 @@
>          Device, //
>      },
>      dma::{
> +        Coherent,
>          DmaAddress,
>          DmaMask, //
>      },
> @@ -20,7 +21,6 @@
>  };
>  
>  use crate::{
> -    dma::DmaObject,
>      driver::Bar0,
>      falcon::hal::LoadMethod,
>      gpu::Chipset,
> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
>      fn dma_wr(
>          &self,
>          bar: &Bar0,
> -        dma_obj: &DmaObject,
> +        dma_obj: &Coherent<[u8]>,
>          target_mem: FalconMem,
>          load_offsets: FalconDmaLoadTarget,
>      ) -> Result {
> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>          fw: &F,
>      ) -> Result {
>          // Create DMA object with firmware content as the source of the DMA engine.
> -        let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
> +        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;

Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
In `dma_wr` it breaks this up into 256 byte transfers. Since this
no longer pads out to a page boundary, it means that it could now error
(around "DMA transfer goes beyond range of DMA object") if the Dmem 
section's size is not divisible by 256. But tbh, I find it odd that 
`dma_wr` doesn't check that FalconDmaLoadTarget's length is a
multiple of 256 anyway, because it looks like it'll write a bunch of
unrelated bytes (since it rounds up to the nearest 256 to copy).

Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
by 256?

For this series if for all firmwares it's divisible by 256 then I think
it's fine to leave this as is for now, but I do find the lack of
checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
odd.

>  
>          self.dma_reset(bar);
>          regs::NV_PFALCON_FBIF_TRANSCFG::update(bar, &E::ID, 0, |v| {

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

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

[Danilo Krummrich]

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.

Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent

[Alexandre Courbot]

On Wed Mar 25, 2026 at 11:14 AM JST, Eliot Courtney wrote:
> On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>> fulfill the same role.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/falcon.rs | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>> index 5bf8da8760bf..f6239c44dd80 100644
>> --- a/drivers/gpu/nova-core/falcon.rs
>> +++ b/drivers/gpu/nova-core/falcon.rs
>> @@ -10,6 +10,7 @@
>>          Device, //
>>      },
>>      dma::{
>> +        Coherent,
>>          DmaAddress,
>>          DmaMask, //
>>      },
>> @@ -20,7 +21,6 @@
>>  };
>>  
>>  use crate::{
>> -    dma::DmaObject,
>>      driver::Bar0,
>>      falcon::hal::LoadMethod,
>>      gpu::Chipset,
>> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
>>      fn dma_wr(
>>          &self,
>>          bar: &Bar0,
>> -        dma_obj: &DmaObject,
>> +        dma_obj: &Coherent<[u8]>,
>>          target_mem: FalconMem,
>>          load_offsets: FalconDmaLoadTarget,
>>      ) -> Result {
>> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>>          fw: &F,
>>      ) -> Result {
>>          // Create DMA object with firmware content as the source of the DMA engine.
>> -        let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
>> +        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
>
> Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
> In `dma_wr` it breaks this up into 256 byte transfers. Since this
> no longer pads out to a page boundary, it means that it could now error
> (around "DMA transfer goes beyond range of DMA object") if the Dmem 
> section's size is not divisible by 256. But tbh, I find it odd that 
> `dma_wr` doesn't check that FalconDmaLoadTarget's length is a
> multiple of 256 anyway, because it looks like it'll write a bunch of
> unrelated bytes (since it rounds up to the nearest 256 to copy).
>
> Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
> by 256?
>
> For this series if for all firmwares it's divisible by 256 then I think
> it's fine to leave this as is for now, but I do find the lack of
> checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
> odd.

All coherent allocations are page-aligned (and use full pages), so we
are safe in terms of overflows.

Also `dma_wr` uses `div_ceil(256)` which will skip the last data block
entirely if it is not a multiple of 256. It might be a bit more robust
to explicitly check that the size is a multiple of 256 and return an
error if that is not the case indeed.

Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent

[Gary Guo]

On Thu Mar 26, 2026 at 3:04 PM GMT, Alexandre Courbot wrote:
> On Wed Mar 25, 2026 at 11:14 AM JST, Eliot Courtney wrote:
>> On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>> fulfill the same role.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>>  drivers/gpu/nova-core/falcon.rs | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>>> index 5bf8da8760bf..f6239c44dd80 100644
>>> --- a/drivers/gpu/nova-core/falcon.rs
>>> +++ b/drivers/gpu/nova-core/falcon.rs
>>> @@ -10,6 +10,7 @@
>>>          Device, //
>>>      },
>>>      dma::{
>>> +        Coherent,
>>>          DmaAddress,
>>>          DmaMask, //
>>>      },
>>> @@ -20,7 +21,6 @@
>>>  };
>>>  
>>>  use crate::{
>>> -    dma::DmaObject,
>>>      driver::Bar0,
>>>      falcon::hal::LoadMethod,
>>>      gpu::Chipset,
>>> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
>>>      fn dma_wr(
>>>          &self,
>>>          bar: &Bar0,
>>> -        dma_obj: &DmaObject,
>>> +        dma_obj: &Coherent<[u8]>,
>>>          target_mem: FalconMem,
>>>          load_offsets: FalconDmaLoadTarget,
>>>      ) -> Result {
>>> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>>>          fw: &F,
>>>      ) -> Result {
>>>          // Create DMA object with firmware content as the source of the DMA engine.
>>> -        let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
>>> +        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
>>
>> Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
>> In `dma_wr` it breaks this up into 256 byte transfers. Since this
>> no longer pads out to a page boundary, it means that it could now error
>> (around "DMA transfer goes beyond range of DMA object") if the Dmem 
>> section's size is not divisible by 256. But tbh, I find it odd that 
>> `dma_wr` doesn't check that FalconDmaLoadTarget's length is a
>> multiple of 256 anyway, because it looks like it'll write a bunch of
>> unrelated bytes (since it rounds up to the nearest 256 to copy).
>>
>> Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
>> by 256?
>>
>> For this series if for all firmwares it's divisible by 256 then I think
>> it's fine to leave this as is for now, but I do find the lack of
>> checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
>> odd.
>
> All coherent allocations are page-aligned (and use full pages), so we
> are safe in terms of overflows.

Let's not rely on this behaviour. There is no guarantee on what's at the end
of allocation whatsoever. There's no guarantee that it will be initialized.
Even with __GFP_ZERO only the size provided will be zeroed.

If the GPU is going to read beyond ranges covered by `Coherent` (not just rely
on the alignment), let's align up the allocation.

>
> Also `dma_wr` uses `div_ceil(256)` which will skip the last data block
> entirely if it is not a multiple of 256. It might be a bit more robust
> to explicitly check that the size is a multiple of 256 and return an
> error if that is not the case indeed.

div_ceil will not skip the last block, it will over-read beyond the end.
div_floor would have skipped the block.

Best,
Gary

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

[Miguel Ojeda]

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

Re: [PATCH 4/7] gpu: nova-core: falcon: use dma::Coherent

[Alexandre Courbot]

On Fri Mar 27, 2026 at 12:35 AM JST, Gary Guo wrote:
> On Thu Mar 26, 2026 at 3:04 PM GMT, Alexandre Courbot wrote:
>> On Wed Mar 25, 2026 at 11:14 AM JST, Eliot Courtney wrote:
>>> On Sat Mar 21, 2026 at 10:36 PM JST, Alexandre Courbot wrote:
>>>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>>>> fulfill the same role.
>>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> ---
>>>>  drivers/gpu/nova-core/falcon.rs | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>>>> index 5bf8da8760bf..f6239c44dd80 100644
>>>> --- a/drivers/gpu/nova-core/falcon.rs
>>>> +++ b/drivers/gpu/nova-core/falcon.rs
>>>> @@ -10,6 +10,7 @@
>>>>          Device, //
>>>>      },
>>>>      dma::{
>>>> +        Coherent,
>>>>          DmaAddress,
>>>>          DmaMask, //
>>>>      },
>>>> @@ -20,7 +21,6 @@
>>>>  };
>>>>  
>>>>  use crate::{
>>>> -    dma::DmaObject,
>>>>      driver::Bar0,
>>>>      falcon::hal::LoadMethod,
>>>>      gpu::Chipset,
>>>> @@ -636,7 +636,7 @@ pub(crate) fn pio_load<F: FalconFirmware<Target = E> + FalconPioLoadable>(
>>>>      fn dma_wr(
>>>>          &self,
>>>>          bar: &Bar0,
>>>> -        dma_obj: &DmaObject,
>>>> +        dma_obj: &Coherent<[u8]>,
>>>>          target_mem: FalconMem,
>>>>          load_offsets: FalconDmaLoadTarget,
>>>>      ) -> Result {
>>>> @@ -740,7 +740,7 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>>>>          fw: &F,
>>>>      ) -> Result {
>>>>          // Create DMA object with firmware content as the source of the DMA engine.
>>>> -        let dma_obj = DmaObject::from_data(dev, fw.as_slice())?;
>>>> +        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
>>>
>>> Is it guaranteed that fw.as_slice() is a multiple of 256 in size?
>>> In `dma_wr` it breaks this up into 256 byte transfers. Since this
>>> no longer pads out to a page boundary, it means that it could now error
>>> (around "DMA transfer goes beyond range of DMA object") if the Dmem 
>>> section's size is not divisible by 256. But tbh, I find it odd that 
>>> `dma_wr` doesn't check that FalconDmaLoadTarget's length is a
>>> multiple of 256 anyway, because it looks like it'll write a bunch of
>>> unrelated bytes (since it rounds up to the nearest 256 to copy).
>>>
>>> Maybe we should enforce that `FalconDmaLoadTarget` length is divisible
>>> by 256?
>>>
>>> For this series if for all firmwares it's divisible by 256 then I think
>>> it's fine to leave this as is for now, but I do find the lack of
>>> checking in `dma_wr` (or anywhere else for FalconDmaLoadTarget) a bit
>>> odd.
>>
>> All coherent allocations are page-aligned (and use full pages), so we
>> are safe in terms of overflows.
>
> Let's not rely on this behaviour. There is no guarantee on what's at the end
> of allocation whatsoever. There's no guarantee that it will be initialized.
> Even with __GFP_ZERO only the size provided will be zeroed.
>
> If the GPU is going to read beyond ranges covered by `Coherent` (not just rely
> on the alignment), let's align up the allocation.
>
>>
>> Also `dma_wr` uses `div_ceil(256)` which will skip the last data block
>> entirely if it is not a multiple of 256. It might be a bit more robust
>> to explicitly check that the size is a multiple of 256 and return an
>> error if that is not the case indeed.
>
> div_ceil will not skip the last block, it will over-read beyond the end.
> div_floor would have skipped the block.

Ooopsie yes, of course. Making `dma_wr` check that the data is a
multiple of 256 is the simplest, I'll send a patch for that (with maybe
some padding code as I think I remember Turing at least did not always
follow the 256-alignment requirement).