On 2026-10-14 03:17, Christian König wrote:
>
> On 10/14/26 04:21, Benjamin Cheng wrote:
>> On gem object close, we previously only attached the fence of its own
>> clear_freed() call to the BO's resv. If an unmap of this BO happened
>> prior to the close call, clear_freed() would be invoked, leaving
>> vm->freed empty, and no meaningful fence to be attached to the resv on
>> close. Because of that the memory may be freed before the TLB flush is
>> complete, and give a running userq potential access to a stale
>> address translation.
> Good catch.
>
>> Fix it by tracking the last TLB fence of each VM and attaching it to the
>> BO on close.
>>
>> Fixes: a1d201e16940 ("drm/amdgpu: enable GFX-V11 userqueue support")
>> Signed-off-by: Benjamin Cheng <[email protected]>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
>> 3 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 8dbd7bb77305..7484c52e5850 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -360,6 +360,8 @@ static void amdgpu_gem_object_close(struct
>> drm_gem_object *obj,
>>
>> amdgpu_vm_bo_del(adev, bo_va);
>> amdgpu_vm_bo_update_shared(bo);
>> + if (!dma_fence_is_signaled(vm->last_tlb_flush_complete))
> Drop that extra check, it is actually harmfull.
I will drop it in the next revision, but could you explain why it's harmful?
(Sorry, still somewhat new to dma_fence semantics).
>
> You also need to increase the amount of reserved fence slots.
amdgpu_bo_fence already does that.
>
>
>> + amdgpu_bo_fence(bo, vm->last_tlb_flush_complete, true);
>> if (!amdgpu_vm_ready(vm))
>> goto out_unlock;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 7d3659ef11e5..c5411933a76c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1097,6 +1097,9 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params
>> *params,
>> if (!params->unlocked && vm->need_tlb_fence) {
>> amdgpu_vm_tlb_fence_create(params->adev, vm, fence);
>>
>> + dma_fence_put(vm->last_tlb_flush_complete);
>> + vm->last_tlb_flush_complete = dma_fence_get(*fence);
> I already added that in another patch, looks like amd-staging-drm-next is not
> up to date.
I guess it's last_tlb_inv from "drm/amdgpu: fix TLB invalidation fence
handling"? I'll rebase against that and use it if there are no other concerns.
>
> Let me double check that.
>
> Regards,
> Christian.
>
>> +
>> /* Makes sure no PD/PT is freed before the flush */
>> dma_resv_add_fence(vm->root.bo->tbo.base.resv, *fence,
>> DMA_RESV_USAGE_BOOKKEEP);
>> @@ -2648,6 +2651,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
>> amdgpu_vm *vm,
>> vm->last_update = dma_fence_get_stub();
>> vm->last_unlocked = dma_fence_get_stub();
>> vm->last_tlb_flush = dma_fence_get_stub();
>> + vm->last_tlb_flush_complete = dma_fence_get_stub();
>> vm->generation = amdgpu_vm_generation(adev, NULL);
>>
>> mutex_init(&vm->eviction_lock);
>> @@ -2693,6 +2697,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
>> amdgpu_vm *vm,
>> dma_fence_put(vm->last_update);
>> dma_fence_put(vm->last_tlb_flush);
>> dma_fence_put(vm->last_unlocked);
>> + dma_fence_put(vm->last_tlb_flush_complete);
>> ttm_lru_bulk_move_fini(&adev->mman.bdev, &vm->lru_bulk_move);
>> amdgpu_vm_fini_entities(vm);
>>
>> @@ -2839,6 +2844,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
>> amdgpu_vm *vm)
>> dma_fence_lock_irqsave(vm->last_tlb_flush, flags);
>> dma_fence_unlock_irqrestore(vm->last_tlb_flush, flags);
>> dma_fence_put(vm->last_tlb_flush);
>> + dma_fence_put(vm->last_tlb_flush_complete);
>>
>> list_for_each_entry_safe(mapping, tmp, &vm->freed, list) {
>> if (mapping->flags & AMDGPU_VM_PAGE_PRT && prt_fini_needed) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index ddcdc7256ef0..b9932c5e066f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -331,6 +331,7 @@ struct amdgpu_vm {
>> /* Last finished delayed update */
>> atomic64_t tlb_seq;
>> struct dma_fence *last_tlb_flush;
>> + struct dma_fence *last_tlb_flush_complete;
>> atomic64_t kfd_last_flushed_seq;
>> uint64_t tlb_fence_context;
>>
