[PATCH v3 0/9] gpu: nova-core: expose the logging buffers via debugfs

[PATCH v3 0/9] gpu: nova-core: expose the logging buffers via debugfs

[Timur Tabi]

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.

Based on Linus' repository, not drm-rust-next.

Summary of changes:

1. Replace module_pci_driver! with explicit init function.
2. Creates root, per-gpu, and individual buffer debugfs entries.
3. Adds a pci::name() method to generate a PCI device name string.
4. Adds a lookup() method to debugfs, so that the root debugfs entry
   doesn't need to be a global variable like it is in Nouveau.
5. Makes some updates to debugfs core code.

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

Timur Tabi (8):
  rust: pci: add PCI device name method
  rust: debugfs: add lookup contructor
  rust: debugfs: add Dir::empty() for no-op directory handle
  rust: debugfs: fix Dir::scope() to not borrow self for returned
    lifetime
  gpu: nova-core: Replace module_pci_driver! with explicit module init
  gpu: nova-core: create debugfs root when driver loads
  gpu: nova-core: use pin projection in method boot()
  gpu: nova-core: create GSP-RM logging buffers debugfs entries

 drivers/gpu/nova-core/gsp.rs       |  66 +++++++++++++++---
 drivers/gpu/nova-core/gsp/boot.rs  |  15 ++--
 drivers/gpu/nova-core/nova_core.rs |  29 +++++++-
 rust/kernel/debugfs.rs             | 108 ++++++++++++++++++++++-------
 rust/kernel/debugfs/entry.rs       |  49 ++++++++++++-
 rust/kernel/pci.rs                 |  37 ++++++++++
 6 files changed, 259 insertions(+), 45 deletions(-)


base-commit: ea1013c1539270e372fc99854bc6e4d94eaeff66
-- 
2.52.0

[PATCH v3 1/9] rust: pci: add PCI device name method

[Timur Tabi]

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

Re: [PATCH v3 1/9] rust: pci: add PCI device name method

[Alice Ryhl]

On Wed, Dec 17, 2025 at 07:39:02PM -0600, Timur Tabi wrote:
> 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 {

What's the actual unsafe operation in this block? We generally try to
limit the scope of unsafe as much as possible.

Alice

> +            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
> 

Re: [PATCH v3 1/9] rust: pci: add PCI device name method

[Danilo Krummrich]

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> +    #[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)
> +        }
> +    }
>  }

This does not belong into pci::Device, but device::Device.

Also, please do not open-code the init_name dance, please use dev_name() instead.

Re: [PATCH v3 1/9] rust: pci: add PCI device name method

[Miguel Ojeda]

On Thu, Dec 18, 2025 at 2:40 AM Timur Tabi <ttabi@nvidia.com> wrote:
>
> +    /// 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)

It probably looks better like (i.e. code spans + ending in period):

    /// 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).

> +            // If init_name is set, use it; otherwise use the kobject name

Similarly:

    // If `init_name` is set, use it; otherwise use the kobject name.

Thanks!

Cheers,
Miguel

[PATCH v3 2/9] rust: debugfs: add lookup contructor

[Timur Tabi]

Add a new constructor for Dir that uses the debugfs_lookup() API to
obtain a reference to an existing debugfs directory entry.

The key difference from Dir::new() and Dir::subdir() is the cleanup
semantics: when a Dir obtained via lookup() is dropped, it calls
dput() to release the reference rather than debugfs_remove() which
would delete the directory.

To implement this cleanup distinction, the Entry class now includes
an is_lookup boolean that specifies how the entry was created and
therefore how it should be dropped.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/kernel/debugfs.rs       | 43 +++++++++++++++++++++++++++++++
 rust/kernel/debugfs/entry.rs | 49 +++++++++++++++++++++++++++++++++---
 2 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index facad81e8290..eee799f64f79 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -110,6 +110,49 @@ pub fn new(name: &CStr) -> Self {
         Dir::create(name, None)
     }
 
+    /// Looks up an existing directory in DebugFS.
+    ///
+    /// If `parent` is [`None`], the lookup is performed from the root of the debugfs filesystem.
+    ///
+    /// Returns [`Some(Dir)`] representing the looked-up directory if found, or [`None`] if the
+    /// directory does not exist or if debugfs is not enabled. When dropped, the [`Dir`] will
+    /// release its reference to the dentry without removing the directory from the filesystem.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// // Look up a top-level directory
+    /// let nova_core = Dir::lookup(c_str!("nova_core"), None);
+    ///
+    /// // Look up a subdirectory within a parent
+    /// let parent = Dir::new(c_str!("parent"));
+    /// let child = parent.subdir(c_str!("child"));
+    /// let looked_up = Dir::lookup(c_str!("child"), Some(&parent));
+    /// // `looked_up` now refers to the same directory as `child`.
+    /// // Dropping `looked_up` will not remove the directory.
+    /// ```
+    pub fn lookup(name: &CStr, parent: Option<&Dir>) -> Option<Self> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            let parent_entry = match parent {
+                // If the parent couldn't be allocated, just early-return
+                Some(Dir(None)) => return None,
+                Some(Dir(Some(entry))) => Some(entry.clone()),
+                None => None,
+            };
+            let entry = Entry::lookup(name, parent_entry)?;
+            Some(Self(
+                // If Arc creation fails, the `Entry` will be dropped, so the reference will be
+                // released.
+                Arc::new(entry, GFP_KERNEL).ok(),
+            ))
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        None
+    }
+
     /// Creates a subdirectory within this directory.
     ///
     /// # Examples
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
index 706cb7f73d6c..455d7bbb8036 100644
--- a/rust/kernel/debugfs/entry.rs
+++ b/rust/kernel/debugfs/entry.rs
@@ -18,6 +18,9 @@ pub(crate) struct Entry<'a> {
     _parent: Option<Arc<Entry<'static>>>,
     // If we were created with a non-owning parent, this prevents us from outliving it
     _phantom: PhantomData<&'a ()>,
+    // If true, this entry was obtained via debugfs_lookup and should be released
+    // with dput() instead of debugfs_remove().
+    is_lookup: bool,
 }
 
 // SAFETY: [`Entry`] is just a `dentry` under the hood, which the API promises can be transferred
@@ -43,9 +46,38 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
             entry,
             _parent: parent,
             _phantom: PhantomData,
+            is_lookup: false,
         }
     }
 
+    /// Looks up an existing entry in debugfs.
+    ///
+    /// Returns [`Some(Entry)`] representing the looked-up dentry if the entry exists, or [`None`]
+    /// if the entry was not found. When dropped, the entry will release its reference via `dput()`
+    /// instead of removing the directory.
+    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.
+        let entry = unsafe { bindings::debugfs_lookup(name.as_char_ptr(), parent_ptr) };
+
+        if entry.is_null() {
+            return None;
+        }
+
+        Some(Entry {
+            entry,
+            _parent: parent,
+            _phantom: PhantomData,
+            is_lookup: true,
+        })
+    }
+
     /// # Safety
     ///
     /// * `data` must outlive the returned `Entry`.
@@ -76,6 +108,7 @@ pub(crate) unsafe fn dynamic_file<T>(
             entry,
             _parent: Some(parent),
             _phantom: PhantomData,
+            is_lookup: false,
         }
     }
 }
@@ -97,6 +130,7 @@ pub(crate) fn dir(name: &CStr, parent: Option<&'a Entry<'_>>) -> Self {
             entry,
             _parent: None,
             _phantom: PhantomData,
+            is_lookup: false,
         }
     }
 
@@ -129,6 +163,7 @@ pub(crate) fn file<T>(
             entry,
             _parent: None,
             _phantom: PhantomData,
+            is_lookup: false,
         }
     }
 }
@@ -140,6 +175,7 @@ pub(crate) fn empty() -> Self {
             entry: core::ptr::null_mut(),
             _parent: None,
             _phantom: PhantomData,
+            is_lookup: false,
         }
     }
 
@@ -157,8 +193,15 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::dentry {
 
 impl Drop for Entry<'_> {
     fn drop(&mut self) {
-        // SAFETY: `debugfs_remove` can take `NULL`, error values, and legal DebugFS dentries.
-        // `as_ptr` guarantees that the pointer is of this form.
-        unsafe { bindings::debugfs_remove(self.as_ptr()) }
+        if self.is_lookup {
+            // SAFETY: `dput` can take `NULL` and legal dentries.
+            // `as_ptr` guarantees that the pointer is of this form.
+            // This entry was obtained via `debugfs_lookup`, so we release the reference.
+            unsafe { bindings::dput(self.as_ptr()) }
+        } else {
+            // SAFETY: `debugfs_remove` can take `NULL`, error values, and legal DebugFS dentries.
+            // `as_ptr` guarantees that the pointer is of this form.
+            unsafe { bindings::debugfs_remove(self.as_ptr()) }
+        }
     }
 }
-- 
2.52.0

Re: [PATCH v3 2/9] rust: debugfs: add lookup contructor

[Danilo Krummrich]

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> Add a new constructor for Dir that uses the debugfs_lookup() API to
> obtain a reference to an existing debugfs directory entry.
>
> The key difference from Dir::new() and Dir::subdir() is the cleanup
> semantics: when a Dir obtained via lookup() is dropped, it calls
> dput() to release the reference rather than debugfs_remove() which
> would delete the directory.
>
> To implement this cleanup distinction, the Entry class now includes
> an is_lookup boolean that specifies how the entry was created and
> therefore how it should be dropped.

I have two main comments about this.

(1) I think it would be better to wrap "lookup entries" into a new type in order
to account for the fact that they do not behave like regular entries. I.e. when
a "lookup entry" is dropped, it does not cause the entry to disappear from the
file system. Analogously, when the regular entry is dropped it disappears from
the file system regardless of whether a "lookup entry" still has a reference
count.

(2) The commit message lacks the motiviation for adding this functionality,
which is only for the purpose of a workaround of an unrelated limitation with
accessing the fields of a module structure (which Gary already works on a
solution for).

I understand it might take a while until the solution Gary works on is ready and
I'm perfectly fine with a temporary workaround. But, this workaround has to be
on nova-core only and not introduce a feature in core code that in the near
future becomes dead code.

If we have a use-case for lookup() elsewhere, I'd like to see it first. But if
we don't I prefer not to add this feature as a pure workaround for something
else.

In fact, I think that debugfs_lookup() rarely is the correct solution and more
often indicates some kind of (design) issue, like in this case.

- Danilo

Re: [PATCH v3 2/9] rust: debugfs: add lookup contructor

[Matthew Maurer]

On Wed, Dec 17, 2025 at 5:40 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> Add a new constructor for Dir that uses the debugfs_lookup() API to
> obtain a reference to an existing debugfs directory entry.
>
> The key difference from Dir::new() and Dir::subdir() is the cleanup
> semantics: when a Dir obtained via lookup() is dropped, it calls
> dput() to release the reference rather than debugfs_remove() which
> would delete the directory.
>
> To implement this cleanup distinction, the Entry class now includes
> an is_lookup boolean that specifies how the entry was created and
> therefore how it should be dropped.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/debugfs.rs       | 43 +++++++++++++++++++++++++++++++
>  rust/kernel/debugfs/entry.rs | 49 +++++++++++++++++++++++++++++++++---
>  2 files changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index facad81e8290..eee799f64f79 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -110,6 +110,49 @@ pub fn new(name: &CStr) -> Self {
>          Dir::create(name, None)
>      }
>
> +    /// Looks up an existing directory in DebugFS.
> +    ///
> +    /// If `parent` is [`None`], the lookup is performed from the root of the debugfs filesystem.
> +    ///
> +    /// Returns [`Some(Dir)`] representing the looked-up directory if found, or [`None`] if the
> +    /// directory does not exist or if debugfs is not enabled. When dropped, the [`Dir`] will
> +    /// release its reference to the dentry without removing the directory from the filesystem.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// // Look up a top-level directory
> +    /// let nova_core = Dir::lookup(c_str!("nova_core"), None);
> +    ///
> +    /// // Look up a subdirectory within a parent
> +    /// let parent = Dir::new(c_str!("parent"));
> +    /// let child = parent.subdir(c_str!("child"));
> +    /// let looked_up = Dir::lookup(c_str!("child"), Some(&parent));
> +    /// // `looked_up` now refers to the same directory as `child`.
> +    /// // Dropping `looked_up` will not remove the directory.
> +    /// ```

This also breaks the assumption people were able to have with `Dir`
previously that if they have a `Dir` and debugfs is enabled, it's
usable for creating subdirs/files.

This might be considered an issue, but at minimum it needs to be documented.

> +    pub fn lookup(name: &CStr, parent: Option<&Dir>) -> Option<Self> {

We were explicitly asked by Greg *not* to expose error conditions in
directory constructors. I would expect that to extend to `lookup` as
well, so this would return a `Self`, not allowing the caller to find
out if the file was actually there. This might be a bit of a grey area
as the comment he cited on `debugfs_create_file` and friends has
explicit verbiage about it being bad form to check errors here and
`debugfs_lookup` mentions explicitly a null return for the file not
being there.

Another point in favor of just returning `Self` rather than
`Option<Self>` is that in your proposed real-world usage of this, you
create a dummy directory in the `None` case.

> +        #[cfg(CONFIG_DEBUG_FS)]
> +        {

We shouldn't need to track a parent entry here, and so shouldn't need
to do the whole cloning dance.

> +            let parent_entry = match parent {
> +                // If the parent couldn't be allocated, just early-return
> +                Some(Dir(None)) => return None,
> +                Some(Dir(Some(entry))) => Some(entry.clone()),
> +                None => None,
> +            };
> +            let entry = Entry::lookup(name, parent_entry)?;

There's no guarantee that this lookup has returned a directory - a
lookup can return files too afaict

> +            Some(Self(
> +                // If Arc creation fails, the `Entry` will be dropped, so the reference will be
> +                // released.
> +                Arc::new(entry, GFP_KERNEL).ok(),
> +            ))
> +        }
> +        #[cfg(not(CONFIG_DEBUG_FS))]
> +        None
> +    }
> +
>      /// Creates a subdirectory within this directory.
>      ///
>      /// # Examples
> diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
> index 706cb7f73d6c..455d7bbb8036 100644
> --- a/rust/kernel/debugfs/entry.rs
> +++ b/rust/kernel/debugfs/entry.rs
> @@ -18,6 +18,9 @@ pub(crate) struct Entry<'a> {
>      _parent: Option<Arc<Entry<'static>>>,
>      // If we were created with a non-owning parent, this prevents us from outliving it
>      _phantom: PhantomData<&'a ()>,
> +    // If true, this entry was obtained via debugfs_lookup and should be released
> +    // with dput() instead of debugfs_remove().
> +    is_lookup: bool,

I agree with Danilo - I think this would be cleaner as a different
type (or at least an enum variant) rather than adding another field
(if we need it). Notably:

1. `_parent` is not required for a lookup entry - the lookup doesn't
need to keep it alive, because it's not using the "If I have the
directory handle, I can create real files in it" semantics we had
before.
2. The `_phantom` is irrelevant for a lookup entry because they can't be scoped
3. The drop behavior is different (which you're gating with `is_lookup`).

Basically the only part that is the same is the presence of a dentry
pointer under the hood.


>  }
>
>  // SAFETY: [`Entry`] is just a `dentry` under the hood, which the API promises can be transferred
> @@ -43,9 +46,38 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
>              entry,
>              _parent: parent,
>              _phantom: PhantomData,
> +            is_lookup: false,
>          }
>      }
>
> +    /// Looks up an existing entry in debugfs.
> +    ///
> +    /// Returns [`Some(Entry)`] representing the looked-up dentry if the entry exists, or [`None`]
> +    /// if the entry was not found. When dropped, the entry will release its reference via `dput()`
> +    /// instead of removing the directory.
> +    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.
> +        let entry = unsafe { bindings::debugfs_lookup(name.as_char_ptr(), parent_ptr) };
> +
> +        if entry.is_null() {
> +            return None;
> +        }
> +
> +        Some(Entry {
> +            entry,
> +            _parent: parent,
> +            _phantom: PhantomData,
> +            is_lookup: true,
> +        })
> +    }
> +
>      /// # Safety
>      ///
>      /// * `data` must outlive the returned `Entry`.
> @@ -76,6 +108,7 @@ pub(crate) unsafe fn dynamic_file<T>(
>              entry,
>              _parent: Some(parent),
>              _phantom: PhantomData,
> +            is_lookup: false,
>          }
>      }
>  }
> @@ -97,6 +130,7 @@ pub(crate) fn dir(name: &CStr, parent: Option<&'a Entry<'_>>) -> Self {
>              entry,
>              _parent: None,
>              _phantom: PhantomData,
> +            is_lookup: false,
>          }
>      }
>
> @@ -129,6 +163,7 @@ pub(crate) fn file<T>(
>              entry,
>              _parent: None,
>              _phantom: PhantomData,
> +            is_lookup: false,
>          }
>      }
>  }
> @@ -140,6 +175,7 @@ pub(crate) fn empty() -> Self {
>              entry: core::ptr::null_mut(),
>              _parent: None,
>              _phantom: PhantomData,
> +            is_lookup: false,
>          }
>      }
>
> @@ -157,8 +193,15 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::dentry {
>
>  impl Drop for Entry<'_> {
>      fn drop(&mut self) {
> -        // SAFETY: `debugfs_remove` can take `NULL`, error values, and legal DebugFS dentries.
> -        // `as_ptr` guarantees that the pointer is of this form.
> -        unsafe { bindings::debugfs_remove(self.as_ptr()) }
> +        if self.is_lookup {
> +            // SAFETY: `dput` can take `NULL` and legal dentries.
> +            // `as_ptr` guarantees that the pointer is of this form.
> +            // This entry was obtained via `debugfs_lookup`, so we release the reference.
> +            unsafe { bindings::dput(self.as_ptr()) }
> +        } else {
> +            // SAFETY: `debugfs_remove` can take `NULL`, error values, and legal DebugFS dentries.
> +            // `as_ptr` guarantees that the pointer is of this form.
> +            unsafe { bindings::debugfs_remove(self.as_ptr()) }
> +        }
>      }
>  }
> --
> 2.52.0
>
>

[PATCH v3 3/9] rust: debugfs: add Dir::empty() for no-op directory handle

[Timur Tabi]

Add a Dir::empty() constructor that returns a Dir representing no
directory. Operations on this directory (creating files, subdirectories)
become no-ops, but the data is still stored.

This is useful as a fallback when a directory lookup fails but you still
need a Dir value to pass to APIs like Dir::scope(). For example:

    let dir = Dir::lookup(c_str!("parent"), None)
        .unwrap_or_else(Dir::empty);

When CONFIG_DEBUG_FS is enabled, returns Dir(None). When disabled,
returns the empty tuple struct Dir().

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

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index eee799f64f79..b0cfe22982d6 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -110,6 +110,30 @@ pub fn new(name: &CStr) -> Self {
         Dir::create(name, None)
     }
 
+    /// Creates an empty [`Dir`] that represents no directory.
+    ///
+    /// Operations on this directory (such as creating files or subdirectories) will be no-ops.
+    /// This is useful as a fallback when a directory lookup fails but you still need a [`Dir`]
+    /// value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// let dir = Dir::lookup(c_str!("maybe_exists"), None)
+    ///     .unwrap_or_else(Dir::empty);
+    /// // If lookup failed, file creation becomes a no-op
+    /// ```
+    pub fn empty() -> Self {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            Self(None)
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        Self()
+    }
+
     /// Looks up an existing directory in DebugFS.
     ///
     /// If `parent` is [`None`], the lookup is performed from the root of the debugfs filesystem.
-- 
2.52.0

[PATCH v3 4/9] rust: debugfs: fix Dir::scope() to not borrow self for returned lifetime

[Timur Tabi]

Dir::scope() was declared as taking &'a self, which tied the returned
PinInit's lifetime to the Dir. This prevented using scope() with a
locally-created Dir:

    let dir = Dir::lookup(...).unwrap_or_else(Dir::empty);
    let scope = dir.scope(...);  // Error: returns value referencing local

The borrow was unnecessary since scoped_dir() internally clones the
Arc<Entry>. Fix this by cloning self.0 before the closure, allowing the
closure to capture the cloned value via move instead of borrowing self.

This also removes the now-unused scoped_dir() helper method, inlining
its logic directly into scope().

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

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index b0cfe22982d6..35f9cbb261e7 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -397,27 +397,6 @@ pub fn write_callback_file<'a, T, E: 'a, W>(
         self.create_file(name, data, file_ops)
     }
 
-    // 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 `Dir` 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(name, Some(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
@@ -427,7 +406,7 @@ fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
     /// The entire directory tree created within the scope will be removed when the returned
     /// `Scope` handle is dropped.
     pub fn scope<'a, T: 'a, E: 'a, F>(
-        &'a self,
+        &self,
         data: impl PinInit<T, E> + 'a,
         name: &'a CStr,
         init: F,
@@ -435,8 +414,22 @@ pub fn scope<'a, T: 'a, E: 'a, F>(
     where
         F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
     {
-        Scope::new(data, |data| {
-            let scoped = self.scoped_dir(name);
+        // Clone the parent entry so the closure doesn't need to borrow `self`.
+        #[cfg(CONFIG_DEBUG_FS)]
+        let parent_entry = self.0.clone();
+
+        Scope::new(data, move |data| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            let scoped = match parent_entry {
+                None => ScopedDir::empty(),
+                Some(entry) => ScopedDir {
+                    entry: ManuallyDrop::new(Entry::dynamic_dir(name, Some(entry))),
+                    _phantom: PhantomData,
+                },
+            };
+            #[cfg(not(CONFIG_DEBUG_FS))]
+            let scoped = ScopedDir::empty();
+
             init(data, &scoped);
             scoped.into_entry()
         })
-- 
2.52.0

Re: [PATCH v3 4/9] rust: debugfs: fix Dir::scope() to not borrow self for returned lifetime

[Matthew Maurer]

On Wed, Dec 17, 2025 at 5:49 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> Dir::scope() was declared as taking &'a self, which tied the returned
> PinInit's lifetime to the Dir. This prevented using scope() with a
> locally-created Dir:
>
>     let dir = Dir::lookup(...).unwrap_or_else(Dir::empty);
>     let scope = dir.scope(...);  // Error: returns value referencing local

This looks like you're trying to return an `impl PinInit<Scope>` from
your function (which isn't written in the example, but I assume that's
what you eventually do, otherwise this wouldn't be an error).

The generalized solution to this is to use `pin_init_scope`, i.e.

```
pin_init_scope(|| {
  let dir = // Generate a `Dir` somehow here
  let scope = dir.scope(/* Scope defining function here */)
  // Anything else you wanted to do
  Ok(scope)
})
```

>
> The borrow was unnecessary since scoped_dir() internally clones the
> Arc<Entry>. Fix this by cloning self.0 before the closure, allowing the
> closure to capture the cloned value via move instead of borrowing self.
>
> This also removes the now-unused scoped_dir() helper method, inlining
> its logic directly into scope().
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/debugfs.rs | 41 +++++++++++++++++------------------------
>  1 file changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index b0cfe22982d6..35f9cbb261e7 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -397,27 +397,6 @@ pub fn write_callback_file<'a, T, E: 'a, W>(
>          self.create_file(name, data, file_ops)
>      }
>
> -    // 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 `Dir` 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(name, Some(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
> @@ -427,7 +406,7 @@ fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
>      /// The entire directory tree created within the scope will be removed when the returned
>      /// `Scope` handle is dropped.
>      pub fn scope<'a, T: 'a, E: 'a, F>(
> -        &'a self,
> +        &self,
>          data: impl PinInit<T, E> + 'a,
>          name: &'a CStr,
>          init: F,
> @@ -435,8 +414,22 @@ pub fn scope<'a, T: 'a, E: 'a, F>(
>      where
>          F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
>      {
> -        Scope::new(data, |data| {
> -            let scoped = self.scoped_dir(name);
> +        // Clone the parent entry so the closure doesn't need to borrow `self`.
> +        #[cfg(CONFIG_DEBUG_FS)]
> +        let parent_entry = self.0.clone();

This isn't necessarily a show-stopper, but this lets us create a weird
intermediate state where from the user's PoV, there are no references
to a directory (they don't have the `Dir` anymore, and a `Scope`
doesn't exist yet, just a `PinInit<Scope>`), but the directory is held
alive. It feels weird, but I suppose users won't usually have a
`PinInit<Scope>` around for long.

The weirdness in debugging would look like:
1. Create dir
2. See dir in filesystem
3. Call `scope`, but don't actually activate the PinInit yet (either
due to coding error or because the target data structure won't be
available until later.
4. See no subdirectories under dir in filesystem
5. Drop `dir`
6. See dir still hanging around

This would also happen with my `pin_init_scope` suggestion, but at
least then user code would be explicitly capturing the `Dir` right in
front of them in the existing-dir case, and in the "create-or-lookup
new dir" case like you've got here, would delay directory creation
until we're actually instantiating the directory somewhere.

> +
> +        Scope::new(data, move |data| {
> +            #[cfg(CONFIG_DEBUG_FS)]
> +            let scoped = match parent_entry {
> +                None => ScopedDir::empty(),
> +                Some(entry) => ScopedDir {
> +                    entry: ManuallyDrop::new(Entry::dynamic_dir(name, Some(entry))),
> +                    _phantom: PhantomData,
> +                },
> +            };
> +            #[cfg(not(CONFIG_DEBUG_FS))]
> +            let scoped = ScopedDir::empty();
> +
>              init(data, &scoped);
>              scoped.into_entry()
>          })
> --
> 2.52.0
>
>


On Wed, Dec 17, 2025 at 5:49 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> Dir::scope() was declared as taking &'a self, which tied the returned
> PinInit's lifetime to the Dir. This prevented using scope() with a
> locally-created Dir:
>
>     let dir = Dir::lookup(...).unwrap_or_else(Dir::empty);
>     let scope = dir.scope(...);  // Error: returns value referencing local
>
> The borrow was unnecessary since scoped_dir() internally clones the
> Arc<Entry>. Fix this by cloning self.0 before the closure, allowing the
> closure to capture the cloned value via move instead of borrowing self.
>
> This also removes the now-unused scoped_dir() helper method, inlining
> its logic directly into scope().
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/debugfs.rs | 41 +++++++++++++++++------------------------
>  1 file changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index b0cfe22982d6..35f9cbb261e7 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -397,27 +397,6 @@ pub fn write_callback_file<'a, T, E: 'a, W>(
>          self.create_file(name, data, file_ops)
>      }
>
> -    // 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 `Dir` 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(name, Some(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
> @@ -427,7 +406,7 @@ fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
>      /// The entire directory tree created within the scope will be removed when the returned
>      /// `Scope` handle is dropped.
>      pub fn scope<'a, T: 'a, E: 'a, F>(
> -        &'a self,
> +        &self,
>          data: impl PinInit<T, E> + 'a,
>          name: &'a CStr,
>          init: F,
> @@ -435,8 +414,22 @@ pub fn scope<'a, T: 'a, E: 'a, F>(
>      where
>          F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
>      {
> -        Scope::new(data, |data| {
> -            let scoped = self.scoped_dir(name);
> +        // Clone the parent entry so the closure doesn't need to borrow `self`.
> +        #[cfg(CONFIG_DEBUG_FS)]
> +        let parent_entry = self.0.clone();
> +
> +        Scope::new(data, move |data| {
> +            #[cfg(CONFIG_DEBUG_FS)]
> +            let scoped = match parent_entry {
> +                None => ScopedDir::empty(),
> +                Some(entry) => ScopedDir {
> +                    entry: ManuallyDrop::new(Entry::dynamic_dir(name, Some(entry))),
> +                    _phantom: PhantomData,
> +                },
> +            };
> +            #[cfg(not(CONFIG_DEBUG_FS))]
> +            let scoped = ScopedDir::empty();
> +
>              init(data, &scoped);
>              scoped.into_entry()
>          })
> --
> 2.52.0
>
>

[PATCH v3 5/9] gpu: nova-core: Replace module_pci_driver! with explicit module init

[Timur Tabi]

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 | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b98a1c03f13d..01353be103ca 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,9 @@
 
 //! Nova Core GPU Driver
 
+use kernel::{error::Error, pci, prelude::*, InPlaceModule};
+use pin_init::{PinInit, pinned_drop};
+
 #[macro_use]
 mod bitfield;
 
@@ -21,13 +24,27 @@
 
 pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 
-kernel::module_pci_driver! {
-    type: driver::NovaCore,
+#[pin_data(PinnedDrop)]
+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> {
+        pr_info!("NovaCore GPU driver loaded\n");
+        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

Re: [PATCH v3 5/9] gpu: nova-core: Replace module_pci_driver! with explicit module init

[Danilo Krummrich]

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index b98a1c03f13d..01353be103ca 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,6 +2,9 @@
>  
>  //! Nova Core GPU Driver
>  
> +use kernel::{error::Error, pci, prelude::*, InPlaceModule};
> +use pin_init::{PinInit, pinned_drop};

Please use kernel vertical style [1].

[1] https://docs.kernel.org/rust/coding-guidelines.html#imports

> +
>  #[macro_use]
>  mod bitfield;
>  
> @@ -21,13 +24,27 @@
>  
>  pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>  
> -kernel::module_pci_driver! {
> -    type: driver::NovaCore,
> +#[pin_data(PinnedDrop)]
> +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> {
> +        pr_info!("NovaCore GPU driver loaded\n");

Please don't add such print statements, drivers should be silent when they work
properly.

> +        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: [],

This change seems unrelated, please send a separate patch for this.

>  }
>  
>  kernel::module_firmware!(firmware::ModInfoBuilder);
> -- 
> 2.52.0

[PATCH v3 6/9] gpu: nova-core: create debugfs root when driver loads

[Timur Tabi]

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.

Field order in Rust structs determines drop order - fields are dropped
in declaration order. By placing _driver before _debugfs_root:

1. The driver registration is dropped first, which removes all PCI
   devices and their associated debugfs subdirectories
2. Then _debugfs_root is dropped, removing the now-empty nova_core
   directory

This ordering is critical because debugfs_remove() performs recursive
removal. If the parent directory were removed first, it would free the
child dentries, and then the child's drop would attempt to remove an
already-freed dentry, causing a use-after-free crash.

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

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 01353be103ca..f17505e5e2b3 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,8 +2,7 @@
 
 //! Nova Core GPU Driver
 
-use kernel::{error::Error, pci, prelude::*, InPlaceModule};
-use pin_init::{PinInit, pinned_drop};
+use kernel::{debugfs::Dir, error::Error, pci, prelude::*, InPlaceModule};
 
 #[macro_use]
 mod bitfield;
@@ -24,17 +23,24 @@
 
 pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 
-#[pin_data(PinnedDrop)]
+#[pin_data]
 struct NovaCoreModule {
+    // Note: field order matters for drop order. The driver must be dropped before
+    // the debugfs directory so that device subdirectories are removed first.
     #[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> {
         pr_info!("NovaCore GPU driver loaded\n");
+
+        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

Re: [PATCH v3 6/9] gpu: nova-core: create debugfs root when driver loads

[Danilo Krummrich]

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> 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.
>
> Field order in Rust structs determines drop order - fields are dropped
> in declaration order. By placing _driver before _debugfs_root:
>
> 1. The driver registration is dropped first, which removes all PCI
>    devices and their associated debugfs subdirectories
> 2. Then _debugfs_root is dropped, removing the now-empty nova_core
>    directory
>
> This ordering is critical because debugfs_remove() performs recursive
> removal. If the parent directory were removed first, it would free the
> child dentries, and then the child's drop would attempt to remove an
> already-freed dentry, causing a use-after-free crash.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/nova_core.rs | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index 01353be103ca..f17505e5e2b3 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,8 +2,7 @@
>  
>  //! Nova Core GPU Driver
>  
> -use kernel::{error::Error, pci, prelude::*, InPlaceModule};
> -use pin_init::{PinInit, pinned_drop};
> +use kernel::{debugfs::Dir, error::Error, pci, prelude::*, InPlaceModule};

Again, please use kernel vertical style.

>  
>  #[macro_use]
>  mod bitfield;
> @@ -24,17 +23,24 @@
>  
>  pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>  
> -#[pin_data(PinnedDrop)]
> +#[pin_data]
>  struct NovaCoreModule {
> +    // Note: field order matters for drop order. The driver must be dropped before
> +    // the debugfs directory so that device subdirectories are removed first.

This is not correct, subdirectories have a reference count of their parent
directory. Hence, the drop order does not matter.

>      #[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> {
>          pr_info!("NovaCore GPU driver loaded\n");
> +
> +        let debugfs_root = Dir::new(kernel::c_str!("nova_core"));

I'd just inline this statement below.

> +
>          try_pin_init!(Self {
>              _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
> +            _debugfs_root: debugfs_root,
>          })
>      }
>  }
> -- 
> 2.52.0

[PATCH v3 7/9] gpu: nova-core: implement BinaryWriter for LogBuffer

[Timur Tabi]

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 fb6f74797178..860674dac31e 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>) -> Result<impl PinInit<Self, Error>> {
-- 
2.52.0

Re: [PATCH v3 7/9] gpu: nova-core: implement BinaryWriter for LogBuffer

[Danilo Krummrich]

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> 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 fb6f74797178..860674dac31e 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()) }?;

Unfortunately, it's still undefined behavior to create a slice when the backing
buffer can change unexpectedly.

You can use UserSliceWriter::write() instead or add an unsafe method to
UserSliceWriter that takes a raw pointer, length and offset.

Given the context of a debug log buffer, I suggest the former.

> +        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>) -> Result<impl PinInit<Self, Error>> {
> -- 
> 2.52.0

Re: [PATCH v3 7/9] gpu: nova-core: implement BinaryWriter for LogBuffer

[Alexandre Courbot]

On Thu Dec 18, 2025 at 10:39 AM JST, Timur Tabi wrote:
> 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 fb6f74797178..860674dac31e 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 {}
> +

Did I actually write all this? I don't have any recollection of
submitting such a patch (and the safety comments don't look like my
style).

[PATCH v3 8/9] gpu: nova-core: use pin projection in method boot()

[Timur Tabi]

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 | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 54937606b5b0..1db1099bd344 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -162,12 +162,13 @@ 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
-            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
-        self.cmdq.send_command(bar, commands::SetRegistry::new())?;
+        let this = self.as_mut().project();
+
+        this.cmdq.send_command(bar, commands::SetSystemInfo::new(pdev))?;
+        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),
@@ -234,13 +235,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)?;
         dev_info!(
             pdev.as_ref(),
             "GPU name: {}\n",
-- 
2.52.0

[PATCH v3 9/9] gpu: nova-core: create GSP-RM logging buffers debugfs entries

[Timur Tabi]

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 | 42 +++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 860674dac31e..409ff80f55e6 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -3,6 +3,7 @@
 mod boot;
 
 use kernel::{
+    c_str,
     debugfs,
     device,
     dma::{
@@ -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.
@@ -143,7 +151,9 @@ 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>) -> Result<impl PinInit<Self, Error>> {
+    pub(crate) fn new<'a>(
+        pdev: &'a pci::Device<device::Bound>,
+    ) -> Result<impl PinInit<Self, Error> + 'a> {
         let dev = pdev.as_ref();
         let libos = CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
             dev,
@@ -173,11 +183,27 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
         dma_write!(rmargs[0] = fw::GspArgumentsCached::new(&cmdq))?;
         dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs))?;
 
-        Ok(try_pin_init!(Self {
-            libos,
+        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::Dir::lookup(c_str!("nova_core"), None)
+            .unwrap_or_else(debugfs::Dir::empty);
+
+        let logs = 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);
+        });
+
+        Ok(try_pin_init!(Self {
+            libos,
+            logs <- logs,
             rmargs,
             cmdq,
         }))
-- 
2.52.0

Re: [PATCH v3 9/9] gpu: nova-core: create GSP-RM logging buffers debugfs entries

[kernel test robot]

Hi Timur,

kernel test robot noticed the following build errors:

[auto build test ERROR on ea1013c1539270e372fc99854bc6e4d94eaeff66]

url:    https://github.com/intel-lab-lkp/linux/commits/Timur-Tabi/rust-pci-add-PCI-device-name-method/20251218-121537
base:   ea1013c1539270e372fc99854bc6e4d94eaeff66
patch link:    https://lore.kernel.org/r/20251218013910.459045-10-ttabi%40nvidia.com
patch subject: [PATCH v3 9/9] gpu: nova-core: create GSP-RM logging buffers debugfs entries
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20251221/202512211136.RZfacOxt-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/20251221/202512211136.RZfacOxt-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/202512211136.RZfacOxt-lkp@intel.com/

All errors (new ones prefixed by >>):

   PATH=/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   INFO PATH=/opt/cross/rustc-1.88.0-bindgen-0.72.1/cargo/bin:/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   /usr/bin/timeout -k 100 12h /usr/bin/make KCFLAGS= -fno-crash-diagnostics -Wno-error=return-type -Wreturn-type -funsigned-char -Wundef W=1 --keep-going LLVM=1 -j32 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck
   make: Entering directory '/kbuild/src/consumer'
   make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
   Diff in drivers/gpu/nova-core/gsp/boot.rs:164:
    
            let this = self.as_mut().project();
    
   -        this.cmdq.send_command(bar, commands::SetSystemInfo::new(pdev))?;
   +        this.cmdq
   +            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
            this.cmdq.send_command(bar, commands::SetRegistry::new())?;
    
            gsp_falcon.reset(bar)?;
   Diff in drivers/gpu/nova-core/gsp/boot.rs:164:
    
            let this = self.as_mut().project();
    
   -        this.cmdq.send_command(bar, commands::SetSystemInfo::new(pdev))?;
   +        this.cmdq
   +            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
            this.cmdq.send_command(bar, commands::SetRegistry::new())?;
    
            gsp_falcon.reset(bar)?;
>> Diff in drivers/gpu/nova-core/gsp.rs:192:
            // 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::Dir::lookup(c_str!("nova_core"), None)
   -            .unwrap_or_else(debugfs::Dir::empty);
   +        let debugfs_dir =
   +            debugfs::Dir::lookup(c_str!("nova_core"), None).unwrap_or_else(debugfs::Dir::empty);
    
            let logs = debugfs_dir.scope(log_buffers, pdev.name(), |logs, dir| {
                dir.read_binary_file(c_str!("loginit"), &logs.loginit);
   Diff in drivers/gpu/nova-core/gsp/boot.rs:164:
    
            let this = self.as_mut().project();
    
   -        this.cmdq.send_command(bar, commands::SetSystemInfo::new(pdev))?;
   +        this.cmdq
   +            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
            this.cmdq.send_command(bar, commands::SetRegistry::new())?;
    
            gsp_falcon.reset(bar)?;
>> Diff in drivers/gpu/nova-core/gsp.rs:192:
            // 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::Dir::lookup(c_str!("nova_core"), None)
   -            .unwrap_or_else(debugfs::Dir::empty);
   +        let debugfs_dir =
   +            debugfs::Dir::lookup(c_str!("nova_core"), None).unwrap_or_else(debugfs::Dir::empty);
    
            let logs = debugfs_dir.scope(log_buffers, pdev.name(), |logs, dir| {
                dir.read_binary_file(c_str!("loginit"), &logs.loginit);
   make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
   make[2]: *** [Makefile:1871: rustfmt] Error 123
   make[2]: Target 'rustfmtcheck' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'rustfmtcheck' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'rustfmtcheck' not remade because of errors.
   make: Leaving directory '/kbuild/src/consumer'

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

Re: [PATCH v3 0/9] gpu: nova-core: expose the logging buffers via debugfs

[Alice Ryhl]

On Wed, Dec 17, 2025 at 07:39:01PM -0600, Timur Tabi wrote:
> 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.
> 
> Based on Linus' repository, not drm-rust-next.
> 
> Summary of changes:
> 
> 1. Replace module_pci_driver! with explicit init function.
> 2. Creates root, per-gpu, and individual buffer debugfs entries.
> 3. Adds a pci::name() method to generate a PCI device name string.
> 4. Adds a lookup() method to debugfs, so that the root debugfs entry
>    doesn't need to be a global variable like it is in Nouveau.
> 5. Makes some updates to debugfs core code.
> 
> Alexandre Courbot (1):
>   gpu: nova-core: implement BinaryWriter for LogBuffer
> 
> Timur Tabi (8):
>   rust: pci: add PCI device name method
>   rust: debugfs: add lookup contructor
>   rust: debugfs: add Dir::empty() for no-op directory handle
>   rust: debugfs: fix Dir::scope() to not borrow self for returned
>     lifetime
>   gpu: nova-core: Replace module_pci_driver! with explicit module init
>   gpu: nova-core: create debugfs root when driver loads
>   gpu: nova-core: use pin projection in method boot()
>   gpu: nova-core: create GSP-RM logging buffers debugfs entries
> 
>  drivers/gpu/nova-core/gsp.rs       |  66 +++++++++++++++---
>  drivers/gpu/nova-core/gsp/boot.rs  |  15 ++--
>  drivers/gpu/nova-core/nova_core.rs |  29 +++++++-
>  rust/kernel/debugfs.rs             | 108 ++++++++++++++++++++++-------
>  rust/kernel/debugfs/entry.rs       |  49 ++++++++++++-
>  rust/kernel/pci.rs                 |  37 ++++++++++
>  6 files changed, 259 insertions(+), 45 deletions(-)

Adding Matthew as this touches debugfs.

Alice

Re: [PATCH v3 0/9] gpu: nova-core: expose the logging buffers via debugfs

[Danilo Krummrich]

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> Summary of changes:
>
> 1. Replace module_pci_driver! with explicit init function.
> 2. Creates root, per-gpu, and individual buffer debugfs entries.
> 3. Adds a pci::name() method to generate a PCI device name string.
> 4. Adds a lookup() method to debugfs, so that the root debugfs entry
>    doesn't need to be a global variable like it is in Nouveau.
> 5. Makes some updates to debugfs core code.

This is nice, but given that this is a v3, where is the changelog?