Re: [PATCH v2 06/10] drm/xe/vsec: Support Crescent Island PMT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The Crescent Island (CRI) PMT feature is completely non-functional and unreachable due to multiple missing integration steps across the VSEC initialization and decoding paths.
--

--- Patch [6]: [PATCH v2 06/10] drm/xe/vsec: Support Crescent Island PMT ---
commit 0135683ab7b7649c2500e35c35f765f517103d98
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>

    drm/xe/vsec: Support Crescent Island PMT
    
    Crescent Island (CRI) supports PMT telemetry and crashlog.
    
    Add Crescent Island (CRI) discovery structure (DVSEC)
    information to allow for Xe registration.

> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 8abe11e6312f0..6345b0b4b26b8 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -21,8 +21,19 @@
>  
>  #include "regs/xe_pmt.h"
>  
> -/* PMT GUID value for BMG devices.  NOTE: this is NOT a PCI id */
> +/* PMT GUID value for BMG and CRI devices.  NOTE: this is NOT a PCI id */
>  #define BMG_DEVICE_ID 0xE2F8
> +#define CRI_DEVICE_ID 0xE2FA

[Severity: Medium]
Does CRI_DEVICE_ID need to be added to xe_guid_decode()?

Currently xe_guid_decode() unconditionally checks for BMG_DEVICE_ID:

drivers/gpu/drm/xe/xe_vsec.c:xe_guid_decode() {
	...
	if (device_id != BMG_DEVICE_ID)
		return -ENODEV;
	...
}

Will this reject CRI devices when they try to decode their GUID?

[ ... ]

>  enum xe_vsec {
>  	XE_VSEC_UNKNOWN = 0,
>  	XE_VSEC_BMG,
> +	XE_VSEC_CRI,
>  };
>  
>  static struct intel_vsec_platform_info xe_vsec_info[] = {
> @@ -60,6 +98,10 @@ static struct intel_vsec_platform_info xe_vsec_info[] = {
>  		.caps = VSEC_CAP_TELEMETRY | VSEC_CAP_CRASHLOG,
>  		.headers = bmg_capabilities,
>  	},
> +	[XE_VSEC_CRI] = {
> +		.caps = VSEC_CAP_TELEMETRY | VSEC_CAP_CRASHLOG,
> +		.headers = cri_capabilities,
> +	},
>  	{ }
>  };

[Severity: Medium]
Are there missing updates to get_platform_info() and vsec_platforms[] to
actually return XE_VSEC_CRI?

Currently get_platform_info() returns XE_VSEC_UNKNOWN for any platform
greater than XE_BATTLEMAGE:

drivers/gpu/drm/xe/xe_vsec.c:get_platform_info() {
	if (xe->info.platform > XE_BATTLEMAGE)
		return XE_VSEC_UNKNOWN;

	return vsec_platforms[xe->info.platform];
}

Additionally, does xe_vsec_init() need a case for XE_VSEC_CRI to set up
the PMT callbacks?

drivers/gpu/drm/xe/xe_vsec.c:xe_vsec_init() {
	...
	switch (platform) {
	case XE_VSEC_BMG:
		if (!xe->soc_remapper.set_telem_region)
			return;
		info->priv_data = &xe_pmt_cb;
		break;
	default:
		break;
	}
	...
}

Without these additions, it appears the CRI platform will not be fully
detected and the PMT feature will remain unreachable.

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