Re: [Intel-xe] [PATCH] drm/xe: Fix unbind of unaccessed VMA (fault mode)

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Nov 06, 2023 at 02:15:38PM -0800, Brian Welty wrote:
> In fault mode, page table binding is deferred until fault handler.
> Thus vma->tile_present will be unset unless the VMA is accessed by GPU.
> 
> During a later unbind, the logic doesn't account for the fact that local
> fence variable will be NULL in this case, leading to pass NULL into
> dma_fence_add_callback() and causing few WARN_ONs to print to console.
> The fix is already present in the code, just hoist the fence variable
> computation to be done earlier.
> 
> Resolves warnings seen with igt@xe_exec_fault_mode@once-invalid-fault
> 
> Signed-off-by: Brian Welty <brian.welty@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_vm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index d26c90f0d702..d45f4f1d490f 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1738,14 +1738,14 @@ xe_vm_unbind_vma(struct xe_vma *vma, struct xe_exec_queue *q,
>  		}
>  	}
>  
> +	fence = cf ? &cf->base : !fence ?
> +		xe_exec_queue_last_fence_get(wait_exec_queue, vm) : fence;
>  	if (last_op) {
>  		for (i = 0; i < num_syncs; i++)
> -			xe_sync_entry_signal(&syncs[i], NULL,
> -					     cf ? &cf->base : fence);
> +			xe_sync_entry_signal(&syncs[i], NULL, fence);
>  	}
>  
> -	return cf ? &cf->base : !fence ?
> -		xe_exec_queue_last_fence_get(wait_exec_queue, vm) : fence;
> +	return fence;
>  
>  err_fences:
>  	if (fences) {
> -- 
> 2.38.0
>