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

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

[Vladislav Zaharov]

The GSP-RM log buffers are exposed through debugfs, but the entries are
owned by the Gpu that probe() builds, and the buffers themselves are DMA
allocations that cannot outlive the device. They are therefore gone as
soon as the GPU is unbound, and in particular as soon as probe() fails -
which is the case todo.rst singled out, and the one where a GSP log is
worth having.

Patch 1 is a fix this series needs: init() builds the Registration before
the guard that clears the debugfs root, so a registration that fails
leaves the root behind with nothing left to remove it, and the next load
finds the name taken. Patch 2 adds a gsp_keep_logs module parameter: when
it is set, whatever the GSP wrote is copied into memory owned by the
module and exposed under a "retained" directory until the module is
unloaded. Patch 3 drops the now completed task from todo.rst.

Changes since v2:
 - rebase onto current drm-rust-next, where Coherent carries the lifetime
   of the bound device; the live log buffers borrow the device rather
   than holding a reference to it, and only the copies keep one
 - make gsp_keep_logs a bool, now that module parameters support it
 - add the dma_rmb() discussed on v1, as dma_mb(Read), between reading
   the "put" pointer and copying the buffer
 - build the debugfs guard before the Registration (patch 1), so that a
   failed module init cannot leave either directory behind
 - keep nothing when CONFIG_DEBUG_FS is off, where a Dir is a zero-sized
   type and the copies could never be read back
 - take the snapshots before acquiring the global lock, rather than
   holding it across three 64 KiB allocations

Testing was done on top of drm-rust-next with the TLV firmware images
installed. On a GB203:

  - with gsp_keep_logs unset, no "retained" directory is created and the
    entries disappear on unbind, as before;
  - with gsp_keep_logs=1, retained/<BDF>/{loginit,logintr,logrm} hold
    the contents the live entries had, all 64 KiB of each readable;
  - binding the GPU again recreates the live entries without disturbing
    the copies, and unbinding it a second time replaces them, leaving
    exactly one set behind;
  - with a failure injected after the GSP has booted, probe() fails with
    -EINVAL, the driver stays unbound, and the logs of that attempt are
    still readable;
  - with a failure injected into the Registration instead, the module
    fails to load and leaves no debugfs directory behind, where before
    patch 1 the next load would have found the name taken;
  - the copies are released on module unload, and three load/unload
    cycles leave nothing behind;
  - 1, Y and y turn it on, 0 and N turn it off, and a value that is
    neither is refused at load time.

The parameter does need a value: the Rust bool param ops do not set
KERNEL_PARAM_OPS_FL_NOARG, so a bare gsp_keep_logs is refused, where the
C bool would have taken it.

No warnings, oopses or refcount complaints in dmesg throughout. Built and
checked with CLIPPY=1 and rustfmtcheck. checkpatch --strict is clean
apart from the MAINTAINERS note for the new file, which is already
covered by the existing "F: drivers/gpu/nova-core/" pattern.

John Hubbard's r000 series adds three more log buffers in gsp.rs.
Whichever of the two lands second needs a small rebase; the retained
copies extend to the new buffers by adding them to RetainedLogBuffers.

v2: https://lore.kernel.org/nova-gpu/20260815050826.306717-1-vladazaharova2018@gmail.com/
v1: https://lore.kernel.org/nova-gpu/20260812113752.532537-1-vladazaharova2018@gmail.com/

Vladislav Zaharov (3):
  gpu: nova-core: build the debugfs guard before registering the driver
  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 | 267 +++++++++++++++++++++++++
 drivers/gpu/nova-core/nova_core.rs     |  36 +++-
 4 files changed, 320 insertions(+), 95 deletions(-)
 create mode 100644 drivers/gpu/nova-core/gsp/logbuffer.rs


base-commit: 73e5616f3d197c1af5a04a481fe0f13aa3913bd1
-- 
2.55.0

[PATCH v3 1/3] gpu: nova-core: build the debugfs guard before registering the driver

[Vladislav Zaharov]

init() creates the debugfs root, hands it to a static, and leaves it to
DebugfsRootGuard to clear that static once the module goes away.

try_pin_init! builds fields in the order they are written, and an
initializer that fails drops only what it has already built. The guard
is written after the Registration, so a registration that fails leaves
it unbuilt and its drop never runs. Statics are not dropped either, and
the module is unloaded right after, so the "nova-core" directory outlives
everything that could remove it.

The next load then finds the name taken: debugfs_create_dir() returns
-EEXIST, which Entry keeps as it would any other pointer, and every
debugfs file of the driver silently fails to appear until the machine is
rebooted.

Build the guard first. Drops still run in declaration order, so the
driver is still unregistered before the guard clears the static.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Vladislav Zaharov <vladazaharova2018@gmail.com>
---
 drivers/gpu/nova-core/nova_core.rs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 1133c6ce5c55..11fe1d2858a9 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -47,7 +47,8 @@ fn drop(&mut self) {
 #[pin_data]
 struct NovaCoreModule {
     // Fields are dropped in declaration order, so `_driver` is dropped first,
-    // then `_debugfs_guard` clears `DEBUGFS_ROOT`.
+    // then `_debugfs_guard` clears `DEBUGFS_ROOT`. They are initialized the
+    // other way round, see `init()`.
     #[pin]
     _driver: Registration<pci::Adapter<driver::NovaCoreDriver>>,
     _debugfs_guard: DebugfsRootGuard,
@@ -61,9 +62,14 @@ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
         // cannot be any concurrent access to `DEBUGFS_ROOT`.
         unsafe { DEBUGFS_ROOT = Some(dir) };
 
+        // Fields are initialized in the order written here, and an initializer that fails drops
+        // what it has already built, so the guard goes first: should registration fail, its drop
+        // still takes `DEBUGFS_ROOT` down with it. Nothing would otherwise, as statics are never
+        // dropped and the module is unloaded right away, leaving a directory behind that the
+        // next load cannot create again.
         try_pin_init!(Self {
-            _driver <- Registration::new(MODULE_NAME, module),
             _debugfs_guard: DebugfsRootGuard,
+            _driver <- Registration::new(MODULE_NAME, module),
         })
     }
 }
-- 
2.55.0

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

[Vladislav Zaharov]

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.

The copies live in a "retained" directory, created during module init
rather than on first use, which keeps the teardown path from having to
reach for DEBUGFS_ROOT. 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.

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

Assisted-by: Claude:claude-opus-5
Signed-off-by: Vladislav Zaharov <vladazaharova2018@gmail.com>
---
 drivers/gpu/nova-core/gsp.rs           | 100 ++-------
 drivers/gpu/nova-core/gsp/logbuffer.rs | 267 +++++++++++++++++++++++++
 drivers/gpu/nova-core/nova_core.rs     |  32 ++-
 3 files changed, 315 insertions(+), 84 deletions(-)
 create mode 100644 drivers/gpu/nova-core/gsp/logbuffer.rs

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 25ea43f1cbe9..1a1eb7f37075 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -12,11 +12,7 @@
         CoherentView,
         DmaAddress, //
     },
-    io::{
-        io_project,
-        io_write,
-        Io, //
-    },
+    io::io_write,
     pci,
     prelude::*, //
 };
@@ -24,9 +20,13 @@
 pub(crate) mod cmdq;
 pub(crate) mod commands;
 mod fw;
+mod logbuffer;
 mod regs;
 mod sequencer;
 
+use logbuffer::LogBuffers;
+pub(crate) use logbuffer::RetainedLogs;
+
 pub(crate) use fw::{
     GspFmcBootParams,
     GspFwWprMeta,
@@ -77,10 +77,6 @@ pub(crate) fn dev(&self) -> &'gpu device::Device<device::Bound> {
     }
 }
 
-/// Number of GSP pages to use in a RM log buffer.
-const RM_LOG_BUFFER_NUM_PAGES: usize = 0x10;
-const LOG_BUFFER_SIZE: usize = RM_LOG_BUFFER_NUM_PAGES * GSP_PAGE_SIZE;
-
 /// Array of page table entries, as understood by the GSP bootloader.
 #[repr(C)]
 #[derive(FromBytes, IntoBytes)]
@@ -101,49 +97,6 @@ fn init(view: CoherentView<'_, Self>, start: DmaAddress) -> Result<()> {
     }
 }
 
-/// The logging buffers are byte queues that contain encoded printf-like
-/// messages from GSP-RM.  They need to be decoded by a special application
-/// that can parse the buffers.
-///
-/// The 'loginit' buffer contains logs from early GSP-RM init and
-/// exception dumps.  The 'logrm' buffer contains the subsequent logs. Both are
-/// written to directly by GSP-RM and can be any multiple of GSP_PAGE_SIZE.
-///
-/// The physical address map for the log buffer is stored in the buffer
-/// itself, starting with offset 1. Offset 0 contains the "put" pointer (pp).
-/// Initially, pp is equal to 0. If the buffer has valid logging data in it,
-/// then pp points to index into the buffer where the next logging entry will
-/// be written. Therefore, the logging data is valid if:
-///   1 <= pp < sizeof(buffer)/sizeof(u64)
-struct LogBuffer<'a>(Coherent<'a, [u8; LOG_BUFFER_SIZE]>);
-
-impl<'a> LogBuffer<'a> {
-    /// Creates a new `LogBuffer` mapped on `dev`.
-    fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
-        let obj = Self(Coherent::zeroed(dev, GFP_KERNEL)?);
-
-        let start_addr = obj.0.dma_address();
-
-        let pte_view = io_project!(
-            obj.0,
-            [build: size_of::<u64>()..][build: ..RM_LOG_BUFFER_NUM_PAGES * size_of::<u64>()]
-        )
-        .try_cast::<PteArray<RM_LOG_BUFFER_NUM_PAGES>>()?;
-        PteArray::init(pte_view, start_addr)?;
-
-        Ok(obj)
-    }
-}
-
-struct LogBuffers<'a> {
-    /// Init log buffer.
-    loginit: LogBuffer<'a>,
-    /// Interrupts log buffer.
-    logintr: LogBuffer<'a>,
-    /// RM log buffer.
-    logrm: LogBuffer<'a>,
-}
-
 /// GSP runtime data.
 #[pin_data]
 pub(crate) struct Gsp<'gsp> {
@@ -165,9 +118,7 @@ pub(crate) fn new(pdev: &'gsp pci::Device<device::Bound>) -> impl PinInit<Self,
         pin_init::pin_init_scope(move || {
             let dev = pdev.as_ref();
 
-            let loginit = LogBuffer::new(dev)?;
-            let logintr = LogBuffer::new(dev)?;
-            let logrm = LogBuffer::new(dev)?;
+            let log_buffers = LogBuffers::new(dev)?;
 
             // Initialise the logging structures. The OpenRM equivalents are in:
             // _kgspInitLibosLoggingStructures (allocates memory for buffers)
@@ -182,36 +133,23 @@ pub(crate) fn new(pdev: &'gsp pci::Device<device::Bound>) -> impl PinInit<Self,
                         GFP_KERNEL,
                     )?;
 
-                    libos.init_at(0, LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0))?;
-                    libos.init_at(1, LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0))?;
-                    libos.init_at(2, LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
+                    libos.init_at(
+                        0,
+                        LibosMemoryRegionInitArgument::new("LOGINIT", &log_buffers.loginit.0),
+                    )?;
+                    libos.init_at(
+                        1,
+                        LibosMemoryRegionInitArgument::new("LOGINTR", &log_buffers.logintr.0),
+                    )?;
+                    libos.init_at(
+                        2,
+                        LibosMemoryRegionInitArgument::new("LOGRM", &log_buffers.logrm.0),
+                    )?;
                     libos.init_at(3, LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
 
                     libos.into()
                 },
-                logs <- {
-                    let log_buffers = LogBuffers {
-                        loginit,
-                        logintr,
-                        logrm,
-                    };
-
-                    #[allow(static_mut_refs)]
-                    // SAFETY: `DEBUGFS_ROOT` is created before driver registration and cleared
-                    // after driver unregistration, so no probe() can race with its modification.
-                    //
-                    // PANIC: `DEBUGFS_ROOT` cannot be `None` here.  It is set before driver
-                    // registration and cleared after driver unregistration, so it is always
-                    // `Some` for the entire lifetime that probe() can be called.
-                    let log_parent: &debugfs::Dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }
-                        .expect("DEBUGFS_ROOT not initialized");
-
-                    log_parent.scope(log_buffers, dev.name(), |logs, dir| {
-                        dir.read_binary_file(c"loginit", &logs.loginit.0);
-                        dir.read_binary_file(c"logintr", &logs.logintr.0);
-                        dir.read_binary_file(c"logrm", &logs.logrm.0);
-                    })
-                },
+                logs <- log_buffers.scope(),
             }))
         })
     }
diff --git a/drivers/gpu/nova-core/gsp/logbuffer.rs b/drivers/gpu/nova-core/gsp/logbuffer.rs
new file mode 100644
index 000000000000..22c47f199169
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/logbuffer.rs
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! GSP-RM log buffers, and the debugfs entries exposing them.
+
+use core::convert::Infallible;
+
+use kernel::{
+    debugfs,
+    device,
+    dma::Coherent,
+    io::{
+        io_project,
+        Io, //
+    },
+    prelude::*,
+    sync::{
+        aref::ARef,
+        barrier::{
+            dma_mb,
+            Read, //
+        }, //
+    }, //
+};
+
+use crate::gsp::{
+    PteArray,
+    GSP_PAGE_SIZE, //
+};
+
+/// Number of GSP pages to use in a RM log buffer.
+const RM_LOG_BUFFER_NUM_PAGES: usize = 0x10;
+const LOG_BUFFER_SIZE: usize = RM_LOG_BUFFER_NUM_PAGES * GSP_PAGE_SIZE;
+
+/// The logging buffers are byte queues that contain encoded printf-like
+/// messages from GSP-RM.  They need to be decoded by a special application
+/// that can parse the buffers.
+///
+/// The 'loginit' buffer contains logs from early GSP-RM init and
+/// exception dumps.  The 'logrm' buffer contains the subsequent logs. Both are
+/// written to directly by GSP-RM and can be any multiple of GSP_PAGE_SIZE.
+///
+/// The physical address map for the log buffer is stored in the buffer
+/// itself, starting with offset 1. Offset 0 contains the "put" pointer (pp).
+/// Initially, pp is equal to 0. If the buffer has valid logging data in it,
+/// then pp points to index into the buffer where the next logging entry will
+/// be written. Therefore, the logging data is valid if:
+///   1 <= pp < sizeof(buffer)/sizeof(u64)
+pub(super) struct LogBuffer<'a>(pub(super) Coherent<'a, [u8; LOG_BUFFER_SIZE]>);
+
+impl<'a> LogBuffer<'a> {
+    /// Creates a new `LogBuffer` mapped on `dev`.
+    fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
+        let obj = Self(Coherent::zeroed(dev, GFP_KERNEL)?);
+
+        let start_addr = obj.0.dma_address();
+
+        let pte_view = io_project!(
+            obj.0,
+            [build: size_of::<u64>()..][build: ..RM_LOG_BUFFER_NUM_PAGES * size_of::<u64>()]
+        )
+        .try_cast::<PteArray<RM_LOG_BUFFER_NUM_PAGES>>()?;
+        PteArray::init(pte_view, start_addr)?;
+
+        Ok(obj)
+    }
+
+    /// Copies the contents of this buffer into memory that does not belong to the device.
+    ///
+    /// A buffer the GSP never wrote to yields an empty vector, as it holds nothing worth keeping.
+    fn snapshot(&self) -> Result<VVec<u8>> {
+        // Offset 0 holds the "put" pointer, which the GSP advances as it appends entries. It is
+        // still zero if nothing was ever logged, which is all that is tested here: a buffer that
+        // was written to is copied whole, and making sense of "put" is left to the decoder.
+        let put = io_project!(self.0, [build: ..size_of::<u64>()]).try_cast::<u64>()?;
+        if put.read_val() == 0 {
+            return Ok(VVec::new());
+        }
+
+        // ORDERING: LOAD->LOAD ordering needed to order the "put" read before the data read. The
+        // GSP has normally been stopped by the time this runs, but a boot that timed out can leave
+        // it still appending.
+        dma_mb(Read);
+
+        let mut snapshot = VVec::zeroed(LOG_BUFFER_SIZE, GFP_KERNEL)?;
+        io_project!(self.0, [build: ..]).copy_to_slice(&mut snapshot);
+
+        Ok(snapshot)
+    }
+}
+
+/// The log buffers of a GPU, for as long as it is bound to the driver.
+pub(super) struct LogBuffers<'a> {
+    /// Device the buffers belong to. Also names their debugfs directory.
+    dev: &'a device::Device<device::Bound>,
+    /// Init log buffer.
+    pub(super) loginit: LogBuffer<'a>,
+    /// Interrupts log buffer.
+    pub(super) logintr: LogBuffer<'a>,
+    /// RM log buffer.
+    pub(super) logrm: LogBuffer<'a>,
+}
+
+impl<'a> LogBuffers<'a> {
+    /// Allocates the three log buffers of `dev`.
+    pub(super) fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
+        Ok(Self {
+            dev,
+            loginit: LogBuffer::new(dev)?,
+            logintr: LogBuffer::new(dev)?,
+            logrm: LogBuffer::new(dev)?,
+        })
+    }
+
+    /// Creates an initializer exposing these buffers under a directory named after their device.
+    pub(super) fn scope(self) -> impl PinInit<debugfs::Scope<Self>, Infallible> + 'a {
+        let dev = self.dev;
+
+        #[allow(static_mut_refs)]
+        // SAFETY: `DEBUGFS_ROOT` is created before driver registration and cleared
+        // after driver unregistration, so no probe() can race with its modification.
+        //
+        // PANIC: `DEBUGFS_ROOT` cannot be `None` here.  It is set before driver
+        // registration and cleared after driver unregistration, so it is always
+        // `Some` for the entire lifetime that probe() can be called.
+        let log_parent: &debugfs::Dir =
+            unsafe { crate::DEBUGFS_ROOT.as_ref() }.expect("DEBUGFS_ROOT not initialized");
+
+        log_parent.scope(self, dev.name(), |logs, dir| {
+            dir.read_binary_file(c"loginit", &logs.loginit.0);
+            dir.read_binary_file(c"logintr", &logs.logintr.0);
+            dir.read_binary_file(c"logrm", &logs.logrm.0);
+        })
+    }
+
+    /// Preserves whatever the GSP logged, so it can still be read once the GPU is gone.
+    ///
+    /// The buffers are DMA allocations of the device and cannot outlive it, so their contents are
+    /// copied into memory owned by the module and exposed through fresh debugfs entries. Those
+    /// live until the module is unloaded.
+    ///
+    /// Does nothing if `gsp_keep_logs` was not set when the module was loaded, as there is then
+    /// no directory to put the copies in.
+    fn retain(&self) -> Result {
+        // Copying is only worth it if there is somewhere to put the result, but the lock is
+        // dropped right away: what follows allocates 64 KiB three times, and no other device
+        // should have to wait for that.
+        if !crate::RETAINED_LOGS.lock().is_enabled() {
+            return Ok(());
+        }
+
+        let logs = RetainedLogBuffers {
+            dev: self.dev.into(),
+            loginit: self.loginit.snapshot()?,
+            logintr: self.logintr.snapshot()?,
+            logrm: self.logrm.snapshot()?,
+        };
+
+        // Nothing was ever logged, so there is nothing to keep. A copy from an earlier run of
+        // this device is deliberately left alone: logs from a run that failed are worth more
+        // than the silence of one that did not.
+        if logs.loginit.is_empty() && logs.logintr.is_empty() && logs.logrm.is_empty() {
+            return Ok(());
+        }
+
+        // Take every allocation that can fail before the previous copy of this device is
+        // dropped, so that running out of memory here cannot leave it with no logs at all.
+        let scope = KBox::<debugfs::Scope<RetainedLogBuffers>>::new_uninit(GFP_KERNEL)?;
+
+        let mut retained = crate::RETAINED_LOGS.lock();
+
+        // The module may have been unloaded out from under us while the copies were taken.
+        let Some(dir) = retained.dir.clone() else {
+            return Ok(());
+        };
+
+        retained.gpus.reserve(1, GFP_KERNEL)?;
+
+        // An earlier run of the same device may have left a copy behind, and its directory
+        // carries the name about to be used again, so it has to go first. Nothing below can
+        // fail, so the replacement is guaranteed to take its place.
+        retained
+            .gpus
+            .retain(|gpu| gpu.dev.name() != self.dev.name());
+
+        let scope = scope.write_pin_init(dir.scope(logs, self.dev.name(), |logs, dir| {
+            if !logs.loginit.is_empty() {
+                dir.read_binary_file(c"loginit", &logs.loginit);
+            }
+            if !logs.logintr.is_empty() {
+                dir.read_binary_file(c"logintr", &logs.logintr);
+            }
+            if !logs.logrm.is_empty() {
+                dir.read_binary_file(c"logrm", &logs.logrm);
+            }
+        }))?;
+
+        retained.gpus.push(scope, GFP_KERNEL)?;
+
+        dev_dbg!(self.dev, "GSP-RM log buffers retained\n");
+
+        Ok(())
+    }
+}
+
+impl Drop for LogBuffers<'_> {
+    fn drop(&mut self) {
+        if let Err(e) = self.retain() {
+            dev_warn!(self.dev, "failed to retain GSP-RM log buffers: {:?}\n", e);
+        }
+    }
+}
+
+/// Copies of the log buffers of a GPU that is no longer around.
+struct RetainedLogBuffers {
+    /// Device the buffers came from.
+    dev: ARef<device::Device>,
+    /// Contents of the init log buffer, empty if it was never written to.
+    loginit: VVec<u8>,
+    /// Contents of the interrupts log buffer, empty if it was never written to.
+    logintr: VVec<u8>,
+    /// Contents of the RM log buffer, empty if it was never written to.
+    logrm: VVec<u8>,
+}
+
+/// Log buffers of GPUs that are gone, and the debugfs entries exposing them.
+///
+/// The copies live under a `retained` directory of their own instead of next to the entries of
+/// the GPUs that are actually bound, so that a device coming back does not find its name taken.
+pub(crate) struct RetainedLogs {
+    /// Parent directory of all copies. `None` unless retaining was asked for.
+    dir: Option<debugfs::Dir>,
+    /// One entry per GPU.
+    gpus: KVec<Pin<KBox<debugfs::Scope<RetainedLogBuffers>>>>,
+}
+
+impl RetainedLogs {
+    /// Creates an empty set of retained log buffers, retaining disabled.
+    pub(crate) const fn new() -> Self {
+        Self {
+            dir: None,
+            gpus: KVec::new(),
+        }
+    }
+
+    /// Creates the directory the copies will live in, enabling retaining.
+    ///
+    /// Does nothing without `CONFIG_DEBUG_FS`, where a [`debugfs::Dir`] is a zero-sized type and
+    /// the copies could never be read back.
+    pub(crate) fn enable(&mut self, parent: &debugfs::Dir) {
+        if !cfg!(CONFIG_DEBUG_FS) {
+            return;
+        }
+
+        self.dir = Some(parent.subdir(c"retained"));
+    }
+
+    /// Returns whether copies are being kept.
+    pub(crate) fn is_enabled(&self) -> bool {
+        self.dir.is_some()
+    }
+
+    /// Releases every copy and the directory holding them.
+    pub(crate) fn clear(&mut self) {
+        self.gpus.clear();
+        self.dir = None;
+    }
+}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 11fe1d2858a9..557cc611f3fc 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -33,11 +33,21 @@
 // TODO: Move this into per-module data once that exists.
 static mut DEBUGFS_ROOT: Option<debugfs::Dir> = None;
 
+kernel::sync::global_lock! {
+    /// Log buffers of GPUs that are gone, kept around until the module is unloaded.
+    // TODO: Move this into per-module data once that exists.
+    unsafe(uninit) static RETAINED_LOGS: Mutex<gsp::RetainedLogs> = gsp::RetainedLogs::new();
+}
+
 /// Guard that clears `DEBUGFS_ROOT` when dropped.
 struct DebugfsRootGuard;
 
 impl Drop for DebugfsRootGuard {
     fn drop(&mut self) {
+        // Retained log buffers own debugfs entries below `DEBUGFS_ROOT`, so they have to go away
+        // before it does.
+        RETAINED_LOGS.lock().clear();
+
         // SAFETY: This guard is dropped after `_driver` (due to field order),
         // so the driver is unregistered and no probe() can be running.
         unsafe { DEBUGFS_ROOT = None };
@@ -58,15 +68,25 @@ impl InPlaceModule for NovaCoreModule {
     fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
         let dir = debugfs::Dir::new(c"nova-core");
 
+        // SAFETY: Module initialization runs exactly once, and before the driver is registered,
+        // so no probe can have touched `RETAINED_LOGS` yet.
+        unsafe { RETAINED_LOGS.init() };
+
+        // Creating the directory up front is what makes retaining possible without reaching for
+        // `DEBUGFS_ROOT` later, from the teardown path of a device.
+        if module_parameters::gsp_keep_logs.value() {
+            RETAINED_LOGS.lock().enable(&dir);
+        }
+
         // SAFETY: We are the only driver code running during init, so there
         // cannot be any concurrent access to `DEBUGFS_ROOT`.
         unsafe { DEBUGFS_ROOT = Some(dir) };
 
         // Fields are initialized in the order written here, and an initializer that fails drops
         // what it has already built, so the guard goes first: should registration fail, its drop
-        // still takes `DEBUGFS_ROOT` down with it. Nothing would otherwise, as statics are never
-        // dropped and the module is unloaded right away, leaving a directory behind that the
-        // next load cannot create again.
+        // still takes `DEBUGFS_ROOT` and the retained copies down with it. Nothing would
+        // otherwise, as statics are never dropped and the module is unloaded right away, leaving
+        // directories behind that the next load cannot create again.
         try_pin_init!(Self {
             _debugfs_guard: DebugfsRootGuard,
             _driver <- Registration::new(MODULE_NAME, module),
@@ -81,6 +101,12 @@ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
     description: "Nova Core GPU driver",
     license: "GPL v2",
     firmware: [],
+    params: {
+        gsp_keep_logs: bool {
+            default: false,
+            description: "Keep the GSP-RM log buffers in debugfs after their GPU is gone",
+        },
+    },
 }
 
 kernel::module_firmware!(firmware::ModInfoBuilder);
-- 
2.55.0

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

[Gary Guo]

On Sat Sep 12, 2026 at 8:18 AM BST, 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.
>
> The copies live in a "retained" directory, created during module init
> rather than on first use, which keeps the teardown path from having to
> reach for DEBUGFS_ROOT. 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.
>
> While at it, move the log buffer code out of gsp.rs into gsp/logbuffer.rs.
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Vladislav Zaharov <vladazaharova2018@gmail.com>
> ---
>  drivers/gpu/nova-core/gsp.rs           | 100 ++-------
>  drivers/gpu/nova-core/gsp/logbuffer.rs | 267 +++++++++++++++++++++++++
>  drivers/gpu/nova-core/nova_core.rs     |  32 ++-
>  3 files changed, 315 insertions(+), 84 deletions(-)
>  create mode 100644 drivers/gpu/nova-core/gsp/logbuffer.rs
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 25ea43f1cbe9..1a1eb7f37075 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -12,11 +12,7 @@
>          CoherentView,
>          DmaAddress, //
>      },
> -    io::{
> -        io_project,
> -        io_write,
> -        Io, //
> -    },
> +    io::io_write,
>      pci,
>      prelude::*, //
>  };
> @@ -24,9 +20,13 @@
>  pub(crate) mod cmdq;
>  pub(crate) mod commands;
>  mod fw;
> +mod logbuffer;
>  mod regs;
>  mod sequencer;
>  
> +use logbuffer::LogBuffers;
> +pub(crate) use logbuffer::RetainedLogs;
> +
>  pub(crate) use fw::{
>      GspFmcBootParams,
>      GspFwWprMeta,
> @@ -77,10 +77,6 @@ pub(crate) fn dev(&self) -> &'gpu device::Device<device::Bound> {
>      }
>  }
>  
> -/// Number of GSP pages to use in a RM log buffer.
> -const RM_LOG_BUFFER_NUM_PAGES: usize = 0x10;
> -const LOG_BUFFER_SIZE: usize = RM_LOG_BUFFER_NUM_PAGES * GSP_PAGE_SIZE;
> -
>  /// Array of page table entries, as understood by the GSP bootloader.
>  #[repr(C)]
>  #[derive(FromBytes, IntoBytes)]
> @@ -101,49 +97,6 @@ fn init(view: CoherentView<'_, Self>, start: DmaAddress) -> Result<()> {
>      }
>  }
>  
> -/// The logging buffers are byte queues that contain encoded printf-like
> -/// messages from GSP-RM.  They need to be decoded by a special application
> -/// that can parse the buffers.
> -///
> -/// The 'loginit' buffer contains logs from early GSP-RM init and
> -/// exception dumps.  The 'logrm' buffer contains the subsequent logs. Both are
> -/// written to directly by GSP-RM and can be any multiple of GSP_PAGE_SIZE.
> -///
> -/// The physical address map for the log buffer is stored in the buffer
> -/// itself, starting with offset 1. Offset 0 contains the "put" pointer (pp).
> -/// Initially, pp is equal to 0. If the buffer has valid logging data in it,
> -/// then pp points to index into the buffer where the next logging entry will
> -/// be written. Therefore, the logging data is valid if:
> -///   1 <= pp < sizeof(buffer)/sizeof(u64)
> -struct LogBuffer<'a>(Coherent<'a, [u8; LOG_BUFFER_SIZE]>);
> -
> -impl<'a> LogBuffer<'a> {
> -    /// Creates a new `LogBuffer` mapped on `dev`.
> -    fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
> -        let obj = Self(Coherent::zeroed(dev, GFP_KERNEL)?);
> -
> -        let start_addr = obj.0.dma_address();
> -
> -        let pte_view = io_project!(
> -            obj.0,
> -            [build: size_of::<u64>()..][build: ..RM_LOG_BUFFER_NUM_PAGES * size_of::<u64>()]
> -        )
> -        .try_cast::<PteArray<RM_LOG_BUFFER_NUM_PAGES>>()?;
> -        PteArray::init(pte_view, start_addr)?;
> -
> -        Ok(obj)
> -    }
> -}
> -
> -struct LogBuffers<'a> {
> -    /// Init log buffer.
> -    loginit: LogBuffer<'a>,
> -    /// Interrupts log buffer.
> -    logintr: LogBuffer<'a>,
> -    /// RM log buffer.
> -    logrm: LogBuffer<'a>,
> -}
> -
>  /// GSP runtime data.
>  #[pin_data]
>  pub(crate) struct Gsp<'gsp> {
> @@ -165,9 +118,7 @@ pub(crate) fn new(pdev: &'gsp pci::Device<device::Bound>) -> impl PinInit<Self,
>          pin_init::pin_init_scope(move || {
>              let dev = pdev.as_ref();
>  
> -            let loginit = LogBuffer::new(dev)?;
> -            let logintr = LogBuffer::new(dev)?;
> -            let logrm = LogBuffer::new(dev)?;
> +            let log_buffers = LogBuffers::new(dev)?;
>  
>              // Initialise the logging structures. The OpenRM equivalents are in:
>              // _kgspInitLibosLoggingStructures (allocates memory for buffers)
> @@ -182,36 +133,23 @@ pub(crate) fn new(pdev: &'gsp pci::Device<device::Bound>) -> impl PinInit<Self,
>                          GFP_KERNEL,
>                      )?;
>  
> -                    libos.init_at(0, LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0))?;
> -                    libos.init_at(1, LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0))?;
> -                    libos.init_at(2, LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
> +                    libos.init_at(
> +                        0,
> +                        LibosMemoryRegionInitArgument::new("LOGINIT", &log_buffers.loginit.0),
> +                    )?;
> +                    libos.init_at(
> +                        1,
> +                        LibosMemoryRegionInitArgument::new("LOGINTR", &log_buffers.logintr.0),
> +                    )?;
> +                    libos.init_at(
> +                        2,
> +                        LibosMemoryRegionInitArgument::new("LOGRM", &log_buffers.logrm.0),
> +                    )?;
>                      libos.init_at(3, LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
>  
>                      libos.into()
>                  },
> -                logs <- {
> -                    let log_buffers = LogBuffers {
> -                        loginit,
> -                        logintr,
> -                        logrm,
> -                    };
> -
> -                    #[allow(static_mut_refs)]
> -                    // SAFETY: `DEBUGFS_ROOT` is created before driver registration and cleared
> -                    // after driver unregistration, so no probe() can race with its modification.
> -                    //
> -                    // PANIC: `DEBUGFS_ROOT` cannot be `None` here.  It is set before driver
> -                    // registration and cleared after driver unregistration, so it is always
> -                    // `Some` for the entire lifetime that probe() can be called.
> -                    let log_parent: &debugfs::Dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }
> -                        .expect("DEBUGFS_ROOT not initialized");
> -
> -                    log_parent.scope(log_buffers, dev.name(), |logs, dir| {
> -                        dir.read_binary_file(c"loginit", &logs.loginit.0);
> -                        dir.read_binary_file(c"logintr", &logs.logintr.0);
> -                        dir.read_binary_file(c"logrm", &logs.logrm.0);
> -                    })
> -                },
> +                logs <- log_buffers.scope(),
>              }))
>          })
>      }
> diff --git a/drivers/gpu/nova-core/gsp/logbuffer.rs b/drivers/gpu/nova-core/gsp/logbuffer.rs
> new file mode 100644
> index 000000000000..22c47f199169
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/logbuffer.rs
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! GSP-RM log buffers, and the debugfs entries exposing them.
> +
> +use core::convert::Infallible;
> +
> +use kernel::{
> +    debugfs,
> +    device,
> +    dma::Coherent,
> +    io::{
> +        io_project,
> +        Io, //
> +    },
> +    prelude::*,
> +    sync::{
> +        aref::ARef,
> +        barrier::{
> +            dma_mb,
> +            Read, //
> +        }, //
> +    }, //
> +};
> +
> +use crate::gsp::{
> +    PteArray,
> +    GSP_PAGE_SIZE, //
> +};
> +
> +/// Number of GSP pages to use in a RM log buffer.
> +const RM_LOG_BUFFER_NUM_PAGES: usize = 0x10;
> +const LOG_BUFFER_SIZE: usize = RM_LOG_BUFFER_NUM_PAGES * GSP_PAGE_SIZE;
> +
> +/// The logging buffers are byte queues that contain encoded printf-like
> +/// messages from GSP-RM.  They need to be decoded by a special application
> +/// that can parse the buffers.
> +///
> +/// The 'loginit' buffer contains logs from early GSP-RM init and
> +/// exception dumps.  The 'logrm' buffer contains the subsequent logs. Both are
> +/// written to directly by GSP-RM and can be any multiple of GSP_PAGE_SIZE.
> +///
> +/// The physical address map for the log buffer is stored in the buffer
> +/// itself, starting with offset 1. Offset 0 contains the "put" pointer (pp).
> +/// Initially, pp is equal to 0. If the buffer has valid logging data in it,
> +/// then pp points to index into the buffer where the next logging entry will
> +/// be written. Therefore, the logging data is valid if:
> +///   1 <= pp < sizeof(buffer)/sizeof(u64)
> +pub(super) struct LogBuffer<'a>(pub(super) Coherent<'a, [u8; LOG_BUFFER_SIZE]>);
> +
> +impl<'a> LogBuffer<'a> {
> +    /// Creates a new `LogBuffer` mapped on `dev`.
> +    fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
> +        let obj = Self(Coherent::zeroed(dev, GFP_KERNEL)?);
> +
> +        let start_addr = obj.0.dma_address();
> +
> +        let pte_view = io_project!(
> +            obj.0,
> +            [build: size_of::<u64>()..][build: ..RM_LOG_BUFFER_NUM_PAGES * size_of::<u64>()]
> +        )
> +        .try_cast::<PteArray<RM_LOG_BUFFER_NUM_PAGES>>()?;
> +        PteArray::init(pte_view, start_addr)?;
> +
> +        Ok(obj)
> +    }
> +
> +    /// Copies the contents of this buffer into memory that does not belong to the device.
> +    ///
> +    /// A buffer the GSP never wrote to yields an empty vector, as it holds nothing worth keeping.
> +    fn snapshot(&self) -> Result<VVec<u8>> {
> +        // Offset 0 holds the "put" pointer, which the GSP advances as it appends entries. It is
> +        // still zero if nothing was ever logged, which is all that is tested here: a buffer that
> +        // was written to is copied whole, and making sense of "put" is left to the decoder.
> +        let put = io_project!(self.0, [build: ..size_of::<u64>()]).try_cast::<u64>()?;
> +        if put.read_val() == 0 {
> +            return Ok(VVec::new());
> +        }
> +
> +        // ORDERING: LOAD->LOAD ordering needed to order the "put" read before the data read. The
> +        // GSP has normally been stopped by the time this runs, but a boot that timed out can leave
> +        // it still appending.
> +        dma_mb(Read);
> +
> +        let mut snapshot = VVec::zeroed(LOG_BUFFER_SIZE, GFP_KERNEL)?;
> +        io_project!(self.0, [build: ..]).copy_to_slice(&mut snapshot);
> +
> +        Ok(snapshot)
> +    }
> +}
> +
> +/// The log buffers of a GPU, for as long as it is bound to the driver.
> +pub(super) struct LogBuffers<'a> {
> +    /// Device the buffers belong to. Also names their debugfs directory.
> +    dev: &'a device::Device<device::Bound>,
> +    /// Init log buffer.
> +    pub(super) loginit: LogBuffer<'a>,
> +    /// Interrupts log buffer.
> +    pub(super) logintr: LogBuffer<'a>,
> +    /// RM log buffer.
> +    pub(super) logrm: LogBuffer<'a>,
> +}
> +
> +impl<'a> LogBuffers<'a> {
> +    /// Allocates the three log buffers of `dev`.
> +    pub(super) fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
> +        Ok(Self {
> +            dev,
> +            loginit: LogBuffer::new(dev)?,
> +            logintr: LogBuffer::new(dev)?,
> +            logrm: LogBuffer::new(dev)?,
> +        })
> +    }
> +
> +    /// Creates an initializer exposing these buffers under a directory named after their device.
> +    pub(super) fn scope(self) -> impl PinInit<debugfs::Scope<Self>, Infallible> + 'a {
> +        let dev = self.dev;
> +
> +        #[allow(static_mut_refs)]
> +        // SAFETY: `DEBUGFS_ROOT` is created before driver registration and cleared
> +        // after driver unregistration, so no probe() can race with its modification.
> +        //
> +        // PANIC: `DEBUGFS_ROOT` cannot be `None` here.  It is set before driver
> +        // registration and cleared after driver unregistration, so it is always
> +        // `Some` for the entire lifetime that probe() can be called.
> +        let log_parent: &debugfs::Dir =
> +            unsafe { crate::DEBUGFS_ROOT.as_ref() }.expect("DEBUGFS_ROOT not initialized");
> +
> +        log_parent.scope(self, dev.name(), |logs, dir| {
> +            dir.read_binary_file(c"loginit", &logs.loginit.0);
> +            dir.read_binary_file(c"logintr", &logs.logintr.0);
> +            dir.read_binary_file(c"logrm", &logs.logrm.0);
> +        })
> +    }
> +
> +    /// Preserves whatever the GSP logged, so it can still be read once the GPU is gone.
> +    ///
> +    /// The buffers are DMA allocations of the device and cannot outlive it, so their contents are
> +    /// copied into memory owned by the module and exposed through fresh debugfs entries. Those
> +    /// live until the module is unloaded.
> +    ///
> +    /// Does nothing if `gsp_keep_logs` was not set when the module was loaded, as there is then
> +    /// no directory to put the copies in.
> +    fn retain(&self) -> Result {
> +        // Copying is only worth it if there is somewhere to put the result, but the lock is
> +        // dropped right away: what follows allocates 64 KiB three times, and no other device
> +        // should have to wait for that.
> +        if !crate::RETAINED_LOGS.lock().is_enabled() {
> +            return Ok(());
> +        }
> +
> +        let logs = RetainedLogBuffers {
> +            dev: self.dev.into(),
> +            loginit: self.loginit.snapshot()?,
> +            logintr: self.logintr.snapshot()?,
> +            logrm: self.logrm.snapshot()?,
> +        };
> +
> +        // Nothing was ever logged, so there is nothing to keep. A copy from an earlier run of
> +        // this device is deliberately left alone: logs from a run that failed are worth more
> +        // than the silence of one that did not.
> +        if logs.loginit.is_empty() && logs.logintr.is_empty() && logs.logrm.is_empty() {
> +            return Ok(());
> +        }
> +
> +        // Take every allocation that can fail before the previous copy of this device is
> +        // dropped, so that running out of memory here cannot leave it with no logs at all.
> +        let scope = KBox::<debugfs::Scope<RetainedLogBuffers>>::new_uninit(GFP_KERNEL)?;
> +
> +        let mut retained = crate::RETAINED_LOGS.lock();
> +
> +        // The module may have been unloaded out from under us while the copies were taken.
> +        let Some(dir) = retained.dir.clone() else {
> +            return Ok(());
> +        };
> +
> +        retained.gpus.reserve(1, GFP_KERNEL)?;
> +
> +        // An earlier run of the same device may have left a copy behind, and its directory
> +        // carries the name about to be used again, so it has to go first. Nothing below can
> +        // fail, so the replacement is guaranteed to take its place.
> +        retained
> +            .gpus
> +            .retain(|gpu| gpu.dev.name() != self.dev.name());
> +
> +        let scope = scope.write_pin_init(dir.scope(logs, self.dev.name(), |logs, dir| {
> +            if !logs.loginit.is_empty() {
> +                dir.read_binary_file(c"loginit", &logs.loginit);
> +            }
> +            if !logs.logintr.is_empty() {
> +                dir.read_binary_file(c"logintr", &logs.logintr);
> +            }
> +            if !logs.logrm.is_empty() {
> +                dir.read_binary_file(c"logrm", &logs.logrm);
> +            }
> +        }))?;
> +
> +        retained.gpus.push(scope, GFP_KERNEL)?;
> +
> +        dev_dbg!(self.dev, "GSP-RM log buffers retained\n");
> +
> +        Ok(())
> +    }
> +}
> +
> +impl Drop for LogBuffers<'_> {
> +    fn drop(&mut self) {
> +        if let Err(e) = self.retain() {
> +            dev_warn!(self.dev, "failed to retain GSP-RM log buffers: {:?}\n", e);
> +        }
> +    }
> +}
> +
> +/// Copies of the log buffers of a GPU that is no longer around.
> +struct RetainedLogBuffers {
> +    /// Device the buffers came from.
> +    dev: ARef<device::Device>,
> +    /// Contents of the init log buffer, empty if it was never written to.
> +    loginit: VVec<u8>,
> +    /// Contents of the interrupts log buffer, empty if it was never written to.
> +    logintr: VVec<u8>,
> +    /// Contents of the RM log buffer, empty if it was never written to.
> +    logrm: VVec<u8>,
> +}
> +
> +/// Log buffers of GPUs that are gone, and the debugfs entries exposing them.
> +///
> +/// The copies live under a `retained` directory of their own instead of next to the entries of
> +/// the GPUs that are actually bound, so that a device coming back does not find its name taken.
> +pub(crate) struct RetainedLogs {
> +    /// Parent directory of all copies. `None` unless retaining was asked for.
> +    dir: Option<debugfs::Dir>,
> +    /// One entry per GPU.
> +    gpus: KVec<Pin<KBox<debugfs::Scope<RetainedLogBuffers>>>>,
> +}
> +
> +impl RetainedLogs {
> +    /// Creates an empty set of retained log buffers, retaining disabled.
> +    pub(crate) const fn new() -> Self {
> +        Self {
> +            dir: None,
> +            gpus: KVec::new(),
> +        }
> +    }
> +
> +    /// Creates the directory the copies will live in, enabling retaining.
> +    ///
> +    /// Does nothing without `CONFIG_DEBUG_FS`, where a [`debugfs::Dir`] is a zero-sized type and
> +    /// the copies could never be read back.
> +    pub(crate) fn enable(&mut self, parent: &debugfs::Dir) {
> +        if !cfg!(CONFIG_DEBUG_FS) {
> +            return;
> +        }
> +
> +        self.dir = Some(parent.subdir(c"retained"));
> +    }
> +
> +    /// Returns whether copies are being kept.
> +    pub(crate) fn is_enabled(&self) -> bool {
> +        self.dir.is_some()
> +    }
> +
> +    /// Releases every copy and the directory holding them.
> +    pub(crate) fn clear(&mut self) {
> +        self.gpus.clear();
> +        self.dir = None;
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index 11fe1d2858a9..557cc611f3fc 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -33,11 +33,21 @@
>  // TODO: Move this into per-module data once that exists.
>  static mut DEBUGFS_ROOT: Option<debugfs::Dir> = None;
>  
> +kernel::sync::global_lock! {
> +    /// Log buffers of GPUs that are gone, kept around until the module is unloaded.
> +    // TODO: Move this into per-module data once that exists.
> +    unsafe(uninit) static RETAINED_LOGS: Mutex<gsp::RetainedLogs> = gsp::RetainedLogs::new();

One global is already too many, we don't need more.

Consider instead to put everything that needs to be shared between multiple
devices in a single struct, and smuggle the pointer to it via a single global
`static mut` in place of today's `DEBUGFS_ROOT`. Then we just need unsafe in one
place.

Something like:

    static mut DEBUGFS_DATA: Option<&'static DebugfsData> = None;

    struct DebugfsData {
        root: debugfs::Dir,
        // You can put everything here, and still have initialized during module
        // init now..
        retained_logs: ...,
    }

    struct NovaCoreModule {
        ...
        // Put this last so it's destroyed last
        debugfs_data: DebugfsData,
    }

    // Module init
    fn init(..) -> impl PinInit<Self, Error> {
        try_pin_init!(Self {
            debugfs_data <- /* construct everything here, safely */
            _: {
                DEBUGFS_DATA = Some(unsafe { &*core::ptr::from_ref(debugfs_data) });
            },
            driver <- Registration::new(MODULE_NAME, module),
        }
    }

    // Probe
    fn probe<'bound>(pdev: &'bound Device<Core<'_>>, ...) -> ... {
        // Coerce this back to `&'bound` is okay because DEBUGFS_DATA outlives
        // registration and thus outlives bound device.
        let debugfs_data: &'bound _ = unsafe { DEBUGFS_DATA.unwrap() };
    }

Best,
Gary

> +}
> +
>  /// Guard that clears `DEBUGFS_ROOT` when dropped.
>  struct DebugfsRootGuard;
>  
>  impl Drop for DebugfsRootGuard {
>      fn drop(&mut self) {
> +        // Retained log buffers own debugfs entries below `DEBUGFS_ROOT`, so they have to go away
> +        // before it does.
> +        RETAINED_LOGS.lock().clear();
> +
>          // SAFETY: This guard is dropped after `_driver` (due to field order),
>          // so the driver is unregistered and no probe() can be running.
>          unsafe { DEBUGFS_ROOT = None };
> @@ -58,15 +68,25 @@ impl InPlaceModule for NovaCoreModule {
>      fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
>          let dir = debugfs::Dir::new(c"nova-core");
>  
> +        // SAFETY: Module initialization runs exactly once, and before the driver is registered,
> +        // so no probe can have touched `RETAINED_LOGS` yet.
> +        unsafe { RETAINED_LOGS.init() };
> +
> +        // Creating the directory up front is what makes retaining possible without reaching for
> +        // `DEBUGFS_ROOT` later, from the teardown path of a device.
> +        if module_parameters::gsp_keep_logs.value() {
> +            RETAINED_LOGS.lock().enable(&dir);
> +        }
> +
>          // SAFETY: We are the only driver code running during init, so there
>          // cannot be any concurrent access to `DEBUGFS_ROOT`.
>          unsafe { DEBUGFS_ROOT = Some(dir) };
>  
>          // Fields are initialized in the order written here, and an initializer that fails drops
>          // what it has already built, so the guard goes first: should registration fail, its drop
> -        // still takes `DEBUGFS_ROOT` down with it. Nothing would otherwise, as statics are never
> -        // dropped and the module is unloaded right away, leaving a directory behind that the
> -        // next load cannot create again.
> +        // still takes `DEBUGFS_ROOT` and the retained copies down with it. Nothing would
> +        // otherwise, as statics are never dropped and the module is unloaded right away, leaving
> +        // directories behind that the next load cannot create again.
>          try_pin_init!(Self {
>              _debugfs_guard: DebugfsRootGuard,
>              _driver <- Registration::new(MODULE_NAME, module),
> @@ -81,6 +101,12 @@ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
>      description: "Nova Core GPU driver",
>      license: "GPL v2",
>      firmware: [],
> +    params: {
> +        gsp_keep_logs: bool {
> +            default: false,
> +            description: "Keep the GSP-RM log buffers in debugfs after their GPU is gone",
> +        },
> +    },
>  }
>  
>  kernel::module_firmware!(firmware::ModInfoBuilder);

[PATCH v3 3/3] Documentation: nova: remove completed GSP log buffer task

[Vladislav Zaharov]

Exposing the GSP-RM log buffers through debugfs is implemented, and with
the gsp_keep_logs module parameter they now also survive a failed probe,
which was the part of the task that was still missing.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Vladislav Zaharov <vladazaharova2018@gmail.com>
---
 Documentation/gpu/nova/core/todo.rst | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst
index d5130b2b08fb..cae0578d32f1 100644
--- a/Documentation/gpu/nova/core/todo.rst
+++ b/Documentation/gpu/nova/core/todo.rst
@@ -141,18 +141,6 @@ Implement support for instmem (bar2) used to store page tables.
 GPU System Processor (GSP)
 ==========================
 
-Export GSP log buffers
-----------------------
-
-Recent patches from Timur Tabi [1] added support to expose GSP-RM log buffers
-(even after failure to probe the driver) through debugfs.
-
-This is also an interesting feature for nova-core, especially in the early days.
-
-| Link: https://lore.kernel.org/nouveau/20241030202952.694055-2-ttabi@nvidia.com/ [1]
-| Reference: Debugfs abstractions
-| Complexity: Intermediate
-
 GSP firmware abstraction
 ------------------------
 
-- 
2.55.0