Re: [PATCH v2 3/4] gpu: nova-core: fix barrier usage in CPU->GSP messaging path

[sashiko-bot@kernel.org]

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 <gary@garyguo.net>

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/20260609-rust-barrier-v2-0-30fcc48e1cd0@garyguo.net?part=3