Re: [PATCH] gpu: nova-core: make pramin/buddy optional

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpu: nova-core: Gpu::new() always passes Some for total_fb_end to GpuMm::new()
--

commit c48efe432798819fed4e7632bc17f4fc720dd7e4
Author: Dave Airlie <airlied@redhat.com>

gpu: nova-core: make pramin/buddy optional

This commit updates GpuMm so that the GpuBuddy allocator and Pramin manager
are optional to support GPU systems with no framebuffer memory.

> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 4e59072cab507..5d53fde2cdc1e 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -564,18 +564,19 @@ pub(crate) fn new<'a>(
>  
>              // Create GPU memory manager owning memory management resources.
>              mm: {
> -                let usable_vram = gsp_static_info.usable_fb_regions.first().ok_or(ENODEV)?;
> -                let buddy_params = GpuBuddyParams {
> -                    base_offset: usable_vram.start,
> -                    size: usable_vram.end - usable_vram.start,
> -                    chunk_size: Alignment::new::<SZ_4K>(),
> -                };
> +                let buddy_params = gsp_static_info.usable_fb_regions.first().map(|usable_vram| {
> +                    GpuBuddyParams {
> +                        base_offset: usable_vram.start,
> +                        size: usable_vram.end - usable_vram.start,
> +                        chunk_size: Alignment::new::<SZ_4K>(),
> +                    }
> +                });
>  
>                  GpuMm::new(
>                      bar,
>                      gsp_resources.spec.chipset,
>                      buddy_params,
> -                    VramAddress::from_raw(gsp_static_info.total_fb_end),
> +                    Some(VramAddress::from_raw(gsp_static_info.total_fb_end)),
>                  )?
>              },

[Severity: Medium]
In Gpu::new(), the total_fb_end argument to GpuMm::new() is unconditionally
wrapped in Some(...). Since Gpu::new() is the only caller of GpuMm::new(),
won't GpuMm::new() always receive Some(...) for total_fb_end and
unconditionally call pramin::Pramin::new(), which writes the hardware BAR0
PRAMIN window base register via hal.write_pramin_window_base(bar, base) and
leaves GpuMm::pramin populated even on systems without framebuffer memory?

Also, in drivers/gpu/nova-core/gsp/commands.rs, GetGspStaticInfoReply::read()
still requires msg.total_fb_end() to be present when called from
Gpu::new() -> Gsp::get_static_info():

drivers/gpu/nova-core/gsp/commands.rs:GetGspStaticInfoReply::read() {
    ...
    let total_fb_end = msg.total_fb_end().ok_or(EINVAL)?;
    ...
}

When GSP reports no framebuffer regions, msg.total_fb_end() returns None,
causing GetGspStaticInfoReply::read() to fail probe with EINVAL before
GpuMm::new() is reached.

Should GetGspStaticInfoReply::total_fb_end be changed to Option<u64> (or
Option<VramAddress>) and passed through to GpuMm::new() without wrapping it
unconditionally in Some(...)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-nova-pramin-option-v1-1-d073b3f047fc@redhat.com?part=1