Re: [PATCH v5 6/6] gpu: nova-core: reserve vGPU WPR2 heap

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

Pre-existing issues:
- [High] Unchecked subtractions during framebuffer layout calculation can underflow if the device's video memory size is unexpectedly small.
--

--- Patch [6]: [PATCH v5 6/6] gpu: nova-core: reserve vGPU WPR2 heap ---
commit f23fa75b7168b0143fdda0d52c83938a849ca02d
Author: Zhi Wang <[email protected]>

gpu: nova-core: reserve vGPU WPR2 heap

This commit updates the framebuffer layout calculation to pass the detected
vGPU state. It keeps baremetal boots on the existing heap sizing path and
uses a firmware-dependent 570.144 vGPU default heap binding when vGPU is
enabled.

> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
[ ... ]
> @@ -234,10 +240,19 @@ pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Resu
>              FbRange(elf_addr..elf_addr + elf_size)
>          };
>  
> +        let (vf_partition_count, wpr2_heap_size) = match vgpu_state {
> +            VgpuState::Disabled => (
> +                0,
> +                gsp::LibosParams::from_chipset(chipset).wpr_heap_size(chipset, fb.end)?,
> +            ),
> +            VgpuState::Enabled { total_vfs } => (
> +                u8::try_from(total_vfs.get()).map_err(|_| EINVAL)?,
> +                gsp::LibosParams::vgpu_wpr_heap_size(),
> +            ),
> +        };
> +
>          let wpr2_heap = {
>              const WPR2_HEAP_DOWN_ALIGN: Alignment = Alignment::new::<SZ_1M>();
> -            let wpr2_heap_size =
> -                gsp::LibosParams::from_chipset(chipset).wpr_heap_size(chipset, fb.end)?;
>              let wpr2_heap_addr = (elf.start - wpr2_heap_size).align_down(WPR2_HEAP_DOWN_ALIGN);

[Severity: High]
This is a pre-existing issue, but could this subtraction underflow if the
hardware or a hypervisor reports a malformed, undersized framebuffer?

Since this patch introduces a hardcoded ~581MB requirement for
wpr2_heap_size in vGPU mode, the risk seems significantly amplified. If the
hardware-reported video memory size isn't large enough to contain the
bootloader, elf, and this new large heap, elf.start - wpr2_heap_size will
either panic or wrap around to a massive out-of-bounds address.

Should there be a validation check against the total fb_size before
allocating these ranges backwards from the end of the framebuffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6