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

["Danilo Krummrich" <dakr@kernel.org>]

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