Re: [PATCH 1/2] gpu: nova-core: check for overflow to DMATRFBASE1

[John Hubbard <jhubbard@nvidia.com>]

On 1/6/26 5:24 PM, Timur Tabi wrote:
> The NV_PFALCON_FALCON_DMATRFBASE/1 register pair supports DMA address
> up to 49 bits only, but the write to DMATRFBASE1 could exceed that.
> To mitigate, check first that the DMA address will fit.
> 
> Fixes: 69f5cd67ce41 ("gpu: nova-core: add falcon register definitions and base code")
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/falcon.rs | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 82c661aef594..fe5abf64dd2b 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -493,7 +493,11 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
>              Some(_) => (),
>          };
>  
> -        // Set up the base source DMA address.
> +        // Set up the base source DMA address.  DMATRFBASE only supports a 49-bit address.

Your commit log said it accurately, so I think we should use similar wording
here. So maybe:

   // Set up the base source DMA address.  The DMATRFBASE/1 register pair
   // only supports a 49-bit address.

However, one could argue that only the DMATRFBASE base matters
when considering overflow. But even so, it's slightly confusing
at first glance, to refer to a 32-bit register as only supporting
49-bit addresses. Of course yes, they can look at the register
which is shifted up, but still.

Either way is fine, I'm just fussing over nearly nothing here,
so:

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


thanks,
-- 
John Hubbard

> +        if dma_start > kernel::dma::DmaMask::new::<49>().value() {
> +            dev_err!(self.dev, "DMA address {:#x} exceeds 49 bits\n", dma_start);
> +            return Err(ERANGE);
> +        }
>  
>          regs::NV_PFALCON_FALCON_DMATRFBASE::default()
>              // CAST: `as u32` is used on purpose since we do want to strip the upper bits, which
> 
> base-commit: 4348796233e736147e2e79c58784d0a9fb48867d