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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing dma_rmb() between reading the 'put' pointer and reading the log buffer contents.
--

--- Patch [1]: [PATCH 1/2] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind ---
commit 4ba5ac610ccf9ff75ece2b976056ff21931acf77
Author: Vladislav Zaharov <[email protected]>

gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind

This commit adds a configuration option to retain GSP-RM log buffers after
the GPU is unbound. It copies the GSP log data into module-owned memory
so it can still be accessed through debugfs to help debug failed probes.

> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 13f361406a6c3..7e2aa2dfbea3d 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs

[ ... ]

> @@ -133,9 +135,30 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
>  
>          Ok(obj)
>      }
> +
> +    /// Copies the contents of this buffer into memory that does not belong to the device.
> +    ///
> +    /// A buffer the GSP never wrote to yields an empty vector, as it holds nothing worth keeping.
> +    #[cfg(CONFIG_NOVA_CORE_KEEP_GSP_LOGS)]
> +    fn snapshot(&self) -> Result<KVec<u8>> {
> +        // Offset 0 holds the "put" pointer, which the GSP advances as it appends entries. It is
> +        // still zero if nothing was ever logged.
> +        let put = io_project!(self.0, [build: ..size_of::<u64>()]).try_cast::<u64>()?;
> +        if put.read_val() == 0 {
> +            return Ok(KVec::new());
> +        }
> +
> +        let mut snapshot = KVec::zeroed(LOG_BUFFER_SIZE, GFP_KERNEL)?;
> +        io_project!(self.0, [build: ..]).copy_to_slice(&mut snapshot);

[Severity: Medium]
Does this code need a memory barrier (such as dma_rmb()) between reading the
'put' pointer and copying the buffer contents?

Because the 'put' status pointer is read from DMA memory using a volatile read,
a control dependency does not prevent the CPU from speculatively executing
subsequent loads before the status check completes. This could cause the CPU
to read stale or zeroed memory before the GSP's DMA writes are fully visible,
leading to an empty or corrupted debugfs log dump.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1