Re: [PATCH 1/4] gpu: nova-core: move GSP unload state to a pinned Gpu subobject

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving the GSP unload logic into `GspResources` while leaving `sysmem_flush` declared before `gsp_resources` in `Gpu` causes the sysmem flush memory page to be unregistered before the GSP is shut down, leading to hardware faults or DMA use-after-free.
--

commit 77c9cd387aef2d5054c5d4097053c58f9a0d5c45
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: move GSP unload state to a pinned Gpu subobject

This commit moves the GSP state and unload bundle into a dedicated pinned
GspResources object to ensure that the GSP unload sequence is executed even
if a later Gpu field initialization fails.

> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index b3c91731db45d..6b3e02c71dee1 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs

[ ... ]

> +/// Structure holding the resources required to operate the GPU.
> +#[pin_data]
> +pub(crate) struct Gpu<'gpu> {
> +    spec: Spec,
> +    /// System memory page required for flushing all pending GPU-side memory writes done through
> +    /// PCIE into system memory, via sysmembar (A GPU-initiated HW memory-barrier operation).
> +    sysmem_flush: SysmemFlush<'gpu>,
> +    /// GSP and its resources.
> +    #[pin]
> +    gsp_resources: GspResources<'gpu>,
> +}

[Severity: High]
Since Rust drops struct fields in their declaration order, will sysmem_flush
be dropped before gsp_resources here?

If sysmem_flush drops first, it will unregister the system memory DMA page
by writing 0 to the hardware register. Next, when gsp_resources drops, its
PinnedDrop implementation will execute this.gsp.unload(...) which resets the
falcons.

Since the unload sequence strictly relies on the sysmembar flush page to
acknowledge resets and flush writes, won't this cause the hardware to attempt
to write to physical address 0 after the page has already been unregistered,
resulting in an IOMMU fault or driver timeout?

Should gsp_resources be declared before sysmem_flush to ensure the GSP is
shut down fully before the flush page is unregistered?

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