Re: [PATCH v4 3/3] CI-ONLY:Defer VM teardown until exec queue cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] drm/xe: deferred VM teardown from xe_file_close() outlives
  xe_pci_remove()
--

commit 4fe47c5c24ce78241a708ea647a388784ed800ab
Author: Arvind Yadav <arvind.yadav@intel.com>

CI-ONLY:Defer VM teardown until exec queue cleanup

This commit tracks user exec queues and user-created bind queues on their
VM and defers VM teardown onto xe_destroy_wq until all tracked queues have
completed final destruction.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 1ef9e276bbcad..f380186798357 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -177,6 +177,7 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>  	struct xe_file *xef = file->driver_priv;
>  	struct xe_vm *vm;
>  	struct xe_exec_queue *q;
> +	bool close_deferred = false;
>  	unsigned long idx;
>  
>  	guard(xe_pm_runtime)(xe);
> @@ -194,11 +195,26 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>  	xa_for_each(&xef->exec_queue.xa, idx, q) {
>  		if (q->vm && q->hwe->hw_engine_group)
>  			xe_hw_engine_group_del_exec_queue(q->hwe->hw_engine_group, q);
> -		xe_exec_queue_kill(q);
> +		/*
> +		 * Killing a bind queue can force pending PT jobs to clear
> +		 * mappings while hardware exec queues are still running.
> +		 * Let bind jobs drain before their queue is destroyed.
> +		 */
> +		if (!(q->flags & EXEC_QUEUE_FLAG_VM))
> +			xe_exec_queue_kill(q);
>  		xe_exec_queue_put(q);
>  	}
> +
> +	/*
> +	 * Queue cleanup is asynchronous. Retain VM mappings until tracked
> +	 * queues complete final destruction.
> +	 */
>  	xa_for_each(&xef->vm.xa, idx, vm)
> -		xe_vm_close_and_put(vm);
> +		close_deferred |= xe_vm_close_and_put_deferred(vm);

[Severity: High]
Can deferred VM teardown from xe_file_close() outlive xe_pci_remove()?

In xe_file_close(), xe_exec_queue_kill() and xe_exec_queue_put() only
initiate asynchronous GuC cleanup on gt->ordered_wq, so
xe_vm_close_and_put_deferred() returns while the VM is still open with
pinned page-table BOs, userptr/SVM DMA mappings, and vm->q alive.
Teardown then chains across multiple work items:

1. gt->ordered_wq queues q->guc->destroy_async onto xe_destroy_wq, where
   __xe_exec_queue_free() -> xe_vm_remove_close_queue() queues
   vm->close.work onto xe_destroy_wq.
2. vm_close_work_func() runs xe_vm_close_and_put() ->
   xe_vm_kill_bind_queues(), which starts asynchronous cleanup of vm->q on
   gt->ordered_wq while vm->q->user_vm holds a VM reference.
3. When __xe_exec_queue_free(vm->q) later runs on xe_destroy_wq and drops
   vm->q->user_vm, xe_vm_free() queues vm->destroy_work onto
   xe_destroy_wq.

If the PCI device is unbound or hot-unplugged after the DRM file closes,
xe_pci_remove() runs xe_bo_pci_dev_remove_all() inside xe_device_remove()
before vm->close.work has run, and then flushes xe_destroy_wq only once:

drivers/gpu/drm/xe/xe_pci.c:xe_pci_remove() {
	...
	xe_device_remove(xe);

	/*
	 * Preserve remove-time flush after moving destroy work to module
	 * lifetime.
	 */
	xe_destroy_wq_flush();
	xe_pm_fini(xe);
}

A single flush_workqueue(xe_destroy_wq) does not drain work still pending
on gt->ordered_wq or subsequently chain-queued onto xe_destroy_wq.
xe_pci_remove() therefore finishes xe_pm_fini() and returns to the PCI
core while the VM is still open, leaving page-table BOs pinned and DMA
mappings active across pci_dma_cleanup(), allowing MMU invalidation
notifiers on the unclosed VM to write to unmapped VRAM BARs in
xe_pt_zap_ptes_entry() after devres release unmaps tile->mem.vram->mapping,
and running runtime PM calls in guc_exec_queue_do_destroy() and
vm_destroy_work_func() after xe_pm_fini().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115101933.1163260-1-arvind.yadav@intel.com?part=3