AMD General Hi Christian, Ok. Consider this patch abandonned. Please CC me when you have fix so we can validate that use case on our side.
~David > -----Original Message----- > From: Koenig, Christian <[email protected]> > Sent: Tuesday, October 6, 2026 3:27 AM > To: Yat Sin, David <[email protected]>; [email protected] > Cc: Mani, Bincy <[email protected]>; Cursor <[email protected]>; > Natalie Vock <[email protected]>; Khatri, Sunil <[email protected]> > Subject: Re: [PATCH] drm/amdgpu: don't drop the TLB flush fence in the GEM VA > ioctl path > > Hi David, > > dropping the internal list and adding the public list and a few people. > > On 10/5/26 23:43, David Yat Sin wrote: > > When a compute VM is managed through the DRM GEM VA ioctl, the VM > > timeline syncobj can signal before the TLB invalidation has completed. > > Userspace that waits on the timeline and then reuses the VA can hit a > > stale translation and access the pages of the previous mapping. > > > > This was the call flow: > > amdgpu_vm_clear_freed() already asks for a TLB invalidation: it calls > > amdgpu_vm_update_range() with flush_tlb set, which reaches > > amdgpu_vm_tlb_flush() and, for a VM with need_tlb_fence, replaces the > > update fence with one created by amdgpu_vm_tlb_fence_create(). That > > fence only signals once amdgpu_gmc_flush_gpu_tlb_pasid() has run, so > > vm->last_update correctly covers the invalidation at that point. > > > > amdgpu_gem_va_update_vm() then calls amdgpu_vm_update_pdes(), whose > > commit() does swap(*fence, f) on &vm->last_update. When any PDE is > > dirty this overwrites the TLB flush fence with the PDE update job > > fence, and that is what gets attached to the user visible VM timeline > > syncobj. > > Good catch, we already had multiple people running into this problem. > Including > Sunil with userqueues. > > But this fix here is completely broken. What we need to do is to track the TLB > invalidation and page tables updates separately. > > Natalie and I already discussed parts of what is necessary here and will > probably > come up with a full patch to fix the issue. > > Regards, > Christian. > > > > > Userspace waiting on that timeline therefore only learns that the page > > table write landed, not that the TLB was invalidated. When no PDE is > > dirty amdgpu_vm_update_pdes() returns early, the fence survives and > > the same sequence works, which is why this only reproduces for some sizes. > > > > Publish a fence that covers the invalidation for compute VMs, with the > > page table update as its dependency, instead of blocking in the ioctl. > > > > Signed-off-by: David Yat Sin <[email protected]> > > Assisted-by: Cursor <[email protected]> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > > index efc270e99b4e..84f7d56a15ea 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > > @@ -799,6 +799,16 @@ amdgpu_gem_va_update_vm(struct amdgpu_device > *adev, > > fence = dma_fence_get(vm->last_update); > > } > > > > + /* > > + * A compute VM is shared with KFD and userspace reuses the VA as soon > > + * as this fence signals. The TLB flush fence produced while clearing > > + * the freed mappings is dropped when amdgpu_vm_update_pdes() replaces > > + * vm->last_update, so publish a fence that covers the invalidation > > + * instead of blocking here. > > + */ > > + if (vm->is_compute_context) > > + amdgpu_vm_tlb_fence_create(adev, vm, &fence); > > + > > return fence; > > > > error:
