Re: [PATCH v3 09/33] gpu: nova-core: add GMC API message types

[John Hubbard]

On 9/18/26 2:45 PM, Timur Tabi wrote:
> On Thu, 2026-09-17 at 18:06 -0700, John Hubbard wrote:
>>
>> +#[repr(C)]
>> +pub(crate) struct QueueElementHeader {
>> +    magic: u32,
>> +    /// Length of the whole element: the queue element header, the message header and the
>> +    /// payload. Open RM calls it `mctpPayloadSize`.
>> +    element_len: u32,
>> +    mctp: MctpHeader,
>> +    nvdm: NvdmHeader,
>> +    /// Length of the message header and the payload, the queue element header excluded. Open
>> RM
>> +    /// calls it `nvdmPayloadSize`.
>> +    message_len: u32,
>> +    reserved: u32,
>> +}
>> +
>> +static_assert!(
>> +    core::mem::offset_of!(QueueElementHeader, magic)
>> +        == core::mem::offset_of!(r000_00::GSP_MSG_QUEUE_ELEMENT, mctpMagic)
>> +);
>> +static_assert!(
>> +    core::mem::offset_of!(QueueElementHeader, element_len)
>> +        == core::mem::offset_of!(r000_00::GSP_MSG_QUEUE_ELEMENT, mctpPayloadSize)
>> +);
>> +static_assert!(
>> +    core::mem::offset_of!(QueueElementHeader, mctp)
>> +        == core::mem::offset_of!(r000_00::GSP_MSG_QUEUE_ELEMENT, mctpHeader)
>> +);
>> +static_assert!(
>> +    core::mem::offset_of!(QueueElementHeader, nvdm)
>> +        == core::mem::offset_of!(r000_00::GSP_MSG_QUEUE_ELEMENT, nvdmHeader)
>> +);
> 
> Can you also static_assert! on the size of the struct?

Not directly, because it's got a union involved, but I've added in some
assorted static_asserts that attempt to cover as much as can be asserted
in this situation.

Open RM's layout:

  typedef struct {
      NvU32 mctpMagic;
      NvU32 mctpPayloadSize;
      NvU32 mctpHeader;
      NvU32 nvdmHeader;
      union {
          struct {
              GSP_MSG_QUEUE_ENCRYPTION_TAG encryptionTag;  /* 16 bytes */
              NvU32 nvdmPayloadSize;
              NvU32 reserved;
              NvU8  payload[];
          } withEncryption;
          struct {
              NvU32 nvdmPayloadSize;
              NvU32 reserved;
              NvU8  payload[];
          } noEncryption;
      };
  } GSP_MSG_QUEUE_ELEMENT;

QueueElementHeader uses the noEncryption layout.

Open RM has the same mismatch and defines the unencrypted size separately,
as GSP_MSG_QUEUE_ELEMENT_SIZE_NO_ENCRYPTION, which is the offset of the
union plus two words.

I've added these static asserts, for v4:

  * The size of QueueElementHeader equals the offset of the union plus
    the size of the noEncryption struct.

  * The offset of message_len equals the offset of the union plus the
    offset of nvdmPayloadSize within noEncryption.

  * The offset of reserved equals the offset of the union plus the
    offset of reserved within noEncryption.

> 
>> +    /// Returns the length of the payload that follows a message header of
>> +    /// Returns the length of the payload that follows a message header of
>> `message_header_len`
>> +    /// bytes.
>> +    fn payload_len(&self, message_header_len: usize) -> usize {
>> +        num::u32_as_usize(self.message_len).saturating_sub(message_header_len)
> 
> Is it possible for message_header_len to exceed self.message_len?  If so, then I think that
> warrants a comment.  If not, well, then that should have a comment, too.

Yes, it is possible.

So, in v4, I've made this return an Option<usize> in this patch, instead
of waiting until later patches to do that.

The v4 doc comment on payload_len() now says that None means the message
length in the queue element header is shorter than the message header.


thanks,
-- 
John Hubbard