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

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

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