Re: [PATCH 2/3] drm/xe/pf: Invalidate LMTT during LMEM unprovisioning

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

On Thu, Jul 03, 2025 at 07:42:25PM +0200, Michal Wajdeczko wrote:
> 
> 
> On 03.07.2025 19:31, Matthew Brost wrote:
> > On Thu, Jul 03, 2025 at 12:30:40AM +0200, Michal Wajdeczko wrote:
> >> Invalidate LMTT immediately after removing VF's LMTT page tables
> >> and clearing root PTE in the LMTT PD to avoid any invalid access
> >> by the hardware (and VF) due to stale data.
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Michał Winiarski <michal.winiarski@intel.com>
> >> Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_device.h              |  4 ++
> >>  drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 35 +++++++++++++
> >>  drivers/gpu/drm/xe/xe_gt_tlb_invalidation.h |  2 +
> >>  drivers/gpu/drm/xe/xe_lmtt.c                | 56 +++++++++++++++++++++
> >>  drivers/gpu/drm/xe/xe_lmtt.h                |  1 +
> >>  5 files changed, 98 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> >> index e4da797a984b..a7acd899aa76 100644
> >> --- a/drivers/gpu/drm/xe/xe_device.h
> >> +++ b/drivers/gpu/drm/xe/xe_device.h
> >> @@ -130,6 +130,10 @@ static inline bool xe_device_uc_enabled(struct xe_device *xe)
> >>  	for ((id__) = 1; (id__) < (xe__)->info.tile_count; (id__)++) \
> >>  		for_each_if((tile__) = &(xe__)->tiles[(id__)])
> >>  
> >> +#define for_each_gt_on_tile(gt__, tile__, id__) \
> >> +	for ((id__) = 0; (id__) < XE_MAX_GT_PER_TILE; (id__)++) \
> >> +		for_each_if((gt__) = xe_tile_get_gt((tile__), (id__)))
> >> +
> >>  /*
> >>   * FIXME: This only works for now since multi-tile and standalone media
> >>   * happen to be mutually exclusive.  Future platforms may change this...
> >> diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> >> index 6088df8e159c..4fdd5b300265 100644
> >> --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> >> +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> >> @@ -330,6 +330,41 @@ int xe_gt_tlb_invalidation_ggtt(struct xe_gt *gt)
> >>  	return 0;
> >>  }
> >>  
> >> +static int send_tlb_invalidation_all(struct xe_gt *gt,
> >> +				     struct xe_gt_tlb_invalidation_fence *fence)
> >> +{
> >> +	u32 action[] = {
> >> +		XE_GUC_ACTION_TLB_INVALIDATION_ALL,
> >> +		0,  /* seqno, replaced in send_tlb_invalidation */
> >> +		MAKE_INVAL_OP(XE_GUC_TLB_INVAL_FULL),
> >> +	};
> >> +
> >> +	return send_tlb_invalidation(&gt->uc.guc, fence, action, ARRAY_SIZE(action));
> >> +}
> >> +
> >> +/**
> >> + * xe_gt_tlb_invalidation_all_async - Invalidate all TLBs across PF and all VFs.
> >> + * @gt: the &xe_gt structure
> >> + * @fence: the &xe_gt_tlb_invalidation_fence to be signaled on completion
> >> + *
> >> + * Send a request to invalidate all TLBs across PF and all VFs.
> >> + *
> >> + * Return: 0 on success, negative error code on error
> >> + */
> >> +int xe_gt_tlb_invalidation_all_async(struct xe_gt *gt,
> >> +				     struct xe_gt_tlb_invalidation_fence *fence)
> > 
> > I'd drop _async part of the naming as I think it is implied with the
> > fence argument that is this async, like xe_gt_tlb_invalidation_range.
> 
> this suffix is here because initially I was also having this variant:
> 
> 	int xe_gt_tlb_invalidation_all(struct xe_gt *gt);
> 
> which actually was waiting for the TLB fence inside - like it is done in
> the xe_gt_tlb_invalidation_ggtt() - but finally dropped it from this
> patch as not used in current code
> 

Ok, makes sense. If you need xe_gt_tlb_invalidation_all to be sync, I'd
suggest passing in a NULL fence, then have xe_gt_tlb_invalidation_all
initialize a fence on the stack + wait on it.

> > 
> > All the changes around GT TLB invalidations look correct (for now, this
> > whole layer needs rework).
> 
> yes, please ;)

I took pass at it while back, I thought other were going to pick this
up though. Will check on this.

Matt 

> 
> > 
> > Matt 
> >