Re: [PATCH v3 03/10] drm/xe/vsec: Protect against missing config

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Subsystem guidelines violation: The patch uses the absence of `soc_remapper.set_telem_region` as a proxy check to prevent execution on Virtual Functions (VF) instead of an explicit `IS_SRIOV_VF()` guard.

Pre-existing issues:
- [High] The `xe_pmt_cb` struct is missing the `.read_reg` and `.write_reg` callbacks required for safe PMT Crashlog control register access on discrete GPUs.
--

--- Patch [3]: [PATCH v3 03/10] drm/xe/vsec: Protect against missing config ---
commit 6d90522f74020b6e17f1f98588ed1740977adfb4
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>

drm/xe/vsec: Protect against missing config

This commit prevents PMT registration and telemetry read access when the
soc-remapper callbacks are not configured, specifically targeting Virtual
Function (VF) init paths.

> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
[ ... ]
> @@ -210,6 +210,8 @@ void xe_vsec_init(struct xe_device *xe)
>  
>  	switch (platform) {
>  	case XE_VSEC_BMG:
> +		if (!xe->soc_remapper.set_telem_region)
> +			return;

[Severity: Medium]
The commit message mentions that some init paths (VF) will not provide
soc-remapper callbacks. Is it safe to use this feature presence as a proxy
check to prevent execution on Virtual Functions?

According to the SR-IOV guidelines for the xe driver, VFs must be explicitly
blocked from PF-only resource registration using an IS_SRIOV_VF() guard to
prevent hardware failures and MMIO timeouts.

Should this code explicitly check for the VF context instead of relying on
the absence of the set_telem_region callback?

>  		info->priv_data = &xe_pmt_cb;

[Severity: High]
This is a pre-existing issue, but does assigning xe_pmt_cb here expose us to
unprotected MMIO accesses during suspend?

Since BMG exposes VSEC_CAP_CRASHLOG in xe_vsec_info, the crashlog driver
will probe and create sysfs entries. However, the xe_pmt_cb structure lacks
the .read_reg and .write_reg callbacks.

When userspace accesses crashlog sysfs attributes, the driver will fall back
to using direct readl()/writel() on entry->disc_table.

If the GPU is in D3cold, will these direct accesses bypass runtime PM checks
and attempt to read or write powered-off MMIO, potentially triggering MCE or
AER kernel panics?

>  		break;
>  	default:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824162317.2450380-12-michael.j.ruhl@intel.com?part=3