Re: [PATCH 05/16] drm/xe/multi_queue: Handle invalid exec queue property setting

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Oct 31, 2025 at 11:29:25AM -0700, Niranjana Vishwanathapura wrote:
> Only MULTI_QUEUE_PRIORITY property is valid for secondary queues of a
> multi queue group. MULTI_QUEUE_PRIORITY only applies to multi queue group
> queues. Detect invalid user queue property setting and return error.
> 
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_exec_queue.c | 66 ++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 0da256428916..78b3a0e2ddd3 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -61,7 +61,7 @@ enum xe_exec_queue_sched_prop {
>  };
>  
>  static int exec_queue_user_extensions(struct xe_device *xe, struct xe_exec_queue *q,
> -				      u64 extensions, int ext_number);
> +				      u64 extensions);
>  
>  static void xe_exec_queue_group_cleanup(struct xe_exec_queue *q)
>  {
> @@ -206,7 +206,7 @@ static struct xe_exec_queue *__xe_exec_queue_alloc(struct xe_device *xe,
>  		 * may set q->usm, must come before xe_lrc_create(),
>  		 * may overwrite q->sched_props, must come before q->ops->init()
>  		 */
> -		err = exec_queue_user_extensions(xe, q, extensions, 0);
> +		err = exec_queue_user_extensions(xe, q, extensions);
>  		if (err) {
>  			__xe_exec_queue_free(q);
>  			return ERR_PTR(err);
> @@ -747,9 +747,35 @@ static const xe_exec_queue_set_property_fn exec_queue_set_property_funcs[] = {
>  							exec_queue_set_multi_queue_priority,
>  };
>  
> +static int exec_queue_user_ext_check(struct xe_exec_queue *q, u64 properties)
> +{
> +	u64 secondary_queue_valid_props = BIT_ULL(DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP) |
> +				  BIT_ULL(DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY);
> +
> +	/*
> +	 * Only MULTI_QUEUE_PRIORITY property is valid for secondary queues of a
> +	 * multi-queue group.
> +	 */
> +	if (xe_exec_queue_is_multi_queue_secondary(q) &&
> +	    properties & ~secondary_queue_valid_props)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int exec_queue_user_ext_check_final(struct xe_exec_queue *q, u64 properties)
> +{
> +	/* MULTI_QUEUE_PRIORITY only applies to multi-queue group queues */
> +	if ((properties & BIT_ULL(DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY)) &&
> +	    !(properties & BIT_ULL(DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP)))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static int exec_queue_user_ext_set_property(struct xe_device *xe,
>  					    struct xe_exec_queue *q,
> -					    u64 extension)
> +					    u64 extension, u64 *properties)
>  {
>  	u64 __user *address = u64_to_user_ptr(extension);
>  	struct drm_xe_ext_set_property ext;
> @@ -774,20 +800,25 @@ static int exec_queue_user_ext_set_property(struct xe_device *xe,
>  	if (!exec_queue_set_property_funcs[idx])
>  		return -EINVAL;
>  
> +	*properties |= BIT_ULL(idx);
> +	err = exec_queue_user_ext_check(q, *properties);
> +	if (XE_IOCTL_DBG(xe, err))
> +		return err;
> +
>  	return exec_queue_set_property_funcs[idx](xe, q, ext.value);
>  }
>  
>  typedef int (*xe_exec_queue_user_extension_fn)(struct xe_device *xe,
>  					       struct xe_exec_queue *q,
> -					       u64 extension);
> +					       u64 extension, u64 *properties);
>  
>  static const xe_exec_queue_user_extension_fn exec_queue_user_extension_funcs[] = {
>  	[DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY] = exec_queue_user_ext_set_property,
>  };
>  
>  #define MAX_USER_EXTENSIONS	16
> -static int exec_queue_user_extensions(struct xe_device *xe, struct xe_exec_queue *q,
> -				      u64 extensions, int ext_number)
> +static int __exec_queue_user_extensions(struct xe_device *xe, struct xe_exec_queue *q,
> +					u64 extensions, int ext_number, u64 *properties)
>  {
>  	u64 __user *address = u64_to_user_ptr(extensions);
>  	struct drm_xe_user_extension ext;
> @@ -808,13 +839,30 @@ static int exec_queue_user_extensions(struct xe_device *xe, struct xe_exec_queue
>  
>  	idx = array_index_nospec(ext.name,
>  				 ARRAY_SIZE(exec_queue_user_extension_funcs));
> -	err = exec_queue_user_extension_funcs[idx](xe, q, extensions);
> +	err = exec_queue_user_extension_funcs[idx](xe, q, extensions, properties);
>  	if (XE_IOCTL_DBG(xe, err))
>  		return err;
>  
>  	if (ext.next_extension)
> -		return exec_queue_user_extensions(xe, q, ext.next_extension,
> -						  ++ext_number);
> +		return __exec_queue_user_extensions(xe, q, ext.next_extension,
> +						    ++ext_number, properties);
> +
> +	return 0;
> +}
> +
> +static int exec_queue_user_extensions(struct xe_device *xe, struct xe_exec_queue *q,
> +				      u64 extensions)
> +{
> +	u64 properties = 0;
> +	int err;
> +
> +	err = __exec_queue_user_extensions(xe, q, extensions, 0, &properties);
> +	if (XE_IOCTL_DBG(xe, err))
> +		return err;
> +
> +	err = exec_queue_user_ext_check_final(q, properties);
> +	if (XE_IOCTL_DBG(xe, err))
> +		return err;
>  
>  	if (xe_exec_queue_is_multi_queue_primary(q)) {
>  		err = xe_exec_queue_group_init(xe, q);
> -- 
> 2.43.0
>