From: Matthew Brost <matthew.brost@intel.com>
To: "Summers, Stuart" <stuart.summers@intel.com>
Cc: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v3 04/11] drm/xe: Add vm to exec queues association
Date: Mon, 12 Jan 2026 17:30:14 -0800 [thread overview]
Message-ID: <aWWgJnJ0OCz36WeO@lstrano-desk.jf.intel.com> (raw)
In-Reply-To: <840197fff028c4de7c80d62e64b963dbef425979.camel@intel.com>
On Mon, Jan 12, 2026 at 06:04:53PM -0700, Summers, Stuart wrote:
> On Mon, 2026-01-12 at 15:27 -0800, Matthew Brost wrote:
> > Maintain a list of exec queues per vm which will be used by TLB
> > invalidation code to do context-ID based tlb invalidations.
> >
> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Reviewed-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_device.h | 7 ----
> > drivers/gpu/drm/xe/xe_device_types.h | 7 ++++
> > drivers/gpu/drm/xe/xe_exec_queue.c | 7 +++-
> > drivers/gpu/drm/xe/xe_exec_queue_types.h | 3 ++
> > drivers/gpu/drm/xe/xe_vm.c | 46
> > ++++++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_vm.h | 3 ++
> > drivers/gpu/drm/xe/xe_vm_types.h | 13 +++++++
> > 7 files changed, 78 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_device.h
> > b/drivers/gpu/drm/xe/xe_device.h
> > index 10d04c324257..db939fc82f71 100644
> > --- a/drivers/gpu/drm/xe/xe_device.h
> > +++ b/drivers/gpu/drm/xe/xe_device.h
> > @@ -62,13 +62,6 @@ static inline struct xe_tile
> > *xe_device_get_root_tile(struct xe_device *xe)
> > return &xe->tiles[0];
> > }
> >
> > -/*
> > - * Highest GT/tile count for any platform. Used only for memory
> > allocation
> > - * sizing. Any logic looping over GTs or mapping userspace GT IDs
> > into GT
> > - * structures should use the per-platform xe->info.max_gt_per_tile
> > instead.
> > - */
> > -#define XE_MAX_GT_PER_TILE 2
> > -
> > static inline struct xe_gt *xe_device_get_gt(struct xe_device *xe,
> > u8 gt_id)
> > {
> > struct xe_tile *tile;
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> > b/drivers/gpu/drm/xe/xe_device_types.h
> > index 4dab3057f58d..8db870aaa382 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -79,6 +79,13 @@ enum xe_wedged_mode {
> > #define XE_GT1 1
> > #define XE_MAX_TILES_PER_DEVICE (XE_GT1 + 1)
> >
> > +/*
> > + * Highest GT/tile count for any platform. Used only for memory
> > allocation
> > + * sizing. Any logic looping over GTs or mapping userspace GT IDs
> > into GT
> > + * structures should use the per-platform xe->info.max_gt_per_tile
> > instead.
> > + */
> > +#define XE_MAX_GT_PER_TILE 2
> > +
> > #define XE_MAX_ASID (BIT(20))
> >
> > #define IS_PLATFORM_STEP(_xe, _platform, min_step, max_step) \
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c
> > b/drivers/gpu/drm/xe/xe_exec_queue.c
> > index b5737563ee14..ce309d848916 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > @@ -154,8 +154,10 @@ static void __xe_exec_queue_free(struct
> > xe_exec_queue *q)
> > if (xe_exec_queue_is_multi_queue(q))
> > xe_exec_queue_group_cleanup(q);
> >
> > - if (q->vm)
> > + if (q->vm) {
> > + xe_vm_remove_exec_queue(q->vm, q);
> > xe_vm_put(q->vm);
> > + }
> >
> > if (q->xef)
> > xe_file_put(q->xef);
> > @@ -226,6 +228,7 @@ static struct xe_exec_queue
> > *__xe_exec_queue_alloc(struct xe_device *xe,
> > q->ring_ops = gt->ring_ops[hwe->class];
> > q->ops = gt->exec_queue_ops;
> > INIT_LIST_HEAD(&q->lr.link);
> > + INIT_LIST_HEAD(&q->vm_exec_queue_link);
> > INIT_LIST_HEAD(&q->multi_gt_link);
> > INIT_LIST_HEAD(&q->hw_engine_group_link);
> > INIT_LIST_HEAD(&q->pxp.link);
> > @@ -1205,6 +1208,8 @@ int xe_exec_queue_create_ioctl(struct
> > drm_device *dev, void *data,
> > }
> >
> > q->xef = xe_file_get(xef);
> > + if (eci[0].engine_class != DRM_XE_ENGINE_CLASS_VM_BIND)
> > + xe_vm_add_exec_queue(vm, q);
>
> This still will hit an issue if we're sending a TLB invalidation before
> we register the context with GuC, just to reiterate... of course we
> agreed to move forward here.
>
Ah, no. The active vfunc called in last patch prevents against this.
> >
> > /* user id alloc must always be last in ioctl to prevent UAF
> > */
> > err = xa_alloc(&xef->exec_queue.xa, &id, q, xa_limit_32b,
> > GFP_KERNEL);
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h
> > b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> > index 5fc516b0bb77..d3e2789cf5bc 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> > @@ -205,6 +205,9 @@ struct xe_exec_queue {
> > struct dma_fence *last_fence;
> > } tlb_inval[XE_EXEC_QUEUE_TLB_INVAL_COUNT];
> >
> > + /** @vm_exec_queue_link: Link to track exec queue within a
> > VM's list of exec queues. */
> > + struct list_head vm_exec_queue_link;
> > +
> > /** @pxp: PXP info tracking */
> > struct {
> > /** @pxp.type: PXP session type used by this queue */
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index bdb04d6bf39c..3a1a1bd74d8d 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -1530,11 +1530,23 @@ struct xe_vm *xe_vm_create(struct xe_device
> > *xe, u32 flags, struct xe_file *xef)
> > INIT_WORK(&vm->destroy_work, vm_destroy_work_func);
> >
> > INIT_LIST_HEAD(&vm->preempt.exec_queues);
> > + INIT_LIST_HEAD(&vm->exec_queues.list);
> > if (flags & XE_VM_FLAG_FAULT_MODE)
> > vm->preempt.min_run_period_ms = xe-
> > >min_run_period_pf_ms;
> > else
> > vm->preempt.min_run_period_ms = xe-
> > >min_run_period_lr_ms;
> >
> > + init_rwsem(&vm->exec_queues.lock);
> > + if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
> > + fs_reclaim_acquire(GFP_KERNEL);
> > + might_lock(&vm->exec_queues.lock);
> > + fs_reclaim_release(GFP_KERNEL);
> > +
> > + down_read(&vm->exec_queues.lock);
> > + might_lock(&xe_root_mmio_gt(xe)->uc.guc.ct.lock);
> > + up_read(&vm->exec_queues.lock);
> > + }
> > +
> > for_each_tile(tile, xe, id)
> > xe_range_fence_tree_init(&vm->rftree[id]);
> >
> > @@ -4570,3 +4582,37 @@ int xe_vm_alloc_cpu_addr_mirror_vma(struct
> > xe_vm *vm, uint64_t start, uint64_t r
> > return xe_vm_alloc_vma(vm, &map_req, false);
> > }
> >
> > +/**
> > + * xe_vm_add_exec_queue() - Add exec queue to VM
> > + * @vm: The VM.
> > + * @q: The exec_queue
> > + */
> > +void xe_vm_add_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q)
> > +{
> > + /* User VMs and queues only */
> > + xe_assert(vm->xe, !(q->flags & EXEC_QUEUE_FLAG_KERNEL));
> > + xe_assert(vm->xe, !(q->flags & EXEC_QUEUE_FLAG_PERMANENT));
> > + xe_assert(vm->xe, !(q->flags & EXEC_QUEUE_FLAG_VM));
> > + xe_assert(vm->xe, !(q->flags & EXEC_QUEUE_FLAG_MIGRATE));
> > + xe_assert(vm->xe, vm->xef);
>
> Can you add a check here against has_ctx_tlb_inval? No need to maintain
> this list if the platform doesn't support this invalidation.
>
Yes, I do that.
> Can we also assert that the q->vm is the same as the vm being passed
> above? We want to make sure we aren't adding a queue from another VM
> here.
>
Yes, I can also do that.
Matt
> Thanks,
> Stuart
>
> > +
> > + down_write(&vm->exec_queues.lock);
> > + list_add(&q->vm_exec_queue_link, &vm->exec_queues.list);
> > + ++vm->exec_queues.count[q->gt->info.id];
> > + up_write(&vm->exec_queues.lock);
> > +}
> > +
> > +/**
> > + * xe_vm_remove_exec_queue() - Remove exec queue from VM
> > + * @vm: The VM.
> > + * @q: The exec_queue
> > + */
> > +void xe_vm_remove_exec_queue(struct xe_vm *vm, struct xe_exec_queue
> > *q)
> > +{
> > + down_write(&vm->exec_queues.lock);
> > + if (!list_empty(&q->vm_exec_queue_link)) {
> > + list_del(&q->vm_exec_queue_link);
> > + --vm->exec_queues.count[q->gt->info.id];
> > + }
> > + up_write(&vm->exec_queues.lock);
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
> > index 6cc98df47291..288115c7844a 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.h
> > +++ b/drivers/gpu/drm/xe/xe_vm.h
> > @@ -287,6 +287,9 @@ static inline struct dma_resv *xe_vm_resv(struct
> > xe_vm *vm)
> >
> > void xe_vm_kill(struct xe_vm *vm, bool unlocked);
> >
> > +void xe_vm_add_exec_queue(struct xe_vm *vm, struct xe_exec_queue
> > *q);
> > +void xe_vm_remove_exec_queue(struct xe_vm *vm, struct xe_exec_queue
> > *q);
> > +
> > /**
> > * xe_vm_assert_held(vm) - Assert that the vm's reservation object
> > is held.
> > * @vm: The vm
> > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h
> > b/drivers/gpu/drm/xe/xe_vm_types.h
> > index 437f64202f3b..87318a4f5b08 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_types.h
> > +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> > @@ -298,6 +298,19 @@ struct xe_vm {
> > struct list_head pm_activate_link;
> > } preempt;
> >
> > + /** @exec_queues: Manages list of exec queues attached to
> > this VM, protected by lock. */
> > + struct {
> > + /** @exec_queues.list: list of exec queues attached
> > to this VM */
> > + struct list_head list;
> > + /**
> > + * @exec_queues.count: count of exec queues attached
> > to this VM,
> > + * per GT
> > + */
> > + int count[XE_MAX_TILES_PER_DEVICE *
> > XE_MAX_GT_PER_TILE];
> > + /** @exec_queues.lock: lock to protect exec_queues
> > list */
> > + struct rw_semaphore lock;
> > + } exec_queues;
> > +
> > /** @um: unified memory state */
> > struct {
> > /** @asid: address space ID, unique to each VM */
>
next prev parent reply other threads:[~2026-01-13 1:30 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-12 23:27 [PATCH v3 00/11] Context based TLB invalidations Matthew Brost
2026-01-12 23:27 ` [PATCH v3 01/11] drm/xe: Add normalize_invalidation_range Matthew Brost
2026-01-12 23:27 ` [PATCH v3 02/11] drm/xe: Make usm.asid_to_vm allocation use GFP_NOWAIT Matthew Brost
2026-01-12 23:27 ` [PATCH v3 03/11] drm/xe: Add xe_device_asid_to_vm helper Matthew Brost
2026-01-13 1:01 ` Summers, Stuart
2026-01-13 1:36 ` Matthew Brost
2026-01-12 23:27 ` [PATCH v3 04/11] drm/xe: Add vm to exec queues association Matthew Brost
2026-01-13 1:04 ` Summers, Stuart
2026-01-13 1:30 ` Matthew Brost [this message]
2026-01-13 21:55 ` Summers, Stuart
2026-01-12 23:27 ` [PATCH v3 05/11] drm/xe: Taint TLB invalidation seqno lock with GFP_KERNEL Matthew Brost
2026-01-12 23:27 ` [PATCH v3 06/11] drm/xe: Rename send_tlb_inval_ppgtt to send_tlb_inval_asid_ppgtt Matthew Brost
2026-01-12 23:27 ` [PATCH v3 07/11] drm/xe: Add send_tlb_inval_ppgtt helper Matthew Brost
2026-01-12 23:27 ` [PATCH v3 08/11] drm/xe: Add xe_tlb_inval_idle helper Matthew Brost
2026-01-12 23:27 ` [PATCH v3 09/11] drm/xe: Add exec queue active vfunc Matthew Brost
2026-01-12 23:27 ` [PATCH v3 10/11] drm/xe: Add context-based invalidation to GuC TLB invalidation backend Matthew Brost
2026-01-13 1:28 ` Summers, Stuart
2026-01-13 1:34 ` Matthew Brost
2026-01-13 22:36 ` Summers, Stuart
2026-01-14 16:09 ` Matthew Brost
2026-01-14 23:45 ` Summers, Stuart
2026-01-12 23:27 ` [PATCH v3 11/11] drm/xe: Enable context TLB invalidations for CI Matthew Brost
2026-01-12 23:37 ` ✓ CI.KUnit: success for Context based TLB invalidations (rev3) Patchwork
2026-01-13 0:17 ` ✗ Xe.CI.BAT: failure " 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=aWWgJnJ0OCz36WeO@lstrano-desk.jf.intel.com \
--to=matthew.brost@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=stuart.summers@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