Re: [PATCH v3 11/33] gpu: nova-core: add GMC transport receive path

[John Hubbard]

On 9/18/26 2:57 PM, Timur Tabi wrote:
> On Thu, 2026-09-17 at 18:06 -0700, John Hubbard wrote:
>>
>> +/// The check of [`QueueElementHeader::validate`] that a queue element header fails.
>> +#[derive(Debug, Clone, Copy)]
>> +pub(crate) enum QueueElementHeaderError {
>> +    /// The first word is not `"MCTP"`.
>> +    BadMagic,
>> +    /// The MCTP header carries a version other than the one that this driver uses.
>> +    BadMctpVersion,
>> +    /// The NVDM header names a vendor other than NVIDIA, or a message type other than
>> +    /// vendor-defined.
>> +    BadNvdmVendor,
>> +    /// The element length is shorter than the queue element header and the message together,
>> or
>> +    /// above the maximum element size.
>> +    BadLength,
>>  }
> 
> Can you add a comment somewhere explaining why we're adding a new error type, instead of just
> using the standard error codes?

Yes. v4 adds a sentence to the enum's doc comment: the receive path
logs the variant when it poisons the queue, so that the log names the
check that failed. An errno carries one code and cannot.

Although now that the IRQ series removes queue poisoning, I need to
reconsider this, since I expect IRQ to get merged before this series.

> 
>> -    /// Validates this header against the expected NVIDIA NVDM format and type.
>> -    pub(crate) fn validate(self, expected_type: NvdmType) -> bool {
>> +    pub(crate) fn has_nvidia_vendor(self) -> bool {
>>          u8::from(self.msg_type()) == MSG_TYPE_VENDOR_PCI
>>              && u16::from(self.vendor_id()) == Vendor::NVIDIA.as_raw()
>> +    }
> 
> When would we ever receive a non-Nvidia message?  Normally this check is done if we're receiving
> messages from an external source.

No, never, at least, not from a working GSP-RM.

It is basically checking for message corruption.

The FSP receive path makes the same check, and so does Open RM's
GspMsgQueueReceiveStatus() in 615.71.09.

v4 keeps it and adds the missing doc comment on has_nvidia_vendor().


thanks,
-- 
John Hubbard