gpu: nova-core: expose the logging buffers via debugfs

index 82e128431f08..125fb39f4316 100644
@@ -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> {

index 766fd9905358..9b73c96fbd3a 100644
@@ -3,6 +3,7 @@
 mod boot;
 
 use kernel::{
+    debugfs,
     device,
     dma::{
         CoherentAllocation,

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

index c1121e7c64c5..d0df8db3693d 100644
@@ -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);

index 581b412554dc..68a8acaf8c54 100644
@@ -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),

index facad81e8290..ed740eb90fc9 100644
@@ -35,6 +35,18 @@
 #[cfg(CONFIG_DEBUG_FS)]
 use entry::Entry;
 
+/// Trait for DebugFS directory operations.
+///
+/// This trait abstracts the core operations that can be performed on a DebugFS directory,
+/// allowing for alternative implementations (e.g., for testing).
+pub trait Directory: Clone + Sized {
+    /// Create a new directory at the DebugFS root.
+    fn new(name: &CStr) -> Self;
+
+    /// Creates a subdirectory within this directory.
+    fn subdir(&self, name: &CStr) -> Self;
+}
+
 /// Owning handle to a DebugFS directory.
 ///
 /// The directory in the filesystem represented by [`Dir`] will be removed when handle has been

@@ -96,7 +108,9 @@ fn create_file<'a, T, E: 'a>(
             } ? E
         }
     }
+}
 
+impl Directory for Dir {
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples

@@ -106,7 +120,7 @@ fn create_file<'a, T, E: 'a>(
     /// # use kernel::debugfs::Dir;
     /// let debugfs = Dir::new(c_str!("parent"));
     /// ```
-    pub fn new(name: &CStr) -> Self {
+    fn new(name: &CStr) -> Self {
         Dir::create(name, None)
     }
 

@@ -120,9 +134,12 @@ pub fn new(name: &CStr) -> Self {
     /// let parent = Dir::new(c_str!("parent"));
     /// let child = parent.subdir(c_str!("child"));
     /// ```
-    pub fn subdir(&self, name: &CStr) -> Self {
+    fn subdir(&self, name: &CStr) -> Self {
         Dir::create(name, Some(self))
     }
+}
+
+impl Dir {
 
     /// Creates a read-only file in this directory.
     ///

index 706cb7f73d6c..3152e4f96219 100644
@@ -7,6 +7,16 @@
 use crate::sync::Arc;
 use core::marker::PhantomData;
 
+/// Type-erased parent that keeps either an `Entry` or `LookupEntry` alive.
+///
+/// This allows an `Entry` to have either type of parent while maintaining proper
+/// lifetime management. The parent is kept alive via `Arc` reference counting.
+#[allow(dead_code)]
+pub(crate) enum DynParent {
+    /// Parent is an owned `Entry` (will be removed on drop).
+    Entry(Arc<Entry<'static>>),
+}
+
 /// Owning handle to a DebugFS entry.
 ///
 /// # Invariants

@@ -15,7 +25,7 @@
 pub(crate) struct Entry<'a> {
     entry: *mut bindings::dentry,
     // If we were created with an owning parent, this is the keep-alive
-    _parent: Option<Arc<Entry<'static>>>,
+    _parent: Option<DynParent>,
     // If we were created with a non-owning parent, this prevents us from outliving it
     _phantom: PhantomData<&'a ()>,
 }

@@ -41,7 +51,7 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
 
         Entry {
             entry,
-            _parent: parent,
+            _parent: parent.map(DynParent::Entry),
             _phantom: PhantomData,
         }
     }

@@ -74,7 +84,7 @@ pub(crate) unsafe fn dynamic_file<T>(
 
         Entry {
             entry,
-            _parent: Some(parent),
+            _parent: Some(DynParent::Entry(parent)),
             _phantom: PhantomData,
         }
     }

index ed740eb90fc9..2ff70d72240c 100644
@@ -34,6 +34,8 @@
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]
 use entry::Entry;
+#[cfg(CONFIG_DEBUG_FS)]
+use entry::LookupEntry;
 
 /// Trait for DebugFS directory operations.
 ///

@@ -393,6 +395,131 @@ pub fn scope<'a, T: 'a, E: 'a, F>(
     }
 }
 
+/// Non-owning handle to an existing DebugFS directory.
+///
+/// Unlike [`Dir`], a [`LookupDir`] does not create a new directory. Instead, it looks up an
+/// existing directory in the DebugFS filesystem. When dropped, the directory is **not** removed
+/// from the filesystem - only the reference is released.
+///
+/// This is useful when you want to add files to an existing directory created by another part
+/// of the system without taking ownership of that directory.
+#[derive(Clone)]
+pub struct LookupDir(#[cfg(CONFIG_DEBUG_FS)] Option<Arc<LookupEntry>>);
+
+impl LookupDir {
+    /// Looks up an existing directory in DebugFS.
+    ///
+    /// If `parent` is [`None`], the lookup is performed from the root of the debugfs filesystem.
+    ///
+    /// Returns [`Some(LookupDir)`] if the directory exists, [`None`] otherwise.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::LookupDir;
+    /// // Look up a top-level directory
+    /// if let Some(dir) = LookupDir::lookup(c_str!("existing_dir"), None) {
+    ///     // Use the directory...
+    /// }
+    /// // When `dir` is dropped, the directory is NOT removed.
+    /// ```
+    pub fn lookup(name: &CStr, parent: Option<&LookupDir>) -> Option<Self> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            let parent_entry = match parent {
+                Some(LookupDir(None)) => return None,
+                Some(LookupDir(Some(entry))) => Some(entry.clone()),
+                None => None,
+            };
+            let entry = LookupEntry::lookup(name, parent_entry)?;
+            Some(Self(Arc::new(entry, GFP_KERNEL).ok()))
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        None
+    }
+
+    // While this function is safe, it is intentionally not public because it's a bit of a
+    // footgun.
+    //
+    // Unless you also extract the `entry` later and schedule it for `Drop` at the appropriate
+    // time, a `ScopedDir` with a `LookupDir` parent will never be deleted.
+    fn scoped_dir<'data>(&self, name: &CStr) -> ScopedDir<'data, 'static> {
+        #[cfg(CONFIG_DEBUG_FS)]
+        {
+            let parent_entry = match &self.0 {
+                None => return ScopedDir::empty(),
+                Some(entry) => entry.clone(),
+            };
+            ScopedDir {
+                entry: ManuallyDrop::new(Entry::dynamic_dir_with_lookup_parent(name, parent_entry)),
+                _phantom: PhantomData,
+            }
+        }
+        #[cfg(not(CONFIG_DEBUG_FS))]
+        ScopedDir::empty()
+    }
+
+    /// Creates a new scope, which is a directory associated with some data `T`.
+    ///
+    /// The created directory will be a subdirectory of `self`. The `init` closure is called to
+    /// populate the directory with files and subdirectories. These files can reference the data
+    /// stored in the scope.
+    ///
+    /// The entire directory tree created within the scope will be removed when the returned
+    /// `Scope` handle is dropped.
+    ///
+    /// Note: Unlike [`Dir::scope`], this method consumes `self` because the parent entry
+    /// is kept alive by the created scope via [`DynParent`], not by the `LookupDir` handle.
+    pub fn scope<'a, T: 'a, E: 'a, F>(
+        self,
+        data: impl PinInit<T, E> + 'a,
+        name: &'a CStr,
+        init: F,
+    ) -> impl PinInit<Scope<T>, E> + 'a
+    where
+        F: for<'data, 'dir> FnOnce(&'data T, &'dir ScopedDir<'data, 'dir>) + 'a,
+    {
+        Scope::new(data, move |data| {
+            let scoped = self.scoped_dir(name);
+            init(data, &scoped);
+            scoped.into_entry()
+        })
+    }
+}
+
+impl Directory for LookupDir {
+    /// Looks up an existing directory at the DebugFS root.
+    ///
+    /// Note: Unlike [`Dir::new`], this will return an empty handle if the directory
+    /// does not exist, rather than creating it.
+    fn new(name: &CStr) -> Self {
+        Self::lookup(name, None).unwrap_or_else(|| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            {
+                Self(None)
+            }
+            #[cfg(not(CONFIG_DEBUG_FS))]
+            Self()
+        })
+    }
+
+    /// Looks up a subdirectory within this directory.
+    ///
+    /// Note: Unlike [`Dir::subdir`], this will return an empty handle if the subdirectory
+    /// does not exist, rather than creating it.
+    fn subdir(&self, name: &CStr) -> Self {
+        Self::lookup(name, Some(self)).unwrap_or_else(|| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            {
+                Self(None)
+            }
+            #[cfg(not(CONFIG_DEBUG_FS))]
+            Self()
+        })
+    }
+}
+
 #[pin_data]
 /// Handle to a DebugFS scope, which ensures that attached `data` will outlive the DebugFS entry
 /// without moving.

99 lines not on this page · raw message

index d0df8db3693d..c9b93ccd6092 100644
@@ -3,6 +3,7 @@
 //! Nova Core GPU Driver
 
 use kernel::{
+    debugfs::{Dir, Directory},
     error::Error,
     pci,
     prelude::*,

@@ -31,12 +32,17 @@
 struct NovaCoreModule {
     #[pin]
     _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
+    _debugfs_root: Dir,
 }
 
 impl InPlaceModule for NovaCoreModule {
     fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+        // Create the debugfs top-level directory.  Each GPU will create a subdirectory.
+        let debugfs_root = Dir::new(kernel::c_str!("nova_core"));
+
         try_pin_init!(Self {
             _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
+            _debugfs_root: debugfs_root,
         })
     }
 }

index 9b73c96fbd3a..dc4a009862ef 100644
@@ -3,7 +3,8 @@
 mod boot;
 
 use kernel::{
-    debugfs,
+    c_str,
+    debugfs::{self, Directory},
     device,
     dma::{
         CoherentAllocation,

@@ -101,17 +102,24 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
     }
 }
 
-/// GSP runtime data.
-#[pin_data]
-pub(crate) struct Gsp {
-    /// Libos arguments.
-    pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
+/// Log buffers used by GSP-RM for debug logging.
+struct LogBuffers {
     /// Init log buffer.
     loginit: LogBuffer,
     /// Interrupts log buffer.
     logintr: LogBuffer,
     /// RM log buffer.
     logrm: LogBuffer,
+}
+
+/// GSP runtime data.
+#[pin_data]
+pub(crate) struct Gsp {
+    /// Libos arguments.
+    pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
+    /// Log buffers, optionally exposed via debugfs.
+    #[pin]
+    logs: debugfs::Scope<LogBuffers>,
     /// Command queue.
     pub(crate) cmdq: Cmdq,
     /// RM arguments.

@@ -147,15 +155,17 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
         pin_init::pin_init_scope(move || {
             let dev = pdev.as_ref();
 
+            // Create log buffers before try_pin_init! so they're accessible throughout
+            let loginit = LogBuffer::new(dev)?;
+            let logintr = LogBuffer::new(dev)?;
+            let logrm = LogBuffer::new(dev)?;
+
             Ok(try_pin_init!(Self {
                 libos: CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
                     dev,
                     GSP_PAGE_SIZE / size_of::<LibosMemoryRegionInitArgument>(),
                     GFP_KERNEL | __GFP_ZERO,
                 )?,
-                loginit: LogBuffer::new(dev)?,
-                logintr: LogBuffer::new(dev)?,
-                logrm: LogBuffer::new(dev)?,
                 cmdq: Cmdq::new(dev)?,
                 rmargs: CoherentAllocation::<GspArgumentsCached>::alloc_coherent(
                     dev,

@@ -176,6 +186,24 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
                     dma_write!(rmargs[0] = fw::GspArgumentsCached::new(cmdq))?;
                     dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
                 },
+                logs <- {
+                    let log_buffers = LogBuffers {
+                        loginit,
+                        logintr,
+                        logrm,
+                    };
+
+                    // Look up nova_core debugfs directory - only create entries if it exists.
+                    // If nova_core doesn't exist, debugfs_dir will be empty and file creation
+                    // becomes a no-op (data is still stored, just not exposed via debugfs).
+                    let debugfs_dir = debugfs::LookupDir::new(c_str!("nova_core"));
+
+                    debugfs_dir.scope(log_buffers, pdev.name(), |logs, dir| {
+                        dir.read_binary_file(c_str!("loginit"), &logs.loginit);
+                        dir.read_binary_file(c_str!("logintr"), &logs.logintr);
+                        dir.read_binary_file(c_str!("logrm"), &logs.logrm);
+                    })
+                },
             }))
         })
     }