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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpu: nova-core: unordered put pointer in log buffer snapshot
--

commit e3a18fe9cea338c661ee3d185d004b01a5ff3d7d
Author: Vladislav Zaharov <vladazaharova2018@gmail.com>

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

This commit adds a gsp_keep_logs module parameter to retain GSP-RM log
buffers after GPU unbind or probe failure. It creates copies of the
buffers in memory owned by the module and exposes them through debugfs,
moving the buffer logic to a separate logbuffer.rs file.

> diff --git a/drivers/gpu/nova-core/gsp/logbuffer.rs b/drivers/gpu/nova-core/gsp/logbuffer.rs
> new file mode 100644
> index 0000000000000..b1f912fb3fae1
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/logbuffer.rs
[ ... ]
> +    fn snapshot(&self) -> Result<VVec<u8>> {
> +        // Offset 0 holds the "put" pointer, which the GSP advances as it appends entries. It is
> +        // still zero if nothing was ever logged, which is all that is tested here: a buffer that
> +        // was written to is copied whole, and making sense of "put" is left to the decoder.
> +        let put = io_project!(self.0, [build: ..size_of::<u64>()]).try_cast::<u64>()?;
> +        if put.read_val() == 0 {
> +            return Ok(VVec::new());
> +        }
> +
> +        // ORDERING: LOAD->LOAD ordering needed to order the "put" read before the data read. The
> +        // GSP has normally been stopped by the time this runs, but a boot that timed out can leave
> +        // it still appending.
> +        dma_mb(Read);
> +
> +        let mut snapshot = VVec::zeroed(LOG_BUFFER_SIZE, GFP_KERNEL)?;
> +        io_project!(self.0, [build: ..]).copy_to_slice(&mut snapshot);

[Severity: Medium]
Since copy_to_slice() reads the entire buffer, including the put pointer at
offset 0, and has no internal read ordering guarantees, can the read of the
put pointer inside copy_to_slice() be reordered after the data reads?

If the GSP is still appending data concurrently (e.g. after a boot timeout),
the snapshot might capture a newer put pointer but stale log data. This could
cause the log parser to read garbage or fail, defeating the purpose of the
explicit dma_mb(Read) barrier.

> +
> +        Ok(snapshot)
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923045551.229259-1-vladazaharova2018@gmail.com?part=2