Re: [PATCH v11 12/12] gpu: nova-core: use the Generic Bootloader to boot FWSEC on Turing

["Eliot Courtney" <ecourtney@nvidia.com>]

On Fri Mar 6, 2026 at 1:52 PM JST, Alexandre Courbot wrote:
> +/// Descriptor used by RM to figure out the requirements of the boot loader.
> +///
> +/// Most of its fields appear to be legacy and carry incorrect values, so they are left unused.
> +#[repr(C)]
> +#[derive(Debug, Clone)]
> +struct BootloaderDesc {
> +    /// Starting tag of bootloader.
> +    start_tag: u32,
> +    /// DMEM load offset - unused here as we always load at offset `0`.
> +    _dmem_load_off: u32,
> +    /// Offset of code section in the image. Unused as there is only one section in the bootloader
> +    /// binary.
> +    _code_off: u32,

I still think it would be slightly better to use this value, I posted
some more context here:
https://lore.kernel.org/all/DGXZCHSH4JPB.1ZZW2B72MHCMT@nvidia.com/

> +        // `BootloaderDmemDescV2` expects the source to be a mirror image of the destination
> +        // and uses the same offset parameter for both.
> +        //
> +        // Thus, the start of the source object needs to be padded with the difference betwen
> +        // the destination and source offsets.
> +        //
> +        // In practice, this is expected to always be zero but is required for code
> +        // correctness.
> +        let (align_padding, firmware_dma) = {
> +            let align_padding = {
> +                let imem_sec = firmware.imem_sec_load_params();
> +
> +                imem_sec
> +                    .dst_start
> +                    .checked_sub(imem_sec.src_start)
> +                    .map(usize::from_safe_cast)
> +                    .ok_or(EOVERFLOW)?
> +            };
> +
> +            let mut firmware_obj = KVVec::new();
> +            firmware_obj.extend_with(align_padding, 0u8, GFP_KERNEL)?;
> +            firmware_obj.extend_from_slice(firmware.ucode.0.as_slice(), GFP_KERNEL)?;
> +
> +            (
> +                align_padding,
> +                DmaObject::from_data(dev, firmware_obj.as_slice())?,
> +            )
> +        };
> +
> +        let dmem_desc = {
> +            // Bootloader payload is in non-coherent system memory.
> +            const FALCON_DMAIDX_PHYS_SYS_NCOH: u32 = 4;
> +
> +            let imem_sec = firmware.imem_sec_load_params();
> +            let imem_ns = firmware.imem_ns_load_params().ok_or(EINVAL)?;
> +            let dmem = firmware.dmem_load_params();
> +
> +            // The bootloader does not have a data destination offset field and copies the data at
> +            // the start of DMEM, so it can only be used if the destination offset of the firmware
> +            // is 0.
> +            if dmem.dst_start != 0 {
> +                return Err(EINVAL);
> +            }
> +
> +            BootloaderDmemDescV2 {
> +                reserved: [0; 4],
> +                signature: [0; 4],
> +                ctx_dma: FALCON_DMAIDX_PHYS_SYS_NCOH,
> +                code_dma_base: firmware_dma.dma_handle(),
> +                // `dst_start` is also valid as the source offset since the firmware DMA object is
> +                // a mirror image of the target IMEM layout.
> +                non_sec_code_off: imem_ns.dst_start,
> +                non_sec_code_size: imem_ns.len,
> +                // `dst_start` is also valid as the source offset since the firmware DMA object is
> +                // a mirror image of the target IMEM layout.
> +                sec_code_off: imem_sec.dst_start,

nit: it's incorrect to use `src_start` but the comment implies that it
would also be ok to use `src_start` "is also valid". IIUC we create the padded
firmware above (good catch on finding that!) and that uses `src_start`,
then since the falcon expects the same layout between the source
constructed image and the destination in its memory, it uses these
*_off values doubly to compute the source and destination addresses.
The aligning above is a way to make sure that `dst_start` can properly
perform this double duty. Some comment explaining this might be useful,
IMO.

Apart from that,
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>