Re: [PATCH 1/6] drm/nouveau/gsp: vendor the RUSD header

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] 32-bit platform ABI mismatch in shared telemetry buffer layout due to missing struct padding, leading to memory corruption.
- [Medium] RUSD telemetry structures use native integer types, masking endianness mismatches on big-endian architectures.
--

--- Patch [1]: [PATCH 1/6] drm/nouveau/gsp: vendor the RUSD header ---
commit c1a3b8ad153588da7ba08ef117defa77f7ba5f2d
Author: Mohamed Ahmed <mohamedahmedegypt2001@gmail.com>

drm/nouveau/gsp: vendor the RUSD header

RM User Shared Data (RUSD) is GSP-RM's telemetry channel: the kernel
hands the firmware a buffer and the firmware fills it with telemetry
data (temperatures, power draw, power limits, clocks, utilizations,
etc) on its own timer. This commit vendors the ABI needed to use it
from the 570.144 tag of NVIDIA's open-gpu-kernel-modules.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/rusd.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/rusd.h
> new file mode 100644
> index 0000000000000..9f1099a064301
> --- /dev/null
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/rusd.h
[ ... ]
> +typedef enum RUSD_TEMPERATURE_TYPE {
> +   RUSD_TEMPERATURE_TYPE_GPU,
> +   RUSD_TEMPERATURE_TYPE_HBM,
> +   RUSD_TEMPERATURE_TYPE_MAX
> +} RUSD_TEMPERATURE_TYPE;
> +
> +typedef struct RUSD_TEMPERATURE {
> +   volatile NvU64 lastModifiedTimestamp;
> +   NvTemp temperature;
> +} RUSD_TEMPERATURE;

[Severity: High]
Does this struct lack explicit padding, causing an ABI mismatch on 32-bit
platforms?

On 32-bit platforms (like x86-32), NvU64 has a 4-byte natural alignment,
making sizeof(RUSD_TEMPERATURE) exactly 12 bytes. On 64-bit systems and the
64-bit RISC-V GSP firmware, it requires padding and is 16 bytes.

[ ... ]
> +typedef struct NV00DE_SHARED_DATA {
> +   NV_DECLARE_ALIGNED(RUSD_BAR1_MEMORY_INFO bar1MemoryInfo, 8);

[Severity: Medium]
Will these fields be read incorrectly on big-endian architectures?

The shared data structures define fields with native integer types (NvU32,
NvU64) instead of explicit little-endian types (__le32, __le64). On a
big-endian host, the natively-read values will be byte-swapped versions of
the little-endian data written by the RISC-V GSP firmware.

[ ... ]
> +   // POLL_POWER
> +   // Module Limit is not supported on Ampere/Hopper
> +   NV_DECLARE_ALIGNED(RUSD_POWER_LIMITS powerLimitGpu, 8);
> +
> +   // POLL_THERMAL
> +   NV_DECLARE_ALIGNED(RUSD_TEMPERATURE temperatures[RUSD_TEMPERATURE_TYPE_MAX], 8);

[Severity: High]
Will the missing padding in RUSD_TEMPERATURE cause memory corruption here?

Because temperatures is declared as an array of 3 elements, the array occupies
36 bytes on 32-bit versus 48 bytes on 64-bit. This difference in array stride
shifts all subsequent fields by 8 bytes.

The kernel will allocate a smaller buffer than the firmware expects, causing
the 64-bit firmware to write past the end of the allocation when writing
telemetry data.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714211429.238355-1-mohamedahmedegypt2001@gmail.com?part=1