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 <[email protected]> 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/[email protected]?part=1
