[PATCH 6/8] drm/xe: Prep TLB invalidation fence before sending
Summers, Stuart
stuart.summers at intel.com
Thu Aug 7 20:22:43 UTC 2025
On Thu, 2025-08-07 at 13:17 -0700, Matthew Brost wrote:
> On Thu, Aug 07, 2025 at 07:36:47PM +0000, stuartsummers wrote:
> > From: Matthew Brost <matthew.brost at intel.com>
> >
> > It is a bit backwards to add a TLB invalidation fence to the
> > pending
> > list after issuing the invalidation. Perform this step before
> > issuing
> > the TLB invalidation in a helper function.
> >
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > Reviewed-by: Stuart Summers <stuart.summers at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_tlb_inval.c | 105 +++++++++++++++-----------
> > ----
> > 1 file changed, 52 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > index 995699108bcb..967e5a261bb6 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > @@ -65,19 +65,19 @@ __inval_fence_signal(struct xe_device *xe,
> > struct xe_tlb_inval_fence *fence)
> > static void
> > inval_fence_signal(struct xe_device *xe, struct xe_tlb_inval_fence
> > *fence)
> > {
> > + lockdep_assert_held(&fence->tlb_inval->pending_lock);
> > +
> > list_del(&fence->link);
> > __inval_fence_signal(xe, fence);
> > }
> >
> > -void xe_tlb_inval_fence_signal(struct xe_tlb_inval_fence *fence)
> > +static void
> > +inval_fence_signal_unlocked(struct xe_device *xe,
> > + struct xe_tlb_inval_fence *fence)
> > {
> > - struct xe_gt *gt;
> > -
> > - if (WARN_ON_ONCE(!fence->tlb_inval))
> > - return;
> > -
> > - gt = fence->tlb_inval->private;
> > - __inval_fence_signal(gt_to_xe(gt), fence);
> > + spin_lock_irq(&fence->tlb_inval->pending_lock);
> > + inval_fence_signal(xe, fence);
> > + spin_unlock_irq(&fence->tlb_inval->pending_lock);
> > }
> >
> > static void xe_gt_tlb_fence_timeout(struct work_struct *work)
> > @@ -208,14 +208,10 @@ static bool tlb_inval_seqno_past(struct xe_gt
> > *gt, int seqno)
> > return seqno_recv >= seqno;
> > }
> >
> > -static int send_tlb_inval(struct xe_guc *guc,
> > - struct xe_tlb_inval_fence *fence,
> > +static int send_tlb_inval(struct xe_guc *guc, struct
> > xe_tlb_inval_fence *fence,
> > u32 *action, int len)
> > {
> > struct xe_gt *gt = guc_to_gt(guc);
> > - struct xe_device *xe = gt_to_xe(gt);
> > - int seqno;
> > - int ret;
> >
> > xe_gt_assert(gt, fence);
> >
> > @@ -225,47 +221,38 @@ static int send_tlb_inval(struct xe_guc *guc,
> > * need to be updated.
> > */
> >
> > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_TLB_INVAL, 1);
> > + action[1] = fence->seqno;
> > +
> > + return xe_guc_ct_send(&guc->ct, action, len,
> > + G2H_LEN_DW_TLB_INVALIDATE, 1);
> > +}
> > +
> > +static void xe_tlb_inval_fence_prep(struct xe_tlb_inval_fence
> > *fence)
> > +{
> > + struct xe_tlb_inval *tlb_inval = fence->tlb_inval;
> > + struct xe_gt *gt = tlb_inval->private;
> > + struct xe_device *xe = gt_to_xe(gt);
> > +
> > mutex_lock(>->tlb_inval.seqno_lock);
>
> The seqno lock should be asserted here, with the caller taking this
> and
> holding until send_tlb_inval is called.
Ah thanks again, you're right. I'll have a new rev posted shortly...
-Stuart
>
> Matt
>
> > - seqno = gt->tlb_inval.seqno;
> > - fence->seqno = seqno;
> > + fence->seqno = tlb_inval->seqno;
> > trace_xe_tlb_inval_fence_send(xe, fence);
> > - action[1] = seqno;
> > - ret = xe_guc_ct_send(&guc->ct, action, len,
> > - G2H_LEN_DW_TLB_INVALIDATE, 1);
> > - if (!ret) {
> > - spin_lock_irq(>->tlb_inval.pending_lock);
> > - /*
> > - * We haven't actually published the TLB fence as
> > per
> > - * pending_fences, but in theory our seqno could
> > have already
> > - * been written as we acquired the pending_lock. In
> > such a case
> > - * we can just go ahead and signal the fence here.
> > - */
> > - if (tlb_inval_seqno_past(gt, seqno)) {
> > - __inval_fence_signal(xe, fence);
> > - } else {
> > - fence->inval_time = ktime_get();
> > - list_add_tail(&fence->link,
> > - >-
> > >tlb_inval.pending_fences);
> > -
> > - if (list_is_singular(>-
> > >tlb_inval.pending_fences))
> > - queue_delayed_work(system_wq,
> > - >-
> > >tlb_inval.fence_tdr,
> > -
> > tlb_timeout_jiffies(gt));
> > - }
> > - spin_unlock_irq(>->tlb_inval.pending_lock);
> > - } else {
> > - __inval_fence_signal(xe, fence);
> > - }
> > - if (!ret) {
> > - gt->tlb_inval.seqno = (gt->tlb_inval.seqno + 1) %
> > - TLB_INVALIDATION_SEQNO_MAX;
> > - if (!gt->tlb_inval.seqno)
> > - gt->tlb_inval.seqno = 1;
> > - }
> > - mutex_unlock(>->tlb_inval.seqno_lock);
> > - xe_gt_stats_incr(gt, XE_GT_STATS_ID_TLB_INVAL, 1);
> >
> > - return ret;
> > + spin_lock_irq(&tlb_inval->pending_lock);
> > + fence->inval_time = ktime_get();
> > + list_add_tail(&fence->link, &tlb_inval->pending_fences);
> > +
> > + if (list_is_singular(&tlb_inval->pending_fences))
> > + queue_delayed_work(system_wq,
> > + &tlb_inval->fence_tdr,
> > + tlb_timeout_jiffies(gt));
> > + spin_unlock_irq(&tlb_inval->pending_lock);
> > +
> > + tlb_inval->seqno = (tlb_inval->seqno + 1) %
> > + TLB_INVALIDATION_SEQNO_MAX;
> > + if (!tlb_inval->seqno)
> > + tlb_inval->seqno = 1;
> > + mutex_unlock(>->tlb_inval.seqno_lock);
> > }
> >
> > #define MAKE_INVAL_OP(type) ((type <<
> > XE_GUC_TLB_INVAL_TYPE_SHIFT) | \
> > @@ -293,7 +280,12 @@ static int xe_tlb_inval_guc(struct xe_gt *gt,
> > };
> > int ret;
> >
> > + xe_tlb_inval_fence_prep(fence);
> > +
> > ret = send_tlb_inval(>->uc.guc, fence, action,
> > ARRAY_SIZE(action));
> > + if (ret < 0)
> > + inval_fence_signal_unlocked(gt_to_xe(gt), fence);
> > +
> > /*
> > * -ECANCELED indicates the CT is stopped for a GT reset.
> > TLB caches
> > * should be nuked on a GT reset so this error can be
> > ignored.
> > @@ -420,7 +412,7 @@ int xe_tlb_inval_range(struct xe_tlb_inval
> > *tlb_inval,
> > #define MAX_TLB_INVALIDATION_LEN 7
> > u32 action[MAX_TLB_INVALIDATION_LEN];
> > u64 length = end - start;
> > - int len = 0;
> > + int len = 0, ret;
> >
> > xe_gt_assert(gt, fence);
> >
> > @@ -481,7 +473,14 @@ int xe_tlb_inval_range(struct xe_tlb_inval
> > *tlb_inval,
> >
> > xe_gt_assert(gt, len <= MAX_TLB_INVALIDATION_LEN);
> >
> > - return send_tlb_inval(>->uc.guc, fence, action, len);
> > + xe_tlb_inval_fence_prep(fence);
> > +
> > + ret = send_tlb_inval(>->uc.guc, fence, action,
> > + ARRAY_SIZE(action));
> > + if (ret < 0)
> > + inval_fence_signal_unlocked(xe, fence);
> > +
> > + return ret;
> > }
> >
> > /**
> > --
> > 2.34.1
> >
More information about the Intel-xe
mailing list