Re: [PATCH v6 2/3] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind

[Alexandre Courbot]

On Thu Oct 15, 2026 at 12:45 AM JST, Vladislav Zaharov wrote:
> The GSP-RM log buffers are exposed through debugfs, but the Scope that
> owns them lives in Gsp, inside GspResources, inside the Gpu built by
> probe(). They are DMA allocations of the device and cannot outlive it,
> so the entries go away as soon as the GPU is unbound - and, more to the
> point, as soon as probe() fails, which is exactly when the log of a GSP
> that did not come up is the thing one wants to read.
>
> Add a gsp_keep_logs module parameter. When it is set, dropping the log
> buffers copies whatever the GSP wrote into memory owned by the module
> and exposes the copies until the module is unloaded. A buffer whose
> "put" pointer is still zero was never written to and is skipped.
>
> The GSP has normally been stopped by the time the buffers are dropped,
> but a boot that timed out can leave it still appending, so a DMA read
> barrier orders the read of the "put" pointer before the copy, and that
> value, not the one the copy reads again, is what the copy keeps.

That paragraph is nice as a code comment, but that's an implementation
detail that doesn't belong to a commit log. The commit log should tell
us *why* you are doing this, not go into excruciating details of *how*
you did it - only the big lines to make review easier.

I'd appreciate if you could do a pass on the commit logs and code
comments to deslop them and make them shorter and to the point
(similarly to what I tried to do on patch 1). Right now this patch
is pretty heavy to read.

>
> The copies belong to the module data next to the debugfs root, and live
> in a "retained" directory created during module init rather than on
> first use, which keeps the teardown path of a device from having to
> create anything. Keeping them out of the directory used by bound GPUs
> also means a device coming back does not find its debugfs name taken by
> its own history; nouveau, which recreates the entries under the name of
> the GPU that just went away, has that problem.

What Nouveau does is irrevelant - that's the kind of unneeeded
information I'd like us to avoid.

>
> While at it, move the log buffer code out of gsp.rs into gsp/logbuffer.rs.

"While at it" means this should be two patches: a first one that moves
the code into `gsp/logbuffer.rs`, a second one that performs the changes
you need there.

Right now this moves things while changing them, making it more
difficult to review than it could be.