Re: [PATCH] drm/amdgpu: Make emit_vm_flush() check explicit in amdgpu_vm_flush()

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 15-Mar-26 12:25 PM, Srinivasan Shanmugam wrote:
> amdgpu_vm_flush() sends commands to the GPU to update the VM page tables
> for a job.
> 
> When a job uses a GPU virtual address space, the GPU needs to refresh
> its address translations after the driver updates the page tables.
> A VM flush tells the GPU to forget old address translations and use the
> updated page table mappings.
> 
> This flush command is not supported on all rings. Only rings that
> implement the emit_vm_flush() callback know how to emit the correct
> hardware command for this operation.
> 
> The function already gates vm_flush_needed on the presence of
> ring->funcs->emit_vm_flush earlier in the logic. However, static
> analysis tools such as Smatch may not track this relationship through
> the vm_flush_needed boolean and warn that emit_vm_flush() could be NULL
> when the VM flush command is emitted later.
> 
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:826 amdgpu_vm_flush() error: we previously assumed 'ring->funcs->emit_vm_flush' could be null (see line 788)
> 

I think the code logic is correct and the check is implicit in 
vm_flush_needed. We don't need to write code to eliminate 'false 
warnings'. Moreover, it is inappropriate to use a Fixes tag for this.

Thanks,
Lijo

> Fixes: b3cd285fa68d ("drm/amdgpu: update the PASID mapping only on demand")
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index b89013a6aa0b..cc79cb7dd4e1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -807,7 +807,7 @@ void amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
>   	if (cleaner_shader_needed)
>   		ring->funcs->emit_cleaner_shader(ring);
>   
> -	if (vm_flush_needed) {
> +	if (vm_flush_needed && ring->funcs->emit_vm_flush) {
>   		trace_amdgpu_vm_flush(ring, job->vmid, job->vm_pd_addr);
>   		amdgpu_ring_emit_vm_flush(ring, job->vmid, job->vm_pd_addr);
>   	}