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:
