Re: [PATCH v4 4/5] drm/xe/svm: Enable UNMAP for VMA merging operations

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

On Mon, Nov 03, 2025 at 11:39:56AM +0530, Himal Prasad Ghimiray wrote:
> ALLOW UNMAP of VMAs associated with SVM mappings when the MAP operation
> is intended to merge adjacent CPU_ADDR_MIRROR VMAs.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vm.c       | 10 +++++++---
>  drivers/gpu/drm/xe/xe_vm_types.h |  3 ++-
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index d78eb95a954f..12354e03c3e2 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2244,8 +2244,10 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops,
>  
>  	switch (operation) {
>  	case DRM_XE_VM_BIND_OP_MAP:
> -		if (flags & DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR)
> +		if (flags & DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR) {
>  			xe_vm_find_cpu_addr_mirror_vma_range(vm, &range_start, &range_end);
> +			vops->flags |= XE_VMA_OPS_FLAG_ALLOW_SVM_UNMAP;
> +		}
>  
>  		fallthrough;
>  	case DRM_XE_VM_BIND_OP_MAP_USERPTR: {
> @@ -2727,7 +2729,8 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct drm_gpuva_ops *ops,
>  
>  			if (xe_vma_is_cpu_addr_mirror(vma) &&
>  			    xe_svm_has_mapping(vm, xe_vma_start(vma),
> -					       xe_vma_end(vma)))
> +					       xe_vma_end(vma)) &&
> +			    !(vops->flags & XE_VMA_OPS_FLAG_ALLOW_SVM_UNMAP))

Do we need similar change to the DRM_GPUVA_OP_REMAP case statement which
has similar logic?

>  				return -EBUSY;
>  
>  			if (!xe_vma_is_cpu_addr_mirror(vma))
> @@ -4295,6 +4298,8 @@ static int xe_vm_alloc_vma(struct xe_vm *vm,
>  
>  	if (is_madvise)
>  		vops.flags |= XE_VMA_OPS_FLAG_MADVISE;
> +	else
> +		vops.flags |= XE_VMA_OPS_FLAG_ALLOW_SVM_UNMAP;
>  
>  	err = vm_bind_ioctl_ops_parse(vm, ops, &vops);
>  	if (err)
> @@ -4371,7 +4376,6 @@ int xe_vm_alloc_madvise_vma(struct xe_vm *vm, uint64_t start, uint64_t range)
>  static bool is_cpu_addr_vma_with_default_attr(struct xe_vma *vma)
>  {
>  	return vma && xe_vma_is_cpu_addr_mirror(vma) &&
> -	       !xe_svm_has_mapping(xe_vma_vm(vma), xe_vma_start(vma), xe_vma_end(vma)) &&

Ah, ok this answers my question in patch #1 [1]. But then in this patch
should we also remove the similar check in patch #2 [2] in which I asked
the same question?

Matt

[1] https://patchwork.freedesktop.org/patch/685118/?series=154832&rev=4#comment_1267065
[2] https://patchwork.freedesktop.org/patch/685119/?series=154832&rev=4

>  	       xe_vma_has_default_mem_attrs(vma);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index 830ed7b05c27..7578ccada2cf 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -470,7 +470,8 @@ struct xe_vma_ops {
>  	/** @flag: signify the properties within xe_vma_ops*/
>  #define XE_VMA_OPS_FLAG_HAS_SVM_PREFETCH BIT(0)
>  #define XE_VMA_OPS_FLAG_MADVISE          BIT(1)
> -#define XE_VMA_OPS_ARRAY_OF_BINDS	 BIT(2)
> +#define XE_VMA_OPS_FLAG_ALLOW_SVM_UNMAP  BIT(2)
> +#define XE_VMA_OPS_ARRAY_OF_BINDS        BIT(3)
>  	u32 flags;
>  #ifdef TEST_VM_OPS_ERROR
>  	/** @inject_error: inject error to test error handling */
> -- 
> 2.34.1
>