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? Thanks, Srini
