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

[Matthew Auld]

On 23/11/2023 15:31, 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).
> 
> 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>
> ---
>   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);

err != 0 ? I assume wrap here would be a bug?

Reviewed-by: Matthew Auld <matthew.auld at intel.com>

And r-b remains for the rest of the series.

> +		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;
>   	}
>