Re: [Intel-xe] [PATCH] drm/xe: Set max pte size when skipping rebinds

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Aug 02, 2023 at 08:23:17PM -0700, Matthew Brost wrote:
> When a rebind is skipped, we must set the max pte size of the newly
> created vma to value of the old vma as we do not pte walk for the new
> vma. Without this future rebinds may be incorrectly skipped due to the
> wrong max pte size. Null binds are more likely to expose this bug as
> larger ptes are more frequently used compared to normal bindings.
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> Testcase: dEQP-VK.sparse_resources.buffer.ssbo.sparse_residency.buffer_size_2_24
> Reported-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Fixes: a042bb4ca894 ("drm/xe: Avoid doing rebinds")
> Reference: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23045

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_vm.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index cb28dbc2bdbb..936492915ecb 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2406,6 +2406,20 @@ static u64 xe_vma_max_pte_size(struct xe_vma *vma)
>  	return SZ_4K;
>  }
>  
> +static u64 xe_vma_set_pte_size(struct xe_vma *vma, u64 size)
> +{
> +	switch (size) {
> +	case SZ_1G:
> +		vma->gpuva.flags |= XE_VMA_PTE_1G;
> +		break;
> +	case SZ_2M:
> +		vma->gpuva.flags |= XE_VMA_PTE_2M;
> +		break;
> +	}
> +
> +	return SZ_4K;
> +}
> +
>  /*
>   * Parse operations list and create any resources needed for the operations
>   * prior to fully committing to the operations. This setup can fail.
> @@ -2516,6 +2530,7 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_exec_queue *q,
>  						IS_ALIGNED(xe_vma_end(vma),
>  							   xe_vma_max_pte_size(old));
>  					if (op->remap.skip_prev) {
> +						xe_vma_set_pte_size(vma, xe_vma_max_pte_size(old));
>  						op->remap.range -=
>  							xe_vma_end(vma) -
>  							xe_vma_start(old);
> @@ -2550,10 +2565,12 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_exec_queue *q,
>  					op->remap.skip_next = !xe_vma_is_userptr(old) &&
>  						IS_ALIGNED(xe_vma_start(vma),
>  							   xe_vma_max_pte_size(old));
> -					if (op->remap.skip_next)
> +					if (op->remap.skip_next) {
> +						xe_vma_set_pte_size(vma, xe_vma_max_pte_size(old));
>  						op->remap.range -=
>  							xe_vma_end(old) -
>  							xe_vma_start(vma);
> +					}
>  				}
>  				break;
>  			}
> -- 
> 2.34.1
>