Re: [PATCH v2] drm/amdgpu: wait for tlb flush before freeing BO

["Christian König" <christian.koenig@amd.com>]

On 11/6/26 16:20, 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.
> 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.
> 
> Fix this by attaching the last_tlb_inv fence to the BO's resv.
> 
> Fixes: a1d201e16940 ("drm/amdgpu: enable GFX-V11 userqueue support")
> Signed-off-by: Benjamin Cheng <benjamin.cheng@amd.com>
> ---
> v2:
> - Rebase on "drm/amdgpu: fix TLB invalidation fence handling v4" to get
>   last_tlb_inv
> - Move the fence after the clear_freed call to make sure we wait for the
>   right invalidation.
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 6137ab4211be..36b0bd85616f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -371,11 +371,14 @@ static void amdgpu_gem_object_close(struct drm_gem_object *obj,
>  		dev_err(adev->dev, "failed to clear page "
>  			"tables on GEM object close (%ld)\n", r);
>  	if (r || !fence)
> -		goto out_unlock;
> +		goto out_tlb;
>  
>  	amdgpu_bo_fence(bo, fence, true);
>  	dma_fence_put(fence);
>  
> +out_tlb:
> +	amdgpu_bo_fence(bo, vm->last_tlb_inv, true);

You need to change the drm_exec_prepare_obj() call above from 1 to 2 since we are now attaching 2 fences.

Apart from that looks good to me.

Regards,
Christian.

> +
>  out_unlock:
>  	if (r && !drm_dev_is_unplugged(adev_to_drm(adev)))
>  		dev_err(adev->dev, "leaking bo va (%ld)\n", r);