Re: [PATCH v5 17/20] gpu: nova-core: use I/O projection for cleaner encapsulation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data corruption in GSP command queue due to missing memory barrier before updating the ring buffer write pointer.
--

commit abd2f0cd14180222d86d47f37c313be60a30d773
Author: Gary Guo <gary@garyguo.net>

gpu: nova-core: use I/O projection for cleaner encapsulation

This commit refactors the nova-core GSP driver to use the io_project!,
io_read!, and io_write! macros for memory-mapped I/O access. It replaces
the previous dma_read! and dma_write! usage to enforce better encapsulation
for PTE arrays and message queues.

> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs

[ ... ]

> @@ -438,12 +441,17 @@
>      // Informs the GSP that it can process `elem_count` new pages from the command queue.
>      fn advance_cpu_write_ptr(&mut self, elem_count: u32) {
> -        super::fw::gsp_mem::advance_cpu_write_ptr(&self.0, elem_count)
> +        let tx = io_project!(self.0, .cpuq.tx);
> +        let wptr = MsgqTxHeader::write_ptr(tx).wrapping_add(elem_count) % MSGQ_NUM_PAGES;
> +        MsgqTxHeader::set_write_ptr(tx, wptr);
> +
> +        // Ensure all command data is visible before triggering the GSP read.
> +        fence(Ordering::SeqCst);

[Severity: High]
This is a pre-existing issue, but is it possible that the memory barrier is
placed too late in this function?

In advance_cpu_write_ptr(), the fence(Ordering::SeqCst) is placed after
updating the ring buffer's write pointer via set_write_ptr(). Since the
payload is written to coherent memory using normal memory stores while the
write pointer uses a volatile store, weakly-ordered CPUs might reorder the
operations.

If the GSP firmware polls the write pointer before the payload writes are
globally visible, could it read uninitialized or stale data? Should the
fence be placed before the set_write_ptr() call to prevent this race?

>      }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-io_projection-v5-0-d0961471ae50@garyguo.net?part=17