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

[PATCH v5 0/8] 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.

Changes since v4:
1. Replaced all debugfs_lookup() code with the original global DEBUGFS_ROOT variable.
2. Added a Dir::empty() to support !DEBUGFS
3. Added UserSliceWriter::write_buffer() to avoid creating a slice over the log buffer.
4. Replaced pci::name() with device::name()

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

Timur Tabi (7):
  rust: pci: add device name method
  rust: debugfs: add Dir::empty() for conditional debugfs usage
  rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer
    writes
  gpu: nova-core: Replace module_pci_driver! with explicit module init
  gpu: nova-core: use pin projection in method boot()
  gpu: nova-core: create debugfs root in module init
  gpu: nova-core: create GSP-RM logging buffers debugfs entries

 drivers/gpu/nova-core/gsp.rs       | 85 +++++++++++++++++++++++++++---
 drivers/gpu/nova-core/gsp/boot.rs  | 14 ++---
 drivers/gpu/nova-core/nova_core.rs | 44 ++++++++++++++--
 rust/helpers/device.c              |  5 ++
 rust/kernel/debugfs.rs             | 18 +++++++
 rust/kernel/device.rs              | 16 ++++++
 rust/kernel/uaccess.rs             | 50 ++++++++++++++++++
 7 files changed, 215 insertions(+), 17 deletions(-)


base-commit: 654826aa4a8f25cf825ad9254f37e6cb5092098f
-- 
2.52.0

[PATCH v5 1/8] rust: pci: add device name method

[Timur Tabi]

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

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/helpers/device.c |  5 +++++
 rust/kernel/device.rs | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/rust/helpers/device.c b/rust/helpers/device.c
index 9a4316bafedf..4609b62f6a06 100644
--- a/rust/helpers/device.c
+++ b/rust/helpers/device.c
@@ -25,3 +25,8 @@ void rust_helper_dev_set_drvdata(struct device *dev, void *data)
 {
 	dev_set_drvdata(dev, data);
 }
+
+const char *rust_helper_dev_name(const struct device *dev)
+{
+	return dev_name(dev);
+}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index c79be2e2bfe3..636c522a8374 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -483,6 +483,22 @@ pub fn fwnode(&self) -> Option<&property::FwNode> {
         // defined as a `#[repr(transparent)]` wrapper around `fwnode_handle`.
         Some(unsafe { &*fwnode_handle.cast() })
     }
+
+    /// Returns the name of the device.
+    ///
+    /// This is the kobject name of the device, or its initial name if the kobject is not yet
+    /// available.
+    ///
+    /// For PCI devices, the 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)
+    pub fn name(&self) -> &CStr {
+        // SAFETY: By its type invariant `self.as_raw()` is a valid pointer to a `struct device`.
+        // The returned string is valid for the lifetime of the device.
+        unsafe { CStr::from_char_ptr(bindings::dev_name(self.as_raw())) }
+    }
 }
 
 // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
-- 
2.52.0

Re: [PATCH v5 1/8] rust: pci: add device name method

[Danilo Krummrich]

On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
> Add a name() method to the `Device` type, which returns a CStr that
> contains the device name, typically the BDF address for PCI devices.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/helpers/device.c |  5 +++++
>  rust/kernel/device.rs | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/rust/helpers/device.c b/rust/helpers/device.c
> index 9a4316bafedf..4609b62f6a06 100644
> --- a/rust/helpers/device.c
> +++ b/rust/helpers/device.c
> @@ -25,3 +25,8 @@ void rust_helper_dev_set_drvdata(struct device *dev, void *data)
>  {
>  	dev_set_drvdata(dev, data);
>  }
> +
> +const char *rust_helper_dev_name(const struct device *dev)

Please add the __rust_helper annotation.

> +{
> +	return dev_name(dev);
> +}
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index c79be2e2bfe3..636c522a8374 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -483,6 +483,22 @@ pub fn fwnode(&self) -> Option<&property::FwNode> {
>          // defined as a `#[repr(transparent)]` wrapper around `fwnode_handle`.
>          Some(unsafe { &*fwnode_handle.cast() })
>      }
> +
> +    /// Returns the name of the device.
> +    ///
> +    /// This is the kobject name of the device, or its initial name if the kobject is not yet
> +    /// available.
> +    ///
> +    /// For PCI devices, the name in the format "DDDD:BB:DD.F" where:

This is driver core code, please drop any details for PCI. Also, the subject of
the patch seems wrong.

> +    /// - 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)
> +    pub fn name(&self) -> &CStr {
> +        // SAFETY: By its type invariant `self.as_raw()` is a valid pointer to a `struct device`.
> +        // The returned string is valid for the lifetime of the device.
> +        unsafe { CStr::from_char_ptr(bindings::dev_name(self.as_raw())) }
> +    }
>  }
>  
>  // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
> -- 
> 2.52.0

Re: [PATCH v5 1/8] rust: pci: add device name method

[Timur Tabi]

On Sat, 2026-01-17 at 12:09 +0100, Danilo Krummrich wrote:
> 

> > @@ -25,3 +25,8 @@ void rust_helper_dev_set_drvdata(struct device *dev, void *data)
> >  {
> >  	dev_set_drvdata(dev, data);
> >  }
> > +
> > +const char *rust_helper_dev_name(const struct device *dev)
> 
> Please add the __rust_helper annotation.

Should I add it to all of the functions in this file?  None of the existing functions have
"__rust_helper".

> 
> > +
> > +    /// Returns the name of the device.
> > +    ///
> > +    /// This is the kobject name of the device, or its initial name if the kobject is not
> > yet
> > +    /// available.
> > +    ///
> > +    /// For PCI devices, the name in the format "DDDD:BB:DD.F" where:
> 
> This is driver core code, please drop any details for PCI. Also, the subject of
> the patch seems wrong.

Ok.

Re: [PATCH v5 1/8] rust: pci: add device name method

[Danilo Krummrich]

On Tue Jan 27, 2026 at 10:04 PM CET, Timur Tabi wrote:
> On Sat, 2026-01-17 at 12:09 +0100, Danilo Krummrich wrote:
>> 
>
>> > @@ -25,3 +25,8 @@ void rust_helper_dev_set_drvdata(struct device *dev, void *data)
>> >  {
>> >  	dev_set_drvdata(dev, data);
>> >  }
>> > +
>> > +const char *rust_helper_dev_name(const struct device *dev)
>> 
>> Please add the __rust_helper annotation.
>
> Should I add it to all of the functions in this file?  None of the existing functions have
> "__rust_helper".

This is only because your base is from a different tree, within the
driver-core-next branch they should all have this annotation already. So, just
add it to the new one you introduce please.

[PATCH v5 2/8] rust: debugfs: add Dir::empty() for conditional debugfs usage

[Timur Tabi]

Add Dir::empty() which returns a &'static reference to a no-op Dir.
All file and subdirectory operations on this directory silently succeed
without creating actual filesystem entries.

This enables callers to use a single code path regardless of whether
debugfs is available at runtime. For example:

    let dir = optional_debugfs_root.unwrap_or_else(|| Dir::empty());
    dir.scope(data, name, |d, s| { ... })

The returned reference has 'static lifetime, allowing initializers
returned by scope() to be used outside the immediate scope where the
Dir reference was obtained.

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

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index facad81e8290..44ff2b37786a 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -110,6 +110,24 @@ pub fn new(name: &CStr) -> Self {
         Dir::create(name, None)
     }
 
+    /// Returns a reference to a static no-op directory that doesn't create any debugfs entries.
+    ///
+    /// All file and subdirectory creation operations on this directory will silently succeed
+    /// without creating actual filesystem entries. This is useful when you want to conditionally
+    /// enable debugfs but use the same code path regardless.
+    pub fn empty() -> &'static Self {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            static EMPTY: Dir = Dir(None);
+            &EMPTY
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        {
+            static EMPTY: Dir = Dir();
+            &EMPTY
+        }
+    }
+
     /// Creates a subdirectory within this directory.
     ///
     /// # Examples
-- 
2.52.0

Re: [PATCH v5 2/8] rust: debugfs: add Dir::empty() for conditional debugfs usage

[Gary Guo]

On Fri Jan 16, 2026 at 9:49 PM GMT, Timur Tabi wrote:
> Add Dir::empty() which returns a &'static reference to a no-op Dir.
> All file and subdirectory operations on this directory silently succeed
> without creating actual filesystem entries.
>
> This enables callers to use a single code path regardless of whether
> debugfs is available at runtime. For example:
>
>     let dir = optional_debugfs_root.unwrap_or_else(|| Dir::empty());
>     dir.scope(data, name, |d, s| { ... })
>
> The returned reference has 'static lifetime, allowing initializers
> returned by scope() to be used outside the immediate scope where the
> Dir reference was obtained.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/debugfs.rs | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index facad81e8290..44ff2b37786a 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -110,6 +110,24 @@ pub fn new(name: &CStr) -> Self {
>          Dir::create(name, None)
>      }
>  
> +    /// Returns a reference to a static no-op directory that doesn't create any debugfs entries.
> +    ///
> +    /// All file and subdirectory creation operations on this directory will silently succeed
> +    /// without creating actual filesystem entries. This is useful when you want to conditionally
> +    /// enable debugfs but use the same code path regardless.
> +    pub fn empty() -> &'static Self {
> +        #[cfg(CONFIG_DEBUG_FS)]
> +        {
> +            static EMPTY: Dir = Dir(None);
> +            &EMPTY
> +        }
> +        #[cfg(not(CONFIG_DEBUG_FS))]
> +        {
> +            static EMPTY: Dir = Dir();
> +            &EMPTY
> +        }

You can apply `#[cfg]` on the static item. The `&EMPTY` part doesn't need to be
inside cfg.

Best,
Gary

> +    }
> +
>      /// Creates a subdirectory within this directory.
>      ///
>      /// # Examples

Re: [PATCH v5 2/8] rust: debugfs: add Dir::empty() for conditional debugfs usage

[Matthew Maurer]

On Fri, Jan 16, 2026 at 1:50 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> Add Dir::empty() which returns a &'static reference to a no-op Dir.
> All file and subdirectory operations on this directory silently succeed
> without creating actual filesystem entries.

I thought the whole point of Greg's admonition to never report errors
and to automatically create no-op structures if debugfs was disable
was that we *don't* do things like this, and instead you just do
something like

```
// Always do this, whether or not debugfs is available.
let debugfs_root = Dir::new("my_thing");
debugfs_root.scope(data, name, |d, s| { ... })
```

If DebugFS is not available, it already returns an empty fake
directory. That variant of a fake directory is also size 0, so you
aren't paying anything to use it...

>
> This enables callers to use a single code path regardless of whether
> debugfs is available at runtime. For example:
>
>     let dir = optional_debugfs_root.unwrap_or_else(|| Dir::empty());
>     dir.scope(data, name, |d, s| { ... })
>
> The returned reference has 'static lifetime, allowing initializers
> returned by scope() to be used outside the immediate scope where the
> Dir reference was obtained.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/debugfs.rs | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index facad81e8290..44ff2b37786a 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -110,6 +110,24 @@ pub fn new(name: &CStr) -> Self {
>          Dir::create(name, None)
>      }
>
> +    /// Returns a reference to a static no-op directory that doesn't create any debugfs entries.
> +    ///
> +    /// All file and subdirectory creation operations on this directory will silently succeed
> +    /// without creating actual filesystem entries. This is useful when you want to conditionally
> +    /// enable debugfs but use the same code path regardless.
> +    pub fn empty() -> &'static Self {
> +        #[cfg(CONFIG_DEBUG_FS)]
> +        {
> +            static EMPTY: Dir = Dir(None);
> +            &EMPTY
> +        }
> +        #[cfg(not(CONFIG_DEBUG_FS))]
> +        {
> +            static EMPTY: Dir = Dir();
> +            &EMPTY
> +        }
> +    }
> +
>      /// Creates a subdirectory within this directory.
>      ///
>      /// # Examples
> --
> 2.52.0
>

Re: [PATCH v5 2/8] rust: debugfs: add Dir::empty() for conditional debugfs usage

[Danilo Krummrich]

On Tue Jan 20, 2026 at 6:54 PM CET, Matthew Maurer wrote:
> I thought the whole point of Greg's admonition to never report errors
> and to automatically create no-op structures if debugfs was disable
> was that we *don't* do things like this, and instead you just do
> something like
>
> ```
> // Always do this, whether or not debugfs is available.
> let debugfs_root = Dir::new("my_thing");
> debugfs_root.scope(data, name, |d, s| { ... })
> ```
>
> If DebugFS is not available, it already returns an empty fake
> directory. That variant of a fake directory is also size 0, so you
> aren't paying anything to use it...

Correct, I also explained that in [1]. This method may be useful if the driver
conditionally wants to provide debugfs entries regardless of CONFIG_DEBUG_FS
though.

Of course, we should only introduce it if we have a user for it.

[1] https://lore.kernel.org/all/DFR7AHOX0T8G.2NB9VOS3IZRZE@kernel.org/

[PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Timur Tabi]

Add a new method to UserSliceWriter that copies data from a raw kernel
pointer to userspace, without requiring a Rust slice reference.

The method takes:
  - data: raw pointer to the source buffer
  - len: total size of the source buffer (for bounds checking)
  - offset: byte offset into the source buffer to start copying from
  - count: number of bytes to copy

The method is marked unsafe because the caller must ensure the pointer
is valid for the specified length and that the memory is not mutated
during the call.

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

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index f989539a31b4..8bbb0084abb1 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -481,6 +481,56 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
         Ok(())
     }
 
+    /// Writes raw data to this user pointer from a raw kernel pointer.
+    ///
+    /// This is similar to [`Self::write_slice`] but takes a raw pointer instead of a slice,
+    /// along with a total buffer length, an offset into the that buffer, and a count of bytes
+    /// to copy.
+    ///
+    /// Returns error if the offset+count exceeds the buffer size.
+    ///
+    /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
+    /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice
+    /// even if it returns an error.
+    ///
+    /// # Safety
+    ///
+    /// - `data` must point to a valid memory region of at least `len` bytes that remains allocated
+    ///   for the duration of this call.
+    ///
+    /// Note: Unlike [`Self::write_slice`], this method does not require exclusive access to the
+    /// source memory. The memory may be concurrently modified by other threads or hardware (e.g.,
+    /// DMA buffers). In such cases, the copied data may be inconsistent, but this does not cause
+    /// undefined behavior.
+    pub unsafe fn write_buffer(
+        &mut self,
+        data: *const u8,
+        len: usize,
+        offset: usize,
+        count: usize,
+    ) -> Result {
+        if offset.checked_add(count).ok_or(EOVERFLOW)? > len {
+            return Err(ERANGE);
+        }
+
+        // SAFETY: The caller guarantees that `data` is valid for reads of `len` bytes,
+        // so `data.add(offset_val)` is valid for reads of `count` bytes.
+        let src_ptr = unsafe { data.add(offset) };
+
+        // SAFETY: `src_ptr` is valid for reads of `to_write` bytes per the above.
+        let res = unsafe {
+            bindings::copy_to_user(self.ptr.as_mut_ptr(), src_ptr.cast::<c_void>(), count)
+        };
+        if res != 0 {
+            return Err(EFAULT);
+        }
+
+        self.ptr = self.ptr.wrapping_byte_add(count);
+        self.length -= count;
+
+        Ok(())
+    }
+
     /// Writes raw data to this user pointer from a kernel buffer partially.
     ///
     /// This is the same as [`Self::write_slice`] but considers the given `offset` into `data` and
-- 
2.52.0

Re: [PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Danilo Krummrich]

(Cc: Alice)

Alice, for context, this is used when exporting a DMA buffer through debugfs,
while the DMA buffer may be in use by the device, i.e. no slice can be created.

On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
> Add a new method to UserSliceWriter that copies data from a raw kernel
> pointer to userspace, without requiring a Rust slice reference.
>
> The method takes:
>   - data: raw pointer to the source buffer
>   - len: total size of the source buffer (for bounds checking)
>   - offset: byte offset into the source buffer to start copying from
>   - count: number of bytes to copy
>
> The method is marked unsafe because the caller must ensure the pointer
> is valid for the specified length and that the memory is not mutated
> during the call.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/uaccess.rs | 50 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index f989539a31b4..8bbb0084abb1 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -481,6 +481,56 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
>          Ok(())
>      }
>  
> +    /// Writes raw data to this user pointer from a raw kernel pointer.
> +    ///
> +    /// This is similar to [`Self::write_slice`] but takes a raw pointer instead of a slice,
> +    /// along with a total buffer length, an offset into the that buffer, and a count of bytes
> +    /// to copy.
> +    ///
> +    /// Returns error if the offset+count exceeds the buffer size.
> +    ///
> +    /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
> +    /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice
> +    /// even if it returns an error.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `data` must point to a valid memory region of at least `len` bytes that remains allocated
> +    ///   for the duration of this call.
> +    ///
> +    /// Note: Unlike [`Self::write_slice`], this method does not require exclusive access to the
> +    /// source memory. The memory may be concurrently modified by other threads or hardware (e.g.,
> +    /// DMA buffers). In such cases, the copied data may be inconsistent, but this does not cause
> +    /// undefined behavior.
> +    pub unsafe fn write_buffer(
> +        &mut self,
> +        data: *const u8,
> +        len: usize,
> +        offset: usize,
> +        count: usize,
> +    ) -> Result {

Instead of this we could probably also add a safe method write_dma() that takes
a dma::CoherentAllocation<u8> instead. Once we have generic I/O in place, this
could be replaced with a generic write_io() method.

Re: [PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Danilo Krummrich]

(Really Cc: Alice :)

On Sat Jan 17, 2026 at 12:18 PM CET, Danilo Krummrich wrote:
> (Cc: Alice)
>
> Alice, for context, this is used when exporting a DMA buffer through debugfs,
> while the DMA buffer may be in use by the device, i.e. no slice can be created.
>
> On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
>> Add a new method to UserSliceWriter that copies data from a raw kernel
>> pointer to userspace, without requiring a Rust slice reference.
>>
>> The method takes:
>>   - data: raw pointer to the source buffer
>>   - len: total size of the source buffer (for bounds checking)
>>   - offset: byte offset into the source buffer to start copying from
>>   - count: number of bytes to copy
>>
>> The method is marked unsafe because the caller must ensure the pointer
>> is valid for the specified length and that the memory is not mutated
>> during the call.
>>
>> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>> ---
>>  rust/kernel/uaccess.rs | 50 ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>
>> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> index f989539a31b4..8bbb0084abb1 100644
>> --- a/rust/kernel/uaccess.rs
>> +++ b/rust/kernel/uaccess.rs
>> @@ -481,6 +481,56 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
>>          Ok(())
>>      }
>>  
>> +    /// Writes raw data to this user pointer from a raw kernel pointer.
>> +    ///
>> +    /// This is similar to [`Self::write_slice`] but takes a raw pointer instead of a slice,
>> +    /// along with a total buffer length, an offset into the that buffer, and a count of bytes
>> +    /// to copy.
>> +    ///
>> +    /// Returns error if the offset+count exceeds the buffer size.
>> +    ///
>> +    /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
>> +    /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice
>> +    /// even if it returns an error.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// - `data` must point to a valid memory region of at least `len` bytes that remains allocated
>> +    ///   for the duration of this call.
>> +    ///
>> +    /// Note: Unlike [`Self::write_slice`], this method does not require exclusive access to the
>> +    /// source memory. The memory may be concurrently modified by other threads or hardware (e.g.,
>> +    /// DMA buffers). In such cases, the copied data may be inconsistent, but this does not cause
>> +    /// undefined behavior.
>> +    pub unsafe fn write_buffer(
>> +        &mut self,
>> +        data: *const u8,
>> +        len: usize,
>> +        offset: usize,
>> +        count: usize,
>> +    ) -> Result {
>
> Instead of this we could probably also add a safe method write_dma() that takes
> a dma::CoherentAllocation<u8> instead. Once we have generic I/O in place, this
> could be replaced with a generic write_io() method.

Re: [PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Alice Ryhl]

On Fri, Jan 16, 2026 at 03:49:54PM -0600, Timur Tabi wrote:
> Add a new method to UserSliceWriter that copies data from a raw kernel
> pointer to userspace, without requiring a Rust slice reference.
> 
> The method takes:
>   - data: raw pointer to the source buffer
>   - len: total size of the source buffer (for bounds checking)
>   - offset: byte offset into the source buffer to start copying from
>   - count: number of bytes to copy
> 
> The method is marked unsafe because the caller must ensure the pointer
> is valid for the specified length and that the memory is not mutated
> during the call.
> 
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/uaccess.rs | 50 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index f989539a31b4..8bbb0084abb1 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -481,6 +481,56 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
>          Ok(())
>      }
>  
> +    /// Writes raw data to this user pointer from a raw kernel pointer.
> +    ///
> +    /// This is similar to [`Self::write_slice`] but takes a raw pointer instead of a slice,
> +    /// along with a total buffer length, an offset into the that buffer, and a count of bytes
> +    /// to copy.
> +    ///
> +    /// Returns error if the offset+count exceeds the buffer size.
> +    ///
> +    /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
> +    /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice
> +    /// even if it returns an error.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `data` must point to a valid memory region of at least `len` bytes that remains allocated
> +    ///   for the duration of this call.
> +    ///
> +    /// Note: Unlike [`Self::write_slice`], this method does not require exclusive access to the
> +    /// source memory. The memory may be concurrently modified by other threads or hardware (e.g.,
> +    /// DMA buffers). In such cases, the copied data may be inconsistent, but this does not cause
> +    /// undefined behavior.
> +    pub unsafe fn write_buffer(
> +        &mut self,
> +        data: *const u8,
> +        len: usize,
> +        offset: usize,
> +        count: usize,
> +    ) -> Result {

Why not this signature?

	unsafe fn write_raw_slice(&mut self, data: *const [u8]) -> Result;

You can implement `write_slice` in terms of it.

Alice

Re: [PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Alice Ryhl]

On Sat, Jan 17, 2026 at 01:23:57PM +0000, Alice Ryhl wrote:
> On Fri, Jan 16, 2026 at 03:49:54PM -0600, Timur Tabi wrote:
> > Add a new method to UserSliceWriter that copies data from a raw kernel
> > pointer to userspace, without requiring a Rust slice reference.
> > 
> > The method takes:
> >   - data: raw pointer to the source buffer
> >   - len: total size of the source buffer (for bounds checking)
> >   - offset: byte offset into the source buffer to start copying from
> >   - count: number of bytes to copy
> > 
> > The method is marked unsafe because the caller must ensure the pointer
> > is valid for the specified length and that the memory is not mutated
> > during the call.
> > 
> > Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> > ---
> >  rust/kernel/uaccess.rs | 50 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> > 
> > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> > index f989539a31b4..8bbb0084abb1 100644
> > --- a/rust/kernel/uaccess.rs
> > +++ b/rust/kernel/uaccess.rs
> > @@ -481,6 +481,56 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
> >          Ok(())
> >      }
> >  
> > +    /// Writes raw data to this user pointer from a raw kernel pointer.
> > +    ///
> > +    /// This is similar to [`Self::write_slice`] but takes a raw pointer instead of a slice,
> > +    /// along with a total buffer length, an offset into the that buffer, and a count of bytes
> > +    /// to copy.
> > +    ///
> > +    /// Returns error if the offset+count exceeds the buffer size.
> > +    ///
> > +    /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
> > +    /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice
> > +    /// even if it returns an error.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// - `data` must point to a valid memory region of at least `len` bytes that remains allocated
> > +    ///   for the duration of this call.
> > +    ///
> > +    /// Note: Unlike [`Self::write_slice`], this method does not require exclusive access to the
> > +    /// source memory. The memory may be concurrently modified by other threads or hardware (e.g.,
> > +    /// DMA buffers). In such cases, the copied data may be inconsistent, but this does not cause
> > +    /// undefined behavior.
> > +    pub unsafe fn write_buffer(
> > +        &mut self,
> > +        data: *const u8,
> > +        len: usize,
> > +        offset: usize,
> > +        count: usize,
> > +    ) -> Result {
> 
> Why not this signature?
> 
> 	unsafe fn write_raw_slice(&mut self, data: *const [u8]) -> Result;
> 
> You can implement `write_slice` in terms of it.

To clarify, I think this would be a simpler signature for
`write_buffer()`. And `write_raw_slice()` can be used both for DMA and
to simplify the existing `write_slice`.

Alice

Re: [PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Danilo Krummrich]

On Sat Jan 17, 2026 at 3:23 PM CET, Alice Ryhl wrote:
> On Sat, Jan 17, 2026 at 01:23:57PM +0000, Alice Ryhl wrote:
>> On Fri, Jan 16, 2026 at 03:49:54PM -0600, Timur Tabi wrote:
>> > +    pub unsafe fn write_buffer(
>> > +        &mut self,
>> > +        data: *const u8,
>> > +        len: usize,
>> > +        offset: usize,
>> > +        count: usize,
>> > +    ) -> Result {
>> 
>> Why not this signature?
>> 
>> 	unsafe fn write_raw_slice(&mut self, data: *const [u8]) -> Result;
>> 
>> You can implement `write_slice` in terms of it.
>
> To clarify, I think this would be a simpler signature for
> `write_buffer()`. And `write_raw_slice()` can be used both for DMA and
> to simplify the existing `write_slice`.

I.e. you can use it also to create a safe helper for DMA:

	fn write_dma(
	    &mut self,
	    data: &dma::CoherentAllocation<u8>,
	    offset: usize,
	    count: usize
	) -> Result;

Re: [PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Gary Guo]

On Sat Jan 17, 2026 at 2:35 PM GMT, Danilo Krummrich wrote:
> On Sat Jan 17, 2026 at 3:23 PM CET, Alice Ryhl wrote:
>> On Sat, Jan 17, 2026 at 01:23:57PM +0000, Alice Ryhl wrote:
>>> On Fri, Jan 16, 2026 at 03:49:54PM -0600, Timur Tabi wrote:
>>> > +    pub unsafe fn write_buffer(
>>> > +        &mut self,
>>> > +        data: *const u8,
>>> > +        len: usize,
>>> > +        offset: usize,
>>> > +        count: usize,
>>> > +    ) -> Result {
>>> 
>>> Why not this signature?
>>> 
>>> 	unsafe fn write_raw_slice(&mut self, data: *const [u8]) -> Result;
>>> 
>>> You can implement `write_slice` in terms of it.
>>
>> To clarify, I think this would be a simpler signature for
>> `write_buffer()`. And `write_raw_slice()` can be used both for DMA and
>> to simplify the existing `write_slice`.
>
> I.e. you can use it also to create a safe helper for DMA:
>
> 	fn write_dma(
> 	    &mut self,
> 	    data: &dma::CoherentAllocation<u8>,
> 	    offset: usize,
> 	    count: usize
> 	) -> Result;

Would it make sense to expose a `&CoherentAllocation<u8>` -> `&[Atomic<u8>]`
conversion method and have a `write_slice(&mut self, data: &[Atomic<[u8]>)` for
`UserSliceWriter`?

Best,
Gary

Re: [PATCH v5 3/8] rust: uaccess: add UserSliceWriter::write_buffer() for raw pointer writes

[Danilo Krummrich]

On Mon Jan 19, 2026 at 1:13 PM CET, Gary Guo wrote:
> On Sat Jan 17, 2026 at 2:35 PM GMT, Danilo Krummrich wrote:
>> On Sat Jan 17, 2026 at 3:23 PM CET, Alice Ryhl wrote:
>>> On Sat, Jan 17, 2026 at 01:23:57PM +0000, Alice Ryhl wrote:
>>>> On Fri, Jan 16, 2026 at 03:49:54PM -0600, Timur Tabi wrote:
>>>> > +    pub unsafe fn write_buffer(
>>>> > +        &mut self,
>>>> > +        data: *const u8,
>>>> > +        len: usize,
>>>> > +        offset: usize,
>>>> > +        count: usize,
>>>> > +    ) -> Result {
>>>> 
>>>> Why not this signature?
>>>> 
>>>> 	unsafe fn write_raw_slice(&mut self, data: *const [u8]) -> Result;
>>>> 
>>>> You can implement `write_slice` in terms of it.
>>>
>>> To clarify, I think this would be a simpler signature for
>>> `write_buffer()`. And `write_raw_slice()` can be used both for DMA and
>>> to simplify the existing `write_slice`.
>>
>> I.e. you can use it also to create a safe helper for DMA:
>>
>> 	fn write_dma(
>> 	    &mut self,
>> 	    data: &dma::CoherentAllocation<u8>,
>> 	    offset: usize,
>> 	    count: usize
>> 	) -> Result;
>
> Would it make sense to expose a `&CoherentAllocation<u8>` -> `&[Atomic<u8>]`
> conversion method and have a `write_slice(&mut self, data: &[Atomic<[u8]>)` for
> `UserSliceWriter`?

But we don't need any atomic operations for this. Maybe a transparent new type
that only allows access with {read,write}_volatile(), or maybe an UnsafeSlice
type that can provide accessors that internally use *mut [u8] with
{read,write}_volatile().

But for this problem we only need something to eventually pass to
write_slice_raw() which will just use copy_to_user(), so it might be orthogonal.

[PATCH v5 4/8] 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 | 40 ++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 766fd9905358..273327c33aa7 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,45 @@ 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> {
+        if offset.is_negative() {
+            return Err(EINVAL);
+        }
+
+        let offset_val: usize = (*offset).try_into().map_err(|_| EINVAL)?;
+        let len = self.0.count();
+
+        if offset_val >= len {
+            return Ok(0);
+        }
+
+        let count = (len - offset_val).min(writer.len());
+
+        // SAFETY:
+        // - `start_ptr()` returns a valid pointer to a memory region of `count()` bytes,
+        //   as guaranteed by the `CoherentAllocation` invariants.
+        // - `len` equals `self.0.count()`, so the pointer is valid for `len` bytes.
+        // - `offset_val < len` is guaranteed by the check above.
+        // - `count = (len - offset_val).min(writer.len())`, so `offset_val + count <= len`.
+        unsafe { writer.write_buffer(self.0.start_ptr(), len, offset_val, count)? };
+
+        *offset += count as i64;
+        Ok(count)
+    }
+}
+
+// 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

Re: [PATCH v5 4/8] gpu: nova-core: implement BinaryWriter for LogBuffer

[Danilo Krummrich]

On Fri Jan 16, 2026 at 10:49 PM 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 | 40 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 766fd9905358..273327c33aa7 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,45 @@ pub(crate) struct Gsp {
>      rmargs: CoherentAllocation<GspArgumentsCached>,
>  }
>  
> +impl debugfs::BinaryWriter for LogBuffer {

Let's move this code into rust/kernel/dma.rs and make it an impl
debugfs::BinaryWriter for CoherentAllocation<u8>.

Re: [PATCH v5 4/8] gpu: nova-core: implement BinaryWriter for LogBuffer

[Gary Guo]

On Fri Jan 16, 2026 at 9:49 PM GMT, 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 | 40 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 766fd9905358..273327c33aa7 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,45 @@ 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> {
> +        if offset.is_negative() {
> +            return Err(EINVAL);
> +        }
> +
> +        let offset_val: usize = (*offset).try_into().map_err(|_| EINVAL)?;
> +        let len = self.0.count();
> +
> +        if offset_val >= len {
> +            return Ok(0);
> +        }
> +
> +        let count = (len - offset_val).min(writer.len());
> +
> +        // SAFETY:
> +        // - `start_ptr()` returns a valid pointer to a memory region of `count()` bytes,
> +        //   as guaranteed by the `CoherentAllocation` invariants.
> +        // - `len` equals `self.0.count()`, so the pointer is valid for `len` bytes.
> +        // - `offset_val < len` is guaranteed by the check above.
> +        // - `count = (len - offset_val).min(writer.len())`, so `offset_val + count <= len`.
> +        unsafe { writer.write_buffer(self.0.start_ptr(), len, offset_val, count)? };
> +
> +        *offset += count as i64;
> +        Ok(count)
> +    }
> +}
> +
> +// 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 {}

Can we just implement `Sync` on `CoherentAllocation`?

Best,
Gary

> +
>  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> + '_ {

Re: [PATCH v5 4/8] gpu: nova-core: implement BinaryWriter for LogBuffer

[Timur Tabi]

On Mon, 2026-01-19 at 12:17 +0000, Gary Guo wrote:
> > +// 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 {}
> 
> Can we just implement `Sync` on `CoherentAllocation`?

When I moved this to dma.rs, I had to add this:

// SAFETY: All methods that access the underlying DMA buffer (`field_read`, `field_write`,
// `as_slice`, `as_slice_mut`) are `unsafe`, and callers are responsible for ensuring no data
// races occur between kernel threads. The safe methods only return metadata (e.g. `count()`,
// `dma_handle()`) or raw pointers whose use requires `unsafe`. It is safe to send or share
// a `CoherentAllocation` across threads if `T` can be sent or shared.
unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
unsafe impl<T: AsBytes + FromBytes + Sync> Sync for CoherentAllocation<T> {}

This allowed me to eliminate the "Sync for LogBuffer".

Please let me know if you think the safety comment needs to change.  I will post a v6 soon.

[PATCH v5 5/8] 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 | 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

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

[Danilo Krummrich]

On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
> 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,

This should be covered by prelude.

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

Let's import driver, such that this becomes driver::Registration<_>.

> +}
> +
> +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),

Same here.

> +        })
> +    }
> +}
> +
> +module! {
> +    type: NovaCoreModule,
>      name: "NovaCore",
>      authors: ["Danilo Krummrich"],
>      description: "Nova Core GPU driver",
>      license: "GPL v2",
> -    firmware: [],

Should be a separate patch.

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

[Timur Tabi]

On Sat, 2026-01-17 at 12:24 +0100, Danilo Krummrich wrote:
> > -kernel::module_pci_driver! {
> > -    type: driver::NovaCore,
> > +#[pin_data]
> > +struct NovaCoreModule {
> > +    #[pin]
> > +    _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
> 
> Let's import driver, such that this becomes driver::Registration<_>.

We already have "mod driver", so if I try to import kernel::driver, the compiler just complains.  I
can import driver::Registration, but that just gets me:

    _driver: Registration<pci::Adapter<driver::NovaCore>>,

I don't see how I can replace this with just driver::Registration<_>.

> > +        })
> > +    }
> > +}
> > +
> > +module! {
> > +    type: NovaCoreModule,
> >       name: "NovaCore",
> >       authors: ["Danilo Krummrich"],
> >       description: "Nova Core GPU driver",
> >       license: "GPL v2",
> > -    firmware: [],
> 
> Should be a separate patch.

I'll just put it back.

[PATCH v5 6/8] 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 | 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

[PATCH v5 7/8] gpu: nova-core: create debugfs root in module init

[Timur Tabi]

Create the 'nova_core' root debugfs entry when the driver loads.

Normally, non-const global variables need to be protected by a
mutex.  Instead, we use unsafe code, as we know the entry is never
modified after the driver is loaded.  This solves the lifetime
issue of the mutex guard, which would otherwise have required the
use of `pin_init_scope`.

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

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index d0df8db3693d..eceefa41cddc 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,
     error::Error,
     pci,
     prelude::*,
@@ -27,7 +28,9 @@
 
 pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 
-#[pin_data]
+static mut DEBUGFS_ROOT: Option<Dir> = None;
+
+#[pin_data(PinnedDrop)]
 struct NovaCoreModule {
     #[pin]
     _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
@@ -35,12 +38,27 @@ struct NovaCoreModule {
 
 impl InPlaceModule for NovaCoreModule {
     fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+        let dir = Dir::new(kernel::c_str!("nova_core"));
+
+        // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
+        // `DEBUGFS_ROOT`.
+        unsafe { DEBUGFS_ROOT = Some(dir) };
+
         try_pin_init!(Self {
             _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
         })
     }
 }
 
+#[pinned_drop]
+impl PinnedDrop for NovaCoreModule {
+    fn drop(self: Pin<&mut Self>) {
+        // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
+        // `DEBUGFS_ROOT`.
+        unsafe { DEBUGFS_ROOT = None };
+    }
+}
+
 module! {
     type: NovaCoreModule,
     name: "NovaCore",
-- 
2.52.0

Re: [PATCH v5 7/8] gpu: nova-core: create debugfs root in module init

[Danilo Krummrich]

On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
> Create the 'nova_core' root debugfs entry when the driver loads.
>
> Normally, non-const global variables need to be protected by a
> mutex.  Instead, we use unsafe code, as we know the entry is never
> modified after the driver is loaded.  This solves the lifetime
> issue of the mutex guard, which would otherwise have required the
> use of `pin_init_scope`.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/nova_core.rs | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index d0df8db3693d..eceefa41cddc 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,
>      error::Error,
>      pci,
>      prelude::*,
> @@ -27,7 +28,9 @@
>  
>  pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>  
> -#[pin_data]
> +static mut DEBUGFS_ROOT: Option<Dir> = None;

Please add the FIXME comment that we talked about in the last revision and use
debugfs::Dir rather than just Dir.

> +
> +#[pin_data(PinnedDrop)]
>  struct NovaCoreModule {
>      #[pin]
>      _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
> @@ -35,12 +38,27 @@ struct NovaCoreModule {
>  
>  impl InPlaceModule for NovaCoreModule {
>      fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
> +        let dir = Dir::new(kernel::c_str!("nova_core"));
> +
> +        // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
> +        // `DEBUGFS_ROOT`.
> +        unsafe { DEBUGFS_ROOT = Some(dir) };
> +
>          try_pin_init!(Self {
>              _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
>          })
>      }
>  }
>  
> +#[pinned_drop]
> +impl PinnedDrop for NovaCoreModule {
> +    fn drop(self: Pin<&mut Self>) {
> +        // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
> +        // `DEBUGFS_ROOT`.

The driver::Registration is not yet dropped at this point, so there might be
other driver code running. You have to justify that this does not race with
module_init() (which module_exit() never does) and that it does not race with
Gsp::new(), i.e. probe().

Note that probe() may run async, so you have to drop the driver registration
manually before dropping the debugfs dir.

> +        unsafe { DEBUGFS_ROOT = None };

You also have to drop the debugfs::Dir.

> +    }
> +}
> +
>  module! {
>      type: NovaCoreModule,
>      name: "NovaCore",
> -- 
> 2.52.0

Re: [PATCH v5 7/8] gpu: nova-core: create debugfs root in module init

[Danilo Krummrich]

On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
>  impl InPlaceModule for NovaCoreModule {
>      fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
> +        let dir = Dir::new(kernel::c_str!("nova_core"));
> +
> +        // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
> +        // `DEBUGFS_ROOT`.
> +        unsafe { DEBUGFS_ROOT = Some(dir) };

I think you forgot to add a new Kconfig to nova-core to enable this and in case
not set leave it as None.

Re: [PATCH v5 7/8] gpu: nova-core: create debugfs root in module init

[Timur Tabi]

On Sat, 2026-01-17 at 13:29 +0100, Danilo Krummrich wrote:
> On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
> >   impl InPlaceModule for NovaCoreModule {
> >       fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
> > +        let dir = Dir::new(kernel::c_str!("nova_core"));
> > +
> > +        // SAFETY: we are the only driver code running, so there cannot be any concurrent
> > access to
> > +        // `DEBUGFS_ROOT`.
> > +        unsafe { DEBUGFS_ROOT = Some(dir) };
> 
> I think you forgot to add a new Kconfig to nova-core to enable this and in case
> not set leave it as None.

You don't want the debugfs entries created by default?  That's how it is in Nouveau.

Re: [PATCH v5 7/8] gpu: nova-core: create debugfs root in module init

[Danilo Krummrich]

On Sat Jan 17, 2026 at 9:58 PM CET, Timur Tabi wrote:
> On Sat, 2026-01-17 at 13:29 +0100, Danilo Krummrich wrote:
>> On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
>> >   impl InPlaceModule for NovaCoreModule {
>> >       fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
>> > +        let dir = Dir::new(kernel::c_str!("nova_core"));
>> > +
>> > +        // SAFETY: we are the only driver code running, so there cannot be any concurrent
>> > access to
>> > +        // `DEBUGFS_ROOT`.
>> > +        unsafe { DEBUGFS_ROOT = Some(dir) };
>> 
>> I think you forgot to add a new Kconfig to nova-core to enable this and in case
>> not set leave it as None.
>
> You don't want the debugfs entries created by default?  That's how it is in Nouveau.

I think it's OK to always have the entries on keeping them beyond device unbind
has to be behind a Kconfig option.

I thought you want this behind a Kconfig in general, otherwise what is
debugfs::Dir::empty() for?

Re: [PATCH v5 7/8] gpu: nova-core: create debugfs root in module init

[Timur Tabi]

On Sat, 2026-01-17 at 22:35 +0100, Danilo Krummrich wrote:
> > You don't want the debugfs entries created by default?  That's how it is in Nouveau.
> 
> I think it's OK to always have the entries on keeping them beyond device unbind
> has to be behind a Kconfig option.
> 
> I thought you want this behind a Kconfig in general, otherwise what is
> debugfs::Dir::empty() for?

Oh, I see what you're saying.  Dir::empty() is for when CONFIGFS_DEBUG_FS is disabled.  So
obviously, I should not default DEBUGFS_ROOT to Some(dir) if CONFIGFS_DEBUG_FS is disabled.

Re: [PATCH v5 7/8] gpu: nova-core: create debugfs root in module init

[Danilo Krummrich]

On Sat Jan 17, 2026 at 10:52 PM CET, Timur Tabi wrote:
> On Sat, 2026-01-17 at 22:35 +0100, Danilo Krummrich wrote:
>> > You don't want the debugfs entries created by default?  That's how it is in Nouveau.
>> 
>> I think it's OK to always have the entries on keeping them beyond device unbind
>> has to be behind a Kconfig option.
>> 
>> I thought you want this behind a Kconfig in general, otherwise what is
>> debugfs::Dir::empty() for?
>
> Oh, I see what you're saying.  Dir::empty() is for when CONFIGFS_DEBUG_FS is disabled.  So
> obviously, I should not default DEBUGFS_ROOT to Some(dir) if CONFIGFS_DEBUG_FS is disabled.

This case is already handled, look at how debugfs::Dir::create() [1] is
implemented. I.e. it is fully transparent and you don't have to care about this
case.

debugfs::Dir::empty() would be useful if you have an additional case where you
want to have a debugfs file tree conditionally though, which is why I assumed
you wanted an additional Kconfig for nova-core.

[1] https://rust.docs.kernel.org/src/kernel/debugfs.rs.html#53

[PATCH v5 8/8] 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 | 45 +++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 273327c33aa7..4b194bb805b4 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.
@@ -163,15 +171,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,
@@ -192,6 +202,25 @@ 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,
+                    };
+
+                    #[allow(static_mut_refs)]
+                    // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is
+                    // safe to create a shared reference to it.
+                    let debugfs_root = unsafe { crate::DEBUGFS_ROOT.as_ref() }
+                        .unwrap_or_else(|| debugfs::Dir::empty());
+
+                    debugfs_root.scope(log_buffers, dev.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

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

[Danilo Krummrich]

On Fri Jan 16, 2026 at 10:49 PM CET, Timur Tabi wrote:
> 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 | 45 +++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 273327c33aa7..4b194bb805b4 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.
> @@ -163,15 +171,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,
> @@ -192,6 +202,25 @@ 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,
> +                    };
> +
> +                    #[allow(static_mut_refs)]
> +                    // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is
> +                    // safe to create a shared reference to it.

Well, it is modified in module_exit(), so you have to justify why this can't
race with module_exit(). I.e. because you drop the driver registration first.

You also have to say that the debugfs dir is created before the driver
registration.

> +                    let debugfs_root = unsafe { crate::DEBUGFS_ROOT.as_ref() }
> +                        .unwrap_or_else(|| debugfs::Dir::empty());
> +
> +                    debugfs_root.scope(log_buffers, dev.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