Re: [RFC 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages

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

On Wed, Jun 03, 2026 at 02:56:18PM +0800, Honglei Huang wrote:
> From: Honglei Huang <honghuan@amd.com>
> 
> With drm_gpusvm_pages now self contained, make xe stop relying
> on the drm_gpusvm_range pages and take responsibility for the page
> lifecycle on the driver side.
> 
> Driver side (xe):
> 
>   - Embed struct drm_gpusvm_pages in xe_svm_range and route all
>     xe accesses through it instead of range->base.pages.
>   - Take over the page lifecycle: xe_svm_range_get_pages() calls
>     drm_gpusvm_get_pages() directly with &xe->drm; the notifier
>     event_end and xe_svm_range_free() paths drive unmap/free on
>     the embedded pages object.
>   - Switch xe_svm_range_pages_valid() to drm_gpusvm_pages_valid().
> 
> Framework side (drm_gpusvm):
> 
>   - Export drm_gpusvm_pages_valid() to let driver owned pages
>     can query mapping state without going through a range.
>   - Contract change: drm_gpusvm_range_remove() no longer unmaps or
>     frees pages; drivers that own a drm_gpusvm_pages instance must
>     do that themselves.
> 
> Side effect / contract: drivers that own a drm_gpusvm_pages
> are now responsible for its lifecycle, in particular for calling
> drm_gpusvm_unmap_pages() and drm_gpusvm_free_pages() at the
> appropriate points.
> 
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
>  drivers/gpu/drm/drm_gpusvm.c |  9 +++------
>  drivers/gpu/drm/xe/xe_pt.c   |  2 +-
>  drivers/gpu/drm/xe/xe_svm.c  | 22 +++++++++++++++-------
>  drivers/gpu/drm/xe/xe_svm.h  |  9 +++++++--
>  include/drm/drm_gpusvm.h     |  3 +++
>  5 files changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 3f076178b2a..a4b56cefeb2 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1231,8 +1231,6 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_free_pages);
>  void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm,
>  			     struct drm_gpusvm_range *range)
>  {
> -	unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
> -					       drm_gpusvm_range_end(range));
>  	struct drm_gpusvm_notifier *notifier;
>  
>  	drm_gpusvm_driver_lock_held(gpusvm);
> @@ -1244,8 +1242,6 @@ void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm,
>  		return;
>  
>  	drm_gpusvm_notifier_lock(gpusvm);
> -	__drm_gpusvm_unmap_pages(gpusvm, &range->pages, npages);
> -	__drm_gpusvm_free_pages(gpusvm, &range->pages);
>  	__drm_gpusvm_range_remove(notifier, range);
>  	drm_gpusvm_notifier_unlock(gpusvm);
>  
> @@ -1324,13 +1320,14 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_range_put);
>   *
>   * Return: True if GPU SVM range has valid pages, False otherwise
>   */
> -static bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
> -				   struct drm_gpusvm_pages *svm_pages)
> +bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
> +			    struct drm_gpusvm_pages *svm_pages)
>  {
>  	lockdep_assert_held(&gpusvm->notifier_lock);
>  
>  	return svm_pages->flags.has_devmem_pages || svm_pages->flags.has_dma_mapping;
>  }
> +EXPORT_SYMBOL_GPL(drm_gpusvm_pages_valid);
>  
>  /**
>   * drm_gpusvm_range_pages_valid() - GPU SVM range pages valid
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 2669ff5ee74..e82b0d8fab1 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -758,7 +758,7 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
>  			return -EAGAIN;
>  		}
>  		if (xe_svm_range_has_dma_mapping(range)) {
> -			xe_res_first_dma(range->base.pages.dma_addr, 0,
> +			xe_res_first_dma(range->pages.dma_addr, 0,
>  					 xe_svm_range_size(range),
>  					 &curs);
>  			xe_svm_range_debug(range, "BIND PREPARE - MIXED");
> diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> index 3acfddb7c5b..33c26df5111 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -66,7 +66,7 @@ static bool xe_svm_range_in_vram(struct xe_svm_range *range)
>  
>  	struct drm_gpusvm_pages_flags flags = {
>  		/* Pairs with WRITE_ONCE in drm_gpusvm.c */
> -		.__flags = READ_ONCE(range->base.pages.flags.__flags),
> +		.__flags = READ_ONCE(range->pages.flags.__flags),
>  	};
>  
>  	return flags.has_devmem_pages;
> @@ -96,7 +96,7 @@ static struct xe_vm *range_to_vm(struct drm_gpusvm_range *r)
>  	       (r__)->base.gpusvm,					\
>  	       xe_svm_range_in_vram((r__)) ? 1 : 0,			\
>  	       xe_svm_range_has_vram_binding((r__)) ? 1 : 0,		\
> -	       (r__)->base.pages.notifier_seq,				\
> +	       (r__)->pages.notifier_seq,				\
>  	       xe_svm_range_start((r__)), xe_svm_range_end((r__)),	\
>  	       xe_svm_range_size((r__)))
>  
> @@ -115,6 +115,7 @@ xe_svm_range_alloc(struct drm_gpusvm *gpusvm)
>  		return NULL;
>  
>  	INIT_LIST_HEAD(&range->garbage_collector_link);
> +	range->pages.notifier_seq = LONG_MAX;

As discussed in the cover-letter let's do a drm_gpusvm_init_pages()
function to set the notifier_seq.

If we want to include 'drm' in the init function as discussed in patch
#2, to fish this out in Xe you can do '&gpusvm_to_vm(gpusvm)->xe->drm'.

>  	xe_vm_get(gpusvm_to_vm(gpusvm));
>  
>  	return &range->base;
> @@ -122,8 +123,10 @@ xe_svm_range_alloc(struct drm_gpusvm *gpusvm)
>  
>  static void xe_svm_range_free(struct drm_gpusvm_range *range)
>  {
> +	drm_gpusvm_free_pages(range->gpusvm, &(to_xe_range(range)->pages),
> +			      drm_gpusvm_range_size(range) >> PAGE_SHIFT);
>  	xe_vm_put(range_to_vm(range));
> -	kfree(range);
> +	kfree(to_xe_range(range));
>  }
>  
>  static void
> @@ -208,7 +211,8 @@ xe_svm_range_notifier_event_end(struct xe_vm *vm, struct drm_gpusvm_range *r,
>  
>  	xe_svm_assert_in_notifier(vm);
>  
> -	drm_gpusvm_range_unmap_pages(&vm->svm.gpusvm, r, &ctx);
> +	drm_gpusvm_unmap_pages(&vm->svm.gpusvm, &(to_xe_range(r)->pages),
> +			       drm_gpusvm_range_size(r) >> PAGE_SHIFT, &ctx);
>  	if (!xe_vm_is_closed(vm) && mmu_range->event == MMU_NOTIFY_UNMAP)
>  		xe_svm_garbage_collector_add_range(vm, to_xe_range(r),
>  						   mmu_range);
> @@ -952,7 +956,7 @@ void xe_svm_fini(struct xe_vm *vm)
>  static bool xe_svm_range_has_pagemap_locked(const struct xe_svm_range *range,
>  					    const struct drm_pagemap *dpagemap)
>  {
> -	return range->base.pages.dpagemap == dpagemap;
> +	return range->pages.dpagemap == dpagemap;
>  }
>  
>  static bool xe_svm_range_has_pagemap(struct xe_svm_range *range,
> @@ -1017,7 +1021,7 @@ bool xe_svm_range_validate(struct xe_vm *vm,
>  	if (dpagemap)
>  		ret = ret && xe_svm_range_has_pagemap_locked(range, dpagemap);
>  	else
> -		ret = ret && !range->base.pages.dpagemap;
> +		ret = ret && !range->pages.dpagemap;
>  
>  	xe_svm_notifier_unlock(vm);
>  
> @@ -1510,7 +1514,11 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
>  	if (READ_ONCE(range->base.flags.unmapped))
>  		return -EFAULT;
>  
> -	err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx);
> +	err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages,
> +				   &vm->xe->drm, vm->svm.gpusvm.mm,
> +				   &range->base.notifier->notifier,
> +				   drm_gpusvm_range_start(&range->base),
> +				   drm_gpusvm_range_end(&range->base), ctx);
>  	if (err == -EOPNOTSUPP) {
>  		range_debug(range, "PAGE FAULT - EVICT PAGES");
>  		drm_gpusvm_range_evict(&vm->svm.gpusvm, &range->base);
> diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
> index b7b8eeacf19..ea73241d3d9 100644
> --- a/drivers/gpu/drm/xe/xe_svm.h
> +++ b/drivers/gpu/drm/xe/xe_svm.h
> @@ -31,6 +31,11 @@ struct xe_vram_region;
>  struct xe_svm_range {
>  	/** @base: base drm_gpusvm_range */
>  	struct drm_gpusvm_range base;
> +	/**
> +	 * @pages: Per-device DMA mapping state; single instance since
> +	 * xe svm is 1 svm : 1 drm_device.

s/xe/Xe

Matt

> +	 */
> +	struct drm_gpusvm_pages pages;
>  	/**
>  	 * @garbage_collector_link: Link into VM's garbage collect SVM range
>  	 * list. Protected by VM's garbage collect lock.
> @@ -74,7 +79,7 @@ struct xe_pagemap {
>   */
>  static inline bool xe_svm_range_pages_valid(struct xe_svm_range *range)
>  {
> -	return drm_gpusvm_range_pages_valid(range->base.gpusvm, &range->base);
> +	return drm_gpusvm_pages_valid(range->base.gpusvm, &range->pages);
>  }
>  
>  int xe_devm_add(struct xe_tile *tile, struct xe_vram_region *vr);
> @@ -132,7 +137,7 @@ void *xe_svm_private_page_owner(struct xe_vm *vm, bool force_smem);
>  static inline bool xe_svm_range_has_dma_mapping(struct xe_svm_range *range)
>  {
>  	lockdep_assert_held(&range->base.gpusvm->notifier_lock);
> -	return range->base.pages.flags.has_dma_mapping;
> +	return range->pages.flags.has_dma_mapping;
>  }
>  
>  /**
> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index ed228d9ff6b..21baf91ec7e 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -306,6 +306,9 @@ void drm_gpusvm_range_put(struct drm_gpusvm_range *range);
>  bool drm_gpusvm_range_pages_valid(struct drm_gpusvm *gpusvm,
>  				  struct drm_gpusvm_range *range);
>  
> +bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm,
> +			    struct drm_gpusvm_pages *svm_pages);
> +
>  int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm,
>  			       struct drm_gpusvm_range *range,
>  			       const struct drm_gpusvm_ctx *ctx);
> -- 
> 2.34.1
>