[PATCH v4 20/20] drm/xe/madvise: Skip vma invalidation if mem attr are unchanged

[Matthew Brost]

On Fri, Jun 13, 2025 at 06:25:58PM +0530, Himal Prasad Ghimiray wrote:
> If a VMA within the madvise input range already has the same memory
> attribute as the one requested by the user, skip PTE zapping for that
> VMA to avoid unnecessary invalidation.
> 
> Suggested-by: Matthew Brost <matthew.brost at intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vm.c         |  1 +
>  drivers/gpu/drm/xe/xe_vm_madvise.c | 57 ++++++++++++++++++------------
>  drivers/gpu/drm/xe/xe_vm_types.h   |  6 ++++
>  3 files changed, 42 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index d9ce25f3abf4..56d6c286e3d3 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1226,6 +1226,7 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
>  		vma->gpuva.flags |= XE_VMA_ATOMIC_PTE_BIT;
>  
>  	vma->attr = *attr;
> +	vma->skip_invalidation = 0;

This kzalloc'd so not needed.

>  
>  	if (bo) {
>  		struct drm_gpuvm_bo *vm_bo;
> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index 5b96c8fc73a5..06e40ab0970e 100644
> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> @@ -88,13 +88,18 @@ static void madvise_preferred_mem_loc(struct xe_device *xe, struct xe_vm *vm,
>  	xe_assert(vm->xe, op->type == DRM_XE_VMA_ATTR_PREFERRED_LOC);
>  
>  	for (i = 0; i < num_vmas; i++) {
> -		vmas[i]->attr.preferred_loc.devmem_fd = op->preferred_mem_loc.devmem_fd;
> -
> -		/* Till multi-device support is not added migration_policy
> -		 * is of no use and can be ignored.
> -		 */
> -		vmas[i]->attr.preferred_loc.migration_policy =
> +		if (vmas[i]->attr.preferred_loc.devmem_fd == op->preferred_mem_loc.devmem_fd &&
> +		    vmas[i]->attr.preferred_loc.migration_policy ==
> +		    op->preferred_mem_loc.migration_policy) {
> +			vmas[i]->skip_invalidation = 1;

In the else statement, you need to clear this so subsequent madvise
which change properties do issue invalidations. 

> +		} else {
> +			vmas[i]->attr.preferred_loc.devmem_fd = op->preferred_mem_loc.devmem_fd;
> +			/* Till multi-device support is not added migration_policy
> +			 * is of no use and can be ignored.
> +			 */
> +			vmas[i]->attr.preferred_loc.migration_policy =
>  						op->preferred_mem_loc.migration_policy;
> +		}
>  	}
>  }
>  
> @@ -109,7 +114,10 @@ static void madvise_atomic(struct xe_device *xe, struct xe_vm *vm,
>  	xe_assert(vm->xe, op->atomic.val <= DRM_XE_VMA_ATOMIC_CPU);
>  
>  	for (i = 0; i < num_vmas; i++) {
> -		vmas[i]->attr.atomic_access = op->atomic.val;
> +		if (vmas[i]->attr.atomic_access == op->atomic.val)
> +			vmas[i]->skip_invalidation = 1;
> +		else
> +			vmas[i]->attr.atomic_access = op->atomic.val;
>  
>  		bo = xe_vma_bo(vmas[i]);
>  		if (!bo)
> @@ -134,9 +142,12 @@ static void madvise_pat_index(struct xe_device *xe, struct xe_vm *vm,
>  
>  	xe_assert(vm->xe, op->type == DRM_XE_VMA_ATTR_PAT);
>  
> -	for (i = 0; i < num_vmas; i++)
> -		vmas[i]->attr.pat_index = op->pat_index.val;
> -
> +	for (i = 0; i < num_vmas; i++) {
> +		if (vmas[i]->attr.pat_index == op->pat_index.val)
> +			vmas[i]->skip_invalidation = 1;
> +		else
> +			vmas[i]->attr.pat_index = op->pat_index.val;
> +	}
>  }
>  
>  typedef void (*madvise_func)(struct xe_device *xe, struct xe_vm *vm,
> @@ -161,23 +172,25 @@ static void xe_zap_ptes_in_madvise_range(struct xe_vm *vm, u64 start, u64 end, u
>  				  false, MAX_SCHEDULE_TIMEOUT) <= 0)
>  		XE_WARN_ON(1);
>  
> -	*tile_mask = xe_svm_ranges_zap_ptes_in_range(vm, start, end);
> -
>  	drm_gpuvm_for_each_va_range(gpuva, &vm->gpuvm, start, end) {
>  		struct xe_vma *vma = gpuva_to_vma(gpuva);
>  
> -		if (xe_vma_is_cpu_addr_mirror(vma))
> +		if (vma->skip_invalidation)
>  			continue;
>  
> -		if (xe_vma_is_userptr(vma)) {
> -			WARN_ON_ONCE(!dma_resv_test_signaled(xe_vm_resv(xe_vma_vm(vma)),
> -							     DMA_RESV_USAGE_BOOKKEEP));
> -		}
> -
> -		for_each_tile(tile, vm->xe, id) {
> -			if (xe_pt_zap_ptes(tile, vma)) {
> -				*tile_mask |= BIT(id);
> -				vma->tile_invalidated |= BIT(id);
> +		if (xe_vma_is_cpu_addr_mirror(vma)) {
> +			*tile_mask |= xe_svm_ranges_zap_ptes_in_range(vm,
> +								      xe_vma_start(vma),
> +								      xe_vma_end(vma));
> +		} else {
> +			if (xe_vma_is_userptr(vma))
> +				WARN_ON_ONCE(!dma_resv_test_signaled(xe_vm_resv(xe_vma_vm(vma)),
> +								     DMA_RESV_USAGE_BOOKKEEP));
> +			for_each_tile(tile, vm->xe, id) {
> +				if (xe_pt_zap_ptes(tile, vma)) {
> +					*tile_mask |= BIT(id);
> +					vma->tile_invalidated |= BIT(id);
> +				}
>  			}
>  		}
>  	}
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index c7156f8e2ed2..3b3019ecbfab 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -151,6 +151,12 @@ struct xe_vma {
>  	/** @tile_staged: bind is staged for this VMA */
>  	u8 tile_staged;
>  
> +	/**
> +	 * @skip_invalidation: Used in madvise to avoid invalidation
> +	 * if mem attributes doesn't change
> +	 */
> +	u32 skip_invalidation;

bool or I think you can safely stuff this into gpuva flags (e.g.,
DRM_GPUVA_USERBITS) as this only set / cleared / viewed by a single
thread and all other flags are set at init time or bind time (i.e., no
races). If you choose the latter, or regardless, we should probably
document the gpuva usage.

Matt

> +
>  	/**
>  	 * @ufence: The user fence that was provided with MAP.
>  	 * Needs to be signalled before UNMAP can be processed.
> -- 
> 2.34.1
>