Re: [PATCH v1] drm/xe/sysctrl: Fix byte-order reversal in mailbox frame transport

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe/sysctrl: transport fix breaks host-endian payload parsing on BE systems
--

commit 017fa1e0d0cfc7a44d6ea80c21fb3e61c82f81e1
Author: Anoop Vijay <anoop.c.vijay@intel.com>
drm/xe/sysctrl: Fix byte-order reversal in mailbox frame transport

This commit fixes byte-swapping in the mailbox frame transport on
big-endian systems by using get_unaligned_le32() and put_unaligned_le32().
This preserves the little-endian wire format instead of treating the byte
stream as host-endian u32 values.

Link: https://patchwork.freedesktop.org/patch/751309/?series=172513&rev=5

> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index 72baf1aa4b3ad..d5a34a24a0d60 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c

[ ... ]

> @@ -84,16 +85,16 @@ static int sysctrl_read_frame(struct xe_sysctrl *sc, void *frame,
>  		SYSCTRL_MB_DATA0, SYSCTRL_MB_DATA1, SYSCTRL_MB_DATA2, SYSCTRL_MB_DATA3
>  	};
>  	struct xe_device *xe = sc_to_xe(sc);
> -	u32 val[XE_SYSCTRL_MB_FRAME_SIZE / sizeof(u32)] = {0};
> +	u8 buf[XE_SYSCTRL_MB_FRAME_SIZE] = {};
>  	u32 dw = DIV_ROUND_UP(len, sizeof(u32));
>  	u32 i;
>  
>  	xe_assert(xe, len > 0 && len <= XE_SYSCTRL_MB_FRAME_SIZE);
>  
>  	for (i = 0; i < dw; i++)
> -		val[i] = xe_mmio_read32(sc->mmio, regs[i]);
> +		put_unaligned_le32(xe_mmio_read32(sc->mmio, regs[i]), buf + i * sizeof(u32));

[Severity: High]
Does this correctly enforce a strict little-endian byte stream in the
buffer, but inadvertently break host-endian payload parsing on big-endian
systems?

While this fixes the transport byte-order, application payload structures
like xe_sysctrl_app_status_resp and xe_ras_get_counter_response define
their fields as native host-endian u32 and u64. Because they are natively
accessed by callers, they will read byte-swapped, corrupted data on
big-endian architectures.

For example, in drivers/gpu/drm/xe/xe_sysctrl.c:xe_sysctrl_check_app_status():
    flags = resp.flags;

And in drivers/gpu/drm/xe/xe_ras.c:get_counter():
    *value = response.value;

Prior to this patch, readl()/writel() accidentally byte-swapped the entire
payload word-by-word on big-endian systems, which masked the fact that
payload struct fields weren't using __le32/__le64.

With this transport fix, will all sysctrl payload structs need to be updated
to use __le32/__le64 with explicit endianness conversions to prevent sysctrl
status checks and RAS counters from failing or reporting incorrect health
data?

>  
> -	memcpy(frame, val, len);
> +	memcpy(frame, buf, len);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921113030.4075591-2-anoop.c.vijay@intel.com?part=1