Re: [PATCH v5 14/18] drm/xe/vsec: Crescent Island PMT callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Using memcpy_toio() and memcpy_fromio() for 32-bit MMIO control registers violates atomicity, lacks memory barriers, and breaks endianness on big-endian architectures.
--

commit dbad67bc0c83cc3b6a60d78b23589e00b666a0f0
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>

drm/xe/vsec: Crescent Island PMT callbacks

This commit introduces callbacks for Crescent Island PMT support to access
the discovery status and control areas. It adds the glue logic and registers
the functionality for the XE_CRESCENTISLAND platform.

> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 0438810337dd3..336c4b7d284bc 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
[ ... ]
> +static int xe_pmt_read_reg(struct device *dev, u32 guid, u32 *reg, u32 offset)
> +{
[ ... ]
> +	xe_pm_runtime_get(xe);
> +	mutex_lock(&xe->pmt.lock);
> +
> +	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> +
> +	memcpy_fromio(reg, disc_addr, sizeof(*reg));

[Severity: Medium]
Does using memcpy_fromio() here instead of readl() cause endianness issues
on big-endian platforms?

Since this is reading a 32-bit register, memcpy_fromio() omits endian
conversion and implicit memory barriers. It might also break atomicity or
cause PCIe target aborts if the underlying implementation falls back to 8-bit
byte accesses, which some hardware rejects for 32-bit registers.

> +
> +	mutex_unlock(&xe->pmt.lock);
> +	xe_pm_runtime_put(xe);
[ ... ]
> +static int xe_pmt_write_reg(struct device *dev, u32 guid, u32 reg, u32 offset)
> +{
[ ... ]
> +	xe_pm_runtime_get(xe);
> +	mutex_lock(&xe->pmt.lock);
> +
> +	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> +
> +	memcpy_toio(disc_addr, &reg, sizeof(reg));

[Severity: Medium]
Could memcpy_toio() write these bytes in CPU endianness instead of the
device's expected little-endian format? 

Using writel() here would properly handle endianness conversion, ensure 
I/O memory barriers, and guarantee a single atomic 32-bit write operation.

> +
> +	mutex_unlock(&xe->pmt.lock);
> +	xe_pm_runtime_put(xe);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903201002.1064889-20-michael.j.ruhl@intel.com?part=14