Re: [PATCH 2/2] drm/xe/cri: Expose device UUID through sysfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing SR-IOV VF guard for CRI_DEVICE_UUID MMIO read and sysfs registration.
- [High] MMIO read of CRI_DEVICE_UUID is performed before pcode uncore_init completion.
--

commit 23a0b7f2a1ab91a9391ba06e886cb50c9626ce7d
Author: Badal Nilawar <badal.nilawar@intel.com>

drm/xe/cri: Expose device UUID through sysfs

This commit exposes a read-only sysfs attribute to report the GPU's unique
hardware identifier.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 8583b2e9ecf4c..e8db8871990b2 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -801,6 +801,9 @@ int xe_device_probe_early(struct xe_device *xe)
>  	if (IS_SRIOV_VF(xe))
>  		vf_update_device_info(xe);
>  
> +	if (xe->info.platform == XE_CRESCENTISLAND)
> +		xe->device_uuid = xe_mmio_read64_2x32(xe_root_tile_mmio(xe), CRI_DEVICE_UUID);
> +

[Severity: High]
Is it safe to read this MMIO register from a Virtual Function (VF) context?

If this register is PF-only, accessing it from a VF might cause MMIO timeouts
or hardware errors. Should this be guarded with !IS_SRIOV_VF(xe)?

[Severity: High]
Does this MMIO read happen too early?

The comment right below this read states that "any MMIO or lmem access from
the driver will be blocked" until pcode uncore_init is done in
xe_pcode_probe_early(). If we read the register before this initialization
completes, will it result in a bus hang or reading invalid data?

>  	/*
>  	 * Check for pcode uncore_init status to confirm if the SoC
>  	 * initialization is complete. Until done, any MMIO or lmem access from
>  	 * the driver will be blocked
>  	 */
>  	err = xe_pcode_probe_early(xe);

[ ... ]

> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index a73e0e957cb0b..a4d326159dce3 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -285,5 +315,11 @@ int xe_device_sysfs_init(struct xe_device *xe)
>  			return ret;
>  	}
>  
> +	if (xe->info.platform == XE_CRESCENTISLAND) {
> +		ret = devm_device_add_group(dev, &device_uuid_attr_group);
> +		if (ret)
> +			return ret;
> +	}
> +

[Severity: High]
If the device UUID cannot be read on VFs, should we prevent registering the
sysfs attribute for VFs as well?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904160608.2004174-4-badal.nilawar@intel.com?part=2