Skip to content

Commit 9b7e601

Browse files
Intel-ArvindYadavThomas Hellström
authored andcommitted
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: 2d2be27 ("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 da1124a) Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
1 parent 299bc6d commit 9b7e601

7 files changed

Lines changed: 100 additions & 32 deletions

File tree

drivers/gpu/drm/xe/xe_device.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ int xe_device_init_early(struct xe_device *xe)
580580
WQ_MEM_RECLAIM);
581581
xe->ordered_wq = alloc_ordered_workqueue("xe-ordered-wq", 0);
582582
xe->unordered_wq = alloc_workqueue("xe-unordered-wq", WQ_PERCPU, 0);
583-
xe->destroy_wq = alloc_workqueue("xe-destroy-wq", WQ_PERCPU, 0);
583+
xe->destroy_wq = alloc_workqueue("xe-destroy-wq", WQ_PERCPU | WQ_MEM_RECLAIM, 0);
584584
if (!xe->ordered_wq || !xe->unordered_wq ||
585585
!xe->preempt_fence_wq || !xe->destroy_wq) {
586586
/*

drivers/gpu/drm/xe/xe_device_types.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ struct xe_device {
355355
/** @unordered_wq: used to serialize unordered work */
356356
struct workqueue_struct *unordered_wq;
357357

358-
/** @destroy_wq: used to serialize user destroy work, like queue */
358+
/** @destroy_wq: used to serialize SVM pagemap destroy work */
359359
struct workqueue_struct *destroy_wq;
360360

361361
/** @tiles: device tiles */

drivers/gpu/drm/xe/xe_guc_submit.c

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <linux/circ_buf.h>
1111
#include <linux/dma-fence-array.h>
1212

13+
#include <drm/drm_drv.h>
1314
#include <drm/drm_managed.h>
1415

1516
#include "abi/guc_actions_abi.h"
@@ -37,6 +38,7 @@
3738
#include "xe_macros.h"
3839
#include "xe_map.h"
3940
#include "xe_mocs.h"
41+
#include "xe_module.h"
4042
#include "xe_pm.h"
4143
#include "xe_ring_ops_types.h"
4244
#include "xe_sched_job.h"
@@ -232,17 +234,9 @@ static bool exec_queue_killed_or_banned_or_wedged(struct xe_exec_queue *q)
232234
static void guc_submit_sw_fini(struct drm_device *drm, void *arg)
233235
{
234236
struct xe_guc *guc = arg;
235-
struct xe_device *xe = guc_to_xe(guc);
236237
struct xe_gt *gt = guc_to_gt(guc);
237-
int ret;
238-
239-
ret = wait_event_timeout(guc->submission_state.fini_wq,
240-
xa_empty(&guc->submission_state.exec_queue_lookup),
241-
HZ * 5);
242238

243-
drain_workqueue(xe->destroy_wq);
244-
245-
xe_gt_assert(gt, ret);
239+
xe_gt_assert(gt, xa_empty(&guc->submission_state.exec_queue_lookup));
246240

247241
xa_destroy(&guc->submission_state.exec_queue_lookup);
248242
}
@@ -319,8 +313,6 @@ int xe_guc_submit_init(struct xe_guc *guc, unsigned int num_ids)
319313

320314
xa_init(&guc->submission_state.exec_queue_lookup);
321315

322-
init_waitqueue_head(&guc->submission_state.fini_wq);
323-
324316
primelockdep(guc);
325317

326318
guc->submission_state.initialized = true;
@@ -411,9 +403,6 @@ static void __release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q,
411403
xe_guc_id_mgr_release_locked(&guc->submission_state.idm,
412404
q->guc->id, q->width);
413405

414-
if (xa_empty(&guc->submission_state.exec_queue_lookup))
415-
wake_up(&guc->submission_state.fini_wq);
416-
417406
mutex_unlock(&guc->submission_state.lock);
418407
}
419408

@@ -1685,6 +1674,7 @@ static void guc_exec_queue_fini(struct xe_exec_queue *q)
16851674
{
16861675
struct xe_guc_exec_queue *ge = q->guc;
16871676
struct xe_guc *guc = exec_queue_to_guc(q);
1677+
struct drm_device *drm = &guc_to_xe(guc)->drm;
16881678

16891679
if (xe_exec_queue_is_multi_queue_secondary(q)) {
16901680
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)
17031693
* (timeline name).
17041694
*/
17051695
kfree_rcu(ge, rcu);
1696+
1697+
drm_dev_put(drm);
17061698
}
17071699

1708-
static void __guc_exec_queue_destroy_async(struct work_struct *w)
1700+
static void guc_exec_queue_do_destroy(struct xe_exec_queue *q)
17091701
{
1710-
struct xe_guc_exec_queue *ge =
1711-
container_of(w, struct xe_guc_exec_queue, destroy_async);
1712-
struct xe_exec_queue *q = ge->q;
1702+
struct xe_guc_exec_queue *ge = q->guc;
17131703
struct xe_guc *guc = exec_queue_to_guc(q);
1704+
struct xe_device *xe = guc_to_xe(guc);
1705+
struct drm_device *drm = &xe->drm;
17141706

1715-
guard(xe_pm_runtime)(guc_to_xe(guc));
1716-
trace_xe_exec_queue_destroy(q);
1707+
/*
1708+
* guc_exec_queue_fini() drops the queue's drm_device ref.
1709+
* Keep the device alive until the PM-runtime guard unwinds.
1710+
*/
1711+
drm_dev_get(drm);
17171712

1718-
/* Confirm no work left behind accessing device structures */
1719-
cancel_delayed_work_sync(&ge->sched.base.work_tdr);
1713+
scoped_guard(xe_pm_runtime, xe) {
1714+
trace_xe_exec_queue_destroy(q);
17201715

1721-
xe_exec_queue_fini(q);
1716+
/* Confirm no work left behind accessing device structures */
1717+
cancel_delayed_work_sync(&ge->sched.base.work_tdr);
1718+
1719+
xe_exec_queue_fini(q);
1720+
}
1721+
1722+
drm_dev_put(drm);
17221723
}
17231724

1724-
static void guc_exec_queue_destroy_async(struct xe_exec_queue *q)
1725+
static void __guc_exec_queue_destroy_async(struct work_struct *w)
17251726
{
1726-
struct xe_guc *guc = exec_queue_to_guc(q);
1727-
struct xe_device *xe = guc_to_xe(guc);
1727+
struct xe_guc_exec_queue *ge =
1728+
container_of(w, struct xe_guc_exec_queue, destroy_async);
1729+
1730+
guc_exec_queue_do_destroy(ge->q);
1731+
}
17281732

1733+
static void guc_exec_queue_destroy_async(struct xe_exec_queue *q)
1734+
{
17291735
INIT_WORK(&q->guc->destroy_async, __guc_exec_queue_destroy_async);
17301736

17311737
/* We must block on kernel engines so slabs are empty on driver unload */
17321738
if (q->flags & EXEC_QUEUE_FLAG_PERMANENT || exec_queue_wedged(q))
1733-
__guc_exec_queue_destroy_async(&q->guc->destroy_async);
1739+
guc_exec_queue_do_destroy(q);
17341740
else
1735-
queue_work(xe->destroy_wq, &q->guc->destroy_async);
1741+
xe_destroy_wq_queue(&q->guc->destroy_async);
17361742
}
17371743

17381744
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)
19271933
{
19281934
struct xe_gpu_scheduler *sched;
19291935
struct xe_guc *guc = exec_queue_to_guc(q);
1936+
struct drm_device *drm = &guc_to_xe(guc)->drm;
19301937
struct workqueue_struct *submit_wq = NULL;
19311938
struct xe_guc_exec_queue *ge;
19321939
long timeout;
@@ -1938,6 +1945,8 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
19381945
if (!ge)
19391946
return -ENOMEM;
19401947

1948+
drm_dev_get(drm);
1949+
19411950
q->guc = ge;
19421951
ge->q = q;
19431952
init_rcu_head(&ge->rcu);
@@ -2016,6 +2025,7 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
20162025
release_guc_id(guc, q);
20172026
err_free:
20182027
kfree(ge);
2028+
drm_dev_put(drm);
20192029

20202030
return err;
20212031
}

drivers/gpu/drm/xe/xe_guc_types.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,6 @@ struct xe_guc {
100100
* even initialized - before that not even the lock is valid
101101
*/
102102
bool initialized;
103-
/** @submission_state.fini_wq: submit fini wait queue */
104-
wait_queue_head_t fini_wq;
105103
} submission_state;
106104

107105
/** @hwconfig: Hardware config state */

drivers/gpu/drm/xe/xe_module.c

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <linux/init.h>
99
#include <linux/module.h>
10+
#include <linux/workqueue.h>
1011

1112
#include <drm/drm_module.h>
1213

@@ -91,6 +92,50 @@ static int xe_check_nomodeset(void)
9192
return 0;
9293
}
9394

95+
static struct workqueue_struct *xe_destroy_wq;
96+
97+
static int __init xe_destroy_wq_module_init(void)
98+
{
99+
xe_destroy_wq = alloc_workqueue("xe-guc-destroy-wq", WQ_UNBOUND, 0);
100+
if (!xe_destroy_wq)
101+
return -ENOMEM;
102+
return 0;
103+
}
104+
105+
static void xe_destroy_wq_module_exit(void)
106+
{
107+
if (xe_destroy_wq)
108+
destroy_workqueue(xe_destroy_wq);
109+
xe_destroy_wq = NULL;
110+
}
111+
112+
/**
113+
* xe_destroy_wq_queue() - Queue work on the destroy workqueue
114+
* @work: work item to queue
115+
*
116+
* The destroy workqueue has module lifetime and is used for GuC exec queue
117+
* teardown that can outlive a single xe_device. SVM pagemap destroy uses the
118+
* per-device xe->destroy_wq instead.
119+
*
120+
* Return: %true if @work was queued, %false if it was already pending.
121+
*/
122+
bool xe_destroy_wq_queue(struct work_struct *work)
123+
{
124+
return queue_work(xe_destroy_wq, work);
125+
}
126+
127+
/**
128+
* xe_destroy_wq_flush() - Flush the destroy workqueue
129+
*
130+
* Drains all pending destroy work. Called from PCI remove to ensure
131+
* teardown ordering before the device is destroyed.
132+
*/
133+
void xe_destroy_wq_flush(void)
134+
{
135+
if (xe_destroy_wq)
136+
flush_workqueue(xe_destroy_wq);
137+
}
138+
94139
struct init_funcs {
95140
int (*init)(void);
96141
void (*exit)(void);
@@ -112,6 +157,10 @@ static const struct init_funcs init_funcs[] = {
112157
.init = xe_sched_job_module_init,
113158
.exit = xe_sched_job_module_exit,
114159
},
160+
{
161+
.init = xe_destroy_wq_module_init,
162+
.exit = xe_destroy_wq_module_exit,
163+
},
115164
{
116165
.init = xe_register_pci_driver,
117166
.exit = xe_unregister_pci_driver,

drivers/gpu/drm/xe/xe_module.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
#include <linux/types.h>
1010

11+
struct work_struct;
12+
1113
/* Module modprobe variables */
1214
struct xe_modparam {
1315
bool force_execlist;
@@ -27,5 +29,8 @@ struct xe_modparam {
2729

2830
extern struct xe_modparam xe_modparam;
2931

32+
bool xe_destroy_wq_queue(struct work_struct *work);
33+
void xe_destroy_wq_flush(void);
34+
3035
#endif
3136

drivers/gpu/drm/xe/xe_pci.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,12 @@ static void xe_pci_remove(struct pci_dev *pdev)
10481048
return;
10491049

10501050
xe_device_remove(xe);
1051+
1052+
/*
1053+
* Preserve remove-time flush after moving destroy work to module
1054+
* lifetime.
1055+
*/
1056+
xe_destroy_wq_flush();
10511057
xe_pm_fini(xe);
10521058
}
10531059

0 commit comments

Comments
 (0)