[PATCH v3 33/33] gpu: nova-core: gsp: match a GMC response by flag, id and sequence

[John Hubbard]

From: Zhi Wang <zhiw@xxxxxxxxxx>

GSP-RM marks a response with a flag bit in the GMC command word, and it
echoes back the RPC sequence number that the request carried. An event
carries neither, and an event can name any command id. The r000 GSP-RM
raises no event that names the GSP_INIT id.

Nova-core matched the GSP_INIT reply on the command id alone. The
receive path would have decoded an event that named the same id as the
reply, and dropped the real reply, so only the absence of such an event
kept the boot working.

Return the sequence number that a GMC send used, and require the
response flag, the command id and that sequence to match before
decoding a reply. A command that GSP-RM does not answer goes through
its own send path, which has no sequence to return.

Signed-off-by: Zhi Wang <zhiw@xxxxxxxxxx>
Assisted-by: LLM
Co-developed-by: John Hubbard <jhubbard@xxxxxxxxxx>
Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
---
 drivers/gpu/nova-core/gsp/cmdq.rs     | 44 ++++++++++++++++++---------
 drivers/gpu/nova-core/gsp/commands.rs |  8 +++--
 drivers/gpu/nova-core/gsp/fw.rs       | 26 +++++++++++++---
 3 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 2664ace70ef8..dd0b3dfc6f37 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -642,8 +642,8 @@ pub(crate) fn send_command_no_wait<M>(&self, command: M) -> Result
         self.inner.lock().send_command(command).map(|_| ())
     }
 
-    /// Waits for the response to the GMC request with command id `command_id`, and passes every
-    /// other GMC element that arrives first to `on_other`.
+    /// Waits for the response to the GMC request with command id `command_id` and RPC sequence
+    /// number `sequence`, and passes every other GMC element that arrives first to `on_other`.
     ///
     /// This method may sleep while waiting. The queue mutex stays locked across the whole wait and
     /// across the `on_other` and `decode` calls, so neither may call back into this [`Cmdq`].
@@ -652,16 +652,19 @@ pub(crate) fn send_command_no_wait<M>(&self, command: M) -> Result
     pub(crate) fn await_gmc_response<R>(
         &self,
         command_id: u32,
+        sequence: u32,
         on_other: impl FnMut(&GspGmcMsgElement, &[u8], &[u8]) -> Result,
         decode: impl FnMut(&[u8], &[u8]) -> Result<R>,
     ) -> Result<R> {
         self.inner
             .lock()
-            .await_gmc_response(command_id, on_other, decode)
+            .await_gmc_response(command_id, sequence, on_other, decode)
     }
 
     /// Sends a GMC API request to the GSP without waiting for the response.
     ///
+    /// Returns the RPC sequence number that the request carries.
+    ///
     /// # Errors
     ///
     /// Errors from [`DmaGspMem::allocate_command`] are propagated as-is.
@@ -670,12 +673,24 @@ pub(crate) fn send_gmc_no_wait(
         command_id: u32,
         payload: &[u8],
         max_response_size: u32,
-    ) -> Result {
+    ) -> Result<u32> {
         self.inner
             .lock()
             .send_gmc(command_id, payload, max_response_size)
     }
 
+    /// Sends a GMC API request that GSP-RM does not answer.
+    ///
+    /// # Errors
+    ///
+    /// Errors from [`DmaGspMem::allocate_command`] are propagated as-is.
+    pub(crate) fn send_gmc_no_reply(&self, command_id: u32, payload: &[u8]) -> Result {
+        self.inner
+            .lock()
+            .send_gmc(command_id, payload, 0)
+            .map(|_| ())
+    }
+
     /// Waits for an unsolicited GSP event of type `M`. Events that arrive before it are logged and
     /// consumed.
     ///
@@ -844,10 +859,12 @@ fn poison(&self, reason: fmt::Arguments<'_>) -> Error {
     /// response that the caller accepts. The request carries the next RPC sequence number, which
     /// GSP-RM copies into its response. The number is consumed even if the send fails.
     ///
+    /// Returns the RPC sequence number that the request carries.
+    ///
     /// # Errors
     ///
     /// Errors from [`DmaGspMem::allocate_command`] are propagated as-is.
-    fn send_gmc(&mut self, command_id: u32, payload: &[u8], max_response_size: u32) -> Result {
+    fn send_gmc(&mut self, command_id: u32, payload: &[u8], max_response_size: u32) -> Result<u32> {
         let rpc_seq = self.rpc_seq;
         self.rpc_seq = self.rpc_seq.wrapping_add(1);
 
@@ -855,12 +872,8 @@ fn send_gmc(&mut self, command_id: u32, payload: &[u8], max_response_size: u32)
             .gsp_mem
             .allocate_command::<GspGmcMsgElement>(payload.len(), Self::ALLOCATE_TIMEOUT)?;
 
-        let msg_element = GspGmcMsgElement::init(
-            command_id,
-            u64::from(rpc_seq),
-            payload.len(),
-            max_response_size,
-        );
+        let msg_element =
+            GspGmcMsgElement::init(command_id, rpc_seq, payload.len(), max_response_size);
         // SAFETY: `dst.header` is a valid reference, and not written if the initializer fails.
         unsafe {
             pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element)?;
@@ -880,7 +893,7 @@ fn send_gmc(&mut self, command_id: u32, payload: &[u8], max_response_size: u32)
         let elem_count = dst.header.element_count();
         self.gsp_mem.advance_cpu_write_ptr(elem_count);
 
-        Ok(())
+        Ok(rpc_seq)
     }
 
     /// Receives an element from the GSP.
@@ -1302,8 +1315,8 @@ fn receive_gmc_and_dispatch<R>(
         })
     }
 
-    /// Waits for the response to the GMC request with command id `command_id`, up to
-    /// [`Cmdq::RECEIVE_TIMEOUT`] from the call.
+    /// Waits for the response to the GMC request with command id `command_id` and RPC sequence
+    /// number `sequence`, up to [`Cmdq::RECEIVE_TIMEOUT`] from the call.
     ///
     /// The response's payload is passed to `decode`, as two slices because the ring may wrap.
     /// Every other GMC element that arrives first is passed to `on_other` with the headers that
@@ -1321,6 +1334,7 @@ fn receive_gmc_and_dispatch<R>(
     fn await_gmc_response<R>(
         &mut self,
         command_id: u32,
+        sequence: u32,
         mut on_other: impl FnMut(&GspGmcMsgElement, &[u8], &[u8]) -> Result,
         mut decode: impl FnMut(&[u8], &[u8]) -> Result<R>,
     ) -> Result<R> {
@@ -1334,7 +1348,7 @@ fn await_gmc_response<R>(
 
             let response =
                 self.receive_gmc_and_dispatch(remaining, |header, payload_0, payload_1| {
-                    if header.gmc.command_id() != command_id {
+                    if !header.gmc.is_response_to(command_id, sequence) {
                         return on_other(header, payload_0, payload_1).map(|()| None);
                     }
 
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 128d6f8dcb43..f1a3c0613e70 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -79,10 +79,12 @@ pub(crate) fn gsp_init(
     // Qualified because `zerocopy::IntoBytes` also gives `[T]` an `as_bytes`.
     let payload = AsBytes::as_bytes(payload);
 
-    cmdq.send_gmc_no_wait(GMCAPI_CMD_GSP_INIT, payload, GSP_INIT_MAX_RESPONSE_SIZE)?;
+    let sequence =
+        cmdq.send_gmc_no_wait(GMCAPI_CMD_GSP_INIT, payload, GSP_INIT_MAX_RESPONSE_SIZE)?;
 
     cmdq.await_gmc_response(
         GMCAPI_CMD_GSP_INIT,
+        sequence,
         on_unsolicited_element,
         decode_gsp_init_reply,
     )
@@ -124,9 +126,9 @@ fn decode_gsp_init_reply(payload_0: &[u8], payload_1: &[u8]) -> Result<GspStatic
 ///
 /// # Errors
 ///
-/// Errors from [`Cmdq::send_gmc_no_wait`] are propagated as-is.
+/// Errors from [`Cmdq::send_gmc_no_reply`] are propagated as-is.
 pub(crate) fn gsp_suspend(cmdq: &Cmdq<'_>, level: PowerStateLevel) -> Result {
     let params = fw::commands::GspSuspend::new(level);
 
-    cmdq.send_gmc_no_wait(GMCAPI_CMD_GSP_SUSPEND, AsBytes::as_bytes(&params), 0)
+    cmdq.send_gmc_no_reply(GMCAPI_CMD_GSP_SUSPEND, AsBytes::as_bytes(&params))
 }
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index f3dff49af2bf..f7e83e75e53e 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -725,6 +725,9 @@ pub(crate) struct GmcApiHeader {
 /// Bits of [`GmcApiHeader::command`] that hold the command id. The high byte holds flags.
 const GMCAPI_COMMAND_ID_MASK: u32 = 0x00ff_ffff;
 
+/// Flag bit of [`GmcApiHeader::command`] that GSP-RM sets on a response.
+const GMCAPI_COMMAND_FLAGS_RESPONSE: u32 = 0x0100_0000;
+
 /// GMC request that carries the system information and registry keys to GSP-RM. GSP-RM answers
 /// it with the static GPU configuration once it has finished starting.
 pub(crate) const GMCAPI_CMD_GSP_INIT: u32 = bindings::GMCAPI_COMMANDS_GMCAPI_CMD_GSP_INIT;
@@ -771,13 +774,26 @@ pub(crate) fn command_id(&self) -> u32 {
         self.command & GMCAPI_COMMAND_ID_MASK
     }
 
+    /// Returns `true` if GSP-RM sent this header as a response rather than an event.
+    fn is_response(&self) -> bool {
+        self.command & GMCAPI_COMMAND_FLAGS_RESPONSE != 0
+    }
+
     /// Returns the `NV_STATUS` that a response carries.
     ///
-    /// The value is meaningful only on a response, which GSP-RM marks with a flag in the command
-    /// word. In a request, the same word holds the largest response that the sender accepts.
+    /// The value is meaningful only when [`Self::is_response`] is `true`. In a request, the same
+    /// word holds the largest response that the sender accepts.
     pub(crate) fn status(&self) -> u32 {
         self.max_resp_or_status
     }
+
+    /// Returns `true` if this header answers the request with command id `command_id` and RPC
+    /// sequence number `sequence`.
+    pub(crate) fn is_response_to(&self, command_id: u32, sequence: u32) -> bool {
+        self.is_response()
+            && self.command_id() == command_id
+            && self.sequence == u64::from(sequence)
+    }
 }
 
 // SAFETY: All fields are integer types with no uninitialized padding bytes.
@@ -801,7 +817,7 @@ pub(crate) struct GspGmcMsgElement {
 
 impl GspGmcMsgElement {
     /// Creates the queue element header and the GMC API header of a request that carries
-    /// `payload_size` bytes of payload.
+    /// `payload_size` bytes of payload under the RPC sequence number `sequence`.
     ///
     /// `max_response_size` is the largest response that the sender accepts, and zero for a request
     /// that GSP-RM does not answer.
@@ -811,7 +827,7 @@ impl GspGmcMsgElement {
     /// - `EOVERFLOW` if a length does not fit its 32-bit field.
     pub(crate) fn init(
         command_id: u32,
-        sequence: u64,
+        sequence: u32,
         payload_size: usize,
         max_response_size: u32,
     ) -> impl Init<Self, Error> {
@@ -825,7 +841,7 @@ pub(crate) fn init(
             gmc: GmcApiHeader {
                 command: command_id,
                 size: payload_size.try_into().map_err(|_| EOVERFLOW)?,
-                sequence,
+                sequence: u64::from(sequence),
                 max_resp_or_status: max_response_size,
                 reserved: [0; 5],
             },
-- 
2.55.0