Re: [PATCH v6 13/24] drm/xe: Enable CPU binds for jobs

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Sep 21, 2026 at 07:11:17PM +0530, Ghimiray, Himal Prasad wrote:
> 
> 
> On 05-09-2026 02:46, Matthew Brost wrote:
> > No reason to use the GPU for binds.
> > 
> > Benefits of CPU-based binds:
> > - Lower latency once dependencies are resolved, as there is no
> >    interaction with the GuC or a hardware context switch both of which
> >    are relatively slow.
> > - Large arrays of binds do not risk running out of migration PTEs,
> >    avoiding -ENOBUFS being returned to userspace.
> > - Kernel binds are decoupled from the migration exec queue (which issues
> >    copies and clears), so they cannot get stuck behind unrelated
> >    jobs—this can be a problem with parallel GPU faults.
> > - Paves the for path decouping binds from tiles and individual engines
> > - Enables ULLS on the migration exec queue, as this queue has exclusive
> >    access to the paging copy engine.
> > 
> > Update migration layer to formulate a PT job which will issue CPU bind
> > in the submission backend.
> > 
> > All code related to GPU-based binding has been removed.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Link: https://patch.msgid.link/20260228013501.106680-14-matthew.brost@intel.com
> > Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> > ---
> >   drivers/gpu/drm/xe/xe_bo_types.h |   2 -
> >   drivers/gpu/drm/xe/xe_migrate.c  | 249 +++----------------------------
> >   drivers/gpu/drm/xe/xe_pt.c       |   1 -
> >   3 files changed, 17 insertions(+), 235 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_bo_types.h b/drivers/gpu/drm/xe/xe_bo_types.h
> > index 0eb93052c1d5..8ec4a01a0092 100644
> > --- a/drivers/gpu/drm/xe/xe_bo_types.h
> > +++ b/drivers/gpu/drm/xe/xe_bo_types.h
> > @@ -90,8 +90,6 @@ struct xe_bo {
> >   	/** @freed: List node for delayed put. */
> >   	struct llist_node freed;
> > -	/** @update_index: Update index if PT BO */
> > -	int update_index;
> >   	/** @created: Whether the bo has passed initial creation */
> >   	bool created;
> > diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> > index ba8e195afcc8..e5c46e0fa960 100644
> > --- a/drivers/gpu/drm/xe/xe_migrate.c
> > +++ b/drivers/gpu/drm/xe/xe_migrate.c
> > @@ -77,18 +77,12 @@ struct xe_migrate {
> >   	 * Protected by @job_mutex.
> >   	 */
> >   	struct dma_fence *fence;
> > -	/**
> > -	 * @vm_update_sa: For integrated, used to suballocate page-tables
> > -	 * out of the pt_bo.
> > -	 */
> > -	struct drm_suballoc_manager vm_update_sa;
> >   	/** @min_chunk_size: For dgfx, Minimum chunk size */
> >   	u64 min_chunk_size;
> >   };
> >   #define MAX_PREEMPTDISABLE_TRANSFER SZ_8M /* Around 1ms. */
> >   #define MAX_CCS_LIMITED_TRANSFER SZ_4M /* XE_PAGE_SIZE * (FIELD_MAX(XE2_CCS_SIZE_MASK) + 1) */
> > -#define NUM_KERNEL_PDE 15
> >   #define NUM_PT_SLOTS 48
> >   #define LEVEL0_PAGE_TABLE_ENCODE_SIZE SZ_2M
> >   #define MAX_NUM_PTE 512
> > @@ -113,7 +107,6 @@ static void xe_migrate_fini(void *arg)
> >   	dma_fence_put(m->fence);
> >   	xe_bo_put(m->pt_bo);
> > -	drm_suballoc_manager_fini(&m->vm_update_sa);
> >   	mutex_destroy(&m->job_mutex);
> >   	xe_vm_close_and_put(m->q->vm);
> >   	xe_exec_queue_put(m->q);
> > @@ -234,8 +227,6 @@ static int xe_migrate_pt_bo_alloc(struct xe_tile *tile, struct xe_migrate *m,
> >   	BUILD_BUG_ON(NUM_PT_SLOTS > SZ_2M/XE_PAGE_SIZE);
> >   	/* Must be a multiple of 64K to support all platforms */
> >   	BUILD_BUG_ON(NUM_PT_SLOTS * XE_PAGE_SIZE % SZ_64K);
> > -	/* And one slot reserved for the 4KiB page table updates */
> > -	BUILD_BUG_ON(!(NUM_KERNEL_PDE & 1));
> >   	/* Need to be sure everything fits in the first PT, or create more */
> >   	xe_tile_assert(tile, m->batch_base_ofs + xe_bo_size(batch) < SZ_2M);
> > @@ -391,17 +382,9 @@ static void xe_migrate_prepare_vm(struct xe_tile *tile, struct xe_migrate *m,
> >   		}
> >   	}
> > -	if (ofs)
> > -		*ofs = map_ofs;
> > -}
> > -
> > -static void xe_migrate_suballoc_manager_init(struct xe_migrate *m, u32 map_ofs)
> > -{
> >   	/*
> >   	 * Example layout created above, with root level = 3:
> >   	 * [PT0...PT7]: kernel PT's for copy/clear; 64 or 4KiB PTE's
> > -	 * [PT8]: Kernel PT for VM_BIND, 4 KiB PTE's
> > -	 * [PT9...PT40]: Userspace PT's for VM_BIND, 4 KiB PTE's
> >   	 * [PT41 = PDE 0] [PT44...PT47 = 4K and 2M vram identity maps]
> >   	 *
> >   	 * This makes the lowest part of the VM point to the pagetables.
> > @@ -409,19 +392,13 @@ static void xe_migrate_suballoc_manager_init(struct xe_migrate *m, u32 map_ofs)
> >   	 * and flushes, other parts of the VM can be used either for copying and
> >   	 * clearing.
> >   	 *
> > -	 * For performance, the kernel reserves PDE's, so about 20 are left
> > -	 * for async VM updates.
> > -	 *
> >   	 * To make it easier to work, each scratch PT is put in slot (1 + PT #)
> >   	 * everywhere, this allows lockless updates to scratch pages by using
> >   	 * the different addresses in VM.
> >   	 */
> > -#define NUM_VMUSA_UNIT_PER_PAGE	32
> > -#define VM_SA_UPDATE_UNIT_SIZE		(XE_PAGE_SIZE / NUM_VMUSA_UNIT_PER_PAGE)
> > -#define NUM_VMUSA_WRITES_PER_UNIT	(VM_SA_UPDATE_UNIT_SIZE / sizeof(u64))
> > -	drm_suballoc_manager_init(&m->vm_update_sa,
> > -				  (size_t)(map_ofs / XE_PAGE_SIZE - NUM_KERNEL_PDE) *
> > -				  NUM_VMUSA_UNIT_PER_PAGE, 0);
> > +
> > +	if (ofs)
> > +		*ofs = map_ofs;
> >   }
> >   static bool xe_migrate_needs_ccs_emit(struct xe_device *xe)
> > @@ -466,7 +443,6 @@ static int xe_migrate_lock_prepare_vm(struct xe_tile *tile, struct xe_migrate *m
> >   			return err;
> >   		xe_migrate_prepare_vm(tile, m, vm, &map_ofs);
> > -		xe_migrate_suballoc_manager_init(m, map_ofs);
> >   		drm_exec_retry_on_contention(&exec);
> >   		xe_validation_retry_on_oom(&ctx, &err);
> >   	}
> > @@ -1169,6 +1145,9 @@ struct xe_lrc *xe_migrate_lrc(struct xe_migrate *migrate)
> >   	return migrate->q->lrc[0];
> >   }
> > +/* XXX: With CPU binds this can be removed in a follow up */
> > +#define NUM_KERNEL_PDE 15
> > +
> >   static u64 migrate_vm_ppgtt_addr_tlb_inval(void)
> >   {
> >   	/*
> > @@ -1788,56 +1767,6 @@ struct dma_fence *xe_migrate_clear(struct xe_migrate *m,
> >   	return fence;
> >   }
> > -static void write_pgtable(struct xe_tile *tile, struct xe_bb *bb, u64 ppgtt_ofs,
> > -			  const struct xe_vm_pgtable_update_op *pt_op,
> > -			  const struct xe_vm_pgtable_update *update,
> > -			  struct xe_migrate_pt_update *pt_update)
> > -{
> > -	const struct xe_migrate_pt_update_ops *ops = pt_update->ops;
> > -	struct xe_vm *vm = pt_update->vops->vm;
> > -	u32 chunk;
> > -	u32 ofs = update->ofs, size = update->qwords;
> > -
> > -	/*
> > -	 * If we have 512 entries (max), we would populate it ourselves,
> > -	 * and update the PDE above it to the new pointer.
> > -	 * The only time this can only happen if we have to update the top
> > -	 * PDE. This requires a BO that is almost vm->size big.
> > -	 *
> > -	 * This shouldn't be possible in practice.. might change when 16K
> > -	 * pages are used. Hence the assert.
> > -	 */
> > -	xe_tile_assert(tile, update->qwords < MAX_NUM_PTE);
> > -	if (!ppgtt_ofs)
> > -		ppgtt_ofs = xe_migrate_vram_ofs(tile_to_xe(tile),
> > -						xe_bo_addr(update->pt_bo, 0,
> > -							   XE_PAGE_SIZE), false);
> > -
> > -	do {
> > -		u64 addr = ppgtt_ofs + ofs * 8;
> > -
> > -		chunk = min(size, MAX_PTE_PER_SDI);
> > -
> > -		/* Ensure populatefn can do memset64 by aligning bb->cs */
> > -		if (!(bb->len & 1))
> > -			bb->cs[bb->len++] = MI_NOOP;
> > -
> > -		bb->cs[bb->len++] = MI_STORE_DATA_IMM | MI_SDI_NUM_QW(chunk);
> > -		bb->cs[bb->len++] = lower_32_bits(addr);
> > -		bb->cs[bb->len++] = upper_32_bits(addr);
> > -		if (pt_op->bind)
> > -			ops->populate(tile, NULL, bb->cs + bb->len,
> > -				      ofs, chunk, update);
> > -		else
> > -			ops->clear(vm, tile, NULL, bb->cs + bb->len,
> > -				   ofs, chunk, update);
> > -
> > -		bb->len += chunk * 2;
> > -		ofs += chunk;
> > -		size -= chunk;
> > -	} while (size);
> > -}
> > -
> >   struct xe_vm *xe_migrate_get_vm(struct xe_migrate *m)
> >   {
> >   	return xe_vm_get(m->q->vm);
> > @@ -1938,162 +1867,18 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
> >   {
> >   	const struct xe_migrate_pt_update_ops *ops = pt_update->ops;
> >   	struct xe_tile *tile = m->tile;
> > -	struct xe_gt *gt = tile->primary_gt;
> > -	struct xe_device *xe = tile_to_xe(tile);
> >   	struct xe_sched_job *job;
> >   	struct dma_fence *fence;
> > -	struct drm_suballoc *sa_bo = NULL;
> > -	struct xe_bb *bb;
> > -	u32 i, j, batch_size = 0, ppgtt_ofs, update_idx, page_ofs = 0;
> > -	u32 num_updates = 0, current_update = 0;
> > -	u64 addr;
> > -	int err = 0;
> >   	bool is_migrate = is_migrate_queue(m, pt_update_ops->q);
> > -	bool usm = is_migrate && xe->info.has_usm;
> > -
> > -	for (i = 0; i < pt_update_ops->num_ops; ++i) {
> > -		struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->pt_job_ops->ops[i];
> > -		struct xe_vm_pgtable_update *updates = pt_op->entries;
> > -
> > -		num_updates += pt_op->num_entries;
> > -		for (j = 0; j < pt_op->num_entries; ++j) {
> > -			u32 num_cmds = DIV_ROUND_UP(updates[j].qwords,
> > -						    MAX_PTE_PER_SDI);
> > -
> > -			/* align noop + MI_STORE_DATA_IMM cmd prefix */
> > -			batch_size += 4 * num_cmds + updates[j].qwords * 2;
> > -		}
> > -	}
> > -
> > -	/* fixed + PTE entries */
> > -	if (IS_DGFX(xe))
> > -		batch_size += 2;
> > -	else
> > -		batch_size += 6 * (num_updates / MAX_PTE_PER_SDI + 1) +
> > -			num_updates * 2;
> > -
> > -	bb = xe_bb_new(gt, batch_size, usm);
> > -	if (IS_ERR(bb))
> > -		return ERR_CAST(bb);
> > -
> > -	/* For sysmem PTE's, need to map them in our hole.. */
> > -	if (!IS_DGFX(xe)) {
> > -		u16 pat_index = xe_cache_pat_idx(xe, XE_CACHE_WB);
> > -		u32 ptes, ofs;
> > -
> > -		ppgtt_ofs = NUM_KERNEL_PDE - 1;
> > -		if (!is_migrate) {
> > -			u32 num_units = DIV_ROUND_UP(num_updates,
> > -						     NUM_VMUSA_WRITES_PER_UNIT);
> > -
> > -			if (num_units > m->vm_update_sa.size) {
> > -				err = -ENOBUFS;
> > -				goto err_bb;
> > -			}
> > -			sa_bo = drm_suballoc_new(&m->vm_update_sa, num_units,
> > -						 GFP_KERNEL, true, 0);
> > -			if (IS_ERR(sa_bo)) {
> > -				err = PTR_ERR(sa_bo);
> > -				goto err_bb;
> > -			}
> > -
> > -			ppgtt_ofs = NUM_KERNEL_PDE +
> > -				(drm_suballoc_soffset(sa_bo) /
> > -				 NUM_VMUSA_UNIT_PER_PAGE);
> > -			page_ofs = (drm_suballoc_soffset(sa_bo) %
> > -				    NUM_VMUSA_UNIT_PER_PAGE) *
> > -				VM_SA_UPDATE_UNIT_SIZE;
> > -		}
> > -
> > -		/* Map our PT's to gtt */
> > -		i = 0;
> > -		j = 0;
> > -		ptes = num_updates;
> > -		ofs = ppgtt_ofs * XE_PAGE_SIZE + page_ofs;
> > -		while (ptes) {
> > -			u32 chunk = min(MAX_PTE_PER_SDI, ptes);
> > -			u32 idx = 0;
> > -
> > -			bb->cs[bb->len++] = MI_STORE_DATA_IMM |
> > -				MI_SDI_NUM_QW(chunk);
> > -			bb->cs[bb->len++] = ofs;
> > -			bb->cs[bb->len++] = 0; /* upper_32_bits */
> > -
> > -			for (; i < pt_update_ops->num_ops; ++i) {
> > -				struct xe_vm_pgtable_update_op *pt_op =
> > -					&pt_update_ops->pt_job_ops->ops[i];
> > -				struct xe_vm_pgtable_update *updates = pt_op->entries;
> > -
> > -				for (; j < pt_op->num_entries; ++j, ++current_update, ++idx) {
> > -					struct xe_vm *vm = pt_update->vops->vm;
> > -					struct xe_bo *pt_bo = updates[j].pt_bo;
> > -
> > -					if (idx == chunk)
> > -						goto next_cmd;
> > -
> > -					xe_tile_assert(tile, xe_bo_size(pt_bo) == SZ_4K);
> > -
> > -					/* Map a PT at most once */
> > -					if (pt_bo->update_index < 0)
> > -						pt_bo->update_index = current_update;
> > -
> > -					addr = vm->pt_ops->pte_encode_bo(pt_bo, 0,
> > -									 pat_index, 0);
> > -					bb->cs[bb->len++] = lower_32_bits(addr);
> > -					bb->cs[bb->len++] = upper_32_bits(addr);
> > -				}
> > -
> > -				j = 0;
> > -			}
> > -
> > -next_cmd:
> > -			ptes -= chunk;
> > -			ofs += chunk * sizeof(u64);
> > -		}
> > -
> > -		bb->cs[bb->len++] = MI_BATCH_BUFFER_END;
> > -		update_idx = bb->len;
> > -
> > -		addr = xe_migrate_vm_addr(ppgtt_ofs, 0) +
> > -			(page_ofs / sizeof(u64)) * XE_PAGE_SIZE;
> > -		for (i = 0; i < pt_update_ops->num_ops; ++i) {
> > -			struct xe_vm_pgtable_update_op *pt_op =
> > -				&pt_update_ops->pt_job_ops->ops[i];
> > -			struct xe_vm_pgtable_update *updates = pt_op->entries;
> > -
> > -			for (j = 0; j < pt_op->num_entries; ++j) {
> > -				struct xe_bo *pt_bo = updates[j].pt_bo;
> > -
> > -				write_pgtable(tile, bb, addr +
> > -					      pt_bo->update_index * XE_PAGE_SIZE,
> > -					      pt_op, &updates[j], pt_update);
> > -			}
> > -		}
> > -	} else {
> > -		/* phys pages, no preamble required */
> > -		bb->cs[bb->len++] = MI_BATCH_BUFFER_END;
> > -		update_idx = bb->len;
> > -
> > -		for (i = 0; i < pt_update_ops->num_ops; ++i) {
> > -			struct xe_vm_pgtable_update_op *pt_op =
> > -				&pt_update_ops->pt_job_ops->ops[i];
> > -			struct xe_vm_pgtable_update *updates = pt_op->entries;
> > -
> > -			for (j = 0; j < pt_op->num_entries; ++j)
> > -				write_pgtable(tile, bb, 0, pt_op, &updates[j],
> > -					      pt_update);
> > -		}
> > -	}
> > +	int err;
> > -	job = xe_bb_create_migration_job(pt_update_ops->q, bb,
> > -					 xe_migrate_batch_base(m, usm),
> > -					 update_idx);
> > +	job = xe_sched_job_create(pt_update_ops->q, NULL);
> >   	if (IS_ERR(job)) {
> >   		err = PTR_ERR(job);
> > -		goto err_sa;
> > +		goto err_out;
> >   	}
> > -	xe_sched_job_add_migrate_flush(job, MI_INVALIDATE_TLB);
> > +	xe_tile_assert(tile, job->is_pt_job);
> >   	if (ops->pre_commit) {
> >   		pt_update->job = job;
> > @@ -2104,6 +1889,12 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
> >   	if (is_migrate)
> >   		mutex_lock(&m->job_mutex);
> > +	job->pt_update[0].vm = pt_update->vops->vm;
> > +	job->pt_update[0].tile = tile;
> > +	job->pt_update[0].ops = ops;
> > +	job->pt_update[0].pt_job_ops =
> > +		xe_pt_job_ops_get(pt_update_ops->pt_job_ops);
> > +
> >   	xe_sched_job_arm(job);
> >   	fence = dma_fence_get(&job->drm.s_fence->finished);
> >   	xe_sched_job_push(job);
> > @@ -2111,17 +1902,11 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
> >   	if (is_migrate)
> >   		mutex_unlock(&m->job_mutex);
> > -	xe_bb_free(bb, fence);
> > -	drm_suballoc_free(sa_bo, fence);
> > -
> >   	return fence;
> >   err_job:
> >   	xe_sched_job_put(job);
> > -err_sa:
> > -	drm_suballoc_free(sa_bo, NULL);
> > -err_bb:
> > -	xe_bb_free(bb, NULL);
> > +err_out:
> >   	return ERR_PTR(err);
> >   }
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index 16126ffc2ec1..24190ba4f533 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -387,7 +387,6 @@ xe_pt_new_shared(struct xe_walk_update *wupd, struct xe_pt *parent,
> >   	entry->pt = parent;
> >   	entry->flags = 0;
> >   	entry->qwords = 0;
> > -	entry->pt_bo->update_index = -1;
> 
> Patch LGTM
> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> 
> Sashiko's comment regarding "Memory leak of `xe_pt_job_ops` and `job->fence"
> needs to be handled in
> https://patchwork.freedesktop.org/patch/751036/?series=149888&rev=8

No, once we call _arm(), run_job() is always called. Sashiko doesn't
seem to understand this - it probably because the scheduler doesn't make
this guarnetee but Xe usage of the scheduler ensure this.

Matt

> 
> 
> >   	entry->level = parent->level;
> >   	if (alloc_entries) {
>