Skip to content

Commit 0995c2f

Browse files
mbrost05lucasdemarchi
authored andcommitted
drm/xe: Enforce correct user fence signaling order using
Prevent application hangs caused by out-of-order fence signaling when user fences are attached. Use drm_syncobj (via dma-fence-chain) to guarantee that each user fence signals in order, regardless of the signaling order of the attached fences. Ensure user fence writebacks to user space occur in the correct sequence. v7: - Skip drm_syncbj create of error (CI) Fixes: dd08ebf ("drm/xe: Introduce a new DRM driver for Intel GPUs") Signed-off-by: Matthew Brost <matthew.brost@intel.com> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Link: https://patch.msgid.link/20251031234050.3043507-2-matthew.brost@intel.com (cherry picked from commit adda4e8) Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
1 parent b11a020 commit 0995c2f

File tree

9 files changed

+86
-18
lines changed

9 files changed

+86
-18
lines changed

drivers/gpu/drm/xe/xe_exec.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
165165

166166
for (num_syncs = 0; num_syncs < args->num_syncs; num_syncs++) {
167167
err = xe_sync_entry_parse(xe, xef, &syncs[num_syncs],
168-
&syncs_user[num_syncs], SYNC_PARSE_FLAG_EXEC |
168+
&syncs_user[num_syncs], NULL, 0,
169+
SYNC_PARSE_FLAG_EXEC |
169170
(xe_vm_in_lr_mode(vm) ?
170171
SYNC_PARSE_FLAG_LR_MODE : 0));
171172
if (err)

drivers/gpu/drm/xe/xe_exec_queue.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <drm/drm_device.h>
1111
#include <drm/drm_drv.h>
1212
#include <drm/drm_file.h>
13+
#include <drm/drm_syncobj.h>
1314
#include <uapi/drm/xe_drm.h>
1415

1516
#include "xe_dep_scheduler.h"
@@ -324,6 +325,16 @@ struct xe_exec_queue *xe_exec_queue_create_bind(struct xe_device *xe,
324325
}
325326
xe_vm_put(migrate_vm);
326327

328+
if (!IS_ERR(q)) {
329+
int err = drm_syncobj_create(&q->ufence_syncobj,
330+
DRM_SYNCOBJ_CREATE_SIGNALED,
331+
NULL);
332+
if (err) {
333+
xe_exec_queue_put(q);
334+
return ERR_PTR(err);
335+
}
336+
}
337+
327338
return q;
328339
}
329340
ALLOW_ERROR_INJECTION(xe_exec_queue_create_bind, ERRNO);
@@ -333,6 +344,9 @@ void xe_exec_queue_destroy(struct kref *ref)
333344
struct xe_exec_queue *q = container_of(ref, struct xe_exec_queue, refcount);
334345
struct xe_exec_queue *eq, *next;
335346

347+
if (q->ufence_syncobj)
348+
drm_syncobj_put(q->ufence_syncobj);
349+
336350
if (xe_exec_queue_uses_pxp(q))
337351
xe_pxp_exec_queue_remove(gt_to_xe(q->gt)->pxp, q);
338352

drivers/gpu/drm/xe/xe_exec_queue_types.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "xe_hw_fence_types.h"
1616
#include "xe_lrc_types.h"
1717

18+
struct drm_syncobj;
1819
struct xe_execlist_exec_queue;
1920
struct xe_gt;
2021
struct xe_guc_exec_queue;
@@ -155,6 +156,12 @@ struct xe_exec_queue {
155156
struct list_head link;
156157
} pxp;
157158

159+
/** @ufence_syncobj: User fence syncobj */
160+
struct drm_syncobj *ufence_syncobj;
161+
162+
/** @ufence_timeline_value: User fence timeline value */
163+
u64 ufence_timeline_value;
164+
158165
/** @ops: submission backend exec queue operations */
159166
const struct xe_exec_queue_ops *ops;
160167

drivers/gpu/drm/xe/xe_oa.c

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include <drm/drm_drv.h>
1212
#include <drm/drm_managed.h>
13+
#include <drm/drm_syncobj.h>
1314
#include <uapi/drm/xe_drm.h>
1415

1516
#include <generated/xe_wa_oob.h>
@@ -1389,7 +1390,9 @@ static int xe_oa_user_extensions(struct xe_oa *oa, enum xe_oa_user_extn_from fro
13891390
return 0;
13901391
}
13911392

1392-
static int xe_oa_parse_syncs(struct xe_oa *oa, struct xe_oa_open_param *param)
1393+
static int xe_oa_parse_syncs(struct xe_oa *oa,
1394+
struct xe_oa_stream *stream,
1395+
struct xe_oa_open_param *param)
13931396
{
13941397
int ret, num_syncs, num_ufence = 0;
13951398

@@ -1409,7 +1412,9 @@ static int xe_oa_parse_syncs(struct xe_oa *oa, struct xe_oa_open_param *param)
14091412

14101413
for (num_syncs = 0; num_syncs < param->num_syncs; num_syncs++) {
14111414
ret = xe_sync_entry_parse(oa->xe, param->xef, &param->syncs[num_syncs],
1412-
&param->syncs_user[num_syncs], 0);
1415+
&param->syncs_user[num_syncs],
1416+
stream->ufence_syncobj,
1417+
++stream->ufence_timeline_value, 0);
14131418
if (ret)
14141419
goto err_syncs;
14151420

@@ -1539,7 +1544,7 @@ static long xe_oa_config_locked(struct xe_oa_stream *stream, u64 arg)
15391544
return -ENODEV;
15401545

15411546
param.xef = stream->xef;
1542-
err = xe_oa_parse_syncs(stream->oa, &param);
1547+
err = xe_oa_parse_syncs(stream->oa, stream, &param);
15431548
if (err)
15441549
goto err_config_put;
15451550

@@ -1635,6 +1640,7 @@ static void xe_oa_destroy_locked(struct xe_oa_stream *stream)
16351640
if (stream->exec_q)
16361641
xe_exec_queue_put(stream->exec_q);
16371642

1643+
drm_syncobj_put(stream->ufence_syncobj);
16381644
kfree(stream);
16391645
}
16401646

@@ -1826,6 +1832,7 @@ static int xe_oa_stream_open_ioctl_locked(struct xe_oa *oa,
18261832
struct xe_oa_open_param *param)
18271833
{
18281834
struct xe_oa_stream *stream;
1835+
struct drm_syncobj *ufence_syncobj;
18291836
int stream_fd;
18301837
int ret;
18311838

@@ -1836,17 +1843,31 @@ static int xe_oa_stream_open_ioctl_locked(struct xe_oa *oa,
18361843
goto exit;
18371844
}
18381845

1846+
ret = drm_syncobj_create(&ufence_syncobj, DRM_SYNCOBJ_CREATE_SIGNALED,
1847+
NULL);
1848+
if (ret)
1849+
goto exit;
1850+
18391851
stream = kzalloc(sizeof(*stream), GFP_KERNEL);
18401852
if (!stream) {
18411853
ret = -ENOMEM;
1842-
goto exit;
1854+
goto err_syncobj;
18431855
}
1844-
1856+
stream->ufence_syncobj = ufence_syncobj;
18451857
stream->oa = oa;
1846-
ret = xe_oa_stream_init(stream, param);
1858+
1859+
ret = xe_oa_parse_syncs(oa, stream, param);
18471860
if (ret)
18481861
goto err_free;
18491862

1863+
ret = xe_oa_stream_init(stream, param);
1864+
if (ret) {
1865+
while (param->num_syncs--)
1866+
xe_sync_entry_cleanup(&param->syncs[param->num_syncs]);
1867+
kfree(param->syncs);
1868+
goto err_free;
1869+
}
1870+
18501871
if (!param->disabled) {
18511872
ret = xe_oa_enable_locked(stream);
18521873
if (ret)
@@ -1870,6 +1891,8 @@ static int xe_oa_stream_open_ioctl_locked(struct xe_oa *oa,
18701891
xe_oa_stream_destroy(stream);
18711892
err_free:
18721893
kfree(stream);
1894+
err_syncobj:
1895+
drm_syncobj_put(ufence_syncobj);
18731896
exit:
18741897
return ret;
18751898
}
@@ -2083,22 +2106,14 @@ int xe_oa_stream_open_ioctl(struct drm_device *dev, u64 data, struct drm_file *f
20832106
goto err_exec_q;
20842107
}
20852108

2086-
ret = xe_oa_parse_syncs(oa, &param);
2087-
if (ret)
2088-
goto err_exec_q;
2089-
20902109
mutex_lock(&param.hwe->gt->oa.gt_lock);
20912110
ret = xe_oa_stream_open_ioctl_locked(oa, &param);
20922111
mutex_unlock(&param.hwe->gt->oa.gt_lock);
20932112
if (ret < 0)
2094-
goto err_sync_cleanup;
2113+
goto err_exec_q;
20952114

20962115
return ret;
20972116

2098-
err_sync_cleanup:
2099-
while (param.num_syncs--)
2100-
xe_sync_entry_cleanup(&param.syncs[param.num_syncs]);
2101-
kfree(param.syncs);
21022117
err_exec_q:
21032118
if (param.exec_q)
21042119
xe_exec_queue_put(param.exec_q);

drivers/gpu/drm/xe/xe_oa_types.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#include "regs/xe_reg_defs.h"
1616
#include "xe_hw_engine_types.h"
1717

18+
struct drm_syncobj;
19+
1820
#define DEFAULT_XE_OA_BUFFER_SIZE SZ_16M
1921

2022
enum xe_oa_report_header {
@@ -248,6 +250,12 @@ struct xe_oa_stream {
248250
/** @xef: xe_file with which the stream was opened */
249251
struct xe_file *xef;
250252

253+
/** @ufence_syncobj: User fence syncobj */
254+
struct drm_syncobj *ufence_syncobj;
255+
256+
/** @ufence_timeline_value: User fence timeline value */
257+
u64 ufence_timeline_value;
258+
251259
/** @last_fence: fence to use in stream destroy when needed */
252260
struct dma_fence *last_fence;
253261

drivers/gpu/drm/xe/xe_sync.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ static void user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
113113
int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
114114
struct xe_sync_entry *sync,
115115
struct drm_xe_sync __user *sync_user,
116+
struct drm_syncobj *ufence_syncobj,
117+
u64 ufence_timeline_value,
116118
unsigned int flags)
117119
{
118120
struct drm_xe_sync sync_in;
@@ -192,10 +194,15 @@ int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
192194
if (exec) {
193195
sync->addr = sync_in.addr;
194196
} else {
197+
sync->ufence_timeline_value = ufence_timeline_value;
195198
sync->ufence = user_fence_create(xe, sync_in.addr,
196199
sync_in.timeline_value);
197200
if (XE_IOCTL_DBG(xe, IS_ERR(sync->ufence)))
198201
return PTR_ERR(sync->ufence);
202+
sync->ufence_chain_fence = dma_fence_chain_alloc();
203+
if (!sync->ufence_chain_fence)
204+
return -ENOMEM;
205+
sync->ufence_syncobj = ufence_syncobj;
199206
}
200207

201208
break;
@@ -239,7 +246,12 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
239246
} else if (sync->ufence) {
240247
int err;
241248

242-
dma_fence_get(fence);
249+
drm_syncobj_add_point(sync->ufence_syncobj,
250+
sync->ufence_chain_fence,
251+
fence, sync->ufence_timeline_value);
252+
sync->ufence_chain_fence = NULL;
253+
254+
fence = drm_syncobj_fence_get(sync->ufence_syncobj);
243255
user_fence_get(sync->ufence);
244256
err = dma_fence_add_callback(fence, &sync->ufence->cb,
245257
user_fence_cb);
@@ -259,7 +271,8 @@ void xe_sync_entry_cleanup(struct xe_sync_entry *sync)
259271
drm_syncobj_put(sync->syncobj);
260272
dma_fence_put(sync->fence);
261273
dma_fence_chain_free(sync->chain_fence);
262-
if (sync->ufence)
274+
dma_fence_chain_free(sync->ufence_chain_fence);
275+
if (!IS_ERR_OR_NULL(sync->ufence))
263276
user_fence_put(sync->ufence);
264277
}
265278

drivers/gpu/drm/xe/xe_sync.h

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

99
#include "xe_sync_types.h"
1010

11+
struct drm_syncobj;
1112
struct xe_device;
1213
struct xe_exec_queue;
1314
struct xe_file;
@@ -21,6 +22,8 @@ struct xe_vm;
2122
int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
2223
struct xe_sync_entry *sync,
2324
struct drm_xe_sync __user *sync_user,
25+
struct drm_syncobj *ufence_syncobj,
26+
u64 ufence_timeline_value,
2427
unsigned int flags);
2528
int xe_sync_entry_add_deps(struct xe_sync_entry *sync,
2629
struct xe_sched_job *job);

drivers/gpu/drm/xe/xe_sync_types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ struct xe_sync_entry {
1818
struct drm_syncobj *syncobj;
1919
struct dma_fence *fence;
2020
struct dma_fence_chain *chain_fence;
21+
struct dma_fence_chain *ufence_chain_fence;
22+
struct drm_syncobj *ufence_syncobj;
2123
struct xe_user_fence *ufence;
2224
u64 addr;
2325
u64 timeline_value;
26+
u64 ufence_timeline_value;
2427
u32 type;
2528
u32 flags;
2629
};

drivers/gpu/drm/xe/xe_vm.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3606,8 +3606,12 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
36063606

36073607
syncs_user = u64_to_user_ptr(args->syncs);
36083608
for (num_syncs = 0; num_syncs < args->num_syncs; num_syncs++) {
3609+
struct xe_exec_queue *__q = q ?: vm->q[0];
3610+
36093611
err = xe_sync_entry_parse(xe, xef, &syncs[num_syncs],
36103612
&syncs_user[num_syncs],
3613+
__q->ufence_syncobj,
3614+
++__q->ufence_timeline_value,
36113615
(xe_vm_in_lr_mode(vm) ?
36123616
SYNC_PARSE_FLAG_LR_MODE : 0) |
36133617
(!args->num_binds ?

0 commit comments

Comments
 (0)