linux-kernel - [PATCH v2] gpu: nova-core: falcon: align DMA transfers to 256 bytes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20250821-falcondma_256b-v2-1-83e8647a24b5@nvidia.com>
Date: Thu, 21 Aug 2025 13:49:32 +0900
From: Alexandre Courbot <acourbot@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>, 
 Simona Vetter <simona@...ll.ch>
Cc: nouveau@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, 
 rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Alexandre Courbot <acourbot@...dia.com>
Subject: [PATCH v2] gpu: nova-core: falcon: align DMA transfers to 256
 bytes

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@...dia.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@...dia.com>