Re: [PATCH 16/27] gpu: nova-core: add GMC transport receive path

[Zhi Wang]

On Tue, 18 Aug 2026 20:52:09 -0700
John Hubbard <jhubbard@xxxxxxxxxx> wrote:

> GSP-RM posts GMC and RPC messages on the same message queue. Both use
> the same MCTP and NVDM transport headers, but RPC messages continue
> with rpc_message_header_v and GMC messages continue with GmcApiHeader.
> Existing RPC receive code cannot parse the GMC layout.
> 

snip


> +    fn wait_for_gmc_msg(&self, timeout: Delta) ->
> Result<GmcMessage<'_>> {
> +        if self.poisoned.get() {
> +            return Err(EIO);
> +        }
> +
> +        let (slice_1, slice_2) = read_poll_timeout(
> +            || Ok(self.gsp_mem.driver_read_area()),
> +            |driver_area| !driver_area.0.is_empty(),
> +            Delta::from_millis(1),
> +            timeout,
> +        )
> +        .map(|(slice_1, slice_2)| (slice_1.as_flattened(),
> slice_2.as_flattened()))?; +
> +        let Some((header, slice_1)) =
> GspGmcMsgElement::from_bytes_prefix(slice_1) else {
> +            self.poisoned.set(true);
> +            return Err(EIO);
> +        };
> +
> +        // Checked before any length field is read, since a bad
> magic leaves them untrusted.
> +        if !header.has_valid_magic() {
> +            dev_err!(&self.dev, "GSP GMC: receive: bad MCTP
> magic\n");
> +            self.poisoned.set(true);
> +            return Err(EIO);
> +        }
> +

IMO, should we also check the MCTP version here?

> +        let payload_length = header.payload_length();
> +
> +        // Check that the driver read area is large enough for the
> message.
> +        if slice_1.len() + slice_2.len() < payload_length {
> +            self.poisoned.set(true);
> +            return Err(EIO);
> +        }
> +

Do we need more checks here? Here we only checked the nvdm payload
size, while in PATCH 17, the queue cursor is advanced by mctp payload
size. It would be better we can confirm the frame correctness here in
PATCH 16, e.g. check the mctp payload size with various constraints.
(size > min size, size < QUEUE MAX SIZE, size < available space,
etc...r000 firmware does check these) 


> +        // Cut the message slices down to the actual length of the
> message.
> +        let (slice_1, slice_2) = if slice_1.len() > payload_length {
> +            // PANIC: we checked above that `slice_1` is at least as
> long as `payload_length`.
> +            (slice_1.split_at(payload_length).0, &slice_2[0..0])
> +        } else {
> +            (
> +                slice_1,
> +                // PANIC: we checked above that `slice_1.len() +
> slice_2.len()` is at least as
> +                // large as `payload_length`.
> +                slice_2.split_at(payload_length - slice_1.len()).0,
> +            )
> +        };
> +
> +        Ok(GmcMessage {
> +            header,
> +            contents: (slice_1, slice_2),
> +        })
> +    }
>  }
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs
> b/drivers/gpu/nova-core/gsp/fw.rs index 56f255a3d49c..9e6b5ec6aadb
> 100644 --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -1055,11 +1055,21 @@ pub(crate) fn init(
>          })
>      }
>  
> +    /// Returns the length of the response payload (data after the
> [`GmcApiHeader`]).
> +    pub(crate) fn payload_length(&self) -> usize {
> +
> num::u32_as_usize(self.nvdm_payload_size).saturating_sub(size_of::<GmcApiHeader>())
> +    }
> +
>      /// Returns the total length of the message, transport and GMC
> headers included. pub(crate) fn length(&self) -> usize {
>          num::u32_as_usize(self.mctp_payload_size)
>      }
>  
> +    /// Returns `true` if the MCTP magic field contains the expected
> value.
> +    pub(crate) fn has_valid_magic(&self) -> bool {
> +        self.mctp_magic == MCTP_MAGIC
> +    }
> +
>      /// Returns the number of elements (i.e. memory pages) used by
> this message. pub(crate) fn element_count(&self) -> u32 {
>          self.mctp_payload_size