[Intel-xe] [PATCH v3 4/4] drm/xe/vm: Fix ASID XA usage

[Thomas Hellström]

On Fri, 2023-11-24 at 11:11 +0000, Matthew Auld wrote:
> On 24/11/2023 09:11, Thomas Hellström wrote:
> > xa_alloc_cyclic() returns 1 on successful allocation, if wrapping
> > occurs,
> > but the code incorrectly treats that as an error. Fix that.
> > Also, xa_alloc_cyclic() requires xa_init_flags(...,
> > XA_FLAGS_ALLOC), so
> > fix that, and assuming we don't want a zero ASID, instead of using
> > XA_FLAGS_ALLOC1, adjust the xa limits at alloc_cyclic time.
> > 
> > v2:
> > - On CONFIG_DRM_XE_DEBUG, Initialize the cyclic ASID allocation in
> > such a
> >    way that the next allocated ASID will be the maximum one, and
> > the one
> >    following will cause an ASID wrap, (all to have CI test high
> > ASIDs
> >    and ASID wraps).
> > v3:
> > - Stricter return value checking from xa_alloc_cyclic() (Matthew
> > Auld)
> 
> A missing git add? :)

Oh, how did that happen :/ 

Thanks,

I'll send v4.
/Thomas


> 
> > 
> > Suggested-by: Ohad Sharabi <osharabi at habana.ai>
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/946
> > Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> > Reviewed-by: Ohad Sharabi <osharabi at habana.ai> #v1
> > Reviewed-by: Matthew Auld <matthew.auld at intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_device.c | 15 ++++++++++++++-
> >   drivers/gpu/drm/xe/xe_vm.c     |  5 +++--
> >   2 files changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device.c
> > b/drivers/gpu/drm/xe/xe_device.c
> > index 8be765adf702..9e5260422f6d 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -230,7 +230,20 @@ struct xe_device *xe_device_create(struct
> > pci_dev *pdev,
> >         init_waitqueue_head(&xe->ufence_wq);
> >   
> >         drmm_mutex_init(&xe->drm, &xe->usm.lock);
> > -       xa_init_flags(&xe->usm.asid_to_vm, XA_FLAGS_ALLOC1);
> > +       xa_init_flags(&xe->usm.asid_to_vm, XA_FLAGS_ALLOC);
> > +
> > +       if (IS_ENABLED(CONFIG_DRM_XE_DEBUG)) {
> > +               /* Trigger a large asid and an early asid wrap. */
> > +               u32 asid;
> > +
> > +               BUILD_BUG_ON(XE_MAX_ASID < 2);
> > +               err = xa_alloc_cyclic(&xe->usm.asid_to_vm, &asid,
> > NULL,
> > +                                     XA_LIMIT(XE_MAX_ASID - 2,
> > XE_MAX_ASID - 1),
> > +                                     &xe->usm.next_asid,
> > GFP_KERNEL);
> > +               drm_WARN_ON(&xe->drm, err < 0);
> > +               if (err >= 0)
> > +                       xa_erase(&xe->usm.asid_to_vm, asid);
> > +       }
> >   
> >         drmm_mutex_init(&xe->drm, &xe->persistent_engines.lock);
> >         INIT_LIST_HEAD(&xe->persistent_engines.list);
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c
> > b/drivers/gpu/drm/xe/xe_vm.c
> > index b39c6f43f01f..85b9d6f864ec 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -1994,13 +1994,14 @@ int xe_vm_create_ioctl(struct drm_device
> > *dev, void *data,
> >         if (xe->info.has_asid) {
> >                 mutex_lock(&xe->usm.lock);
> >                 err = xa_alloc_cyclic(&xe->usm.asid_to_vm, &asid,
> > vm,
> > -                                     XA_LIMIT(0, XE_MAX_ASID - 1),
> > +                                     XA_LIMIT(1, XE_MAX_ASID - 1),
> >                                       &xe->usm.next_asid,
> > GFP_KERNEL);
> >                 mutex_unlock(&xe->usm.lock);
> > -               if (err) {
> > +               if (err < 0) {
> >                         xe_vm_close_and_put(vm);
> >                         return err;
> >                 }
> > +               err = 0;
> >                 vm->usm.asid = asid;
> >         }
> >