Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Missing dma_rmb() between reading the 'put' pointer and reading the log buffer contents. --
--- Patch [1]: [PATCH 1/2] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind --- commit 4ba5ac610ccf9ff75ece2b976056ff21931acf77 Author: Vladislav Zaharov <[email protected]> gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind This commit adds a configuration option to retain GSP-RM log buffers after the GPU is unbound. It copies the GSP log data into module-owned memory so it can still be accessed through debugfs to help debug failed probes. > diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs > index 13f361406a6c3..7e2aa2dfbea3d 100644 > --- a/drivers/gpu/nova-core/gsp.rs > +++ b/drivers/gpu/nova-core/gsp.rs [ ... ] > @@ -133,9 +135,30 @@ fn new(dev: &device::Device<device::Bound>) -> > Result<Self> { > > Ok(obj) > } > + > + /// Copies the contents of this buffer into memory that does not belong > to the device. > + /// > + /// A buffer the GSP never wrote to yields an empty vector, as it holds > nothing worth keeping. > + #[cfg(CONFIG_NOVA_CORE_KEEP_GSP_LOGS)] > + fn snapshot(&self) -> Result<KVec<u8>> { > + // Offset 0 holds the "put" pointer, which the GSP advances as it > appends entries. It is > + // still zero if nothing was ever logged. > + let put = io_project!(self.0, [build: > ..size_of::<u64>()]).try_cast::<u64>()?; > + if put.read_val() == 0 { > + return Ok(KVec::new()); > + } > + > + let mut snapshot = KVec::zeroed(LOG_BUFFER_SIZE, GFP_KERNEL)?; > + io_project!(self.0, [build: ..]).copy_to_slice(&mut snapshot); [Severity: Medium] Does this code need a memory barrier (such as dma_rmb()) between reading the 'put' pointer and copying the buffer contents? Because the 'put' status pointer is read from DMA memory using a volatile read, a control dependency does not prevent the CPU from speculatively executing subsequent loads before the status check completes. This could cause the CPU to read stale or zeroed memory before the GSP's DMA writes are fully visible, leading to an empty or corrupted debugfs log dump. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
