[PATCH 0/2] drm: Add Common drm_user_fence helper and Convert XE

[PATCH 0/2] drm: Add Common drm_user_fence helper and Convert XE

[Srinivasan Shanmugam]

When a GPU dma-fence signals, drivers often need to access userspace
memory from a kthread context — either to write a fence completion
value to a userspace VA (XE) or to signal a per-queue eventfd (AMDGPU).
Both require borrowing the process MM via kthread_use_mm(), and both
use the same kref-managed dma-fence-callback-to-workqueue pattern.

Extracting this pattern into a shared DRM helper allows both XE and
AMDGPU to use it instead of maintaining independent open-coded
implementations.

This series does that:

Patch 1 introduces drm_user_fence — an embeddable base structure with
driver-supplied worker/destroy callbacks. The common code handles:
 - process MM grab at init (mmgrab)
 - dma-fence callback registration
 - workqueue dispatch on fence signal
 - mmget_not_zero/kthread_use_mm/mmput in the worker
 - kref lifetime management

Patch 2 converts XE to use the new helper. struct xe_user_fence embeds
struct drm_user_fence as its base. XE-specific fields (xe_device pointer
for ufence_wq wake-up, userspace VA, expected value, signalled flag)
remain in the wrapper. No behavioral change is intended.

A follow-on patch (not in this series) will wire AMDGPU's render-node
EOP eventfd signaling path to the same helper.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org

Srinivasan Shanmugam (2):
  drm: Add common drm_user_fence helper
  drm/xe: Convert xe_user_fence to drm_user_fence

 drivers/gpu/drm/Makefile           |   1 +
 drivers/gpu/drm/drm_user_fence.c   | 130 +++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_sync.c       | 114 ++++++++++---------------
 drivers/gpu/drm/xe/xe_sync_types.h |   1 -
 include/drm/drm_user_fence.h       |  68 +++++++++++++++
 5 files changed, 244 insertions(+), 70 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 include/drm/drm_user_fence.h

-- 
2.34.1

[PATCH 1/2] drm: Add common drm_user_fence helper

[Srinivasan Shanmugam]

Introduce a common DRM user fence helper providing the kref-managed,
MM-borrowing dma-fence-callback-to-workqueue pattern used by drivers
that must access userspace memory from a kthread context when a GPU
fence signals.

XE uses this pattern (xe_sync.c) to write a fence completion value
to a userspace VA. AMDGPU will use the same pattern to signal a
per-queue eventfd from a user-queue EOP fence callback.

The helper provides:
 - struct drm_user_fence: embeddable base structure
 - struct drm_user_fence_ops: worker/destroy callbacks
 - drm_user_fence_init(): initialize and grab the process MM
 - drm_user_fence_get/put(): reference counting
 - drm_user_fence_add_callback(): attach to a dma-fence

The worker callback receives a bool indicating whether the process
MM was successfully obtained, allowing drivers to handle the
unavailable-MM case (log, skip the userspace write, etc.) without
duplicating the mmget/kthread_use_mm/mmput boilerplate.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_user_fence.c | 130 +++++++++++++++++++++++++++++++
 include/drm/drm_user_fence.h     |  68 ++++++++++++++++
 3 files changed, 199 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 include/drm/drm_user_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..52de1f474535 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -69,6 +69,7 @@ drm-y := \
 	drm_syncobj.o \
 	drm_sysfs.o \
 	drm_trace_points.o \
+	drm_user_fence.o \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
new file mode 100644
index 000000000000..bd76e3d03120
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * Common DRM user fence helper.
+ *
+ * When a GPU dma-fence signals, drivers often need to write a value to a
+ * userspace VA or notify userspace via an eventfd. Both operations require
+ * a valid process MM, which is not available in IRQ context.
+ *
+ * This helper queues a work item on fence signal. The work item borrows the
+ * process MM via kthread_use_mm() and calls ops->worker(), which the driver
+ * implements to perform the actual userspace access.
+ */
+
+#include <linux/kthread.h>
+#include <linux/sched/mm.h>
+#include <linux/workqueue.h>
+
+#include <drm/drm_user_fence.h>
+
+static void drm_user_fence_destroy(struct kref *kref)
+{
+	struct drm_user_fence *ufence =
+		container_of(kref, struct drm_user_fence, refcount);
+
+	mmdrop(ufence->mm);
+	ufence->ops->destroy(ufence);
+}
+
+/**
+ * drm_user_fence_get - Acquire a reference to a user fence
+ * @ufence: user fence
+ */
+void drm_user_fence_get(struct drm_user_fence *ufence)
+{
+	kref_get(&ufence->refcount);
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_get);
+
+/**
+ * drm_user_fence_put - Release a reference to a user fence
+ * @ufence: user fence
+ */
+void drm_user_fence_put(struct drm_user_fence *ufence)
+{
+	kref_put(&ufence->refcount, drm_user_fence_destroy);
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_put);
+
+static void drm_user_fence_work(struct work_struct *w)
+{
+	struct drm_user_fence *ufence =
+		container_of(w, struct drm_user_fence, work);
+	bool mm_ok = false;
+
+	if (mmget_not_zero(ufence->mm)) {
+		kthread_use_mm(ufence->mm);
+		mm_ok = true;
+	}
+
+	ufence->ops->worker(ufence, mm_ok);
+
+	if (mm_ok) {
+		kthread_unuse_mm(ufence->mm);
+		mmput(ufence->mm);
+	}
+
+	drm_user_fence_put(ufence);
+}
+
+static void drm_user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+{
+	struct drm_user_fence *ufence =
+		container_of(cb, struct drm_user_fence, cb);
+
+	INIT_WORK(&ufence->work, drm_user_fence_work);
+	queue_work(ufence->wq, &ufence->work);
+}
+
+/**
+ * drm_user_fence_init - Initialize a user fence
+ * @ufence: user fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ *
+ * Must be called from process context. Grabs a reference to current->mm.
+ */
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops)
+{
+	kref_init(&ufence->refcount);
+	ufence->mm = current->mm;
+	mmgrab(ufence->mm);
+	ufence->wq = wq;
+	ufence->ops = ops;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_init);
+
+/**
+ * drm_user_fence_add_callback - Attach a user fence to a dma-fence
+ * @ufence: user fence
+ * @fence: dma-fence to watch; caller retains ownership of this reference
+ *
+ * When @fence signals, a work item is queued that calls ops->worker() with
+ * the process MM active. If @fence has already signaled the work item is
+ * queued immediately.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+				struct dma_fence *fence)
+{
+	int err;
+
+	drm_user_fence_get(ufence);
+	err = dma_fence_add_callback(fence, &ufence->cb, drm_user_fence_cb);
+	if (err == -ENOENT) {
+		/* fence already signaled — queue work immediately */
+		INIT_WORK(&ufence->work, drm_user_fence_work);
+		queue_work(ufence->wq, &ufence->work);
+		err = 0;
+	} else if (err) {
+		drm_user_fence_put(ufence);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_add_callback);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
new file mode 100644
index 000000000000..de8e3f47be18
--- /dev/null
+++ b/include/drm/drm_user_fence.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_USER_FENCE_H__
+#define __DRM_USER_FENCE_H__
+
+#include <linux/dma-fence.h>
+#include <linux/kref.h>
+#include <linux/workqueue.h>
+
+struct drm_user_fence;
+
+/**
+ * struct drm_user_fence_ops - driver callbacks for a DRM user fence
+ */
+struct drm_user_fence_ops {
+	/**
+	 * @worker: Called from workqueue context.
+	 *
+	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
+	 * (copy_to_user, eventfd_signal, etc.) may be accessed safely.
+	 * If @mm_ok is false, the process MM was already gone; the driver
+	 * should log a warning and skip the userspace write.
+	 *
+	 * wake_up() or other post-signal housekeeping should also happen here.
+	 */
+	void (*worker)(struct drm_user_fence *ufence, bool mm_ok);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_user_fence *ufence);
+};
+
+/**
+ * struct drm_user_fence - embeddable DRM user fence
+ *
+ * Drivers embed this in their own structure and implement
+ * &drm_user_fence_ops. Call drm_user_fence_init() at creation and
+ * drm_user_fence_add_callback() to arm on a dma-fence.
+ */
+struct drm_user_fence {
+	/** @refcount: Reference count. */
+	struct kref refcount;
+	/** @mm: Process MM grabbed at init time. */
+	struct mm_struct *mm;
+	/** @work: Work item queued when the dma-fence signals. */
+	struct work_struct work;
+	/** @cb: dma-fence callback. */
+	struct dma_fence_cb cb;
+	/** @wq: Workqueue to run @work on. */
+	struct workqueue_struct *wq;
+	/** @ops: Driver operations. */
+	const struct drm_user_fence_ops *ops;
+};
+
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops);
+void drm_user_fence_get(struct drm_user_fence *ufence);
+void drm_user_fence_put(struct drm_user_fence *ufence);
+int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+				struct dma_fence *fence);
+
+#endif /* __DRM_USER_FENCE_H__ */
-- 
2.34.1

Re: [PATCH 1/2] drm: Add common drm_user_fence helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Lack of a teardown/cancellation API allows Use-After-Free of driver resources and module memory if a fence signals after context teardown.
- [Medium] Deferred dynamic initialization of `work_struct` leads to uninitialized memory access on teardown, data races, and breaks lockdep tracking.
--

commit 4ecbf619a439e885246ba05ae0272714c1c83eb8
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

drm: Add common drm_user_fence helper

Introduce a common DRM user fence helper providing the kref-managed,
MM-borrowing dma-fence-callback-to-workqueue pattern used by drivers
that must access userspace memory from a kthread context when a GPU
fence signals.

> diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
> index 0000000000000..bd76e3d03120e 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_user_fence.c

[ ... ]

> +static void drm_user_fence_work(struct work_struct *w)
> +{
> +	struct drm_user_fence *ufence =
> +		container_of(w, struct drm_user_fence, work);
> +	bool mm_ok = false;
> +
> +	if (mmget_not_zero(ufence->mm)) {
> +		kthread_use_mm(ufence->mm);
> +		mm_ok = true;
> +	}
> +
> +	ufence->ops->worker(ufence, mm_ok);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
If the driver module has been unloaded after tearing down its context, will
this jump into unmapped driver module text and cause a kernel panic?

This can happen if a foreign dma-fence signals after driver teardown, as
there is no way to safely detach or cancel the user fence.

> +
> +	if (mm_ok) {
> +		kthread_unuse_mm(ufence->mm);
> +		mmput(ufence->mm);
> +	}
> +
> +	drm_user_fence_put(ufence);
> +}
> +
> +static void drm_user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
> +{
> +	struct drm_user_fence *ufence =
> +		container_of(cb, struct drm_user_fence, cb);
> +
> +	INIT_WORK(&ufence->work, drm_user_fence_work);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Will calling INIT_WORK() dynamically here instead of during object
initialization hide the work_struct state from teardown paths and break
lockdep tracking?

The work_struct remains completely uninitialized between the call to
drm_user_fence_init() and the fence signaling, preventing the use of standard
workqueue teardown functions prior to signaling.

> +	queue_work(ufence->wq, &ufence->work);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Could this dereference a freed workqueue if a foreign dma-fence signals
after the driver has torn down its context?

Since drm_user_fence_add_callback() takes a kref on the ufence and there is
no cancellation API, the ufence remains alive on the foreign dma-fence's
callback list even after the driver device is unbound or the module is
unloaded.

> +}
> +
> +/**
> + * drm_user_fence_init - Initialize a user fence
> + * @ufence: user fence to initialize
> + * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
> + * @ops: driver operations
> + *
> + * Must be called from process context. Grabs a reference to current->mm.
> + */
> +void drm_user_fence_init(struct drm_user_fence *ufence,
> +			 struct workqueue_struct *wq,
> +			 const struct drm_user_fence_ops *ops)
> +{
> +	kref_init(&ufence->refcount);
> +	ufence->mm = current->mm;
> +	mmgrab(ufence->mm);
> +	ufence->wq = wq;
> +	ufence->ops = ops;
> +}

[Severity: Medium]
Should INIT_WORK() be called here instead?

Initializing the work_struct here would prevent lockdep class divergence and
ensure the work_struct is in a valid state for teardown functions.

> +EXPORT_SYMBOL_GPL(drm_user_fence_init);
> +
> +/**
> + * drm_user_fence_add_callback - Attach a user fence to a dma-fence
> + * @ufence: user fence
> + * @fence: dma-fence to watch; caller retains ownership of this reference
> + *
> + * When @fence signals, a work item is queued that calls ops->worker() with
> + * the process MM active. If @fence has already signaled the work item is
> + * queued immediately.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int drm_user_fence_add_callback(struct drm_user_fence *ufence,
> +				struct dma_fence *fence)
> +{
> +	int err;
> +
> +	drm_user_fence_get(ufence);
> +	err = dma_fence_add_callback(fence, &ufence->cb, drm_user_fence_cb);
> +	if (err == -ENOENT) {
> +		/* fence already signaled — queue work immediately */
> +		INIT_WORK(&ufence->work, drm_user_fence_work);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Does calling INIT_WORK() dynamically in two different locations cause lockdep
class divergence?

Since INIT_WORK() allocates a static lock class key at the call site, the
work_struct will receive different lockdep keys depending on whether the
fence was already signaled or not.

> +		queue_work(ufence->wq, &ufence->work);
> +		err = 0;
> +	} else if (err) {
> +		drm_user_fence_put(ufence);
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(drm_user_fence_add_callback);

> diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
> index 0000000000000..de8e3f47be186 100644
> --- /dev/null
> +++ b/include/drm/drm_user_fence.h

[ ... ]

> +/**
> + * struct drm_user_fence - embeddable DRM user fence
> + *
> + * Drivers embed this in their own structure and implement
> + * &drm_user_fence_ops. Call drm_user_fence_init() at creation and
> + * drm_user_fence_add_callback() to arm on a dma-fence.
> + */
> +struct drm_user_fence {
> +	/** @refcount: Reference count. */
> +	struct kref refcount;
> +	/** @mm: Process MM grabbed at init time. */
> +	struct mm_struct *mm;
> +	/** @work: Work item queued when the dma-fence signals. */
> +	struct work_struct work;
> +	/** @cb: dma-fence callback. */
> +	struct dma_fence_cb cb;
> +	/** @wq: Workqueue to run @work on. */
> +	struct workqueue_struct *wq;
> +	/** @ops: Driver operations. */
> +	const struct drm_user_fence_ops *ops;
> +};

[Severity: High]
Does this API need a cancellation mechanism (e.g., drm_user_fence_cancel())?

Without a way to safely detach the ufence from a foreign dma-fence's
callback list, the ufence remains alive and bound to driver resources even
after the driver tears down its context, which can lead to a use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827062142.4038272-1-srinivasan.shanmugam@amd.com?part=1

[PATCH 2/2] drm/xe: Convert xe_user_fence to drm_user_fence

[Srinivasan Shanmugam]

Replace the open-coded user fence implementation in xe_sync.c with the
new common drm_user_fence helper.

struct xe_user_fence now embeds struct drm_user_fence as its base.
XE-specific fields (xe_device pointer for the ufence_wq wake-up,
userspace VA, expected value, signalled flag) remain in the wrapper.

The local user_fence_destroy/get/put/worker/kick_ufence/user_fence_cb
functions are removed. Their logic moves to xe_ufence_ops.worker and
xe_ufence_ops.destroy, which are called by drm_user_fence_work().

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: intel-xe@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/xe/xe_sync.c       | 114 ++++++++++++-----------------
 drivers/gpu/drm/xe/xe_sync_types.h |   1 -
 2 files changed, 45 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
index 37866768d64c..59ff08010b2f 100644
--- a/drivers/gpu/drm/xe/xe_sync.c
+++ b/drivers/gpu/drm/xe/xe_sync.c
@@ -6,12 +6,11 @@
 #include "xe_sync.h"
 
 #include <linux/dma-fence-array.h>
-#include <linux/kthread.h>
-#include <linux/sched/mm.h>
 #include <linux/uaccess.h>
 
 #include <drm/drm_print.h>
 #include <drm/drm_syncobj.h>
+#include <drm/drm_user_fence.h>
 #include <uapi/drm/xe_drm.h>
 
 #include "xe_device.h"
@@ -19,36 +18,51 @@
 #include "xe_macros.h"
 #include "xe_sched_job_types.h"
 
+/*
+ * xe_user_fence wraps drm_user_fence with XE-specific fields.
+ * The drm_user_fence base handles MM borrowing and work-item lifetime.
+ */
 struct xe_user_fence {
-	struct xe_device *xe;
-	struct kref refcount;
-	struct dma_fence_cb cb;
-	struct work_struct worker;
-	struct mm_struct *mm;
-	u64 __user *addr;
-	u64 value;
-	int signalled;
+	struct drm_user_fence base;
+	struct xe_device     *xe;
+	u64 __user           *addr;
+	u64                   value;
+	int                   signalled;
 };
 
-static void user_fence_destroy(struct kref *kref)
+static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok)
 {
-	struct xe_user_fence *ufence = container_of(kref, struct xe_user_fence,
-						 refcount);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
 
-	mmdrop(ufence->mm);
-	kfree(ufence);
-}
+	/*
+	 * Mark signalled before waking waiters so UMD can safely reuse
+	 * the same ufence without hitting -EBUSY.
+	 */
+	WRITE_ONCE(ufence->signalled, 1);
 
-static void user_fence_get(struct xe_user_fence *ufence)
-{
-	kref_get(&ufence->refcount);
+	if (mm_ok) {
+		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
+			XE_WARN_ON("Copy to user failed");
+	} else {
+		drm_dbg(&ufence->xe->drm,
+			"mmget_not_zero() failed, ufence wasn't signaled\n");
+	}
+
+	wake_up_all(&ufence->xe->ufence_wq);
 }
 
-static void user_fence_put(struct xe_user_fence *ufence)
+static void xe_ufence_destroy(struct drm_user_fence *base)
 {
-	kref_put(&ufence->refcount, user_fence_destroy);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
+
+	kfree(ufence);
 }
 
+static const struct drm_user_fence_ops xe_ufence_ops = {
+	.worker  = xe_ufence_worker,
+	.destroy = xe_ufence_destroy,
+};
+
 static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 					       u64 value)
 {
@@ -63,51 +77,22 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 	if (!ufence)
 		return ERR_PTR(-ENOMEM);
 
-	ufence->xe = xe;
-	kref_init(&ufence->refcount);
-	ufence->addr = ptr;
+	ufence->xe    = xe;
+	ufence->addr  = ptr;
 	ufence->value = value;
-	ufence->mm = current->mm;
-	mmgrab(ufence->mm);
+	drm_user_fence_init(&ufence->base, xe->ordered_wq, &xe_ufence_ops);
 
 	return ufence;
 }
 
-static void user_fence_worker(struct work_struct *w)
-{
-	struct xe_user_fence *ufence = container_of(w, struct xe_user_fence, worker);
-
-	WRITE_ONCE(ufence->signalled, 1);
-	if (mmget_not_zero(ufence->mm)) {
-		kthread_use_mm(ufence->mm);
-		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
-			XE_WARN_ON("Copy to user failed");
-		kthread_unuse_mm(ufence->mm);
-		mmput(ufence->mm);
-	} else {
-		drm_dbg(&ufence->xe->drm, "mmget_not_zero() failed, ufence wasn't signaled\n");
-	}
-
-	/*
-	 * Wake up waiters only after updating the ufence state, allowing the UMD
-	 * to safely reuse the same ufence without encountering -EBUSY errors.
-	 */
-	wake_up_all(&ufence->xe->ufence_wq);
-	user_fence_put(ufence);
-}
-
-static void kick_ufence(struct xe_user_fence *ufence, struct dma_fence *fence)
+static void user_fence_get(struct xe_user_fence *ufence)
 {
-	INIT_WORK(&ufence->worker, user_fence_worker);
-	queue_work(ufence->xe->ordered_wq, &ufence->worker);
-	dma_fence_put(fence);
+	drm_user_fence_get(&ufence->base);
 }
 
-static void user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+static void user_fence_put(struct xe_user_fence *ufence)
 {
-	struct xe_user_fence *ufence = container_of(cb, struct xe_user_fence, cb);
-
-	kick_ufence(ufence, fence);
+	drm_user_fence_put(&ufence->base);
 }
 
 int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
@@ -282,24 +267,15 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
 	} else if (sync->syncobj) {
 		drm_syncobj_replace_fence(sync->syncobj, fence);
 	} else if (sync->ufence) {
-		int err;
-
 		drm_syncobj_add_point(sync->ufence_syncobj,
 				      sync->ufence_chain_fence,
 				      fence, sync->ufence_timeline_value);
 		sync->ufence_chain_fence = NULL;
 
 		fence = drm_syncobj_fence_get(sync->ufence_syncobj);
-		user_fence_get(sync->ufence);
-		err = dma_fence_add_callback(fence, &sync->ufence->cb,
-					     user_fence_cb);
-		if (err == -ENOENT) {
-			kick_ufence(sync->ufence, fence);
-		} else if (err) {
+		if (drm_user_fence_add_callback(&sync->ufence->base, fence))
 			XE_WARN_ON("failed to add user fence");
-			user_fence_put(sync->ufence);
-			dma_fence_put(fence);
-		}
+		dma_fence_put(fence);
 	}
 }
 
@@ -443,4 +419,3 @@ void xe_sync_ufence_put(struct xe_user_fence *ufence)
 int xe_sync_ufence_get_status(struct xe_user_fence *ufence)
 {
 	return READ_ONCE(ufence->signalled);
-}
+}
diff --git a/drivers/gpu/drm/xe/xe_sync_types.h b/drivers/gpu/drm/xe/xe_sync_types.h
index b88f1833e28c..67048199b070 100644
--- a/drivers/gpu/drm/xe/xe_sync_types.h
+++ b/drivers/gpu/drm/xe/xe_sync_types.h
@@ -12,7 +12,6 @@ struct drm_syncobj;
 struct dma_fence;
 struct dma_fence_chain;
 struct drm_xe_sync;
-struct user_fence;
 
 struct xe_sync_entry {
 	struct drm_syncobj *syncobj;
-- 
2.34.1

Re: [PATCH 2/2] drm/xe: Convert xe_user_fence to drm_user_fence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Prematurely dropping the dma_fence reference allows userspace to trigger a kernel WARNING and memory leak via syncobj reset.
--

commit c721250695d79a85665bd6cb949b881db777982f
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

    drm/xe: Convert xe_user_fence to drm_user_fence
    
    Replace the open-coded user fence implementation in xe_sync.c with the
    new common drm_user_fence helper.

> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> index 37866768d64c2..e95f50338454c 100644
> --- a/drivers/gpu/drm/xe/xe_sync.c
> +++ b/drivers/gpu/drm/xe/xe_sync.c
[ ... ]
> @@ -282,24 +267,15 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
>  	} else if (sync->syncobj) {
>  		drm_syncobj_replace_fence(sync->syncobj, fence);
>  	} else if (sync->ufence) {
> -		int err;
> -
>  		drm_syncobj_add_point(sync->ufence_syncobj,
>  				      sync->ufence_chain_fence,
>  				      fence, sync->ufence_timeline_value);
>  		sync->ufence_chain_fence = NULL;
>  
>  		fence = drm_syncobj_fence_get(sync->ufence_syncobj);
> -		user_fence_get(sync->ufence);
> -		err = dma_fence_add_callback(fence, &sync->ufence->cb,
> -					     user_fence_cb);
> -		if (err == -ENOENT) {
> -			kick_ufence(sync->ufence, fence);
> -		} else if (err) {
> +		if (drm_user_fence_add_callback(&sync->ufence->base, fence))
>  			XE_WARN_ON("failed to add user fence");
> -			user_fence_put(sync->ufence);
> -			dma_fence_put(fence);
> -		}
> +		dma_fence_put(fence);
>  	}
>  }

[Severity: High]
Does this code prematurely drop the local fence reference? 

In xe_sync_entry_signal(), we add the user fence callback using 
drm_user_fence_add_callback(), but then immediately call dma_fence_put() 
unconditionally.

If userspace submits a job with a user fence and then subsequently resets 
the syncobj before the fence signals, the syncobj will drop its reference. 
Since we already dropped our local reference here, could the fence be 
destroyed while the callback is still pending? 

This looks like it would trigger a kernel warning from the non-empty callback 
list in dma_fence_release and permanently leak the user fence object since 
the callback is never executed. The previous implementation appeared to avoid 
this by holding the fence reference until the callback actually fired.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827062142.4038272-1-srinivasan.shanmugam@amd.com?part=2

✗ CI.checkpatch: warning for drm: Add Common drm_user_fence helper and Convert XE

[Patchwork]

== Series Details ==

Series: drm: Add Common drm_user_fence helper and Convert XE
URL   : https://patchwork.freedesktop.org/series/172833/
State : warning

== Summary ==

+ KERNEL=/kernel
+ git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt
Cloning into 'mt'...
warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/
+ git -C mt rev-list -n1 origin/master
061140b9bc586ae7f40abc1249c97e1cc72d1b9d
+ cd /kernel
+ git config --global --add safe.directory /kernel
+ git log -n1
commit 3dde61fb11022c0746106ddc338ce6572f28b6ee
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Date:   Thu Aug 27 11:51:42 2026 +0530

    drm/xe: Convert xe_user_fence to drm_user_fence
    
    Replace the open-coded user fence implementation in xe_sync.c with the
    new common drm_user_fence helper.
    
    struct xe_user_fence now embeds struct drm_user_fence as its base.
    XE-specific fields (xe_device pointer for the ufence_wq wake-up,
    userspace VA, expected value, signalled flag) remain in the wrapper.
    
    The local user_fence_destroy/get/put/worker/kick_ufence/user_fence_cb
    functions are removed. Their logic moves to xe_ufence_ops.worker and
    xe_ufence_ops.destroy, which are called by drm_user_fence_work().
    
    Cc: Matthew Brost <matthew.brost@intel.com>
    Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
    Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
    Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
    Cc: David Airlie <airlied@gmail.com>
    Cc: Simona Vetter <simona@ffwll.ch>
    Cc: Sumit Semwal <sumit.semwal@linaro.org>
    Cc: Christian König <christian.koenig@amd.com>
    Cc: Alex Deucher <alexander.deucher@amd.com>
    Cc: intel-xe@lists.freedesktop.org
    Cc: dri-devel@lists.freedesktop.org
    Cc: linux-media@vger.kernel.org
    Cc: linaro-mm-sig@lists.linaro.org
    Cc: linux-kernel@vger.kernel.org
    Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
+ /mt/dim checkpatch acd8191c76e28f9b260f6701573fd1d2c76dd0f6 drm-intel
9e629553bfd3 drm: Add common drm_user_fence helper
-:61: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#61: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 205 lines checked
3dde61fb1102 drm/xe: Convert xe_user_fence to drm_user_fence

✗ CI.KUnit: failure for drm: Add Common drm_user_fence helper and Convert XE

[Patchwork]

== Series Details ==

Series: drm: Add Common drm_user_fence helper and Convert XE
URL   : https://patchwork.freedesktop.org/series/172833/
State : failure

== Summary ==

+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
ERROR:root:../drivers/gpu/drm/xe/xe_sync.c: In function ‘xe_sync_ufence_get_status’:
../drivers/gpu/drm/xe/xe_sync.c:421:9: error: expected declaration or statement at end of input
  421 |         return READ_ONCE(ufence->signalled);
      |         ^~~~~~
make[7]: *** [../scripts/Makefile.build:289: drivers/gpu/drm/xe/xe_sync.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[6]: *** [../scripts/Makefile.build:549: drivers/gpu/drm/xe] Error 2
make[5]: *** [../scripts/Makefile.build:549: drivers/gpu/drm] Error 2
make[4]: *** [../scripts/Makefile.build:549: drivers/gpu] Error 2
make[3]: *** [../scripts/Makefile.build:549: drivers] Error 2
make[2]: *** [/kernel/Makefile:2187: .] Error 2
make[1]: *** [/kernel/Makefile:248: __sub-make] Error 2
make: *** [Makefile:248: __sub-make] Error 2

[06:28:50] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[06:28:55] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel

[PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE

[Srinivasan Shanmugam]

When a GPU dma-fence signals, drivers often need to perform work that
cannot run in IRQ context. This pattern is currently open-coded in
multiple drivers.

This series introduces two layered helpers:

Patch 1 introduces drm_work_fence — a generic embeddable base structure
that handles the dma-fence-callback-to-workqueue pattern. Any driver
needing deferred fence work can use this directly.

Patch 2 introduces drm_user_fence — a thin layer on top of
drm_work_fence that adds kthread_use_mm() support for drivers that need
to access userspace memory when a fence signals.

Patch 3 converts XE to use drm_user_fence. XE continues to write a
fence completion value to a userspace VA using the new helper.

Patch 4 adds optional per-signal compare functionality to drm_user_fence.
When cmp_addr is set, the worker is called only if the value at cmp_addr
satisfies the configured comparison. This enables AMDGPU's EOP eventfd
per-signal filtering without open-coding the read+compare pattern.

A follow-on patch (not in this series) will wire AMDGPU's render-node
EOP eventfd signaling path to drm_work_fence.

v5:
 - Split drm_user_fence into drm_work_fence (generic) and drm_user_fence
   (MM-borrowing subclass) per Matthew Brost's suggestion.
 - Add per-signal compare functionality (drm_user_fence_set_compare())
   per Christian König's suggestion.
 - Use mmput_async() instead of mmput() to avoid potential deadlock in
   MMU notifier release path. (Sashiko review)

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org

Srinivasan Shanmugam (4):
  drm: Add drm_work_fence helper
  drm: Add drm_user_fence helper
  drm/xe: Convert xe_user_fence to drm_user_fence
  drm: Add per-signal compare functionality to drm_user_fence

 drivers/gpu/drm/Makefile           |   2 +
 drivers/gpu/drm/drm_user_fence.c   | 147 ++++++++++++++++++++++
 drivers/gpu/drm/drm_work_fence.c   | 195 +++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_sync.c       | 149 ++++++++++++----------
 drivers/gpu/drm/xe/xe_sync.h       |   2 +
 drivers/gpu/drm/xe/xe_sync_types.h |   1 -
 drivers/gpu/drm/xe/xe_vm.c         |   1 +
 include/drm/drm_user_fence.h       | 115 +++++++++++++++++
 include/drm/drm_work_fence.h       |  76 +++++++++++
 9 files changed, 619 insertions(+), 69 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 drivers/gpu/drm/drm_work_fence.c
 create mode 100644 include/drm/drm_user_fence.h
 create mode 100644 include/drm/drm_work_fence.h

-- 
2.34.1

Re: [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE

[Thomas Hellström]

On Mon, 2026-08-31 at 11:11 +0530, Srinivasan Shanmugam wrote:
> When a GPU dma-fence signals, drivers often need to perform work that
> cannot run in IRQ context. This pattern is currently open-coded in
> multiple drivers.
> 
> This series introduces two layered helpers:
> 
> Patch 1 introduces drm_work_fence — a generic embeddable base
> structure
> that handles the dma-fence-callback-to-workqueue pattern. Any driver
> needing deferred fence work can use this directly.
> 
> Patch 2 introduces drm_user_fence — a thin layer on top of
> drm_work_fence that adds kthread_use_mm() support for drivers that
> need
> to access userspace memory when a fence signals.
> 
> Patch 3 converts XE to use drm_user_fence. XE continues to write a
> fence completion value to a userspace VA using the new helper.
> 
> Patch 4 adds optional per-signal compare functionality to
> drm_user_fence.
> When cmp_addr is set, the worker is called only if the value at
> cmp_addr
> satisfies the configured comparison. This enables AMDGPU's EOP
> eventfd
> per-signal filtering without open-coding the read+compare pattern.
> 
> A follow-on patch (not in this series) will wire AMDGPU's render-node
> EOP eventfd signaling path to drm_work_fence.
> 
> v5:
>  - Split drm_user_fence into drm_work_fence (generic) and
> drm_user_fence
>    (MM-borrowing subclass) per Matthew Brost's suggestion.
>  - Add per-signal compare functionality
> (drm_user_fence_set_compare())
>    per Christian König's suggestion.
>  - Use mmput_async() instead of mmput() to avoid potential deadlock
> in
>    MMU notifier release path. (Sashiko review)
> 
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org

I think the get_user() and put_user() of 64-bit values in drm (driver
common) code is not safe for typical use-cases on 32-bit systems. For
xe we officially don't (yet at least) support 32-bit systems so hence
the code is a bit sloppy but for drm helpers I'm not sure we can get
away with this. At least not without some form of warning or assert.

I think to make 32-bit systems 64-bit user-fence safe, we would need to
user pin_user_pages() combined with cmpxchg64() and a similar cmpxchg
operation on the user-space side.

Thanks,
Thomas





> 
> Srinivasan Shanmugam (4):
>   drm: Add drm_work_fence helper
>   drm: Add drm_user_fence helper
>   drm/xe: Convert xe_user_fence to drm_user_fence
>   drm: Add per-signal compare functionality to drm_user_fence
> 
>  drivers/gpu/drm/Makefile           |   2 +
>  drivers/gpu/drm/drm_user_fence.c   | 147 ++++++++++++++++++++++
>  drivers/gpu/drm/drm_work_fence.c   | 195
> +++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_sync.c       | 149 ++++++++++++----------
>  drivers/gpu/drm/xe/xe_sync.h       |   2 +
>  drivers/gpu/drm/xe/xe_sync_types.h |   1 -
>  drivers/gpu/drm/xe/xe_vm.c         |   1 +
>  include/drm/drm_user_fence.h       | 115 +++++++++++++++++
>  include/drm/drm_work_fence.h       |  76 +++++++++++
>  9 files changed, 619 insertions(+), 69 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_user_fence.c
>  create mode 100644 drivers/gpu/drm/drm_work_fence.c
>  create mode 100644 include/drm/drm_user_fence.h
>  create mode 100644 include/drm/drm_work_fence.h

RE: [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE

[SHANMUGAM, SRINIVASAN]

AMD General

> -----Original Message-----
> From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Sent: Monday, August 31, 2026 3:46 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Matthew Brost <matthew.brost@intel.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Mika Kuoppala
> <mika.kuoppala@linux.intel.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org
> Subject: Re: [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence
> helpers and convert XE
>
> On Mon, 2026-08-31 at 11:11 +0530, Srinivasan Shanmugam wrote:
> > When a GPU dma-fence signals, drivers often need to perform work that
> > cannot run in IRQ context. This pattern is currently open-coded in
> > multiple drivers.
> >
> > This series introduces two layered helpers:
> >
> > Patch 1 introduces drm_work_fence — a generic embeddable base
> > structure that handles the dma-fence-callback-to-workqueue pattern.
> > Any driver needing deferred fence work can use this directly.
> >
> > Patch 2 introduces drm_user_fence — a thin layer on top of
> > drm_work_fence that adds kthread_use_mm() support for drivers that
> > need to access userspace memory when a fence signals.
> >
> > Patch 3 converts XE to use drm_user_fence. XE continues to write a
> > fence completion value to a userspace VA using the new helper.
> >
> > Patch 4 adds optional per-signal compare functionality to
> > drm_user_fence.
> > When cmp_addr is set, the worker is called only if the value at
> > cmp_addr satisfies the configured comparison. This enables AMDGPU's
> > EOP eventfd per-signal filtering without open-coding the read+compare
> > pattern.
> >
> > A follow-on patch (not in this series) will wire AMDGPU's render-node
> > EOP eventfd signaling path to drm_work_fence.
> >
> > v5:
> >  - Split drm_user_fence into drm_work_fence (generic) and
> > drm_user_fence
> >    (MM-borrowing subclass) per Matthew Brost's suggestion.
> >  - Add per-signal compare functionality
> > (drm_user_fence_set_compare())
> >    per Christian König's suggestion.
> >  - Use mmput_async() instead of mmput() to avoid potential deadlock in
> >    MMU notifier release path. (Sashiko review)
> >
> > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > Suggested-by: Christian König <christian.koenig@amd.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: intel-xe@lists.freedesktop.org
> > Cc: amd-gfx@lists.freedesktop.org
>
> I think the get_user() and put_user() of 64-bit values in drm (driver
> common) code is not safe for typical use-cases on 32-bit systems. For xe we
> officially don't (yet at least) support 32-bit systems so hence the code is a bit sloppy
> but for drm helpers I'm not sure we can get away with this. At least not without some
> form of warning or assert.
>
> I think to make 32-bit systems 64-bit user-fence safe, we would need to user
> pin_user_pages() combined with cmpxchg64() and a similar cmpxchg operation on
> the user-space side.

Hi Thomas,

Thanks for the review.

For the 32-bit safety concern on get_user() of u64 values —
since no current GPU driver supports 32-bit user fences
(XE explicitly excludes 32-bit, and AMDGPU targets modern hardware),
would adding a BUILD_BUG_ON or IS_ENABLED(CONFIG_64BIT) guard in
drm_user_fence_set_compare() be acceptable for now?

If a 32-bit driver ever needs this in the future, we can follow up
with pin_user_pages() + cmpxchg64() for proper atomic access.

Does that approach work for you?

Thanks,
Srini

Re: [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE

[Thomas Hellström]

On Mon, 2026-08-31 at 11:13 +0000, SHANMUGAM, SRINIVASAN wrote:
> AMD General
> 
> > -----Original Message-----
> > From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Sent: Monday, August 31, 2026 3:46 PM
> > To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> > Matthew Brost <matthew.brost@intel.com>; Koenig, Christian
> > <Christian.Koenig@amd.com>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org; Mika Kuoppala
> > <mika.kuoppala@linux.intel.com>; Maarten Lankhorst
> > <maarten.lankhorst@linux.intel.com>;
> > dri-devel@lists.freedesktop.org; intel-
> > xe@lists.freedesktop.org
> > Subject: Re: [PATCH v5 0/4] drm: Add common
> > drm_work_fence/drm_user_fence
> > helpers and convert XE
> > 
> > On Mon, 2026-08-31 at 11:11 +0530, Srinivasan Shanmugam wrote:
> > > When a GPU dma-fence signals, drivers often need to perform work
> > > that
> > > cannot run in IRQ context. This pattern is currently open-coded
> > > in
> > > multiple drivers.
> > > 
> > > This series introduces two layered helpers:
> > > 
> > > Patch 1 introduces drm_work_fence — a generic embeddable base
> > > structure that handles the dma-fence-callback-to-workqueue
> > > pattern.
> > > Any driver needing deferred fence work can use this directly.
> > > 
> > > Patch 2 introduces drm_user_fence — a thin layer on top of
> > > drm_work_fence that adds kthread_use_mm() support for drivers
> > > that
> > > need to access userspace memory when a fence signals.
> > > 
> > > Patch 3 converts XE to use drm_user_fence. XE continues to write
> > > a
> > > fence completion value to a userspace VA using the new helper.
> > > 
> > > Patch 4 adds optional per-signal compare functionality to
> > > drm_user_fence.
> > > When cmp_addr is set, the worker is called only if the value at
> > > cmp_addr satisfies the configured comparison. This enables
> > > AMDGPU's
> > > EOP eventfd per-signal filtering without open-coding the
> > > read+compare
> > > pattern.
> > > 
> > > A follow-on patch (not in this series) will wire AMDGPU's render-
> > > node
> > > EOP eventfd signaling path to drm_work_fence.
> > > 
> > > v5:
> > >  - Split drm_user_fence into drm_work_fence (generic) and
> > > drm_user_fence
> > >    (MM-borrowing subclass) per Matthew Brost's suggestion.
> > >  - Add per-signal compare functionality
> > > (drm_user_fence_set_compare())
> > >    per Christian König's suggestion.
> > >  - Use mmput_async() instead of mmput() to avoid potential
> > > deadlock in
> > >    MMU notifier release path. (Sashiko review)
> > > 
> > > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > > Suggested-by: Christian König <christian.koenig@amd.com>
> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: intel-xe@lists.freedesktop.org
> > > Cc: amd-gfx@lists.freedesktop.org
> > 
> > I think the get_user() and put_user() of 64-bit values in drm
> > (driver
> > common) code is not safe for typical use-cases on 32-bit systems.
> > For xe we
> > officially don't (yet at least) support 32-bit systems so hence the
> > code is a bit sloppy
> > but for drm helpers I'm not sure we can get away with this. At
> > least not without some
> > form of warning or assert.
> > 
> > I think to make 32-bit systems 64-bit user-fence safe, we would
> > need to user
> > pin_user_pages() combined with cmpxchg64() and a similar cmpxchg
> > operation on
> > the user-space side.
> 
> Hi Thomas,
> 
> Thanks for the review.
> 
> For the 32-bit safety concern on get_user() of u64 values —
> since no current GPU driver supports 32-bit user fences
> (XE explicitly excludes 32-bit, and AMDGPU targets modern hardware),
> would adding a BUILD_BUG_ON or IS_ENABLED(CONFIG_64BIT) guard in
> drm_user_fence_set_compare() be acceptable for now?
> 
> If a 32-bit driver ever needs this in the future, we can follow up
> with pin_user_pages() + cmpxchg64() for proper atomic access.
> 
> Does that approach work for you?

Xe supports building on 32-bit but not running. Can we use a
drm_WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT)) or similar somewhere?
Perhaps that was your second suggestion?

Thanks,
Thomas


> 
> Thanks,
> Srini

RE: [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE

[SHANMUGAM, SRINIVASAN]

AMD General

> -----Original Message-----
> From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Sent: Monday, August 31, 2026 5:52 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Matthew Brost <matthew.brost@intel.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Mika Kuoppala
> <mika.kuoppala@linux.intel.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org
> Subject: Re: [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence
> helpers and convert XE
>
> On Mon, 2026-08-31 at 11:13 +0000, SHANMUGAM, SRINIVASAN wrote:
> > AMD General
> >
> > > -----Original Message-----
> > > From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > Sent: Monday, August 31, 2026 3:46 PM
> > > To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Matthew
> > > Brost <matthew.brost@intel.com>; Koenig, Christian
> > > <Christian.Koenig@amd.com>; Deucher, Alexander
> > > <Alexander.Deucher@amd.com>
> > > Cc: amd-gfx@lists.freedesktop.org; Mika Kuoppala
> > > <mika.kuoppala@linux.intel.com>; Maarten Lankhorst
> > > <maarten.lankhorst@linux.intel.com>;
> > > dri-devel@lists.freedesktop.org; intel- xe@lists.freedesktop.org
> > > Subject: Re: [PATCH v5 0/4] drm: Add common
> > > drm_work_fence/drm_user_fence helpers and convert XE
> > >
> > > On Mon, 2026-08-31 at 11:11 +0530, Srinivasan Shanmugam wrote:
> > > > When a GPU dma-fence signals, drivers often need to perform work
> > > > that cannot run in IRQ context. This pattern is currently
> > > > open-coded in multiple drivers.
> > > >
> > > > This series introduces two layered helpers:
> > > >
> > > > Patch 1 introduces drm_work_fence — a generic embeddable base
> > > > structure that handles the dma-fence-callback-to-workqueue
> > > > pattern.
> > > > Any driver needing deferred fence work can use this directly.
> > > >
> > > > Patch 2 introduces drm_user_fence — a thin layer on top of
> > > > drm_work_fence that adds kthread_use_mm() support for drivers that
> > > > need to access userspace memory when a fence signals.
> > > >
> > > > Patch 3 converts XE to use drm_user_fence. XE continues to write a
> > > > fence completion value to a userspace VA using the new helper.
> > > >
> > > > Patch 4 adds optional per-signal compare functionality to
> > > > drm_user_fence.
> > > > When cmp_addr is set, the worker is called only if the value at
> > > > cmp_addr satisfies the configured comparison. This enables
> > > > AMDGPU's EOP eventfd per-signal filtering without open-coding the
> > > > read+compare
> > > > pattern.
> > > >
> > > > A follow-on patch (not in this series) will wire AMDGPU's render-
> > > > node EOP eventfd signaling path to drm_work_fence.
> > > >
> > > > v5:
> > > >  - Split drm_user_fence into drm_work_fence (generic) and
> > > > drm_user_fence
> > > >    (MM-borrowing subclass) per Matthew Brost's suggestion.
> > > >  - Add per-signal compare functionality
> > > > (drm_user_fence_set_compare())
> > > >    per Christian König's suggestion.
> > > >  - Use mmput_async() instead of mmput() to avoid potential
> > > > deadlock in
> > > >    MMU notifier release path. (Sashiko review)
> > > >
> > > > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > > > Suggested-by: Christian König <christian.koenig@amd.com>
> > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc: intel-xe@lists.freedesktop.org
> > > > Cc: amd-gfx@lists.freedesktop.org
> > >
> > > I think the get_user() and put_user() of 64-bit values in drm
> > > (driver
> > > common) code is not safe for typical use-cases on 32-bit systems.
> > > For xe we
> > > officially don't (yet at least) support 32-bit systems so hence the
> > > code is a bit sloppy but for drm helpers I'm not sure we can get
> > > away with this. At least not without some form of warning or assert.
> > >
> > > I think to make 32-bit systems 64-bit user-fence safe, we would need
> > > to user
> > > pin_user_pages() combined with cmpxchg64() and a similar cmpxchg
> > > operation on the user-space side.
> >
> > Hi Thomas,
> >
> > Thanks for the review.
> >
> > For the 32-bit safety concern on get_user() of u64 values — since no
> > current GPU driver supports 32-bit user fences (XE explicitly excludes
> > 32-bit, and AMDGPU targets modern hardware), would adding a
> > BUILD_BUG_ON or IS_ENABLED(CONFIG_64BIT) guard in
> > drm_user_fence_set_compare() be acceptable for now?
> >
> > If a 32-bit driver ever needs this in the future, we can follow up
> > with pin_user_pages() + cmpxchg64() for proper atomic access.
> >
> > Does that approach work for you?
>
> Xe supports building on 32-bit but not running. Can we use a
> drm_WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT)) or similar somewhere?
> Perhaps that was your second suggestion?

Hi Thomas,

Yes, that matches our suggestion. We will add:

    WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT));

in drm_user_fence_set_compare(). We cannot use drm_WARN_ON_ONCE()
since drm_user_fence has no struct drm_device * reference.

Is plain WARN_ON_ONCE acceptable, or should we add a drm_device
pointer to drm_user_fence_set_compare() to use drm_WARN_ON_ONCE()?

Thanks,
Srini

Re: [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE

[Thomas Hellström]

On Mon, 2026-08-31 at 12:36 +0000, SHANMUGAM, SRINIVASAN wrote:
> AMD General
> 
> > -----Original Message-----
> > From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Sent: Monday, August 31, 2026 5:52 PM
> > To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> > Matthew Brost <matthew.brost@intel.com>; Koenig, Christian
> > <Christian.Koenig@amd.com>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org; Mika Kuoppala
> > <mika.kuoppala@linux.intel.com>; Maarten Lankhorst
> > <maarten.lankhorst@linux.intel.com>;
> > dri-devel@lists.freedesktop.org; intel-
> > xe@lists.freedesktop.org
> > Subject: Re: [PATCH v5 0/4] drm: Add common
> > drm_work_fence/drm_user_fence
> > helpers and convert XE
> > 
> > On Mon, 2026-08-31 at 11:13 +0000, SHANMUGAM, SRINIVASAN wrote:
> > > AMD General
> > > 
> > > > -----Original Message-----
> > > > From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > > Sent: Monday, August 31, 2026 3:46 PM
> > > > To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> > Matthew
> > > > Brost <matthew.brost@intel.com>; Koenig, Christian
> > > > <Christian.Koenig@amd.com>; Deucher, Alexander
> > > > <Alexander.Deucher@amd.com>
> > > > Cc: amd-gfx@lists.freedesktop.org; Mika Kuoppala
> > > > <mika.kuoppala@linux.intel.com>; Maarten Lankhorst
> > > > <maarten.lankhorst@linux.intel.com>;
> > > > dri-devel@lists.freedesktop.org; intel-
> > > > xe@lists.freedesktop.org
> > > > Subject: Re: [PATCH v5 0/4] drm: Add common
> > > > drm_work_fence/drm_user_fence helpers and convert XE
> > > > 
> > > > On Mon, 2026-08-31 at 11:11 +0530, Srinivasan Shanmugam wrote:
> > > > > When a GPU dma-fence signals, drivers often need to perform
> > > > > work
> > > > > that cannot run in IRQ context. This pattern is currently
> > > > > open-coded in multiple drivers.
> > > > > 
> > > > > This series introduces two layered helpers:
> > > > > 
> > > > > Patch 1 introduces drm_work_fence — a generic embeddable base
> > > > > structure that handles the dma-fence-callback-to-workqueue
> > > > > pattern.
> > > > > Any driver needing deferred fence work can use this directly.
> > > > > 
> > > > > Patch 2 introduces drm_user_fence — a thin layer on top of
> > > > > drm_work_fence that adds kthread_use_mm() support for drivers
> > > > > that
> > > > > need to access userspace memory when a fence signals.
> > > > > 
> > > > > Patch 3 converts XE to use drm_user_fence. XE continues to
> > > > > write a
> > > > > fence completion value to a userspace VA using the new
> > > > > helper.
> > > > > 
> > > > > Patch 4 adds optional per-signal compare functionality to
> > > > > drm_user_fence.
> > > > > When cmp_addr is set, the worker is called only if the value
> > > > > at
> > > > > cmp_addr satisfies the configured comparison. This enables
> > > > > AMDGPU's EOP eventfd per-signal filtering without open-coding
> > > > > the
> > > > > read+compare
> > > > > pattern.
> > > > > 
> > > > > A follow-on patch (not in this series) will wire AMDGPU's
> > > > > render-
> > > > > node EOP eventfd signaling path to drm_work_fence.
> > > > > 
> > > > > v5:
> > > > >  - Split drm_user_fence into drm_work_fence (generic) and
> > > > > drm_user_fence
> > > > >    (MM-borrowing subclass) per Matthew Brost's suggestion.
> > > > >  - Add per-signal compare functionality
> > > > > (drm_user_fence_set_compare())
> > > > >    per Christian König's suggestion.
> > > > >  - Use mmput_async() instead of mmput() to avoid potential
> > > > > deadlock in
> > > > >    MMU notifier release path. (Sashiko review)
> > > > > 
> > > > > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > > > > Suggested-by: Christian König <christian.koenig@amd.com>
> > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Cc: intel-xe@lists.freedesktop.org
> > > > > Cc: amd-gfx@lists.freedesktop.org
> > > > 
> > > > I think the get_user() and put_user() of 64-bit values in drm
> > > > (driver
> > > > common) code is not safe for typical use-cases on 32-bit
> > > > systems.
> > > > For xe we
> > > > officially don't (yet at least) support 32-bit systems so hence
> > > > the
> > > > code is a bit sloppy but for drm helpers I'm not sure we can
> > > > get
> > > > away with this. At least not without some form of warning or
> > > > assert.
> > > > 
> > > > I think to make 32-bit systems 64-bit user-fence safe, we would
> > > > need
> > > > to user
> > > > pin_user_pages() combined with cmpxchg64() and a similar
> > > > cmpxchg
> > > > operation on the user-space side.
> > > 
> > > Hi Thomas,
> > > 
> > > Thanks for the review.
> > > 
> > > For the 32-bit safety concern on get_user() of u64 values — since
> > > no
> > > current GPU driver supports 32-bit user fences (XE explicitly
> > > excludes
> > > 32-bit, and AMDGPU targets modern hardware), would adding a
> > > BUILD_BUG_ON or IS_ENABLED(CONFIG_64BIT) guard in
> > > drm_user_fence_set_compare() be acceptable for now?
> > > 
> > > If a 32-bit driver ever needs this in the future, we can follow
> > > up
> > > with pin_user_pages() + cmpxchg64() for proper atomic access.
> > > 
> > > Does that approach work for you?
> > 
> > Xe supports building on 32-bit but not running. Can we use a
> > drm_WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT)) or similar somewhere?
> > Perhaps that was your second suggestion?
> 
> Hi Thomas,
> 
> Yes, that matches our suggestion. We will add:
> 
>     WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT));
> 
> in drm_user_fence_set_compare(). We cannot use drm_WARN_ON_ONCE()
> since drm_user_fence has no struct drm_device * reference.
> 
> Is plain WARN_ON_ONCE acceptable, or should we add a drm_device
> pointer to drm_user_fence_set_compare() to use drm_WARN_ON_ONCE()?

For this purpose, IMO WARN_ON_ONCE() is fine. Not sure if drm has a
general recommendation to add a device pointer, though.

Thanks,
Thomas


> 
> Thanks,
> Srini

[PATCH v5 1/4] drm: Add drm_work_fence helper

[Srinivasan Shanmugam]

GPU drivers often need to queue work when a dma-fence signals
because certain operations (copy_to_user, eventfd_signal, memory
allocation) cannot run in IRQ context. This pattern is currently
open-coded in multiple drivers.

Introduce drm_work_fence — an embeddable base structure that handles
the dma-fence-callback-to-workqueue pattern in one place. Drivers
embed this in their own structure and implement ops->work() for the
deferred work and ops->destroy() for cleanup.

The helper manages:
 - kref lifetime
 - dma-fence callback registration
 - workqueue dispatch on fence signal
 - safe cancellation before driver teardown

For work that additionally requires borrowing the process MM via
kthread_use_mm(), see drm_user_fence which builds on top of this.

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_work_fence.c | 195 +++++++++++++++++++++++++++++++
 include/drm/drm_work_fence.h     |  76 ++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_work_fence.c
 create mode 100644 include/drm/drm_work_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..c5be8e80d0c8 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -72,6 +72,7 @@ drm-y := \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
+	drm_work_fence.o \
 	drm_writeback.o
 drm-$(CONFIG_DRM_CLIENT) += \
 	drm_client.o \
diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
new file mode 100644
index 000000000000..9f6b779d0fe9
--- /dev/null
+++ b/drivers/gpu/drm/drm_work_fence.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * Common DRM work fence helper.
+ *
+ * When a GPU dma-fence signals, drivers often need to perform work that
+ * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
+ * eventfd_signal). This helper queues a work item when a dma-fence
+ * signals, allowing that work to run safely in a workqueue context.
+ *
+ * NOTE: This helper consumes dma_fences but CANNOT implement
+ * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
+ * callbacks are called under the fence spinlock and must not sleep.
+ *
+ * For work that additionally requires accessing userspace memory via
+ * kthread_use_mm(), see drm_user_fence which builds on top of this.
+ */
+
+#include <linux/workqueue.h>
+
+#include <drm/drm_work_fence.h>
+
+static void drm_work_fence_destroy(struct kref *kref)
+{
+	struct drm_work_fence *wfence =
+		container_of(kref, struct drm_work_fence, refcount);
+
+	if (wfence->fence)
+		dma_fence_put(wfence->fence);
+
+	wfence->ops->destroy(wfence);
+}
+
+/**
+ * drm_work_fence_get - Acquire a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_get(struct drm_work_fence *wfence)
+{
+	kref_get(&wfence->refcount);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_get);
+
+/**
+ * drm_work_fence_put - Release a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_put(struct drm_work_fence *wfence)
+{
+	kref_put(&wfence->refcount, drm_work_fence_destroy);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_put);
+
+static void drm_work_fence_work(struct work_struct *w)
+{
+	struct drm_work_fence *wfence =
+		container_of(w, struct drm_work_fence, work);
+
+	wfence->ops->work(wfence);
+	drm_work_fence_put(wfence);
+}
+
+static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+{
+	struct drm_work_fence *wfence =
+		container_of(cb, struct drm_work_fence, cb);
+
+	queue_work(wfence->wq, &wfence->work);
+	/*
+	 * Put the transferred reference from add_callback. The stored
+	 * reference in wfence->fence is released in drm_work_fence_destroy().
+	 */
+	dma_fence_put(fence);
+}
+
+/**
+ * drm_work_fence_init - Initialize a work fence
+ * @wfence: work fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ */
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops)
+{
+	kref_init(&wfence->refcount);
+	wfence->wq = wq;
+	wfence->ops = ops;
+	wfence->fence = NULL;
+	INIT_WORK(&wfence->work, drm_work_fence_work);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_init);
+
+/**
+ * drm_work_fence_add_callback - Attach a work fence to a dma-fence
+ * @wfence: work fence
+ * @fence: dma-fence to watch; ownership of this reference is transferred
+ *         to the callback — caller must NOT put it afterward.
+ *
+ * When @fence signals, a work item is queued that calls ops->work().
+ * If @fence has already signaled, the work item is queued immediately.
+ *
+ * An additional reference to @fence is stored internally in @wfence to
+ * allow drm_work_fence_cancel() to be called safely without the caller
+ * needing to hold a separate fence reference.
+ *
+ * On any return value the caller's fence reference is consumed.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				struct dma_fence *fence)
+{
+	int err;
+
+	drm_work_fence_get(wfence);
+	wfence->fence = dma_fence_get(fence);
+
+	err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
+	if (err == -ENOENT) {
+		queue_work(wfence->wq, &wfence->work);
+		dma_fence_put(fence);
+		err = 0;
+	} else if (err) {
+		dma_fence_put(wfence->fence);
+		wfence->fence = NULL;
+		drm_work_fence_put(wfence);
+		dma_fence_put(fence);
+	}
+	/* on success: transferred ref goes to drm_work_fence_cb */
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_add_callback);
+
+/**
+ * drm_work_fence_cancel - Cancel a pending work fence callback
+ * @wfence: work fence
+ *
+ * Attempts to remove the pending callback before driver context teardown.
+ * The caller must hold a reference to @wfence across this call.
+ *
+ * If the callback has already fired this returns false and all cleanup
+ * has been handled internally.
+ *
+ * If removal succeeds the callback reference is released internally.
+ * The caller must still release its own reference via drm_work_fence_put().
+ *
+ * This function is safe to call from atomic context as it only acquires
+ * the dma-fence spinlock internally. If the caller also needs to wait
+ * for the worker to finish, use drm_work_fence_cancel_sync() instead,
+ * which may sleep.
+ *
+ * Return: true if callback was removed, false if it had already fired.
+ */
+bool drm_work_fence_cancel(struct drm_work_fence *wfence)
+{
+	struct dma_fence *fence = wfence->fence;
+
+	if (!fence)
+		return false;
+
+	if (dma_fence_remove_callback(fence, &wfence->cb)) {
+		wfence->fence = NULL;
+		dma_fence_put(fence);  /* callback ref */
+		dma_fence_put(fence);  /* stored ref */
+		drm_work_fence_put(wfence);
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_cancel);
+
+/**
+ * drm_work_fence_cancel_sync - Cancel callback and wait for worker to finish
+ * @wfence: work fence
+ *
+ * Calls drm_work_fence_cancel() then cancel_work_sync() to guarantee
+ * the worker has fully completed before returning.
+ *
+ * This function may sleep. Must not be called from atomic or interrupt
+ * context. Use drm_work_fence_cancel() instead when sleeping is not allowed.
+ *
+ * Drivers must call this during teardown before freeing any resources
+ * accessed by ops->work().
+ */
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
+{
+	drm_work_fence_cancel(wfence);
+	if (cancel_work_sync(&wfence->work))
+		drm_work_fence_put(wfence);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_cancel_sync);
diff --git a/include/drm/drm_work_fence.h b/include/drm/drm_work_fence.h
new file mode 100644
index 000000000000..4fa369f937d7
--- /dev/null
+++ b/include/drm/drm_work_fence.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_WORK_FENCE_H__
+#define __DRM_WORK_FENCE_H__
+
+#include <linux/dma-fence.h>
+#include <linux/kref.h>
+#include <linux/workqueue.h>
+
+struct drm_work_fence;
+
+/**
+ * struct drm_work_fence_ops - driver callbacks for a DRM work fence
+ */
+struct drm_work_fence_ops {
+	/**
+	 * @work: Called from workqueue context when the dma-fence signals.
+	 * Perform any work that cannot run in IRQ context here.
+	 */
+	void (*work)(struct drm_work_fence *wfence);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_work_fence *wfence);
+};
+
+/**
+ * struct drm_work_fence - embeddable DRM work fence
+ *
+ * Provides a dma-fence callback that queues a work item when the fence
+ * signals, allowing work that cannot run in IRQ context to be deferred
+ * to a workqueue. Drivers embed this in their own structure.
+ *
+ * NOTE: This helper is a *consumer* of dma_fences only. It CANNOT be
+ * used to implement dma_fence_ops. dma_fence callbacks are invoked
+ * while holding the fence spinlock; work queued here may sleep
+ * (copy_to_user, kthread_use_mm, eventfd_signal) and must not be
+ * called under that spinlock.
+ *
+ * Call drm_work_fence_init() at creation and drm_work_fence_add_callback()
+ * to arm. Call drm_work_fence_cancel_sync() before driver teardown.
+ */
+struct drm_work_fence {
+	/** @refcount: Reference count. */
+	struct kref refcount;
+	/** @work: Work item queued when the dma-fence signals. */
+	struct work_struct work;
+	/** @cb: dma-fence callback. */
+	struct dma_fence_cb cb;
+	/**
+	 * @fence: Extra reference held for safe cancel(). Set during
+	 * add_callback, released in destroy().
+	 */
+	struct dma_fence *fence;
+	/** @wq: Workqueue to run @work on. */
+	struct workqueue_struct *wq;
+	/** @ops: Driver operations. */
+	const struct drm_work_fence_ops *ops;
+};
+
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops);
+void drm_work_fence_get(struct drm_work_fence *wfence);
+void drm_work_fence_put(struct drm_work_fence *wfence);
+int drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				struct dma_fence *fence);
+bool drm_work_fence_cancel(struct drm_work_fence *wfence);
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence);
+
+#endif /* __DRM_WORK_FENCE_H__ */
-- 
2.34.1

[PATCH v5 2/4] drm: Add drm_user_fence helper

[Srinivasan Shanmugam]

Introduce a common DRM user fence helper providing the kref-managed,
MM-borrowing dma-fence-callback-to-workqueue pattern used by drivers
that must access userspace memory from a kthread context when a GPU
fence signals.

XE uses this pattern (xe_sync.c) to write a fence completion value
to a userspace VA. AMDGPU will use the same pattern to signal a
per-queue eventfd from a user-queue EOP fence callback.

The helper provides:
 - struct drm_user_fence: embeddable base structure
 - struct drm_user_fence_ops: worker/destroy callbacks
 - drm_user_fence_init(): initialize and grab the process MM
 - drm_user_fence_get/put(): reference counting
 - drm_user_fence_add_callback(): attach to a dma-fence

The worker callback receives a bool indicating whether the process
MM was successfully obtained, allowing drivers to handle the
unavailable-MM case (log, skip the userspace write, etc.) without
duplicating the mmget/kthread_use_mm/mmput boilerplate.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/Makefile         |  1 +
 drivers/gpu/drm/drm_user_fence.c | 69 +++++++++++++++++++++++++
 include/drm/drm_user_fence.h     | 86 ++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 include/drm/drm_user_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index c5be8e80d0c8..ddb770738992 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -69,6 +69,7 @@ drm-y := \
 	drm_syncobj.o \
 	drm_sysfs.o \
 	drm_trace_points.o \
+	drm_user_fence.o \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
new file mode 100644
index 000000000000..664178e2d74c
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * DRM user fence — extends drm_work_fence with kthread_use_mm() support.
+ *
+ * Use this when a GPU fence signals and work needs to access userspace
+ * memory (copy_to_user, fault-able operations) from a kthread context.
+ * For work that does not require userspace memory access, use
+ * drm_work_fence directly.
+ */
+
+#include <linux/kthread.h>
+#include <linux/sched/mm.h>
+
+#include <drm/drm_user_fence.h>
+
+static void drm_user_fence_do_work(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+	bool mm_ok = false;
+
+	if (mmget_not_zero(ufence->mm)) {
+		kthread_use_mm(ufence->mm);
+		mm_ok = true;
+	}
+
+	ufence->ops->worker(ufence, mm_ok);
+
+	if (mm_ok) {
+		kthread_unuse_mm(ufence->mm);
+		mmput_async(ufence->mm);
+	}
+}
+
+static void drm_user_fence_do_destroy(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+
+	mmdrop(ufence->mm);
+	ufence->ops->destroy(ufence);
+}
+
+static const struct drm_work_fence_ops drm_user_fence_wf_ops = {
+	.work    = drm_user_fence_do_work,
+	.destroy = drm_user_fence_do_destroy,
+};
+
+/**
+ * drm_user_fence_init - Initialize a user fence
+ * @ufence: user fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ *
+ * Must be called from process context with a valid current->mm.
+ * Grabs a reference to current->mm via mmgrab().
+ */
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops)
+{
+	drm_work_fence_init(&ufence->base, wq, &drm_user_fence_wf_ops);
+	ufence->mm = current->mm;
+	mmgrab(ufence->mm);
+	ufence->ops = ops;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_init);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
new file mode 100644
index 000000000000..2b2b640f510f
--- /dev/null
+++ b/include/drm/drm_user_fence.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_USER_FENCE_H__
+#define __DRM_USER_FENCE_H__
+
+#include <drm/drm_work_fence.h>
+
+struct drm_user_fence;
+
+/**
+ * struct drm_user_fence_ops - driver callbacks for a DRM user fence
+ */
+struct drm_user_fence_ops {
+	/**
+	 * @worker: Called from workqueue context with the process MM active.
+	 *
+	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
+	 * (copy_to_user, etc.) may be accessed safely.
+	 * If @mm_ok is false, the process MM was already gone; the driver
+	 * should log a warning and skip the userspace write.
+	 *
+	 * wake_up() or other post-signal housekeeping should also happen here.
+	 */
+	void (*worker)(struct drm_user_fence *ufence, bool mm_ok);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_user_fence *ufence);
+};
+
+/**
+ * struct drm_user_fence - DRM user fence with MM borrowing
+ *
+ * Extends drm_work_fence with kthread_use_mm() support for drivers
+ * that need to access userspace memory when a GPU fence signals.
+ * For work that does not need userspace memory access, use
+ * drm_work_fence directly.
+ *
+ * Call drm_user_fence_init() at creation and drm_user_fence_add_callback()
+ * to arm on a dma-fence. Call drm_user_fence_cancel_sync() before teardown.
+ */
+struct drm_user_fence {
+	/** @base: Base work fence. Must be first. */
+	struct drm_work_fence base;
+	/** @mm: Process MM grabbed at init time. */
+	struct mm_struct *mm;
+	/** @ops: Driver operations. */
+	const struct drm_user_fence_ops *ops;
+};
+
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops);
+
+static inline void drm_user_fence_get(struct drm_user_fence *ufence)
+{
+	drm_work_fence_get(&ufence->base);
+}
+
+static inline void drm_user_fence_put(struct drm_user_fence *ufence)
+{
+	drm_work_fence_put(&ufence->base);
+}
+
+static inline int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+					      struct dma_fence *fence)
+{
+	return drm_work_fence_add_callback(&ufence->base, fence);
+}
+
+static inline bool drm_user_fence_cancel(struct drm_user_fence *ufence)
+{
+	return drm_work_fence_cancel(&ufence->base);
+}
+
+static inline void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
+{
+	drm_work_fence_cancel_sync(&ufence->base);
+}
+
+#endif /* __DRM_USER_FENCE_H__ */
-- 
2.34.1

[PATCH v5 3/4] drm/xe: Convert xe_user_fence to drm_user_fence

[Srinivasan Shanmugam]

Replace the open-coded user fence implementation in xe_sync.c with the
new common drm_user_fence helper.

struct xe_user_fence now embeds struct drm_user_fence as its base.
XE-specific fields (xe_device pointer for the ufence_wq wake-up,
userspace VA, expected value, signalled flag) remain in the wrapper.

The local user_fence_destroy/get/put/worker/kick_ufence/user_fence_cb
functions are removed. Their logic moves to xe_ufence_ops.worker and
xe_ufence_ops.destroy, which are called by drm_user_fence_work().

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/xe/xe_sync.c       | 149 ++++++++++++++++-------------
 drivers/gpu/drm/xe/xe_sync.h       |   2 +
 drivers/gpu/drm/xe/xe_sync_types.h |   1 -
 drivers/gpu/drm/xe/xe_vm.c         |   1 +
 4 files changed, 84 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
index 37866768d64c..2d1e07792506 100644
--- a/drivers/gpu/drm/xe/xe_sync.c
+++ b/drivers/gpu/drm/xe/xe_sync.c
@@ -6,12 +6,11 @@
 #include "xe_sync.h"
 
 #include <linux/dma-fence-array.h>
-#include <linux/kthread.h>
-#include <linux/sched/mm.h>
 #include <linux/uaccess.h>
 
 #include <drm/drm_print.h>
 #include <drm/drm_syncobj.h>
+#include <drm/drm_user_fence.h>
 #include <uapi/drm/xe_drm.h>
 
 #include "xe_device.h"
@@ -19,36 +18,60 @@
 #include "xe_macros.h"
 #include "xe_sched_job_types.h"
 
+/*
+ * xe_user_fence wraps drm_user_fence with XE-specific fields.
+ * The drm_user_fence base handles MM borrowing and work-item lifetime.
+ */
 struct xe_user_fence {
-	struct xe_device *xe;
-	struct kref refcount;
-	struct dma_fence_cb cb;
-	struct work_struct worker;
-	struct mm_struct *mm;
-	u64 __user *addr;
-	u64 value;
-	int signalled;
+	struct drm_user_fence base;
+	struct xe_device     *xe;
+	u64 __user           *addr;
+	u64                   value;
+	int                   signalled;
 };
 
-static void user_fence_destroy(struct kref *kref)
+static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok)
 {
-	struct xe_user_fence *ufence = container_of(kref, struct xe_user_fence,
-						 refcount);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
 
-	mmdrop(ufence->mm);
-	kfree(ufence);
-}
+	/*
+	 * Mark signalled before waking waiters so UMD can safely reuse
+	 * the same ufence without hitting -EBUSY.
+	 */
+	WRITE_ONCE(ufence->signalled, 1);
 
-static void user_fence_get(struct xe_user_fence *ufence)
-{
-	kref_get(&ufence->refcount);
+	/*
+	 * Ensure the signalled store is visible before the user memory write
+	 * on weakly ordered architectures (e.g. ARM64). Without this barrier
+	 * the CPU may reorder stores, causing userspace to observe the user
+	 * memory update before signalled == 1.
+	 */
+	smp_wmb();
+
+	if (mm_ok) {
+		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
+			drm_dbg(&ufence->xe->drm,
+				"copy_to_user failed, user fence wasn't signaled\n");
+	} else {
+		drm_dbg(&ufence->xe->drm,
+			"mmget_not_zero() failed, ufence wasn't signaled\n");
+	}
+
+	wake_up_all(&ufence->xe->ufence_wq);
 }
 
-static void user_fence_put(struct xe_user_fence *ufence)
+static void xe_ufence_destroy(struct drm_user_fence *base)
 {
-	kref_put(&ufence->refcount, user_fence_destroy);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
+
+	kfree(ufence);
 }
 
+static const struct drm_user_fence_ops xe_ufence_ops = {
+	.worker  = xe_ufence_worker,
+	.destroy = xe_ufence_destroy,
+};
+
 static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 					       u64 value)
 {
@@ -63,51 +86,22 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 	if (!ufence)
 		return ERR_PTR(-ENOMEM);
 
-	ufence->xe = xe;
-	kref_init(&ufence->refcount);
-	ufence->addr = ptr;
+	ufence->xe    = xe;
+	ufence->addr  = ptr;
 	ufence->value = value;
-	ufence->mm = current->mm;
-	mmgrab(ufence->mm);
+	drm_user_fence_init(&ufence->base, xe->ordered_wq, &xe_ufence_ops);
 
 	return ufence;
 }
 
-static void user_fence_worker(struct work_struct *w)
-{
-	struct xe_user_fence *ufence = container_of(w, struct xe_user_fence, worker);
-
-	WRITE_ONCE(ufence->signalled, 1);
-	if (mmget_not_zero(ufence->mm)) {
-		kthread_use_mm(ufence->mm);
-		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
-			XE_WARN_ON("Copy to user failed");
-		kthread_unuse_mm(ufence->mm);
-		mmput(ufence->mm);
-	} else {
-		drm_dbg(&ufence->xe->drm, "mmget_not_zero() failed, ufence wasn't signaled\n");
-	}
-
-	/*
-	 * Wake up waiters only after updating the ufence state, allowing the UMD
-	 * to safely reuse the same ufence without encountering -EBUSY errors.
-	 */
-	wake_up_all(&ufence->xe->ufence_wq);
-	user_fence_put(ufence);
-}
-
-static void kick_ufence(struct xe_user_fence *ufence, struct dma_fence *fence)
+static void user_fence_get(struct xe_user_fence *ufence)
 {
-	INIT_WORK(&ufence->worker, user_fence_worker);
-	queue_work(ufence->xe->ordered_wq, &ufence->worker);
-	dma_fence_put(fence);
+	drm_user_fence_get(&ufence->base);
 }
 
-static void user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+static void user_fence_put(struct xe_user_fence *ufence)
 {
-	struct xe_user_fence *ufence = container_of(cb, struct xe_user_fence, cb);
-
-	kick_ufence(ufence, fence);
+	drm_user_fence_put(&ufence->base);
 }
 
 int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
@@ -282,24 +276,15 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
 	} else if (sync->syncobj) {
 		drm_syncobj_replace_fence(sync->syncobj, fence);
 	} else if (sync->ufence) {
-		int err;
-
 		drm_syncobj_add_point(sync->ufence_syncobj,
 				      sync->ufence_chain_fence,
 				      fence, sync->ufence_timeline_value);
 		sync->ufence_chain_fence = NULL;
 
 		fence = drm_syncobj_fence_get(sync->ufence_syncobj);
-		user_fence_get(sync->ufence);
-		err = dma_fence_add_callback(fence, &sync->ufence->cb,
-					     user_fence_cb);
-		if (err == -ENOENT) {
-			kick_ufence(sync->ufence, fence);
-		} else if (err) {
+		if (drm_user_fence_add_callback(&sync->ufence->base, fence))
 			XE_WARN_ON("failed to add user fence");
-			user_fence_put(sync->ufence);
-			dma_fence_put(fence);
-		}
+		/* fence ref consumed by drm_user_fence_add_callback */
 	}
 }
 
@@ -434,6 +419,34 @@ void xe_sync_ufence_put(struct xe_user_fence *ufence)
 	user_fence_put(ufence);
 }
 
+/**
+ * xe_sync_ufence_cancel() - Non-blocking cancel of user fence callback
+ * @ufence: user fence reference
+ *
+ * Attempts to cancel the pending callback without waiting for the worker.
+ * Safe to call while holding dma_resv_lock or vm->lock. If the callback
+ * has already fired, the worker runs independently — xe_ufence_worker
+ * only accesses device-level and userspace resources, both safe after
+ * VMA teardown.
+ */
+void xe_sync_ufence_cancel(struct xe_user_fence *ufence)
+{
+	drm_user_fence_cancel(&ufence->base);
+}
+
+/**
+ * xe_sync_ufence_cancel_sync() - Cancel user fence callback and wait for worker
+ * @ufence: user fence reference
+ *
+ * Cancels any pending dma-fence callback and waits for the worker to fully
+ * complete before returning. Must be called during teardown before freeing
+ * any resources accessed by the worker.
+ */
+void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence)
+{
+	drm_user_fence_cancel_sync(&ufence->base);
+}
+
 /**
  * xe_sync_ufence_get_status() - Get user fence status
  * @ufence: user fence
diff --git a/drivers/gpu/drm/xe/xe_sync.h b/drivers/gpu/drm/xe/xe_sync.h
index 6b949194acff..b5ff3387c592 100644
--- a/drivers/gpu/drm/xe/xe_sync.h
+++ b/drivers/gpu/drm/xe/xe_sync.h
@@ -44,6 +44,8 @@ static inline bool xe_sync_is_ufence(struct xe_sync_entry *sync)
 struct xe_user_fence *__xe_sync_ufence_get(struct xe_user_fence *ufence);
 struct xe_user_fence *xe_sync_ufence_get(struct xe_sync_entry *sync);
 void xe_sync_ufence_put(struct xe_user_fence *ufence);
+void xe_sync_ufence_cancel(struct xe_user_fence *ufence);
+void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence);
 int xe_sync_ufence_get_status(struct xe_user_fence *ufence);
 
 #endif
diff --git a/drivers/gpu/drm/xe/xe_sync_types.h b/drivers/gpu/drm/xe/xe_sync_types.h
index b88f1833e28c..67048199b070 100644
--- a/drivers/gpu/drm/xe/xe_sync_types.h
+++ b/drivers/gpu/drm/xe/xe_sync_types.h
@@ -12,7 +12,6 @@ struct drm_syncobj;
 struct dma_fence;
 struct dma_fence_chain;
 struct drm_xe_sync;
-struct user_fence;
 
 struct xe_sync_entry {
 	struct drm_syncobj *syncobj;
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index b01f31ed4417..fbd2c28de8a6 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -1181,6 +1181,7 @@ static void xe_vma_destroy_late(struct xe_vma *vma)
 	struct xe_bo *bo = xe_vma_bo(vma);
 
 	if (vma->ufence) {
+		xe_sync_ufence_cancel(vma->ufence);
 		xe_sync_ufence_put(vma->ufence);
 		vma->ufence = NULL;
 	}
-- 
2.34.1

Re: [PATCH v5 3/4] drm/xe: Convert xe_user_fence to drm_user_fence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Possible Use-After-Free of xe_device when a foreign dma-fence signals after device teardown.
--

commit 3bd2b743d1b606804ebc5081e49badce29c41a21
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

drm/xe: Convert xe_user_fence to drm_user_fence

Replace the open-coded user fence implementation in xe_sync.c with the
new common drm_user_fence helper.

> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> index 37866768d64c2..2d1e07792506e 100644
> --- a/drivers/gpu/drm/xe/xe_sync.c
> +++ b/drivers/gpu/drm/xe/xe_sync.c
> @@ -19,36 +18,60 @@

[ ... ]

> +static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok)
> +{
> +	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);

[ ... ]

> +	if (mm_ok) {
> +		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
> +			drm_dbg(&ufence->xe->drm,
> +				"copy_to_user failed, user fence wasn't signaled\n");
> +	} else {
> +		drm_dbg(&ufence->xe->drm,
> +			"mmget_not_zero() failed, ufence wasn't signaled\n");
> +	}
> +
> +	wake_up_all(&ufence->xe->ufence_wq);
> +}

[ ... ]

>  static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
>  					       u64 value)
>  {
> @@ -63,51 +86,22 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
>  	if (!ufence)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ufence->xe = xe;
> -	kref_init(&ufence->refcount);
> -	ufence->addr = ptr;
> +	ufence->xe    = xe;
> +	ufence->addr  = ptr;
>  	ufence->value = value;
> -	ufence->mm = current->mm;
> -	mmgrab(ufence->mm);
> +	drm_user_fence_init(&ufence->base, xe->ordered_wq, &xe_ufence_ops);

[Severity: High]
This is a pre-existing issue, but does this code leave the xe_device vulnerable
to a use-after-free when an external dma-fence signals after device teardown?

When a user creates a user fence that depends on an external dma-fence (e.g.,
from amdgpu) by passing a syncobj, xe_sync_entry_parse() creates the user fence
without taking a reference to the xe_device.

If the user then closes the xe file descriptor or the device is hot-unplugged,
the last reference to xe_device can be dropped. xe_sync_entry_cleanup()
drops the sync entry's reference to the user fence but does not cancel
the pending callback.

When the external dma-fence eventually signals, the callback invokes
queue_work(). Because the workqueue points to xe->ordered_wq which was already
destroyed during device teardown, this causes a UAF crash. If the worker
does run, it will also access the freed xe_device pointer in xe_ufence_worker()
to wake up waiters.

Could user_fence_create() take a reference to the xe_device, or could
xe_sync_entry_cleanup() be updated to explicitly cancel the pending callback?

>  
>  	return ufence;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827062142.4038272-1-srinivasan.shanmugam@amd.com?part=3

[PATCH v6 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE

[Srinivasan Shanmugam]

When a GPU dma-fence signals, drivers often need to perform work that
cannot run in IRQ context. This pattern is currently open-coded in
multiple drivers.

This series introduces two layered helpers:

Patch 1 introduces drm_work_fence — a generic embeddable base structure
that handles the dma-fence-callback-to-workqueue pattern. Any driver
needing deferred fence work can use this directly.

Patch 2 introduces drm_user_fence — a thin layer on top of
drm_work_fence that adds kthread_use_mm() support for drivers that need
to access userspace memory when a fence signals.

Patch 3 converts XE to use drm_user_fence. XE continues to write a
fence completion value to a userspace VA using the new helper.

Patch 4 adds optional per-signal compare functionality to drm_user_fence.
When cmp_addr is set, the worker is called only if the value at cmp_addr
satisfies the configured comparison. This enables AMDGPU's EOP eventfd
per-signal filtering without open-coding the read+compare pattern.

A follow-on patch (not in this series) will wire AMDGPU's render-node
EOP eventfd signaling path to drm_work_fence.

v6:
 - Add WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT)) in
   drm_user_fence_set_compare() since get_user() of u64 is not safe
   on 32-bit systems. Plain WARN_ON_ONCE() is used as drm_user_fence
   holds no struct drm_device * reference. (Thomas Hellström review)

v5:
 - Split drm_user_fence into drm_work_fence (generic) and drm_user_fence
   (MM-borrowing subclass) per Matthew Brost's suggestion.
 - Add per-signal compare functionality (drm_user_fence_set_compare())
   per Christian König's suggestion.
 - Use mmput_async() instead of mmput() to avoid potential deadlock in
   MMU notifier release path. (Sashiko review)
 - Use cmp_op != DRM_USER_FENCE_CMP_NONE as gate for compare logic.
   Add WARN_ON for invalid set_compare() arguments. (Sashiko review)

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org

Srinivasan Shanmugam (4):
  drm: Add drm_work_fence helper
  drm: Add drm_user_fence helper
  drm/xe: Convert xe_user_fence to drm_user_fence
  drm: Add per-signal compare functionality to drm_user_fence

 drivers/gpu/drm/Makefile           |   2 +
 drivers/gpu/drm/drm_user_fence.c   | 149 ++++++++++++++++++++++
 drivers/gpu/drm/drm_work_fence.c   | 195 +++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_sync.c       | 149 ++++++++++++----------
 drivers/gpu/drm/xe/xe_sync.h       |   2 +
 drivers/gpu/drm/xe/xe_sync_types.h |   1 -
 drivers/gpu/drm/xe/xe_vm.c         |   1 +
 include/drm/drm_user_fence.h       | 115 +++++++++++++++++
 include/drm/drm_work_fence.h       |  76 +++++++++++
 9 files changed, 621 insertions(+), 69 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 drivers/gpu/drm/drm_work_fence.c
 create mode 100644 include/drm/drm_user_fence.h
 create mode 100644 include/drm/drm_work_fence.h

-- 
2.34.1

[PATCH v6 1/4] drm: Add drm_work_fence helper

[Srinivasan Shanmugam]

GPU drivers often need to queue work when a dma-fence signals
because certain operations (copy_to_user, eventfd_signal, memory
allocation) cannot run in IRQ context. This pattern is currently
open-coded in multiple drivers.

Introduce drm_work_fence — an embeddable base structure that handles
the dma-fence-callback-to-workqueue pattern in one place. Drivers
embed this in their own structure and implement ops->work() for the
deferred work and ops->destroy() for cleanup.

The helper manages:
 - kref lifetime
 - dma-fence callback registration
 - workqueue dispatch on fence signal
 - safe cancellation before driver teardown

For work that additionally requires borrowing the process MM via
kthread_use_mm(), see drm_user_fence which builds on top of this.

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_work_fence.c | 195 +++++++++++++++++++++++++++++++
 include/drm/drm_work_fence.h     |  76 ++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_work_fence.c
 create mode 100644 include/drm/drm_work_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..c5be8e80d0c8 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -72,6 +72,7 @@ drm-y := \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
+	drm_work_fence.o \
 	drm_writeback.o
 drm-$(CONFIG_DRM_CLIENT) += \
 	drm_client.o \
diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
new file mode 100644
index 000000000000..9f6b779d0fe9
--- /dev/null
+++ b/drivers/gpu/drm/drm_work_fence.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * Common DRM work fence helper.
+ *
+ * When a GPU dma-fence signals, drivers often need to perform work that
+ * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
+ * eventfd_signal). This helper queues a work item when a dma-fence
+ * signals, allowing that work to run safely in a workqueue context.
+ *
+ * NOTE: This helper consumes dma_fences but CANNOT implement
+ * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
+ * callbacks are called under the fence spinlock and must not sleep.
+ *
+ * For work that additionally requires accessing userspace memory via
+ * kthread_use_mm(), see drm_user_fence which builds on top of this.
+ */
+
+#include <linux/workqueue.h>
+
+#include <drm/drm_work_fence.h>
+
+static void drm_work_fence_destroy(struct kref *kref)
+{
+	struct drm_work_fence *wfence =
+		container_of(kref, struct drm_work_fence, refcount);
+
+	if (wfence->fence)
+		dma_fence_put(wfence->fence);
+
+	wfence->ops->destroy(wfence);
+}
+
+/**
+ * drm_work_fence_get - Acquire a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_get(struct drm_work_fence *wfence)
+{
+	kref_get(&wfence->refcount);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_get);
+
+/**
+ * drm_work_fence_put - Release a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_put(struct drm_work_fence *wfence)
+{
+	kref_put(&wfence->refcount, drm_work_fence_destroy);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_put);
+
+static void drm_work_fence_work(struct work_struct *w)
+{
+	struct drm_work_fence *wfence =
+		container_of(w, struct drm_work_fence, work);
+
+	wfence->ops->work(wfence);
+	drm_work_fence_put(wfence);
+}
+
+static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+{
+	struct drm_work_fence *wfence =
+		container_of(cb, struct drm_work_fence, cb);
+
+	queue_work(wfence->wq, &wfence->work);
+	/*
+	 * Put the transferred reference from add_callback. The stored
+	 * reference in wfence->fence is released in drm_work_fence_destroy().
+	 */
+	dma_fence_put(fence);
+}
+
+/**
+ * drm_work_fence_init - Initialize a work fence
+ * @wfence: work fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ */
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops)
+{
+	kref_init(&wfence->refcount);
+	wfence->wq = wq;
+	wfence->ops = ops;
+	wfence->fence = NULL;
+	INIT_WORK(&wfence->work, drm_work_fence_work);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_init);
+
+/**
+ * drm_work_fence_add_callback - Attach a work fence to a dma-fence
+ * @wfence: work fence
+ * @fence: dma-fence to watch; ownership of this reference is transferred
+ *         to the callback — caller must NOT put it afterward.
+ *
+ * When @fence signals, a work item is queued that calls ops->work().
+ * If @fence has already signaled, the work item is queued immediately.
+ *
+ * An additional reference to @fence is stored internally in @wfence to
+ * allow drm_work_fence_cancel() to be called safely without the caller
+ * needing to hold a separate fence reference.
+ *
+ * On any return value the caller's fence reference is consumed.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				struct dma_fence *fence)
+{
+	int err;
+
+	drm_work_fence_get(wfence);
+	wfence->fence = dma_fence_get(fence);
+
+	err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
+	if (err == -ENOENT) {
+		queue_work(wfence->wq, &wfence->work);
+		dma_fence_put(fence);
+		err = 0;
+	} else if (err) {
+		dma_fence_put(wfence->fence);
+		wfence->fence = NULL;
+		drm_work_fence_put(wfence);
+		dma_fence_put(fence);
+	}
+	/* on success: transferred ref goes to drm_work_fence_cb */
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_add_callback);
+
+/**
+ * drm_work_fence_cancel - Cancel a pending work fence callback
+ * @wfence: work fence
+ *
+ * Attempts to remove the pending callback before driver context teardown.
+ * The caller must hold a reference to @wfence across this call.
+ *
+ * If the callback has already fired this returns false and all cleanup
+ * has been handled internally.
+ *
+ * If removal succeeds the callback reference is released internally.
+ * The caller must still release its own reference via drm_work_fence_put().
+ *
+ * This function is safe to call from atomic context as it only acquires
+ * the dma-fence spinlock internally. If the caller also needs to wait
+ * for the worker to finish, use drm_work_fence_cancel_sync() instead,
+ * which may sleep.
+ *
+ * Return: true if callback was removed, false if it had already fired.
+ */
+bool drm_work_fence_cancel(struct drm_work_fence *wfence)
+{
+	struct dma_fence *fence = wfence->fence;
+
+	if (!fence)
+		return false;
+
+	if (dma_fence_remove_callback(fence, &wfence->cb)) {
+		wfence->fence = NULL;
+		dma_fence_put(fence);  /* callback ref */
+		dma_fence_put(fence);  /* stored ref */
+		drm_work_fence_put(wfence);
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_cancel);
+
+/**
+ * drm_work_fence_cancel_sync - Cancel callback and wait for worker to finish
+ * @wfence: work fence
+ *
+ * Calls drm_work_fence_cancel() then cancel_work_sync() to guarantee
+ * the worker has fully completed before returning.
+ *
+ * This function may sleep. Must not be called from atomic or interrupt
+ * context. Use drm_work_fence_cancel() instead when sleeping is not allowed.
+ *
+ * Drivers must call this during teardown before freeing any resources
+ * accessed by ops->work().
+ */
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
+{
+	drm_work_fence_cancel(wfence);
+	if (cancel_work_sync(&wfence->work))
+		drm_work_fence_put(wfence);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_cancel_sync);
diff --git a/include/drm/drm_work_fence.h b/include/drm/drm_work_fence.h
new file mode 100644
index 000000000000..4fa369f937d7
--- /dev/null
+++ b/include/drm/drm_work_fence.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_WORK_FENCE_H__
+#define __DRM_WORK_FENCE_H__
+
+#include <linux/dma-fence.h>
+#include <linux/kref.h>
+#include <linux/workqueue.h>
+
+struct drm_work_fence;
+
+/**
+ * struct drm_work_fence_ops - driver callbacks for a DRM work fence
+ */
+struct drm_work_fence_ops {
+	/**
+	 * @work: Called from workqueue context when the dma-fence signals.
+	 * Perform any work that cannot run in IRQ context here.
+	 */
+	void (*work)(struct drm_work_fence *wfence);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_work_fence *wfence);
+};
+
+/**
+ * struct drm_work_fence - embeddable DRM work fence
+ *
+ * Provides a dma-fence callback that queues a work item when the fence
+ * signals, allowing work that cannot run in IRQ context to be deferred
+ * to a workqueue. Drivers embed this in their own structure.
+ *
+ * NOTE: This helper is a *consumer* of dma_fences only. It CANNOT be
+ * used to implement dma_fence_ops. dma_fence callbacks are invoked
+ * while holding the fence spinlock; work queued here may sleep
+ * (copy_to_user, kthread_use_mm, eventfd_signal) and must not be
+ * called under that spinlock.
+ *
+ * Call drm_work_fence_init() at creation and drm_work_fence_add_callback()
+ * to arm. Call drm_work_fence_cancel_sync() before driver teardown.
+ */
+struct drm_work_fence {
+	/** @refcount: Reference count. */
+	struct kref refcount;
+	/** @work: Work item queued when the dma-fence signals. */
+	struct work_struct work;
+	/** @cb: dma-fence callback. */
+	struct dma_fence_cb cb;
+	/**
+	 * @fence: Extra reference held for safe cancel(). Set during
+	 * add_callback, released in destroy().
+	 */
+	struct dma_fence *fence;
+	/** @wq: Workqueue to run @work on. */
+	struct workqueue_struct *wq;
+	/** @ops: Driver operations. */
+	const struct drm_work_fence_ops *ops;
+};
+
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops);
+void drm_work_fence_get(struct drm_work_fence *wfence);
+void drm_work_fence_put(struct drm_work_fence *wfence);
+int drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				struct dma_fence *fence);
+bool drm_work_fence_cancel(struct drm_work_fence *wfence);
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence);
+
+#endif /* __DRM_WORK_FENCE_H__ */
-- 
2.34.1

Re: [PATCH v6 1/4] drm: Add drm_work_fence helper

[Matthew Brost]

On Mon, Aug 31, 2026 at 07:15:36PM +0530, Srinivasan Shanmugam wrote:
> GPU drivers often need to queue work when a dma-fence signals
> because certain operations (copy_to_user, eventfd_signal, memory
> allocation) cannot run in IRQ context. This pattern is currently
> open-coded in multiple drivers.
> 
> Introduce drm_work_fence — an embeddable base structure that handles
> the dma-fence-callback-to-workqueue pattern in one place. Drivers
> embed this in their own structure and implement ops->work() for the
> deferred work and ops->destroy() for cleanup.
> 
> The helper manages:
>  - kref lifetime
>  - dma-fence callback registration
>  - workqueue dispatch on fence signal
>  - safe cancellation before driver teardown
> 
> For work that additionally requires borrowing the process MM via
> kthread_use_mm(), see drm_user_fence which builds on top of this.
> 
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>  drivers/gpu/drm/Makefile         |   1 +
>  drivers/gpu/drm/drm_work_fence.c | 195 +++++++++++++++++++++++++++++++
>  include/drm/drm_work_fence.h     |  76 ++++++++++++
>  3 files changed, 272 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_work_fence.c
>  create mode 100644 include/drm/drm_work_fence.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index e97faabcd783..c5be8e80d0c8 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -72,6 +72,7 @@ drm-y := \
>  	drm_vblank.o \
>  	drm_vblank_work.o \
>  	drm_vma_manager.o \
> +	drm_work_fence.o \
>  	drm_writeback.o
>  drm-$(CONFIG_DRM_CLIENT) += \
>  	drm_client.o \
> diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
> new file mode 100644
> index 000000000000..9f6b779d0fe9
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_work_fence.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 The Linux Foundation
> + *
> + * Common DRM work fence helper.
> + *
> + * When a GPU dma-fence signals, drivers often need to perform work that
> + * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
> + * eventfd_signal). This helper queues a work item when a dma-fence
> + * signals, allowing that work to run safely in a workqueue context.
> + *
> + * NOTE: This helper consumes dma_fences but CANNOT implement
> + * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
> + * callbacks are called under the fence spinlock and must not sleep.
> + *
> + * For work that additionally requires accessing userspace memory via
> + * kthread_use_mm(), see drm_user_fence which builds on top of this.
> + */
> +
> +#include <linux/workqueue.h>
> +
> +#include <drm/drm_work_fence.h>
> +
> +static void drm_work_fence_destroy(struct kref *kref)
> +{
> +	struct drm_work_fence *wfence =
> +		container_of(kref, struct drm_work_fence, refcount);
> +
> +	if (wfence->fence)
> +		dma_fence_put(wfence->fence);
> +
> +	wfence->ops->destroy(wfence);

I'd invert these for safety in case destroy wants to looks at the fence,
admittedly that would be an odd use case.

So...

	struct drm_work_fence *wfence =
		container_of(kref, struct drm_work_fence, refcount);
	struct dma_fence *fence = wfence->fence;

	wfence->ops->destroy(wfence);
	dma_fence_put(fence);	/* this has a NULL check */


> +}
> +
> +/**
> + * drm_work_fence_get - Acquire a reference to a work fence
> + * @wfence: work fence
> + */
> +void drm_work_fence_get(struct drm_work_fence *wfence)
> +{
> +	kref_get(&wfence->refcount);
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_get);
> +
> +/**
> + * drm_work_fence_put - Release a reference to a work fence
> + * @wfence: work fence
> + */
> +void drm_work_fence_put(struct drm_work_fence *wfence)
> +{
> +	kref_put(&wfence->refcount, drm_work_fence_destroy);
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_put);
> +
> +static void drm_work_fence_work(struct work_struct *w)
> +{
> +	struct drm_work_fence *wfence =
> +		container_of(w, struct drm_work_fence, work);
> +
> +	wfence->ops->work(wfence);
> +	drm_work_fence_put(wfence);
> +}
> +
> +static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
> +{
> +	struct drm_work_fence *wfence =
> +		container_of(cb, struct drm_work_fence, cb);
> +
> +	queue_work(wfence->wq, &wfence->work);
> +	/*
> +	 * Put the transferred reference from add_callback. The stored
> +	 * reference in wfence->fence is released in drm_work_fence_destroy().
> +	 */
> +	dma_fence_put(fence);
> +}
> +
> +/**
> + * drm_work_fence_init - Initialize a work fence
> + * @wfence: work fence to initialize
> + * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
> + * @ops: driver operations
> + */
> +void drm_work_fence_init(struct drm_work_fence *wfence,
> +			 struct workqueue_struct *wq,
> +			 const struct drm_work_fence_ops *ops)
> +{
> +	kref_init(&wfence->refcount);
> +	wfence->wq = wq;
> +	wfence->ops = ops;
> +	wfence->fence = NULL;
> +	INIT_WORK(&wfence->work, drm_work_fence_work);
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_init);
> +
> +/**
> + * drm_work_fence_add_callback - Attach a work fence to a dma-fence
> + * @wfence: work fence
> + * @fence: dma-fence to watch; ownership of this reference is transferred
> + *         to the callback — caller must NOT put it afterward.

This isn't right. It is perfectly reasonable for caller to hold more
than 1 reference to @fence, thus put it again. It consumes a single
reference @fence on success or failure - that is it.

> + *
> + * When @fence signals, a work item is queued that calls ops->work().
> + * If @fence has already signaled, the work item is queued immediately.
> + *
> + * An additional reference to @fence is stored internally in @wfence to
> + * allow drm_work_fence_cancel() to be called safely without the caller
> + * needing to hold a separate fence reference.
> + *

Ideally get rid of double ref count on @fence. I don't think above
reasoning justifies the needed for a double ref on the fence. I'd tie
exactly one refernece @fence which is attached to lifetime of @wfence
(i.e., drop the dma_fence_put in drm_work_fence_cb).

> + * On any return value the caller's fence reference is consumed.
> + *

I'd mention regardless of success or fail, a reference to drm_work_fence
is consumed too.

> + * Return: 0 on success, negative errno on error.
> + */
> +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> +				struct dma_fence *fence)
> +{
> +	int err;
> +
> +	drm_work_fence_get(wfence);
> +	wfence->fence = dma_fence_get(fence);
> +
> +	err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
> +	if (err == -ENOENT) {
> +		queue_work(wfence->wq, &wfence->work);
> +		dma_fence_put(fence);

Keep the implementation in one place?

drm_work_fence_work(&wfence->work);

> +		err = 0;
> +	} else if (err) {
> +		dma_fence_put(wfence->fence);
> +		wfence->fence = NULL;
> +		drm_work_fence_put(wfence);

Won't drm_work_fence_put just drop the 'wfence->fence' reference if
'wfence->fence' isn't set to NULL. i.e., drm_work_fence_put(wfence) can
replace the above 3 lines.

> +		dma_fence_put(fence);
> +	}
> +	/* on success: transferred ref goes to drm_work_fence_cb */
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_add_callback);
> +
> +/**
> + * drm_work_fence_cancel - Cancel a pending work fence callback
> + * @wfence: work fence
> + *
> + * Attempts to remove the pending callback before driver context teardown.
> + * The caller must hold a reference to @wfence across this call.
> + *
> + * If the callback has already fired this returns false and all cleanup
> + * has been handled internally.
> + *
> + * If removal succeeds the callback reference is released internally.
> + * The caller must still release its own reference via drm_work_fence_put().
> + *
> + * This function is safe to call from atomic context as it only acquires
> + * the dma-fence spinlock internally. If the caller also needs to wait
> + * for the worker to finish, use drm_work_fence_cancel_sync() instead,
> + * which may sleep.
> + *
> + * Return: true if callback was removed, false if it had already fired.
> + */
> +bool drm_work_fence_cancel(struct drm_work_fence *wfence)
> +{
> +	struct dma_fence *fence = wfence->fence;
> +
> +	if (!fence)
> +		return false;
> +
> +	if (dma_fence_remove_callback(fence, &wfence->cb)) {
> +		wfence->fence = NULL;
> +		dma_fence_put(fence);  /* callback ref */
> +		dma_fence_put(fence);  /* stored ref */
> +		drm_work_fence_put(wfence);

Same comments as above: No need for 'wfence->fence = NULL' and
drm_work_fence_put, drm_work_fence_put is work by itself. Also see my
comment about dropped the double ref, that isn't need either.

> +		return true;
> +	}
> +
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_cancel);
> +
> +/**
> + * drm_work_fence_cancel_sync - Cancel callback and wait for worker to finish
> + * @wfence: work fence
> + *
> + * Calls drm_work_fence_cancel() then cancel_work_sync() to guarantee
> + * the worker has fully completed before returning.
> + *
> + * This function may sleep. Must not be called from atomic or interrupt
> + * context. Use drm_work_fence_cancel() instead when sleeping is not allowed.
> + *
> + * Drivers must call this during teardown before freeing any resources
> + * accessed by ops->work().
> + */
> +void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
> +{
> +	drm_work_fence_cancel(wfence);
> +	if (cancel_work_sync(&wfence->work))
> +		drm_work_fence_put(wfence);

This will UAF if drm_work_fence_cancel removed the callback.

I actually don't think drm_work_fence_cancel, drm_work_fence_cancel_sync
is safe unless the caller has reference to drm_work_fence.

Consider the following case:

- A driver calls drm_work_fence_add_callback
- Sometime later if calls drm_work_fence_cancel or drm_work_fence_cancel_sync
- drm_work_fence_work completes before either drm_work_fence_cancel,
  drm_work_fence_cancel_sync completes, we UAF

So with additional reference at the caller assumed...

I'd write this like:

if (drm_work_fence_cancel(wfence))
	return;	/* Worker not running, all internal refs dropped */

if (cancel_work_sync(&wfence->work))
	drm_work_fence_put(wfence);	/* Worker cancelled, drop it ref */

> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_cancel_sync);
> diff --git a/include/drm/drm_work_fence.h b/include/drm/drm_work_fence.h
> new file mode 100644
> index 000000000000..4fa369f937d7
> --- /dev/null
> +++ b/include/drm/drm_work_fence.h
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 The Linux Foundation
> + */
> +
> +#ifndef __DRM_WORK_FENCE_H__
> +#define __DRM_WORK_FENCE_H__
> +
> +#include <linux/dma-fence.h>
> +#include <linux/kref.h>
> +#include <linux/workqueue.h>
> +
> +struct drm_work_fence;
> +
> +/**
> + * struct drm_work_fence_ops - driver callbacks for a DRM work fence
> + */
> +struct drm_work_fence_ops {
> +	/**
> +	 * @work: Called from workqueue context when the dma-fence signals.
> +	 * Perform any work that cannot run in IRQ context here.
> +	 */
> +	void (*work)(struct drm_work_fence *wfence);
> +
> +	/**
> +	 * @destroy: Called when the last reference is dropped.
> +	 * Free the containing structure here.
> +	 */
> +	void (*destroy)(struct drm_work_fence *wfence);
> +};
> +
> +/**
> + * struct drm_work_fence - embeddable DRM work fence
> + *
> + * Provides a dma-fence callback that queues a work item when the fence
> + * signals, allowing work that cannot run in IRQ context to be deferred
> + * to a workqueue. Drivers embed this in their own structure.
> + *
> + * NOTE: This helper is a *consumer* of dma_fences only. It CANNOT be
> + * used to implement dma_fence_ops. dma_fence callbacks are invoked
> + * while holding the fence spinlock; work queued here may sleep
> + * (copy_to_user, kthread_use_mm, eventfd_signal) and must not be
> + * called under that spinlock.
> + *
> + * Call drm_work_fence_init() at creation and drm_work_fence_add_callback()
> + * to arm. Call drm_work_fence_cancel_sync() before driver teardown.
> + */
> +struct drm_work_fence {
> +	/** @refcount: Reference count. */
> +	struct kref refcount;
> +	/** @work: Work item queued when the dma-fence signals. */
> +	struct work_struct work;
> +	/** @cb: dma-fence callback. */
> +	struct dma_fence_cb cb;

You could likely use union trick here on work_struct, dma_fence_cb and
only defer the INIT_WORK to drm_work_fence_cb.

> +	/**
> +	 * @fence: Extra reference held for safe cancel(). Set during
> +	 * add_callback, released in destroy().
> +	 */

See my comments this ref count. Ideally: "A single reference held for
the lifetime of drm_work_fence after drm_work_fence_init is called"

Matt

> +	struct dma_fence *fence;
> +	/** @wq: Workqueue to run @work on. */
> +	struct workqueue_struct *wq;
> +	/** @ops: Driver operations. */
> +	const struct drm_work_fence_ops *ops;
> +};
> +
> +void drm_work_fence_init(struct drm_work_fence *wfence,
> +			 struct workqueue_struct *wq,
> +			 const struct drm_work_fence_ops *ops);
> +void drm_work_fence_get(struct drm_work_fence *wfence);
> +void drm_work_fence_put(struct drm_work_fence *wfence);
> +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> +				struct dma_fence *fence);
> +bool drm_work_fence_cancel(struct drm_work_fence *wfence);
> +void drm_work_fence_cancel_sync(struct drm_work_fence *wfence);
> +
> +#endif /* __DRM_WORK_FENCE_H__ */
> -- 
> 2.34.1
> 

RE: [PATCH v6 1/4] drm: Add drm_work_fence helper

[SHANMUGAM, SRINIVASAN]

AMD General

> -----Original Message-----
> From: Matthew Brost <matthew.brost@intel.com>
> Sent: Tuesday, September 1, 2026 1:52 AM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>; dri-
> devel@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Koenig, Christian
> <Christian.Koenig@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org; Maarten
> Lankhorst <maarten.lankhorst@linux.intel.com>
> Subject: Re: [PATCH v6 1/4] drm: Add drm_work_fence helper
>
> On Mon, Aug 31, 2026 at 07:15:36PM +0530, Srinivasan Shanmugam wrote:
> > GPU drivers often need to queue work when a dma-fence signals because
> > certain operations (copy_to_user, eventfd_signal, memory
> > allocation) cannot run in IRQ context. This pattern is currently
> > open-coded in multiple drivers.
> >
> > Introduce drm_work_fence — an embeddable base structure that handles
> > the dma-fence-callback-to-workqueue pattern in one place. Drivers
> > embed this in their own structure and implement ops->work() for the
> > deferred work and ops->destroy() for cleanup.
> >
> > The helper manages:
> >  - kref lifetime
> >  - dma-fence callback registration
> >  - workqueue dispatch on fence signal
> >  - safe cancellation before driver teardown
> >
> > For work that additionally requires borrowing the process MM via
> > kthread_use_mm(), see drm_user_fence which builds on top of this.
> >
> > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: intel-xe@lists.freedesktop.org
> > Cc: amd-gfx@lists.freedesktop.org
> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> > ---
> >  drivers/gpu/drm/Makefile         |   1 +
> >  drivers/gpu/drm/drm_work_fence.c | 195
> +++++++++++++++++++++++++++++++
> >  include/drm/drm_work_fence.h     |  76 ++++++++++++
> >  3 files changed, 272 insertions(+)
> >  create mode 100644 drivers/gpu/drm/drm_work_fence.c  create mode
> > 100644 include/drm/drm_work_fence.h
> >
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index
> > e97faabcd783..c5be8e80d0c8 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -72,6 +72,7 @@ drm-y := \
> >     drm_vblank.o \
> >     drm_vblank_work.o \
> >     drm_vma_manager.o \
> > +   drm_work_fence.o \
> >     drm_writeback.o
> >  drm-$(CONFIG_DRM_CLIENT) += \
> >     drm_client.o \
> > diff --git a/drivers/gpu/drm/drm_work_fence.c
> > b/drivers/gpu/drm/drm_work_fence.c
> > new file mode 100644
> > index 000000000000..9f6b779d0fe9
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_work_fence.c
> > @@ -0,0 +1,195 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2024 The Linux Foundation
> > + *
> > + * Common DRM work fence helper.
> > + *
> > + * When a GPU dma-fence signals, drivers often need to perform work
> > +that
> > + * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
> > + * eventfd_signal). This helper queues a work item when a dma-fence
> > + * signals, allowing that work to run safely in a workqueue context.
> > + *
> > + * NOTE: This helper consumes dma_fences but CANNOT implement
> > + * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
> > + * callbacks are called under the fence spinlock and must not sleep.
> > + *
> > + * For work that additionally requires accessing userspace memory via
> > + * kthread_use_mm(), see drm_user_fence which builds on top of this.
> > + */
> > +
> > +#include <linux/workqueue.h>
> > +
> > +#include <drm/drm_work_fence.h>
> > +
> > +static void drm_work_fence_destroy(struct kref *kref) {
> > +   struct drm_work_fence *wfence =
> > +           container_of(kref, struct drm_work_fence, refcount);
> > +
> > +   if (wfence->fence)
> > +           dma_fence_put(wfence->fence);
> > +
> > +   wfence->ops->destroy(wfence);
>
> I'd invert these for safety in case destroy wants to looks at the fence, admittedly
> that would be an odd use case.
>
> So...
>
>       struct drm_work_fence *wfence =
>               container_of(kref, struct drm_work_fence, refcount);
>       struct dma_fence *fence = wfence->fence;
>
>       wfence->ops->destroy(wfence);
>       dma_fence_put(fence);   /* this has a NULL check */
>
>
> > +}
> > +
> > +/**
> > + * drm_work_fence_get - Acquire a reference to a work fence
> > + * @wfence: work fence
> > + */
> > +void drm_work_fence_get(struct drm_work_fence *wfence) {
> > +   kref_get(&wfence->refcount);
> > +}
> > +EXPORT_SYMBOL_GPL(drm_work_fence_get);
> > +
> > +/**
> > + * drm_work_fence_put - Release a reference to a work fence
> > + * @wfence: work fence
> > + */
> > +void drm_work_fence_put(struct drm_work_fence *wfence) {
> > +   kref_put(&wfence->refcount, drm_work_fence_destroy); }
> > +EXPORT_SYMBOL_GPL(drm_work_fence_put);
> > +
> > +static void drm_work_fence_work(struct work_struct *w) {
> > +   struct drm_work_fence *wfence =
> > +           container_of(w, struct drm_work_fence, work);
> > +
> > +   wfence->ops->work(wfence);
> > +   drm_work_fence_put(wfence);
> > +}
> > +
> > +static void drm_work_fence_cb(struct dma_fence *fence, struct
> > +dma_fence_cb *cb) {
> > +   struct drm_work_fence *wfence =
> > +           container_of(cb, struct drm_work_fence, cb);
> > +
> > +   queue_work(wfence->wq, &wfence->work);
> > +   /*
> > +    * Put the transferred reference from add_callback. The stored
> > +    * reference in wfence->fence is released in drm_work_fence_destroy().
> > +    */
> > +   dma_fence_put(fence);
> > +}
> > +
> > +/**
> > + * drm_work_fence_init - Initialize a work fence
> > + * @wfence: work fence to initialize
> > + * @wq: workqueue to run the worker on (must be ordered if sequencing
> > +matters)
> > + * @ops: driver operations
> > + */
> > +void drm_work_fence_init(struct drm_work_fence *wfence,
> > +                    struct workqueue_struct *wq,
> > +                    const struct drm_work_fence_ops *ops) {
> > +   kref_init(&wfence->refcount);
> > +   wfence->wq = wq;
> > +   wfence->ops = ops;
> > +   wfence->fence = NULL;
> > +   INIT_WORK(&wfence->work, drm_work_fence_work); }
> > +EXPORT_SYMBOL_GPL(drm_work_fence_init);
> > +
> > +/**
> > + * drm_work_fence_add_callback - Attach a work fence to a dma-fence
> > + * @wfence: work fence
> > + * @fence: dma-fence to watch; ownership of this reference is transferred
> > + *         to the callback — caller must NOT put it afterward.
>
> This isn't right. It is perfectly reasonable for caller to hold more than 1 reference to
> @fence, thus put it again. It consumes a single reference @fence on success or
> failure - that is it.
>
> > + *
> > + * When @fence signals, a work item is queued that calls ops->work().
> > + * If @fence has already signaled, the work item is queued immediately.
> > + *
> > + * An additional reference to @fence is stored internally in @wfence
> > + to
> > + * allow drm_work_fence_cancel() to be called safely without the
> > + caller
> > + * needing to hold a separate fence reference.
> > + *
>
> Ideally get rid of double ref count on @fence. I don't think above reasoning justifies
> the needed for a double ref on the fence. I'd tie exactly one refernece @fence which
> is attached to lifetime of @wfence (i.e., drop the dma_fence_put in
> drm_work_fence_cb).
>
> > + * On any return value the caller's fence reference is consumed.
> > + *
>
> I'd mention regardless of success or fail, a reference to drm_work_fence is
> consumed too.
>
> > + * Return: 0 on success, negative errno on error.
> > + */
> > +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> > +                           struct dma_fence *fence)
> > +{
> > +   int err;
> > +
> > +   drm_work_fence_get(wfence);
> > +   wfence->fence = dma_fence_get(fence);
> > +
> > +   err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
> > +   if (err == -ENOENT) {
> > +           queue_work(wfence->wq, &wfence->work);
> > +           dma_fence_put(fence);
>
> Keep the implementation in one place?
>
> drm_work_fence_work(&wfence->work);

Hi Matt,

Thanks for your feedbacks once again!,

For the ENOENT path — I'm planning to extract a small shared helper:

    static void drm_work_fence_queue(struct drm_work_fence *wfence)
    {
        queue_work(wfence->wq, &wfence->work);
    }

and call it from both drm_work_fence_cb() and the ENOENT path in
add_callback(). This keeps the implementation in one place while
preserving async execution.

May I kno pls, is that what you had in mind, or did you mean something different?

Thanks,
Srini

Re: [PATCH v6 1/4] drm: Add drm_work_fence helper

[Matthew Brost]

On Tue, Sep 01, 2026 at 07:39:40AM +0000, SHANMUGAM, SRINIVASAN wrote:
> AMD General
> 
> > -----Original Message-----
> > From: Matthew Brost <matthew.brost@intel.com>
> > Sent: Tuesday, September 1, 2026 1:52 AM
> > To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>; dri-
> > devel@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Koenig, Christian
> > <Christian.Koenig@amd.com>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org; Maarten
> > Lankhorst <maarten.lankhorst@linux.intel.com>
> > Subject: Re: [PATCH v6 1/4] drm: Add drm_work_fence helper
> >
> > On Mon, Aug 31, 2026 at 07:15:36PM +0530, Srinivasan Shanmugam wrote:
> > > GPU drivers often need to queue work when a dma-fence signals because
> > > certain operations (copy_to_user, eventfd_signal, memory
> > > allocation) cannot run in IRQ context. This pattern is currently
> > > open-coded in multiple drivers.
> > >
> > > Introduce drm_work_fence — an embeddable base structure that handles
> > > the dma-fence-callback-to-workqueue pattern in one place. Drivers
> > > embed this in their own structure and implement ops->work() for the
> > > deferred work and ops->destroy() for cleanup.
> > >
> > > The helper manages:
> > >  - kref lifetime
> > >  - dma-fence callback registration
> > >  - workqueue dispatch on fence signal
> > >  - safe cancellation before driver teardown
> > >
> > > For work that additionally requires borrowing the process MM via
> > > kthread_use_mm(), see drm_user_fence which builds on top of this.
> > >
> > > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Christian König <christian.koenig@amd.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: intel-xe@lists.freedesktop.org
> > > Cc: amd-gfx@lists.freedesktop.org
> > > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> > > ---
> > >  drivers/gpu/drm/Makefile         |   1 +
> > >  drivers/gpu/drm/drm_work_fence.c | 195
> > +++++++++++++++++++++++++++++++
> > >  include/drm/drm_work_fence.h     |  76 ++++++++++++
> > >  3 files changed, 272 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/drm_work_fence.c  create mode
> > > 100644 include/drm/drm_work_fence.h
> > >
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index
> > > e97faabcd783..c5be8e80d0c8 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -72,6 +72,7 @@ drm-y := \
> > >     drm_vblank.o \
> > >     drm_vblank_work.o \
> > >     drm_vma_manager.o \
> > > +   drm_work_fence.o \
> > >     drm_writeback.o
> > >  drm-$(CONFIG_DRM_CLIENT) += \
> > >     drm_client.o \
> > > diff --git a/drivers/gpu/drm/drm_work_fence.c
> > > b/drivers/gpu/drm/drm_work_fence.c
> > > new file mode 100644
> > > index 000000000000..9f6b779d0fe9
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/drm_work_fence.c
> > > @@ -0,0 +1,195 @@
> > > +// SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2024 The Linux Foundation
> > > + *
> > > + * Common DRM work fence helper.
> > > + *
> > > + * When a GPU dma-fence signals, drivers often need to perform work
> > > +that
> > > + * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
> > > + * eventfd_signal). This helper queues a work item when a dma-fence
> > > + * signals, allowing that work to run safely in a workqueue context.
> > > + *
> > > + * NOTE: This helper consumes dma_fences but CANNOT implement
> > > + * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
> > > + * callbacks are called under the fence spinlock and must not sleep.
> > > + *
> > > + * For work that additionally requires accessing userspace memory via
> > > + * kthread_use_mm(), see drm_user_fence which builds on top of this.
> > > + */
> > > +
> > > +#include <linux/workqueue.h>
> > > +
> > > +#include <drm/drm_work_fence.h>
> > > +
> > > +static void drm_work_fence_destroy(struct kref *kref) {
> > > +   struct drm_work_fence *wfence =
> > > +           container_of(kref, struct drm_work_fence, refcount);
> > > +
> > > +   if (wfence->fence)
> > > +           dma_fence_put(wfence->fence);
> > > +
> > > +   wfence->ops->destroy(wfence);
> >
> > I'd invert these for safety in case destroy wants to looks at the fence, admittedly
> > that would be an odd use case.
> >
> > So...
> >
> >       struct drm_work_fence *wfence =
> >               container_of(kref, struct drm_work_fence, refcount);
> >       struct dma_fence *fence = wfence->fence;
> >
> >       wfence->ops->destroy(wfence);
> >       dma_fence_put(fence);   /* this has a NULL check */
> >
> >
> > > +}
> > > +
> > > +/**
> > > + * drm_work_fence_get - Acquire a reference to a work fence
> > > + * @wfence: work fence
> > > + */
> > > +void drm_work_fence_get(struct drm_work_fence *wfence) {
> > > +   kref_get(&wfence->refcount);
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_work_fence_get);
> > > +
> > > +/**
> > > + * drm_work_fence_put - Release a reference to a work fence
> > > + * @wfence: work fence
> > > + */
> > > +void drm_work_fence_put(struct drm_work_fence *wfence) {
> > > +   kref_put(&wfence->refcount, drm_work_fence_destroy); }
> > > +EXPORT_SYMBOL_GPL(drm_work_fence_put);
> > > +
> > > +static void drm_work_fence_work(struct work_struct *w) {
> > > +   struct drm_work_fence *wfence =
> > > +           container_of(w, struct drm_work_fence, work);
> > > +
> > > +   wfence->ops->work(wfence);
> > > +   drm_work_fence_put(wfence);
> > > +}
> > > +
> > > +static void drm_work_fence_cb(struct dma_fence *fence, struct
> > > +dma_fence_cb *cb) {
> > > +   struct drm_work_fence *wfence =
> > > +           container_of(cb, struct drm_work_fence, cb);
> > > +
> > > +   queue_work(wfence->wq, &wfence->work);
> > > +   /*
> > > +    * Put the transferred reference from add_callback. The stored
> > > +    * reference in wfence->fence is released in drm_work_fence_destroy().
> > > +    */
> > > +   dma_fence_put(fence);
> > > +}
> > > +
> > > +/**
> > > + * drm_work_fence_init - Initialize a work fence
> > > + * @wfence: work fence to initialize
> > > + * @wq: workqueue to run the worker on (must be ordered if sequencing
> > > +matters)
> > > + * @ops: driver operations
> > > + */
> > > +void drm_work_fence_init(struct drm_work_fence *wfence,
> > > +                    struct workqueue_struct *wq,
> > > +                    const struct drm_work_fence_ops *ops) {
> > > +   kref_init(&wfence->refcount);
> > > +   wfence->wq = wq;
> > > +   wfence->ops = ops;
> > > +   wfence->fence = NULL;
> > > +   INIT_WORK(&wfence->work, drm_work_fence_work); }
> > > +EXPORT_SYMBOL_GPL(drm_work_fence_init);
> > > +
> > > +/**
> > > + * drm_work_fence_add_callback - Attach a work fence to a dma-fence
> > > + * @wfence: work fence
> > > + * @fence: dma-fence to watch; ownership of this reference is transferred
> > > + *         to the callback — caller must NOT put it afterward.
> >
> > This isn't right. It is perfectly reasonable for caller to hold more than 1 reference to
> > @fence, thus put it again. It consumes a single reference @fence on success or
> > failure - that is it.
> >
> > > + *
> > > + * When @fence signals, a work item is queued that calls ops->work().
> > > + * If @fence has already signaled, the work item is queued immediately.
> > > + *
> > > + * An additional reference to @fence is stored internally in @wfence
> > > + to
> > > + * allow drm_work_fence_cancel() to be called safely without the
> > > + caller
> > > + * needing to hold a separate fence reference.
> > > + *
> >
> > Ideally get rid of double ref count on @fence. I don't think above reasoning justifies
> > the needed for a double ref on the fence. I'd tie exactly one refernece @fence which
> > is attached to lifetime of @wfence (i.e., drop the dma_fence_put in
> > drm_work_fence_cb).
> >
> > > + * On any return value the caller's fence reference is consumed.
> > > + *
> >
> > I'd mention regardless of success or fail, a reference to drm_work_fence is
> > consumed too.
> >
> > > + * Return: 0 on success, negative errno on error.
> > > + */
> > > +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> > > +                           struct dma_fence *fence)
> > > +{
> > > +   int err;
> > > +
> > > +   drm_work_fence_get(wfence);
> > > +   wfence->fence = dma_fence_get(fence);
> > > +
> > > +   err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
> > > +   if (err == -ENOENT) {
> > > +           queue_work(wfence->wq, &wfence->work);
> > > +           dma_fence_put(fence);
> >
> > Keep the implementation in one place?
> >
> > drm_work_fence_work(&wfence->work);

This is a bad suggestion actually, I was a bit distracted I guess - you
can't directly execute the worker at least in Xe as
drm_work_fence_add_callback is called holding the dma-resv lock and copy
to user can take mmap_read lock and we'd insert.

> 
> Hi Matt,
> 
> Thanks for your feedbacks once again!,
> 
> For the ENOENT path — I'm planning to extract a small shared helper:
> 
>     static void drm_work_fence_queue(struct drm_work_fence *wfence)
>     {
>         queue_work(wfence->wq, &wfence->work);
>     }
> 
> and call it from both drm_work_fence_cb() and the ENOENT path in
> add_callback(). This keeps the implementation in one place while
> preserving async execution.
> 

Yes, basically whatever drm_work_fence_cb does, stick into a helper and
call it here so if implementation diverges for the CB, we only have to
change it in one place.

Matt


> May I kno pls, is that what you had in mind, or did you mean something different?
> 
> Thanks,
> Srini

[PATCH v7 1/4] drm: Add drm_work_fence helper

[Srinivasan Shanmugam]

GPU drivers often need to queue work when a dma-fence signals
because certain operations (copy_to_user, eventfd_signal, memory
allocation) cannot run in IRQ context. This pattern is currently
open-coded in multiple drivers.

Introduce drm_work_fence — an embeddable base structure that handles the
dma-fence-callback-to-workqueue pattern in one place. Drivers embed this
in their own structure and implement ops->writeback() for the deferred
work and ops->destroy() for cleanup.

The helper manages:
 - kref lifetime
 - dma-fence callback registration
 - workqueue dispatch on fence signal
 - safe cancellation before driver teardown

For work that additionally requires borrowing the process MM via
kthread_use_mm(), see drm_user_fence which builds on top of this.

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_work_fence.c | 184 +++++++++++++++++++++++++++++++
 include/drm/drm_work_fence.h     |  69 ++++++++++++
 3 files changed, 254 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_work_fence.c
 create mode 100644 include/drm/drm_work_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..c5be8e80d0c8 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -72,6 +72,7 @@ drm-y := \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
+	drm_work_fence.o \
 	drm_writeback.o
 drm-$(CONFIG_DRM_CLIENT) += \
 	drm_client.o \
diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
new file mode 100644
index 000000000000..482eb3d4475f
--- /dev/null
+++ b/drivers/gpu/drm/drm_work_fence.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * Common DRM work fence helper.
+ *
+ * When a GPU dma-fence signals, drivers often need to perform work that
+ * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
+ * eventfd_signal). This helper queues a work item when a dma-fence
+ * signals, allowing that work to run safely in a workqueue context.
+ *
+ * NOTE: This helper consumes dma_fences but CANNOT implement
+ * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
+ * callbacks are called under the fence spinlock and must not sleep.
+ *
+ * For work that additionally requires accessing userspace memory via
+ * kthread_use_mm(), see drm_user_fence which builds on top of this.
+ */
+
+#include <linux/workqueue.h>
+
+#include <drm/drm_work_fence.h>
+
+static void drm_work_fence_destroy(struct kref *kref)
+{
+	struct drm_work_fence *wfence =
+		container_of(kref, struct drm_work_fence, refcount);
+	struct dma_fence *fence = wfence->fence;
+
+	wfence->ops->destroy(wfence);
+	dma_fence_put(fence);	/* NULL-safe */
+}
+
+/**
+ * drm_work_fence_get - Acquire a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_get(struct drm_work_fence *wfence)
+{
+	kref_get(&wfence->refcount);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_get);
+
+/**
+ * drm_work_fence_put - Release a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_put(struct drm_work_fence *wfence)
+{
+	kref_put(&wfence->refcount, drm_work_fence_destroy);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_put);
+
+static void drm_work_fence_work(struct work_struct *w)
+{
+	struct drm_work_fence *wfence =
+		container_of(w, struct drm_work_fence, work);
+
+	wfence->ops->writeback(wfence);
+	drm_work_fence_put(wfence);
+}
+
+static void drm_work_fence_queue(struct drm_work_fence *wfence)
+{
+	queue_work(wfence->wq, &wfence->work);
+}
+
+static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+{
+	struct drm_work_fence *wfence =
+		container_of(cb, struct drm_work_fence, cb);
+
+	drm_work_fence_queue(wfence);
+	/* Single ref: wfence->fence released in drm_work_fence_destroy(). */
+}
+
+/**
+ * drm_work_fence_init - Initialize a work fence
+ * @wfence: work fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ */
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops)
+{
+	kref_init(&wfence->refcount);
+	wfence->wq = wq;
+	wfence->ops = ops;
+	wfence->fence = NULL;
+	INIT_WORK(&wfence->work, drm_work_fence_work);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_init);
+
+/**
+ * drm_work_fence_add_callback - Attach a work fence to a dma-fence
+ * @wfence: work fence; caller retains their reference and must release
+ *          it via drm_work_fence_put() when no longer needed
+ * @fence: dma-fence to watch; one reference is consumed on any return value
+ *
+ * When @fence signals, a work item is queued that calls ops->writeback().
+ * If @fence has already signaled, the work item is queued immediately.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				struct dma_fence *fence)
+{
+	int err;
+
+	drm_work_fence_get(wfence);
+	wfence->fence = fence;	/* transfer caller's ref — single ref, no get */
+
+	err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
+	if (err == -ENOENT) {
+		drm_work_fence_queue(wfence);
+		err = 0;
+	} else if (err) {
+		wfence->fence = NULL;
+		dma_fence_put(fence);
+		drm_work_fence_put(wfence);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_add_callback);
+
+/**
+ * drm_work_fence_cancel - Cancel a pending work fence callback
+ * @wfence: work fence
+ *
+ * Attempts to remove the pending callback before driver context teardown.
+ * The caller must hold a reference to @wfence across this call.
+ *
+ * If the callback has already fired this returns false and all cleanup
+ * has been handled internally.
+ *
+ * If removal succeeds the callback reference is released internally.
+ * The caller must still release its own reference via drm_work_fence_put().
+ *
+ * This function is safe to call from atomic context as it only acquires
+ * the dma-fence spinlock internally. If the caller also needs to wait
+ * for the worker to finish, use drm_work_fence_cancel_sync() instead,
+ * which may sleep.
+ *
+ * Return: true if callback was removed, false if it had already fired.
+ */
+bool drm_work_fence_cancel(struct drm_work_fence *wfence)
+{
+	struct dma_fence *fence = wfence->fence;
+
+	if (!fence)
+		return false;
+
+	if (dma_fence_remove_callback(fence, &wfence->cb)) {
+		drm_work_fence_put(wfence);	/* drop ref from add_callback */
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_cancel);
+
+/**
+ * drm_work_fence_cancel_sync - Cancel callback and wait for worker to finish
+ * @wfence: work fence
+ *
+ * Calls drm_work_fence_cancel() then cancel_work_sync() to guarantee
+ * the worker has fully completed before returning.
+ *
+ * This function may sleep. Must not be called from atomic or interrupt
+ * context. Use drm_work_fence_cancel() instead when sleeping is not allowed.
+ *
+ * Drivers must call this during teardown before freeing any resources
+ * accessed by ops->writeback().
+ */
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
+{
+	if (drm_work_fence_cancel(wfence))
+		return;
+	if (cancel_work_sync(&wfence->work))
+		drm_work_fence_put(wfence);
+}
+EXPORT_SYMBOL_GPL(drm_work_fence_cancel_sync);
diff --git a/include/drm/drm_work_fence.h b/include/drm/drm_work_fence.h
new file mode 100644
index 000000000000..c8e3c5b9f0c4
--- /dev/null
+++ b/include/drm/drm_work_fence.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_WORK_FENCE_H__
+#define __DRM_WORK_FENCE_H__
+
+#include <linux/dma-fence.h>
+#include <linux/kref.h>
+#include <linux/workqueue.h>
+
+struct drm_work_fence;
+
+/**
+ * struct drm_work_fence_ops - driver callbacks for a DRM work fence
+ */
+struct drm_work_fence_ops {
+	/**
+	 * @writeback: Called from workqueue context when the dma-fence signals.
+	 *
+	 * Perform the deferred work here (copy_to_user, eventfd_signal, etc.).
+	 * May sleep. Must not requeue the fence.
+	 */
+	void (*writeback)(struct drm_work_fence *wfence);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_work_fence *wfence);
+};
+
+/**
+ * struct drm_work_fence - DRM dma-fence-to-workqueue helper
+ *
+ * Embeddable base structure that queues a work item when a dma-fence signals.
+ * Drivers embed this in their own structure and implement ops->writeback()
+ * for the deferred work and ops->destroy() for cleanup.
+ *
+ * Call drm_work_fence_init() at creation and drm_work_fence_add_callback()
+ * to arm on a dma-fence. Call drm_work_fence_cancel_sync() before teardown.
+ */
+struct drm_work_fence {
+	/** @refcount: Reference count. */
+	struct kref refcount;
+	/** @wq: Workqueue on which to run the worker. */
+	struct workqueue_struct *wq;
+	/** @ops: Driver operations. */
+	const struct drm_work_fence_ops *ops;
+	/** @fence: The watched dma-fence; holds a single reference. */
+	struct dma_fence *fence;
+	/** @work: Work item queued when the fence signals. */
+	struct work_struct work;
+	/** @cb: Callback registered on the dma-fence. */
+	struct dma_fence_cb cb;
+};
+
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops);
+void drm_work_fence_get(struct drm_work_fence *wfence);
+void drm_work_fence_put(struct drm_work_fence *wfence);
+int  drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				 struct dma_fence *fence);
+bool drm_work_fence_cancel(struct drm_work_fence *wfence);
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence);
+
+#endif /* __DRM_WORK_FENCE_H__ */
-- 
2.34.1

[PATCH v7 2/4] drm: Add drm_user_fence helper

[Srinivasan Shanmugam]

Introduce a common DRM user fence helper providing the kref-managed,
MM-borrowing dma-fence-callback-to-workqueue pattern used by drivers
that must access userspace memory from a kthread context when a GPU
fence signals.

XE uses this pattern (xe_sync.c) to write a fence completion value
to a userspace VA. AMDGPU will use the same pattern to signal a
per-queue eventfd from a user-queue EOP fence callback.

The helper provides:
 - struct drm_user_fence: embeddable base structure
 - struct drm_user_fence_ops: worker/destroy callbacks
 - drm_user_fence_init(): initialize and grab the process MM
 - drm_user_fence_get/put(): reference counting
 - drm_user_fence_add_callback(): attach to a dma-fence

The worker callback receives a bool indicating whether the process
MM was successfully obtained, allowing drivers to handle the
unavailable-MM case (log, skip the userspace write, etc.) without
duplicating the mmget/kthread_use_mm/mmput boilerplate.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Change-Id: I09da42c688392326ed78235b302fba893e570eff
---
 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_user_fence.c |  70 ++++++++++++++++++
 include/drm/drm_user_fence.h     | 122 +++++++++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 include/drm/drm_user_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index c5be8e80d0c8..ddb770738992 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -69,6 +69,7 @@ drm-y := \
 	drm_syncobj.o \
 	drm_sysfs.o \
 	drm_trace_points.o \
+	drm_user_fence.o \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
new file mode 100644
index 000000000000..0f229b7210a9
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * DRM user fence — extends drm_work_fence with kthread_use_mm() support.
+ *
+ * Use this when a GPU fence signals and work needs to access userspace
+ * memory (copy_to_user, fault-able operations) from a kthread context.
+ * For work that does not require userspace memory access, use
+ * drm_work_fence directly.
+ */
+
+#include <linux/kthread.h>
+#include <linux/sched/mm.h>
+
+#include <drm/drm_user_fence.h>
+
+static void drm_user_fence_do_destroy(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+	struct mm_struct *mm = ufence->mm;
+
+	ufence->ops->destroy(ufence);
+	mmdrop(mm);
+}
+
+static void drm_user_fence_do_work(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+	struct mm_struct *mm = NULL;
+
+	if (mmget_not_zero(ufence->mm)) {
+		mm = ufence->mm;
+		kthread_use_mm(mm);
+	}
+
+	ufence->ops->worker(ufence, !!mm);
+
+	if (mm) {
+		kthread_unuse_mm(mm);
+		mmput_async(mm);
+	}
+}
+
+static const struct drm_work_fence_ops drm_user_fence_wfence_ops = {
+	.writeback = drm_user_fence_do_work,
+	.destroy   = drm_user_fence_do_destroy,
+};
+
+/**
+ * drm_user_fence_init - Initialize a user fence
+ * @ufence: user fence to initialize
+ * @wq: workqueue on which to run the worker
+ * @ops: driver operations
+ *
+ * Must be called from process context with a valid current->mm.
+ * Grabs a reference to current->mm via mmgrab().
+ */
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops)
+{
+	drm_work_fence_init(&ufence->base, wq, &drm_user_fence_wfence_ops);
+	ufence->mm = current->mm;
+	mmgrab(ufence->mm);
+	ufence->ops = ops;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_init);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
new file mode 100644
index 000000000000..d35438eaa9e2
--- /dev/null
+++ b/include/drm/drm_user_fence.h
@@ -0,0 +1,122 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_USER_FENCE_H__
+#define __DRM_USER_FENCE_H__
+
+#include <linux/dma-fence.h>
+
+#include <drm/drm_work_fence.h>
+
+struct drm_user_fence;
+
+/**
+ * struct drm_user_fence_ops - driver callbacks for a DRM user fence
+ */
+struct drm_user_fence_ops {
+	/**
+	 * @worker: Called from workqueue context with the process MM active.
+	 *
+	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
+	 * (copy_to_user, etc.) may be accessed safely.
+	 * If @mm_ok is false, the process MM was already gone; skip the
+	 * userspace write.
+	 *
+	 * wake_up() or other post-signal housekeeping should also happen here.
+	 *
+	 * WARNING: Fault-able operations such as copy_to_user() may block
+	 * indefinitely if userspace registers the target address with
+	 * userfaultfd or backs it with a FUSE mount. Drivers that cannot
+	 * tolerate blocking should use copy_to_user_nofault() instead.
+	 */
+	void (*worker)(struct drm_user_fence *ufence, bool mm_ok);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_user_fence *ufence);
+};
+
+/**
+ * struct drm_user_fence - DRM user fence with MM borrowing
+ *
+ * Extends drm_work_fence with kthread_use_mm() support for drivers
+ * that need to access userspace memory when a GPU fence signals.
+ *
+ * Call drm_user_fence_init() at creation and drm_user_fence_add_callback()
+ * to arm on a dma-fence. Call drm_user_fence_cancel_sync() before teardown.
+ */
+struct drm_user_fence {
+	/** @base: Base work fence. Must be first. */
+	struct drm_work_fence base;
+	/** @mm: Process MM grabbed at init time. */
+	struct mm_struct *mm;
+	/** @ops: Driver operations. */
+	const struct drm_user_fence_ops *ops;
+};
+
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops);
+
+/**
+ * drm_user_fence_get - Acquire a reference to a user fence
+ * @ufence: user fence
+ */
+static inline void drm_user_fence_get(struct drm_user_fence *ufence)
+{
+	drm_work_fence_get(&ufence->base);
+}
+
+/**
+ * drm_user_fence_put - Release a reference to a user fence
+ * @ufence: user fence
+ */
+static inline void drm_user_fence_put(struct drm_user_fence *ufence)
+{
+	drm_work_fence_put(&ufence->base);
+}
+
+/**
+ * drm_user_fence_add_callback - Attach a user fence to a dma-fence
+ * @ufence: user fence; caller retains their reference and must release
+ *          it via drm_user_fence_put() when no longer needed
+ * @fence: dma-fence to watch; one reference is consumed on any return value
+ *
+ * When @fence signals, ops->worker() is called from workqueue context.
+ * If @fence has already signaled, the worker is queued immediately.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static inline int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+					      struct dma_fence *fence)
+{
+	return drm_work_fence_add_callback(&ufence->base, fence);
+}
+
+/**
+ * drm_user_fence_cancel - Cancel a pending user fence callback
+ * @ufence: user fence
+ *
+ * Return: true if callback was removed, false if it had already fired.
+ */
+static inline bool drm_user_fence_cancel(struct drm_user_fence *ufence)
+{
+	return drm_work_fence_cancel(&ufence->base);
+}
+
+/**
+ * drm_user_fence_cancel_sync - Cancel callback and wait for worker to finish
+ * @ufence: user fence
+ *
+ * Must be called during teardown before freeing resources. May sleep.
+ */
+static inline void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
+{
+	drm_work_fence_cancel_sync(&ufence->base);
+}
+
+#endif /* __DRM_USER_FENCE_H__ */
-- 
2.34.1

[PATCH v7 3/4] drm/xe: Convert xe_user_fence to drm_user_fence

[Srinivasan Shanmugam]

Replace the open-coded user fence implementation in xe_sync.c with the
new common drm_user_fence helper.

struct xe_user_fence now embeds struct drm_user_fence as its base.
XE-specific fields (xe_device pointer for the ufence_wq wake-up,
userspace VA, expected value, signalled flag) remain in the wrapper.

The local user_fence_destroy/get/put/worker/kick_ufence/user_fence_cb
functions are removed. Their logic moves to xe_ufence_ops.worker and
xe_ufence_ops.destroy, which are called by the drm_user_fence helper.

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/xe/xe_sync.c       | 147 ++++++++++++++++-------------
 drivers/gpu/drm/xe/xe_sync.h       |   2 +
 drivers/gpu/drm/xe/xe_sync_types.h |   1 -
 drivers/gpu/drm/xe/xe_vm.c         |   1 +
 4 files changed, 82 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
index 37866768d64c..05f6794af1ee 100644
--- a/drivers/gpu/drm/xe/xe_sync.c
+++ b/drivers/gpu/drm/xe/xe_sync.c
@@ -6,12 +6,11 @@
 #include "xe_sync.h"
 
 #include <linux/dma-fence-array.h>
-#include <linux/kthread.h>
-#include <linux/sched/mm.h>
 #include <linux/uaccess.h>
 
 #include <drm/drm_print.h>
 #include <drm/drm_syncobj.h>
+#include <drm/drm_user_fence.h>
 #include <uapi/drm/xe_drm.h>
 
 #include "xe_device.h"
@@ -19,36 +18,58 @@
 #include "xe_macros.h"
 #include "xe_sched_job_types.h"
 
+/*
+ * xe_user_fence wraps drm_user_fence with XE-specific fields.
+ * The drm_user_fence base handles MM borrowing and work-item lifetime.
+ */
 struct xe_user_fence {
-	struct xe_device *xe;
-	struct kref refcount;
-	struct dma_fence_cb cb;
-	struct work_struct worker;
-	struct mm_struct *mm;
-	u64 __user *addr;
-	u64 value;
-	int signalled;
+	struct drm_user_fence base;
+	struct xe_device     *xe;
+	u64 __user           *addr;
+	u64                   value;
+	int                   signalled;
 };
 
-static void user_fence_destroy(struct kref *kref)
+static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok)
 {
-	struct xe_user_fence *ufence = container_of(kref, struct xe_user_fence,
-						 refcount);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
 
-	mmdrop(ufence->mm);
-	kfree(ufence);
-}
+	if (mm_ok) {
+		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
+			drm_dbg(&ufence->xe->drm,
+				"copy_to_user failed, user fence wasn't signaled\n");
+	} else {
+		drm_dbg(&ufence->xe->drm,
+			"mmget_not_zero() failed, ufence wasn't signaled\n");
+	}
 
-static void user_fence_get(struct xe_user_fence *ufence)
-{
-	kref_get(&ufence->refcount);
+	/*
+	 * Ensure the fence value write is visible before signalled=1.
+	 * A UMD polling signalled must see the committed fence value.
+	 */
+	smp_wmb();
+
+	/*
+	 * Mark signalled after the user memory write so UMD can safely
+	 * reuse the same ufence without hitting -EBUSY.
+	 */
+	WRITE_ONCE(ufence->signalled, 1);
+
+	wake_up_all(&ufence->xe->ufence_wq);
 }
 
-static void user_fence_put(struct xe_user_fence *ufence)
+static void xe_ufence_destroy(struct drm_user_fence *base)
 {
-	kref_put(&ufence->refcount, user_fence_destroy);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
+
+	kfree(ufence);
 }
 
+static const struct drm_user_fence_ops xe_ufence_ops = {
+	.worker  = xe_ufence_worker,
+	.destroy = xe_ufence_destroy,
+};
+
 static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 					       u64 value)
 {
@@ -63,51 +84,22 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 	if (!ufence)
 		return ERR_PTR(-ENOMEM);
 
-	ufence->xe = xe;
-	kref_init(&ufence->refcount);
-	ufence->addr = ptr;
+	ufence->xe    = xe;
+	ufence->addr  = ptr;
 	ufence->value = value;
-	ufence->mm = current->mm;
-	mmgrab(ufence->mm);
+	drm_user_fence_init(&ufence->base, xe->ordered_wq, &xe_ufence_ops);
 
 	return ufence;
 }
 
-static void user_fence_worker(struct work_struct *w)
-{
-	struct xe_user_fence *ufence = container_of(w, struct xe_user_fence, worker);
-
-	WRITE_ONCE(ufence->signalled, 1);
-	if (mmget_not_zero(ufence->mm)) {
-		kthread_use_mm(ufence->mm);
-		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
-			XE_WARN_ON("Copy to user failed");
-		kthread_unuse_mm(ufence->mm);
-		mmput(ufence->mm);
-	} else {
-		drm_dbg(&ufence->xe->drm, "mmget_not_zero() failed, ufence wasn't signaled\n");
-	}
-
-	/*
-	 * Wake up waiters only after updating the ufence state, allowing the UMD
-	 * to safely reuse the same ufence without encountering -EBUSY errors.
-	 */
-	wake_up_all(&ufence->xe->ufence_wq);
-	user_fence_put(ufence);
-}
-
-static void kick_ufence(struct xe_user_fence *ufence, struct dma_fence *fence)
+static void user_fence_get(struct xe_user_fence *ufence)
 {
-	INIT_WORK(&ufence->worker, user_fence_worker);
-	queue_work(ufence->xe->ordered_wq, &ufence->worker);
-	dma_fence_put(fence);
+	drm_user_fence_get(&ufence->base);
 }
 
-static void user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+static void user_fence_put(struct xe_user_fence *ufence)
 {
-	struct xe_user_fence *ufence = container_of(cb, struct xe_user_fence, cb);
-
-	kick_ufence(ufence, fence);
+	drm_user_fence_put(&ufence->base);
 }
 
 int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
@@ -282,24 +274,15 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
 	} else if (sync->syncobj) {
 		drm_syncobj_replace_fence(sync->syncobj, fence);
 	} else if (sync->ufence) {
-		int err;
-
 		drm_syncobj_add_point(sync->ufence_syncobj,
 				      sync->ufence_chain_fence,
 				      fence, sync->ufence_timeline_value);
 		sync->ufence_chain_fence = NULL;
 
 		fence = drm_syncobj_fence_get(sync->ufence_syncobj);
-		user_fence_get(sync->ufence);
-		err = dma_fence_add_callback(fence, &sync->ufence->cb,
-					     user_fence_cb);
-		if (err == -ENOENT) {
-			kick_ufence(sync->ufence, fence);
-		} else if (err) {
+		if (drm_user_fence_add_callback(&sync->ufence->base, fence))
 			XE_WARN_ON("failed to add user fence");
-			user_fence_put(sync->ufence);
-			dma_fence_put(fence);
-		}
+		/* fence ref consumed by drm_user_fence_add_callback */
 	}
 }
 
@@ -434,6 +417,34 @@ void xe_sync_ufence_put(struct xe_user_fence *ufence)
 	user_fence_put(ufence);
 }
 
+/**
+ * xe_sync_ufence_cancel() - Non-blocking cancel of user fence callback
+ * @ufence: user fence reference
+ *
+ * Attempts to cancel the pending callback without waiting for the worker.
+ * Safe to call while holding dma_resv_lock or vm->lock. If the callback
+ * has already fired, the worker runs independently — xe_ufence_worker
+ * only accesses device-level and userspace resources, both safe after
+ * VMA teardown.
+ */
+void xe_sync_ufence_cancel(struct xe_user_fence *ufence)
+{
+	drm_user_fence_cancel(&ufence->base);
+}
+
+/**
+ * xe_sync_ufence_cancel_sync() - Cancel user fence callback and wait for worker
+ * @ufence: user fence reference
+ *
+ * Cancels any pending dma-fence callback and waits for the worker to fully
+ * complete before returning. Must be called during teardown before freeing
+ * any resources accessed by the worker.
+ */
+void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence)
+{
+	drm_user_fence_cancel_sync(&ufence->base);
+}
+
 /**
  * xe_sync_ufence_get_status() - Get user fence status
  * @ufence: user fence
diff --git a/drivers/gpu/drm/xe/xe_sync.h b/drivers/gpu/drm/xe/xe_sync.h
index 6b949194acff..b5ff3387c592 100644
--- a/drivers/gpu/drm/xe/xe_sync.h
+++ b/drivers/gpu/drm/xe/xe_sync.h
@@ -44,6 +44,8 @@ static inline bool xe_sync_is_ufence(struct xe_sync_entry *sync)
 struct xe_user_fence *__xe_sync_ufence_get(struct xe_user_fence *ufence);
 struct xe_user_fence *xe_sync_ufence_get(struct xe_sync_entry *sync);
 void xe_sync_ufence_put(struct xe_user_fence *ufence);
+void xe_sync_ufence_cancel(struct xe_user_fence *ufence);
+void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence);
 int xe_sync_ufence_get_status(struct xe_user_fence *ufence);
 
 #endif
diff --git a/drivers/gpu/drm/xe/xe_sync_types.h b/drivers/gpu/drm/xe/xe_sync_types.h
index b88f1833e28c..67048199b070 100644
--- a/drivers/gpu/drm/xe/xe_sync_types.h
+++ b/drivers/gpu/drm/xe/xe_sync_types.h
@@ -12,7 +12,6 @@ struct drm_syncobj;
 struct dma_fence;
 struct dma_fence_chain;
 struct drm_xe_sync;
-struct user_fence;
 
 struct xe_sync_entry {
 	struct drm_syncobj *syncobj;
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index b01f31ed4417..fbd2c28de8a6 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -1181,6 +1181,7 @@ static void xe_vma_destroy_late(struct xe_vma *vma)
 	struct xe_bo *bo = xe_vma_bo(vma);
 
 	if (vma->ufence) {
+		xe_sync_ufence_cancel(vma->ufence);
 		xe_sync_ufence_put(vma->ufence);
 		vma->ufence = NULL;
 	}
-- 
2.34.1

Re: [PATCH v7 3/4] drm/xe: Convert xe_user_fence to drm_user_fence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Race condition in user fence signaling causes spurious -EBUSY failures for valid VMA unmap operations.

Pre-existing issues:
- [High] Userspace can stall the global xe->ordered_wq indefinitely via userfaultfd on user fence signaling, causing a Denial of Service for all compute applications.
--

commit e66f332fb8f0deb8b5c5b3ca223be49da69834d3
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
drm/xe: Convert xe_user_fence to drm_user_fence

This commit replaces the open-coded user fence implementation in xe_sync.c
with the new common drm_user_fence helper. The xe_user_fence structure now
embeds drm_user_fence and the previous local functions are replaced by
operations provided to the drm helper.

> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> index 37866768d64c2..05f6794af1ee2 100644
> --- a/drivers/gpu/drm/xe/xe_sync.c
> +++ b/drivers/gpu/drm/xe/xe_sync.c
[ ... ]
> @@ -19,36 +18,58 @@
>  #include "xe_macros.h"
>  #include "xe_sched_job_types.h"
>  
> +/*
> + * xe_user_fence wraps drm_user_fence with XE-specific fields.
> + * The drm_user_fence base handles MM borrowing and work-item lifetime.
> + */
>  struct xe_user_fence {
> -	struct xe_device *xe;
> -	struct kref refcount;
> -	struct dma_fence_cb cb;
> -	struct work_struct worker;
> -	struct mm_struct *mm;
> -	u64 __user *addr;
> -	u64 value;
> -	int signalled;
> +	struct drm_user_fence base;
> +	struct xe_device     *xe;
> +	u64 __user           *addr;
> +	u64                   value;
> +	int                   signalled;
>  };
>  
> -static void user_fence_destroy(struct kref *kref)
> +static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok)
>  {
> -	struct xe_user_fence *ufence = container_of(kref, struct xe_user_fence,
> -						 refcount);
> +	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
>  
> -	mmdrop(ufence->mm);
> -	kfree(ufence);
> -}
> +	if (mm_ok) {
> +		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))

[Severity: High]
This is a pre-existing issue, but can this copy_to_user() call cause a
denial of service?

If a malicious unprivileged userspace process creates a user fence using
DRM_XE_SYNC_TYPE_USER_FENCE and provides a memory address backed by
userfaultfd, the page fault will block indefinitely here because userspace
can refuse to resolve it.

> +			drm_dbg(&ufence->xe->drm,
> +				"copy_to_user failed, user fence wasn't signaled\n");
> +	} else {
> +		drm_dbg(&ufence->xe->drm,
> +			"mmget_not_zero() failed, ufence wasn't signaled\n");
> +	}
>  
> -static void user_fence_get(struct xe_user_fence *ufence)
> -{
> -	kref_get(&ufence->refcount);
> +	/*
> +	 * Ensure the fence value write is visible before signalled=1.
> +	 * A UMD polling signalled must see the committed fence value.
> +	 */
> +	smp_wmb();
> +
> +	/*
> +	 * Mark signalled after the user memory write so UMD can safely
> +	 * reuse the same ufence without hitting -EBUSY.
> +	 */
> +	WRITE_ONCE(ufence->signalled, 1);

[Severity: High]
Does this new ordering introduce a race condition causing spurious -EBUSY
failures for valid VMA unmap operations?

The original code updated the ufence->signalled state before calling
copy_to_user(). By inverting the order, if userspace is polling the memory
address, it will observe the memory update and immediately issue a VM unmap
ioctl. 

If this unmap ioctl reaches the driver before the worker thread executes
WRITE_ONCE(ufence->signalled, 1), the kernel state will still read as 0 and
incorrectly fail the unmap with -EBUSY.

> +
> +	wake_up_all(&ufence->xe->ufence_wq);
>  }
[ ... ]
> @@ -63,51 +84,22 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
>  	if (!ufence)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ufence->xe = xe;
> -	kref_init(&ufence->refcount);
> -	ufence->addr = ptr;
> +	ufence->xe    = xe;
> +	ufence->addr  = ptr;
>  	ufence->value = value;
> -	ufence->mm = current->mm;
> -	mmgrab(ufence->mm);
> +	drm_user_fence_init(&ufence->base, xe->ordered_wq, &xe_ufence_ops);

[Severity: High]
This is a pre-existing issue, but does using the global device-wide
xe->ordered_wq amplify the impact of the userfaultfd block mentioned above?

Since critical tasks like vm->preempt.rebind_work are queued on this same
ordered workqueue, stalling it prevents preempt rebinds globally for all
VMs, completely hanging the GPU driver.

>  
>  	return ufence;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902152049.183685-1-srinivasan.shanmugam@amd.com?part=3

[PATCH v7 4/4] drm: Add per-signal compare functionality to drm_user_fence

[Srinivasan Shanmugam]

GPU drivers sometimes need to read a value from a userspace VA when a
dma-fence signals, compare it with an expected value, and only perform
the deferred work (e.g. eventfd_signal) if the comparison passes. This
is the per-signal filtering pattern used in AMDGPU's EOP eventfd path.

Add optional compare fields to drm_user_fence and a new helper
drm_user_fence_set_compare() to configure them. Supported operators are
==, !=, >=.

When cmp_op is set, drm_user_fence reads the value from userspace via
copy_from_user_nofault() and calls ops->worker() with mm_ok=true only if
the comparison passes. If the process MM is gone or the read fails, the
worker is called with mm_ok=false to allow mandatory housekeeping (e.g.
wake_up()).

Drivers that do not need filtering (e.g. XE) leave cmp_op unset and the
worker is called unconditionally — no behavioral change.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Change-Id: I200dd4bf32286ba9bdd2ecb68fffb3c205f9fd52
---
 drivers/gpu/drm/drm_user_fence.c | 97 +++++++++++++++++++++++++++++---
 include/drm/drm_user_fence.h     | 47 +++++++++++++++-
 2 files changed, 135 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
index 0f229b7210a9..06a5683db10d 100644
--- a/drivers/gpu/drm/drm_user_fence.c
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -2,16 +2,25 @@
 /*
  * Copyright © 2024 The Linux Foundation
  *
- * DRM user fence — extends drm_work_fence with kthread_use_mm() support.
+ * DRM user fence helper.
  *
- * Use this when a GPU fence signals and work needs to access userspace
- * memory (copy_to_user, fault-able operations) from a kthread context.
- * For work that does not require userspace memory access, use
- * drm_work_fence directly.
+ * Extends drm_work_fence with the ability to access userspace memory
+ * from workqueue context by borrowing the process MM via kthread_use_mm().
+ *
+ * Drivers that need to write completion status to userspace (e.g., user
+ * fences, signaling eventfds) embed drm_user_fence and implement
+ * ops->worker() to do the actual write.
+ *
+ * Optionally, drivers may configure a per-signal compare via
+ * drm_user_fence_set_compare(): work is skipped unless the value at a
+ * userspace address matches the expected value at signal time.
  */
 
 #include <linux/kthread.h>
+#include <linux/mm.h>
 #include <linux/sched/mm.h>
+#include <linux/uaccess.h>
+#include <linux/workqueue.h>
 
 #include <drm/drm_user_fence.h>
 
@@ -30,13 +39,46 @@ static void drm_user_fence_do_work(struct drm_work_fence *wfence)
 	struct drm_user_fence *ufence =
 		container_of(wfence, struct drm_user_fence, base);
 	struct mm_struct *mm = NULL;
+	bool call_worker = true;
 
 	if (mmget_not_zero(ufence->mm)) {
 		mm = ufence->mm;
 		kthread_use_mm(mm);
 	}
 
-	ufence->ops->worker(ufence, !!mm);
+	if (ufence->cmp_op != DRM_USER_FENCE_CMP_NONE &&
+	    !(wfence->fence && wfence->fence->error)) {
+		if (!mm) {
+			call_worker = false;
+		} else {
+			__le64 raw;
+
+			/*
+			 * Use copy_from_user_nofault() to prevent a
+			 * userfaultfd-registered page from blocking this
+			 * workqueue thread indefinitely (DoS).
+			 */
+			if (copy_from_user_nofault(&raw, ufence->cmp_addr,
+						   sizeof(raw))) {
+				call_worker = false;
+			} else {
+				/* GPU writes LE; convert before comparing. */
+				u64 cur_val = le64_to_cpu(raw);
+
+				if (!drm_user_fence_cmp_match(cur_val,
+							      ufence->cmp_value,
+							      ufence->cmp_op))
+					call_worker = false;
+			}
+		}
+	}
+
+	/*
+	 * Always invoke the worker so drivers can perform mandatory
+	 * housekeeping (e.g. wake_up()). Pass false if the compare
+	 * filter suppressed the write.
+	 */
+	ufence->ops->worker(ufence, call_worker ? !!mm : false);
 
 	if (mm) {
 		kthread_unuse_mm(mm);
@@ -63,8 +105,47 @@ void drm_user_fence_init(struct drm_user_fence *ufence,
 			 const struct drm_user_fence_ops *ops)
 {
 	drm_work_fence_init(&ufence->base, wq, &drm_user_fence_wfence_ops);
-	ufence->mm = current->mm;
+	ufence->mm        = current->mm;
 	mmgrab(ufence->mm);
-	ufence->ops = ops;
+	ufence->ops       = ops;
+	ufence->cmp_op    = DRM_USER_FENCE_CMP_NONE;
+	ufence->cmp_addr  = NULL;
+	ufence->cmp_value = 0;
 }
 EXPORT_SYMBOL_GPL(drm_user_fence_init);
+
+/**
+ * drm_user_fence_set_compare - Set per-signal compare filter
+ * @ufence: user fence
+ * @addr: 8-byte-aligned userspace address to read from at signal time
+ * @value: expected value to compare against
+ * @op: comparison operator; pass %DRM_USER_FENCE_CMP_NONE to disable
+ *
+ * When @op is not %DRM_USER_FENCE_CMP_NONE, the worker is only called
+ * with mm_ok=true if the value at @addr matches @value according to @op.
+ * The worker is always called for mandatory housekeeping.
+ * If the MM is gone or the read fails, mm_ok is passed as false.
+ *
+ * Must only be called before drm_work_fence_add_callback().
+ */
+void drm_user_fence_set_compare(struct drm_user_fence *ufence,
+				u64 __user *addr, u64 value,
+				enum drm_user_fence_cmp op)
+{
+	/*
+	 * get_user() of u64 is not atomic on 32-bit — caller should not
+	 * reach here on non-64-bit kernels.
+	 */
+	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT)))
+		return;
+
+	if (op != DRM_USER_FENCE_CMP_NONE) {
+		if (!addr || !IS_ALIGNED((unsigned long)addr, sizeof(u64)))
+			return;
+	}
+
+	ufence->cmp_addr  = addr;
+	ufence->cmp_value = value;
+	ufence->cmp_op    = op;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_set_compare);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
index d35438eaa9e2..65287052370f 100644
--- a/include/drm/drm_user_fence.h
+++ b/include/drm/drm_user_fence.h
@@ -40,11 +40,23 @@ struct drm_user_fence_ops {
 	void (*destroy)(struct drm_user_fence *ufence);
 };
 
+/**
+ * enum drm_user_fence_cmp - compare operator for per-signal filtering
+ */
+enum drm_user_fence_cmp {
+	DRM_USER_FENCE_CMP_NONE = 0,
+	DRM_USER_FENCE_CMP_EQ,
+	DRM_USER_FENCE_CMP_NEQ,
+	DRM_USER_FENCE_CMP_GTE,
+};
+
 /**
  * struct drm_user_fence - DRM user fence with MM borrowing
  *
  * Extends drm_work_fence with kthread_use_mm() support for drivers
  * that need to access userspace memory when a GPU fence signals.
+ * For work that does not need userspace memory access, use
+ * drm_work_fence directly.
  *
  * Call drm_user_fence_init() at creation and drm_user_fence_add_callback()
  * to arm on a dma-fence. Call drm_user_fence_cancel_sync() before teardown.
@@ -56,11 +68,20 @@ struct drm_user_fence {
 	struct mm_struct *mm;
 	/** @ops: Driver operations. */
 	const struct drm_user_fence_ops *ops;
+	/** @cmp_addr: Userspace address to read for per-signal compare. */
+	u64 __user *cmp_addr;
+	/** @cmp_value: Expected value for per-signal compare. */
+	u64 cmp_value;
+	/** @cmp_op: Compare operator; DRM_USER_FENCE_CMP_NONE disables. */
+	enum drm_user_fence_cmp cmp_op;
 };
 
 void drm_user_fence_init(struct drm_user_fence *ufence,
 			 struct workqueue_struct *wq,
 			 const struct drm_user_fence_ops *ops);
+void drm_user_fence_set_compare(struct drm_user_fence *ufence,
+				u64 __user *addr, u64 value,
+				enum drm_user_fence_cmp op);
 
 /**
  * drm_user_fence_get - Acquire a reference to a user fence
@@ -112,11 +133,35 @@ static inline bool drm_user_fence_cancel(struct drm_user_fence *ufence)
  * drm_user_fence_cancel_sync - Cancel callback and wait for worker to finish
  * @ufence: user fence
  *
- * Must be called during teardown before freeing resources. May sleep.
+ * Must be called during teardown before freeing any resources accessed
+ * by ops->worker(). May sleep.
  */
 static inline void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
 {
 	drm_work_fence_cancel_sync(&ufence->base);
 }
 
+/**
+ * drm_user_fence_cmp_match - Test a value against the compare filter
+ * @cur_val: value read from userspace (already converted from LE)
+ * @cmp_value: expected value
+ * @op: comparison operator
+ *
+ * Return: true if the comparison passes, false otherwise.
+ */
+static inline bool drm_user_fence_cmp_match(u64 cur_val, u64 cmp_value,
+					    enum drm_user_fence_cmp op)
+{
+	switch (op) {
+	case DRM_USER_FENCE_CMP_EQ:
+		return cur_val == cmp_value;
+	case DRM_USER_FENCE_CMP_NEQ:
+		return cur_val != cmp_value;
+	case DRM_USER_FENCE_CMP_GTE:
+		return cur_val >= cmp_value;
+	default:
+		return false;
+	}
+}
+
 #endif /* __DRM_USER_FENCE_H__ */
-- 
2.34.1

Re: [PATCH v7 4/4] drm: Add per-signal compare functionality to drm_user_fence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] copy_from_user_nofault() disables page faults, causing legitimate completion events to be permanently dropped if the CPU PTE is not populated.
- [Medium] drm_user_fence_set_compare() returns void and silently ignores invalid configuration arguments.
--

commit ff537705b8c71e85d2a5b3d68ffe3a36096823b1
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

drm: Add per-signal compare functionality to drm_user_fence
    
This patch adds optional compare fields to drm_user_fence to support per-signal
filtering, as used in the EOP eventfd path. It introduces
drm_user_fence_set_compare to configure the comparison and modifies the worker
to read the userspace value using copy_from_user_nofault before signaling.

> diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
> index 0f229b7210a9b..06a5683db10d9 100644
> --- a/drivers/gpu/drm/drm_user_fence.c
> +++ b/drivers/gpu/drm/drm_user_fence.c

[ ... ]

> @@ -30,13 +39,46 @@ static void drm_user_fence_do_work(struct drm_work_fence *wfence)

[ ... ]

> +	if (ufence->cmp_op != DRM_USER_FENCE_CMP_NONE &&
> +	    !(wfence->fence && wfence->fence->error)) {
> +		if (!mm) {
> +			call_worker = false;
> +		} else {
> +			__le64 raw;
> +
> +			/*
> +			 * Use copy_from_user_nofault() to prevent a
> +			 * userfaultfd-registered page from blocking this
> +			 * workqueue thread indefinitely (DoS).
> +			 */
> +			if (copy_from_user_nofault(&raw, ufence->cmp_addr,
> +						   sizeof(raw))) {
> +				call_worker = false;

[Severity: High]
Because copy_from_user_nofault disables page faults in drm_user_fence_do_work,
will this return -EFAULT and permanently drop legitimate completion events if
the CPU PTE is not populated?

This can happen if the page is not resident, such as when NUMA balancing
sets PROT_NONE, or if the memory uses lazy mapping and the CPU hasn't
touched it since the GPU wrote to it.

> +			} else {
> +				/* GPU writes LE; convert before comparing. */
> +				u64 cur_val = le64_to_cpu(raw);

[ ... ]

> @@ -63,8 +105,47 @@ void drm_user_fence_init(struct drm_user_fence *ufence,

[ ... ]

> +void drm_user_fence_set_compare(struct drm_user_fence *ufence,
> +				u64 __user *addr, u64 value,
> +				enum drm_user_fence_cmp op)
> +{
> +	/*
> +	 * get_user() of u64 is not atomic on 32-bit — caller should not
> +	 * reach here on non-64-bit kernels.
> +	 */
> +	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT)))
> +		return;

[Severity: Medium]
Because this function returns void, does this early return on unsupported
architectures silently fail without the caller knowing?

If a driver attempts to configure a comparison here, it appears it would
silently leave cmp_op disabled as DRM_USER_FENCE_CMP_NONE, treating the
fence as an unconditional signal.

Could this function return an error code instead so the driver can
propagate the failure to userspace?

> +
> +	if (op != DRM_USER_FENCE_CMP_NONE) {
> +		if (!addr || !IS_ALIGNED((unsigned long)addr, sizeof(u64)))
> +			return;

[Severity: Medium]
Similarly, if a driver passes an unaligned userspace address, does this
return silently bypass the intended logic and leave the fence as an
unconditional signal?

> +	}
> +
> +	ufence->cmp_addr  = addr;
> +	ufence->cmp_value = value;
> +	ufence->cmp_op    = op;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902152049.183685-1-srinivasan.shanmugam@amd.com?part=4

[PATCH v6 2/4] drm: Add drm_user_fence helper

[Srinivasan Shanmugam]

Introduce a common DRM user fence helper providing the kref-managed,
MM-borrowing dma-fence-callback-to-workqueue pattern used by drivers
that must access userspace memory from a kthread context when a GPU
fence signals.

XE uses this pattern (xe_sync.c) to write a fence completion value
to a userspace VA. AMDGPU will use the same pattern to signal a
per-queue eventfd from a user-queue EOP fence callback.

The helper provides:
 - struct drm_user_fence: embeddable base structure
 - struct drm_user_fence_ops: worker/destroy callbacks
 - drm_user_fence_init(): initialize and grab the process MM
 - drm_user_fence_get/put(): reference counting
 - drm_user_fence_add_callback(): attach to a dma-fence

The worker callback receives a bool indicating whether the process
MM was successfully obtained, allowing drivers to handle the
unavailable-MM case (log, skip the userspace write, etc.) without
duplicating the mmget/kthread_use_mm/mmput boilerplate.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/Makefile         |  1 +
 drivers/gpu/drm/drm_user_fence.c | 69 +++++++++++++++++++++++++
 include/drm/drm_user_fence.h     | 86 ++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 include/drm/drm_user_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index c5be8e80d0c8..ddb770738992 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -69,6 +69,7 @@ drm-y := \
 	drm_syncobj.o \
 	drm_sysfs.o \
 	drm_trace_points.o \
+	drm_user_fence.o \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
new file mode 100644
index 000000000000..664178e2d74c
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * DRM user fence — extends drm_work_fence with kthread_use_mm() support.
+ *
+ * Use this when a GPU fence signals and work needs to access userspace
+ * memory (copy_to_user, fault-able operations) from a kthread context.
+ * For work that does not require userspace memory access, use
+ * drm_work_fence directly.
+ */
+
+#include <linux/kthread.h>
+#include <linux/sched/mm.h>
+
+#include <drm/drm_user_fence.h>
+
+static void drm_user_fence_do_work(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+	bool mm_ok = false;
+
+	if (mmget_not_zero(ufence->mm)) {
+		kthread_use_mm(ufence->mm);
+		mm_ok = true;
+	}
+
+	ufence->ops->worker(ufence, mm_ok);
+
+	if (mm_ok) {
+		kthread_unuse_mm(ufence->mm);
+		mmput_async(ufence->mm);
+	}
+}
+
+static void drm_user_fence_do_destroy(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+
+	mmdrop(ufence->mm);
+	ufence->ops->destroy(ufence);
+}
+
+static const struct drm_work_fence_ops drm_user_fence_wf_ops = {
+	.work    = drm_user_fence_do_work,
+	.destroy = drm_user_fence_do_destroy,
+};
+
+/**
+ * drm_user_fence_init - Initialize a user fence
+ * @ufence: user fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ *
+ * Must be called from process context with a valid current->mm.
+ * Grabs a reference to current->mm via mmgrab().
+ */
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops)
+{
+	drm_work_fence_init(&ufence->base, wq, &drm_user_fence_wf_ops);
+	ufence->mm = current->mm;
+	mmgrab(ufence->mm);
+	ufence->ops = ops;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_init);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
new file mode 100644
index 000000000000..2b2b640f510f
--- /dev/null
+++ b/include/drm/drm_user_fence.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_USER_FENCE_H__
+#define __DRM_USER_FENCE_H__
+
+#include <drm/drm_work_fence.h>
+
+struct drm_user_fence;
+
+/**
+ * struct drm_user_fence_ops - driver callbacks for a DRM user fence
+ */
+struct drm_user_fence_ops {
+	/**
+	 * @worker: Called from workqueue context with the process MM active.
+	 *
+	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
+	 * (copy_to_user, etc.) may be accessed safely.
+	 * If @mm_ok is false, the process MM was already gone; the driver
+	 * should log a warning and skip the userspace write.
+	 *
+	 * wake_up() or other post-signal housekeeping should also happen here.
+	 */
+	void (*worker)(struct drm_user_fence *ufence, bool mm_ok);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_user_fence *ufence);
+};
+
+/**
+ * struct drm_user_fence - DRM user fence with MM borrowing
+ *
+ * Extends drm_work_fence with kthread_use_mm() support for drivers
+ * that need to access userspace memory when a GPU fence signals.
+ * For work that does not need userspace memory access, use
+ * drm_work_fence directly.
+ *
+ * Call drm_user_fence_init() at creation and drm_user_fence_add_callback()
+ * to arm on a dma-fence. Call drm_user_fence_cancel_sync() before teardown.
+ */
+struct drm_user_fence {
+	/** @base: Base work fence. Must be first. */
+	struct drm_work_fence base;
+	/** @mm: Process MM grabbed at init time. */
+	struct mm_struct *mm;
+	/** @ops: Driver operations. */
+	const struct drm_user_fence_ops *ops;
+};
+
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops);
+
+static inline void drm_user_fence_get(struct drm_user_fence *ufence)
+{
+	drm_work_fence_get(&ufence->base);
+}
+
+static inline void drm_user_fence_put(struct drm_user_fence *ufence)
+{
+	drm_work_fence_put(&ufence->base);
+}
+
+static inline int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+					      struct dma_fence *fence)
+{
+	return drm_work_fence_add_callback(&ufence->base, fence);
+}
+
+static inline bool drm_user_fence_cancel(struct drm_user_fence *ufence)
+{
+	return drm_work_fence_cancel(&ufence->base);
+}
+
+static inline void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
+{
+	drm_work_fence_cancel_sync(&ufence->base);
+}
+
+#endif /* __DRM_USER_FENCE_H__ */
-- 
2.34.1

Re: [PATCH v6 2/4] drm: Add drm_user_fence helper

[Matthew Brost]

On Mon, Aug 31, 2026 at 07:15:37PM +0530, Srinivasan Shanmugam wrote:
> Introduce a common DRM user fence helper providing the kref-managed,
> MM-borrowing dma-fence-callback-to-workqueue pattern used by drivers
> that must access userspace memory from a kthread context when a GPU
> fence signals.
> 
> XE uses this pattern (xe_sync.c) to write a fence completion value
> to a userspace VA. AMDGPU will use the same pattern to signal a
> per-queue eventfd from a user-queue EOP fence callback.
> 
> The helper provides:
>  - struct drm_user_fence: embeddable base structure
>  - struct drm_user_fence_ops: worker/destroy callbacks
>  - drm_user_fence_init(): initialize and grab the process MM
>  - drm_user_fence_get/put(): reference counting
>  - drm_user_fence_add_callback(): attach to a dma-fence
> 
> The worker callback receives a bool indicating whether the process
> MM was successfully obtained, allowing drivers to handle the
> unavailable-MM case (log, skip the userspace write, etc.) without
> duplicating the mmget/kthread_use_mm/mmput boilerplate.
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>  drivers/gpu/drm/Makefile         |  1 +
>  drivers/gpu/drm/drm_user_fence.c | 69 +++++++++++++++++++++++++
>  include/drm/drm_user_fence.h     | 86 ++++++++++++++++++++++++++++++++
>  3 files changed, 156 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_user_fence.c
>  create mode 100644 include/drm/drm_user_fence.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index c5be8e80d0c8..ddb770738992 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -69,6 +69,7 @@ drm-y := \
>  	drm_syncobj.o \
>  	drm_sysfs.o \
>  	drm_trace_points.o \
> +	drm_user_fence.o \
>  	drm_vblank.o \
>  	drm_vblank_work.o \
>  	drm_vma_manager.o \
> diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
> new file mode 100644
> index 000000000000..664178e2d74c
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_user_fence.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 The Linux Foundation
> + *
> + * DRM user fence — extends drm_work_fence with kthread_use_mm() support.
> + *
> + * Use this when a GPU fence signals and work needs to access userspace
> + * memory (copy_to_user, fault-able operations) from a kthread context.
> + * For work that does not require userspace memory access, use
> + * drm_work_fence directly.
> + */
> +
> +#include <linux/kthread.h>
> +#include <linux/sched/mm.h>
> +
> +#include <drm/drm_user_fence.h>
> +
> +static void drm_user_fence_do_work(struct drm_work_fence *wfence)
> +{
> +	struct drm_user_fence *ufence =
> +		container_of(wfence, struct drm_user_fence, base);
> +	bool mm_ok = false;
> +
> +	if (mmget_not_zero(ufence->mm)) {
> +		kthread_use_mm(ufence->mm);
> +		mm_ok = true;
> +	}
> +
> +	ufence->ops->worker(ufence, mm_ok);
> +
> +	if (mm_ok) {
> +		kthread_unuse_mm(ufence->mm);
> +		mmput_async(ufence->mm);

Xe does this incorrectly, but ufence shouldn't be looked after 'worker'.

Also mm_ok probably isn't needed either. I'd write this like:

struct mm_struct *mm = NULL;

if (mmget_not_zero(ufence->mm)) {
	mm = ufence->mm;
	kthread_use_mm(mm);
}

ufence->ops->worker(ufence, !!mm); /* Or just pass in 'mm' */

if (mm) {
	kthread_unuse_mm(mm);
	mmput_async(mm);
}

> +	}
> +}
> +
> +static void drm_user_fence_do_destroy(struct drm_work_fence *wfence)
> +{
> +	struct drm_user_fence *ufence =
> +		container_of(wfence, struct drm_user_fence, base);
> +
> +	mmdrop(ufence->mm);
> +	ufence->ops->destroy(ufence);

I'd invert this:

struct mm_struct *mm = ufence->mm;

ufence->ops->destroy(ufence);
mmdrop(mm);

> +}
> +
> +static const struct drm_work_fence_ops drm_user_fence_wf_ops = {
> +	.work    = drm_user_fence_do_work,

I wouldn't use the name 'work' here. I think writeback is more apporiate.

> +	.destroy = drm_user_fence_do_destroy,
> +};
> +
> +/**
> + * drm_user_fence_init - Initialize a user fence
> + * @ufence: user fence to initialize
> + * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
> + * @ops: driver operations
> + *
> + * Must be called from process context with a valid current->mm.
> + * Grabs a reference to current->mm via mmgrab().
> + */
> +void drm_user_fence_init(struct drm_user_fence *ufence,
> +			 struct workqueue_struct *wq,
> +			 const struct drm_user_fence_ops *ops)
> +{
> +	drm_work_fence_init(&ufence->base, wq, &drm_user_fence_wf_ops);
> +	ufence->mm = current->mm;
> +	mmgrab(ufence->mm);
> +	ufence->ops = ops;
> +}
> +EXPORT_SYMBOL_GPL(drm_user_fence_init);
> diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
> new file mode 100644
> index 000000000000..2b2b640f510f
> --- /dev/null
> +++ b/include/drm/drm_user_fence.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 The Linux Foundation
> + */
> +
> +#ifndef __DRM_USER_FENCE_H__
> +#define __DRM_USER_FENCE_H__
> +
> +#include <drm/drm_work_fence.h>
> +
> +struct drm_user_fence;
> +
> +/**
> + * struct drm_user_fence_ops - driver callbacks for a DRM user fence
> + */
> +struct drm_user_fence_ops {
> +	/**
> +	 * @worker: Called from workqueue context with the process MM active.
> +	 *
> +	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
> +	 * (copy_to_user, etc.) may be accessed safely.
> +	 * If @mm_ok is false, the process MM was already gone; the driver
> +	 * should log a warning and skip the userspace write.

I'd wouldn't dicate if caller should log a warning - rather just say if
should skip the userspace write.

> +	 *
> +	 * wake_up() or other post-signal housekeeping should also happen here.
> +	 */
> +	void (*worker)(struct drm_user_fence *ufence, bool mm_ok);
> +
> +	/**
> +	 * @destroy: Called when the last reference is dropped.
> +	 * Free the containing structure here.
> +	 */
> +	void (*destroy)(struct drm_user_fence *ufence);
> +};
> +
> +/**
> + * struct drm_user_fence - DRM user fence with MM borrowing
> + *
> + * Extends drm_work_fence with kthread_use_mm() support for drivers
> + * that need to access userspace memory when a GPU fence signals.
> + * For work that does not need userspace memory access, use
> + * drm_work_fence directly.
> + *
> + * Call drm_user_fence_init() at creation and drm_user_fence_add_callback()
> + * to arm on a dma-fence. Call drm_user_fence_cancel_sync() before teardown.
> + */
> +struct drm_user_fence {
> +	/** @base: Base work fence. Must be first. */
> +	struct drm_work_fence base;
> +	/** @mm: Process MM grabbed at init time. */
> +	struct mm_struct *mm;
> +	/** @ops: Driver operations. */
> +	const struct drm_user_fence_ops *ops;
> +};
> +
> +void drm_user_fence_init(struct drm_user_fence *ufence,
> +			 struct workqueue_struct *wq,
> +			 const struct drm_user_fence_ops *ops);
> +

Kernel doc for all the inlines.

Matt

> +static inline void drm_user_fence_get(struct drm_user_fence *ufence)
> +{
> +	drm_work_fence_get(&ufence->base);
> +}
> +
> +static inline void drm_user_fence_put(struct drm_user_fence *ufence)
> +{
> +	drm_work_fence_put(&ufence->base);
> +}
> +
> +static inline int drm_user_fence_add_callback(struct drm_user_fence *ufence,
> +					      struct dma_fence *fence)
> +{
> +	return drm_work_fence_add_callback(&ufence->base, fence);
> +}
> +
> +static inline bool drm_user_fence_cancel(struct drm_user_fence *ufence)
> +{
> +	return drm_work_fence_cancel(&ufence->base);
> +}
> +
> +static inline void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
> +{
> +	drm_work_fence_cancel_sync(&ufence->base);
> +}
> +
> +#endif /* __DRM_USER_FENCE_H__ */
> -- 
> 2.34.1
> 

[PATCH v6 3/4] drm/xe: Convert xe_user_fence to drm_user_fence

[Srinivasan Shanmugam]

Replace the open-coded user fence implementation in xe_sync.c with the
new common drm_user_fence helper.

struct xe_user_fence now embeds struct drm_user_fence as its base.
XE-specific fields (xe_device pointer for the ufence_wq wake-up,
userspace VA, expected value, signalled flag) remain in the wrapper.

The local user_fence_destroy/get/put/worker/kick_ufence/user_fence_cb
functions are removed. Their logic moves to xe_ufence_ops.worker and
xe_ufence_ops.destroy, which are called by drm_user_fence_work().

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/xe/xe_sync.c       | 149 ++++++++++++++++-------------
 drivers/gpu/drm/xe/xe_sync.h       |   2 +
 drivers/gpu/drm/xe/xe_sync_types.h |   1 -
 drivers/gpu/drm/xe/xe_vm.c         |   1 +
 4 files changed, 84 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
index 37866768d64c..2d1e07792506 100644
--- a/drivers/gpu/drm/xe/xe_sync.c
+++ b/drivers/gpu/drm/xe/xe_sync.c
@@ -6,12 +6,11 @@
 #include "xe_sync.h"
 
 #include <linux/dma-fence-array.h>
-#include <linux/kthread.h>
-#include <linux/sched/mm.h>
 #include <linux/uaccess.h>
 
 #include <drm/drm_print.h>
 #include <drm/drm_syncobj.h>
+#include <drm/drm_user_fence.h>
 #include <uapi/drm/xe_drm.h>
 
 #include "xe_device.h"
@@ -19,36 +18,60 @@
 #include "xe_macros.h"
 #include "xe_sched_job_types.h"
 
+/*
+ * xe_user_fence wraps drm_user_fence with XE-specific fields.
+ * The drm_user_fence base handles MM borrowing and work-item lifetime.
+ */
 struct xe_user_fence {
-	struct xe_device *xe;
-	struct kref refcount;
-	struct dma_fence_cb cb;
-	struct work_struct worker;
-	struct mm_struct *mm;
-	u64 __user *addr;
-	u64 value;
-	int signalled;
+	struct drm_user_fence base;
+	struct xe_device     *xe;
+	u64 __user           *addr;
+	u64                   value;
+	int                   signalled;
 };
 
-static void user_fence_destroy(struct kref *kref)
+static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok)
 {
-	struct xe_user_fence *ufence = container_of(kref, struct xe_user_fence,
-						 refcount);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
 
-	mmdrop(ufence->mm);
-	kfree(ufence);
-}
+	/*
+	 * Mark signalled before waking waiters so UMD can safely reuse
+	 * the same ufence without hitting -EBUSY.
+	 */
+	WRITE_ONCE(ufence->signalled, 1);
 
-static void user_fence_get(struct xe_user_fence *ufence)
-{
-	kref_get(&ufence->refcount);
+	/*
+	 * Ensure the signalled store is visible before the user memory write
+	 * on weakly ordered architectures (e.g. ARM64). Without this barrier
+	 * the CPU may reorder stores, causing userspace to observe the user
+	 * memory update before signalled == 1.
+	 */
+	smp_wmb();
+
+	if (mm_ok) {
+		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
+			drm_dbg(&ufence->xe->drm,
+				"copy_to_user failed, user fence wasn't signaled\n");
+	} else {
+		drm_dbg(&ufence->xe->drm,
+			"mmget_not_zero() failed, ufence wasn't signaled\n");
+	}
+
+	wake_up_all(&ufence->xe->ufence_wq);
 }
 
-static void user_fence_put(struct xe_user_fence *ufence)
+static void xe_ufence_destroy(struct drm_user_fence *base)
 {
-	kref_put(&ufence->refcount, user_fence_destroy);
+	struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base);
+
+	kfree(ufence);
 }
 
+static const struct drm_user_fence_ops xe_ufence_ops = {
+	.worker  = xe_ufence_worker,
+	.destroy = xe_ufence_destroy,
+};
+
 static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 					       u64 value)
 {
@@ -63,51 +86,22 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr,
 	if (!ufence)
 		return ERR_PTR(-ENOMEM);
 
-	ufence->xe = xe;
-	kref_init(&ufence->refcount);
-	ufence->addr = ptr;
+	ufence->xe    = xe;
+	ufence->addr  = ptr;
 	ufence->value = value;
-	ufence->mm = current->mm;
-	mmgrab(ufence->mm);
+	drm_user_fence_init(&ufence->base, xe->ordered_wq, &xe_ufence_ops);
 
 	return ufence;
 }
 
-static void user_fence_worker(struct work_struct *w)
-{
-	struct xe_user_fence *ufence = container_of(w, struct xe_user_fence, worker);
-
-	WRITE_ONCE(ufence->signalled, 1);
-	if (mmget_not_zero(ufence->mm)) {
-		kthread_use_mm(ufence->mm);
-		if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value)))
-			XE_WARN_ON("Copy to user failed");
-		kthread_unuse_mm(ufence->mm);
-		mmput(ufence->mm);
-	} else {
-		drm_dbg(&ufence->xe->drm, "mmget_not_zero() failed, ufence wasn't signaled\n");
-	}
-
-	/*
-	 * Wake up waiters only after updating the ufence state, allowing the UMD
-	 * to safely reuse the same ufence without encountering -EBUSY errors.
-	 */
-	wake_up_all(&ufence->xe->ufence_wq);
-	user_fence_put(ufence);
-}
-
-static void kick_ufence(struct xe_user_fence *ufence, struct dma_fence *fence)
+static void user_fence_get(struct xe_user_fence *ufence)
 {
-	INIT_WORK(&ufence->worker, user_fence_worker);
-	queue_work(ufence->xe->ordered_wq, &ufence->worker);
-	dma_fence_put(fence);
+	drm_user_fence_get(&ufence->base);
 }
 
-static void user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+static void user_fence_put(struct xe_user_fence *ufence)
 {
-	struct xe_user_fence *ufence = container_of(cb, struct xe_user_fence, cb);
-
-	kick_ufence(ufence, fence);
+	drm_user_fence_put(&ufence->base);
 }
 
 int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
@@ -282,24 +276,15 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
 	} else if (sync->syncobj) {
 		drm_syncobj_replace_fence(sync->syncobj, fence);
 	} else if (sync->ufence) {
-		int err;
-
 		drm_syncobj_add_point(sync->ufence_syncobj,
 				      sync->ufence_chain_fence,
 				      fence, sync->ufence_timeline_value);
 		sync->ufence_chain_fence = NULL;
 
 		fence = drm_syncobj_fence_get(sync->ufence_syncobj);
-		user_fence_get(sync->ufence);
-		err = dma_fence_add_callback(fence, &sync->ufence->cb,
-					     user_fence_cb);
-		if (err == -ENOENT) {
-			kick_ufence(sync->ufence, fence);
-		} else if (err) {
+		if (drm_user_fence_add_callback(&sync->ufence->base, fence))
 			XE_WARN_ON("failed to add user fence");
-			user_fence_put(sync->ufence);
-			dma_fence_put(fence);
-		}
+		/* fence ref consumed by drm_user_fence_add_callback */
 	}
 }
 
@@ -434,6 +419,34 @@ void xe_sync_ufence_put(struct xe_user_fence *ufence)
 	user_fence_put(ufence);
 }
 
+/**
+ * xe_sync_ufence_cancel() - Non-blocking cancel of user fence callback
+ * @ufence: user fence reference
+ *
+ * Attempts to cancel the pending callback without waiting for the worker.
+ * Safe to call while holding dma_resv_lock or vm->lock. If the callback
+ * has already fired, the worker runs independently — xe_ufence_worker
+ * only accesses device-level and userspace resources, both safe after
+ * VMA teardown.
+ */
+void xe_sync_ufence_cancel(struct xe_user_fence *ufence)
+{
+	drm_user_fence_cancel(&ufence->base);
+}
+
+/**
+ * xe_sync_ufence_cancel_sync() - Cancel user fence callback and wait for worker
+ * @ufence: user fence reference
+ *
+ * Cancels any pending dma-fence callback and waits for the worker to fully
+ * complete before returning. Must be called during teardown before freeing
+ * any resources accessed by the worker.
+ */
+void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence)
+{
+	drm_user_fence_cancel_sync(&ufence->base);
+}
+
 /**
  * xe_sync_ufence_get_status() - Get user fence status
  * @ufence: user fence
diff --git a/drivers/gpu/drm/xe/xe_sync.h b/drivers/gpu/drm/xe/xe_sync.h
index 6b949194acff..b5ff3387c592 100644
--- a/drivers/gpu/drm/xe/xe_sync.h
+++ b/drivers/gpu/drm/xe/xe_sync.h
@@ -44,6 +44,8 @@ static inline bool xe_sync_is_ufence(struct xe_sync_entry *sync)
 struct xe_user_fence *__xe_sync_ufence_get(struct xe_user_fence *ufence);
 struct xe_user_fence *xe_sync_ufence_get(struct xe_sync_entry *sync);
 void xe_sync_ufence_put(struct xe_user_fence *ufence);
+void xe_sync_ufence_cancel(struct xe_user_fence *ufence);
+void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence);
 int xe_sync_ufence_get_status(struct xe_user_fence *ufence);
 
 #endif
diff --git a/drivers/gpu/drm/xe/xe_sync_types.h b/drivers/gpu/drm/xe/xe_sync_types.h
index b88f1833e28c..67048199b070 100644
--- a/drivers/gpu/drm/xe/xe_sync_types.h
+++ b/drivers/gpu/drm/xe/xe_sync_types.h
@@ -12,7 +12,6 @@ struct drm_syncobj;
 struct dma_fence;
 struct dma_fence_chain;
 struct drm_xe_sync;
-struct user_fence;
 
 struct xe_sync_entry {
 	struct drm_syncobj *syncobj;
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index b01f31ed4417..fbd2c28de8a6 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -1181,6 +1181,7 @@ static void xe_vma_destroy_late(struct xe_vma *vma)
 	struct xe_bo *bo = xe_vma_bo(vma);
 
 	if (vma->ufence) {
+		xe_sync_ufence_cancel(vma->ufence);
 		xe_sync_ufence_put(vma->ufence);
 		vma->ufence = NULL;
 	}
-- 
2.34.1

[PATCH v6 4/4] drm: Add per-signal compare functionality to drm_user_fence

[Srinivasan Shanmugam]

GPU drivers sometimes need to read a value from a userspace VA when a
dma-fence signals, compare it with an expected value, and only perform
the deferred work (e.g. eventfd_signal) if the comparison passes. This
is the per-signal filtering pattern used in AMDGPU's EOP eventfd path.

Add optional compare fields to drm_user_fence and a new helper
drm_user_fence_set_compare() to configure them. Supported operators
are ==, !=, >, >=, <, <=.

When cmp_addr is set, drm_user_fence reads the value from userspace
via get_user() and calls ops->worker() only if the comparison passes.
If the process MM is gone and cmp_addr is set, the worker is skipped
since the comparison cannot be performed.

Drivers that do not need filtering (e.g. XE) leave cmp_addr NULL and
the worker is called unconditionally — no behavioral change.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Change-Id: I03a726d5368674d06bd40fe7d11effd88c492741
---
v6:
 - Add WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT)) in
   drm_user_fence_set_compare() to warn if called on a 32-bit system.
   Plain WARN_ON_ONCE() is used since drm_user_fence has no
   struct drm_device * reference. (Thomas Hellström review)

 drivers/gpu/drm/drm_user_fence.c | 82 +++++++++++++++++++++++++++++++-
 include/drm/drm_user_fence.h     | 29 +++++++++++
 2 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
index 664178e2d74c..e55d03ebf75f 100644
--- a/drivers/gpu/drm/drm_user_fence.c
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -12,21 +12,68 @@
 
 #include <linux/kthread.h>
 #include <linux/sched/mm.h>
+#include <linux/uaccess.h>
 
 #include <drm/drm_user_fence.h>
 
+static bool drm_user_fence_cmp_match(u64 cur_val, u64 expected,
+				     enum drm_user_fence_cmp op)
+{
+	switch (op) {
+	case DRM_USER_FENCE_CMP_EQ:
+		return cur_val == expected;
+	case DRM_USER_FENCE_CMP_NE:
+		return cur_val != expected;
+	case DRM_USER_FENCE_CMP_GT:
+		return cur_val > expected;
+	case DRM_USER_FENCE_CMP_GE:
+		return cur_val >= expected;
+	case DRM_USER_FENCE_CMP_LT:
+		return cur_val < expected;
+	case DRM_USER_FENCE_CMP_LE:
+		return cur_val <= expected;
+	default:
+		return true;
+	}
+}
+
 static void drm_user_fence_do_work(struct drm_work_fence *wfence)
 {
 	struct drm_user_fence *ufence =
 		container_of(wfence, struct drm_user_fence, base);
 	bool mm_ok = false;
+	bool call_worker = true;
 
 	if (mmget_not_zero(ufence->mm)) {
 		kthread_use_mm(ufence->mm);
 		mm_ok = true;
 	}
 
-	ufence->ops->worker(ufence, mm_ok);
+	/*
+	 * Per-signal comparison: read a value from userspace and compare
+	 * with the expected value. Skip ops->worker if the condition is
+	 * not met. Drivers that do not need filtering leave cmp_addr NULL.
+	 *
+	 * If the MM is gone and cmp_addr is set we cannot perform the
+	 * comparison, so skip the worker rather than calling it without
+	 * having verified the condition.
+	 */
+	if (ufence->cmp_op != DRM_USER_FENCE_CMP_NONE) {
+		if (!mm_ok) {
+			call_worker = false;
+		} else {
+			u64 cur_val;
+
+			if (get_user(cur_val, ufence->cmp_addr) ||
+			    !drm_user_fence_cmp_match(cur_val,
+						      ufence->cmp_value,
+						      ufence->cmp_op))
+				call_worker = false;
+		}
+	}
+
+	if (call_worker)
+		ufence->ops->worker(ufence, mm_ok);
 
 	if (mm_ok) {
 		kthread_unuse_mm(ufence->mm);
@@ -65,5 +112,38 @@ void drm_user_fence_init(struct drm_user_fence *ufence,
 	ufence->mm = current->mm;
 	mmgrab(ufence->mm);
 	ufence->ops = ops;
+	ufence->cmp_addr = NULL;
+	ufence->cmp_value = 0;
+	ufence->cmp_op = DRM_USER_FENCE_CMP_NONE;
 }
 EXPORT_SYMBOL_GPL(drm_user_fence_init);
+
+/**
+ * drm_user_fence_set_compare - Configure per-signal value comparison
+ * @ufence: user fence
+ * @addr: userspace VA to read when the fence signals
+ * @value: expected value to compare against
+ * @op: comparison operator (see &enum drm_user_fence_cmp)
+ *
+ * When set, drm_user_fence reads @addr via get_user() each time the
+ * fence signals and calls ops->worker() only if the comparison passes.
+ * This enables per-signal filtering without open-coding the read+compare
+ * pattern in each driver.
+ *
+ * Must be called after drm_user_fence_init() and before
+ * drm_user_fence_add_callback().
+ */
+void drm_user_fence_set_compare(struct drm_user_fence *ufence,
+				u64 __user *addr, u64 value,
+				enum drm_user_fence_cmp op)
+{
+	WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT));
+
+	if (WARN_ON(op != DRM_USER_FENCE_CMP_NONE && !addr))
+		return;
+
+	ufence->cmp_addr = addr;
+	ufence->cmp_value = value;
+	ufence->cmp_op = op;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_set_compare);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
index 2b2b640f510f..764dd55ed3a2 100644
--- a/include/drm/drm_user_fence.h
+++ b/include/drm/drm_user_fence.h
@@ -33,6 +33,22 @@ struct drm_user_fence_ops {
 	void (*destroy)(struct drm_user_fence *ufence);
 };
 
+/**
+ * enum drm_user_fence_cmp - comparison operators for per-signal filtering
+ *
+ * Used with drm_user_fence_set_compare() to control when ops->worker is
+ * called based on a value read from userspace.
+ */
+enum drm_user_fence_cmp {
+	DRM_USER_FENCE_CMP_NONE = 0, /* always call worker — default */
+	DRM_USER_FENCE_CMP_EQ, /* call worker if *addr == value */
+	DRM_USER_FENCE_CMP_NE, /* call worker if *addr != value */
+	DRM_USER_FENCE_CMP_GT, /* call worker if *addr > value */
+	DRM_USER_FENCE_CMP_GE, /* call worker if *addr >= value */
+	DRM_USER_FENCE_CMP_LT, /* call worker if *addr < value */
+	DRM_USER_FENCE_CMP_LE, /* call worker if *addr <= value */
+};
+
 /**
  * struct drm_user_fence - DRM user fence with MM borrowing
  *
@@ -51,11 +67,24 @@ struct drm_user_fence {
 	struct mm_struct *mm;
 	/** @ops: Driver operations. */
 	const struct drm_user_fence_ops *ops;
+	/**
+	 * @cmp_addr: Userspace VA to read for per-signal comparison.
+	 * NULL means always call ops->worker (default XE behavior).
+	 * Set via drm_user_fence_set_compare().
+	 */
+	u64 __user *cmp_addr;
+	/** @cmp_value: Expected value for comparison. */
+	u64 cmp_value;
+	/** @cmp_op: Comparison operator. */
+	enum drm_user_fence_cmp cmp_op;
 };
 
 void drm_user_fence_init(struct drm_user_fence *ufence,
 			 struct workqueue_struct *wq,
 			 const struct drm_user_fence_ops *ops);
+void drm_user_fence_set_compare(struct drm_user_fence *ufence,
+				u64 __user *addr, u64 value,
+				enum drm_user_fence_cmp op);
 
 static inline void drm_user_fence_get(struct drm_user_fence *ufence)
 {
-- 
2.34.1

Re: [PATCH v6 4/4] drm: Add per-signal compare functionality to drm_user_fence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Per-signal comparison silently drops events and hangs userspace if the underlying dma_fence signals with an error (e.g., GPU hang).
- [High] `get_user()` is called in a workqueue thread without disabling page faults, allowing indefinite stalls via userfaultfd.
- [Medium] The comparison API exposes user-triggerable kernel WARNINGs and performs non-atomic 64-bit reads on 32-bit systems, leading to torn reads and lost events.
--

commit 9b6e7653adf98ad34fe45f552b39016755ccc0e5
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Subject: drm: Add per-signal compare functionality to drm_user_fence

This commit adds optional comparison fields to drm_user_fence to support
per-signal filtering, which is used in AMDGPU's EOP eventfd path. It
introduces drm_user_fence_set_compare() to configure these fields,
allowing the worker to be called conditionally based on a value read
from a userspace address.

> diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
> index 664178e2d74cf..e55d03ebf75f5 100644
> --- a/drivers/gpu/drm/drm_user_fence.c
> +++ b/drivers/gpu/drm/drm_user_fence.c

[ ... ]

> @@ -34,7 +49,32 @@ static void drm_user_fence_do_work(struct drm_work_fence *wfence)
>  		mm_ok = true;
>  	}
>  
> -	ufence->ops->worker(ufence, mm_ok);
> +	/*
> +	 * Per-signal comparison: read a value from userspace and compare
> +	 * with the expected value. Skip ops->worker if the condition is
> +	 * not met. Drivers that do not need filtering leave cmp_addr NULL.
> +	 *
> +	 * If the MM is gone and cmp_addr is set we cannot perform the
> +	 * comparison, so skip the worker rather than calling it without
> +	 * having verified the condition.
> +	 */
> +	if (ufence->cmp_op != DRM_USER_FENCE_CMP_NONE) {
> +		if (!mm_ok) {
> +			call_worker = false;
> +		} else {
> +			u64 cur_val;
> +
> +			if (get_user(cur_val, ufence->cmp_addr) ||
> +			    !drm_user_fence_cmp_match(cur_val,
> +						      ufence->cmp_value,
> +						      ufence->cmp_op))
> +				call_worker = false;

[Severity: High]
Is it possible for a page fault here to indefinitely stall the workqueue?

If a malicious user maps cmp_addr to a region backed by userfaultfd, they
could intentionally avoid resolving the fault. Because this executes in a
workqueue thread context without page faults disabled, this would block the
thread forever and stall all other tasks queued on the same workqueue.
Should pagefault_disable() be used around get_user()?

[Severity: High]
What happens if the underlying dma_fence signals with an error, such as
during a GPU hang?

If a GPU hang occurs, the hardware will fail to write the expected sequence
value to userspace. Instead, the GPU reset mechanism will signal all pending
fences with an error code (e.g., -ECANCELED). When this executes, get_user()
reads stale memory, causing drm_user_fence_cmp_match() to fail and
call_worker to be set to false.

Since the worker is completely skipped and the fence only signals once, the
event is permanently dropped, causing waiting userspace processes to hang.
Could the logic check the fence's error state (e.g., wfence->fence->error)
and bypass the comparison when an error is present?

[Severity: Medium]
Can this lead to torn reads on 32-bit architectures?

The get_user() macro on a 64-bit sequence number is not natively atomic on
32-bit systems and is implemented as two separate 32-bit loads. If the GPU
concurrently updates this value, the CPU may read a torn value. A torn read
can cause the comparison to falsely fail, resulting in a permanently lost
event.

> +		}
> +	}
> +
> +	if (call_worker)
> +		ufence->ops->worker(ufence, mm_ok);
>  
>  	if (mm_ok) {
>  		kthread_unuse_mm(ufence->mm);

[ ... ]

> +/**
> + * drm_user_fence_set_compare - Configure per-signal value comparison
> + * @ufence: user fence
> + * @addr: userspace VA to read when the fence signals
> + * @value: expected value to compare against
> + * @op: comparison operator (see &enum drm_user_fence_cmp)
> + *
> + * When set, drm_user_fence reads @addr via get_user() each time the
> + * fence signals and calls ops->worker() only if the comparison passes.
> + * This enables per-signal filtering without open-coding the read+compare
> + * pattern in each driver.
> + *
> + * Must be called after drm_user_fence_init() and before
> + * drm_user_fence_add_callback().
> + */
> +void drm_user_fence_set_compare(struct drm_user_fence *ufence,
> +				u64 __user *addr, u64 value,
> +				enum drm_user_fence_cmp op)
> +{
> +	WARN_ON_ONCE(!IS_ENABLED(CONFIG_64BIT));
> +
> +	if (WARN_ON(op != DRM_USER_FENCE_CMP_NONE && !addr))
> +		return;

[Severity: Medium]
Are these warnings triggerable by unprivileged userspace?

Because this function is used to configure user fences based on ioctl inputs,
this allows unprivileged userspace to trivially trigger kernel WARNINGs.
This creates a denial of service vector on systems with panic_on_warn.
Additionally, the 64-bit warning triggers unconditionally on 32-bit kernels.

> +
> +	ufence->cmp_addr = addr;
> +	ufence->cmp_value = value;
> +	ufence->cmp_op = op;
> +}
> +EXPORT_SYMBOL_GPL(drm_user_fence_set_compare);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827062142.4038272-1-srinivasan.shanmugam@amd.com?part=4