[PATCH v4 15/30] drm/xe: Add xe_vm_pgtable_update_op to xe_vma_ops
Matthew Brost
matthew.brost at intel.com
Tue Mar 26 19:05:32 UTC 2024
On Mon, Mar 25, 2024 at 03:58:10PM -0600, Zeng, Oak wrote:
>
>
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Matthew
> > Brost
> > Sent: Friday, March 8, 2024 12:08 AM
> > To: intel-xe at lists.freedesktop.org
> > Cc: Brost, Matthew <matthew.brost at intel.com>
> > Subject: [PATCH v4 15/30] drm/xe: Add xe_vm_pgtable_update_op to
> > xe_vma_ops
> >
> > Will help with the converstion to 1 job per VM bind IOCTL. Allocation
> > only implemented in this patch.
>
>
> Can you explain why you need to introduce pt_update_op? this patch seems only allocate some slot for pt_update_op but they are not really used for page table update. I will keep looking the rest of this series.
>
> I also don't get why this helps 1job/bind ioctl.
>
Each VMA operations is converted into 0-3 PT per ops tile. i.e. a MAP VMA
operations with immediate clear generates 0 PT ops, a REMAP VMA with
both prev & next generates 3 PT ops. PT ops work on the internal / GPU
PT state compared to VMA ops which operate on the internal GPUVM state.
The flow roughly is:
generate VMA ops from IOCTL input
convert all VMA ops to PT ops, prep and stage PT ops
run PT ops as an atomic unit (1 job)
commit PT ops
commit VMA ops
If at any point a failure occurs before the commit step the PT ops can
be unwound.
Likewise when a failure occurs the VMA ops are unwound too, this is just
layer above the PT ops.
In the end if an errors at point in the IOCTL the original GPUVM and PT
state is restored. This compared to serial execution on tip where is an
error occurs we just kill the VM.
Does this make sense?
Matt
>
> Oak
>
> >
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_pt_types.h | 12 ++++++
> > drivers/gpu/drm/xe/xe_vm.c | 66 +++++++++++++++++++++++++++++---
> > drivers/gpu/drm/xe/xe_vm_types.h | 8 ++++
> > 3 files changed, 81 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_pt_types.h
> > b/drivers/gpu/drm/xe/xe_pt_types.h
> > index cee70cb0f014..2093150f461e 100644
> > --- a/drivers/gpu/drm/xe/xe_pt_types.h
> > +++ b/drivers/gpu/drm/xe/xe_pt_types.h
> > @@ -74,4 +74,16 @@ struct xe_vm_pgtable_update {
> > u32 flags;
> > };
> >
> > +/** struct xe_vm_pgtable_update_op - Page table update operation */
> > +struct xe_vm_pgtable_update_op {
> > + /** @entries: entries to update for this operation */
> > + struct xe_vm_pgtable_update entries[XE_VM_MAX_LEVEL * 2 + 1];
> > + /** @num_entries: number of entries for this update operation */
> > + u32 num_entries;
> > + /** @bind: is a bind */
> > + bool bind;
> > + /** @rebind: is a rebind */
> > + bool rebind;
> > +};
> > +
> > #endif
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index 5b93c71fc5e9..72e9bdab79d5 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -1319,6 +1319,42 @@ static void xe_vma_ops_init(struct xe_vma_ops *vops,
> > struct xe_vm *vm,
> > vops->num_syncs = num_syncs;
> > }
> >
> > +static int xe_vma_ops_alloc(struct xe_vma_ops *vops)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < XE_MAX_TILES_PER_DEVICE; ++i) {
> > + if (!vops->pt_update_ops[i].num_ops)
> > + continue;
> > +
> > + vops->pt_update_ops[i].ops =
> > + kmalloc_array(vops->pt_update_ops[i].num_ops,
> > + sizeof(*vops->pt_update_ops[i].ops),
> > + GFP_KERNEL);
> > + if (!vops->pt_update_ops[i].ops)
> > + return -ENOMEM;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void xe_vma_ops_fini(struct xe_vma_ops *vops)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < XE_MAX_TILES_PER_DEVICE; ++i)
> > + kfree(vops->pt_update_ops[i].ops);
> > +}
> > +
> > +static void xe_vma_ops_incr_pt_update_ops(struct xe_vma_ops *vops, u8
> > tile_mask)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < XE_MAX_TILES_PER_DEVICE; ++i)
> > + if (BIT(i) & tile_mask)
> > + ++vops->pt_update_ops[i].num_ops;
> > +}
> > +
> > struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags)
> > {
> > struct drm_gem_object *vm_resv_obj;
> > @@ -1343,6 +1379,11 @@ struct xe_vm *xe_vm_create(struct xe_device *xe,
> > u32 flags)
> > xe_vma_ops_init(&vm->dummy_ops.vops, vm, NULL, NULL, 0);
> > INIT_LIST_HEAD(&vm->dummy_ops.op.link);
> > list_add(&vm->dummy_ops.op.link, &vm->dummy_ops.vops.list);
> > + for (id = 0; id < XE_MAX_TILES_PER_DEVICE; ++id)
> > + vm->dummy_ops.vops.pt_update_ops[id].num_ops = 1;
> > + err = xe_vma_ops_alloc(&vm->dummy_ops.vops);
> > + if (err)
> > + goto err_free;
> >
> > INIT_LIST_HEAD(&vm->rebind_list);
> >
> > @@ -1468,12 +1509,14 @@ struct xe_vm *xe_vm_create(struct xe_device *xe,
> > u32 flags)
> > return ERR_PTR(err);
> >
> > err_no_resv:
> > - mutex_destroy(&vm->snap_mutex);
> > + if (!(flags & XE_VM_FLAG_MIGRATION))
> > + xe_device_mem_access_put(xe);
> > for_each_tile(tile, xe, id)
> > xe_range_fence_tree_fini(&vm->rftree[id]);
> > +err_free:
> > + mutex_destroy(&vm->snap_mutex);
> > + xe_vma_ops_fini(&vm->dummy_ops.vops);
> > kfree(vm);
> > - if (!(flags & XE_VM_FLAG_MIGRATION))
> > - xe_device_mem_access_put(xe);
> > return ERR_PTR(err);
> > }
> >
> > @@ -1611,6 +1654,7 @@ static void vm_destroy_work_func(struct work_struct
> > *w)
> >
> > trace_xe_vm_free(vm);
> > dma_fence_put(vm->rebind_fence);
> > + xe_vma_ops_fini(&vm->dummy_ops.vops);
> > kfree(vm);
> > }
> >
> > @@ -2284,7 +2328,6 @@ static int xe_vma_op_commit(struct xe_vm *vm, struct
> > xe_vma_op *op)
> > return err;
> > }
> >
> > -
> > static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_exec_queue *q,
> > struct drm_gpuva_ops *ops,
> > struct xe_sync_entry *syncs, u32 num_syncs,
> > @@ -2334,6 +2377,9 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm,
> > struct xe_exec_queue *q,
> > return PTR_ERR(vma);
> >
> > op->map.vma = vma;
> > + if (op->map.immediate || !xe_vm_in_fault_mode(vm))
> > + xe_vma_ops_incr_pt_update_ops(vops,
> > + op->tile_mask);
> > break;
> > }
> > case DRM_GPUVA_OP_REMAP:
> > @@ -2378,6 +2424,8 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm,
> > struct xe_exec_queue *q,
> > vm_dbg(&xe->drm, "REMAP:SKIP_PREV:
> > addr=0x%016llx, range=0x%016llx",
> > (ULL)op->remap.start,
> > (ULL)op->remap.range);
> > + } else {
> > + xe_vma_ops_incr_pt_update_ops(vops,
> > op->tile_mask);
> > }
> > }
> >
> > @@ -2414,13 +2462,16 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm
> > *vm, struct xe_exec_queue *q,
> > vm_dbg(&xe->drm, "REMAP:SKIP_NEXT:
> > addr=0x%016llx, range=0x%016llx",
> > (ULL)op->remap.start,
> > (ULL)op->remap.range);
> > + } else {
> > + xe_vma_ops_incr_pt_update_ops(vops,
> > op->tile_mask);
> > }
> > }
> > + xe_vma_ops_incr_pt_update_ops(vops, op->tile_mask);
> > break;
> > }
> > case DRM_GPUVA_OP_UNMAP:
> > case DRM_GPUVA_OP_PREFETCH:
> > - /* Nothing to do */
> > + xe_vma_ops_incr_pt_update_ops(vops, op->tile_mask);
> > break;
> > default:
> > drm_warn(&vm->xe->drm, "NOT POSSIBLE");
> > @@ -3243,11 +3294,16 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void
> > *data, struct drm_file *file)
> > goto unwind_ops;
> > }
> >
> > + err = xe_vma_ops_alloc(&vops);
> > + if (err)
> > + goto unwind_ops;
> > +
> > err = vm_bind_ioctl_ops_execute(vm, &vops);
> >
> > unwind_ops:
> > if (err && err != -ENODATA)
> > vm_bind_ioctl_ops_unwind(vm, ops, args->num_binds);
> > + xe_vma_ops_fini(&vops);
> > for (i = args->num_binds - 1; i >= 0; --i)
> > if (ops[i])
> > drm_gpuva_ops_free(&vm->gpuvm, ops[i]);
> > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h
> > b/drivers/gpu/drm/xe/xe_vm_types.h
> > index f6ea4df64e79..f5d740dcbba3 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_types.h
> > +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> > @@ -22,6 +22,7 @@ struct xe_device;
> > struct xe_sync_entry;
> > struct xe_user_fence;
> > struct xe_vm;
> > +struct xe_vm_pgtable_update_op;
> >
> > #define XE_VMA_READ_ONLY DRM_GPUVA_USERBITS
> > #define XE_VMA_DESTROYED (DRM_GPUVA_USERBITS << 1)
> > @@ -219,6 +220,13 @@ struct xe_vma_ops {
> > struct xe_sync_entry *syncs;
> > /** @num_syncs: number of syncs */
> > u32 num_syncs;
> > + /** @pt_update_ops: page table update operations */
> > + struct {
> > + /** @ops: operations */
> > + struct xe_vm_pgtable_update_op *ops;
> > + /** @num_ops: number of operations */
> > + u32 num_ops;
> > + } pt_update_ops[XE_MAX_TILES_PER_DEVICE];
> > };
> >
> > struct xe_vm {
> > --
> > 2.34.1
>
More information about the Intel-xe
mailing list