[PATCH 0/2] gpu: nova-core: falcon: use I/O projections for DMA transfers

[PATCH 0/2] gpu: nova-core: falcon: use I/O projections for DMA transfers

[Alexandre Courbot]

Falcon DMA transfers were using cumbersome handle arithmetic and
explicit bounds checks to ensure a requested transfer is valid. This
series replaces this with an I/O projection which, if successful,
carries the same bounds guarantees and does not require performing
operations on DMA handles.

The first patch removes a redundant check that is covered by the
register's `try_with_...` family of methods.

This patchset is based on top of drm-rust-next.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Alexandre Courbot (2):
      gpu: nova-core: falcon: remove unnecessary check
      gpu: nova-core: falcon: use I/O projection to check transfer bounds

 drivers/gpu/nova-core/falcon.rs | 64 ++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 40 deletions(-)
---
base-commit: 6dcbb4b1320fa91fee349462a52bb69135f2e45e
change-id: 20260724-falcon-dma-projections-ea9343cfcdae

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

[PATCH 1/2] gpu: nova-core: falcon: remove unnecessary check

[Alexandre Courbot]

The `try_with_base` call performed on `NV_PFALCON_FALCON_DMATRFBASE1`
already returns `EOVERFLOW` if the address is too large for the
register, making this check redundant.

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

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index a91cbdd5d636..cd05985f5ee6 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -9,8 +9,7 @@
     dma::{
         Coherent,
         CoherentBox,
-        DmaAddress,
-        DmaMask, //
+        DmaAddress, //
     },
     io::{
         poll::read_poll_timeout,
@@ -534,12 +533,6 @@ fn dma_wr(
             return Err(EINVAL);
         }
 
-        // The DMATRFBASE/1 register pair only supports a 49-bit address.
-        if dma_start > DmaMask::new::<49>().value() {
-            dev_err!(self.dev, "DMA address {:#x} exceeds 49 bits\n", dma_start);
-            return Err(ERANGE);
-        }
-
         // 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);

-- 
2.55.0

[PATCH 2/2] gpu: nova-core: falcon: use I/O projection to check transfer bounds

[Alexandre Courbot]

The DMA transfer routine was computing the start of the DMA area by
taking the handle to the beginning of the coherent allocation, and then
adding the transfer's start offset. It then checked manually that the
upper bound was valid.

Convert this to an I/O projection of the same region, which returns
`ERANGE` if the passed range does not fit within the coherent
allocation. This removes the need to perform arithmetic on DMA handles
and to explicitly check for the bounds' validity.

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

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index cd05985f5ee6..344cb1487295 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -12,6 +12,7 @@
         DmaAddress, //
     },
     io::{
+        io_project,
         poll::read_poll_timeout,
         register::{
             RegisterBase,
@@ -511,20 +512,31 @@ fn dma_wr(
     ) -> Result {
         const DMA_LEN: u32 = num::usize_into_u32::<{ MEM_BLOCK_ALIGNMENT }>();
 
+        // 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);
+
         // For IMEM, we want to use the start offset as a virtual address tag for each page, since
         // code addresses in the firmware (and the boot vector) are virtual.
         //
-        // For DMEM we can fold the start offset into the DMA handle.
+        // For DMEM, the start offset is folded into the DMA address.
         let (src_start, dma_start) = match target_mem {
-            FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
-                (load_offsets.src_start, dma_obj.dma_handle())
-            }
-            FalconMem::Dmem => (
-                0,
-                dma_obj.dma_handle() + DmaAddress::from(load_offsets.src_start),
-            ),
+            FalconMem::ImemSecure | FalconMem::ImemNonSecure => (load_offsets.src_start, 0),
+            FalconMem::Dmem => (0, usize::from_safe_cast(load_offsets.src_start)),
         };
-        if dma_start % DmaAddress::from(DMA_LEN) > 0 {
+
+        let dma_handle = {
+            // Upper limit of transfer is `(num_transfers * DMA_LEN) + load_offsets.src_start`.
+            let dma_end = num_transfers
+                .checked_mul(DMA_LEN)
+                .and_then(|size| size.checked_add(load_offsets.src_start))
+                .map(usize::from_safe_cast)
+                .ok_or(EOVERFLOW)?;
+
+            io_project!(dma_obj, [try: dma_start..dma_end]).dma_handle()
+        };
+
+        if dma_handle % DmaAddress::from(DMA_LEN) > 0 {
             dev_err!(
                 self.dev,
                 "DMA transfer start addresses must be a multiple of {}\n",
@@ -533,27 +545,6 @@ fn dma_wr(
             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\n");
-                return Err(EOVERFLOW);
-            }
-            Some(upper_bound) if usize::from_safe_cast(upper_bound) > dma_obj.size() => {
-                dev_err!(self.dev, "DMA transfer goes beyond range of DMA object\n");
-                return Err(EINVAL);
-            }
-            Some(_) => (),
-        };
-
         // Set up the base source DMA address.
 
         self.bar.write(
@@ -561,12 +552,12 @@ fn dma_wr(
             regs::NV_PFALCON_FALCON_DMATRFBASE::zeroed().with_base(
                 // CAST: `as u32` is used on purpose since we do want to strip the upper bits,
                 // which will be written to `NV_PFALCON_FALCON_DMATRFBASE1`.
-                (dma_start >> 8) as u32,
+                (dma_handle >> 8) as u32,
             ),
         );
         self.bar.write(
             WithBase::of::<E>(),
-            regs::NV_PFALCON_FALCON_DMATRFBASE1::zeroed().try_with_base(dma_start >> 40)?,
+            regs::NV_PFALCON_FALCON_DMATRFBASE1::zeroed().try_with_base(dma_handle >> 40)?,
         );
 
         let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::zeroed()

-- 
2.55.0

Re: [PATCH 2/2] gpu: nova-core: falcon: use I/O projection to check transfer bounds

[John Hubbard]

On 7/24/26 4:10 AM, Alexandre Courbot wrote:
> The DMA transfer routine was computing the start of the DMA area by
> taking the handle to the beginning of the coherent allocation, and then
> adding the transfer's start offset. It then checked manually that the
> upper bound was valid.
> 
> Convert this to an I/O projection of the same region, which returns
> `ERANGE` if the passed range does not fit within the coherent
> allocation. This removes the need to perform arithmetic on DMA handles
> and to explicitly check for the bounds' validity.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>   drivers/gpu/nova-core/falcon.rs | 55 +++++++++++++++++------------------------
>   1 file changed, 23 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index cd05985f5ee6..344cb1487295 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -12,6 +12,7 @@
>           DmaAddress, //
>       },
>       io::{
> +        io_project,
>           poll::read_poll_timeout,
>           register::{
>               RegisterBase,
> @@ -511,20 +512,31 @@ fn dma_wr(
>       ) -> Result {
>           const DMA_LEN: u32 = num::usize_into_u32::<{ MEM_BLOCK_ALIGNMENT }>();
>   
> +        // 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);
> +
>           // For IMEM, we want to use the start offset as a virtual address tag for each page, since
>           // code addresses in the firmware (and the boot vector) are virtual.
>           //
> -        // For DMEM we can fold the start offset into the DMA handle.
> +        // For DMEM, the start offset is folded into the DMA address.
>           let (src_start, dma_start) = match target_mem {
> -            FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
> -                (load_offsets.src_start, dma_obj.dma_handle())
> -            }
> -            FalconMem::Dmem => (
> -                0,
> -                dma_obj.dma_handle() + DmaAddress::from(load_offsets.src_start),
> -            ),
> +            FalconMem::ImemSecure | FalconMem::ImemNonSecure => (load_offsets.src_start, 0),
> +            FalconMem::Dmem => (0, usize::from_safe_cast(load_offsets.src_start)),
>           };
> -        if dma_start % DmaAddress::from(DMA_LEN) > 0 {
> +
> +        let dma_handle = {

Sort of "conceptually pre-existing" problem, but "handle" doesn't
quite work as a name, because handles are supposed to be opaque
items that one just uses to find and refer to things.


And below, that is violated:

> +            // Upper limit of transfer is `(num_transfers * DMA_LEN) + load_offsets.src_start`.
> +            let dma_end = num_transfers
> +                .checked_mul(DMA_LEN)
> +                .and_then(|size| size.checked_add(load_offsets.src_start))
> +                .map(usize::from_safe_cast)
> +                .ok_or(EOVERFLOW)?;
> +
> +            io_project!(dma_obj, [try: dma_start..dma_end]).dma_handle()
> +        };
> +
> +        if dma_handle % DmaAddress::from(DMA_LEN) > 0 {
>               dev_err!(
>                   self.dev,
>                   "DMA transfer start addresses must be a multiple of {}\n",
> @@ -533,27 +545,6 @@ fn dma_wr(
>               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\n");
> -                return Err(EOVERFLOW);
> -            }
> -            Some(upper_bound) if usize::from_safe_cast(upper_bound) > dma_obj.size() => {
> -                dev_err!(self.dev, "DMA transfer goes beyond range of DMA object\n");
> -                return Err(EINVAL);
> -            }
> -            Some(_) => (),
> -        };
> -
>           // Set up the base source DMA address.
>   
>           self.bar.write(
> @@ -561,12 +552,12 @@ fn dma_wr(
>               regs::NV_PFALCON_FALCON_DMATRFBASE::zeroed().with_base(
>                   // CAST: `as u32` is used on purpose since we do want to strip the upper bits,
>                   // which will be written to `NV_PFALCON_FALCON_DMATRFBASE1`.
> -                (dma_start >> 8) as u32,
> +                (dma_handle >> 8) as u32,

This is a "whaaat?" moment: shifting dma_start makes sense,
but shifting a handle does not.

Thoughts?


thanks,
-- 
John Hubbard

Re: [PATCH 2/2] gpu: nova-core: falcon: use I/O projection to check transfer bounds

[Danilo Krummrich]

On Fri Jul 24, 2026 at 8:10 PM CEST, John Hubbard wrote:
> Sort of "conceptually pre-existing" problem, but "handle" doesn't
> quite work as a name, because handles are supposed to be opaque
> items that one just uses to find and refer to things.

This is inherited from the C API where from its perspective the returned base
DMA address of the allocation actually serves as a handle.

In Rust the actual handle is represented by different types with different
invariants and hence capabilties.

(For instance, dma::CoherentBox does not expose the DMA address at all, and
hence provides access to its contents through a direct mutable reference,
whereas dma::CoherentHandle only provides access to the DMA address and does not
have a CPU virtual mapping; dma::Coherent provides access to both.)

Both dma::Coherent and dma::CoherentHandle have a dma_handle() method which
provides access to the DMA address; this naming was taken over from the C API.

Given that the base DMA address serving as a handle from the perspective of the
C API is just an implementation detail, we should probably just rename this
method to dma_address().

Re: [PATCH 2/2] gpu: nova-core: falcon: use I/O projection to check transfer bounds

[Alexandre Courbot]

On Sat Jul 25, 2026 at 7:27 AM JST, Danilo Krummrich wrote:
> On Fri Jul 24, 2026 at 8:10 PM CEST, John Hubbard wrote:
>> Sort of "conceptually pre-existing" problem, but "handle" doesn't
>> quite work as a name, because handles are supposed to be opaque
>> items that one just uses to find and refer to things.
>
> This is inherited from the C API where from its perspective the returned base
> DMA address of the allocation actually serves as a handle.
>
> In Rust the actual handle is represented by different types with different
> invariants and hence capabilties.
>
> (For instance, dma::CoherentBox does not expose the DMA address at all, and
> hence provides access to its contents through a direct mutable reference,
> whereas dma::CoherentHandle only provides access to the DMA address and does not
> have a CPU virtual mapping; dma::Coherent provides access to both.)
>
> Both dma::Coherent and dma::CoherentHandle have a dma_handle() method which
> provides access to the DMA address; this naming was taken over from the C API.
>
> Given that the base DMA address serving as a handle from the perspective of the
> C API is just an implementation detail, we should probably just rename this
> method to dma_address().

I was also thinking about using `dma_address`, so let's do that!

I guess the pieces required to support a proper DMA range type are also
starting to fall together.

Re: [PATCH 2/2] gpu: nova-core: falcon: use I/O projection to check transfer bounds

[Danilo Krummrich]

On Sat Jul 25, 2026 at 6:42 AM CEST, Alexandre Courbot wrote:
> On Sat Jul 25, 2026 at 7:27 AM JST, Danilo Krummrich wrote:
>> On Fri Jul 24, 2026 at 8:10 PM CEST, John Hubbard wrote:
>>> Sort of "conceptually pre-existing" problem, but "handle" doesn't
>>> quite work as a name, because handles are supposed to be opaque
>>> items that one just uses to find and refer to things.
>>
>> This is inherited from the C API where from its perspective the returned base
>> DMA address of the allocation actually serves as a handle.
>>
>> In Rust the actual handle is represented by different types with different
>> invariants and hence capabilties.
>>
>> (For instance, dma::CoherentBox does not expose the DMA address at all, and
>> hence provides access to its contents through a direct mutable reference,
>> whereas dma::CoherentHandle only provides access to the DMA address and does not
>> have a CPU virtual mapping; dma::Coherent provides access to both.)
>>
>> Both dma::Coherent and dma::CoherentHandle have a dma_handle() method which
>> provides access to the DMA address; this naming was taken over from the C API.
>>
>> Given that the base DMA address serving as a handle from the perspective of the
>> C API is just an implementation detail, we should probably just rename this
>> method to dma_address().
>
> I was also thinking about using `dma_address`, so let's do that!
>
> I guess the pieces required to support a proper DMA range type are also
> starting to fall together.

Yeah, I proposed a dma::Range type a couple of times, but never actually filed
an issue for it. I now added one in [1]; it should also cover getting rid of
dma_handle(), as it would be dma_range() instead. The corresponding address can
be derived from the Range instead.

[1] https://github.com/Rust-for-Linux/linux/issues/1248