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 <[email protected]> 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/[email protected]?part=17
