nouveau.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] gpu: nova-core: falcon: align DMA transfers to 256 bytes
@ 2025-08-21  4:49 Alexandre Courbot
  2025-08-21 13:26 ` Danilo Krummrich
  0 siblings, 1 reply; 2+ messages in thread
From: Alexandre Courbot @ 2025-08-21  4:49 UTC (permalink / raw)
  To: Danilo Krummrich, David Airlie, Simona Vetter
  Cc: nouveau, dri-devel, rust-for-linux, linux-kernel,
	Alexandre Courbot

Falcon DMA transfers are done in 256 bytes increments, and the method
responsible for initiating the transfer checked that the required length
was indeed a multiple of 256. While correct, this also requires callers
to specifically account for this limitation of DMA transfers, and we had
for instance the fwsec code performing a seemingly arbitrary (and
potentially overflowing) upwards alignment of the DMEM load size to
match this requirement.

Let's move that alignment into the loading code itself instead: since it
is working in terms of number of transfers, we can turn this upwards
alignment into a non-overflowing operation, and check that the requested
transfer remains into the limits of the DMA object. This also allows us
to remove a DMA-specific constant in the fwsec code.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
This came up as I was writing the next iteration of the `Alignment`
patchset: the alignment operation done in `fwsec.rs` would have required
an unsightly unwrap, so let's fix it beforehand.
---
Changes in v2:
- Remove `unsafe` block by checking transfer bounds ourselves.
- Link to v1: https://lore.kernel.org/r/20250808-falcondma_256b-v1-1-15f911d89ffd@nvidia.com
---
 drivers/gpu/nova-core/falcon.rs         | 31 ++++++++++++++++++++++---------
 drivers/gpu/nova-core/firmware/fwsec.rs |  9 +--------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index d235a6f9efca452cc46e2d13c61789eb00252de2..c71c1cb4144200a612cc6bd615ccc5d13192a209 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -463,14 +463,27 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
             );
             return Err(EINVAL);
         }
-        if load_offsets.len % DMA_LEN > 0 {
-            dev_err!(
-                self.dev,
-                "DMA transfer length must be a multiple of {}",
-                DMA_LEN
-            );
-            return Err(EINVAL);
-        }
+
+        // DMA transfers can only be done in units of 256 bytes. Compute how many such transfers we
+        // need to perform.
+        let num_transfers = load_offsets.len.div_ceil(DMA_LEN);
+
+        // Check that the area we are about to transfer is within the bounds of the DMA object.
+        // Upper limit of transfer is `(num_transfers * DMA_LEN) + load_offsets.src_start`.
+        match num_transfers
+            .checked_mul(DMA_LEN)
+            .and_then(|size| size.checked_add(load_offsets.src_start))
+        {
+            None => {
+                dev_err!(self.dev, "DMA transfer length overflow");
+                return Err(EOVERFLOW);
+            }
+            Some(upper_bound) if upper_bound as usize > fw.size() => {
+                dev_err!(self.dev, "DMA transfer goes beyond range of DMA object");
+                return Err(EINVAL);
+            }
+            Some(_) => (),
+        };
 
         // Set up the base source DMA address.
 
@@ -486,7 +499,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
             .set_imem(target_mem == FalconMem::Imem)
             .set_sec(if sec { 1 } else { 0 });
 
-        for pos in (0..load_offsets.len).step_by(DMA_LEN as usize) {
+        for pos in (0..num_transfers).map(|i| i * DMA_LEN) {
             // Perform a transfer of size `DMA_LEN`.
             regs::NV_PFALCON_FALCON_DMATRFMOFFS::default()
                 .set_offs(load_offsets.dst_start + pos)
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index 0dff3cfa90afee0cd4c3348023c8bfd7edccdb29..47f5e4524072888cc3f89520ff4beea766071958 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -202,9 +202,6 @@ pub(crate) struct FwsecFirmware {
     ucode: FirmwareDmaObject<Self, Signed>,
 }
 
-// We need to load full DMEM pages.
-const DMEM_LOAD_SIZE_ALIGN: u32 = 256;
-
 impl FalconLoadParams for FwsecFirmware {
     fn imem_load_params(&self) -> FalconLoadTarget {
         FalconLoadTarget {
@@ -218,11 +215,7 @@ fn dmem_load_params(&self) -> FalconLoadTarget {
         FalconLoadTarget {
             src_start: self.desc.imem_load_size,
             dst_start: self.desc.dmem_phys_base,
-            // TODO[NUMM]: replace with `align_up` once it lands.
-            len: self
-                .desc
-                .dmem_load_size
-                .next_multiple_of(DMEM_LOAD_SIZE_ALIGN),
+            len: self.desc.dmem_load_size,
         }
     }
 

---
base-commit: 0988099646cfc6c72a4448cad39d4ee22ad457a7
change-id: 20250808-falcondma_256b-5fb8a28ed274

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] gpu: nova-core: falcon: align DMA transfers to 256 bytes
  2025-08-21  4:49 [PATCH v2] gpu: nova-core: falcon: align DMA transfers to 256 bytes Alexandre Courbot
@ 2025-08-21 13:26 ` Danilo Krummrich
  0 siblings, 0 replies; 2+ messages in thread
From: Danilo Krummrich @ 2025-08-21 13:26 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: David Airlie, Simona Vetter, nouveau, dri-devel, rust-for-linux,
	linux-kernel

On Thu Aug 21, 2025 at 6:49 AM CEST, Alexandre Courbot wrote:
> Falcon DMA transfers are done in 256 bytes increments, and the method
> responsible for initiating the transfer checked that the required length
> was indeed a multiple of 256. While correct, this also requires callers
> to specifically account for this limitation of DMA transfers, and we had
> for instance the fwsec code performing a seemingly arbitrary (and
> potentially overflowing) upwards alignment of the DMEM load size to
> match this requirement.
>
> Let's move that alignment into the loading code itself instead: since it
> is working in terms of number of transfers, we can turn this upwards
> alignment into a non-overflowing operation, and check that the requested
> transfer remains into the limits of the DMA object. This also allows us
> to remove a DMA-specific constant in the fwsec code.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

Applied to nova-next, thanks!

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-21 13:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-21  4:49 [PATCH v2] gpu: nova-core: falcon: align DMA transfers to 256 bytes Alexandre Courbot
2025-08-21 13:26 ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).