The first-level trap state belongs to the GPUVM because the TBA and TMA are mappings in that GPUVM.
User queues and kernel queues using the same GPUVM use the same first-level trap state. There is no need to keep a separate trap object in the file-private data. Move the trap object into struct amdgpu_vm. Create it when the GPUVM is initialized and free it when the GPUVM is destroyed. Create the trap state after releasing the initial VM root reservation. The trap allocation locks the VM internally, so doing this while the VM root is already reserved could lock the same reservation again. For the same reason, free the trap state before reserving the VM root during VM cleanup. Keep the first-level trap ISA buffer shared at the device level. The ISA is the same for all GPUVMs on the same device, while each GPUVM keeps its own trap mapping and TMA state. Update the user queue path to get the first-level TBA and TMA from its GPUVM. Suggested-by: Christian König <[email protected]> Suggested-by: Alex Deucher <[email protected]> Cc: Felix Kuehling <[email protected]> Cc: Lancelot Six <[email protected]> Cc: James Zhu <[email protected]> Cc: Lijo Lazar <[email protected]> Cc: Samuel Pitoiset <[email protected]> Cc: Natalie Vock <[email protected]> Cc: Timur Kristóf <[email protected]> Cc: Pierre-Eric Pelloux-Prayer <[email protected]> Signed-off-by: Srinivasan Shanmugam <[email protected]> --- Changes in v3: - Initialize the GPUVM trap object explicitly during VM setup. - Improve cleanup when trap allocation fails during VM initialization. - Free the trap state before reserving the VM root during VM teardown to avoid locking the same reservation recursively. - Simplify trap mapping cleanup and remove unnecessary error messages. drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 -- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 9 +---- drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c | 41 ++-------------------- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 31 ++++++++++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 +++ drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 7 ++-- 6 files changed, 41 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index e3763c197138..f810946b2ace 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -328,7 +328,6 @@ struct amdgpu_reset_control; struct amdgpu_coredump_info; struct amdgpu_video_codecs; struct amdgpu_trap_isa; -struct amdgpu_trap_obj; enum amdgpu_cp_irq { AMDGPU_CP_IRQ_GFX_ME0_PIPE0_EOP = 0, @@ -417,7 +416,6 @@ struct amdgpu_fpriv { struct xarray bo_list_handles; struct amdgpu_ctx_mgr ctx_mgr; struct amdgpu_userq_mgr userq_mgr; - struct amdgpu_trap_obj *trap_obj; /* Eviction fence infra */ struct amdgpu_eviction_fence_mgr evf_mgr; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 47e4680db797..1e23a68b5c5b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -1499,7 +1499,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) int num_xcc, r; fpriv = (struct amdgpu_fpriv *)filp->driver_priv; - if (!amdgpu_trap_is_enabled(adev) || !fpriv->trap_obj) + if (!amdgpu_trap_is_enabled(adev) || !fpriv->vm.trap_obj) return -EOPNOTSUPP; num_xcc = amdgpu_xcp_get_num_xcc(adev->xcp_mgr, fpriv->xcp_id); cwsr_info.ctl_stack_size = @@ -1641,12 +1641,6 @@ 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"); - } - amdgpu_ctx_mgr_init(&fpriv->ctx_mgr, adev); file_priv->driver_priv = fpriv; @@ -1716,7 +1710,6 @@ 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); 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 b55b5d016148..02d29e884206 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c @@ -327,8 +327,6 @@ static int amdgpu_trap_map_region(struct amdgpu_device *adev, va &= AMDGPU_GMC_HOLE_MASK; r = amdgpu_vm_bo_map(adev, *bo_va, va, 0, size, va_flags); if (r) { - dev_err(adev->dev, "failed to do bo map of %s region, err=%d\n", - (region == AMDGPU_TRAP_TBA ? "tba" : "tma"), r); amdgpu_vm_bo_del(adev, *bo_va); *bo_va = NULL; return r; @@ -336,9 +334,6 @@ static int amdgpu_trap_map_region(struct amdgpu_device *adev, r = amdgpu_vm_bo_update(adev, *bo_va, false); if (r) { - dev_err(adev->dev, - "failed to do page table update of %s region, err=%d\n", - (region == AMDGPU_TRAP_TBA ? "tba" : "tma"), r); amdgpu_vm_bo_del(adev, *bo_va); *bo_va = NULL; return r; @@ -352,39 +347,6 @@ static int amdgpu_trap_map_region(struct amdgpu_device *adev, return 0; } -static int amdgpu_trap_unmap_region(struct amdgpu_device *adev, - struct amdgpu_trap_obj *cwsr, - enum amdgpu_trap_region region) -{ - struct amdgpu_bo_va **bo_va; - uint64_t va; - int r; - - if (!cwsr) - return -EINVAL; - - if (region == AMDGPU_TRAP_TBA) { - bo_va = &cwsr->tba_va; - va = cwsr->tba_gpu_va_addr; - } else { - bo_va = &cwsr->tma_va; - va = cwsr->tma_gpu_va_addr; - } - - r = amdgpu_vm_bo_unmap(adev, *bo_va, va); - if (r) { - dev_err(adev->dev, - "failed to do bo_unmap on CWSR trap handler, err=%d\n", - r); - return r; - } - - amdgpu_vm_bo_del(adev, *bo_va); - *bo_va = NULL; - - return r; -} - int amdgpu_trap_alloc(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_trap_obj **trap_obj) { @@ -468,7 +430,8 @@ int amdgpu_trap_alloc(struct amdgpu_device *adev, struct amdgpu_vm *vm, r = amdgpu_trap_map_region(adev, vm, cwsr, AMDGPU_TRAP_TBA); if (r) { - amdgpu_trap_unmap_region(adev, cwsr, AMDGPU_TRAP_TMA); + amdgpu_vm_bo_del(adev, cwsr->tma_va); + cwsr->tma_va = NULL; goto err_tma; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 930b744c5f5f..fe1289abb56c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -43,6 +43,7 @@ #include "amdgpu_xgmi.h" #include "amdgpu_dma_buf.h" #include "amdgpu_res_cursor.h" +#include "amdgpu_trap.h" #include "kfd_svm.h" /** @@ -2570,6 +2571,7 @@ void amdgpu_vm_set_task_info(struct amdgpu_vm *vm) int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, int32_t xcp_id) { + struct amdgpu_bo_va_mapping *mapping, *tmp; struct amdgpu_bo *root_bo; struct amdgpu_bo_vm *root; int r, i; @@ -2578,6 +2580,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) vm->reserved_vmid[i] = NULL; + vm->trap_obj = NULL; + amdgpu_vm_bo_status_init(&vm->kernel); amdgpu_vm_bo_status_init(&vm->always_valid); spin_lock_init(&vm->individual_lock); @@ -2644,10 +2648,28 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, dev_dbg(adev->dev, "Failed to create task info for VM\n"); amdgpu_bo_unreserve(vm->root.bo); + + if (amdgpu_trap_is_enabled(adev)) { + r = amdgpu_trap_alloc(adev, vm, &vm->trap_obj); + if (r) + goto error_free_trap; + } + amdgpu_bo_unref(&root_bo); return 0; +error_free_trap: + amdgpu_bo_reserve(root_bo, true); + + list_for_each_entry_safe(mapping, tmp, &vm->freed, list) { + list_del(&mapping->list); + amdgpu_vm_free_mapping(adev, vm, mapping, NULL); + } + + amdgpu_vm_put_task_info(vm->task_info); + dma_fence_put(vm->last_update); + error_free_root: amdgpu_vm_pt_free_root(adev, vm); amdgpu_bo_unreserve(vm->root.bo); @@ -2753,6 +2775,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, &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..d7d5620e2385 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -43,6 +43,7 @@ struct amdgpu_bo_va; struct amdgpu_job; struct amdgpu_bo_list_entry; struct amdgpu_bo_vm; +struct amdgpu_trap_obj; /* * GPUVM handling @@ -488,6 +489,9 @@ struct amdgpu_vm { /* cached fault info */ struct amdgpu_vm_fault_info fault_info; + + /* First-level trap state for this GPUVM; NULL if unavailable. */ + struct amdgpu_trap_obj *trap_obj; }; struct amdgpu_vm_manager { diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c index 2150973b947d..1cce81914be9 100644 --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c @@ -138,7 +138,6 @@ static int convert_to_mes_priority(int priority) static int mes_userq_map(struct amdgpu_usermode_queue *queue) { struct amdgpu_userq_mgr *uq_mgr = queue->userq_mgr; - struct amdgpu_fpriv *fpriv = uq_mgr_to_fpriv(uq_mgr); struct amdgpu_device *adev = uq_mgr->adev; struct amdgpu_userq_obj *ctx = &queue->fw_obj; struct amdgpu_mqd_prop *userq_props = queue->userq_prop; @@ -172,9 +171,9 @@ static int mes_userq_map(struct amdgpu_usermode_queue *queue) queue_input.is_kfd_process = 1; queue_input.is_aql_queue = 1; } - if (fpriv->trap_obj) { - queue_input.tba_addr = fpriv->trap_obj->tba_gpu_va_addr; - queue_input.tma_addr = fpriv->trap_obj->tma_gpu_va_addr; + if (queue->vm->trap_obj) { + queue_input.tba_addr = queue->vm->trap_obj->tba_gpu_va_addr; + queue_input.tma_addr = queue->vm->trap_obj->tma_gpu_va_addr; queue_input.trap_en = !amdgpu_trap_has_dbg_wa(adev); } -- 2.34.1
