Re: [PATCH v4 08/20] drm/xe/svm: Add xe_svm_ranges_zap_ptes_in_range() for PTE zapping

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

On Fri, Jun 13, 2025 at 06:25:46PM +0530, Himal Prasad Ghimiray wrote:
> Introduce xe_svm_ranges_zap_ptes_in_range(), a function to zap page table
> entries (PTEs) for all SVM ranges within a user-specified address range.
> 
> -v2 (Matthew Brost)
> Lock should be called even for tlb_invalidation
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pt.c  | 14 ++++++++++++-
>  drivers/gpu/drm/xe/xe_svm.c | 42 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_svm.h |  7 +++++++
>  3 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 9177c571689e..9a390ef10852 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -950,7 +950,19 @@ bool xe_pt_zap_ptes_range(struct xe_tile *tile, struct xe_vm *vm,
>  	struct xe_pt *pt = vm->pt_root[tile->id];
>  	u8 pt_mask = (range->tile_present & ~range->tile_invalidated);
>  
> -	xe_svm_assert_in_notifier(vm);
> +	/*
> +	 * Locking rules:
> +	 *
> +	 * - notifier_lock (write): full protection against page table changes
> +	 *   and MMU notifier invalidations.
> +	 *
> +	 * - notifier_lock (read) + vm_lock (write): combined protection against
> +	 *   invalidations and concurrent page table modifications. (e.g., madvise)
> +	 *
> +	 */
> +	lockdep_assert(lockdep_is_held_type(&vm->svm.gpusvm.notifier_lock, 0) ||
> +		       (lockdep_is_held_type(&vm->svm.gpusvm.notifier_lock, 1) &&
> +		       lockdep_is_held_type(&vm->lock, 0)));
>  
>  	if (!(pt_mask & BIT(tile->id)))
>  		return false;
> diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> index 2fbbd6a604ea..19420635f1fa 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -999,6 +999,48 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
>  	return err;
>  }
>  
> +/**
> + * xe_svm_ranges_zap_ptes_in_range - clear ptes of svm ranges in input range
> + * @vm: Pointer to the xe_vm structure
> + * @start: Start of the input range
> + * @end: End of the input range
> + *
> + * This function removes the page table entries (PTEs) associated
> + * with the svm ranges within the given input start and end
> + *
> + * Return: tile_mask for which gt's need to be tlb invalidated.
> + */
> +u8 xe_svm_ranges_zap_ptes_in_range(struct xe_vm *vm, u64 start, u64 end)
> +{
> +	struct drm_gpusvm_notifier *notifier;
> +	struct xe_svm_range *range;
> +	u64 adj_start, adj_end;
> +	struct xe_tile *tile;
> +	u8 tile_mask = 0;
> +	u8 id;
> +
> +	lockdep_assert(lockdep_is_held_type(&vm->svm.gpusvm.notifier_lock, 1) &&
> +		       lockdep_is_held_type(&vm->lock, 0));
> +
> +	drm_gpusvm_for_each_notifier(notifier, &vm->svm.gpusvm, start, end) {
> +		struct drm_gpusvm_range *r = NULL;
> +
> +		adj_start = max(start, notifier->itree.start);

s/notifier->itree.start/drm_gpusvm_notifier_start

> +		adj_end = min(end, notifier->itree.last + 1);

s/notifier->itree.last + 1/drm_gpusvm_notifier_end

> +		drm_gpusvm_for_each_range(r, notifier, adj_start, adj_end) {
> +			range = to_xe_range(r);
> +			for_each_tile(tile, vm->xe, id) {
> +				if (xe_pt_zap_ptes_range(tile, vm, range)) {
> +					tile_mask |= BIT(id);
> +					range->tile_invalidated |= BIT(id);

/* WRITE_ONCE pairs with READ_ONCE in xe_vm_has_valid_gpu_mapping() */
WRITE_ONCE(range->tile_invalidated, range->tile_invalidated | BIT(id));

Also, we need to be careful here. If we can fail after this point but
before the TLB invalidation completes, we could break the notifier, as
the notifier would skip the TLB invalidation. The code, as written, can
only fail if the CT channel is down — in that case, all bets are off and
we are issuing a GT reset. So, I think the code as written is okay, but
I’d add a comment here indicating that there must be no failure points
between setting tile_invalidated and issuing the TLB invalidation.

Matt

> +				}
> +			}
> +		}
> +	}
> +
> +	return tile_mask;
> +}
> +
>  #if IS_ENABLED(CONFIG_DRM_XE_DEVMEM_MIRROR)
>  
>  static struct drm_pagemap_device_addr
> diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
> index 19ce4f2754a7..af8f285b6caa 100644
> --- a/drivers/gpu/drm/xe/xe_svm.h
> +++ b/drivers/gpu/drm/xe/xe_svm.h
> @@ -91,6 +91,7 @@ bool xe_svm_range_validate(struct xe_vm *vm,
>  
>  u64 xe_svm_find_vma_start(struct xe_vm *vm, u64 addr, u64 end,  struct xe_vma *vma);
>  
> +u8 xe_svm_ranges_zap_ptes_in_range(struct xe_vm *vm, u64 start, u64 end);
>  /**
>   * xe_svm_range_has_dma_mapping() - SVM range has DMA mapping
>   * @range: SVM range
> @@ -305,6 +306,12 @@ u64 xe_svm_find_vma_start(struct xe_vm *vm, u64 addr, u64 end, struct xe_vma *vm
>  	return ULONG_MAX;
>  }
>  
> +static inline
> +u8 xe_svm_ranges_zap_ptes_in_range(struct xe_vm *vm, u64 start, u64 end)
> +{
> +	return 0;
> +}
> +
>  #define xe_svm_assert_in_notifier(...) do {} while (0)
>  #define xe_svm_range_has_dma_mapping(...) false
>  
> -- 
> 2.34.1
>