Re: [PATCH 2/3] drm/xe/tlb_inval: add xe_tlb_inval_ggtt_full() GGTT invalidation helper

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

On Wed, Aug 26, 2026 at 12:48:02PM -0700, Matthew Brost wrote:
> On Wed, Aug 26, 2026 at 12:30:56PM -0700, Niranjana Vishwanathapura wrote:
> > On Wed, Aug 26, 2026 at 04:55:32PM +0530, tilak.tirumalesh.tangudu@intel.com wrote:
> > > From: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>
> > > 
> > > Add xe_tlb_inval_ggtt_full() and its GuC backend
> > > send_tlb_inval_ggtt_full() for full engine TLB invalidation
> > > within the requesting VF across all engines.
> > > 
> > > v2:
> > > - Refactor with new helper xe_tlb_inval_issue_op_wait() and address
> > >  nits (Niranjana)
> > > v3:
> > > - Avoid TOCTOU issue around CT state (Matt Brost)
> > > v4:
> > > - Propagate real send errors instead of masking as -ECANCELED (Sashiko)
> > > 
> > > Assisted-by: Claude:claude-opus-4-8
> > > Signed-off-by: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>
> > > ---
> > > drivers/gpu/drm/xe/xe_guc_tlb_inval.c   | 25 +++++++++++++++
> > > drivers/gpu/drm/xe/xe_tlb_inval.c       | 42 +++++++++++++++++++++----
> > > drivers/gpu/drm/xe/xe_tlb_inval.h       |  1 +
> > > drivers/gpu/drm/xe/xe_tlb_inval_types.h | 10 ++++++
> > > 4 files changed, 72 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > > index 046d0655122f..665483bf2e67 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > > @@ -98,6 +98,29 @@ static int send_tlb_inval_ggtt(struct xe_tlb_inval *tlb_inval, u32 seqno)
> > > 	return -ECANCELED;
> > > }
> > > 
> > > +/*
> > > + * Emit INVAL_FULL (intra vf) to invalidate engine TLBs across all engines
> > > + * within the VF.
> > > + */
> > > +static int send_tlb_inval_ggtt_full(struct xe_tlb_inval *tlb_inval, u32 seqno)
> > > +{
> > > +	struct xe_guc *guc = tlb_inval->private;
> > > +	u32 action[] = {
> > > +		XE_GUC_ACTION_TLB_INVALIDATION,
> > > +		seqno,
> > > +		MAKE_INVAL_OP(XE_GUC_TLB_INVAL_FULL),
> > > +	};
> > > +	int ret;
> > > +
> > > +	ret = send_tlb_inval(guc, action, ARRAY_SIZE(action));
> > > +
> > > +	/* CT unavailable means nothing runs on the engines; treat as cancelled. */
> > > +	if (ret == -ENODEV || ret == -ECANCELED)
> > > +		return -ECANCELED;
> > 
> > Looks like we don't need the check 'ret == -ECANCELED' here (redundent).
> > Basically, we only need,
> > if (ret == -ENODEV)
> >      ret = -ECANCELED;
> 
> I think actually -ENODEV is needed because it is odd corner that only
> GGTT invalidations can legitimently hit with a blind CT send which was
> my suggestion.
> 
> -ENODEV is the return if the CT is disabled (cold boot start)
> -ECANCELED is return if the CT is stopped (runtime toggle)
> 
> GGTT invalidations can be issued during cold boots as these are kernel
> objects, PPGTT cannot because these are user objects.
> 
> Thus, I think this actually correct in the current code.
> 

Opps one more thing - iirc wedge devices return a different error code,
but that might be getting revert. We should check on that as I think
wedged needs to be handle here.

Matt

> Matt
> 
> > 
> > Also, It seems bit odd that other functions like send_tlb_inval_all(),
> > send_tlb_inval_lmmtt() etc, has similar structure and similar documentation in
> > xe_tlb_inval_types.h (regarding -ECANCELED), but doesn't do this conversion.
> > So, they are not treating XE_GUC_CT_STATE_DISABLED (where guc_ct_send returns
> > ENODEV) as a 'cancelled' state. May be here also we should do that for now,
> > if we need fix, then it will be a fix for all of these callback functions later.
> > 
> > Niranjana
> > 
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > static int send_page_reclaim(struct xe_guc *guc, u32 seqno,
> > > 			     u64 gpu_addr)
> > > {
> > > @@ -346,6 +369,7 @@ static long tlb_inval_timeout_delay(struct xe_tlb_inval *tlb_inval)
> > > static const struct xe_tlb_inval_ops guc_tlb_inval_asid_ops = {
> > > 	.all = send_tlb_inval_all,
> > > 	.ggtt = send_tlb_inval_ggtt,
> > > +	.ggtt_full = send_tlb_inval_ggtt_full,
> > > 	.ppgtt = send_tlb_inval_asid_ppgtt,
> > > 	.initialized = tlb_inval_initialized,
> > > 	.flush = tlb_inval_flush,
> > > @@ -354,6 +378,7 @@ static const struct xe_tlb_inval_ops guc_tlb_inval_asid_ops = {
> > > 
> > > static const struct xe_tlb_inval_ops guc_tlb_inval_ctx_ops = {
> > > 	.ggtt = send_tlb_inval_ggtt,
> > > +	.ggtt_full = send_tlb_inval_ggtt_full,
> > > 	.all = send_tlb_inval_all,
> > > 	.ppgtt = send_tlb_inval_ctx_ppgtt,
> > > 	.initialized = tlb_inval_initialized,
> > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > index bbd21d393062..7c3bf32d9258 100644
> > > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > @@ -301,26 +301,56 @@ int xe_tlb_inval_all(struct xe_tlb_inval *tlb_inval,
> > > }
> > > 
> > > /**
> > > - * xe_tlb_inval_ggtt() - Issue a TLB invalidation for the GGTT
> > > + * xe_tlb_inval_issue_op_wait() - Issue a TLB invalidation and wait
> > >  * @tlb_inval: TLB invalidation client
> > > - *
> > > - * Issue a TLB invalidation for the GGTT. Completion of TLB is asynchronous and
> > > - * caller can use the invalidation fence to wait for completion.
> > > + * @op: backend invalidation hook to issue
> > >  *
> > >  * Return: 0 on success, negative error code on error
> > >  */
> > > -int xe_tlb_inval_ggtt(struct xe_tlb_inval *tlb_inval)
> > > +static int xe_tlb_inval_issue_op_wait(struct xe_tlb_inval *tlb_inval,
> > > +				      int (*op)(struct xe_tlb_inval *tlb_inval, u32 seqno))
> > > {
> > > 	struct xe_tlb_inval_fence fence, *fence_ptr = &fence;
> > > 	int ret;
> > > 
> > > 	xe_tlb_inval_fence_init(tlb_inval, fence_ptr, true);
> > > -	ret = xe_tlb_inval_issue(tlb_inval, fence_ptr, tlb_inval->ops->ggtt);
> > > +	ret = xe_tlb_inval_issue(tlb_inval, fence_ptr, op);
> > > 	xe_tlb_inval_fence_wait(fence_ptr);
> > > 
> > > 	return ret;
> > > }
> > > 
> > > +/**
> > > + * xe_tlb_inval_ggtt() - Issue a TLB invalidation for the GGTT
> > > + * @tlb_inval: TLB invalidation client
> > > + *
> > > + * Issue a TLB invalidation for the GGTT. Completion of TLB is asynchronous and
> > > + * caller can use the invalidation fence to wait for completion.
> > > + *
> > > + * Return: 0 on success, negative error code on error
> > > + */
> > > +int xe_tlb_inval_ggtt(struct xe_tlb_inval *tlb_inval)
> > > +{
> > > +	return xe_tlb_inval_issue_op_wait(tlb_inval, tlb_inval->ops->ggtt);
> > > +}
> > > +
> > > +/**
> > > + * xe_tlb_inval_ggtt_full() - Full engine TLB invalidation within a VF
> > > + * @tlb_inval: TLB invalidation client
> > > + *
> > > + * Issue INVAL_FULL (intra vf) to flush engine TLBs across all engines
> > > + * within the requesting VF.
> > > + *
> > > + * Return: 0 on success, negative error code on error
> > > + */
> > > +int xe_tlb_inval_ggtt_full(struct xe_tlb_inval *tlb_inval)
> > > +{
> > > +	if (!tlb_inval->ops->ggtt_full)
> > > +		return -EOPNOTSUPP;
> > > +
> > > +	return xe_tlb_inval_issue_op_wait(tlb_inval, tlb_inval->ops->ggtt_full);
> > > +}
> > > +
> > > /**
> > >  * xe_tlb_inval_range() - Issue a TLB invalidation for an address range
> > >  * @tlb_inval: TLB invalidation client
> > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.h b/drivers/gpu/drm/xe/xe_tlb_inval.h
> > > index a76b7823a5f2..665072b4b81d 100644
> > > --- a/drivers/gpu/drm/xe/xe_tlb_inval.h
> > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.h
> > > @@ -20,6 +20,7 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval);
> > > int xe_tlb_inval_all(struct xe_tlb_inval *tlb_inval,
> > > 		     struct xe_tlb_inval_fence *fence);
> > > int xe_tlb_inval_ggtt(struct xe_tlb_inval *tlb_inval);
> > > +int xe_tlb_inval_ggtt_full(struct xe_tlb_inval *tlb_inval);
> > > void xe_tlb_inval_vm(struct xe_tlb_inval *tlb_inval, struct xe_vm *vm);
> > > int xe_tlb_inval_range(struct xe_tlb_inval *tlb_inval,
> > > 		       struct xe_tlb_inval_fence *fence,
> > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_types.h b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > index 3d1797d186fd..d77be1aedc91 100644
> > > --- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > @@ -36,6 +36,16 @@ struct xe_tlb_inval_ops {
> > > 	 */
> > > 	int (*ggtt)(struct xe_tlb_inval *tlb_inval, u32 seqno);
> > > 
> > > +	/**
> > > +	 * @ggtt_full: Full engine TLB invalidation within a VF
> > > +	 * @tlb_inval: TLB invalidation client
> > > +	 * @seqno: Seqno of TLB invalidation
> > > +	 *
> > > +	 * Return 0 on success, -ECANCELED if backend is mid-reset, error on
> > > +	 * failure
> > > +	 */
> > > +	int (*ggtt_full)(struct xe_tlb_inval *tlb_inval, u32 seqno);
> > > +
> > > 	/**
> > > 	 * @ppgtt: Invalidate per-process translation TLBs
> > > 	 * @tlb_inval: TLB invalidation client
> > > -- 
> > > 2.46.0
> > >