Re: [PATCH v8 05/10] drm/xe/exec_queue: Introduce xe_exec_queue_reinit()

[Raag Jadav <raag.jadav@intel.com>]

On Thu, Jun 25, 2026 at 05:06:12PM -0700, Daniele Ceraolo Spurio wrote:
> On 6/3/2026 3:17 AM, Raag Jadav wrote:
> > In preparation of usecases which require re-initializing exec queue
> > after PCIe FLR, introduce xe_exec_queue_reinit() helper. All the exec
> > queue LCRs already exist but the context is lost on PCIe FLR and needs
> > re-initialization.
> > 
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > Tested-by: Lukasz Laguna <lukasz.laguna@intel.com>
> > ---
> > v2: Re-initialize migrate context (Matthew Brost)
> > v6: Add IS_DGFX() and EXEC_QUEUE_FLAG_KERNEL asserts (Daniele)
> > ---
> >   drivers/gpu/drm/xe/xe_exec_queue.c | 42 +++++++++++++++++++++++++++---
> >   drivers/gpu/drm/xe/xe_exec_queue.h |  1 +
> >   drivers/gpu/drm/xe/xe_lrc.c        | 17 ++++++++++++
> >   drivers/gpu/drm/xe/xe_lrc.h        |  2 ++
> >   4 files changed, 58 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> > index 1b5ca3ce578a..91b241a7ab24 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > @@ -335,9 +335,8 @@ static void __xe_exec_queue_fini(struct xe_exec_queue *q)
> >   		xe_lrc_put(q->lrc[i]);
> >   }
> > -static int __xe_exec_queue_init(struct xe_exec_queue *q, u32 exec_queue_flags)
> > +static u32 xe_lrc_init_flags(struct xe_exec_queue *q, u32 exec_queue_flags)
> >   {
> > -	int i, err;
> >   	u32 flags = 0;
> >   	/*
> > @@ -360,6 +359,13 @@ static int __xe_exec_queue_init(struct xe_exec_queue *q, u32 exec_queue_flags)
> >   	if (q->flags & EXEC_QUEUE_FLAG_DISABLE_STATE_CACHE_PERF_FIX)
> >   		flags |= XE_LRC_DISABLE_STATE_CACHE_PERF_FIX;
> > +	return flags;
> > +}
> > +
> > +static int __xe_exec_queue_init(struct xe_exec_queue *q, u32 exec_queue_flags)
> > +{
> > +	int i, err;
> > +
> >   	err = q->ops->init(q);
> >   	if (err)
> >   		return err;
> > @@ -383,8 +389,8 @@ static int __xe_exec_queue_init(struct xe_exec_queue *q, u32 exec_queue_flags)
> >   			marker = xe_gt_sriov_vf_wait_valid_ggtt(q->gt);
> > -			lrc = xe_lrc_create(q->hwe, q->vm, q->replay_state,
> > -					    xe_lrc_ring_size(), q->msix_vec, flags);
> > +			lrc = xe_lrc_create(q->hwe, q->vm, q->replay_state, xe_lrc_ring_size(),
> > +					    q->msix_vec, xe_lrc_init_flags(q, exec_queue_flags));
> >   			if (IS_ERR(lrc)) {
> >   				err = PTR_ERR(lrc);
> >   				goto err_lrc;
> > @@ -406,6 +412,34 @@ static int __xe_exec_queue_init(struct xe_exec_queue *q, u32 exec_queue_flags)
> >   	return err;
> >   }
> > +/**
> > + * xe_exec_queue_reinit() - Re-initialize exec queue
> > + * @q: exec queue to re-initialize
> > + *
> > + * Returns: 0 on success, negative error code otherwise.
> > + */
> > +int xe_exec_queue_reinit(struct xe_exec_queue *q)
> > +{
> > +	int i, err;
> > +
> > +	/* TODO: Re-initialize GSC and PXP queues */
> > +	xe_assert(gt_to_xe(q->gt), IS_DGFX(gt_to_xe(q->gt)));
> You can use xe_gt_assert here. Also, note that us not supporting GSC and PXP
> on discrete is a choice, not a generic requirement, so IMO for PXP/GSC
> checking it'd be better to add:
> 
> xe_gt_assert(q->gt, !xe_uc_fw_is_loadable(&gt->uc.gsc.fw));
> 
> xe_gt_assert(q->gt, !xe_pxp_is_enabled(gt_to_xe(q->gt)->pxp));

Sure.

> > +	/* Re-initialization only allowed for kernel queues */
> > +	xe_assert(gt_to_xe(q->gt), q->flags & EXEC_QUEUE_FLAG_KERNEL);
> > +
> > +	/* Re-initialize submission backend */
> > +	q->ops->reinit_kernel(q);
> 
> Thinking about it, the xe_sched_reinit() call from patch 2 could be moved
> here since that's not back-end specific.
> I won't ask you to re-spin just for this, so I'm ok with it being considered
> as a follow-up, but if you need to do a re-spin anyway it might be worth
> doing it now.

We xe_sched_init() inside guc_exec_queue_init() which IIUC is ->init()
backend call, so I thought we should be following the same pattern here?
Or did I miss something?

> > +
> > +	for (i = 0; i < q->width; i++) {
> > +		err = xe_lrc_reinit(q->lrc[i], q->hwe, q->vm, q->replay_state,
> > +				    q->msix_vec, xe_lrc_init_flags(q, q->flags));
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   /**
> >    * xe_exec_queue_create() - Create an exec queue
> >    * @xe: Xe device
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.h b/drivers/gpu/drm/xe/xe_exec_queue.h
> > index a82d99bd77bc..445867d4da26 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.h
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.h
> > @@ -34,6 +34,7 @@ struct xe_exec_queue *xe_exec_queue_create_bind(struct xe_device *xe,
> >   void xe_exec_queue_fini(struct xe_exec_queue *q);
> >   void xe_exec_queue_destroy(struct kref *ref);
> >   void xe_exec_queue_assign_name(struct xe_exec_queue *q, u32 instance);
> > +int xe_exec_queue_reinit(struct xe_exec_queue *q);
> >   static inline struct xe_exec_queue *
> >   xe_exec_queue_get_unless_zero(struct xe_exec_queue *q)
> > diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
> > index a4292a11391d..c425269893fe 100644
> > --- a/drivers/gpu/drm/xe/xe_lrc.c
> > +++ b/drivers/gpu/drm/xe/xe_lrc.c
> > @@ -1629,6 +1629,23 @@ static int xe_lrc_ctx_init(struct xe_lrc *lrc, struct xe_hw_engine *hwe, struct
> >   	return err;
> >   }
> > +/**
> > + * xe_lrc_reinit() - Re-initialize LRC
> > + * @lrc: Pointer to the LRC
> > + * @hwe: Hardware Engine
> > + * @vm: The VM (address space)
> > + * @replay_state: GPU hang replay state
> > + * @msix_vec: MSI-X interrupt vector (for platforms that support it)
> > + * @init_flags: LRC initialization flags
> > + *
> > + * Returns: 0 on success, negative error code otherwise.
> > + */
> > +int xe_lrc_reinit(struct xe_lrc *lrc, struct xe_hw_engine *hwe, struct xe_vm *vm,
> > +		  void *replay_state, u16 msix_vec, u32 init_flags)
> > +{
> > +	return xe_lrc_ctx_init(lrc, hwe, vm, replay_state, msix_vec, init_flags);
> 
> nit: I wonder if it 'd just be easier to just export xe_lrc_ctx_init instead
> of creating xe_lrc_reinit. Not a blocker, so with the asserts fixed:

Coming right up.

> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Awesome :)

Raag

> > +}
> > +
> >   static int xe_lrc_init(struct xe_lrc *lrc, struct xe_hw_engine *hwe, struct xe_vm *vm,
> >   		       void *replay_state, u32 ring_size, u16 msix_vec, u32 init_flags)
> >   {
> > diff --git a/drivers/gpu/drm/xe/xe_lrc.h b/drivers/gpu/drm/xe/xe_lrc.h
> > index 0a3a611391ee..333b11f739ea 100644
> > --- a/drivers/gpu/drm/xe/xe_lrc.h
> > +++ b/drivers/gpu/drm/xe/xe_lrc.h
> > @@ -56,6 +56,8 @@ struct xe_lrc_snapshot {
> >   struct xe_lrc *xe_lrc_create(struct xe_hw_engine *hwe, struct xe_vm *vm,
> >   			     void *replay_state, u32 ring_size, u16 msix_vec, u32 flags);
> > +int xe_lrc_reinit(struct xe_lrc *lrc, struct xe_hw_engine *hwe, struct xe_vm *vm,
> > +		  void *replay_state, u16 msix_vec, u32 init_flags);
> >   void xe_lrc_destroy(struct kref *ref);
> >   /**
>