Re: [PATCH v10 01/10] gpu: nova-core: create falcon firmware DMA objects lazily

["Alexandre Courbot" <acourbot@nvidia.com>]

On Fri Mar 6, 2026 at 10:41 AM JST, Eliot Courtney wrote:
<snip>
>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>> index 37bfee1d0949..8d444cf9d55c 100644
>> --- a/drivers/gpu/nova-core/falcon.rs
>> +++ b/drivers/gpu/nova-core/falcon.rs
>> @@ -2,12 +2,13 @@
>>  
>>  //! Falcon microprocessor base support
>>  
>> -use core::ops::Deref;
>> -
>>  use hal::FalconHal;
>>  
>>  use kernel::{
>> -    device,
>> +    device::{
>> +        self,
>> +        Device, //
>> +    },
>>      dma::{
>>          DmaAddress,
>>          DmaMask, //
>> @@ -15,9 +16,7 @@
>>      io::poll::read_poll_timeout,
>>      prelude::*,
>>      sync::aref::ARef,
>> -    time::{
>> -        Delta, //
>> -    },
>> +    time::Delta,
>
> nit: Missing // guard here.

IIUC the guard is only required to prevent rustfmt from reformatting,
which does not happen here.

<snip>
>> -impl FirmwareDmaObject<FwsecFirmware, Unsigned> {
>> -    fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Result<Self> {
>> +impl FirmwareObject<FwsecFirmware, Unsigned> {
>> +    fn new_fwsec(bios: &Vbios, cmd: FwsecCommand) -> Result<Self> {
>>          let desc = bios.fwsec_image().header()?;
>> -        let ucode = bios.fwsec_image().ucode(&desc)?;
>> -        let mut dma_object = DmaObject::from_data(dev, ucode)?;
>> +        let mut ucode = KVVec::new();
>> +        ucode.extend_from_slice(bios.fwsec_image().ucode(&desc)?, GFP_KERNEL)?;
>>  
>>          let hdr_offset = desc
>>              .imem_load_size()
>> @@ -269,8 +226,9 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
>>              .map(usize::from_safe_cast)
>>              .ok_or(EINVAL)?;
>>  
>> -        // SAFETY: we have exclusive access to `dma_object`.
>> -        let hdr: &FalconAppifHdrV1 = unsafe { transmute(&dma_object, hdr_offset) }?;
>> +        let hdr = FalconAppifHdrV1::from_bytes_prefix(&ucode[hdr_offset..])
>> +            .ok_or(EINVAL)?
>> +            .0;
>
> Is it worth adding // PANIC: comments like we have in some other areas
> of the codebase for each of these indexes into ucode?

Even better, we can do

    let hdr = ucode
                .get(hdr_offset..)
                .and_then(FalconAppifHdrV1::from_bytes_prefix)
                .ok_or(EINVAL)?
                .0;

and have the bounds checked at runtime, as they should be.

>
> Other than those two optional nits,
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

Thanks!