[PATCH 11/17] gpu: nova-core: match GSP RPC replies by sequence, not just function

[John Hubbard <jhubbard@nvidia.com>]

The GSP replies to a command by echoing that command's function code and
its RPC sequence number.

nova-core matched replies on the function alone and never set the
sequence, so a reply for a command that had already timed out could
satisfy a later command using the same function.

Give the RPC sequence its own counter, separate from the per-element
transport sequence, set it on every command, and require both the
function and the sequence to match before accepting a reply. A message
with the expected function but a stale sequence is logged and dropped,
not mistaken for the reply or dispatched as an event. A caller awaiting
an unsolicited event still matches on the function alone.

Assisted-by: Cursor:claude-opus-5
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 89 ++++++++++++++++++++-----------
 drivers/gpu/nova-core/gsp/fw.rs   | 13 +++--
 2 files changed, 67 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 0df52df1da89..3224079abf7e 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -521,7 +521,8 @@ pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Err
                 inner <- new_mutex!(CmdqInner {
                     dev: dev.into(),
                     gsp_mem,
-                    seq: 0,
+                    elem_seq: 0,
+                    rpc_seq: 0,
                 }),
             }))
         })
@@ -569,10 +570,10 @@ pub(crate) fn send_command<M>(&self, bar: Bar0<'_>, command: M) -> Result<M::Rep
         Error: From<<M::Reply as MessageFromGsp>::InitError>,
     {
         let mut inner = self.inner.lock();
-        inner.send_command(bar, command)?;
+        let expected_seq = inner.send_command(bar, command)?;
 
         loop {
-            match inner.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
+            match inner.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT, Some(expected_seq)) {
                 Ok(reply) => break Ok(reply),
                 Err(ERANGE) => continue,
                 Err(e) => break Err(e),
@@ -594,18 +595,19 @@ pub(crate) fn send_command_no_wait<M>(&self, bar: Bar0<'_>, command: M) -> Resul
         M: CommandToGsp<Reply = NoReply>,
         Error: From<M::InitError>,
     {
-        self.inner.lock().send_command(bar, command)
+        self.inner.lock().send_command(bar, command).map(|_| ())
     }
 
     /// Receive a message from the GSP.
     ///
-    /// See [`CmdqInner::receive_msg`] for details.
+    /// Matches on the function code alone, for a caller awaiting an unsolicited GSP event rather
+    /// than a reply to a command. See [`CmdqInner::receive_msg`].
     pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
     {
-        self.inner.lock().receive_msg(timeout)
+        self.inner.lock().receive_msg(timeout, None)
     }
 }
 
@@ -613,8 +615,13 @@ pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M>
 struct CmdqInner {
     /// Device this command queue belongs to.
     dev: ARef<device::Device>,
-    /// Current command sequence number.
-    seq: u32,
+    /// Next transport sequence number for a queue element (the `seqNum` field). Advances once per
+    /// queue element, including each continuation record.
+    elem_seq: u32,
+    /// Next RPC sequence number. The GSP echoes it in a command's reply, which lets
+    /// [`CmdqInner::receive_msg`] match that reply to the awaiting command. Advances once per
+    /// logical command.
+    rpc_seq: u32,
     /// Memory area shared with the GSP for communicating commands and messages.
     gsp_mem: DmaGspMem,
 }
@@ -633,7 +640,7 @@ impl CmdqInner {
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    fn send_single_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result
+    fn send_single_command<M>(&mut self, bar: Bar0<'_>, command: M, rpc_seq: u32) -> Result
     where
         M: CommandToGsp,
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
@@ -650,7 +657,7 @@ fn send_single_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result
         let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
 
         // Fill the header and command in-place.
-        let msg_element = GspMsgElement::init(self.seq, size_in_bytes, M::FUNCTION);
+        let msg_element = GspMsgElement::init(self.elem_seq, rpc_seq, size_in_bytes, M::FUNCTION);
         // SAFETY: `msg_header` and `cmd` are valid references, and not touched if the initializer
         // fails.
         unsafe {
@@ -678,23 +685,25 @@ fn send_single_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result
         dev_dbg!(
             &self.dev,
             "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
-            self.seq,
+            rpc_seq,
             M::FUNCTION,
             dst.header.length(),
         );
 
         // All set - update the write pointer and inform the GSP of the new command.
         let elem_count = dst.header.element_count();
-        self.seq += 1;
+        self.elem_seq = self.elem_seq.wrapping_add(1);
         self.gsp_mem.advance_cpu_write_ptr(elem_count);
         Cmdq::notify_gsp(bar);
 
         Ok(())
     }
 
-    /// Sends `command` to the GSP.
+    /// Sends `command` to the GSP and returns the RPC sequence number assigned to it.
     ///
-    /// The command may be split into multiple messages if it is large.
+    /// The command may be split into multiple messages if it is large. The GSP echoes the
+    /// sequence number in the reply, so a caller passes it to [`Self::receive_msg`] to match the
+    /// reply to this command.
     ///
     /// # Errors
     ///
@@ -703,24 +712,26 @@ fn send_single_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    fn send_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result
+    fn send_command<M>(&mut self, bar: Bar0<'_>, command: M) -> Result<u32>
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
     {
+        let rpc_seq = self.rpc_seq;
+        self.rpc_seq = self.rpc_seq.wrapping_add(1);
+
         match SplitState::new(command)? {
-            SplitState::Single(command) => self.send_single_command(bar, command),
+            SplitState::Single(command) => self.send_single_command(bar, command, rpc_seq)?,
             SplitState::Split(command, mut continuations) => {
-                self.send_single_command(bar, command)?;
+                self.send_single_command(bar, command, rpc_seq)?;
 
                 while let Some(continuation) = continuations.next() {
-                    // Turbofish needed because the compiler cannot infer M here.
-                    self.send_single_command::<ContinuationRecord<'_>>(bar, continuation)?;
+                    self.send_single_command::<ContinuationRecord<'_>>(bar, continuation, rpc_seq)?;
                 }
-
-                Ok(())
             }
         }
+
+        Ok(rpc_seq)
     }
 
     /// Wait for a message to become available on the message queue.
@@ -805,10 +816,14 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
 
     /// Receive a message from the GSP.
     ///
-    /// The expected message type is specified using the `M` generic parameter. A message whose
-    /// function code matches is decoded and returned. Any other message, whether its function code
-    /// is a different one or is unrecognized, goes to [`Self::dispatch_event`] and `ERANGE` is
-    /// returned.
+    /// The expected message type is given by the `M` generic parameter. With `expected_seq` set,
+    /// the message must also carry that RPC sequence number to count as the awaited reply. With
+    /// `None`, the function code alone decides the match.
+    ///
+    /// A matching message is decoded and returned. A message carrying the expected function code
+    /// with a different sequence is a stale reply to a command that already timed out, and is
+    /// logged and dropped. Any other message goes to [`Self::dispatch_event`]. Both non-matching
+    /// cases return `ERANGE`.
     ///
     /// The read pointer is always advanced past the message, regardless of whether it matched.
     ///
@@ -820,7 +835,11 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
     /// - `ERANGE` if the message was not the awaited reply.
     ///
     /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
-    fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
+    fn receive_msg<M: MessageFromGsp>(
+        &mut self,
+        timeout: Delta,
+        expected_seq: Option<u32>,
+    ) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
@@ -828,10 +847,10 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
         let message = self.wait_for_msg(timeout)?;
         let function = message.header.function();
         let seq = message.header.sequence();
-        let matched = matches!(function, Ok(f) if f == M::FUNCTION);
+        let func_matches = matches!(function, Ok(f) if f == M::FUNCTION);
+        let matched = func_matches && expected_seq.is_none_or(|expected| seq == expected);
 
-        // Bind the result rather than returning early. The read pointer must advance past this
-        // message on every path.
+        // Every path must advance the read pointer past this message.
         let result = if matched {
             let (cmd, contents_1) = M::Message::from_bytes_prefix(message.contents.0).ok_or(EIO)?;
             let mut sbuffer = SBufferIter::new_reader([contents_1, message.contents.1]);
@@ -857,7 +876,17 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
         )?);
 
         if !matched {
-            self.dispatch_event(function, seq);
+            if func_matches {
+                dev_warn!(
+                    &self.dev,
+                    "GSP RPC: dropping stale {:?} reply (seq {}, awaiting {:?})\n",
+                    M::FUNCTION,
+                    seq,
+                    expected_seq,
+                );
+            } else {
+                self.dispatch_event(function, seq);
+            }
         }
 
         result
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 05f54fee6186..0b01c81ec092 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -782,13 +782,14 @@ fn new() -> Self {
 }
 
 impl bindings::rpc_message_header_v {
-    fn init(cmd_size: usize, function: MsgFunction) -> impl Init<Self, Error> {
+    fn init(sequence: u32, cmd_size: usize, function: MsgFunction) -> impl Init<Self, Error> {
         type RpcMessageHeader = bindings::rpc_message_header_v;
 
         try_init!(RpcMessageHeader {
             header_version: MsgHeaderVersion::new().into(),
             signature: bindings::NV_VGPU_MSG_SIGNATURE_VALID,
             function: function.into(),
+            sequence,
             length: size_of::<Self>()
                 .checked_add(cmd_size)
                 .ok_or(EOVERFLOW)
@@ -813,25 +814,27 @@ impl GspMsgElement {
     ///
     /// # Arguments
     ///
-    /// * `sequence` - Sequence number of the message.
+    /// * `elem_seq` - Transport sequence number of the queue element (`seqNum`).
+    /// * `rpc_seq` - RPC sequence number, echoed by the GSP in the reply.
     /// * `cmd_size` - Size of the command (not including the message element), in bytes.
     /// * `function` - Function of the message.
     pub(crate) fn init(
-        sequence: u32,
+        elem_seq: u32,
+        rpc_seq: u32,
         cmd_size: usize,
         function: MsgFunction,
     ) -> impl Init<Self, Error> {
         type RpcMessageHeader = bindings::rpc_message_header_v;
         type InnerGspMsgElement = bindings::GSP_MSG_QUEUE_ELEMENT;
         let init_inner = try_init!(InnerGspMsgElement {
-            seqNum: sequence,
+            seqNum: elem_seq,
             elemCount: size_of::<Self>()
                 .checked_add(cmd_size)
                 .ok_or(EOVERFLOW)?
                 .div_ceil(GSP_PAGE_SIZE)
                 .try_into()
                 .map_err(|_| EOVERFLOW)?,
-            rpc <- RpcMessageHeader::init(cmd_size, function),
+            rpc <- RpcMessageHeader::init(rpc_seq, cmd_size, function),
             ..Zeroable::init_zeroed()
         });
 
-- 
2.55.0