Re: [PATCH 2/2] drm/xe/uapi: Hide the madvise autoreset behind a VM_BIND flag

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 14-10-2025 13:57, Thomas Hellström wrote:
> The madvise implementation currently resets the SVM madvise if the
> underlying CPU map is unmapped. This is in an attempt to mimic the
> CPU madvice behaviour. However, it's not clear that this is a desired
> behaviour since if the end app user relies on it for malloc()ed
> objects or stack objects, it may not work as intended.
> 
> Instead of having the autoreset functionality being a direct
> application-facing implicit UAPI, make the UMD explicitly choose
> this behaviour if it wants to expose it by introducing
> DRM_XE_VM_BIND_FLAG_MADVISE_AUTORESET, and add a semantics
> description.
> 
> Fixes: a2eb8aec3ebe ("drm/xe: Reset VMA attributes to default in SVM garbage collector")
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: "Falkowski, John" <john.falkowski@intel.com>
> Cc: "Mrozek, Michal" <michal.mrozek@intel.com>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

LGTM
Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com

> ---
>   drivers/gpu/drm/xe/xe_svm.c      |  5 +++++
>   drivers/gpu/drm/xe/xe_vm.c       | 12 +++++++++---
>   drivers/gpu/drm/xe/xe_vm_types.h |  1 +
>   include/uapi/drm/xe_drm.h        | 15 +++++++++++++++
>   4 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> index b268ee0d2271..f8773cbc2025 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -302,6 +302,11 @@ static int xe_svm_range_set_default_attr(struct xe_vm *vm, u64 range_start, u64
>   	if (!vma)
>   		return -EINVAL;
>   
> +	if (!(vma->gpuva.flags & XE_VMA_MADV_AUTORESET)) {
> +		drm_dbg(&vm->xe->drm, "Skipping madvise reset for vma.\n");
> +		return 0;
> +	}
> +
>   	if (xe_vma_has_default_mem_attrs(vma))
>   		return 0;
>   
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 9c29608aa3bc..d8a9d8090b37 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -646,7 +646,8 @@ static void xe_vma_ops_incr_pt_update_ops(struct xe_vma_ops *vops, u8 tile_mask,
>   	XE_VMA_READ_ONLY |		    \
>   	XE_VMA_DUMPABLE |		    \
>   	XE_VMA_SYSTEM_ALLOCATOR |           \
> -	DRM_GPUVA_SPARSE)
> +	DRM_GPUVA_SPARSE |		    \
> +	XE_VMA_MADV_AUTORESET)
>   
>   static void xe_vm_populate_rebind(struct xe_vma_op *op, struct xe_vma *vma,
>   				  u8 tile_mask)
> @@ -2296,6 +2297,8 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops,
>   				op->map.vma_flags |= XE_VMA_SYSTEM_ALLOCATOR;
>   			if (flags & DRM_XE_VM_BIND_FLAG_DUMPABLE)
>   				op->map.vma_flags |= XE_VMA_DUMPABLE;
> +			if (flags & DRM_XE_VM_BIND_FLAG_MADVISE_AUTORESET)
> +				op->map.vma_flags |= XE_VMA_MADV_AUTORESET;
>   			op->map.pat_index = pat_index;
>   			op->map.invalidate_on_bind =
>   				__xe_vm_needs_clear_scratch_pages(vm, flags);
> @@ -3279,7 +3282,8 @@ ALLOW_ERROR_INJECTION(vm_bind_ioctl_ops_execute, ERRNO);
>   	 DRM_XE_VM_BIND_FLAG_NULL | \
>   	 DRM_XE_VM_BIND_FLAG_DUMPABLE | \
>   	 DRM_XE_VM_BIND_FLAG_CHECK_PXP | \
> -	 DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR)
> +	 DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR | \
> +	 DRM_XE_VM_BIND_FLAG_MADVISE_AUTORESET)
>   
>   #ifdef TEST_VM_OPS_ERROR
>   #define SUPPORTED_FLAGS	(SUPPORTED_FLAGS_STUB | FORCE_OP_ERROR)
> @@ -3394,7 +3398,9 @@ static int vm_bind_ioctl_check_args(struct xe_device *xe, struct xe_vm *vm,
>   		    XE_IOCTL_DBG(xe,  (prefetch_region != DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC &&
>   				       !(BIT(prefetch_region) & xe->info.mem_region_mask))) ||
>   		    XE_IOCTL_DBG(xe, obj &&
> -				 op == DRM_XE_VM_BIND_OP_UNMAP)) {
> +				 op == DRM_XE_VM_BIND_OP_UNMAP) ||
> +		    XE_IOCTL_DBG(xe, (flags & DRM_XE_VM_BIND_FLAG_MADVISE_AUTORESET) &&
> +				 (!is_cpu_addr_mirror || op != DRM_XE_VM_BIND_OP_MAP))) {
>   			err = -EINVAL;
>   			goto free_bind_ops;
>   		}
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index a3b422b27ae8..d6e2a0fdd4b3 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -46,6 +46,7 @@ struct xe_vm_pgtable_update_op;
>   #define XE_VMA_PTE_COMPACT	(DRM_GPUVA_USERBITS << 7)
>   #define XE_VMA_DUMPABLE		(DRM_GPUVA_USERBITS << 8)
>   #define XE_VMA_SYSTEM_ALLOCATOR	(DRM_GPUVA_USERBITS << 9)
> +#define XE_VMA_MADV_AUTORESET	(DRM_GPUVA_USERBITS << 10)
>   
>   /**
>    * struct xe_vma_mem_attr - memory attributes associated with vma
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 40ff19f52a8d..98bc8858e4bd 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -1013,6 +1013,20 @@ struct drm_xe_vm_destroy {
>    *    valid on VMs with DRM_XE_VM_CREATE_FLAG_FAULT_MODE set. The CPU address
>    *    mirror flag are only valid for DRM_XE_VM_BIND_OP_MAP operations, the BO
>    *    handle MBZ, and the BO offset MBZ.
> + *  - %DRM_XE_VM_BIND_FLAG_MADVISE_AUTORESET - Can be used in combination with
> + *    %DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR to reset madvises when the underlying
> + *    CPU address space range is unmapped (typically with munmap(2) or brk(2)).
> + *    The madvise values set with %DRM_IOCTL_XE_MADVISE are reset to the values
> + *    that were present immediately after the %DRM_IOCTL_XE_VM_BIND.
> + *    The reset GPU virtual address range is the intersection of the range bound
> + *    using %DRM_IOCTL_XE_VM_BIND and the virtual CPU address space range
> + *    unmapped.
> + *    This functionality is present to mimic the behaviour of CPU address space
> + *    madvises set using madvise(2), which are typically reset on unmap.
> + *    Note: free(3) may or may not call munmap(2) and/or brk(2), and may thus
> + *    not invoke autoreset. Neither will stack variables going out of scope.
> + *    Therefore it's recommended to always explicitly reset the madvises when
> + *    freeing the memory backing a region used in a %DRM_IOCTL_XE_MADVISE call.
>    *
>    * The @prefetch_mem_region_instance for %DRM_XE_VM_BIND_OP_PREFETCH can also be:
>    *  - %DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC, which ensures prefetching occurs in
> @@ -1119,6 +1133,7 @@ struct drm_xe_vm_bind_op {
>   #define DRM_XE_VM_BIND_FLAG_DUMPABLE	(1 << 3)
>   #define DRM_XE_VM_BIND_FLAG_CHECK_PXP	(1 << 4)
>   #define DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR	(1 << 5)
> +#define DRM_XE_VM_BIND_FLAG_MADVISE_AUTORESET	(1 << 6)
>   	/** @flags: Bind flags */
>   	__u32 flags;
>