From dcb388eddb5f1bbc817d0e9ba341908b0a201ff2 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Mon, 28 Jun 2021 23:29:39 +0200 Subject: drm/amdgpu: use xarray for storing pasid in vm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace idr with xarray as we actually need hash functionality. Cleanup code related to vm pasid by adding helper function. Signed-off-by: Nirmoy Das Acked-by: Felix Kuehling Reviewed-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 149 +++++++++++++++------------------ 1 file changed, 69 insertions(+), 80 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 79cfa2d68487..9ea527ea3238 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -88,6 +88,46 @@ struct amdgpu_prt_cb { struct dma_fence_cb cb; }; +/** + * amdgpu_vm_set_pasid - manage pasid and vm ptr mapping + * + * @adev: amdgpu_device pointer + * @vm: amdgpu_vm pointer + * @pasid: the pasid the VM is using on this GPU + * + * Set the pasid this VM is using on this GPU, can also be used to remove the + * pasid by passing in zero. + * + */ +int amdgpu_vm_set_pasid(struct amdgpu_device *adev, struct amdgpu_vm *vm, + u32 pasid) +{ + int r; + + if (vm->pasid == pasid) + return 0; + + if (vm->pasid) { + r = xa_err(xa_erase_irq(&adev->vm_manager.pasids, vm->pasid)); + if (r < 0) + return r; + + vm->pasid = 0; + } + + if (pasid) { + r = xa_err(xa_store_irq(&adev->vm_manager.pasids, pasid, vm, + GFP_KERNEL)); + if (r < 0) + return r; + + vm->pasid = pasid; + } + + + return 0; +} + /* * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS * happens while holding this lock anywhere to prevent deadlocks when @@ -2945,18 +2985,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid) amdgpu_bo_unreserve(vm->root.bo); - if (pasid) { - unsigned long flags; - - spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); - r = idr_alloc(&adev->vm_manager.pasid_idr, vm, pasid, pasid + 1, - GFP_ATOMIC); - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); - if (r < 0) - goto error_free_root; - - vm->pasid = pasid; - } + r = amdgpu_vm_set_pasid(adev, vm, pasid); + if (r) + goto error_free_root; INIT_KFIFO(vm->faults); @@ -3044,18 +3075,15 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, if (r) goto unreserve_bo; - if (pasid) { - unsigned long flags; - - spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); - r = idr_alloc(&adev->vm_manager.pasid_idr, vm, pasid, pasid + 1, - GFP_ATOMIC); - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); + /* Free the original amdgpu allocated pasid, + * will be replaced with kfd allocated pasid. + */ + if (vm->pasid) + amdgpu_pasid_free(vm->pasid); - if (r == -ENOSPC) - goto unreserve_bo; - r = 0; - } + r = amdgpu_vm_set_pasid(adev, vm, pasid); + if (r) + goto unreserve_bo; /* Check if PD needs to be reinitialized and do it before * changing any other state, in case it fails. @@ -3066,7 +3094,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, to_amdgpu_bo_vm(vm->root.bo), false); if (r) - goto free_idr; + goto free_pasid_entry; } /* Update VM state */ @@ -3083,7 +3111,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, r = amdgpu_bo_sync_wait(vm->root.bo, AMDGPU_FENCE_OWNER_UNDEFINED, true); if (r) - goto free_idr; + goto free_pasid_entry; vm->update_funcs = &amdgpu_vm_cpu_funcs; } else { @@ -3093,36 +3121,13 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, vm->last_update = NULL; vm->is_compute_context = true; - if (vm->pasid) { - unsigned long flags; - - spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); - idr_remove(&adev->vm_manager.pasid_idr, vm->pasid); - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); - - /* Free the original amdgpu allocated pasid - * Will be replaced with kfd allocated pasid - */ - amdgpu_pasid_free(vm->pasid); - vm->pasid = 0; - } - /* Free the shadow bo for compute VM */ amdgpu_bo_unref(&to_amdgpu_bo_vm(vm->root.bo)->shadow); - if (pasid) - vm->pasid = pasid; - goto unreserve_bo; -free_idr: - if (pasid) { - unsigned long flags; - - spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); - idr_remove(&adev->vm_manager.pasid_idr, pasid); - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); - } +free_pasid_entry: + amdgpu_vm_set_pasid(adev, vm, 0); unreserve_bo: amdgpu_bo_unreserve(vm->root.bo); return r; @@ -3138,14 +3143,7 @@ unreserve_bo: */ void amdgpu_vm_release_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm) { - if (vm->pasid) { - unsigned long flags; - - spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); - idr_remove(&adev->vm_manager.pasid_idr, vm->pasid); - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); - } - vm->pasid = 0; + amdgpu_vm_set_pasid(adev, vm, 0); vm->is_compute_context = false; } @@ -3169,15 +3167,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm) root = amdgpu_bo_ref(vm->root.bo); amdgpu_bo_reserve(root, true); - if (vm->pasid) { - unsigned long flags; - - spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); - idr_remove(&adev->vm_manager.pasid_idr, vm->pasid); - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); - vm->pasid = 0; - } - + amdgpu_vm_set_pasid(adev, vm, 0); dma_fence_wait(vm->last_unlocked, false); dma_fence_put(vm->last_unlocked); @@ -3259,8 +3249,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev) adev->vm_manager.vm_update_mode = 0; #endif - idr_init(&adev->vm_manager.pasid_idr); - spin_lock_init(&adev->vm_manager.pasid_lock); + xa_init_flags(&adev->vm_manager.pasids, XA_FLAGS_LOCK_IRQ); } /** @@ -3272,8 +3261,8 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev) */ void amdgpu_vm_manager_fini(struct amdgpu_device *adev) { - WARN_ON(!idr_is_empty(&adev->vm_manager.pasid_idr)); - idr_destroy(&adev->vm_manager.pasid_idr); + WARN_ON(!xa_empty(&adev->vm_manager.pasids)); + xa_destroy(&adev->vm_manager.pasids); amdgpu_vmid_mgr_fini(adev); } @@ -3342,13 +3331,13 @@ void amdgpu_vm_get_task_info(struct amdgpu_device *adev, u32 pasid, struct amdgpu_vm *vm; unsigned long flags; - spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); + xa_lock_irqsave(&adev->vm_manager.pasids, flags); - vm = idr_find(&adev->vm_manager.pasid_idr, pasid); + vm = xa_load(&adev->vm_manager.pasids, pasid); if (vm) *task_info = vm->task_info; - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); + xa_unlock_irqrestore(&adev->vm_manager.pasids, flags); } /** @@ -3390,15 +3379,15 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid, struct amdgpu_vm *vm; int r; - spin_lock_irqsave(&adev->vm_manager.pasid_lock, irqflags); - vm = idr_find(&adev->vm_manager.pasid_idr, pasid); + xa_lock_irqsave(&adev->vm_manager.pasids, irqflags); + vm = xa_load(&adev->vm_manager.pasids, pasid); if (vm) { root = amdgpu_bo_ref(vm->root.bo); is_compute_context = vm->is_compute_context; } else { root = NULL; } - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, irqflags); + xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags); if (!root) return false; @@ -3416,11 +3405,11 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid, goto error_unref; /* Double check that the VM still exists */ - spin_lock_irqsave(&adev->vm_manager.pasid_lock, irqflags); - vm = idr_find(&adev->vm_manager.pasid_idr, pasid); + xa_lock_irqsave(&adev->vm_manager.pasids, irqflags); + vm = xa_load(&adev->vm_manager.pasids, pasid); if (vm && vm->root.bo != root) vm = NULL; - spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, irqflags); + xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags); if (!vm) goto error_unlock; -- cgit From 88f7f88159bcdff96b2a5d244b26c8ba99b5e773 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Fri, 2 Jul 2021 11:10:52 +0200 Subject: drm/amdgpu: separate out vm pasid assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use new helper function amdgpu_vm_set_pasid() to assign vm pasid value. This also ensures that we don't free a pasid from vm code as pasids are allocated somewhere else. Signed-off-by: Nirmoy Das Acked-by: Felix Kuehling Reviewed-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 9ea527ea3238..2a88ed5d983b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -2904,14 +2904,13 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout) * * @adev: amdgpu_device pointer * @vm: requested vm - * @pasid: Process address space identifier * * Init @vm fields. * * Returns: * 0 for success, error for failure. */ -int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid) +int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm) { struct amdgpu_bo *root_bo; struct amdgpu_bo_vm *root; @@ -2985,10 +2984,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid) amdgpu_bo_unreserve(vm->root.bo); - r = amdgpu_vm_set_pasid(adev, vm, pasid); - if (r) - goto error_free_root; - INIT_KFIFO(vm->faults); return 0; @@ -3044,7 +3039,6 @@ static int amdgpu_vm_check_clean_reserved(struct amdgpu_device *adev, * * @adev: amdgpu_device pointer * @vm: requested vm - * @pasid: pasid to use * * This only works on GFX VMs that don't have any BOs added and no * page tables allocated yet. @@ -3052,7 +3046,6 @@ static int amdgpu_vm_check_clean_reserved(struct amdgpu_device *adev, * Changes the following VM parameters: * - use_cpu_for_update * - pte_supports_ats - * - pasid (old PASID is released, because compute manages its own PASIDs) * * Reinitializes the page directory to reflect the changed ATS * setting. @@ -3060,8 +3053,7 @@ static int amdgpu_vm_check_clean_reserved(struct amdgpu_device *adev, * Returns: * 0 for success, -errno for errors. */ -int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, - u32 pasid) +int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm) { bool pte_support_ats = (adev->asic_type == CHIP_RAVEN); int r; @@ -3075,16 +3067,6 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, if (r) goto unreserve_bo; - /* Free the original amdgpu allocated pasid, - * will be replaced with kfd allocated pasid. - */ - if (vm->pasid) - amdgpu_pasid_free(vm->pasid); - - r = amdgpu_vm_set_pasid(adev, vm, pasid); - if (r) - goto unreserve_bo; - /* Check if PD needs to be reinitialized and do it before * changing any other state, in case it fails. */ @@ -3094,7 +3076,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, to_amdgpu_bo_vm(vm->root.bo), false); if (r) - goto free_pasid_entry; + goto unreserve_bo; } /* Update VM state */ @@ -3111,7 +3093,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, r = amdgpu_bo_sync_wait(vm->root.bo, AMDGPU_FENCE_OWNER_UNDEFINED, true); if (r) - goto free_pasid_entry; + goto unreserve_bo; vm->update_funcs = &amdgpu_vm_cpu_funcs; } else { @@ -3126,8 +3108,6 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, goto unreserve_bo; -free_pasid_entry: - amdgpu_vm_set_pasid(adev, vm, 0); unreserve_bo: amdgpu_bo_unreserve(vm->root.bo); return r; -- cgit From 4e7b93ca52fb228b177168d436449c5671415a72 Mon Sep 17 00:00:00 2001 From: Eric Huang Date: Fri, 9 Jul 2021 14:55:03 -0400 Subject: Revert "drm/amdgpu: Fix warning of Function parameter or member not described" This reverts commit 7a68d188d1c4a9d947369acaa19040a58baaaeda. Reason for revert: the related commit is reverted. Signed-off-by: Eric Huang Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 2a88ed5d983b..f735282ba878 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1856,7 +1856,6 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem, * @adev: amdgpu_device pointer * @bo_va: requested BO and VM object * @clear: if true clear the entries - * @table_freed: return true if page table is freed * * Fill in the page table entries for @bo_va. * -- cgit From 024d8811c90ed56d8b90cdcf71e51c9fedeff460 Mon Sep 17 00:00:00 2001 From: Eric Huang Date: Fri, 9 Jul 2021 14:58:16 -0400 Subject: Revert "drm/amdgpu: Add table_freed parameter to amdgpu_vm_bo_update" This reverts commit 075e8080c1a7571563171a07fa9ce47c4bc80044. Reason for revert: the related commit is reverted. Signed-off-by: Eric Huang Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index f735282ba878..6362e861a3f5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1798,7 +1798,7 @@ int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev, r = vm->update_funcs->commit(¶ms, fence); if (table_freed) - *table_freed = *table_freed || params.table_freed; + *table_freed = params.table_freed; error_unlock: amdgpu_vm_eviction_unlock(vm); @@ -1863,7 +1863,7 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem, * 0 for success, -EINVAL for failure. */ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va, - bool clear, bool *table_freed) + bool clear) { struct amdgpu_bo *bo = bo_va->base.bo; struct amdgpu_vm *vm = bo_va->base.vm; @@ -1942,7 +1942,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va, resv, mapping->start, mapping->last, update_flags, mapping->offset, mem, - pages_addr, last_update, table_freed); + pages_addr, last_update, NULL); if (r) return r; } @@ -2194,7 +2194,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev, list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) { /* Per VM BOs never need to bo cleared in the page tables */ - r = amdgpu_vm_bo_update(adev, bo_va, false, NULL); + r = amdgpu_vm_bo_update(adev, bo_va, false); if (r) return r; } @@ -2213,7 +2213,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev, else clear = true; - r = amdgpu_vm_bo_update(adev, bo_va, clear, NULL); + r = amdgpu_vm_bo_update(adev, bo_va, clear); if (r) return r; -- cgit From e9949dd79182a29410379dfebea8870f6abfa6e7 Mon Sep 17 00:00:00 2001 From: Eric Huang Date: Mon, 26 Jul 2021 11:23:17 -0400 Subject: Revert "Revert "drm/amdgpu: Add table_freed parameter to amdgpu_vm_bo_update"" This reverts commit 024d8811c90ed56d8b90cdcf71e51c9fedeff460. Revert reason: The issue has been resolved. Signed-off-by: Eric Huang Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 6362e861a3f5..f735282ba878 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1798,7 +1798,7 @@ int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev, r = vm->update_funcs->commit(¶ms, fence); if (table_freed) - *table_freed = params.table_freed; + *table_freed = *table_freed || params.table_freed; error_unlock: amdgpu_vm_eviction_unlock(vm); @@ -1863,7 +1863,7 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem, * 0 for success, -EINVAL for failure. */ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va, - bool clear) + bool clear, bool *table_freed) { struct amdgpu_bo *bo = bo_va->base.bo; struct amdgpu_vm *vm = bo_va->base.vm; @@ -1942,7 +1942,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va, resv, mapping->start, mapping->last, update_flags, mapping->offset, mem, - pages_addr, last_update, NULL); + pages_addr, last_update, table_freed); if (r) return r; } @@ -2194,7 +2194,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev, list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) { /* Per VM BOs never need to bo cleared in the page tables */ - r = amdgpu_vm_bo_update(adev, bo_va, false); + r = amdgpu_vm_bo_update(adev, bo_va, false, NULL); if (r) return r; } @@ -2213,7 +2213,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev, else clear = true; - r = amdgpu_vm_bo_update(adev, bo_va, clear); + r = amdgpu_vm_bo_update(adev, bo_va, clear, NULL); if (r) return r; -- cgit From 3b2b254425cc92c2c603efac488188d3084595ac Mon Sep 17 00:00:00 2001 From: Eric Huang Date: Mon, 26 Jul 2021 11:27:53 -0400 Subject: Revert "Revert "drm/amdgpu: Fix warning of Function parameter or member not described"" This reverts commit 4e7b93ca52fb228b177168d436449c5671415a72. Revert reason: The issue has been resolved. Signed-off-by: Eric Huang Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index f735282ba878..2a88ed5d983b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1856,6 +1856,7 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem, * @adev: amdgpu_device pointer * @bo_va: requested BO and VM object * @clear: if true clear the entries + * @table_freed: return true if page table is freed * * Fill in the page table entries for @bo_va. * -- cgit