amdgpu_vm_tlb_seq() waits for the tlb_seq callback by taking the lock of vm->last_tlb_flush. Newer page table updates replace vm->last_tlb_flush immediately, without waiting for older callbacks. A job scheduled after an older fence signals but before its callback finishes takes the newer fence's lock, while the callback runs under the older fence's own lock. The job misses that tlb_seq increment and can reuse a VMID whose TLB was last flushed before the older page table update completed, without flushing the TLB again.
how to fix: Count the flush-requiring page table updates whose tlb_seq callback has not run yet, and request a VM flush when a job reuses a VMID while this count is non-zero. Signed-off-by: chong li <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 10 ++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 10 ++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c index 9c879ac68352..b9e058da162b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c @@ -393,7 +393,8 @@ static int amdgpu_vmid_grab_reserved(struct amdgpu_vm *vm, unsigned vmhub = ring->vm_hub; uint64_t fence_context = adev->fence_context + ring->idx; bool needs_flush = vm->use_cpu_for_update; - uint64_t updates = amdgpu_vm_tlb_seq(vm); + bool tlb_pending; + uint64_t updates = amdgpu_vm_tlb_seq_pending(vm, &tlb_pending); int r; *id = vm->reserved_vmid[vmhub]; @@ -405,6 +406,8 @@ static int amdgpu_vmid_grab_reserved(struct amdgpu_vm *vm, !dma_fence_is_signaled((*id)->last_flush))) needs_flush = true; + needs_flush |= tlb_pending; + if ((*id)->owner != vm->immediate.fence_context || (!adev->vm_manager.concurrent_flush && needs_flush)) { struct dma_fence *tmp; @@ -456,7 +459,8 @@ static int amdgpu_vmid_grab_used(struct amdgpu_vm *vm, unsigned vmhub = ring->vm_hub; struct amdgpu_kq_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub]; uint64_t fence_context = adev->fence_context + ring->idx; - uint64_t updates = amdgpu_vm_tlb_seq(vm); + bool tlb_pending; + uint64_t updates = amdgpu_vm_tlb_seq_pending(vm, &tlb_pending); int r; job->vm_needs_flush = vm->use_cpu_for_update; @@ -480,6 +484,8 @@ static int amdgpu_vmid_grab_used(struct amdgpu_vm *vm, if ((*id)->flushed_updates < updates) needs_flush = true; + needs_flush |= tlb_pending; + if (needs_flush && !adev->vm_manager.concurrent_flush) continue; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 2ba33212d507..6d15fd001a42 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1095,6 +1095,9 @@ static void amdgpu_vm_tlb_seq_cb(struct dma_fence *fence, tlb_cb = container_of(cb, typeof(*tlb_cb), cb); atomic64_inc(&tlb_cb->vm->tlb_seq); + /* Pairs with atomic_read_acquire() in amdgpu_vm_tlb_seq_pending() */ + smp_mb__before_atomic(); + atomic_dec(&tlb_cb->vm->tlb_pending); kfree(tlb_cb); } @@ -1281,9 +1284,16 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm, start = tmp; } + /* Must be visible before the update fence can signal */ + if (params.needs_flush) + atomic_inc(&vm->tlb_pending); + r = vm->update_funcs->commit(¶ms, fence); - if (r) + if (r) { + if (params.needs_flush) + atomic_dec(&vm->tlb_pending); goto error_free; + } if (params.needs_flush) { amdgpu_vm_tlb_flush(¶ms, fence, tlb_cb); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index d8e17f493f61..312fd0d268ce 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -337,6 +337,8 @@ struct amdgpu_vm { /* Last finished delayed update */ atomic64_t tlb_seq; + /* Flush updates whose tlb_seq callback has not run yet */ + atomic_t tlb_pending; struct dma_fence *last_tlb_flush; atomic64_t kfd_last_flushed_seq; uint64_t tlb_fence_context; @@ -578,6 +580,14 @@ static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm) return atomic64_read(&vm->tlb_seq); } +/* Read tlb_pending first; pairs with the barrier in amdgpu_vm_tlb_seq_cb() */ +static inline uint64_t amdgpu_vm_tlb_seq_pending(struct amdgpu_vm *vm, + bool *pending) +{ + *pending = atomic_read_acquire(&vm->tlb_pending) != 0; + return amdgpu_vm_tlb_seq(vm); +} + void amdgpu_vm_update_fault_cache(struct amdgpu_device *adev, unsigned int pasid, uint64_t addr, -- 2.48.1
