Re: [PATCH 8/8] gpu: nova-core: convert to new dma::Coherent API

[Alexandre Courbot]

On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
> From: Gary Guo <gary@xxxxxxxxxxx>
>
> Remove all usages of dma::CoherentAllocation and use the new
> dma::Coherent type instead.
>
> Note that there are still remainders of the old dma::CoherentAllocation
> API, such as as_slice() and as_slice_mut().
>
> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/dma.rs      | 19 +++++------
>  drivers/gpu/nova-core/falcon.rs   |  7 ++--
>  drivers/gpu/nova-core/firmware.rs | 10 ++----
>  drivers/gpu/nova-core/gsp.rs      | 18 ++++++----
>  drivers/gpu/nova-core/gsp/cmdq.rs | 55 ++++++++++++-------------------
>  5 files changed, 46 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
> index 7215398969da..3c19d5ffcfe8 100644
> --- a/drivers/gpu/nova-core/dma.rs
> +++ b/drivers/gpu/nova-core/dma.rs
> @@ -9,13 +9,13 @@
>  
>  use kernel::{
>      device,
> -    dma::CoherentAllocation,
> +    dma::Coherent,
>      page::PAGE_SIZE,
>      prelude::*, //
>  };
>  
>  pub(crate) struct DmaObject {
> -    dma: CoherentAllocation<u8>,
> +    dma: Coherent<[u8]>,
>  }

Actually, do we even still have a need for `DmaObject` now that
`Coherent` is available? It would be nice to check (as a follow-up
patch, I can look into it) whether we can remove that whole nova-local
`dma` module.

>  
>  impl DmaObject {
> @@ -24,23 +24,22 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Sel
>              .map_err(|_| EINVAL)?
>              .pad_to_align()
>              .size();
> -        let dma = CoherentAllocation::alloc_coherent(dev, len, GFP_KERNEL | __GFP_ZERO)?;
> +        let dma = Coherent::zeroed_slice(dev, len, GFP_KERNEL)?;
>  
>          Ok(Self { dma })
>      }
>  
>      pub(crate) fn from_data(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self> {
> -        Self::new(dev, data.len()).and_then(|mut dma_obj| {
> -            // SAFETY: We have just allocated the DMA memory, we are the only users and
> -            // we haven't made the device aware of the handle yet.
> -            unsafe { dma_obj.write(data, 0)? }
> -            Ok(dma_obj)
> -        })
> +        let dma_obj = Self::new(dev, data.len())?;
> +        // SAFETY: We have just allocated the DMA memory, we are the only users and
> +        // we haven't made the device aware of the handle yet.
> +        unsafe { dma_obj.as_mut()[..data.len()].copy_from_slice(data) };
> +        Ok(dma_obj)
>      }
>  }
>  
>  impl Deref for DmaObject {
> -    type Target = CoherentAllocation<u8>;
> +    type Target = Coherent<[u8]>;
>  
>      fn deref(&self) -> &Self::Target {
>          &self.dma
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 37bfee1d0949..39f5df568ddb 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -25,10 +25,7 @@
>      driver::Bar0,
>      falcon::hal::LoadMethod,
>      gpu::Chipset,
> -    num::{
> -        FromSafeCast,
> -        IntoSafeCast, //
> -    },
> +    num::FromSafeCast,
>      regs,
>      regs::macros::RegisterBase, //
>  };
> @@ -434,7 +431,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
>              }
>              FalconMem::Dmem => (
>                  0,
> -                fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
> +                fw.dma_handle() + DmaAddress::from(load_offsets.src_start),

Aren't we losing the bounds checking done by `dma_handle_with_offset`?

Mmm, but looking below we are checking that the end of the transfer
doesn't go beyond the object bounds, so that was probably redundant
anyway. Maybe we should do a `checked_add` still for safety.