Re: [PATCH v7 6/9] drm/xe/sysctrl: Replace FIELD_GET() with REG_FIELD_GET()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Type-punning byte streams to a u32 array using memcpy() causes byte order reversal on big-endian systems during MMIO access.
--

commit 62065782d283be78ecb64ef05ff6080ad6f9d489
Author: Mallesh Koujalagi <mallesh.koujalagi@intel.com>

drm/xe/sysctrl: Replace FIELD_GET() with REG_FIELD_GET()

This commit replaces the FIELD_GET() and FIELD_PREP() macros with 
REG_FIELD_GET() and REG_FIELD_PREP() when extracting or preparing fields
from SysCtrl and application headers. This better reflects that the masks
represent register-style bitfields and improves consistency.

> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index 50bbdab072de6..cdd929ebed4a6 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c

[ ... ]

> @@ -186,8 +186,8 @@ static int sysctrl_process_frame(struct xe_sysctrl *sc, void *out,
>  	}
>  
>  	ctrl_reg = xe_mmio_read32(sc->mmio, SYSCTRL_MB_CTRL);
> -	total_frames = FIELD_GET(SYSCTRL_FRAME_TOTAL_MASK, ctrl_reg);
> -	curr_frame = FIELD_GET(SYSCTRL_FRAME_CURRENT_MASK, ctrl_reg);
> +	total_frames = REG_FIELD_GET(SYSCTRL_FRAME_TOTAL_MASK, ctrl_reg);
> +	curr_frame = REG_FIELD_GET(SYSCTRL_FRAME_CURRENT_MASK, ctrl_reg);
>  
>  	sysctrl_read_frame(sc, out, frame_size);

[Severity: High]
This isn't a bug introduced by this patch, but this is a pre-existing issue:
Does this code cause byte order reversal on big-endian systems during MMIO
access?

Looking at the implementation of sysctrl_read_frame() called here, the u32
value read via readl() (which already byte-swapped the little-endian PCI
register to CPU endianness) is incorrectly type-punned back into a byte
stream:

drivers/gpu/drm/xe/xe_sysctrl_mailbox.c:sysctrl_read_frame() {
	...
	for (i = 0; i < dw; i++)
		val[i] = xe_mmio_read32(sc->mmio, regs[i]);

	memcpy(frame, val, len);
}

This reverses the bytes on big-endian systems. A similar issue occurs in
sysctrl_write_frame() where the little-endian byte stream is type-punned
into a u32 array and undergoes an incorrect double byte-swap via writel().

Sending or receiving mailbox frames on a big-endian host through these
functions could lead to complete failure of the sysctrl mailbox
communication, resulting in driver initialization errors or missing
hardware control features.

>  	sysctrl_clear_response(sc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907105313.2208003-11-mallesh.koujalagi@intel.com?part=6