gpu: nova-core: falcon: align firmware DMA object size to required block alignment

[PATCH] gpu: nova-core: falcon: align firmware DMA object size to required block alignment

Posted by Alexandre Courbot 2 months, 1 week ago

Commit a88831502c8f ("gpu: nova-core: falcon: use dma::Coherent")
dropped the nova-local `DmaObject` device memory type for the
kernel-global `Coherent` one.

This switch had a side-effect: `DmaOject` always aligned the requested
size to `PAGE_SIZE`, and also reported that adjusted size when queried.
`Coherent`, on the other hand, does page-align allocation sizes but
only allows CPU access on the exact size provided by the caller.

This change runs into a limitation of falcon DMA copies, namely that DMA
accesses are done on blocks of exactly 256 bytes. If the provided data
does not have a length that is a multiple of 256, `dma_wr` returns
an error.

It was expected that all firmwares would present the proper adjusted
size, but this is not the case at least on my GA107:

    NovaCore 0000:08:00.0: DMA transfer goes beyond range of DMA object
    NovaCore 0000:08:00.0: Failed to load FWSEC firmware: EINVAL
    NovaCore 0000:08:00.0: probe with driver NovaCore failed with error -22

Fix this by aligning the `Coherent`'s size to `MEM_BLOCK_ALIGNMENT`
(i.e. 256) when allocating it and filling it with zeroes, before
copying the firmware on top of it.

Fixes: a88831502c8f ("gpu: nova-core: falcon: use dma::Coherent")
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/falcon.rs | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index e0315fda576b..eb0cf1f7329a 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -11,6 +11,7 @@
     },
     dma::{
         Coherent,
+        CoherentBox,
         DmaAddress,
         DmaMask, //
     },
@@ -613,8 +614,24 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
         bar: &Bar0,
         fw: &F,
     ) -> Result {
-        // Create DMA object with firmware content as the source of the DMA engine.
-        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
+        // DMA object with firmware content as the source of the DMA engine.
+        let dma_obj = {
+            let fw_slice = fw.as_slice();
+
+            // DMA copies are done in chunks of `MEM_BLOCK_ALIGNMENT`, so align the length
+            // accordingly and fill with `0`.
+            let mut dma_obj = CoherentBox::zeroed_slice(
+                dev,
+                fw_slice.len().next_multiple_of(MEM_BLOCK_ALIGNMENT),
+                GFP_KERNEL,
+            )?;
+
+            // PANIC: `dma_obj` has been created with a length equal to or larger than
+            // `fw_slice.len()`, so the range `0..fw_slice.len()` is valid.
+            dma_obj[0..fw_slice.len()].copy_from_slice(fw_slice);
+
+            dma_obj.into()
+        };
 
         self.dma_reset(bar);
         bar.update(regs::NV_PFALCON_FBIF_TRANSCFG::of::<E>().at(0), |v| {

---
base-commit: 7c50d748b4a635bc39802ea3f6b120e66b1b9067
change-id: 20260404-falcon-dma-roundup-0edd764a9840

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

Re: [PATCH] gpu: nova-core: falcon: align firmware DMA object size to required block alignment

Posted by John Hubbard 2 months, 1 week ago

On 4/4/26 5:14 AM, Alexandre Courbot wrote:
> Commit a88831502c8f ("gpu: nova-core: falcon: use dma::Coherent")
> dropped the nova-local `DmaObject` device memory type for the
> kernel-global `Coherent` one.

Hi Alex,

The diffs look good, so:

Reviewed-by: John Hubbard <jhubbard@nvidia.com>


A small typo in 3 places changes the meaning enough to throw
things off:

The subject line should s/align/pad/. Alignment normally means
adjusting the starting address, whereas padding is what you are
doing here.

Same in the comments below:

> 
> This switch had a side-effect: `DmaOject` always aligned the requested
> size to `PAGE_SIZE`, and also reported that adjusted size when queried.
> `Coherent`, on the other hand, does page-align allocation sizes but
> only allows CPU access on the exact size provided by the caller.
> 
> This change runs into a limitation of falcon DMA copies, namely that DMA
> accesses are done on blocks of exactly 256 bytes. If the provided data
> does not have a length that is a multiple of 256, `dma_wr` returns
> an error.
> 
> It was expected that all firmwares would present the proper adjusted
> size, but this is not the case at least on my GA107:
> 
>     NovaCore 0000:08:00.0: DMA transfer goes beyond range of DMA object
>     NovaCore 0000:08:00.0: Failed to load FWSEC firmware: EINVAL
>     NovaCore 0000:08:00.0: probe with driver NovaCore failed with error -22
> 
> Fix this by aligning the `Coherent`'s size to `MEM_BLOCK_ALIGNMENT`

"Fix this by padding"

> (i.e. 256) when allocating it and filling it with zeroes, before
> copying the firmware on top of it.
> 
> Fixes: a88831502c8f ("gpu: nova-core: falcon: use dma::Coherent")
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/falcon.rs | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index e0315fda576b..eb0cf1f7329a 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -11,6 +11,7 @@
>      },
>      dma::{
>          Coherent,
> +        CoherentBox,
>          DmaAddress,
>          DmaMask, //
>      },
> @@ -613,8 +614,24 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>          bar: &Bar0,
>          fw: &F,
>      ) -> Result {
> -        // Create DMA object with firmware content as the source of the DMA engine.
> -        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
> +        // DMA object with firmware content as the source of the DMA engine.
> +        let dma_obj = {
> +            let fw_slice = fw.as_slice();
> +
> +            // DMA copies are done in chunks of `MEM_BLOCK_ALIGNMENT`, so align the length

"pad the length"

> +            // accordingly and fill with `0`.
> +            let mut dma_obj = CoherentBox::zeroed_slice(
> +                dev,
> +                fw_slice.len().next_multiple_of(MEM_BLOCK_ALIGNMENT),
> +                GFP_KERNEL,
> +            )?;
> +
> +            // PANIC: `dma_obj` has been created with a length equal to or larger than
> +            // `fw_slice.len()`, so the range `0..fw_slice.len()` is valid.
> +            dma_obj[0..fw_slice.len()].copy_from_slice(fw_slice);
> +
> +            dma_obj.into()
> +        };
>  
>          self.dma_reset(bar);
>          bar.update(regs::NV_PFALCON_FBIF_TRANSCFG::of::<E>().at(0), |v| {
> 
> ---
> base-commit: 7c50d748b4a635bc39802ea3f6b120e66b1b9067
> change-id: 20260404-falcon-dma-roundup-0edd764a9840
> 
> Best regards,
> --  
> Alexandre Courbot <acourbot@nvidia.com>
> 

thanks,
-- 
John Hubbard

Re: [PATCH] gpu: nova-core: falcon: align firmware DMA object size to required block alignment

Posted by Alexandre Courbot 2 months, 1 week ago

On Sun Apr 5, 2026 at 7:24 AM JST, John Hubbard wrote:
> On 4/4/26 5:14 AM, Alexandre Courbot wrote:
>> Commit a88831502c8f ("gpu: nova-core: falcon: use dma::Coherent")
>> dropped the nova-local `DmaObject` device memory type for the
>> kernel-global `Coherent` one.
>
> Hi Alex,
>
> The diffs look good, so:
>
> Reviewed-by: John Hubbard <jhubbard@nvidia.com>
>
>
> A small typo in 3 places changes the meaning enough to throw
> things off:
>
> The subject line should s/align/pad/. Alignment normally means
> adjusting the starting address, whereas padding is what you are
> doing here.

Oooh indeed that's a bad one. Thanks for catching this - I'll send a v2
to make applying easier.

Re: [PATCH] gpu: nova-core: falcon: align firmware DMA object size to required block alignment

Posted by Gary Guo 2 months, 1 week ago

On Sat Apr 4, 2026 at 1:14 PM BST, Alexandre Courbot wrote:
> Commit a88831502c8f ("gpu: nova-core: falcon: use dma::Coherent")
> dropped the nova-local `DmaObject` device memory type for the
> kernel-global `Coherent` one.
>
> This switch had a side-effect: `DmaOject` always aligned the requested
> size to `PAGE_SIZE`, and also reported that adjusted size when queried.
> `Coherent`, on the other hand, does page-align allocation sizes but
> only allows CPU access on the exact size provided by the caller.
>
> This change runs into a limitation of falcon DMA copies, namely that DMA
> accesses are done on blocks of exactly 256 bytes. If the provided data
> does not have a length that is a multiple of 256, `dma_wr` returns
> an error.
>
> It was expected that all firmwares would present the proper adjusted
> size, but this is not the case at least on my GA107:
>
>     NovaCore 0000:08:00.0: DMA transfer goes beyond range of DMA object
>     NovaCore 0000:08:00.0: Failed to load FWSEC firmware: EINVAL
>     NovaCore 0000:08:00.0: probe with driver NovaCore failed with error -22
>
> Fix this by aligning the `Coherent`'s size to `MEM_BLOCK_ALIGNMENT`
> (i.e. 256) when allocating it and filling it with zeroes, before
> copying the firmware on top of it.
>
> Fixes: a88831502c8f ("gpu: nova-core: falcon: use dma::Coherent")
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

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

> ---
>  drivers/gpu/nova-core/falcon.rs | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index e0315fda576b..eb0cf1f7329a 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -11,6 +11,7 @@
>      },
>      dma::{
>          Coherent,
> +        CoherentBox,
>          DmaAddress,
>          DmaMask, //
>      },
> @@ -613,8 +614,24 @@ fn dma_load<F: FalconFirmware<Target = E> + FalconDmaLoadable>(
>          bar: &Bar0,
>          fw: &F,
>      ) -> Result {
> -        // Create DMA object with firmware content as the source of the DMA engine.
> -        let dma_obj = Coherent::from_slice(dev, fw.as_slice(), GFP_KERNEL)?;
> +        // DMA object with firmware content as the source of the DMA engine.
> +        let dma_obj = {
> +            let fw_slice = fw.as_slice();
> +
> +            // DMA copies are done in chunks of `MEM_BLOCK_ALIGNMENT`, so align the length
> +            // accordingly and fill with `0`.
> +            let mut dma_obj = CoherentBox::zeroed_slice(
> +                dev,
> +                fw_slice.len().next_multiple_of(MEM_BLOCK_ALIGNMENT),
> +                GFP_KERNEL,
> +            )?;
> +
> +            // PANIC: `dma_obj` has been created with a length equal to or larger than
> +            // `fw_slice.len()`, so the range `0..fw_slice.len()` is valid.
> +            dma_obj[0..fw_slice.len()].copy_from_slice(fw_slice);

NIT: could be `dma_obj[..fw_slice.len()]`.

Best,
Gary

> +
> +            dma_obj.into()
> +        };
>  
>          self.dma_reset(bar);
>          bar.update(regs::NV_PFALCON_FBIF_TRANSCFG::of::<E>().at(0), |v| {
>
> ---
> base-commit: 7c50d748b4a635bc39802ea3f6b120e66b1b9067
> change-id: 20260404-falcon-dma-roundup-0edd764a9840
>
> Best regards,
> --  
> Alexandre Courbot <acourbot@nvidia.com>