Re: [PATCH v5 05/13] gpu: nova-core: gsp: centralize missing unload bundle warnings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Diagnostic warnings about a missing unload bundle and the need to reset the GPU are silently dropped if `hal.boot()` fails after a failed unload bundle creation.
--

--- Patch [5]: [PATCH v5 05/13] gpu: nova-core: gsp: centralize missing unload bundle warnings ---
commit 5c2f2654dd3e426ece23d356efc67204a24f8fc0
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: gsp: centralize missing unload bundle warnings

This commit centralizes the warning emitted when the unload bundle cannot
be constructed by moving it from the Tu102-specific boot sequence to the
common Gsp::boot method.

> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index 75054424a9516..430ffc57b1366 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -56,7 +56,15 @@ pub(crate) fn boot(
>          let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>  
>          // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
> -        let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
> +        let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?.or_else(|| {
> +            dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
> +            dev_warn!(
> +                dev,
> +                "The GPU will need to be reset before the driver can bind again.\n"
> +            );
> +
> +            None
> +        });

[Severity: Medium]
Does the `?` operator here inadvertently skip these warnings if a subsequent
step in `hal.boot()` fails?

In the previous implementation, the warnings were printed immediately when
`Sec2UnloadBundle::build()` failed in `Tu102::boot()`.

With this change, if the unload bundle creation fails, but a subsequent
operation inside `hal.boot()` also fails and returns an `Err`, the `?`
operator will immediately propagate that error. This entirely bypasses the
`.or_else()` block, dropping the diagnostic warnings about the GPU needing
a reset.

> diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
> index c0956fb1c9cff..ef465b99af051 100644
> --- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
> +++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
[ ... ]
> @@ -278,14 +278,7 @@ fn boot(
>          // If the unload bundle creation fails, the GPU will need to be reset before the driver can
>          // be probed again.
>          let unload_bundle = Sec2UnloadBundle::build(dev, chipset, &bios, gsp_falcon, sec2_falcon)
> -            .inspect_err(|e| {
> -                dev_warn!(dev, "Failed to prepare unload firmware: {:?}\n", e);
> -                dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
> -                dev_warn!(
> -                    dev,
> -                    "The GPU will need to be reset before the driver can bind again.\n"
> -                );
> -            })
> +            .inspect_err(|e| dev_warn!(dev, "Failed to prepare unload firmware: {:?}\n", e))
>              .ok()
>              .map(crate::gsp::UnloadBundle);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-nova-bootcontext-v5-0-ecad9346387f@nvidia.com?part=5