From: "Summers, Stuart" <stuart.summers@intel.com>
To: "Vishwanathapura, Niranjana" <niranjana.vishwanathapura@intel.com>
Cc: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
"Piorkowski, Piotr" <piotr.piorkowski@intel.com>,
"Wajdeczko, Michal" <Michal.Wajdeczko@intel.com>,
"Winiarski, Michal" <michal.winiarski@intel.com>
Subject: Re: [PATCH] drm/xe: Add LMTT invalidation to VF provisioning flow
Date: Thu, 19 Mar 2026 20:34:10 +0000 [thread overview]
Message-ID: <249fb772e2d53f81ec095ea8bafedcbcdeadccf9.camel@intel.com> (raw)
In-Reply-To: <abxcjWSuj6UKCbq2@nvishwa1-desk>
On Thu, 2026-03-19 at 13:29 -0700, Niranjana Vishwanathapura wrote:
> On Thu, Mar 19, 2026 at 06:16:05PM +0000, Stuart Summers wrote:
> > There is a new invalidation type added to invalidate an LMTT of a
> > particular VF from the PF instead of doing a full invalidation.
> >
> > A new invalidation type has been added to GuC to accommodate this.
> > Add the new invalidation type, add a new TLB invalidation front
> > and back end hook for the new type, and use this in xe_lmtt.c
> > by passing the VF ID any time we are provisioning the LMTT for
> > a particular VF so GuC can then relay that VF ID to hardware
> > during the invalidation.
> >
> > Bspec: 59311
> >
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> > drivers/gpu/drm/xe/abi/guc_actions_abi.h | 2 ++
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 6 +++---
> > drivers/gpu/drm/xe/xe_guc_tlb_inval.c | 18 ++++++++++++++++++
> > drivers/gpu/drm/xe/xe_lmtt.c | 17 ++++++++++-------
> > drivers/gpu/drm/xe/xe_lmtt.h | 2 +-
> > drivers/gpu/drm/xe/xe_tlb_inval.c | 20
> > ++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_tlb_inval.h | 3 +++
> > drivers/gpu/drm/xe/xe_tlb_inval_types.h | 12 ++++++++++++
> > 8 files changed, 69 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> > b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> > index 83a6e7794982..31272de6fd3f 100644
> > --- a/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> > +++ b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> > @@ -239,6 +239,8 @@ enum xe_guc_tlb_invalidation_type {
> > XE_GUC_TLB_INVAL_PAGE_SELECTIVE = 0x1,
> > XE_GUC_TLB_INVAL_PAGE_SELECTIVE_CTX = 0x2,
> > XE_GUC_TLB_INVAL_GUC = 0x3,
> > + XE_GUC_TLB_INVAL_ALL_TLB = 0x4,
>
> XE_GUC_TLB_INVAL_ALL_TLB is unused.
Yeah this was intentional. It is part of the overall ABI even if we
aren't using it in the driver explicitly. I thought it strange to jump
from 0x3 to 0x5 without explanation. We have some other stuff in the
ABI that isn't used in the driver itself (on a quick search,
XE_GUC_ACTION_EXIT_S_STATE for example).
>
> > + XE_GUC_TLB_INVAL_LMTT_ONLY = 0x5,
> > };
> >
> > /*
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> > b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> > index 2f376b5fb088..bcdcb9fec8a1 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> > @@ -1518,7 +1518,7 @@ static int pf_distribute_config_lmem(struct
> > xe_gt *gt, unsigned int vfid, u64 si
> > return 0;
> > }
> >
> > -static void pf_force_lmtt_invalidate(struct xe_device *xe)
> > +static void pf_force_lmtt_invalidate(struct xe_device *xe,
> > unsigned int vfid)
> > {
> > struct xe_lmtt *lmtt;
> > struct xe_tile *tile;
> > @@ -1529,7 +1529,7 @@ static void pf_force_lmtt_invalidate(struct
> > xe_device *xe)
> >
> > for_each_tile(tile, xe, tid) {
> > lmtt = &tile->sriov.pf.lmtt;
> > - xe_lmtt_invalidate_hw(lmtt);
> > + xe_lmtt_invalidate_hw(lmtt, vfid);
> > }
> > }
> >
> > @@ -1592,7 +1592,7 @@ static int pf_update_vf_lmtt(struct xe_device
> > *xe, unsigned int vfid)
> > }
> > }
> >
> > - pf_force_lmtt_invalidate(xe);
> > + pf_force_lmtt_invalidate(xe, vfid);
> > return 0;
> >
> > fail:
> > diff --git a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > index 256759b826bc..978c3905a525 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > @@ -98,6 +98,22 @@ static int send_tlb_inval_ggtt(struct
> > xe_tlb_inval *tlb_inval, u32 seqno)
> > return -ECANCELED;
> > }
> >
> > +static int send_tlb_inval_lmtt(struct xe_tlb_inval *tlb_inval, u32
> > seqno,
> > + unsigned int vfid)
> > +{
> > + struct xe_guc *guc = tlb_inval->private;
> > + u32 action[] = {
> > + XE_GUC_ACTION_TLB_INVALIDATION,
> > + seqno,
> > + MAKE_INVAL_OP(XE_GUC_TLB_INVAL_LMTT_ONLY),
> > + vfid,
> > + };
> > +
> > + xe_gt_assert(guc_to_gt(guc), xe_device_has_lmtt(tlb_inval-
> > >xe));
> > +
> > + return send_tlb_inval(guc, action, ARRAY_SIZE(action));
> > +}
> > +
> > static int send_page_reclaim(struct xe_guc *guc, u32 seqno,
> > u64 gpu_addr)
> > {
> > @@ -355,6 +371,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,
> > + .lmtt = send_tlb_inval_lmtt,
> > .ppgtt = send_tlb_inval_asid_ppgtt,
> > .initialized = tlb_inval_initialized,
> > .flush = tlb_inval_flush,
> > @@ -364,6 +381,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,
> > .all = send_tlb_inval_all,
> > + .lmtt = send_tlb_inval_lmtt,
> > .ppgtt = send_tlb_inval_ctx_ppgtt,
> > .initialized = tlb_inval_initialized,
> > .flush = tlb_inval_flush,
> > diff --git a/drivers/gpu/drm/xe/xe_lmtt.c
> > b/drivers/gpu/drm/xe/xe_lmtt.c
> > index 0c726eda9390..a8bca1e5a633 100644
> > --- a/drivers/gpu/drm/xe/xe_lmtt.c
> > +++ b/drivers/gpu/drm/xe/xe_lmtt.c
> > @@ -252,19 +252,21 @@ void xe_lmtt_init_hw(struct xe_lmtt *lmtt)
> > lmtt_setup_dir_ptr(lmtt);
> > }
> >
> > -static int lmtt_invalidate_hw(struct xe_lmtt *lmtt)
> > +static int lmtt_invalidate_hw(struct xe_lmtt *lmtt, unsigned int
> > vfid)
> > {
> > struct xe_tlb_inval_fence fences[XE_MAX_GT_PER_TILE];
> > struct xe_tlb_inval_fence *fence = fences;
> > struct xe_tile *tile = lmtt_to_tile(lmtt);
> > struct xe_gt *gt;
> > - int result = 0;
> > - int err;
> > + int result = 0, err = 0;
>
> NIT...No need to init err as it is set in both if/else condition.
True, I was trying to follow along with the result here, but that
really is different since it does rely on the 0 case below... I can
revert this particular change.
>
> > u8 id;
> >
> > for_each_gt_on_tile(gt, tile, id) {
> > xe_tlb_inval_fence_init(>->tlb_inval, fence,
> > true);
> > - err = xe_tlb_inval_all(>->tlb_inval, fence);
> > + if (gt_to_xe(gt)->info.has_ctx_tlb_inval)
> > + err = xe_tlb_inval_lmtt(>->tlb_inval,
> > fence, vfid);
> > + else
> > + err = xe_tlb_inval_all(>->tlb_inval,
> > fence);
> > result = result ?: err;
> > fence++;
> > }
> > @@ -286,13 +288,14 @@ static int lmtt_invalidate_hw(struct xe_lmtt
> > *lmtt)
> > /**
> > * xe_lmtt_invalidate_hw - Invalidate LMTT hardware.
> > * @lmtt: the &xe_lmtt to invalidate
> > + * @vfid: VF ID associated with this LMTT
> > *
> > * Send requests to all GuCs on this tile to invalidate all TLBs.
> > * If the platform has a standalone MERT, also invalidate MERT's
> > TLB.
> > *
>
> This description is stale and needs update.
Ah sorry about that and good catch. I'll fix it.
>
> > * This function should be called only when running as a PF driver.
> > */
> > -void xe_lmtt_invalidate_hw(struct xe_lmtt *lmtt)
> > +void xe_lmtt_invalidate_hw(struct xe_lmtt *lmtt, unsigned int
> > vfid)
> > {
> > struct xe_tile *tile = lmtt_to_tile(lmtt);
> > struct xe_device *xe = lmtt_to_xe(lmtt);
> > @@ -300,7 +303,7 @@ void xe_lmtt_invalidate_hw(struct xe_lmtt
> > *lmtt)
> >
> > lmtt_assert(lmtt, IS_SRIOV_PF(xe));
> >
> > - err = lmtt_invalidate_hw(lmtt);
> > + err = lmtt_invalidate_hw(lmtt, vfid);
> > if (err)
> > xe_tile_sriov_err(tile, "LMTT invalidation failed
> > (%pe)",
> > ERR_PTR(err));
> > @@ -367,7 +370,7 @@ static void lmtt_drop_pages(struct xe_lmtt
> > *lmtt, unsigned int vfid)
> > return;
> >
> > lmtt_write_pte(lmtt, pd, LMTT_PTE_INVALID, vfid);
> > - lmtt_invalidate_hw(lmtt);
> > + lmtt_invalidate_hw(lmtt, vfid);
> >
> > lmtt_assert(lmtt, pd->level > 0);
> > lmtt_assert(lmtt, pt->level == pd->level - 1);
> > diff --git a/drivers/gpu/drm/xe/xe_lmtt.h
> > b/drivers/gpu/drm/xe/xe_lmtt.h
> > index 8fa387b38c52..ad370807ff3c 100644
> > --- a/drivers/gpu/drm/xe/xe_lmtt.h
> > +++ b/drivers/gpu/drm/xe/xe_lmtt.h
> > @@ -15,7 +15,7 @@ struct xe_lmtt_ops;
> > #ifdef CONFIG_PCI_IOV
> > int xe_lmtt_init(struct xe_lmtt *lmtt);
> > void xe_lmtt_init_hw(struct xe_lmtt *lmtt);
> > -void xe_lmtt_invalidate_hw(struct xe_lmtt *lmtt);
> > +void xe_lmtt_invalidate_hw(struct xe_lmtt *lmtt, unsigned int
> > vfid);
> > int xe_lmtt_prepare_pages(struct xe_lmtt *lmtt, unsigned int vfid,
> > u64 range);
> > int xe_lmtt_populate_pages(struct xe_lmtt *lmtt, unsigned int vfid,
> > struct xe_bo *bo, u64 offset);
> > void xe_lmtt_drop_pages(struct xe_lmtt *lmtt, unsigned int vfid);
> > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > index 10dcd4abb00f..1283fde94a16 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > @@ -321,6 +321,26 @@ int xe_tlb_inval_ggtt(struct xe_tlb_inval
> > *tlb_inval)
> > return ret;
> > }
> >
> > +/**
> > + * xe_tlb_inval_lmtt() - Issue a TLB invalidation for the LMTT for
> > a VF
> > + * @tlb_inval: TLB invalidation client
> > + * @fence: invalidation fence which will be signal on TLB
> > invalidation
> > + * completion
> > + * @vfid: VF ID for this LMTT
> > + *
> > + * Issue a TLB invalidation for the LMTT for a VF. 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_lmtt(struct xe_tlb_inval *tlb_inval,
> > + struct xe_tlb_inval_fence *fence,
> > + unsigned int vfid)
> > +{
> > + return xe_tlb_inval_issue(tlb_inval, fence, tlb_inval->ops-
> > >lmtt, vfid);
>
> Should assert if ops->lmtt is NULL? Or should caller call
> xe_tlb_inval_all instead
> if ops->lmtt is not supported?
Yeah good point... I might just add an assert here to catch in CI. I
feel like it should be on the caller to decide what kind of
invalidation to use (or whether to fall back or not) since we don't
from this function alone know whether the full invalidation covers
LMTT.
Thanks,
Stuart
>
> Niranjana
>
> > +}
> > +
> > /**
> > * 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..38d312c41365 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval.h
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.h
> > @@ -20,6 +20,9 @@ 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_lmtt(struct xe_tlb_inval *tlb_inval,
> > + struct xe_tlb_inval_fence *fence,
> > + unsigned int vfid);
> > 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..6205f5619303 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > @@ -36,6 +36,18 @@ struct xe_tlb_inval_ops {
> > */
> > int (*ggtt)(struct xe_tlb_inval *tlb_inval, u32 seqno);
> >
> > + /**
> > + * @lmtt: Invalidate the LMTT for a VF
> > + * @tlb_inval: TLB invalidation client
> > + * @seqno: Seqno of TLB invalidation
> > + * @vfid: VF ID for this LMTT
> > + *
> > + * Return 0 on success, -ECANCELED if backend is mid-reset,
> > error on
> > + * failure
> > + */
> > + int (*lmtt)(struct xe_tlb_inval *tlb_inval, u32 seqno,
> > + unsigned int vfid);
> > +
> > /**
> > * @ppgtt: Invalidate per-process translation TLBs
> > * @tlb_inval: TLB invalidation client
> > --
> > 2.43.0
> >
next prev parent reply other threads:[~2026-03-19 20:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-19 18:16 [PATCH] drm/xe: Add LMTT invalidation to VF provisioning flow Stuart Summers
2026-03-19 18:23 ` ✓ CI.KUnit: success for " Patchwork
2026-03-19 19:12 ` ✓ Xe.CI.BAT: " Patchwork
[not found] ` <177394539093.377258.6883218191011530136@a3b018990fe9>
2026-03-19 19:35 ` CI.Hooks: failure " Summers, Stuart
2026-03-19 20:29 ` [PATCH] " Niranjana Vishwanathapura
2026-03-19 20:34 ` Summers, Stuart [this message]
2026-03-20 18:16 ` ✗ Xe.CI.FULL: failure for " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=249fb772e2d53f81ec095ea8bafedcbcdeadccf9.camel@intel.com \
--to=stuart.summers@intel.com \
--cc=Michal.Wajdeczko@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=michal.winiarski@intel.com \
--cc=niranjana.vishwanathapura@intel.com \
--cc=piotr.piorkowski@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox