Re: [PATCH v4 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers

[sashiko-bot@kernel.org]

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 <acourbot@nvidia.com>

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/20260629-nova-bootcontext-v4-0-5539d8469590@nvidia.com?part=3