Re: [PATCH v6 0/3] gpu: nova-core: retain the GSP-RM log buffers

[John Hubbard <jhubbard@nvidia.com>]

On 10/14/26 8:45 AM, Vladislav Zaharov wrote:
> The GSP-RM log buffers are exposed through debugfs, but they are DMA
> allocations of the device, so they go away as soon as the GPU is unbound,
> and in particular when probe() fails. That is the case todo.rst asks
> about, and the one where a GSP log is most useful.
> 
> Patch 1 moves the debugfs root into the module data. Patch 2 adds a
> gsp_keep_logs module parameter: when it is set, whatever the GSP wrote
> is copied into module memory and kept under a "retained" directory until
> the module is unloaded. Patch 3 drops the completed item from todo.rst.
> 
> v5 got no comments, so this version only carries one fix and a rebase.
> 
> Changes since v5:
>  - keep the "put" pointer read before the barrier in the copy, rather
>    than the one the copy reads again, which is not ordered against the
>    rest of the data (reported by the Sashiko review bot)
>  - rebase onto current drm-rust-next
> 
> Tested on a GB203 on top of drm-rust-next with the TLV firmware images:
> 
>   - without gsp_keep_logs nothing changes, and no "retained" directory
>     is created;
>   - with gsp_keep_logs=1, retained/<BDF>/{loginit,logintr,logrm} hold
>     the full 64 KiB of each buffer after unbind;

Although this "works", it has a significant usability issue that I think
we need not accept. We should fix it instead.

Specifically, (as you mention above), I had to add "retained/" to the
directory path in order to retrieve the logs. That is awkward, because
that means that if I ask for gsp_keep_logs=1, now I have to know/remember
that they are in:

    /sys/kernel/debug/nova-core/retained/<BDF>/

instead of:

    /sys/kernel/debug/nova-core/<BDF>

We should try hard to avoid such complications, especially given that
debugging is often done when one has a thousand other complicated things
going on at the same time, late at night, too.

I did a very quick LLM-powered fix on top of your series, and it seems to
work just fine without the "retained/" subdir.

Can you please do that? I can look more closely and provide details and/or
help, if necessary, I have the PoC diffs here.

Let me know how I can help.

thanks,
-- 
John Hubbard


>   - binding again does not disturb the copies, and a second unbind
>     replaces them, leaving one set;
>   - with a failure injected after the GSP has booted, probe() fails with
>     -EINVAL and the logs of that attempt are still readable;
>   - with a failure injected into the driver registration, the module
>     fails to load, leaves no debugfs directory behind, and the next load
>     works;
>   - repeated load/unload cycles leave nothing behind.
> 
> No warnings or oopses in dmesg. Each patch builds on its own, and the
> series is clean with CLIPPY=1, rustfmtcheck and checkpatch --strict
> (apart from the MAINTAINERS note for the new file, which is already
> covered by "F: drivers/gpu/nova-core/").
> 
> v5: https://lore.kernel.org/nova-gpu/20260923045551.229259-1-vladazaharova2018@gmail.com/
> 
> Vladislav Zaharov (3):
>   gpu: nova-core: move the debugfs root into the module data
>   gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind
>   Documentation: nova: remove completed GSP log buffer task
> 
>  Documentation/gpu/nova/core/todo.rst   |  12 --
>  drivers/gpu/nova-core/gsp.rs           | 100 ++--------
>  drivers/gpu/nova-core/gsp/logbuffer.rs | 255 +++++++++++++++++++++++++
>  drivers/gpu/nova-core/nova_core.rs     | 101 ++++++++--
>  4 files changed, 356 insertions(+), 112 deletions(-)
>  create mode 100644 drivers/gpu/nova-core/gsp/logbuffer.rs
> 
> 
> base-commit: 5a1b7121a6c4db6c0239d3ac3fd1c2bb33a40025