drm/xe/guc: Hold device ref until queue teardown completes

GuC exec queue destruction can run asynchronously. If the final device
put happens from a destroy worker, drmm cleanup can end up draining
the same workqueue and deadlock.

Hold a drm_device reference for the queue lifetime and drop it after
queue teardown completes. This keeps drmm cleanup from running while
async destroy work is still pending.

Move GuC destroy work to a module-lifetime Xe workqueue and flush it
on PCI remove so hot-unbind/rebind still waits for pending destroy work.

With queue-held device refs, guc_submit_sw_fini() cannot run with live
GuC IDs. Replace the fini wait with an assertion and remove the unused
fini_wq.

v2:
  - Rebase

v3:
  - Switch to queue-lifetime drm_dev_get()/drm_dev_put() model. (Matt)
  - Queue async teardown on system_dfl_wq instead of xe->destroy_wq. (Matt)
  - Drop separate deferred drm_dev_put worker.
  - Remove stale drain_workqueue(xe->destroy_wq) from guc_submit_sw_fini().

v4:
  - Replace the guc_submit_sw_fini() wait with an assertion and remove
    the now-unused fini_wq. (sashiko)

v5:
  - Move destroy work to a module-lifetime Xe workqueue instead of
    system_dfl_wq. (Matt)
  - Flush the module-lifetime destroy workqueue during PCI remove to
    preserve the old device-remove wait semantics.

v6:
  - Keep SVM pagemap destroy work on the per-device destroy_wq to avoid
    letting it outlive the xe_device/drm_device. (Sashiko)
  - Use WQ_MEM_RECLAIM for xe->destroy_wq because SVM pagemap destroy work
    can be queued from the reclaim path.

v7:
  - Drop the per-device xe->destroy_wq and use the module-level destroy WQ
    for SVM pagemap destroy as well. (Matt)
  - Rename xe_exec_queue_destroy_wq_*() helpers to xe_destroy_wq_*()
    helpers because the WQ is no longer exec-queue specific. (Matt)

v8:
  - Rebase.

v9:
  - Keep SVM pagemap destroy work on the per-device WQ_MEM_RECLAIM
    destroy_wq because it can be queued from reclaim and embeds
    the dev_pagemap used by devres teardown. (Sashiko)
  - Keep the module-level destroy WQ GuC-only and drop WQ_MEM_RECLAIM
    from it.
  - Update the module-WQ kdoc to document the GuC/SVM split.

v10:
  - Keep xe->destroy_wq per-cpu while adding WQ_MEM_RECLAIM to fix the
    workqueue allocation warning.

v11:
  - Drop the SVM pagemap destroy comment as it was revision-specific.
    (Thomas)

v12:
  - Rebase.

Fixes: 2d2be279f1ca ("drm/xe: fix UAF around queue destruction")
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
Link: https://patch.msgid.link/20260716062624.211396-1-arvind.yadav@intel.com
Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
(cherry picked from commit da1124abac689cc2b1d8995e5f0a816f8a122edb)
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index abe25aedeeadef60820247ec5e14678c321f0852..f3eb83644a6f8b1a07c2b735b566bbe250ef2481 100644 (file)
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -580,7 +580,7 @@ int xe_device_init_early(struct xe_device *xe)
                                                       WQ_MEM_RECLAIM);
        xe->ordered_wq = alloc_ordered_workqueue("xe-ordered-wq", 0);
        xe->unordered_wq = alloc_workqueue("xe-unordered-wq", WQ_PERCPU, 0);
-       xe->destroy_wq = alloc_workqueue("xe-destroy-wq", WQ_PERCPU, 0);
+       xe->destroy_wq = alloc_workqueue("xe-destroy-wq", WQ_PERCPU | WQ_MEM_RECLAIM, 0);
        if (!xe->ordered_wq || !xe->unordered_wq ||
            !xe->preempt_fence_wq || !xe->destroy_wq) {
                /*

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 32dd2ffbc796397028a1fb0025f0cc0db2d399d3..aad10899d9ace1be81b84be4c4545945bb6b0916 100644 (file)
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -355,7 +355,7 @@ struct xe_device {
        /** @unordered_wq: used to serialize unordered work */
        struct workqueue_struct *unordered_wq;
 
-       /** @destroy_wq: used to serialize user destroy work, like queue */
+       /** @destroy_wq: used to serialize SVM pagemap destroy work */
        struct workqueue_struct *destroy_wq;
 
        /** @tiles: device tiles */

diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
index 9109f21d367fdbd5e2494730e000c963684f75cd..1c92b96e4c3246d86dea83b408c7e6fe24adf3e0 100644 (file)
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -10,6 +10,7 @@
 #include <linux/circ_buf.h>
 #include <linux/dma-fence-array.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_managed.h>
 
 #include "abi/guc_actions_abi.h"
@@ -37,6 +38,7 @@
 #include "xe_macros.h"
 #include "xe_map.h"
 #include "xe_mocs.h"
+#include "xe_module.h"
 #include "xe_pm.h"
 #include "xe_ring_ops_types.h"
 #include "xe_sched_job.h"
@@ -232,17 +234,9 @@ static bool exec_queue_killed_or_banned_or_wedged(struct xe_exec_queue *q)
 static void guc_submit_sw_fini(struct drm_device *drm, void *arg)
 {
        struct xe_guc *guc = arg;
-       struct xe_device *xe = guc_to_xe(guc);
        struct xe_gt *gt = guc_to_gt(guc);
-       int ret;
-
-       ret = wait_event_timeout(guc->submission_state.fini_wq,
-                                xa_empty(&guc->submission_state.exec_queue_lookup),
-                                HZ * 5);
 
-       drain_workqueue(xe->destroy_wq);
-
-       xe_gt_assert(gt, ret);
+       xe_gt_assert(gt, xa_empty(&guc->submission_state.exec_queue_lookup));
 
        xa_destroy(&guc->submission_state.exec_queue_lookup);
 }
@@ -319,8 +313,6 @@ int xe_guc_submit_init(struct xe_guc *guc, unsigned int num_ids)
 
        xa_init(&guc->submission_state.exec_queue_lookup);
 
-       init_waitqueue_head(&guc->submission_state.fini_wq);
-
        primelockdep(guc);
 
        guc->submission_state.initialized = true;
@@ -411,9 +403,6 @@ static void __release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q,
        xe_guc_id_mgr_release_locked(&guc->submission_state.idm,
                                     q->guc->id, q->width);
 
-       if (xa_empty(&guc->submission_state.exec_queue_lookup))
-               wake_up(&guc->submission_state.fini_wq);
-
        mutex_unlock(&guc->submission_state.lock);
 }
 
@@ -1685,6 +1674,7 @@ static void guc_exec_queue_fini(struct xe_exec_queue *q)
 {
        struct xe_guc_exec_queue *ge = q->guc;
        struct xe_guc *guc = exec_queue_to_guc(q);
+       struct drm_device *drm = &guc_to_xe(guc)->drm;
 
        if (xe_exec_queue_is_multi_queue_secondary(q)) {
                struct xe_exec_queue_group *group = q->multi_queue.group;
@@ -1703,36 +1693,52 @@ static void guc_exec_queue_fini(struct xe_exec_queue *q)
         * (timeline name).
         */
        kfree_rcu(ge, rcu);
+
+       drm_dev_put(drm);
 }
 
-static void __guc_exec_queue_destroy_async(struct work_struct *w)
+static void guc_exec_queue_do_destroy(struct xe_exec_queue *q)
 {
-       struct xe_guc_exec_queue *ge =
-               container_of(w, struct xe_guc_exec_queue, destroy_async);
-       struct xe_exec_queue *q = ge->q;
+       struct xe_guc_exec_queue *ge = q->guc;
        struct xe_guc *guc = exec_queue_to_guc(q);
+       struct xe_device *xe = guc_to_xe(guc);
+       struct drm_device *drm = &xe->drm;
 
-       guard(xe_pm_runtime)(guc_to_xe(guc));
-       trace_xe_exec_queue_destroy(q);
+       /*
+        * guc_exec_queue_fini() drops the queue's drm_device ref.
+        * Keep the device alive until the PM-runtime guard unwinds.
+        */
+       drm_dev_get(drm);
 
-       /* Confirm no work left behind accessing device structures */
-       cancel_delayed_work_sync(&ge->sched.base.work_tdr);
+       scoped_guard(xe_pm_runtime, xe) {
+               trace_xe_exec_queue_destroy(q);
 
-       xe_exec_queue_fini(q);
+               /* Confirm no work left behind accessing device structures */
+               cancel_delayed_work_sync(&ge->sched.base.work_tdr);
+
+               xe_exec_queue_fini(q);
+       }
+
+       drm_dev_put(drm);
 }
 
-static void guc_exec_queue_destroy_async(struct xe_exec_queue *q)
+static void __guc_exec_queue_destroy_async(struct work_struct *w)
 {
-       struct xe_guc *guc = exec_queue_to_guc(q);
-       struct xe_device *xe = guc_to_xe(guc);
+       struct xe_guc_exec_queue *ge =
+               container_of(w, struct xe_guc_exec_queue, destroy_async);
+
+       guc_exec_queue_do_destroy(ge->q);
+}
 
+static void guc_exec_queue_destroy_async(struct xe_exec_queue *q)
+{
        INIT_WORK(&q->guc->destroy_async, __guc_exec_queue_destroy_async);
 
        /* We must block on kernel engines so slabs are empty on driver unload */
        if (q->flags & EXEC_QUEUE_FLAG_PERMANENT || exec_queue_wedged(q))
-               __guc_exec_queue_destroy_async(&q->guc->destroy_async);
+               guc_exec_queue_do_destroy(q);
        else
-               queue_work(xe->destroy_wq, &q->guc->destroy_async);
+               xe_destroy_wq_queue(&q->guc->destroy_async);
 }
 
 static void __guc_exec_queue_destroy(struct xe_guc *guc, struct xe_exec_queue *q)
@@ -1927,6 +1933,7 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
 {
        struct xe_gpu_scheduler *sched;
        struct xe_guc *guc = exec_queue_to_guc(q);
+       struct drm_device *drm = &guc_to_xe(guc)->drm;
        struct workqueue_struct *submit_wq = NULL;
        struct xe_guc_exec_queue *ge;
        long timeout;
@@ -1938,6 +1945,8 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
        if (!ge)
                return -ENOMEM;
 
+       drm_dev_get(drm);
+
        q->guc = ge;
        ge->q = q;
        init_rcu_head(&ge->rcu);
@@ -2016,6 +2025,7 @@ err_release_id:
        release_guc_id(guc, q);
 err_free:
        kfree(ge);
+       drm_dev_put(drm);
 
        return err;
 }

diff --git a/drivers/gpu/drm/xe/xe_guc_types.h b/drivers/gpu/drm/xe/xe_guc_types.h
index c7b9642b41ba74ebb67965b0baefdc55bd571b2b..31a2acb63ac34d93b38e8cd9e7d5c6e5346a64da 100644 (file)
--- a/drivers/gpu/drm/xe/xe_guc_types.h
+++ b/drivers/gpu/drm/xe/xe_guc_types.h
@@ -100,8 +100,6 @@ struct xe_guc {
                 * even initialized - before that not even the lock is valid
                 */
                bool initialized;
-               /** @submission_state.fini_wq: submit fini wait queue */
-               wait_queue_head_t fini_wq;
        } submission_state;
 
        /** @hwconfig: Hardware config state */

diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
index 4cb5781829122950af0e2bb69a1a70c02277bd74..99347f216ec8932216695324f808d0f2ac41d9b9 100644 (file)
--- a/drivers/gpu/drm/xe/xe_module.c
+++ b/drivers/gpu/drm/xe/xe_module.c
@@ -7,6 +7,7 @@
 
 #include <linux/init.h>
 #include <linux/module.h>
+#include <linux/workqueue.h>
 
 #include <drm/drm_module.h>
 
@@ -91,6 +92,50 @@ static int xe_check_nomodeset(void)
        return 0;
 }
 
+static struct workqueue_struct *xe_destroy_wq;
+
+static int __init xe_destroy_wq_module_init(void)
+{
+       xe_destroy_wq = alloc_workqueue("xe-guc-destroy-wq", WQ_UNBOUND, 0);
+       if (!xe_destroy_wq)
+               return -ENOMEM;
+       return 0;
+}
+
+static void xe_destroy_wq_module_exit(void)
+{
+       if (xe_destroy_wq)
+               destroy_workqueue(xe_destroy_wq);
+       xe_destroy_wq = NULL;
+}
+
+/**
+ * xe_destroy_wq_queue() - Queue work on the destroy workqueue
+ * @work: work item to queue
+ *
+ * The destroy workqueue has module lifetime and is used for GuC exec queue
+ * teardown that can outlive a single xe_device. SVM pagemap destroy uses the
+ * per-device xe->destroy_wq instead.
+ *
+ * Return: %true if @work was queued, %false if it was already pending.
+ */
+bool xe_destroy_wq_queue(struct work_struct *work)
+{
+       return queue_work(xe_destroy_wq, work);
+}
+
+/**
+ * xe_destroy_wq_flush() - Flush the destroy workqueue
+ *
+ * Drains all pending destroy work. Called from PCI remove to ensure
+ * teardown ordering before the device is destroyed.
+ */
+void xe_destroy_wq_flush(void)
+{
+       if (xe_destroy_wq)
+               flush_workqueue(xe_destroy_wq);
+}
+
 struct init_funcs {
        int (*init)(void);
        void (*exit)(void);
@@ -112,6 +157,10 @@ static const struct init_funcs init_funcs[] = {
                .init = xe_sched_job_module_init,
                .exit = xe_sched_job_module_exit,
        },
+       {
+               .init = xe_destroy_wq_module_init,
+               .exit = xe_destroy_wq_module_exit,
+       },
        {
                .init = xe_register_pci_driver,
                .exit = xe_unregister_pci_driver,

diff --git a/drivers/gpu/drm/xe/xe_module.h b/drivers/gpu/drm/xe/xe_module.h
index 79cb9639c0f3df6b6ebab70eb2e11d7d23694028..e8e54f701cf3ee8bbd5df9e71b3e9b18cf21c012 100644 (file)
--- a/drivers/gpu/drm/xe/xe_module.h
+++ b/drivers/gpu/drm/xe/xe_module.h
@@ -8,6 +8,8 @@
 
 #include <linux/types.h>
 
+struct work_struct;
+
 /* Module modprobe variables */
 struct xe_modparam {
        bool force_execlist;
@@ -27,5 +29,8 @@ struct xe_modparam {
 
 extern struct xe_modparam xe_modparam;
 
+bool xe_destroy_wq_queue(struct work_struct *work);
+void xe_destroy_wq_flush(void);
+
 #endif
 

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 3165686e3e04bc455c0a98f3c95c265de83d8c69..2bd601eb088b93aad04a6ea5886223e11214dd21 100644 (file)
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -1048,6 +1048,12 @@ static void xe_pci_remove(struct pci_dev *pdev)
                return;
 
        xe_device_remove(xe);
+
+       /*
+        * Preserve remove-time flush after moving destroy work to module
+        * lifetime.
+        */
+       xe_destroy_wq_flush();
        xe_pm_fini(xe);
 }