Re: [PATCH v2] drm/xe: Add multi_queue_active_lrca debugfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless access to `q->lrc[0]` and `group->primary->lrc[0]` causes a NULL pointer dereference if a multi-queue group is actively being created.
--

commit 1227a012a132709dff00d20c63df261a7728bc7f
Author: Varun Gupta <varun.gupta@intel.com>

drm/xe: Add multi_queue_active_lrca debugfs

This commit adds a per-GT debugfs file to print the currently active queue ID
and LRCA of the exec queue occupying that slot in a running multi-queue
group. It resolves the active LRCA by matching the primary LRCA against each
queue's group and picking the queue at the reported active_id position.

> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
[ ... ]
> +u32 xe_guc_submit_active_multi_queue_lrca(struct xe_guc *guc,
> +					  struct xe_hw_engine *hwe,
> +					  u32 cur_lrca, u32 active_id)
> +{
[ ... ]
> +	xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) {
> +		struct xe_exec_queue_group *group = q->multi_queue.group;
> +
> +		if (!q->multi_queue.valid || !group || !group->primary)
> +			continue;
[ ... ]
> +		/*
> +		 * LRCAs are page-aligned (4K) addresses in GGTT; the low
> +		 * bits reported by RING_CURRENT_LRCA are not meaningful, so
> +		 * only compare bits [31:12].
> +		 */
> +		if ((xe_lrc_ggtt_addr(group->primary->lrc[0]) ^ cur_lrca) & GENMASK(31, 12))
> +			continue;
> +
> +		lrca = xe_lrc_ggtt_addr(q->lrc[0]);

[Severity: High]
Can this sequence cause a NULL pointer dereference if a multi-queue group is
actively being created?

If another process creates a multi-queue exec queue concurrently,
__xe_exec_queue_init() calls guc_exec_queue_init() which inserts the
new queue into guc->submission_state.exec_queue_lookup and then drops the
submission lock.

The execution then returns to __xe_exec_queue_init() to allocate the LRCs
and assign them to q->lrc[i]. If the debugfs read iterates over
exec_queue_lookup in this exact window, it can find the partially
initialized queue.

If the queue's pos happens to match the active_id read from the hardware
(e.g. both are 0), it looks like it will unconditionally call
xe_lrc_ggtt_addr() on group->primary->lrc[0] and q->lrc[0] before they
are allocated, resulting in a crash.

Does this require taking q->lrc_lookup_lock or checking for NULL before
accessing the LRCs here?

> +		break;
> +	}
> +
> +	return lrca;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907120223.990784-2-varun.gupta@intel.com?part=1