Re: [PATCH v4 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] The `dma_mb(Read)` memory barrier does not protect against data races because the snapshot relies on the `put` pointer re-read by the unordered `copy_to_slice()`.
--

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

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

Adds a module parameter to retain GSP-RM log buffers after unbind, making
them available through debugfs. This copies the contents into module-owned
memory since the original buffers are DMA allocations that do not outlive
the device.

> diff --git a/drivers/gpu/nova-core/gsp/logbuffer.rs b/drivers/gpu/nova-core/gsp/logbuffer.rs
> new file mode 100644
> index 0000000000000..890aa2f9e38e7
> --- /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]
Does the unordered copy_to_slice() at the end of this function negate the
effect of dma_mb(Read)?

If a boot times out, the GSP may still be appending logs when
LogBuffer::snapshot() is called. The safely ordered "put" value is dropped
here, and the snapshot relies on the "put" pointer being re-read by the
unordered copy_to_slice() operation.

This means copy_to_slice() can read the log payload before reading the "put"
pointer at offset 0. If the GSP concurrently writes new logs and updates the
"put" pointer, the snapshot could capture the updated "put" pointer but stale
log payload data, causing the user-space decoder to parse garbage data.

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