Re: [PATCH v3 1/5] gpu: nova-core: correct FRTS vidmem offset calculation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Brittle Logic / Incorrect Condition for 2MiB alignment

Pre-existing issues:
- [Critical] Architecture/Memory Layout Mismatch
--

--- Patch [1]: [PATCH v3 1/5] gpu: nova-core: correct FRTS vidmem offset calculation ---
commit 281515add8e49de6b8ca823d190ac6318796ffcf
Author: Eliot Courtney <ecourtney@nvidia.com>

gpu: nova-core: correct FRTS vidmem offset calculation

The commit updates how the FRTS vidmem offset is calculated for the FSP Chain
of Trust message. It calculates this offset using a new per-architecture
reserved size at the end of the framebuffer, plus the PMU reserved size.

> diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
> index 1475485bded32..30e53df81aa2f 100644
> --- a/drivers/gpu/nova-core/fsp.rs
> +++ b/drivers/gpu/nova-core/fsp.rs
> @@ -134,20 +134,31 @@ struct FspCotMessage {
>  }
>  
>  impl FspCotMessage {
> +    /// Computes the FRTS vidmem offset for the Chain-of-Trust message. It is measured from the end
> +    /// of the framebuffer.
> +    fn frts_vidmem_offset(hal: &dyn hal::FspHal, fb_layout: &FbLayout) -> Result<u64> {
> +        let mut offset = u64::from(hal.fb_end_reserved_size());
> +

[Severity: Critical]
This is a pre-existing issue, but by updating frts_vidmem_offset() to use
hal.fb_end_reserved_size() + pmu_reserved_size, there is now a mismatch with
how FbLayout::new() calculates the FRTS base address.

FbLayout::new() calculates frts_base by subtracting frts_size directly from
vga_workspace.start:

drivers/gpu/nova-core/fb.rs:FbLayout::new() {
    let frts = {
        ...
        let frts_size: u64 = hal.frts_size();
        let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - frts_size;
        ...
    }
}

This completely omits the PMU reserved memory space. For architectures like
GB202, FbLayout places FRTS ~2MiB from the end of the framebuffer, while the
FSP is told it is located ~28MiB from the end.

Could this discrepancy cause a regression where the host driver loads the GSP
firmware into physical memory locations that contradict the FSP's memory
protection firewall?

> +        if fb_layout.pmu_reserved_size != 0 {
> +            offset = (offset + u64::from(fb_layout.pmu_reserved_size))
> +                // The 2 MiB alignment is r570-specific.
> +                .align_up(Alignment::new::<SZ_2M>())
> +                .ok_or(EINVAL)?;
> +        }

[Severity: Medium]
Is it brittle to gate the 2MiB alignment behind fb_layout.pmu_reserved_size != 0?

The comment notes this alignment is r570-specific. Using the presence of a PMU
reservation as a proxy for checking if the firmware is r570 (or CoT version >= 2)
could cause a regression if a future architecture or firmware requires the
alignment but lacks a PMU reservation.

Could this proxy logic fail if an architecture has no PMU reservation but uses
r570-compatible firmware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-blackwell-fixes-v3-0-01aa6a54e3fd@nvidia.com?part=1