Re: [PATCH 2/4] gpu: nova-core: move GPU static information acquisition to a GSP method

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect drop order in `Gpu` struct causes sysmem flush page to be unregistered before GSP falcons are reset, leading to driver timeouts or hardware faults.
--

commit 70dc6b2e19cb0af3250719038111e9c9fcddf6e7
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: move GPU static information acquisition to a GSP method

The GSP static information is useful during regular driver runtime; however
it is currently obtained from Gsp::boot, with no elegant way to pass it back
to the caller. This moves the code acquiring it to a dedicated method of
Gsp that can be called as soon as the Gsp is booted.

> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 6b3e02c71dee1..a0cb36cdeeead 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
[ ... ]
> @@ -290,6 +291,8 @@ pub(crate) struct Gpu<'gpu> {
>      /// GSP and its resources.
>      #[pin]
>      gsp_resources: GspResources<'gpu>,

[Severity: High]
Does this struct layout result in an incorrect drop order?

In Rust, struct fields are dropped in their declaration order. Since
sysmem_flush is declared before gsp_resources in the Gpu struct, it will be
dropped first.

When sysmem_flush drops, its Drop implementation unregisters the memory flush
page by writing 0 to the hardware registers. Immediately after, gsp_resources
drops, which shuts down the GSP and resets the falcons.

However, the sysmem flush memory page must be registered before any falcon is
reset, because the reset handshake requires system memory writes. Dropping the
flush page too early means the subsequent falcon reset handshake will write
to address 0 or hang, potentially causing a driver timeout or IOMMU fault.

It appears the unload logic was moved to the GspResources subobject in this
patch series without reordering the struct fields, which causes the reset to
run after the flush page is already unregistered.

> +    /// Static GPU information as provided by the GSP.
> +    gsp_static_info: GetGspStaticInfoReply,
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-boot-vram-v1-0-d9382610507a@nvidia.com?part=2