AMD General
> -----Original Message-----
> From: Thomas Hellström <[email protected]>
> Sent: Monday, August 31, 2026 5:52 PM
> To: SHANMUGAM, SRINIVASAN <[email protected]>;
> Matthew Brost <[email protected]>; Koenig, Christian
> <[email protected]>; Deucher, Alexander
> <[email protected]>
> Cc: [email protected]; Mika Kuoppala
> <[email protected]>; Maarten Lankhorst
> <[email protected]>; [email protected]; intel-
> [email protected]
> 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 <[email protected]>
> > > Sent: Monday, August 31, 2026 3:46 PM
> > > To: SHANMUGAM, SRINIVASAN <[email protected]>;
> Matthew
> > > Brost <[email protected]>; Koenig, Christian
> > > <[email protected]>; Deucher, Alexander
> > > <[email protected]>
> > > Cc: [email protected]; Mika Kuoppala
> > > <[email protected]>; Maarten Lankhorst
> > > <[email protected]>;
> > > [email protected]; intel- [email protected]
> > > 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 <[email protected]>
> > > > Suggested-by: Christian König <[email protected]>
> > > > Cc: Mika Kuoppala <[email protected]>
> > > > Cc: Thomas Hellström <[email protected]>
> > > > Cc: Maarten Lankhorst <[email protected]>
> > > > Cc: [email protected]
> > > > Cc: [email protected]
> > > > Cc: [email protected]
> > >
> > > 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
