Re: [PATCH 3/3] drm/xe: Synchronously wait for queue kill on FD close

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/xe: Infinite wait in guc_exec_queue_kill_sync() if GuC hangs
- [High] xe: missing queue kill synchronization in teardown paths
--

commit 85f52ae759a601199a902ba124e22498f6e78a14
Author: Daniel Charles <daniel.charles@intel.com>

drm/xe: Synchronously wait for queue kill on FD close

The early KILL message disables GuC scheduling outside the serialized
TDR path, but xe_file_close() does not wait for it to complete before
dropping queue references. Split FD close teardown into two passes:
first kill every queue, then wait using xe_exec_queue_kill_sync().

> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 7ade915c59a85..af5d9e8f0b16e 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
[ ... ]
> @@ -1623,6 +1623,19 @@ void xe_exec_queue_kill(struct xe_exec_queue *q)
> +/**
> + * xe_exec_queue_kill_sync() - Wait for queue scheduling to stop
> + * @q: The exec queue
> + *
> + * This function waits for the backend to finish stopping the queue after
> + * xe_exec_queue_kill(). It is safe to call multiple times.
> + */
> +void xe_exec_queue_kill_sync(struct xe_exec_queue *q)
> +{
> +	if (q->ops->kill_sync)
> +		q->ops->kill_sync(q);
> +}

[Severity: High]
Does this code leave other teardown paths vulnerable to CAT errors?

While xe_file_close() was updated to use this new sync function, it appears
xe_exec_queue_destroy_ioctl() and xe_vm_close_and_put() both still call
xe_exec_queue_kill() followed immediately by xe_exec_queue_put() without
waiting.

If userspace destroys a user queue and then destroys its VM,
xe_vm_close_and_put() synchronously destroys the VM's page tables while GuC
is still actively scheduling the queue. Similarly, for the internal kernel
bind queue (vm->q), xe_vm_close_and_put() kills it and proceeds to destroy
the page tables. This results in GuC scheduling contexts with torn-down
page tables, causing IOMMU CAT errors and unrecoverable engine resets.

[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 609af66b343d5..36c8c3766f5fa 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
[ ... ]
> +static void guc_exec_queue_kill_sync(struct xe_exec_queue *q)
> +{
[ ... ]
> +	wait_event(guc->ct.wq,
> +		   (!exec_queue_enabled(primary) &&
> +		    !exec_queue_pending_disable(primary) &&
> +		    !exec_queue_pending_enable(primary)) ||
> +		   xe_guc_read_stopped(guc) ||
> +		   vf_recovery(guc) ||
> +		   exec_queue_wedged(primary));
> +}

[Severity: High]
Can this wait hang forever if the GuC firmware silently drops the H2G DISABLE
message or hangs without triggering a global watchdog wedge?

If called on an idle queue (no pending jobs), the Time-out Driven Recovery
(TDR) will not fire. xe_sched_tdr_queue_imm() calls
drm_sched_tdr_queue_imm(), which only schedules the timeout worker if there
are pending jobs. Even if the worker runs, drm_sched_job_timedout() exits
early if the pending list is empty.

Since this uses an unbounded wait_event() without a timeout, it could cause
the close() syscall to hang forever in uninterruptible sleep (D state),
leading to an unkillable process. Other functions like enable_scheduling()
and disable_scheduling_deregister() use wait_event_timeout() and manually
trigger xe_gt_reset_async() on timeout to prevent this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013023442.206915-1-daniel.charles@intel.com?part=3