Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing sequence number validation allows mismatched RPC replies to be accepted after a timeout. -- commit 0c5cf61612018cf7b218d6031a8329297e4ac9dc Author: Gary Guo <[email protected]> gpu: nova-core: fix barrier usage in CPU->GSP messaging path This commit fixes the barrier usage in the CPU->GSP messaging path. It clarifies the LOAD->STORE ordering dependency and moves the STORE->STORE barrier to immediately before the write pointer advance to prevent the GSP from observing an advanced pointer before the command is fully written. > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs > b/drivers/gpu/nova-core/gsp/cmdq.rs > index 070de0731e950..94c2790d943d0 100644 > --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs [ ... ] [Severity: High] This isn't a bug introduced by this patch, but while looking at the messaging path, I noticed a potential issue with sequence number validation. If a previous command times out in Cmdq::send_command(): drivers/gpu/nova-core/gsp/cmdq.rs:Cmdq::send_command() { ... loop { match inner.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) { Ok(reply) => break Ok(reply), Err(ERANGE) => continue, Err(e) => break Err(e), } } ... } The timeout returns ETIMEDOUT and drops the command queue lock, but the delayed reply might eventually arrive and remain in the message queue. When another command with the same function code is subsequently sent, CmdqInner::receive_msg() validates the incoming message: drivers/gpu/nova-core/gsp/cmdq.rs:CmdqInner::receive_msg() { ... let result = if function == M::FUNCTION { let (cmd, contents_1) = M::Message::from_bytes_prefix(... ... } Does this code incorrectly accept the stale reply as a successful response because it only checks if function == M::FUNCTION? Could this break RPC protocol synchronization and cause the driver to process incorrect payload data? Should the sequence number be validated (e.g., via header.sequence()) to prevent mismatched replies? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
