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

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

On Tue, Oct 13, 2026 at 02:59:42AM +0000, sashiko-bot@kernel.org wrote:
> 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.
> 

I think we assume aside from an unexpected file close (segfault on user
app, ctrl-c), a user shall be well behavied (it is) and at worst it
spams dmesgs on debug levels.

> [ ... ]
> > 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?
> 

This is right, would have a reasonable timeout even though eventually a
GT reset pops us out here if the GuC stops responding.

Matt

> 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