Re: [PATCH 1/2] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind

On Wed Aug 12, 2026 at 1:37 PM CEST, Vladislav Zaharov wrote:
>  struct LogBuffers {
> +    /// Device the buffers belong to. Also names their debugfs directory.
> +    #[cfg(CONFIG_NOVA_CORE_KEEP_GSP_LOGS)]
> +    dev: ARef<device::Device>,
>      /// Init log buffer.
>      loginit: LogBuffer,
>      /// Interrupts log buffer.
> @@ -144,6 +167,127 @@ struct LogBuffers {
>      logrm: LogBuffer,
>  }
>  
> +/// Copies of the log buffers of a GPU that is no longer around.
> +#[cfg(CONFIG_NOVA_CORE_KEEP_GSP_LOGS)]
> +struct RetainedLogBuffers {
> +    /// Device the buffers came from.
> +    dev: ARef<device::Device>,
> +    /// Contents of the init log buffer, empty if it was never written to.
> +    loginit: KVec<u8>,
> +    /// Contents of the interrupts log buffer, empty if it was never written to.
> +    logintr: KVec<u8>,
> +    /// Contents of the RM log buffer, empty if it was never written to.
> +    logrm: KVec<u8>,

I think those should use VVec.

> +}

Let's move all the LogBuffer code into gsp/logbuffer.rs to keep gsp.rs clean.

> +#[cfg(CONFIG_NOVA_CORE_KEEP_GSP_LOGS)]
> +impl LogBuffers {
> +    /// Preserves whatever the GSP logged, so it can still be read once the GPU is gone.
> +    ///
> +    /// The buffers are DMA allocations of the device and cannot outlive it, so their contents are
> +    /// copied into memory owned by the module and exposed through fresh debugfs entries. Those
> +    /// live until the module is unloaded.
> +    fn retain(&self) -> Result {
> +        let logs = RetainedLogBuffers {
> +            dev: self.dev.clone(),
> +            loginit: self.loginit.snapshot()?,
> +            logintr: self.logintr.snapshot()?,
> +            logrm: self.logrm.snapshot()?,
> +        };
> +
> +        if logs.loginit.is_empty() && logs.logintr.is_empty() && logs.logrm.is_empty() {
> +            return Ok(());
> +        }
> +
> +        let mut retained = crate::RETAINED_LOGS.lock();
> +
> +        // An earlier run of the same device may have left a copy behind. Its directory carries
> +        // the name about to be used again, and its logs are the older ones, so drop it first.
> +        retained
> +            .gpus
> +            .retain(|gpu| gpu.dev.name() != self.dev.name());
> +
> +        let dir = match retained.dir.clone() {
> +            Some(dir) => dir,
> +            None => {
> +                #[allow(static_mut_refs)]
> +                // SAFETY: `DEBUGFS_ROOT` is set before driver registration and cleared after
> +                // driver unregistration. This runs while a device is still bound, or on the way
> +                // out of a failed probe, so the driver is registered and nothing can be modifying
> +                // it.
> +                let root: &debugfs::Dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }.ok_or(ENODEV)?;

I think we can avoid this additional unsafe if we just create the retained dir
right away in module_init().

> +
> +                let dir = root.subdir(c"retained");
> +                retained.dir = Some(dir.clone());
> +
> +                dir
> +            }
> +        };
> +
> +        let scope = KBox::pin_init(
> +            dir.scope(logs, self.dev.name(), |logs, dir| {
> +                if !logs.loginit.is_empty() {
> +                    dir.read_binary_file(c"loginit", &logs.loginit);
> +                }
> +                if !logs.logintr.is_empty() {
> +                    dir.read_binary_file(c"logintr", &logs.logintr);
> +                }
> +                if !logs.logrm.is_empty() {
> +                    dir.read_binary_file(c"logrm", &logs.logrm);
> +                }
> +            }),
> +            GFP_KERNEL,
> +        )?;
> +
> +        retained.gpus.push(scope, GFP_KERNEL)?;
> +
> +        dev_info!(

dev_dbg!() should be good enough.

> +            self.dev,
> +            "GSP-RM log buffers retained until the module is unloaded\n"
> +        );
> +
> +        Ok(())
> +    }