From: Honglei Huang <[email protected]> SVM mapping updates, MMU notifier unmaps and VM eviction need to use the same lock. Separate locks cannot guarantee the order of GPU page table updates when CPU mappings are invalidated concurrently.
Point vm->eviction_lock at the drm_gpusvm notifier rwsem for SVM VMs. Hold the read side across range validation, PTE and PDE updates, and mapping publication. Use the write side for MMU notifier invalidation, attribute PTE zaps and VM eviction. Publish the selected lock before making vm->svm visible. During teardown, reserve the root BO and remove the MMU notifiers while the alias is still valid, then restore the default lock before releasing the SVM context. Suggested-by: Christian König <[email protected]> Signed-off-by: Honglei Huang <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c | 37 +++++++++---- drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c | 53 ++++++++----------- 2 files changed, 50 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c index c0db597b1e..fcbe23cf7f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c @@ -157,10 +157,13 @@ amdgpu_svm_lookup_by_pasid(struct amdgpu_device *adev, uint32_t pasid) amdgpu_pasid_lock(&irqflags); fpriv = amdgpu_pasid_get_fpriv_locked(pasid); vm = fpriv ? &fpriv->vm : NULL; - if (vm && vm->svm) { - svm = vm->svm; + /* + * eviction_lock changes when SVM is enabled, so acquire vm->svm + * after the new lock alias is published. + */ + svm = vm ? smp_load_acquire(&vm->svm) : NULL; + if (svm) kref_get(&svm->refcount); - } amdgpu_pasid_unlock(irqflags); return svm; @@ -488,6 +491,8 @@ static int amdgpu_svm_init_compute(struct amdgpu_device *adev, struct amdgpu_svm *svm; int ret; + dma_resv_assert_held(vm->root.bo->tbo.base.resv); + if (vm->svm) return 0; @@ -522,7 +527,9 @@ static int amdgpu_svm_init_compute(struct amdgpu_device *adev, 1UL << (svm->default_granularity + PAGE_SHIFT), svm->xnack_enabled ? "enabled" : "disabled"); - vm->svm = svm; + vm->eviction_lock = &svm->gpusvm.notifier_lock; + /* Publish the context after its notifier lock alias. */ + smp_store_release(&vm->svm, svm); return 0; err_free: @@ -598,31 +605,43 @@ void amdgpu_svm_close(struct amdgpu_vm *vm) * amdgpu_svm_fini() - Finalize and release a VM's SVM context * @vm: The VM whose SVM context is being torn down. * - * Close the context, tear down the embedded drm_gpusvm under the SVM lock, - * destroy the attribute tree and work queues, and drop the context - * reference. Safe to call on a VM without an SVM context. + * Tear down the embedded drm_gpusvm, attribute tree and work queues, and drop + * the context reference. */ void amdgpu_svm_fini(struct amdgpu_vm *vm) { struct amdgpu_svm *svm = vm->svm; + struct amdgpu_bo *root; if (!svm) return; amdgpu_svm_close(vm); amdgpu_svm_lock(svm); + + root = amdgpu_bo_ref(vm->root.bo); + amdgpu_bo_reserve(root, true); + drm_gpusvm_fini(&svm->gpusvm); + + vm->eviction_lock = &vm->default_eviction_lock; + /* Publish the lock reset before making SVM unavailable. */ + smp_store_release(&vm->svm, NULL); + amdgpu_svm_unlock(svm); amdgpu_svm_attr_tree_destroy(svm->attr_tree); amdgpu_svm_work_fini(svm); - vm->svm = NULL; amdgpu_svm_put(svm); + + amdgpu_bo_unreserve(root); + amdgpu_bo_unref(&root); } bool amdgpu_svm_is_enabled(struct amdgpu_vm *vm) { - return vm->svm != NULL; + /* Pairs with publishing vm->svm after its lock alias is initialized. */ + return smp_load_acquire(&vm->svm) != NULL; } static int amdgpu_svm_copy_attrs(const struct drm_amdgpu_gem_svm *args, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c index 3e622ac059..046910c2a0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c @@ -65,16 +65,14 @@ amdgpu_svm_range_zap_ptes(struct amdgpu_svm *svm, unsigned long last_page) { struct dma_fence *fence = NULL; - unsigned int flags; int ret; if (last_page < start_page) return 0; - flags = memalloc_noreclaim_save(); - ret = amdgpu_vm_unmap_range(svm->adev, svm->vm, NULL, - start_page, last_page, 0, NULL, &fence); - memalloc_noreclaim_restore(flags); + ret = amdgpu_vm_unmap_range(svm->adev, svm->vm, NULL, start_page, + last_page, 0, svm->vm->eviction_lock, + &fence); if (fence) { dma_fence_wait(fence, false); @@ -210,20 +208,11 @@ amdgpu_svm_range_update_gpu_range(struct amdgpu_svm *svm, mapped_pages += seg_pages; is_last_seg = mapped_pages == npages; - /* - * The struct hmm_range hmm_range field inside amdgpu_hmm_range - * is drm_gpusvm internal state. It is not exposed to the - * implementing driver and is managed entirely by the framework - * as part of the framework logic. This amdgpu_vm_map_range call - * always runs inside the drm_gpusvm notifier, so omitting - * hmm_range here is safe. - */ - ret = amdgpu_vm_map_range(svm->adev, svm->vm, - flush_tlb && is_last_seg, true, NULL, - start_page, last_page, pte_flags, - 0, entry->addr, NULL, NULL, NULL, - NULL, NULL, - wait_fence && is_last_seg ? fence : NULL); + ret = amdgpu_vm_map_range( + svm->adev, svm->vm, flush_tlb && is_last_seg, true, + NULL, start_page, last_page, pte_flags, 0, entry->addr, + NULL, NULL, NULL, svm->vm->eviction_lock, base, + wait_fence && is_last_seg ? fence : NULL); if (ret) return ret; } @@ -304,14 +293,12 @@ int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm, { struct drm_exec exec; struct dma_fence *fence = NULL; - unsigned int flags; int ret; ret = amdgpu_svm_range_lock_vm_pd(svm, &exec, intr); if (ret) return ret; - flags = memalloc_noreclaim_save(); drm_gpusvm_notifier_lock(&svm->gpusvm); if (!amdgpu_svm_range_pages_valid(svm, range)) { @@ -323,24 +310,25 @@ int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm, wait, wait ? &fence : NULL); } - drm_gpusvm_notifier_unlock(&svm->gpusvm); - memalloc_noreclaim_restore(flags); - - if (!ret && fence) - dma_fence_wait(fence, intr); - dma_fence_put(fence); - if (!ret) - ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, false, NULL); + ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, false, + svm->vm->eviction_lock); if (!ret) { - if (flush_tlb) - amdgpu_svm_flush_tlb(svm); WRITE_ONCE(range->attr_flags, attrs->flags); WRITE_ONCE(range->gpu_mapped, true); range->validate_timestamp = ktime_get_boottime(); } + drm_gpusvm_notifier_unlock(&svm->gpusvm); + + if (!ret && fence) + dma_fence_wait(fence, intr); + dma_fence_put(fence); + + if (!ret && flush_tlb) + amdgpu_svm_flush_tlb(svm); + drm_exec_fini(&exec); return ret; } @@ -583,9 +571,12 @@ amdgpu_svm_range_invalidate_interval(struct amdgpu_svm *svm, crosses_boundary ? "ATTR DESTROY" : "ATTR ZAP PTE"); + /* Use the unified eviction/MMU notifier lock for PTE zaps. */ + down_write(svm->vm->eviction_lock); ret = amdgpu_svm_range_zap_ptes(svm, svm_range, drm_gpusvm_range_start(range) >> PAGE_SHIFT, (drm_gpusvm_range_end(range) >> PAGE_SHIFT) - 1); + up_write(svm->vm->eviction_lock); if (ret < 0) { drm_exec_fini(&exec); return ret; -- 2.34.1
