Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The root cause error from `fsp.boot_fmc` is masked by a subsequent timeout error from `wait_for_gsp_lockdown_release`. --
commit 24f94e0b36617b6956b3b5c28c4a63019c91588f Author: Alexandre Courbot <[email protected]> gpu: nova-core: gsp: replace BootUnloadGuard with local handlers This commit removes the BootUnloadGuard abstraction, replacing it with a two-level error handling strategy. HALs handle their own errors and Gsp::boot uses a ScopeGuard to run Gsp::unload. > diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs > b/drivers/gpu/nova-core/gsp/hal/gh100.rs > --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs > +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs [ ... ] > @@ -174,11 +168,13 @@ fn boot<'a>( > false, > )?; > > - fsp.boot_fmc(dev, fb_layout, &args)?; > + // Keep the result as we want to wait for lockdown release even in > case of error, to make > + // sure args is not accessed by the GSP anymore. > + let res = fsp.boot_fmc(dev, fb_layout, &args); > > wait_for_gsp_lockdown_release(dev, gsp_falcon, > args.boot_params_dma_handle())?; [Severity: Medium] Will this mask the original error if fsp.boot_fmc() fails? If fsp.boot_fmc() fails, its error is stored in res. The function then calls wait_for_gsp_lockdown_release() to ensure the boot arguments are no longer accessed. Because the FSP boot failed, the lockdown release polling will inevitably time out. The ? operator on wait_for_gsp_lockdown_release() will propagate this timeout error immediately, causing the function to return early and silently discard the original root cause error held in res. > > - Ok(unload_guard) > + res.map(|()| Some(unload_bundle)) > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
