public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs
@ 2026-01-13 22:53 Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 1/9] rust: pci: add PCI device name method Timur Tabi
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:53 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

GSP-RM writes its printf message to "logging buffers", which are blocks
memory allocated by the driver.  The messages are encoded, so exposing
the buffers as debugfs entries allows the buffers to be extracted and
decoded by a special application.

When the driver loads, a /sys/kernel/debug/nova_core root entry is
created.  To do this, the normal module_pci_driver! macro call is
replaced with an explicit initialization function, as this allows
that debugfs entry to be created once for all GPUs.

Then in each GPU's initialization, a subdirectory based on the PCI
BDF name is created, and the logging buffer entries are created under
that.

Note: the debugfs entry has a file size of 0, because debugfs defaults
a 0 size and the Rust abstractions do not adjust it for the same of
the object.  Nouveau makes this adjustment manually in the driver.

Changes since v3:
1. Implement a LookupDir struct instead of adding lookup methods to Dir.
2. Adds a Directory trait that is implemented by both LookupDir and Dir.
3. Creates DynParent enum to wrap both LookupDir and Dir.
4. No longer tries to "fix" Scope or scoped_dir.

Alexandre Courbot (1):
  gpu: nova-core: implement BinaryWriter for LogBuffer

Timur Tabi (8):
  rust: pci: add PCI device name method
  gpu: nova-core: Replace module_pci_driver! with explicit module init
  gpu: nova-core: use pin projection in method boot()
  rust: debugfs: implement Directory trait for Dir
  rust: debugfs: wrap Entry in an enum to prep for LookupDir
  rust: debugfs: add LookupDir
  gpu: nova-core: create debugfs root when driver loads
  gpu: nova-core: create GSP-RM logging buffers debugfs entries

 drivers/gpu/nova-core/gsp.rs       |  68 +++++++++++--
 drivers/gpu/nova-core/gsp/boot.rs  |  14 +--
 drivers/gpu/nova-core/nova_core.rs |  32 ++++++-
 rust/kernel/debugfs.rs             | 148 ++++++++++++++++++++++++++++-
 rust/kernel/debugfs/entry.rs       | 100 ++++++++++++++++++-
 rust/kernel/pci.rs                 |  37 ++++++++
 6 files changed, 377 insertions(+), 22 deletions(-)


base-commit: 654826aa4a8f25cf825ad9254f37e6cb5092098f
-- 
2.52.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 1/9] rust: pci: add PCI device name method
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 2/9] gpu: nova-core: implement BinaryWriter for LogBuffer Timur Tabi
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

Add a name() method to the PCI `Device` type, which returns a CStr
that contains the device name, typically the BDF address.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/kernel/pci.rs | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 82e128431f08..125fb39f4316 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -427,6 +427,43 @@ pub fn pci_class(&self) -> Class {
         // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
         Class::from_raw(unsafe { (*self.as_raw()).class })
     }
+
+    /// Returns the PCI device name.
+    ///
+    /// This returns the device name in the format "DDDD:BB:DD.F" where:
+    /// - DDDD is the PCI domain (4 hex digits)
+    /// - BB is the bus number (2 hex digits)
+    /// - DD is the device number (2 hex digits)
+    /// - F is the function number (1 hex digit)
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::{c_str, debugfs::Dir, device::Core, pci, prelude::*};
+    /// fn create_debugfs(pdev: &pci::Device<Core>) -> Result {
+    ///     let dir = Dir::new(pdev.name());
+    ///     Ok(())
+    /// }
+    /// ```
+    #[inline]
+    pub fn name(&self) -> &CStr {
+        // SAFETY: By its type invariant `self.as_raw` is always a valid pointer to a
+        // `struct pci_dev`, which contains a `struct device dev` member.
+        unsafe {
+            let pci_dev = self.as_raw();
+            let dev = &raw const (*pci_dev).dev;
+
+            // If init_name is set, use it; otherwise use the kobject name
+            let init_name = (*dev).init_name;
+            let name_ptr = if !init_name.is_null() {
+                init_name
+            } else {
+                (*dev).kobj.name
+            };
+
+            CStr::from_char_ptr(name_ptr)
+        }
+    }
 }
 
 impl Device<device::Core> {
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 2/9] gpu: nova-core: implement BinaryWriter for LogBuffer
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 1/9] rust: pci: add PCI device name method Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 3/9] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

From: Alexandre Courbot <acourbot@nvidia.com>

`LogBuffer` is the entity we ultimately want to dump through debugfs.
Provide a simple implementation of `BinaryWriter` for it, albeit it
might not cut the safety requirements.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/gsp.rs | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 766fd9905358..9b73c96fbd3a 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -3,6 +3,7 @@
 mod boot;
 
 use kernel::{
+    debugfs,
     device,
     dma::{
         CoherentAllocation,
@@ -117,6 +118,29 @@ pub(crate) struct Gsp {
     rmargs: CoherentAllocation<GspArgumentsCached>,
 }
 
+impl debugfs::BinaryWriter for LogBuffer {
+    fn write_to_slice(
+        &self,
+        writer: &mut kernel::uaccess::UserSliceWriter,
+        offset: &mut kernel::fs::file::Offset,
+    ) -> Result<usize> {
+        // SAFETY: This is a debug log buffer. GSP may write concurrently, so the
+        // snapshot may contain partially-written entries. This is acceptable for
+        // debugging purposes - users should be aware logs may be slightly garbled
+        // if read while GSP is actively logging.
+        let slice = unsafe { self.0.as_slice(0, self.0.count()) }?;
+
+        writer.write_slice_file(slice, offset)
+    }
+}
+
+// SAFETY: `LogBuffer` only provides shared access to the underlying `CoherentAllocation`.
+// GSP may write to the buffer concurrently regardless of CPU access, so concurrent reads
+// from multiple CPU threads do not introduce any additional races beyond what already
+// exists with the device. Reads may observe partially-written log entries, which is
+// acceptable for debug logging purposes.
+unsafe impl Sync for LogBuffer {}
+
 impl Gsp {
     // Creates an in-place initializer for a `Gsp` manager for `pdev`.
     pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error> + '_ {
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 3/9] gpu: nova-core: Replace module_pci_driver! with explicit module init
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 1/9] rust: pci: add PCI device name method Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 2/9] gpu: nova-core: implement BinaryWriter for LogBuffer Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 4/9] gpu: nova-core: use pin projection in method boot() Timur Tabi
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

Replace the module_pci_driver! macro with an explicit module
initialization using the standard module! macro and InPlaceModule
trait implementation.  No functional change intended, with the
exception that the driver now prints a message when loaded.

This change is necessary so that we can create a top-level "nova_core"
debugfs entry when the driver is loaded.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/nova_core.rs | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index c1121e7c64c5..d0df8db3693d 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,13 @@
 
 //! Nova Core GPU Driver
 
+use kernel::{
+    error::Error,
+    pci,
+    prelude::*,
+    InPlaceModule, //
+};
+
 #[macro_use]
 mod bitfield;
 
@@ -20,13 +27,26 @@
 
 pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 
-kernel::module_pci_driver! {
-    type: driver::NovaCore,
+#[pin_data]
+struct NovaCoreModule {
+    #[pin]
+    _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
+}
+
+impl InPlaceModule for NovaCoreModule {
+    fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+        try_pin_init!(Self {
+            _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
+        })
+    }
+}
+
+module! {
+    type: NovaCoreModule,
     name: "NovaCore",
     authors: ["Danilo Krummrich"],
     description: "Nova Core GPU driver",
     license: "GPL v2",
-    firmware: [],
 }
 
 kernel::module_firmware!(firmware::ModInfoBuilder);
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 4/9] gpu: nova-core: use pin projection in method boot()
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
                   ` (2 preceding siblings ...)
  2026-01-13 22:54 ` [PATCH v4 3/9] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 5/9] rust: debugfs: implement Directory trait for Dir Timur Tabi
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

Struct Gsp in gsp.rs is tagged with #[pin_data], which allows any of its
fields to be pinned (i.e. with #[pin]).  When #[pin] is added to any
field in a #[pin_data] struct, fields can no longer be directly accessed
via normal field access.  Instead, pin projection must be used to access
those fields.

Currently, no fields are pinned, but that will change.  The boot() method
receives self: Pin<&mut Self>. When struct Gsp contains any pinned
fields, direct field access like self.cmdq is not allowed through
Pin<&mut Self>, as Pin prevents obtaining &mut Self to protect pinned
data from being moved.

Use pin projection via self.as_mut().project() to access struct fields.
The project() method, generated by #[pin_data], returns a projection
struct providing &mut references to non-pinned fields, enabling mutable
access while preserving pin invariants.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 581b412554dc..68a8acaf8c54 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -159,12 +159,14 @@ pub(crate) fn boot(
             CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
         dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
 
-        self.cmdq
+        let this = self.as_mut().project();
+
+        this.cmdq
             .send_command(bar, commands::SetSystemInfo::new(pdev))?;
-        self.cmdq.send_command(bar, commands::SetRegistry::new())?;
+        this.cmdq.send_command(bar, commands::SetRegistry::new())?;
 
         gsp_falcon.reset(bar)?;
-        let libos_handle = self.libos.dma_handle();
+        let libos_handle = this.libos.dma_handle();
         let (mbox0, mbox1) = gsp_falcon.boot(
             bar,
             Some(libos_handle as u32),
@@ -231,13 +233,13 @@ pub(crate) fn boot(
             dev: pdev.as_ref().into(),
             bar,
         };
-        GspSequencer::run(&mut self.cmdq, seq_params)?;
+        GspSequencer::run(this.cmdq, seq_params)?;
 
         // Wait until GSP is fully initialized.
-        commands::wait_gsp_init_done(&mut self.cmdq)?;
+        commands::wait_gsp_init_done(this.cmdq)?;
 
         // Obtain and display basic GPU information.
-        let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
+        let info = commands::get_gsp_info(this.cmdq, bar)?;
         match info.gpu_name() {
             Ok(name) => dev_info!(pdev.as_ref(), "GPU name: {}\n", name),
             Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable: {:?}\n", e),
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 5/9] rust: debugfs: implement Directory trait for Dir
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
                   ` (3 preceding siblings ...)
  2026-01-13 22:54 ` [PATCH v4 4/9] gpu: nova-core: use pin projection in method boot() Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-15 17:27   ` kernel test robot
  2026-01-13 22:54 ` [PATCH v4 6/9] rust: debugfs: wrap Entry in an enum to prep for LookupDir Timur Tabi
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

Instead of having an inherent implementation for struct Dir, create a
Directory trait for it.  This is needed for when we add the LookupDir
struct.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/kernel/debugfs.rs | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index facad81e8290..ed740eb90fc9 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -35,6 +35,18 @@
 #[cfg(CONFIG_DEBUG_FS)]
 use entry::Entry;
 
+/// Trait for DebugFS directory operations.
+///
+/// This trait abstracts the core operations that can be performed on a DebugFS directory,
+/// allowing for alternative implementations (e.g., for testing).
+pub trait Directory: Clone + Sized {
+    /// Create a new directory at the DebugFS root.
+    fn new(name: &CStr) -> Self;
+
+    /// Creates a subdirectory within this directory.
+    fn subdir(&self, name: &CStr) -> Self;
+}
+
 /// Owning handle to a DebugFS directory.
 ///
 /// The directory in the filesystem represented by [`Dir`] will be removed when handle has been
@@ -96,7 +108,9 @@ fn create_file<'a, T, E: 'a>(
             } ? E
         }
     }
+}
 
+impl Directory for Dir {
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples
@@ -106,7 +120,7 @@ fn create_file<'a, T, E: 'a>(
     /// # use kernel::debugfs::Dir;
     /// let debugfs = Dir::new(c_str!("parent"));
     /// ```
-    pub fn new(name: &CStr) -> Self {
+    fn new(name: &CStr) -> Self {
         Dir::create(name, None)
     }
 
@@ -120,9 +134,12 @@ pub fn new(name: &CStr) -> Self {
     /// let parent = Dir::new(c_str!("parent"));
     /// let child = parent.subdir(c_str!("child"));
     /// ```
-    pub fn subdir(&self, name: &CStr) -> Self {
+    fn subdir(&self, name: &CStr) -> Self {
         Dir::create(name, Some(self))
     }
+}
+
+impl Dir {
 
     /// Creates a read-only file in this directory.
     ///
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 6/9] rust: debugfs: wrap Entry in an enum to prep for LookupDir
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
                   ` (4 preceding siblings ...)
  2026-01-13 22:54 ` [PATCH v4 5/9] rust: debugfs: implement Directory trait for Dir Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 7/9] rust: debugfs: add LookupDir Timur Tabi
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

To prepare the Entry struct for adding support for LookupDir, create a
wrapper enum DynParent.  For now, it only holds an Arc<Entry>.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/kernel/debugfs/entry.rs | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
index 706cb7f73d6c..3152e4f96219 100644
--- a/rust/kernel/debugfs/entry.rs
+++ b/rust/kernel/debugfs/entry.rs
@@ -7,6 +7,16 @@
 use crate::sync::Arc;
 use core::marker::PhantomData;
 
+/// Type-erased parent that keeps either an `Entry` or `LookupEntry` alive.
+///
+/// This allows an `Entry` to have either type of parent while maintaining proper
+/// lifetime management. The parent is kept alive via `Arc` reference counting.
+#[allow(dead_code)]
+pub(crate) enum DynParent {
+    /// Parent is an owned `Entry` (will be removed on drop).
+    Entry(Arc<Entry<'static>>),
+}
+
 /// Owning handle to a DebugFS entry.
 ///
 /// # Invariants
@@ -15,7 +25,7 @@
 pub(crate) struct Entry<'a> {
     entry: *mut bindings::dentry,
     // If we were created with an owning parent, this is the keep-alive
-    _parent: Option<Arc<Entry<'static>>>,
+    _parent: Option<DynParent>,
     // If we were created with a non-owning parent, this prevents us from outliving it
     _phantom: PhantomData<&'a ()>,
 }
@@ -41,7 +51,7 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
 
         Entry {
             entry,
-            _parent: parent,
+            _parent: parent.map(DynParent::Entry),
             _phantom: PhantomData,
         }
     }
@@ -74,7 +84,7 @@ pub(crate) unsafe fn dynamic_file<T>(
 
         Entry {
             entry,
-            _parent: Some(parent),
+            _parent: Some(DynParent::Entry(parent)),
             _phantom: PhantomData,
         }
     }
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 7/9] rust: debugfs: add LookupDir
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
                   ` (5 preceding siblings ...)
  2026-01-13 22:54 ` [PATCH v4 6/9] rust: debugfs: wrap Entry in an enum to prep for LookupDir Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 8/9] gpu: nova-core: create debugfs root when driver loads Timur Tabi
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

Add `LookupDir`, a non-owning handle to an existing DebugFS directory.
Unlike `Dir`, which creates a new directory and removes it on drop,
`LookupDir` looks up an existing directory and only releases the
reference when dropped—the directory itself remains in the filesystem.

This is useful when a driver needs to add files to a directory created
by another part of the system without taking ownership of that
directory.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/kernel/debugfs.rs       | 127 +++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/entry.rs |  84 +++++++++++++++++++++++
 2 files changed, 211 insertions(+)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index ed740eb90fc9..2ff70d72240c 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -34,6 +34,8 @@
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]
 use entry::Entry;
+#[cfg(CONFIG_DEBUG_FS)]
+use entry::LookupEntry;
 
 /// Trait for DebugFS directory operations.
 ///
@@ -393,6 +395,131 @@ pub fn scope<'a, T: 'a, E: 'a, F>(
     }
 }
 
+/// Non-owning handle to an existing DebugFS directory.
+///
+/// Unlike [`Dir`], a [`LookupDir`] does not create a new directory. Instead, it looks up an
+/// existing directory in the DebugFS filesystem. When dropped, the directory is **not** removed
+/// from the filesystem - only the reference is released.
+///
+/// This is useful when you want to add files to an existing directory created by another part
+/// of the system without taking ownership of that directory.
+#[derive(Clone)]
+pub struct LookupDir(#[cfg(CONFIG_DEBUG_FS)] Option<Arc<LookupEntry>>);
+
+impl LookupDir {
+    /// Looks up an existing directory in DebugFS.
+    ///
+    /// If `parent` is [`None`], the lookup is performed from the root of the debugfs filesystem.
+    ///
+    /// Returns [`Some(LookupDir)`] if the directory exists, [`None`] otherwise.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::LookupDir;
+    /// // Look up a top-level directory
+    /// if let Some(dir) = LookupDir::lookup(c_str!("existing_dir"), None) {
+    ///     // Use the directory...
+    /// }
+    /// // When `dir` is dropped, the directory is NOT removed.
+    /// ```
+    pub fn lookup(name: &CStr, parent: Option<&LookupDir>) -> Option<Self> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            let parent_entry = match parent {
+                Some(LookupDir(None)) => return None,
+                Some(LookupDir(Some(entry))) => Some(entry.clone()),
+                None => None,
+            };
+            let entry = LookupEntry::lookup(name, parent_entry)?;
+            Some(Self(Arc::new(entry, GFP_KERNEL).ok()))
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        None
+    }
+
+    // While this function is safe, it is intentionally not public because it's a bit of a
+    // footgun.
+    //
+    // Unless you also extract the `entry` later and schedule it for `Drop` at the appropriate
+    // time, a `ScopedDir` with a `LookupDir` parent will never be deleted.
+    fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            let parent_entry = match &self.0 {
+                None => return ScopedDir::empty(),
+                Some(entry) => entry.clone(),
+            };
+            ScopedDir {
+                entry: ManuallyDrop::new(Entry::dynamic_dir_with_lookup_parent(name, parent_entry)),
+                _phantom: PhantomData,
+            }
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        ScopedDir::empty()
+    }
+
+    /// Creates a new scope, which is a directory associated with some data `T`.
+    ///
+    /// The created directory will be a subdirectory of `self`. The `init` closure is called to
+    /// populate the directory with files and subdirectories. These files can reference the data
+    /// stored in the scope.
+    ///
+    /// The entire directory tree created within the scope will be removed when the returned
+    /// `Scope` handle is dropped.
+    ///
+    /// Note: Unlike [`Dir::scope`], this method consumes `self` because the parent entry
+    /// is kept alive by the created scope via [`DynParent`], not by the `LookupDir` handle.
+    pub fn scope<'a, T: 'a, E: 'a, F>(
+        self,
+        data: impl PinInit<T, E> + 'a,
+        name: &'a CStr,
+        init: F,
+    ) -> impl PinInit<Scope<T>, E> + 'a
+    where
+        F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
+    {
+        Scope::new(data, move |data| {
+            let scoped = self.scoped_dir(name);
+            init(data, &scoped);
+            scoped.into_entry()
+        })
+    }
+}
+
+impl Directory for LookupDir {
+    /// Looks up an existing directory at the DebugFS root.
+    ///
+    /// Note: Unlike [`Dir::new`], this will return an empty handle if the directory
+    /// does not exist, rather than creating it.
+    fn new(name: &CStr) -> Self {
+        Self::lookup(name, None).unwrap_or_else(|| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            {
+                Self(None)
+            }
+            #[cfg(not(CONFIG_DEBUG_FS))]
+            Self()
+        })
+    }
+
+    /// Looks up a subdirectory within this directory.
+    ///
+    /// Note: Unlike [`Dir::subdir`], this will return an empty handle if the subdirectory
+    /// does not exist, rather than creating it.
+    fn subdir(&self, name: &CStr) -> Self {
+        Self::lookup(name, Some(self)).unwrap_or_else(|| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            {
+                Self(None)
+            }
+            #[cfg(not(CONFIG_DEBUG_FS))]
+            Self()
+        })
+    }
+}
+
 #[pin_data]
 /// Handle to a DebugFS scope, which ensures that attached `data` will outlive the DebugFS entry
 /// without moving.
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
index 3152e4f96219..1e8a5cedf69d 100644
--- a/rust/kernel/debugfs/entry.rs
+++ b/rust/kernel/debugfs/entry.rs
@@ -15,6 +15,8 @@
 pub(crate) enum DynParent {
     /// Parent is an owned `Entry` (will be removed on drop).
     Entry(Arc<Entry<'static>>),
+    /// Parent is a looked-up `LookupEntry` (will NOT be removed on drop).
+    LookupEntry(Arc<LookupEntry>),
 }
 
 /// Owning handle to a DebugFS entry.
@@ -56,6 +58,24 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
         }
     }
 
+    /// Creates a new directory entry with a `LookupEntry` as parent.
+    ///
+    /// The parent is kept alive via `Arc` reference counting. When this `Entry` is dropped,
+    /// the created directory will be removed, but the parent `LookupEntry` will only have
+    /// its reference count decremented.
+    pub(crate) fn dynamic_dir_with_lookup_parent(name: &CStr, parent: Arc<LookupEntry>) -> Self {
+        // SAFETY: The invariants of this function's arguments ensure the safety of this call.
+        // * `name` is a valid C string by the invariants of `&CStr`.
+        // * `parent.as_ptr()` is a pointer to a valid `dentry` by the invariants of `LookupEntry`.
+        let entry = unsafe { bindings::debugfs_create_dir(name.as_char_ptr(), parent.as_ptr()) };
+
+        Entry {
+            entry,
+            _parent: Some(DynParent::LookupEntry(parent)),
+            _phantom: PhantomData,
+        }
+    }
+
     /// # Safety
     ///
     /// * `data` must outlive the returned `Entry`.
@@ -172,3 +192,67 @@ fn drop(&mut self) {
         unsafe { bindings::debugfs_remove(self.as_ptr()) }
     }
 }
+
+/// Non-owning handle to a DebugFS entry obtained via lookup.
+///
+/// Unlike [`Entry`], dropping a [`LookupEntry`] does not remove the directory from the
+/// filesystem. It only releases the reference obtained via `debugfs_lookup`.
+///
+/// # Invariants
+///
+/// The wrapped pointer will always be `NULL` or a valid DebugFS `dentry` obtained via
+/// `debugfs_lookup`.
+pub(crate) struct LookupEntry {
+    entry: *mut bindings::dentry,
+    // If we were created with an owning parent, this is the keep-alive
+    _parent: Option<Arc<LookupEntry>>,
+}
+
+// SAFETY: [`LookupEntry`] is just a `dentry` under the hood, which the API promises can be
+// transferred between threads.
+unsafe impl Send for LookupEntry {}
+
+// SAFETY: All the C functions we call on the `dentry` pointer are threadsafe.
+unsafe impl Sync for LookupEntry {}
+
+impl LookupEntry {
+    /// Looks up an existing directory in DebugFS.
+    ///
+    /// Returns `Some(LookupEntry)` if the directory exists, `None` otherwise.
+    pub(crate) fn lookup(name: &CStr, parent: Option<Arc<Self>>) -> Option<Self> {
+        let parent_ptr = match &parent {
+            Some(entry) => entry.as_ptr(),
+            None => core::ptr::null_mut(),
+        };
+        // SAFETY: The invariants of this function's arguments ensure the safety of this call.
+        // * `name` is a valid C string by the invariants of `&CStr`.
+        // * `parent_ptr` is either `NULL` (if `parent` is `None`), or a pointer to a valid
+        //   `dentry` by our invariant. `debugfs_lookup` handles `NULL` pointers correctly
+        //   (searches from debugfs root).
+        let entry = unsafe { bindings::debugfs_lookup(name.as_char_ptr(), parent_ptr) };
+
+        if entry.is_null() {
+            None
+        } else {
+            Some(LookupEntry {
+                entry,
+                _parent: parent,
+            })
+        }
+    }
+
+    /// Returns the pointer representation of the DebugFS directory.
+    pub(crate) fn as_ptr(&self) -> *mut bindings::dentry {
+        self.entry
+    }
+}
+
+impl Drop for LookupEntry {
+    fn drop(&mut self) {
+        if !self.entry.is_null() {
+            // SAFETY: `dput` decrements the reference count on a dentry. The pointer is valid
+            // because it was obtained via `debugfs_lookup` which increments the reference count.
+            unsafe { bindings::dput(self.entry) }
+        }
+    }
+}
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 8/9] gpu: nova-core: create debugfs root when driver loads
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
                   ` (6 preceding siblings ...)
  2026-01-13 22:54 ` [PATCH v4 7/9] rust: debugfs: add LookupDir Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 22:54 ` [PATCH v4 9/9] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
  2026-01-13 23:11 ` [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Danilo Krummrich
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

Create the 'nova_core' root debugfs entry when the driver loads.
The Dir entry is part of the NovaCoreModule, so when the module
unloads, the NovaCoreModule object is dropped, and that drops the
_debugfs_root, which automatically deletes the debugfs file.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/nova_core.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index d0df8db3693d..c9b93ccd6092 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -3,6 +3,7 @@
 //! Nova Core GPU Driver
 
 use kernel::{
+    debugfs::{Dir, Directory},
     error::Error,
     pci,
     prelude::*,
@@ -31,12 +32,17 @@
 struct NovaCoreModule {
     #[pin]
     _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
+    _debugfs_root: Dir,
 }
 
 impl InPlaceModule for NovaCoreModule {
     fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+        // Create the debugfs top-level directory.  Each GPU will create a subdirectory.
+        let debugfs_root = Dir::new(kernel::c_str!("nova_core"));
+
         try_pin_init!(Self {
             _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
+            _debugfs_root: debugfs_root,
         })
     }
 }
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 9/9] gpu: nova-core: create GSP-RM logging buffers debugfs entries
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
                   ` (7 preceding siblings ...)
  2026-01-13 22:54 ` [PATCH v4 8/9] gpu: nova-core: create debugfs root when driver loads Timur Tabi
@ 2026-01-13 22:54 ` Timur Tabi
  2026-01-13 23:11 ` [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Danilo Krummrich
  9 siblings, 0 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 22:54 UTC (permalink / raw)
  To: Matthew Maurer, Danilo Krummrich, Gary Guo, John Hubbard,
	Joel Fernandes, Alexandre Courbot, rust-for-linux, nouveau

Create read-only debugfs entries for LOGINIT, LOGRM, and LOGINTR, which
are the three primary printf logging buffers from GSP-RM.  LOGPMU will
be added at a later date, as it requires it support for its RPC message
first.

This patch uses the `pin_init_scope` feature to create the entries.
`pin_init_scope` solves the lifetime issue over the `DEBUGFS_ROOT`
reference by delaying its acquisition until the time the entry is
actually initialized.

Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/gsp.rs | 46 +++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 9b73c96fbd3a..dc4a009862ef 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -3,7 +3,8 @@
 mod boot;
 
 use kernel::{
-    debugfs,
+    c_str,
+    debugfs::{self, Directory},
     device,
     dma::{
         CoherentAllocation,
@@ -101,17 +102,24 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
     }
 }
 
-/// GSP runtime data.
-#[pin_data]
-pub(crate) struct Gsp {
-    /// Libos arguments.
-    pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
+/// Log buffers used by GSP-RM for debug logging.
+struct LogBuffers {
     /// Init log buffer.
     loginit: LogBuffer,
     /// Interrupts log buffer.
     logintr: LogBuffer,
     /// RM log buffer.
     logrm: LogBuffer,
+}
+
+/// GSP runtime data.
+#[pin_data]
+pub(crate) struct Gsp {
+    /// Libos arguments.
+    pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
+    /// Log buffers, optionally exposed via debugfs.
+    #[pin]
+    logs: debugfs::Scope<LogBuffers>,
     /// Command queue.
     pub(crate) cmdq: Cmdq,
     /// RM arguments.
@@ -147,15 +155,17 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
         pin_init::pin_init_scope(move || {
             let dev = pdev.as_ref();
 
+            // Create log buffers before try_pin_init! so they're accessible throughout
+            let loginit = LogBuffer::new(dev)?;
+            let logintr = LogBuffer::new(dev)?;
+            let logrm = LogBuffer::new(dev)?;
+
             Ok(try_pin_init!(Self {
                 libos: CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
                     dev,
                     GSP_PAGE_SIZE / size_of::<LibosMemoryRegionInitArgument>(),
                     GFP_KERNEL | __GFP_ZERO,
                 )?,
-                loginit: LogBuffer::new(dev)?,
-                logintr: LogBuffer::new(dev)?,
-                logrm: LogBuffer::new(dev)?,
                 cmdq: Cmdq::new(dev)?,
                 rmargs: CoherentAllocation::<GspArgumentsCached>::alloc_coherent(
                     dev,
@@ -176,6 +186,24 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
                     dma_write!(rmargs[0] = fw::GspArgumentsCached::new(cmdq))?;
                     dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
                 },
+                logs <- {
+                    let log_buffers = LogBuffers {
+                        loginit,
+                        logintr,
+                        logrm,
+                    };
+
+                    // Look up nova_core debugfs directory - only create entries if it exists.
+                    // If nova_core doesn't exist, debugfs_dir will be empty and file creation
+                    // becomes a no-op (data is still stored, just not exposed via debugfs).
+                    let debugfs_dir = debugfs::LookupDir::new(c_str!("nova_core"));
+
+                    debugfs_dir.scope(log_buffers, pdev.name(), |logs, dir| {
+                        dir.read_binary_file(c_str!("loginit"), &logs.loginit);
+                        dir.read_binary_file(c_str!("logintr"), &logs.logintr);
+                        dir.read_binary_file(c_str!("logrm"), &logs.logrm);
+                    })
+                },
             }))
         })
     }
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs
  2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
                   ` (8 preceding siblings ...)
  2026-01-13 22:54 ` [PATCH v4 9/9] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
@ 2026-01-13 23:11 ` Danilo Krummrich
  2026-01-13 23:26   ` Timur Tabi
  9 siblings, 1 reply; 17+ messages in thread
From: Danilo Krummrich @ 2026-01-13 23:11 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Matthew Maurer, Gary Guo, John Hubbard, Joel Fernandes,
	Alexandre Courbot, rust-for-linux, nouveau

On Tue Jan 13, 2026 at 11:53 PM CET, Timur Tabi wrote:

It seems that a lot of the feedback from the last version has not been
addressed:

> Alexandre Courbot (1):
>   gpu: nova-core: implement BinaryWriter for LogBuffer

https://lore.kernel.org/all/DF19KTQTKOS9.33N1KT9WNLAUO@kernel.org/

> Timur Tabi (8):
>   rust: pci: add PCI device name method

https://lore.kernel.org/all/DF17TCJPO9RT.2BK28ZOQSF9SN@kernel.org/

>   gpu: nova-core: Replace module_pci_driver! with explicit module init

https://lore.kernel.org/all/DF17XPT1MU64.ZXT4LSXR9CIG@kernel.org/

>   gpu: nova-core: use pin projection in method boot()
>   rust: debugfs: implement Directory trait for Dir
>   rust: debugfs: wrap Entry in an enum to prep for LookupDir
>   rust: debugfs: add LookupDir

https://lore.kernel.org/all/DF18RFX3IHVP.3GYNJIYAFFJU6@kernel.org/

Especially this one is a concern, I don't want to add this infrastructure as a
workaround until we land the feature Gary works on.

As mentioned in this reply, I think that debugfs_lookup() rarely is the correct
solution and more often indicates some kind of (design) issue, like it does in
this case.

Before adding this, I would like to see a valid use-case elsewhere.

If we want this before Gary's work is ready, I prefer some hacky temporary
workaround in nova-core to access the module field instead.

>   gpu: nova-core: create debugfs root when driver loads
>   gpu: nova-core: create GSP-RM logging buffers debugfs entries

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs
  2026-01-13 23:11 ` [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Danilo Krummrich
@ 2026-01-13 23:26   ` Timur Tabi
  2026-01-13 23:50     ` Danilo Krummrich
  0 siblings, 1 reply; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 23:26 UTC (permalink / raw)
  To: dakr@kernel.org
  Cc: gary@garyguo.net, nouveau@lists.freedesktop.org, Joel Fernandes,
	Alexandre Courbot, mmaurer@google.com, John Hubbard,
	rust-for-linux@vger.kernel.org

On Wed, 2026-01-14 at 00:11 +0100, Danilo Krummrich wrote:
> On Tue Jan 13, 2026 at 11:53 PM CET, Timur Tabi wrote:
> 
> It seems that a lot of the feedback from the last version has not been
> addressed:
> 
> > Alexandre Courbot (1):
> >   gpu: nova-core: implement BinaryWriter for LogBuffer
> 
> https://lore.kernel.org/all/DF19KTQTKOS9.33N1KT9WNLAUO@kernel.org/
> 
> > Timur Tabi (8):
> >   rust: pci: add PCI device name method
> 
> https://lore.kernel.org/all/DF17TCJPO9RT.2BK28ZOQSF9SN@kernel.org/
> 
> >   gpu: nova-core: Replace module_pci_driver! with explicit module init
> 
> https://lore.kernel.org/all/DF17XPT1MU64.ZXT4LSXR9CIG@kernel.org/

Sorry, I couldn't find these messages in my inbox.  I will address them in v5.

> >   gpu: nova-core: use pin projection in method boot()
> >   rust: debugfs: implement Directory trait for Dir
> >   rust: debugfs: wrap Entry in an enum to prep for LookupDir
> >   rust: debugfs: add LookupDir
> 
> https://lore.kernel.org/all/DF18RFX3IHVP.3GYNJIYAFFJU6@kernel.org/
> 
> Especially this one is a concern, I don't want to add this infrastructure as a
> workaround until we land the feature Gary works on.
> 
> As mentioned in this reply, I think that debugfs_lookup() rarely is the correct
> solution and more often indicates some kind of (design) issue, like it does in
> this case.

Well, I'm at a loss how to proceed, then.  I had a global variable, which is the approach that
Nouveau uses, it was recommended instead that I use debugfs_lookup().  So I implemented that, but
that is also rejected.

So instead of just telling me that this approach is wrong, please tell what the correct approach
should be.  Please note that this patchset is already pushing the limits of my R4L skills, so I will
need specifics.

> Before adding this, I would like to see a valid use-case elsewhere.

I don't have one.

> If we want this before Gary's work is ready, I prefer some hacky temporary
> workaround in nova-core to access the module field instead.

Danilo, you are the one who has been pushing for this code.  Please note that the logging buffers
can only be parsed by Nvidia employees anyway, and we can just use this patchset in house to debug
GSP-RM as needed.  So if you want to wait until Gary's work is ready (I don't know anything about
what he's doing), please let me know now and I will just keep these patches internally.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs
  2026-01-13 23:26   ` Timur Tabi
@ 2026-01-13 23:50     ` Danilo Krummrich
  2026-01-13 23:59       ` Timur Tabi
  0 siblings, 1 reply; 17+ messages in thread
From: Danilo Krummrich @ 2026-01-13 23:50 UTC (permalink / raw)
  To: Timur Tabi
  Cc: gary@garyguo.net, nouveau@lists.freedesktop.org, Joel Fernandes,
	Alexandre Courbot, mmaurer@google.com, John Hubbard,
	rust-for-linux@vger.kernel.org

On Wed Jan 14, 2026 at 12:26 AM CET, Timur Tabi wrote:
> On Wed, 2026-01-14 at 00:11 +0100, Danilo Krummrich wrote:
>> On Tue Jan 13, 2026 at 11:53 PM CET, Timur Tabi wrote:
> Sorry, I couldn't find these messages in my inbox.  I will address them in v5.

Sounds good, thanks.

>> >   gpu: nova-core: use pin projection in method boot()
>> >   rust: debugfs: implement Directory trait for Dir
>> >   rust: debugfs: wrap Entry in an enum to prep for LookupDir
>> >   rust: debugfs: add LookupDir
>> 
>> https://lore.kernel.org/all/DF18RFX3IHVP.3GYNJIYAFFJU6@kernel.org/
>> 
>> Especially this one is a concern, I don't want to add this infrastructure as a
>> workaround until we land the feature Gary works on.
>> 
>> As mentioned in this reply, I think that debugfs_lookup() rarely is the correct
>> solution and more often indicates some kind of (design) issue, like it does in
>> this case.
>
> Well, I'm at a loss how to proceed, then.  I had a global variable, which is the approach that
> Nouveau uses, it was recommended instead that I use debugfs_lookup().  So I implemented that, but
> that is also rejected.

Yeah, the global is not a good approach for a final solution. But as temporary
workaround it's probably fine.

Maybe Gary has an alternative idea for a temporary workaround since he's working
on the proper solution to safely access module fields.

But again, I think a simple global with a FIXME comment should be fine.

> Danilo, you are the one who has been pushing for this code.  Please note that the logging buffers
> can only be parsed by Nvidia employees anyway, and we can just use this patchset in house to debug
> GSP-RM as needed.  So if you want to wait until Gary's work is ready (I don't know anything about
> what he's doing), please let me know now and I will just keep these patches internally.

I can't remember pushing for it, but I do think it would be good to land.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs
  2026-01-13 23:50     ` Danilo Krummrich
@ 2026-01-13 23:59       ` Timur Tabi
  2026-01-14  1:09         ` Gary Guo
  2026-01-14 11:17         ` Danilo Krummrich
  0 siblings, 2 replies; 17+ messages in thread
From: Timur Tabi @ 2026-01-13 23:59 UTC (permalink / raw)
  To: dakr@kernel.org
  Cc: gary@garyguo.net, nouveau@lists.freedesktop.org, Joel Fernandes,
	Alexandre Courbot, mmaurer@google.com, John Hubbard,
	rust-for-linux@vger.kernel.org

On Wed, 2026-01-14 at 00:50 +0100, Danilo Krummrich wrote:
> Maybe Gary has an alternative idea for a temporary workaround since he's working
> on the proper solution to safely access module fields.
> 
> But again, I think a simple global with a FIXME comment should be fine.

That's what I had in my initial version.

I wish you had said something when Joel suggested it:
https://lore.freedesktop.org/nouveau/246c1ad4bb1ca7ef34f331fba33989bbae8618f8.camel@nvidia.com/T/#m69a23a3db642c8cbb22efe0e24647811e8ae3f12

I spent a lot of time over the past few weeks implementing lookup support, and now you're saying it
was for nothing.  

What exactly is Gary's code supposed to do, anyway?  "proper solution to safely access module
fields"  I don't know what this means.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs
  2026-01-13 23:59       ` Timur Tabi
@ 2026-01-14  1:09         ` Gary Guo
  2026-01-14 11:17         ` Danilo Krummrich
  1 sibling, 0 replies; 17+ messages in thread
From: Gary Guo @ 2026-01-14  1:09 UTC (permalink / raw)
  To: Timur Tabi, dakr@kernel.org
  Cc: gary@garyguo.net, nouveau@lists.freedesktop.org, Joel Fernandes,
	Alexandre Courbot, mmaurer@google.com, John Hubbard,
	rust-for-linux@vger.kernel.org

On Tue Jan 13, 2026 at 11:59 PM GMT, Timur Tabi wrote:
> On Wed, 2026-01-14 at 00:50 +0100, Danilo Krummrich wrote:
>> Maybe Gary has an alternative idea for a temporary workaround since he's working
>> on the proper solution to safely access module fields.
>> 
>> But again, I think a simple global with a FIXME comment should be fine.
>
> That's what I had in my initial version.
>
> I wish you had said something when Joel suggested it:
> https://lore.freedesktop.org/nouveau/246c1ad4bb1ca7ef34f331fba33989bbae8618f8.camel@nvidia.com/T/#m69a23a3db642c8cbb22efe0e24647811e8ae3f12
>
> I spent a lot of time over the past few weeks implementing lookup support, and now you're saying it
> was for nothing.  
>
> What exactly is Gary's code supposed to do, anyway?  "proper solution to safely access module
> fields"  I don't know what this means.

There're quite a few ideas being discussed here:
https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Module.2Fdriver-scoped.20data/with/564476412,
although I haven't had time to do any proper implementation work yet.

Best,
Gary

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs
  2026-01-13 23:59       ` Timur Tabi
  2026-01-14  1:09         ` Gary Guo
@ 2026-01-14 11:17         ` Danilo Krummrich
  1 sibling, 0 replies; 17+ messages in thread
From: Danilo Krummrich @ 2026-01-14 11:17 UTC (permalink / raw)
  To: Timur Tabi
  Cc: gary@garyguo.net, nouveau@lists.freedesktop.org, Joel Fernandes,
	Alexandre Courbot, mmaurer@google.com, John Hubbard,
	rust-for-linux@vger.kernel.org

On Wed Jan 14, 2026 at 12:59 AM CET, Timur Tabi wrote:
> On Wed, 2026-01-14 at 00:50 +0100, Danilo Krummrich wrote:
>> Maybe Gary has an alternative idea for a temporary workaround since he's working
>> on the proper solution to safely access module fields.
>> 
>> But again, I think a simple global with a FIXME comment should be fine.
>
> That's what I had in my initial version.
>
> I wish you had said something when Joel suggested it:
> https://lore.freedesktop.org/nouveau/246c1ad4bb1ca7ef34f331fba33989bbae8618f8.camel@nvidia.com/T/#m69a23a3db642c8cbb22efe0e24647811e8ae3f12

This was when I came back from being five weeks out, left with > 10k unread
mails. This was known and I even offered a separate communication channel to
reach out should something come up that requires my attention.

However, only two days later I did catch your subsequent series, replied [1] and
mentioned everything I mentioned in this thread as well.

> I spent a lot of time over the past few weeks implementing lookup support, and now you're saying it
> was for nothing.

I understand that this is frustrating, but I wouldn't say it was for nothing.

You previously mentioned that you are relatively new to Rust; glancing at the
code I can imaginge that you learned a lot from working on this, which I think
is pretty valuable as well.

Also, we can pick up this code at any time should someone find a valid use-case
for this feature.

- Danilo

[1] https://lore.kernel.org/all/DF18RFX3IHVP.3GYNJIYAFFJU6@kernel.org/

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 5/9] rust: debugfs: implement Directory trait for Dir
  2026-01-13 22:54 ` [PATCH v4 5/9] rust: debugfs: implement Directory trait for Dir Timur Tabi
@ 2026-01-15 17:27   ` kernel test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2026-01-15 17:27 UTC (permalink / raw)
  To: Timur Tabi, Matthew Maurer, Danilo Krummrich, Gary Guo,
	John Hubbard, Joel Fernandes, Alexandre Courbot, rust-for-linux,
	nouveau
  Cc: oe-kbuild-all

Hi Timur,

kernel test robot noticed the following build errors:

[auto build test ERROR on 654826aa4a8f25cf825ad9254f37e6cb5092098f]

url:    https://github.com/intel-lab-lkp/linux/commits/Timur-Tabi/rust-pci-add-PCI-device-name-method/20260114-065717
base:   654826aa4a8f25cf825ad9254f37e6cb5092098f
patch link:    https://lore.kernel.org/r/20260113225408.671252-6-ttabi%40nvidia.com
patch subject: [PATCH v4 5/9] rust: debugfs: implement Directory trait for Dir
config: x86_64-randconfig-122-20260114 (https://download.01.org/0day-ci/archive/20260116/202601160158.7mf9LG1y-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260116/202601160158.7mf9LG1y-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601160158.7mf9LG1y-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> rust/doctests_kernel_generated.rs:4085:16
   |
   4085 | let dir = Dir::new(c_str!("my_debugfs_dir"));
   |                ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   3    + use crate::rust_doctest_kernel_alloc_allocator_rs_0::kernel::debugfs::Directory;
   |
--
>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> rust/doctests_kernel_generated.rs:4145:16
   |
   4145 | let dir = Dir::new(c_str!("my_debugfs_dir"));
   |                ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   3    + use crate::rust_doctest_kernel_alloc_allocator_rs_0::kernel::debugfs::Directory;
   |
--
>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> rust/doctests_kernel_generated.rs:4204:16
   |
   4204 | let dir = Dir::new(c_str!("foo"));
   |                ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   3    + use crate::rust_doctest_kernel_alloc_allocator_rs_0::kernel::debugfs::Directory;
   |
--
>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> rust/doctests_kernel_generated.rs:3973:20
   |
   3973 | let debugfs = Dir::new(c_str!("parent"));
   |                    ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   3    + use crate::rust_doctest_kernel_alloc_allocator_rs_0::kernel::debugfs::Directory;
   |
--
>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> rust/doctests_kernel_generated.rs:4028:19
   |
   4028 | let parent = Dir::new(c_str!("parent"));
   |                   ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   3    + use crate::rust_doctest_kernel_alloc_allocator_rs_0::kernel::debugfs::Directory;
   |
--
>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> rust/doctests_kernel_generated.rs:11462:20
   |
   11462 |     let dir = Dir::new(pdev.name());
   |                    ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   3     + use crate::rust_doctest_kernel_alloc_allocator_rs_0::kernel::debugfs::Directory;
   |
--
>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> samples/rust/rust_debugfs.rs:136:28
   |
   136 |         let debugfs = Dir::new(c_str!("sample_debugfs"));
   |                            ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   34  + use kernel::debugfs::Directory;
   |
--
>> error[E0599]: no function or associated item named `new` found for struct `Dir` in the current scope
   --> samples/rust/rust_debugfs_scoped.rs:134:29
   |
   134 |         let base_dir = Dir::new(c_str!("rust_scoped_debugfs"));
   |                             ^^^ function or associated item not found in `Dir`
   |
   = help: items from traits can only be used if the trait is in scope
   help: trait `Directory` which provides `new` is implemented but not in scope; perhaps you want to import it
   |
   9   + use kernel::debugfs::Directory;
   |

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-01-15 17:28 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-13 22:53 [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2026-01-13 22:54 ` [PATCH v4 1/9] rust: pci: add PCI device name method Timur Tabi
2026-01-13 22:54 ` [PATCH v4 2/9] gpu: nova-core: implement BinaryWriter for LogBuffer Timur Tabi
2026-01-13 22:54 ` [PATCH v4 3/9] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
2026-01-13 22:54 ` [PATCH v4 4/9] gpu: nova-core: use pin projection in method boot() Timur Tabi
2026-01-13 22:54 ` [PATCH v4 5/9] rust: debugfs: implement Directory trait for Dir Timur Tabi
2026-01-15 17:27   ` kernel test robot
2026-01-13 22:54 ` [PATCH v4 6/9] rust: debugfs: wrap Entry in an enum to prep for LookupDir Timur Tabi
2026-01-13 22:54 ` [PATCH v4 7/9] rust: debugfs: add LookupDir Timur Tabi
2026-01-13 22:54 ` [PATCH v4 8/9] gpu: nova-core: create debugfs root when driver loads Timur Tabi
2026-01-13 22:54 ` [PATCH v4 9/9] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
2026-01-13 23:11 ` [PATCH v4 0/9] gpu: nova-core: expose the logging buffers via debugfs Danilo Krummrich
2026-01-13 23:26   ` Timur Tabi
2026-01-13 23:50     ` Danilo Krummrich
2026-01-13 23:59       ` Timur Tabi
2026-01-14  1:09         ` Gary Guo
2026-01-14 11:17         ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox