[Intel-xe] [PATCH] drm/xe: Enable scratch page when page fault is enabled
Summers, Stuart
stuart.summers at intel.com
Thu Nov 2 21:16:31 UTC 2023
On Thu, 2023-08-31 at 17:31 -0700, Welty, Brian wrote:
>
>
> On 8/30/2023 2:34 PM, Chang, Bruce wrote:
> > The PVC HW has a limitation that the page fault due to invalid
> > access
> > will halt the corresponding EUs. So, in order to activate the
> > debugger,
> > kmd needs to setup the scratch pages to unhalt the EUs.
> >
> > This feature can only be enabled if scratch flag is set per VM. So,
> > once
> > EU debugger is running, the debugger umd will set the scratch flag,
> > otherwise, this flag should not be set. So, in regular run, this
> > feature
> > will not be activated.
> >
> > The idea is to bind a scratch vma if the page fault is from an
> > invalid access. This patch is taking advantage of null pte.
> > After the bind, the user app can continue to run without causing a
> > fatal failure or reset and stop.
> >
> > In case the app will bind this scratch vma to a valid address,
> > GPUVA
> > handles all of this (e.g. it will create ops to unbind the old
> > VMA, bind the new one).
> >
> > This patch only kicks in when there is a failure for both page
> > fault
> > and bind, so it should have no impact to regular code path. On
> > another hand, it uses actual page tables instead of special scratch
> > page tables, so it may not require to invalidate TLBs when doing
> > unbind if all upper layer page tables are still being used.
> >
> > tested on new scratch igt tests which will be sent out for review.
> >
> > v2: per Matt's suggestion, remove the scratch page unbind.
> > v3: correct error handlings.
>
> Looks good to me.
> Reviewed-by: Brian Welty <brian.welty at intel.com>
>
> I know Stuart had some mis-givings about merging ahead of EU debugger
> support. But you are not adding the scratch feature per se, it's
> already in the uAPI. You are just extending it to support fault
> mode.
> Patch is small, so to me, there's enough justification to go ahead
> and get this merged now.
Really sorry I missed your reply until now Brian!
My main concern here is adding something that won't be used right away.
Ideally the EU debug feature should available when we merge this.
I know that is actively being worked though so I'm ok with just having
a quick note in the commit message.
Acked-by: Stuart Summers <stuart.summers at intel.com>
>
>
> >
> > Cc: Oak Zeng <oak.zeng at intel.com>
> > Cc: Brian Welty <brian.welty at intel.com>
> > Cc: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
> > Cc: Stuart Summers <stuart.summers at intel.com>
> > Cc: Matthew Brost <matthew.brost at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_gt_pagefault.c | 9 +++++++--
> > drivers/gpu/drm/xe/xe_vm.c | 30
> > +++++++++++++++++++++++-----
> > drivers/gpu/drm/xe/xe_vm.h | 2 ++
> > 3 files changed, 34 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > index b6f781b3d9d7..bdd84b109e9e 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > @@ -137,8 +137,13 @@ static int handle_pagefault(struct xe_gt *gt,
> > struct pagefault *pf)
> > write_locked = true;
> > vma = lookup_vma(vm, pf->page_addr);
> > if (!vma) {
> > - ret = -EINVAL;
> > - goto unlock_vm;
> > + if (vm->flags & XE_VM_FLAG_SCRATCH_PAGE)
> > + vma = xe_vm_create_scratch_vma(vm, pf-
> > >page_addr);
> > +
> > + if (IS_ERR_OR_NULL(vma)) {
> > + ret = -EINVAL;
> > + goto unlock_vm;
> > + }
> > }
> >
> > if (!xe_vma_is_userptr(vma) ||
> > !xe_vma_userptr_check_repin(vma)) {
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c
> > b/drivers/gpu/drm/xe/xe_vm.c
> > index 389ac5ba8ddf..58e0309556a0 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -1262,7 +1262,8 @@ struct xe_vm *xe_vm_create(struct xe_device
> > *xe, u32 flags)
> > }
> > }
> >
> > - if (flags & XE_VM_FLAG_SCRATCH_PAGE) {
> > + if (flags & XE_VM_FLAG_SCRATCH_PAGE &&
> > + (!(flags & XE_VM_FLAG_FAULT_MODE))) {
> > for_each_tile(tile, xe, id) {
> > if (!vm->pt_root[id])
> > continue;
> > @@ -1998,10 +1999,6 @@ int xe_vm_create_ioctl(struct drm_device
> > *dev, void *data,
> > if (XE_IOCTL_DBG(xe, args->flags &
> > ~ALL_DRM_XE_VM_CREATE_FLAGS))
> > return -EINVAL;
> >
> > - if (XE_IOCTL_DBG(xe, args->flags &
> > DRM_XE_VM_CREATE_SCRATCH_PAGE &&
> > - args->flags &
> > DRM_XE_VM_CREATE_FAULT_MODE))
> > - return -EINVAL;
> > -
> > if (XE_IOCTL_DBG(xe, args->flags &
> > DRM_XE_VM_CREATE_COMPUTE_MODE &&
> > args->flags &
> > DRM_XE_VM_CREATE_FAULT_MODE))
> > return -EINVAL;
> > @@ -2783,6 +2780,29 @@ static int __xe_vma_op_execute(struct xe_vm
> > *vm, struct xe_vma *vma,
> > return err;
> > }
> >
> > +struct xe_vma *xe_vm_create_scratch_vma(struct xe_vm *vm, u64
> > addr)
> > +{
> > + struct xe_vma *vma;
> > + int err;
> > +
> > + if (xe_vm_is_closed_or_banned(vm))
> > + return ERR_PTR(-ENOENT);
> > +
> > + vma = xe_vma_create(vm, NULL, 0, addr, addr + SZ_64K - 1,
> > false, true, 0);
> > + if (IS_ERR_OR_NULL(vma))
> > + return vma;
> > +
> > + err = xe_vm_insert_vma(vm, vma);
> > + if (err) {
> > + xe_vma_destroy_late(vma);
> > + return ERR_PTR(err);
> > + }
> > +
> > + /* fault will handle the bind */
> > +
> > + return vma;
> > +}
> > +
> > static int xe_vma_op_execute(struct xe_vm *vm, struct xe_vma_op
> > *op)
> > {
> > int ret = 0;
> > diff --git a/drivers/gpu/drm/xe/xe_vm.h
> > b/drivers/gpu/drm/xe/xe_vm.h
> > index 6de6e3edb24a..ddd387333cd2 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.h
> > +++ b/drivers/gpu/drm/xe/xe_vm.h
> > @@ -212,6 +212,8 @@ int xe_vma_userptr_pin_pages(struct xe_vma
> > *vma);
> >
> > int xe_vma_userptr_check_repin(struct xe_vma *vma);
> >
> > +struct xe_vma *xe_vm_create_scratch_vma(struct xe_vm *vm, u64
> > addr);
> > +
> > /*
> > * XE_ONSTACK_TV is used to size the tv_onstack array that is
> > input
> > * to xe_vm_lock_dma_resv() and xe_vm_unlock_dma_resv().
More information about the Intel-xe
mailing list