On 16-Sep-26 11:07 AM, Srinivasan Shanmugam wrote:
The first-level trap mapping is a property of a GPU virtual address
space, not of a queue type. User queues and kernel queues executing in
the same GPUVM read the same TMA, so both paths must not carry separate
trap allocations.
Move ownership of the trap object to struct amdgpu_vm and create it
during VM initialization. Keep fpriv->trap_obj as a read-only alias so
the existing render-node ioctl paths do not need changes. This also
establishes the trap mapping before a GPUVM can be used by any queue
type, which allows the gfxhub setup code in the following patch to
enable TRAP_EN without a deferred allocation.
The first-level ISA binary is identical across VMs for the same hardware
generation. It is copied once into adev->trap_info->isa_bo at device
init and mapped into each GPUVM at vm_init rather than duplicated
per-VM.
Signed-off-by: Srinivasan Shanmugam <[email protected]>
Cc: Christian König <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: Felix Kuehling <[email protected]>
Cc: Lijo Lazar <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 18 ++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +++
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 559c9222fc48..5ff3e62671b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1641,11 +1641,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev,
struct drm_file *file_priv)
amdgpu_evf_mgr_init(&fpriv->evf_mgr);
- if (amdgpu_trap_is_enabled(adev)) {
- r = amdgpu_trap_alloc(adev, &fpriv->vm, &fpriv->trap_obj);
- if (r)
- dev_dbg(adev->dev, "trap handler not enabled");
- }
+ /* The trap object is owned by the GPUVM; expose it as an alias. */
+ fpriv->trap_obj = fpriv->vm.trap_obj;
If it's getting moved, suggest to remove this altogether and fetch
through fpriv->vm always.
Thanks,
Lijo
amdgpu_ctx_mgr_init(&fpriv->ctx_mgr, adev);
@@ -1716,7 +1713,6 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
}
amdgpu_ctx_mgr_fini(&fpriv->ctx_mgr);
- 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_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 930b744c5f5f..d981915ac2f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -44,6 +44,7 @@
#include "amdgpu_dma_buf.h"
#include "amdgpu_res_cursor.h"
#include "kfd_svm.h"
+#include "amdgpu_trap.h"
/**
* DOC: GPUVM
@@ -2639,6 +2640,14 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
if (r)
goto error_free_root;
+ if (amdgpu_trap_is_enabled(adev)) {
+ r = amdgpu_trap_alloc(adev, vm, &vm->trap_obj);
+ if (r) {
+ amdgpu_vm_fini(adev, vm);
+ return r;
+ }
+ }
+
r = amdgpu_vm_create_task_info(vm);
if (r)
dev_dbg(adev->dev, "Failed to create task info for VM\n");
@@ -2753,6 +2762,15 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
amdgpu_amdkfd_gpuvm_destroy_cb(adev, vm);
+ /*
+ * Free trap state before reserving root. amdgpu_trap_free()
+ * uses drm_exec to lock the page directory internally; calling
+ * it after amdgpu_bo_reserve(root) would deadlock on the same
+ * ww_mutex.
+ */
+ if (vm->trap_obj)
+ amdgpu_trap_free(adev, vm, NULL, &vm->trap_obj);
+
root = amdgpu_bo_ref(vm->root.bo);
amdgpu_bo_reserve(root, true);
dma_fence_wait(vm->last_unlocked, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index dd825e179979..8707618c199e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -488,6 +488,9 @@ struct amdgpu_vm {
/* cached fault info */
struct amdgpu_vm_fault_info fault_info;
+
+ /* First-level trap state shared by all queue paths using this GPUVM. */
+ struct amdgpu_trap_obj *trap_obj;
};
struct amdgpu_vm_manager {
