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 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