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

["Gary Guo" <gary@garyguo.net>]

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:
> <...>
>> @@ -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