Re: [PATCH v12 16/22] gpu: nova-core: add MCTP/NVDM protocol types for firmware communication

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Wire-format protocol headers are defined using native `u32` instead of explicit little-endian types, causing incorrect byte-order on big-endian architectures.
--

commit 790a1556a39d680db8fcdb2896578691a18e9d1f
Author: John Hubbard <jhubbard@xxxxxxxxxx>

gpu: nova-core: add MCTP/NVDM protocol types for firmware communication

This commit introduces the Management Component Transport Protocol (MCTP) and
NVIDIA Device Management (NVDM) wire-format types used for communication
between the kernel driver and GPU firmware processors.

> diff --git a/drivers/gpu/nova-core/mctp.rs b/drivers/gpu/nova-core/mctp.rs
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mctp.rs
[ ... ]
> +bitfield! {
> +    pub(crate) struct MctpHeader(u32), "MCTP transport header for NVIDIA firmware messages." {
> +        31:31 som as bool, "Start-of-message bit.";
> +        30:30 eom as bool, "End-of-message bit.";
> +        29:28 seq as u8, "Packet sequence number.";
> +        23:16 seid as u8, "Source endpoint ID.";
> +    }
> +}

[Severity: High]
Since MctpHeader is defined as a native u32 and embedded into FSP payload
structures, will this result in endianness corruption on big-endian
architectures?

If the memory representation of the struct is serialized directly using
as_bytes() during transmission, the hardware could receive the bytes in
completely reversed order on big-endian hosts. 

Should explicit little-endian types be used for these wire-format headers,
or should byte swapping be handled explicitly prior to serialization?

[ ... ]
> +bitfield! {
> +    pub(crate) struct NvdmHeader(u32), "NVIDIA Vendor-Defined Message header over MCTP." {
> +        31:24 nvdm_type as u8 ?=> NvdmType, "NVDM message type.";
> +        23:8 vendor_id as u16, "PCI vendor ID.";
> +        6:0 msg_type as u8, "MCTP vendor-defined message type.";
> +    }
> +}

[Severity: High]
Similarly, does defining NvdmHeader as a native u32 expose the NVDM messages
to the same endianness corruption on big-endian platforms when sent over the
wire?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602032111.224790-1-jhubbard@xxxxxxxxxx?part=16