drm/amdgpu: Unlocked unmap only clear page table leaves

[ Upstream commit 23b645231eeffdaf44021debac881d2f26824150 ]

SVM migration unmap pages from GPU and then update mapping to GPU to
recover page fault. Currently unmap clears the PDE entry for range
length >= huge page and free PTB bo, update mapping to alloc new PT bo.
There is race bug that the freed entry bo maybe still on the pt_free
list, reused when updating mapping and then freed, leave invalid PDE
entry and cause GPU page fault.

By setting the update to clear only one PDE entry or clear PTB, to
avoid unmap to free PTE bo. This fixes the race bug and improve the
unmap and map to GPU performance. Update mapping to huge page will
still free the PTB bo.

With this change, the vm->pt_freed list and work is not needed. Add
WARN_ON(unlocked) in amdgpu_vm_pt_free_dfs to catch if unmap to free the
PTB.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c9c48b782ec1b479471220e19aefe91bf60887c5..48b2c0b3b315a2c30c8c4ae0fea0c3226e85f05f 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2440,8 +2440,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
        spin_lock_init(&vm->status_lock);
        INIT_LIST_HEAD(&vm->freed);
        INIT_LIST_HEAD(&vm->done);
-       INIT_LIST_HEAD(&vm->pt_freed);
-       INIT_WORK(&vm->pt_free_work, amdgpu_vm_pt_free_work);
        INIT_KFIFO(vm->faults);
 
        r = amdgpu_vm_init_entities(adev, vm);
@@ -2613,8 +2611,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 
        amdgpu_amdkfd_gpuvm_destroy_cb(adev, vm);
 
-       flush_work(&vm->pt_free_work);
-
        root = amdgpu_bo_ref(vm->root.bo);
        amdgpu_bo_reserve(root, true);
        amdgpu_vm_put_task_info(vm->task_info);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 5d119ac26c4fe4c890f6f101d6bc0426f2a2a2fc..160889e5e64dd59a5e3158fce8869cf3a1faac48 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -369,10 +369,6 @@ struct amdgpu_vm {
        /* BOs which are invalidated, has been updated in the PTs */
        struct list_head        done;
 
-       /* PT BOs scheduled to free and fill with zero if vm_resv is not hold */
-       struct list_head        pt_freed;
-       struct work_struct      pt_free_work;
-
        /* contains the page directory */
        struct amdgpu_vm_bo_base     root;
        struct dma_fence        *last_update;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index f78a0434a48fa24d5db00a505116cb36fda768be..54ae0e9bc6d77293346afeb30725d88a44e37c2f 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -546,27 +546,6 @@ static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base *entry)
        amdgpu_bo_unref(&entry->bo);
 }
 
-void amdgpu_vm_pt_free_work(struct work_struct *work)
-{
-       struct amdgpu_vm_bo_base *entry, *next;
-       struct amdgpu_vm *vm;
-       LIST_HEAD(pt_freed);
-
-       vm = container_of(work, struct amdgpu_vm, pt_free_work);
-
-       spin_lock(&vm->status_lock);
-       list_splice_init(&vm->pt_freed, &pt_freed);
-       spin_unlock(&vm->status_lock);
-
-       /* flush_work in amdgpu_vm_fini ensure vm->root.bo is valid. */
-       amdgpu_bo_reserve(vm->root.bo, true);
-
-       list_for_each_entry_safe(entry, next, &pt_freed, vm_status)
-               amdgpu_vm_pt_free(entry);
-
-       amdgpu_bo_unreserve(vm->root.bo);
-}
-
 /**
  * amdgpu_vm_pt_free_list - free PD/PT levels
  *
@@ -579,19 +558,15 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
                            struct amdgpu_vm_update_params *params)
 {
        struct amdgpu_vm_bo_base *entry, *next;
-       struct amdgpu_vm *vm = params->vm;
        bool unlocked = params->unlocked;
 
        if (list_empty(&params->tlb_flush_waitlist))
                return;
 
-       if (unlocked) {
-               spin_lock(&vm->status_lock);
-               list_splice_init(&params->tlb_flush_waitlist, &vm->pt_freed);
-               spin_unlock(&vm->status_lock);
-               schedule_work(&vm->pt_free_work);
-               return;
-       }
+       /*
+        * unlocked unmap clear page table leaves, warning to free the page entry.
+        */
+       WARN_ON(unlocked);
 
        list_for_each_entry_safe(entry, next, &params->tlb_flush_waitlist, vm_status)
                amdgpu_vm_pt_free(entry);
@@ -899,7 +874,15 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
                incr = (uint64_t)AMDGPU_GPU_PAGE_SIZE << shift;
                mask = amdgpu_vm_pt_entries_mask(adev, cursor.level);
                pe_start = ((cursor.pfn >> shift) & mask) * 8;
-               entry_end = ((uint64_t)mask + 1) << shift;
+
+               if (cursor.level < AMDGPU_VM_PTB && params->unlocked)
+                       /*
+                        * MMU notifier callback unlocked unmap huge page, leave is PDE entry,
+                        * only clear one entry. Next entry search again for PDE or PTE leave.
+                        */
+                       entry_end = 1ULL << shift;
+               else
+                       entry_end = ((uint64_t)mask + 1) << shift;
                entry_end += cursor.pfn & ~(entry_end - 1);
                entry_end = min(entry_end, end);