Re: [PATCH 1/3] drm/xe/xe3p_lpg: flush userptr/shrinker bo cachelines manually

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi.


On Tue, 2025-11-25 at 15:13 +0530, Tejas Upadhyay wrote:
> Starting NVL, HW will flush cachelines marked with XA only
> when media is off. We have few cases where kernel will have
> non-XA cachelines which needs manual flush as we postpone
> the invalidation. Flush asap from correctness POV to ensure
> non accelerated CPU copy to swap/shmem file will see coherent
> view of memory, but also from security POV where later flush
> can't corrupt the next user of those pages.
> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>

I had a number of concerns last time this patch was sent to the list,
none of which seems to have been addressed? 

https://lore.kernel.org/intel-xe/d2517d66f571e11a760cb143981b7ca238f5cd58.camel@linux.intel.com/

The main concern is that the code indicates that not all GPU caches are
flushed when all fences are signalled (bo / userptr idle)?

Thanks,
Thomas



> ---
>  drivers/gpu/drm/xe/xe_bo.c      |  3 ++-
>  drivers/gpu/drm/xe/xe_device.c  | 20 ++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_device.h  |  1 +
>  drivers/gpu/drm/xe/xe_userptr.c |  3 ++-
>  4 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 465cf9fc7ce9..97e1e9d40e96 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -689,7 +689,8 @@ static int xe_bo_trigger_rebind(struct xe_device
> *xe, struct xe_bo *bo,
>  
>  		if (!xe_vm_in_fault_mode(vm)) {
>  			drm_gpuvm_bo_evict(vm_bo, true);
> -			continue;
> +			if (!xe_device_needs_cache_flush(xe))
> +				continue;
>  		}
>  
>  		if (!idle) {
> diff --git a/drivers/gpu/drm/xe/xe_device.c
> b/drivers/gpu/drm/xe/xe_device.c
> index 92f883dd8877..6e8335b493e8 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -1079,6 +1079,26 @@ void xe_device_l2_flush(struct xe_device *xe)
>  	spin_unlock(&gt->global_invl_lock);
>  }
>  
> +/**
> + * xe_device_needs_cache_flush - Whether the cache needs to be
> flushed
> + * @xe: The device to check.
> + *
> + * Return: true if the device needs cache flush, false otherwise.
> + */
> +bool xe_device_needs_cache_flush(struct xe_device *xe)
> +{
> +	/*
> +	 * Starting NVL, HW will flush cachelines marked with XA
> only when media is off. We have
> +	 * few cases where kernel will have non-XA cachelines which
> needs manual flush and this is
> +	 * one of them as we postpone the invalidation. Flush asap
> from correctness POV to ensure
> +	 * non accelerated CPU copy to swap/shmem file will see
> coherent view of memory, but also
> +	 * from security POV where later flush can't corrupt the
> next user of those pages.
> +	 */
> +	if (GRAPHICS_VER(xe) >= 35 && !IS_DGFX(xe))
> +		return true;
> +	return false;
> +}
> +
>  /**
>   * xe_device_td_flush() - Flush transient L3 cache entries
>   * @xe: The device
> diff --git a/drivers/gpu/drm/xe/xe_device.h
> b/drivers/gpu/drm/xe/xe_device.h
> index 32cc6323b7f6..15e67db44b56 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -179,6 +179,7 @@ void xe_device_snapshot_print(struct xe_device
> *xe, struct drm_printer *p);
>  u64 xe_device_canonicalize_addr(struct xe_device *xe, u64 address);
>  u64 xe_device_uncanonicalize_addr(struct xe_device *xe, u64
> address);
>  
> +bool xe_device_needs_cache_flush(struct xe_device *xe);
>  void xe_device_td_flush(struct xe_device *xe);
>  void xe_device_l2_flush(struct xe_device *xe);
>  
> diff --git a/drivers/gpu/drm/xe/xe_userptr.c
> b/drivers/gpu/drm/xe/xe_userptr.c
> index 0d9130b1958a..a93c7e887cca 100644
> --- a/drivers/gpu/drm/xe/xe_userptr.c
> +++ b/drivers/gpu/drm/xe/xe_userptr.c
> @@ -114,7 +114,8 @@ static void __vma_userptr_invalidate(struct xe_vm
> *vm, struct xe_userptr_vma *uv
>  				    false, MAX_SCHEDULE_TIMEOUT);
>  	XE_WARN_ON(err <= 0);
>  
> -	if (xe_vm_in_fault_mode(vm) && userptr->initial_bind) {
> +	if ((xe_vm_in_fault_mode(vm) ||
> xe_device_needs_cache_flush(vm->xe)) &&
> +	    userptr->initial_bind) {
>  		err = xe_vm_invalidate_vma(vma);
>  		XE_WARN_ON(err);
>  	}