Re: [PATCH 10/13] gpu: nova-core: split FbLayout into FSP and non-FSP versions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The overall framebuffer size (`fb_size`) is calculated but omitted from `FbSizes`, causing the `fbSize` parameter in `GspFwWprMeta` to incorrectly default to 0 on FSP architectures.
--

commit e3787f5a66f5e8d087e62ad8eace078a355f8a6f
Author: Eliot Courtney <ecourtney@nvidia.com>

gpu: nova-core: split FbLayout into FSP and non-FSP versions

The commit splits `FbLayout` into `FbRanges` and `FbSizes` to distinguish
between pre and post FSP architectures. For post-FSP architectures, only
sizes are calculated and passed instead of ranges.

> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 725e428154cf0..facecb8b411fe 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
[ ... ]
> +/// Framebuffer region sizes needed for GSP-FMC boot.
> +#[derive(Debug)]
> +pub(crate) struct FbSizes {
> +    /// VGA workspace size, in bytes.
> +    pub(crate) vga_workspace_size: u64,
> +    /// FRTS size, in bytes.
> +    pub(crate) frts_size: u64,
> +    /// WPR2 heap size, in bytes.
> +    pub(crate) wpr2_heap_size: u64,
> +    /// Non-WPR heap size, in bytes.
> +    pub(crate) heap_size: u64,
> +    /// PMU reserved memory size, in bytes.
> +    pub(crate) pmu_reserved_size: u32,
> +    /// Number of VF partitions.
> +    pub(crate) vf_partition_count: u8,
> +}
> +
> +impl FbSizes {
> +    /// Computes the framebuffer region sizes for GSP-FMC boot.
> +    fn new(chipset: Chipset, bar: Bar0<'_>) -> Result<Self> {
> +        let hal = hal::fb_hal(chipset);
> +        let fb_size = hal.vidmem_size(bar);
> +
> +        Ok(Self {
> +            vga_workspace_size: u64::SZ_128K,
> +            frts_size: hal.frts_size(),
> +            wpr2_heap_size: gsp::LibosParams::from_chipset(chipset)
> +                .wpr_heap_size(chipset, fb_size)?,
> +            heap_size: u64::from(hal.non_wpr_heap_size()),
> +            pmu_reserved_size: hal.pmu_reserved_size(),
> +            vf_partition_count: 0,
> +        })
> +    }
> +}

[ ... ]
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index 4db0cfa4dc4d1..042b0122e98dc 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
[ ... ]
> +        let fields = match fb_info {
[ ... ]
> +            GspFbInfo::Sizes(sizes) => WprMetaFields {
> +                non_wpr_heap_size: sizes.heap_size,
> +                gsp_fw_heap_size: sizes.wpr2_heap_size,
> +                frts_size: sizes.frts_size,
> +                gsp_fw_heap_vf_partition_count: sizes.vf_partition_count,
> +                vga_workspace_size: sizes.vga_workspace_size,
> +                pmu_reserved_size: sizes.pmu_reserved_size,
> +                // When only sizes are supplied, offsets and several other parameters are not used.
> +                ..Default::default()
> +            },
> +        };
> +
[ ... ]
> -            gspFwHeapVfPartitionCount: fb_layout.vf_partition_count,
> -            fbSize: fb_layout.fb.end - fb_layout.fb.start,
[ ... ]
> +            gspFwHeapVfPartitionCount: fields.gsp_fw_heap_vf_partition_count,
> +            fbSize: fields.fb_size,

[Severity: Medium]
Since `fb_size` is calculated in `FbSizes::new()` but not saved into `FbSizes`,
`fields.fb_size` will be set to 0 via `..Default::default()` for FSP architectures.

Even though the comment mentions that "offsets and several other parameters are
not used" by FSP, can omitting `fb_size` here and passing `fbSize` as 0 in
`GspFwWprMeta` cause firmware initialization failures or memory mapping errors
if the firmware still relies on this field?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-blackwell-fixes-v1-0-f2853e49ff7d@nvidia.com?part=10