Re: [PATCH v3 10/25] drm/xe: Update GuC submission backend to run PT jobs

Summers, Stuart
2026-03-05T20:24:38Z

On Wed, 2026-03-04 at 13:53 -0800, Matthew Brost wrote:
> On Wed, Mar 04, 2026 at 01:43:27PM -0700, Summers, Stuart wrote:
> > On Tue, 2026-03-03 at 16:26 -0800, Matthew Brost wrote:
> > > On Tue, Mar 03, 2026 at 04:28:50PM -0700, Summers, Stuart wrote:
> > > > On Fri, 2026-02-27 at 17:34 -0800, Matthew Brost wrote:
> > > > > PT jobs bypass GPU execution for the final step of a bind
> > > > > job,
> > > > > using
> > > > > the
> > > > > CPU to program the required page tables. Teach the GuC
> > > > > submission
> > > > > backend
> > > > > how to execute these jobs.
> > > > >
> > > > > PT job submission is implemented in the GuC backend for
> > > > > simplicity. A
> > > > > follow-up patch could introduce a dedicated backend for PT
> > > > > jobs.
> > > >
> > > > Still looking through the whole series, but standing alone,
> > > > this
> > > > patch
> > > > doesn't feel right to me. I don't see why we'd want to hook
> > > > together
> > > > the PT update flow with the GuC backend...
> > > >
> > >
> > > I don't think it is either, which is why I called out a follow-up
> > > to
> > > implement the PT backend. It’s likely a bigger refactor than one
> > > would
> > > expect, though...
> > >
> > > - Build the backend on top of xe_dep_scheduler
> > > - Introduce xe_pt_job that inherits from xe_dep_job
> > > - Ripple these changes through CPU bind, the PT layer, etc.
> > >
> > > A lot of that is just shuffling code around across those three
> > > steps,
> > > but it’s not too bad and will likely give us some nice layering
> > > cleanups
> > > along the way.
> > >
> > > The real tricky part is handling all the flows that stop/start
> > > the
> > > backends while various global events occur (e.g., PM enter/exit,
> > > GT
> > > resets, VF migration, FLR (WIP), etc.). All of those flows are
> > > currently
> > > GT → UC → GuC layered (or, for VF migration, direct to GuC). So
> > > we’d
> > > need a refactor there as well. It’s doable, but it will end up
> > > touching
> > > quite a few files. Again, once we do this, I suspect we’ll get
> > > additional layering cleanups along the way.
> > >
> > > So for now, given the already large size of the series, I’d like
> > > to
> > > get
> > > the functionality in first and then tackle the layering
> > > refactors.
> >
> > I get what you're saying here. Other than complexity, is there a
> > reason
> > we can't do that work first though? Is there some critical reason
> > we
>
> No reason I couldn't pile in those changes into this series.
>
> > need to get the CPU binding work in first, basically?
> >
>
> CPU binding is the basis for much of the SVM performance work—ULLS on
> the migration queue doesn’t work without it, and parallel
> faults/prefetches don’t work all that well either because of ordering
> issues on the migration queue between copies, clears, binds, etc.
>
> CPU binds also make some of the multi-tile refactors easier (included
> in
> this series; see patches 12, 15, and 16).
>
> So I’d like to get this in without expanding the scope too much. I
> will
> definitely rework this to use a dedicated backend and wire it up
> through
> all the layers in a follow-up, though.

Yeah I get what you're saying. I'll need to finish reviewing those
later patches before I'm ready to review this one. But I'm going to be
a little busy the next few days so likely not able to get back here
until some time next week. Don't feel like you need to hold here for me
if someone else can review in the meantime.

Thanks,
Stuart

>
> Matt
>
> > -Stuart
> >
> > >
> > > Matt
> > >
> > > > Thanks,
> > > > Stuart
> > > >
> > > > >
> > > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/xe/xe_guc_submit.c | 37
> > > > > ++++++++++++++++++++++++++--
> > > > > --
> > > > >  drivers/gpu/drm/xe/xe_migrate.c    | 13 ++++++++++-
> > > > >  drivers/gpu/drm/xe/xe_migrate.h    |  8 +++++++
> > > > >  3 files changed, 52 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
> > > > > b/drivers/gpu/drm/xe/xe_guc_submit.c
> > > > > index 453af51fe87b..1d6ac7a6563b 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > > > > @@ -36,8 +36,10 @@
> > > > >  #include "xe_lrc.h"
> > > > >  #include "xe_macros.h"
> > > > >  #include "xe_map.h"
> > > > > +#include "xe_migrate.h"
> > > > >  #include "xe_mocs.h"
> > > > >  #include "xe_pm.h"
> > > > > +#include "xe_pt.h"
> > > > >  #include "xe_ring_ops_types.h"
> > > > >  #include "xe_sched_job.h"
> > > > >  #include "xe_sleep.h"
> > > > > @@ -1183,6 +1185,20 @@ static void submit_exec_queue(struct
> > > > > xe_exec_queue *q, struct xe_sched_job *job)
> > > > >         }
> > > > >  }
> > > > >
> > > > > +static bool is_pt_job(struct xe_sched_job *job)
> > > > > +{
> > > > > +       return job->is_pt_job;
> > > > > +}
> > > > > +
> > > > > +static void run_pt_job(struct xe_sched_job *job)
> > > > > +{
> > > > > +       xe_migrate_update_pgtables_cpu_execute(job-
> > > > > > pt_update[0].vm,
> > > > > +                                              job-
> > > > > > pt_update[0].tile,
> > > > > +                                              job-
> > > > > > pt_update[0].ops,
> > > > > +                                              job-
> > > > > > pt_update[0].pt_job_ops->ops,
> > > > > +                                              job-
> > > > > > pt_update[0].pt_job_ops->current_op);
> > > > > +}
> > > > > +
> > > > >  static struct dma_fence *
> > > > >  guc_exec_queue_run_job(struct drm_sched_job *drm_job)
> > > > >  {
> > > > > @@ -1210,14 +1226,25 @@ guc_exec_queue_run_job(struct
> > > > > drm_sched_job
> > > > > *drm_job)
> > > > >                                 register_exec_queue(primary,
> > > > > GUC_CONTEXT_NORMAL);
> > > > >                 }
> > > > >
> > > > > -               if (!exec_queue_registered(q))
> > > > > -                       register_exec_queue(q,
> > > > > GUC_CONTEXT_NORMAL);
> > > > > -               if (!job->restore_replay)
> > > > > -                       q->ring_ops->emit_job(job);
> > > > > -               submit_exec_queue(q, job);
> > > > > +               if (is_pt_job(job)) {
> > > > > +                       xe_gt_assert(guc_to_gt(guc),
> > > > > !exec_queue_registered(q));
> > > > > +                       run_pt_job(job);
> > > > > +               } else {
> > > > > +                       if (!exec_queue_registered(q))
> > > > > +                               register_exec_queue(q,
> > > > > GUC_CONTEXT_NORMAL);
> > > > > +                       if (!job->restore_replay)
> > > > > +                               q->ring_ops->emit_job(job);
> > > > > +                       submit_exec_queue(q, job);
> > > > > +               }
> > > > >                 job->restore_replay = false;
> > > > >         }
> > > > >
> > > > > +       if (is_pt_job(job)) {
> > > > > +               xe_pt_job_ops_put(job-
> > > > > >pt_update[0].pt_job_ops);
> > > > > +               dma_fence_put(job->fence);      /* Drop ref
> > > > > from
> > > > > xe_sched_job_arm */
> > > > > +               return NULL;
> > > > > +       }
> > > > > +
> > > > >  run_job_out:
> > > > >
> > > > >         return job->fence;
> > > > > diff --git a/drivers/gpu/drm/xe/xe_migrate.c
> > > > > b/drivers/gpu/drm/xe/xe_migrate.c
> > > > > index cd6802642ef3..e9b9dfe19e48 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_migrate.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_migrate.c
> > > > > @@ -1715,7 +1715,18 @@ struct migrate_test_params {
> > > > >         container_of(_priv, struct migrate_test_params, base)
> > > > >  #endif
> > > > >
> > > > > -static void
> > > > > +/**
> > > > > + * xe_migrate_update_pgtables_cpu_execute() - Update a VM's
> > > > > PTEs
> > > > > via
> > > > > the CPU
> > > > > + * @vm: The VM being updated
> > > > > + * @tile: The tile being updated
> > > > > + * @ops: The migrate PT update ops
> > > > > + * @pt_ops: The VM PT update ops
> > > > > + * @num_ops: The number of The VM PT update ops
> > > > > + *
> > > > > + * Execute the VM PT update ops array which results in a
> > > > > VM's
> > > > > PTEs
> > > > > being updated
> > > > > + * via the CPU.
> > > > > + */
> > > > > +void
> > > > >  xe_migrate_update_pgtables_cpu_execute(struct xe_vm *vm,
> > > > > struct
> > > > > xe_tile *tile,
> > > > >                                        const struct
> > > > > xe_migrate_pt_update_ops *ops,
> > > > >                                        struct
> > > > > xe_vm_pgtable_update_op
> > > > > *pt_op,
> > > > > diff --git a/drivers/gpu/drm/xe/xe_migrate.h
> > > > > b/drivers/gpu/drm/xe/xe_migrate.h
> > > > > index c3c0740f908d..30c9c990a8b1 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_migrate.h
> > > > > +++ b/drivers/gpu/drm/xe/xe_migrate.h
> > > > > @@ -24,6 +24,7 @@ struct xe_pt;
> > > > >  struct xe_tile;
> > > > >  struct xe_vm;
> > > > >  struct xe_vm_pgtable_update;
> > > > > +struct xe_vm_pgtable_update_op;
> > > > >  struct xe_vma;
> > > > >
> > > > >  enum xe_sriov_vf_ccs_rw_ctxs;
> > > > > @@ -157,6 +158,13 @@ struct dma_fence
> > > > > *xe_migrate_clear(struct
> > > > > xe_migrate *m,
> > > > >
> > > > >  struct xe_vm *xe_migrate_get_vm(struct xe_migrate *m);
> > > > >
> > > > > +
> > > > > +void
> > > > > +xe_migrate_update_pgtables_cpu_execute(struct xe_vm *vm,
> > > > > struct
> > > > > xe_tile *tile,
> > > > > +                                      const struct
> > > > > xe_migrate_pt_update_ops *ops,
> > > > > +                                      struct
> > > > > xe_vm_pgtable_update_op
> > > > > *pt_op,
> > > > > +                                      int num_ops);
> > > > > +
> > > > >  struct dma_fence *
> > > > >  xe_migrate_update_pgtables(struct xe_migrate *m,
> > > > >                            struct xe_migrate_pt_update
> > > > > *pt_update);
> > > >
> >