On 9/18/26 05:43, 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?
I already answered that on team but repeating here for everyone on CC: Please map the buffer only temporary while update the values. The background is that keeping the mapping alive for the trap handler is a really complex task and for temporary mapping it you just need to validate it once. Regards, Christian. > > Thanks, > Srini > >
