In the CPU->GSP messaging path, the code reads the read pointer from GSP, writes the command, advances the write pointer, and then notifies the GSP.
A LOAD->STORE ordering is needed after reading the read pointer from GSP and writing the command. A barrier is not needed here because control dependency provides the required ordering. Still, document it explicitly. A STORE->STORE ordering is needed after the command write and the write pointer advance. This is currently incorrectly done after the write pointer advance (and before GSP notification), but this can cause issue if GSP is still processing ring buffer, as it may observe the write pointer advance before command write. Thus move this barrier to be before the write pointer advance. Note that barriers are not needed between write pointer advance and GSP notification, as MMIO accessors already carries the required barrier. Signed-off-by: Gary Guo <[email protected]> --- drivers/gpu/nova-core/gsp/cmdq.rs | 10 ++++++++++ drivers/gpu/nova-core/gsp/fw.rs | 3 --- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index 070de0731e95..94c2790d943d 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -20,6 +20,10 @@ ptr, sync::{ aref::ARef, + barrier::{ + dma_mb, + Write, // + }, Mutex, // }, time::Delta, @@ -274,6 +278,9 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> { (rx - 1, 0) }; + // ORDERING: control dependency provides necessary LOAD->STORE ordering. + // `dma_mb(Full)` may be used here if we don't want to rely on control dependency. + // SAFETY: // - `data` was created from a valid pointer, and `rx` and `tx` are in the // `0..MSGQ_NUM_PAGES` range per the invariants of `cpu_write_ptr` and `gsp_read_ptr`, @@ -443,6 +450,9 @@ fn cpu_write_ptr(&self) -> u32 { // 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) { + // ORDERING: Ensure all command data is visible before updateing ring buffer pointer. + dma_mb(Write); + super::fw::gsp_mem::advance_cpu_write_ptr(&self.0, elem_count) } } diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs index 4db0cfa4dc4d..fae2eae495d1 100644 --- a/drivers/gpu/nova-core/gsp/fw.rs +++ b/drivers/gpu/nova-core/gsp/fw.rs @@ -91,9 +91,6 @@ pub(in crate::gsp) fn advance_cpu_write_ptr(qs: &Coherent<GspMem>, count: u32) { let wptr = cpu_write_ptr(qs).wrapping_add(count) % MSGQ_NUM_PAGES; dma_write!(qs, .cpuq.tx.0.writePtr, wptr); - - // Ensure all command data is visible before triggering the GSP read. - fence(Ordering::SeqCst); } } -- 2.54.0
