Re: [PATCH v2 04/16] drm/xe/multi_queue: Add multi queue priority property

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Tue, Nov 04, 2025 at 02:58:00PM -0800, Matthew Brost wrote:
>On Tue, Nov 04, 2025 at 10:51:38AM -0800, Niranjana Vishwanathapura wrote:
>> Add support for queues of a multi queue group to set
>> their priority within the queue group by adding property
>> DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY.
>> This is the only other property supported by secondary
>> queues of a multi queue group, other than
>> DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE.
>>
>> v2: Add kernel doc for enum xe_multi_queue_priority,
>>     Add assert for priority values, fix includes and
>>     declarations (Matt Brost)
>>
>> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_exec_queue.c       | 17 +++++++++++++-
>>  drivers/gpu/drm/xe/xe_exec_queue_types.h | 16 +++++++++++++
>>  drivers/gpu/drm/xe/xe_guc_submit.c       |  1 +
>>  drivers/gpu/drm/xe/xe_lrc.c              | 29 ++++++++++++++++++++++++
>>  drivers/gpu/drm/xe/xe_lrc.h              |  3 +++
>>  include/uapi/drm/xe_drm.h                |  3 +++
>>  6 files changed, 68 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
>> index 3a701fa477d7..7e9960ede0be 100644
>> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
>> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
>> @@ -179,6 +179,7 @@ static struct xe_exec_queue *__xe_exec_queue_alloc(struct xe_device *xe,
>>  	INIT_LIST_HEAD(&q->multi_gt_link);
>>  	INIT_LIST_HEAD(&q->hw_engine_group_link);
>>  	INIT_LIST_HEAD(&q->pxp.link);
>> +	q->multi_queue.priority = XE_MULTI_QUEUE_PRIORITY_NORMAL;
>>
>>  	q->sched_props.timeslice_us = hwe->eclass->sched_props.timeslice_us;
>>  	q->sched_props.preempt_timeout_us =
>> @@ -737,6 +738,17 @@ static int exec_queue_set_multi_group(struct xe_device *xe, struct xe_exec_queue
>>  	return xe_exec_queue_group_validate(xe, q, value);
>>  }
>>
>> +static int exec_queue_set_multi_queue_priority(struct xe_device *xe, struct xe_exec_queue *q,
>> +					       u64 value)
>> +{
>> +	if (XE_IOCTL_DBG(xe, value > XE_MULTI_QUEUE_PRIORITY_HIGH))
>> +		return -EINVAL;
>> +
>> +	q->multi_queue.priority = value;
>> +
>> +	return 0;
>> +}
>> +
>>  typedef int (*xe_exec_queue_set_property_fn)(struct xe_device *xe,
>>  					     struct xe_exec_queue *q,
>>  					     u64 value);
>> @@ -746,6 +758,8 @@ static const xe_exec_queue_set_property_fn exec_queue_set_property_funcs[] = {
>>  	[DRM_XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE] = exec_queue_set_timeslice,
>>  	[DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE] = exec_queue_set_pxp_type,
>>  	[DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP] = exec_queue_set_multi_group,
>> +	[DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY] =
>> +							exec_queue_set_multi_queue_priority,
>>  };
>>
>>  static int exec_queue_user_ext_set_property(struct xe_device *xe,
>> @@ -767,7 +781,8 @@ static int exec_queue_user_ext_set_property(struct xe_device *xe,
>>  	    XE_IOCTL_DBG(xe, ext.property != DRM_XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY &&
>>  			 ext.property != DRM_XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE &&
>>  			 ext.property != DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE &&
>> -			 ext.property != DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP))
>> +			 ext.property != DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP &&
>> +			 ext.property != DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY))
>>  		return -EINVAL;
>>
>>  	idx = array_index_nospec(ext.property, ARRAY_SIZE(exec_queue_set_property_funcs));
>> diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h b/drivers/gpu/drm/xe/xe_exec_queue_types.h
>> index b9da51ab7eaf..445ae4979c0c 100644
>> --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h
>> +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h
>> @@ -32,6 +32,20 @@ enum xe_exec_queue_priority {
>>  	XE_EXEC_QUEUE_PRIORITY_COUNT
>>  };
>>
>> +/**
>> + * enum xe_multi_queue_priority - Multi Queue priority values
>> + *
>> + * The priority values of the queues within the multi queue group.
>> + */
>> +enum xe_multi_queue_priority {
>> +	/** @XE_MULTI_QUEUE_PRIORITY_LOW: Priority low */
>> +	XE_MULTI_QUEUE_PRIORITY_LOW = 0,
>> +	/** @XE_MULTI_QUEUE_PRIORITY_NORMAL: Priority normal */
>> +	XE_MULTI_QUEUE_PRIORITY_NORMAL,
>> +	/** @XE_MULTI_QUEUE_PRIORITY_HIGH: Priority high */
>> +	XE_MULTI_QUEUE_PRIORITY_HIGH,
>> +};
>> +
>>  /**
>>   * struct xe_exec_queue_group - Execution multi queue group
>>   *
>> @@ -131,6 +145,8 @@ struct xe_exec_queue {
>>  	struct {
>>  		/** @multi_queue.group: Queue group information */
>>  		struct xe_exec_queue_group *group;
>> +		/** @multi_queue.priority: Queue priority within the multi-queue group */
>> +		enum xe_multi_queue_priority priority;
>>  		/** @multi_queue.pos: Position of queue within the multi-queue group */
>>  		u8 pos;
>>  		/** @multi_queue.valid: Queue belongs to a multi queue group */
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index 2e5106687a6e..b01d852ae526 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> @@ -634,6 +634,7 @@ static void xe_guc_exec_queue_group_cgp_sync(struct xe_guc *guc,
>>  		return;
>>  	}
>>
>> +	xe_lrc_set_multi_queue_priority(q->lrc[0], q->multi_queue.priority);
>>  	xe_guc_exec_queue_group_cgp_update(xe, q);
>>
>>  	WRITE_ONCE(group->sync_pending, true);
>> diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
>> index b5083c99dd50..56836a5546d8 100644
>> --- a/drivers/gpu/drm/xe/xe_lrc.c
>> +++ b/drivers/gpu/drm/xe/xe_lrc.c
>> @@ -44,6 +44,11 @@
>>  #define LRC_INDIRECT_CTX_BO_SIZE		SZ_4K
>>  #define LRC_INDIRECT_RING_STATE_SIZE		SZ_4K
>>
>> +#define LRC_PRIORITY				GENMASK_ULL(10, 9)
>> +#define LRC_PRIORITY_LOW			0
>> +#define LRC_PRIORITY_NORMAL			1
>> +#define LRC_PRIORITY_HIGH			2
>> +
>>  /*
>>   * Layout of the LRC and associated data allocated as
>>   * lrc->bo:
>> @@ -1386,6 +1391,30 @@ setup_indirect_ctx(struct xe_lrc *lrc, struct xe_hw_engine *hwe)
>>  	return 0;
>>  }
>>
>> +static u8 xe_multi_queue_prio_to_lrc(struct xe_lrc *lrc, enum xe_multi_queue_priority priority)
>> +{
>> +	struct xe_device *xe = gt_to_xe(lrc->gt);
>> +
>> +	xe_assert(xe, (priority >= XE_MULTI_QUEUE_PRIORITY_LOW &&
>> +		       priority <= XE_MULTI_QUEUE_PRIORITY_HIGH));
>> +
>> +	/* xe_multi_queue_priority is directly mapped to LRC priority values */
>> +	return priority;
>> +}
>> +
>> +/**
>> + * xe_lrc_set_multi_queue_priority() - Set multi queue priority in LRC
>> + * @lrc: Logical Ring Context
>> + * @priority: Multi queue priority of the exec queue
>> + *
>> + * Convert @priority to LRC multi queue priority and update the @lrc descriptor
>> + */
>> +void xe_lrc_set_multi_queue_priority(struct xe_lrc *lrc, enum xe_multi_queue_priority priority)
>> +{
>> +	lrc->desc &= ~LRC_PRIORITY;
>> +	lrc->desc |= FIELD_PREP(LRC_PRIORITY, xe_multi_queue_prio_to_lrc(lrc, priority));
>> +}
>> +
>>  static int xe_lrc_init(struct xe_lrc *lrc, struct xe_hw_engine *hwe,
>>  		       struct xe_vm *vm, u32 ring_size, u16 msix_vec,
>>  		       u32 init_flags)
>> diff --git a/drivers/gpu/drm/xe/xe_lrc.h b/drivers/gpu/drm/xe/xe_lrc.h
>> index 2fb628da5c43..569ca380676e 100644
>> --- a/drivers/gpu/drm/xe/xe_lrc.h
>> +++ b/drivers/gpu/drm/xe/xe_lrc.h
>> @@ -13,6 +13,7 @@ struct drm_printer;
>>  struct xe_bb;
>>  struct xe_device;
>>  struct xe_exec_queue;
>> +enum xe_multi_queue_priority;
>>  enum xe_engine_class;
>>  struct xe_gt;
>>  struct xe_hw_engine;
>> @@ -133,6 +134,8 @@ void xe_lrc_dump_default(struct drm_printer *p,
>>
>>  u32 *xe_lrc_emit_hwe_state_instructions(struct xe_exec_queue *q, u32 *cs);
>>
>> +void xe_lrc_set_multi_queue_priority(struct xe_lrc *lrc, enum xe_multi_queue_priority priority);
>> +
>>  struct xe_lrc_snapshot *xe_lrc_snapshot_capture(struct xe_lrc *lrc);
>>  void xe_lrc_snapshot_capture_delayed(struct xe_lrc_snapshot *snapshot);
>>  void xe_lrc_snapshot_print(struct xe_lrc_snapshot *snapshot, struct drm_printer *p);
>> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>> index 4de21e0a4fcf..6734ef7fada1 100644
>> --- a/include/uapi/drm/xe_drm.h
>> +++ b/include/uapi/drm/xe_drm.h
>> @@ -1260,6 +1260,8 @@ struct drm_xe_vm_bind {
>>   *    queue's exec_queue_id is specified in the lower 32 bits of the 'value' field.
>>   *    All the other non-relevant bits of extension's 'value' field while adding the
>>   *    primary or the secondary queues of the group must be set to 0.
>> + *  - %DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY - Set the queue
>> + *    priority within the multi-queue group.
>
>'Current valid priority values are 0–2 (default is 1), with higher
>values indicating higher priority.' ?
>

Ok, will add.

>Do we have any concerns about exposing XE_MULTI_QUEUE_PRIORITY_HIGH to
>non-elevated privileged users? For example, could this allow a user
>process to cause a denial of service by locking out others? This is why
>we restrict normal privileged users from setting
>DRM_XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY to the highest level.
>

No. Unlike the queue priority, in the multi-queue priority case,
the PRIORITY_HIGH is available to the user with normal privilege also.

>For DRM_XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY, we have the query
>DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY, which indicates the maximum
>queue priority level. Do we need a similar mechanism for multi-queue
>priority?
>

There was some discussion with compute team on this, but no decision
yet. If needed, we can add it later.

Niranjana

>Matt
>
>>   *
>>   * The example below shows how to use @drm_xe_exec_queue_create to create
>>   * a simple exec_queue (no parallel submission) of class
>> @@ -1302,6 +1304,7 @@ struct drm_xe_exec_queue_create {
>>  #define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE		2
>>  #define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP		3
>>  #define     DRM_XE_MULTI_GROUP_CREATE				(1ull << 63)
>> +#define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY	4
>>  	/** @extensions: Pointer to the first extension struct, if any */
>>  	__u64 extensions;
>>
>> --
>> 2.43.0
>>