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

>-----Original Message-----
>From: [email protected] <[email protected]>
>Sent: Wednesday, August 12, 2026 3:49 PM
>To: Ruhl, Michael J <[email protected]>
>Cc: [email protected]
>Subject: Re: [PATCH v2 06/10] drm/xe/vsec: Support Crescent Island PMT
>
>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 <[email protected]>
>
>    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?

This is a preliminary setup patch.  Implementation usage happens in the follow up patches.

M

>[ ... ]
>
>>  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-
>[email protected]?part=6