Re: [PATCH v3 12/33] gpu: nova-core: gsp: add GMC dispatch on receive

[John Hubbard]

On 9/18/26 3:01 PM, Timur Tabi wrote:
> On Thu, 2026-09-17 at 18:06 -0700, John Hubbard wrote:
>>
>>      /// Waits for an unsolicited GSP event of type `M`. Events that arrive before it are
>> logged and
>>      /// consumed.
>>      ///
>> @@ -1097,9 +1110,9 @@ fn payload_slices<'a>(
>>      /// # Errors
>>      ///
>>      /// - `ETIMEDOUT` if no element arrives within `timeout`.
>> -    /// - `EIO` if the queue is already poisoned, or if the framing is invalid, which poisons
>> it
>> -    ///   (see [`Self::poisoned`]).
>> -    #[expect(dead_code)]
>> +    /// - `EIO` if the queue is already poisoned, or if the framing is invalid, or if the GMC
>> API
>> +    ///   header and the queue element header declare different payload sizes. Each of these
>> +    ///   poisons the queue (see [`Self::poisoned`]).
> 
> What does it mean for the size to be "declared"?

Yuk! I have written it with better words, in v4, thanks.

> 
>>      fn wait_for_element(&self, timeout: Delta) -> Result<QueueElement<'_>> {
>>          if self.poisoned.get() {
>>              return Err(EIO);
>> @@ -1146,8 +1159,89 @@ fn wait_for_element(&self, timeout: Delta) -> Result<QueueElement<'_>>
>> {
>>              )));
>>          };
>>  
>> +        // GSP-RM writes both sizes from the same payload, so a difference means that one of
>> the
>> +        // two headers is corrupt, and the driver cannot know which.
>> +        if payload_length != num::u32_as_usize(header.gmc.size) {
>> +            return Err(self.poison(fmt!(
>> +                "GMC seq# {}: GMC API header declares {} payload bytes, element header {}",
>> +                header.gmc.sequence,
>> +                header.gmc.size,
>> +                payload_length
>> +            )));
> 
> This message doesn't clearly specify what the error is.  How about something like:
> 
> "GMC API header declares payload size of {} bytes, but should be {}"

hmmm..."should be {}" assumes that the queue element header holds the right
value, but the driver cannot know which of the two headers is corrupt.

How about this:

        return Err(self.poison(fmt!(
            "GMC element seq# {} has payload size {} in its GMC API header but {} in \
                its queue element header",
            header.gmc.sequence,
            header.gmc.size,
            payload_length
        )));

It names the element, states the fault as one sentence, and puts each number
next to the header that it came from. 

Sample output: 

  GSP receive: queue poisoned: GMC element seq# 7 has payload size 96 in its GMC API header but 80 in its queue element header


thanks,
-- 
John Hubbard