* [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update @ 2026-11-09 14:06 chong li 2026-11-09 14:08 ` Christian König 0 siblings, 1 reply; 4+ messages in thread From: chong li @ 2026-11-09 14:06 UTC (permalink / raw) To: amd-gfx; +Cc: christian.koenig, chong li 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 <chongli2@amd.com> --- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update 2026-11-09 14:06 [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update chong li @ 2026-11-09 14:08 ` Christian König 2026-11-10 9:42 ` Li, Chong(Alan) 0 siblings, 1 reply; 4+ messages in thread From: Christian König @ 2026-11-09 14:08 UTC (permalink / raw) To: chong li, amd-gfx On 11/9/26 15:06, chong li wrote: > 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. That's not even remotely correct. All fences use the same lock, that's why this works correctly. Regards, Christian. > > 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 <chongli2@amd.com> > --- > 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, ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update 2026-11-09 14:08 ` Christian König @ 2026-11-10 9:42 ` Li, Chong(Alan) 2026-11-10 10:12 ` Christian König 0 siblings, 1 reply; 4+ messages in thread From: Li, Chong(Alan) @ 2026-11-10 9:42 UTC (permalink / raw) To: Koenig, Christian, amd-gfx@lists.freedesktop.org AMD General Hi, Christian. The lock of each fence indeed never changes. Each PT update fence has its own lock. The issue is in the fence locking process. When a fence signals, dma_fence_signal() holds the fence's lock and calls amdgpu_vm_tlb_seq_cb(), which increments vm->tlb_seq. amdgpu_vm_tlb_seq() waits on the lock of vm->last_tlb_flush until amdgpu_vm_tlb_seq_cb() completes. The lock of vm->last_tlb_flush ensures that amdgpu_vm_tlb_seq() returns the incremented vm->tlb_seq. This is the normal flow. But vm->last_tlb_flush may already point to a new fence: amdgpu_vm_tlb_flush() replaces it as soon as a new PT update is submitted, usually before the old fence signals. amdgpu_vm_tlb_seq() then takes the new fence's lock, which is not held while the old fence's callback runs, so it is not blocked and returns vm->tlb_seq before it is incremented. Finally, updates = amdgpu_vm_tlb_seq() misses the increment, the condition (*id)->flushed_updates < updates is not satisfied, and needs_flush is false, so the job reuses the VMID without a VM flush. Please let me know if anything is unclear or if I'm missing something. Thanks, Chong. -----Original Message----- From: Koenig, Christian <Christian.Koenig@amd.com> Sent: Monday, November 9, 2026 10:08 PM To: Li, Chong(Alan) <Chong.Li@amd.com>; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update On 11/9/26 15:06, chong li wrote: > 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->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. That's not even remotely correct. All fences use the same lock, that's why this works correctly. Regards, Christian. > > 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 <chongli2@amd.com> > --- > 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, ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update 2026-11-10 9:42 ` Li, Chong(Alan) @ 2026-11-10 10:12 ` Christian König 0 siblings, 0 replies; 4+ messages in thread From: Christian König @ 2026-11-10 10:12 UTC (permalink / raw) To: Li, Chong(Alan), amd-gfx@lists.freedesktop.org Hi Chong, On 11/10/26 10:42, Li, Chong(Alan) wrote: > AMD General > > Hi, Christian. > > The lock of each fence indeed never changes. > Each PT update fence has its own lock. > > The issue is in the fence locking process. > > When a fence signals, dma_fence_signal() holds the fence's lock and > calls amdgpu_vm_tlb_seq_cb(), which increments vm->tlb_seq. > amdgpu_vm_tlb_seq() waits on the lock of vm->last_tlb_flush until > amdgpu_vm_tlb_seq_cb() completes. The lock of vm->last_tlb_flush > ensures that amdgpu_vm_tlb_seq() returns the incremented > vm->tlb_seq. This is the normal flow. > > But vm->last_tlb_flush may already point to a new fence: > amdgpu_vm_tlb_flush() replaces it as soon as a new PT update is > submitted, usually before the old fence signals. amdgpu_vm_tlb_seq() > then takes the new fence's lock, which is not held while the old > fence's callback runs, so it is not blocked and returns vm->tlb_seq > before it is incremented. No you are mixing things up, vm->last_tlb_flush was something completely different. vm->last_seq_flush is the fence where we install the callback on and yes that is replaced with the next installed callback, but that doesn't matter the lock is always the same. vm->last_tlb_inv is the TLB invalidation fence which actively invalidates the TLB for a passid. That are two completely different things. Regards, Christian. > > Finally, updates = amdgpu_vm_tlb_seq() misses the increment, the > condition (*id)->flushed_updates < updates is not satisfied, and > needs_flush is false, so the job reuses the VMID without a VM flush. > > Please let me know if anything is unclear or if I'm missing something. > > > > Thanks, > Chong. > > > > > -----Original Message----- > From: Koenig, Christian <Christian.Koenig@amd.com> > Sent: Monday, November 9, 2026 10:08 PM > To: Li, Chong(Alan) <Chong.Li@amd.com>; amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update > > On 11/9/26 15:06, chong li wrote: >> 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->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. > > That's not even remotely correct. > > All fences use the same lock, that's why this works correctly. > > Regards, > Christian. > >> >> 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 <chongli2@amd.com> >> --- >> 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, > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-10 10:13 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-09 14:06 [PATCH] drm/amdgpu: fix page fault by not skipping VM flush before tlb_seq update chong li 2026-11-09 14:08 ` Christian König 2026-11-10 9:42 ` Li, Chong(Alan) 2026-11-10 10:12 ` Christian König
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.