[PATCH v4 1/3] gpu: nova-core: move the debugfs root into the module data

[Vladislav Zaharov <vladazaharova2018@gmail.com>]

The debugfs root lives in a static that init() fills in and a guard
field of the module data clears again. That costs a `static mut`, an
unsafe write on each side and a guard type whose only job is to undo
the write.

It also leaks. try_pin_init! drops only the fields it has already
built, and the guard is written after the Registration, so a
registration that fails leaves the guard unbuilt and the static set.
Statics are never dropped, and the module is unloaded right away, so
the 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 the driver comes up with
no debugfs at all until the machine is rebooted.

Have the module data own a DebugfsData instead, built before the
registration and dropped after it, and keep only a pointer to it in the
static, for devices that have no other way to reach the data of their
module. What is left is one unsafe read for the users and one write on
each side, with no guard type. A registration that fails now drops the
data that was built before it, and the directory goes with it.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Vladislav Zaharov <vladazaharova2018@gmail.com>
---
 drivers/gpu/nova-core/gsp.rs       | 15 +++---
 drivers/gpu/nova-core/nova_core.rs | 82 +++++++++++++++++++++++-------
 2 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 25ea43f1cbe9..f29e601e6753 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -196,15 +196,12 @@ pub(crate) fn new(pdev: &'gsp pci::Device<device::Bound>) -> impl PinInit<Self,
                         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");
+                    // PANIC: The module data cannot be gone here. It is published before the
+                    // driver is registered and taken down after it is unregistered, so it is
+                    // there for as long as probe() can be called.
+                    let log_parent: &debugfs::Dir = crate::debugfs_data()
+                        .expect("module data not initialized")
+                        .root();
 
                     log_parent.scope(log_buffers, dev.name(), |logs, dir| {
                         dir.read_binary_file(c"loginit", &logs.loginit.0);
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 1133c6ce5c55..0f8501c26e05 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -30,40 +30,84 @@
 
 pub(crate) const MODULE_NAME: &core::ffi::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 
-// TODO: Move this into per-module data once that exists.
-static mut DEBUGFS_ROOT: Option<debugfs::Dir> = None;
+/// Pointer to the [`DebugfsData`] the module owns.
+///
+/// A device has no way to reach the data of its module, so probe() goes through here instead.
+// TODO: Drop this once devices can reach the data of their module.
+static mut DEBUGFS_DATA: *const DebugfsData = core::ptr::null();
 
-/// Guard that clears `DEBUGFS_ROOT` when dropped.
-struct DebugfsRootGuard;
+/// Data the module shares with every GPU it drives.
+///
+/// # Invariants
+///
+/// A non-null `DEBUGFS_DATA` points at a live, pinned instance of this type that outlives the
+/// driver registration.
+#[pin_data(PinnedDrop)]
+pub(crate) struct DebugfsData {
+    /// Root directory of the driver in debugfs.
+    root: debugfs::Dir,
+}
+
+impl DebugfsData {
+    /// Creates the shared data and publishes it, so that [`debugfs_data()`] can hand it out.
+    fn new() -> impl PinInit<Self> {
+        pin_init!(&this in Self {
+            root: debugfs::Dir::new(c"nova-core"),
+            _: {
+                // SAFETY: Module initialization runs once and before the driver is registered, so
+                // nothing can be reading `DEBUGFS_DATA` while it is written here. `this` is where
+                // the data is being built, and it stays there: the module data never moves.
+                unsafe { DEBUGFS_DATA = this.as_ptr() };
+            },
+        })
+    }
+
+    /// Returns the root directory of the driver in debugfs.
+    pub(crate) fn root(&self) -> &debugfs::Dir {
+        &self.root
+    }
+}
 
-impl Drop for DebugfsRootGuard {
-    fn drop(&mut self) {
-        // 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 };
+#[pinned_drop]
+impl PinnedDrop for DebugfsData {
+    fn drop(self: Pin<&mut Self>) {
+        // SAFETY: This runs after the registration is dropped, as the fields of `NovaCoreModule`
+        // are dropped in declaration order, so the driver is unregistered and neither a probe()
+        // nor the teardown of a device can be reading `DEBUGFS_DATA`.
+        unsafe { DEBUGFS_DATA = core::ptr::null() };
     }
 }
 
+/// Returns the data the module shares with its devices, or [`None`] if there is none yet.
+///
+/// Only ever call this while the driver is registered, which is to say from probe() or from the
+/// teardown of a device that is bound: the data is built before the registration and dropped
+/// after it, and nothing else keeps what is returned here alive.
+pub(crate) fn debugfs_data() -> Option<&'static DebugfsData> {
+    // SAFETY: `DEBUGFS_DATA` is written while the module data is initialized, before the driver
+    // is registered, and again when that data is dropped, after the driver is unregistered. Both
+    // happen with no device bound, so a caller in probe() or in the teardown of a device cannot
+    // race with either, and by the type invariant what it gets points at live data that outlives
+    // the device it is used from.
+    unsafe { DEBUGFS_DATA.as_ref() }
+}
+
 #[pin_data]
 struct NovaCoreModule {
-    // Fields are dropped in declaration order, so `_driver` is dropped first,
-    // then `_debugfs_guard` clears `DEBUGFS_ROOT`.
+    // Fields are dropped in declaration order, so the registration goes first and no probe() can
+    // still be running once the shared data is torn down. `init()` builds them the other way
+    // round, as the data has to be there before the first probe() reaches for it.
     #[pin]
     _driver: Registration<pci::Adapter<driver::NovaCoreDriver>>,
-    _debugfs_guard: DebugfsRootGuard,
+    #[pin]
+    _debugfs: DebugfsData,
 }
 
 impl InPlaceModule for NovaCoreModule {
     fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
-        let dir = debugfs::Dir::new(c"nova-core");
-
-        // 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) };
-
         try_pin_init!(Self {
+            _debugfs <- DebugfsData::new(),
             _driver <- Registration::new(MODULE_NAME, module),
-            _debugfs_guard: DebugfsRootGuard,
         })
     }
 }
-- 
2.55.0