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

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

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;

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
>