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 <[email protected]>
> ---
> 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 <[email protected]>
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
