In the GSP->CPU messaging path, the code reads the write pointer from GSP, reads the response and advances the read pointer.
A LOAD->LOAD ordering is required after the write pointer read and the data read. Add it as this is currently missing. A LOAD->STORE ordering is required after the data read and the advance of read pointer. Currently a Rust `SeqCst` barrier is used, which roughly maps to `smp_mb(Full)`; this however does not order DMA operations (notably on ARM, the generate barrier orders inner shareable and not outer shareable, which is ordered by `dma_mb`). This ordering does not need to be in between read pointer read and write, because it's for ordering between the ring buffer data and the pointer; the RMW operation does not internally need a barrier (nor it has to be atomic, as CPU pointers are updated by CPU only), so move it to before the RMW sequence for clarity. Signed-off-by: Gary Guo <[email protected]> --- drivers/gpu/nova-core/gsp/cmdq.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index 70674d2d0f77..9fe393da6b10 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -2,13 +2,7 @@ mod continuation; -use core::{ - mem, - sync::atomic::{ - fence, - Ordering, // - }, -}; +use core::mem; use kernel::{ device, @@ -30,6 +24,7 @@ barrier::{ dma_mb, Full, + Read, Write, // }, Mutex, // @@ -339,6 +334,9 @@ fn driver_write_area_size(&self) -> usize { (MSGQ_NUM_PAGES, tx) }; + // ORDERING: LOAD->LOAD ordering needed to order `gsp_write_ptr` read before data read. + dma_mb(Read); + // SAFETY: // - `data` was created from a valid pointer, and `rx` and `tx` are in the // `0..MSGQ_NUM_PAGES` range per the invariants of `gsp_write_ptr` and `cpu_read_ptr`, @@ -436,12 +434,11 @@ fn cpu_read_ptr(&self) -> u32 { // Informs the GSP that it can send `elem_count` new pages into the message queue. fn advance_cpu_read_ptr(&mut self, elem_count: u32) { + // ORDERING: LOAD->STORE ordering needed to order `cpu_read_ptr` write after data read. + dma_mb(Full); + let rx = io_project!(self.0, .cpuq.rx); let rptr = MsgqRxHeader::read_ptr(rx).wrapping_add(elem_count) % MSGQ_NUM_PAGES; - - // Ensure read pointer is properly ordered. - fence(Ordering::SeqCst); - MsgqRxHeader::set_read_ptr(rx, rptr) } -- 2.54.0
