Re: [PATCH v3 09/14] gpu: nova-core: recover the GSP receive path from corrupt framing

On Fri Sep 4, 2026 at 11:53 AM BST, Alexandre Courbot wrote:

On Thu Sep 3, 2026 at 12:15 PM JST, John Hubbard wrote:
<...>

quoted

@@ -838,23 +871,26 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
         let function = message.header.function();
         let seq = message.header.sequence();
 
-        // 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, including a failed decode.
         let result = if matches!(function, Ok(f) if f == M::FUNCTION) {
-            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]);
-
-            M::read(cmd, &mut sbuffer)
-                .map_err(|e| e.into())
-                .inspect(|_| {
-                    if !sbuffer.is_empty() {
-                        dev_warn!(
-                            &self.dev,
-                            "GSP message {:?} has unprocessed data\n",
-                            M::FUNCTION
-                        );
-                    }
-                })
+            match M::Message::from_bytes_prefix(message.contents.0) {
+                Some((cmd, contents_1)) => {
+                    let mut sbuffer = SBufferIter::new_reader([contents_1, message.contents.1]);
+
+                    M::read(cmd, &mut sbuffer)
+                        .map_err(|e| e.into())
+                        .inspect(|_| {
+                            if !sbuffer.is_empty() {
+                                dev_warn!(
+                                    &self.dev,
+                                    "GSP message {:?} has unprocessed data\n",
+                                    M::FUNCTION
+                                );
+                            }
+                        })
+                }
+                None => Err(EIO),

This error path is the only one without a warning. How about:

    None => Err(EIO)
        .inspect_err(|_| dev_warn!(&self.dev, "GSP message {:?} too short\n", M::FUNCTION)),

I don't see why we want to use `inspect_err` here (just to make it an oneliner?)

Please do

    None => {
        dev_warn!(&self.dev, "GSP message {:?} too short\n", M::FUNCTION);
        Err(EIO)
    }

Instead.

Thanks,
Gary