On 9/18/2026 9:13 AM, SRINIVASAN SHANMUGAM wrote:


On 9/16/2026 8:14 PM, Christian König wrote:
Hi Srini,

please keep Timur, Natalie and Samuel on CC.

On 9/16/26 07:37, Srinivasan Shanmugam wrote:
Render-node user queues had no mechanism to install a userspace
second-level trap handler equivalent to what KFD provides for compute.

On dGPUs the CWSR TMA buffer is placed in VRAM and accessed via MMIO
(PCI BAR). Raw CPU pointer dereferences are unsafe on platforms that
lack full load/store support for MMIO. Replace void *tma_cpu_addr with
struct iosys_map tma_map and use iosys_map_wr() for all TMA writes.

Writing TBA and TMA as two separate stores creates a race where a
trapping wave can observe a mismatched pair. For user queues, evict all
queues and flush the GPU TLB before writing, then restore queues after.
For kernel queues, wait for the VM to be idle via amdgpu_vm_wait_idle()
before eviction and the descriptor write.

Evict user queues in amdgpu_trap_free() at file close. Without eviction
a freed TMA BO can be accessed via a stale TBA, causing a GPU page
fault.

Export amdgpu_userq_restore_all() for immediate queue restore after
SET/CLEAR without waiting for the eviction fence.

Add VM_ALWAYS_VALID validation in amdgpu_trap_validate_user_addr().
SET_L2_TRAP returns -EINVAL if TBA or TMA BO does not have this flag.
Works for me, Timur/Natalie/Samuel any objections to that?

The old TRAP ioctl and its internal handler are removed. The new
SET/CLEAR path is exposed via the VM ioctl in a later patch.

SET_L2_TRAP is per-VM (per-process). Each process has its own TMA buffer
allocated in amdgpu_trap_alloc(). There is no conflict between processes
on user queue VMIDs.

Cc: Christian König<[email protected]>
Cc: Alex Deucher<[email protected]>
Cc: Felix Kuehling<[email protected]>
Cc: Lijo Lazar<[email protected]>
Signed-off-by: Srinivasan Shanmugam<[email protected]>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |   1 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c  | 284 ++++++++++++++++------
  drivers/gpu/drm/amd/amdgpu/amdgpu_trap.h  |  64 ++++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c |  22 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |   1 +
  include/uapi/drm/amdgpu_drm.h             |   2 -
  7 files changed, 285 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index a93ee106ab92..6473faf18d71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -3079,7 +3079,6 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
        DRM_IOCTL_DEF_DRV(AMDGPU_SCHED, amdgpu_sched_ioctl, DRM_MASTER),
        DRM_IOCTL_DEF_DRV(AMDGPU_BO_LIST, amdgpu_bo_list_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF_DRV(AMDGPU_FENCE_TO_HANDLE, 
amdgpu_cs_fence_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
-       DRM_IOCTL_DEF_DRV(AMDGPU_TRAP, amdgpu_trap_ioctl, DRM_AUTH | 
DRM_RENDER_ALLOW),
        /* KMS */
        DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 47e4680db797..559c9222fc48 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1716,7 +1716,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
        }
amdgpu_ctx_mgr_fini(&fpriv->ctx_mgr);
-       amdgpu_trap_free(adev, &fpriv->vm, &fpriv->trap_obj);
+       amdgpu_trap_free(adev, &fpriv->vm, &fpriv->userq_mgr, &fpriv->trap_obj);
if (pasid)
                amdgpu_pasid_free_delayed(pd->tbo.base.resv, pasid);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c
index 31f653ec3fb1..3313bc099cf4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c
@@ -19,12 +19,23 @@
   * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
   * OTHER DEALINGS IN THE SOFTWARE.
   */
+#include <linux/iosys-map.h>
+#include <drm/ttm/ttm_bo.h>
+
  #include <drm/drm_exec.h>
#include "amdgpu.h"
+#include "amdgpu_gmc.h"
+#include "amdgpu_userq.h"
+#include "amdgpu_vm.h"
  #include "cwsr_trap_handler.h"
  #include "amdgpu_trap.h"
+/* First-level TMA layout shared with second-level trap handler ABI */
+#define AMDGPU_TRAP_L2_TBA_INDEX       0
+#define AMDGPU_TRAP_L2_TMA_INDEX       1
+#define AMDGPU_TRAP_DEBUG_FLAG_INDEX   2
+
  extern int cwsr_enable;
#define AMDGPU_TRAP_TBA_MAX_SIZE (2 * AMDGPU_GPU_PAGE_SIZE)
@@ -380,6 +391,7 @@ int amdgpu_trap_alloc(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
        struct amdgpu_trap_obj *cwsr;
        struct amdgpu_bo *bo;
        struct drm_exec exec;
+       void *cpu_addr;
        int r;
if (!amdgpu_trap_is_enabled(adev))
@@ -407,10 +419,21 @@ int amdgpu_trap_alloc(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
r = amdgpu_bo_create_kernel(adev, AMDGPU_TRAP_TMA_MAX_SIZE, PAGE_SIZE,
                                    AMDGPU_GEM_DOMAIN_GTT, &cwsr->tma_bo, NULL,
-                                   &cwsr->tma_cpu_addr);
+                                   &cpu_addr);
        if (r)
                goto err;
+ /*
+        * Build an iosys_map for the TMA CPU mapping. On dGPUs the TMA BO
+        * may be placed in VRAM and accessed via MMIO (PCI BAR). Check
+        * TTM_BO_MAP_IOMEM_MASK to select the correct write accessor.
+        */
+       if (cwsr->tma_bo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK)
+               iosys_map_set_vaddr_iomem(&cwsr->tma_map,
+                                         (void __iomem *)cpu_addr);
+       else
+               iosys_map_set_vaddr(&cwsr->tma_map, cpu_addr);
+
Clear NAK to that hack. Please use ttm_bo_vmap() instead if you need an 
iosys_map.

Hi Christian,

Thanks. One clarification regarding using |ttm_bo_vmap()| here.

The per-VM TMA BO is unpinned and can be moved/evicted. Currently |amdgpu_bo_move_notify()| calls |amdgpu_bo_kunmap()|, so the existing |amdgpu_bo_kmap()| mapping is dropped before a move.

If we replace it with |ttm_bo_vmap()| and keep the returned |iosys_map| in the trap object, should we also handle unmapping/remapping it across BO moves, or would you prefer mapping it only around the TMA accesses?

Since the per-VM TMA BO is unpinned and movable, keeping the |ttm_bo_vmap()| mapping in the trap object would require us to invalidate/remap it whenever the BO moves.

To avoid maintaining a long-lived mapping for an evictable BO, I'm thinking of using |ttm_bo_vmap()| only around the TMA CPU accesses while the BO is reserved, and calling |ttm_bo_vunmap()| before releasing the reservation.

This should also avoid leaving a stale |iosys_map| after BO migration.

Does that sound like the preferred approach?

Thanks,
Srini

Reply via email to