Re: [PATCH v3 05/10] drm/xe/vsec: Support possible hotplug exit

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Aug 24, 2026 at 09:23:21AM -0700, Michael J. Ruhl wrote:
> DRM has an API that will verify that a device is valid in
> the hotplug context.
> 
> Verify device is valid before access in the VSEC callback
> API.
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vsec.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 8d99a3770b5a..8abe11e6312f 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -10,6 +10,8 @@
>  #include <linux/pci.h>
>  #include <linux/types.h>
>  
> +#include <drm/drm_drv.h>
> +
>  #include "xe_device.h"
>  #include "xe_device_types.h"
>  #include "xe_mmio.h"
> @@ -140,6 +142,11 @@ static int xe_guid_decode(u32 guid, int *index, u32 *offset)
>  	return 0;
>  }
>  
> +/*
> + * xe_pmt_telem_read is a callback API.  I.e this can be accessed external to
> + * XE driver (PMT driver scope).  Because of this, DRM hotplug needs to be
> + * verified (drm_dev_enter()).
> + */
>  int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offset,
>  		      u32 count)
>  {
> @@ -148,14 +155,20 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
>  	u32 mem_region;
>  	u32 offset;
> -	int ret;
> +	int ret = 0;
> +	int idx;
>  
> -	if (!xe->soc_remapper.set_telem_region)
> +	if (!drm_dev_enter(&xe->drm, &idx))
>  		return -ENODEV;
>  
> +	if (!xe->soc_remapper.set_telem_region) {

a pre-existing component layer violation, but it would be good
to change this patch once you fix that in the other one,
creating the xe_soc_remapper function entry.

> +		ret = -EINVAL;

probably worth to mention the change of the return errno in the commit msg.

with these 2 changes:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> +		goto dev_exit;
> +	}
> +
>  	ret = xe_guid_decode(guid, &mem_region, &offset);
>  	if (ret)
> -		return ret;
> +		goto dev_exit;
>  
>  	telem_addr += offset + user_offset;
>  
> @@ -165,11 +178,14 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  		xe_pm_runtime_get(xe);
>  		break;
>  	case TELEMETRY:
> -		if (!xe_pm_runtime_get_if_active(xe))
> -			return -ENODATA;
> +		if (!xe_pm_runtime_get_if_active(xe)) {
> +			ret = -ENODATA;
> +			goto dev_exit;
> +		}
>  		break;
>  	case WATCHER:
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto dev_exit;
>  	}
>  
>  	mutex_lock(&xe->pmt.lock);
> @@ -183,7 +199,10 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  
>  	xe_pm_runtime_put(xe);
>  
> -	return count;
> +dev_exit:
> +	drm_dev_exit(idx);
> +
> +	return ret == 0 ? count : ret;
>  }
>  
>  static struct pmt_callbacks xe_pmt_cb = {
> -- 
> 2.43.0
>