[Intel-xe] [PATCH v2] drm/xe: Disallow pinning dma-bufs in VRAM

[Thomas Hellström]

On 9/20/23 11:50, Thomas Hellström wrote:
> For now only support pinning in TT memory, for two reasons:
> 1) Avoid pinning in a placement not accessible to some importers.
> 2) Pinning in VRAM requires PIN accounting which is a to-do.
>
> v2:
> - Adjust the dma-buf kunit test accordingly.
>
> Suggested-by: Oded Gabbay <ogabbay at kernel.org>
> Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Reviewed-by: Oded Gabbay <ogabbay at kernel.org>
> ---
>   drivers/gpu/drm/xe/tests/xe_dma_buf.c | 16 ++++++++++++----
>   drivers/gpu/drm/xe/xe_dma_buf.c       | 25 +++++++++++++++++++++----
>   2 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/tests/xe_dma_buf.c b/drivers/gpu/drm/xe/tests/xe_dma_buf.c
> index 1c3f4bc72b99..ee911df94cc9 100644
> --- a/drivers/gpu/drm/xe/tests/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/tests/xe_dma_buf.c
> @@ -150,11 +150,19 @@ static void xe_test_dmabuf_import_same_driver(struct xe_device *xe)
>   			/* Is everything where we expect it to be? */
>   			xe_bo_lock(import_bo, false);
>   			err = xe_bo_validate(import_bo, NULL, false);
> -			if (err && err != -EINTR && err != -ERESTARTSYS)
> -				KUNIT_FAIL(test,
> -					   "xe_bo_validate() failed with err=%d\n", err);
>   
> -			check_residency(test, bo, import_bo, dmabuf);
> +			/* Pinning in VRAM is not allowed. */
> +			if (!is_dynamic(params) &&
> +			    params->force_different_devices &&
> +			    !(params->mem_mask & XE_BO_CREATE_SYSTEM_BIT))
> +				KUNIT_EXPECT_EQ(test, err, -EINVAL);
> +			/* Otherwise only expect interrupts or success. */
> +			else if (err && err != -EINTR && err != -ERESTARTSYS)

Ugh. That else if should have been an else. Only. That got somehow lost. 
Will post a fixup commit for that. Not that it changes the behaviour of 
the test but the above looks really odd combined with the below.

/Thomas


> +				KUNIT_EXPECT_TRUE(test, !err || err == -EINTR ||
> +						  err == -ERESTARTSYS);
> +
> +			if (!err)
> +				check_residency(test, bo, import_bo, dmabuf);
>   			xe_bo_unlock(import_bo);
>   		}
>   		drm_gem_object_put(import);
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
> index 09343b8b3e96..bd8d51e7f93f 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -49,13 +49,30 @@ static int xe_dma_buf_pin(struct dma_buf_attachment *attach)
>   {
>   	struct drm_gem_object *obj = attach->dmabuf->priv;
>   	struct xe_bo *bo = gem_to_xe_bo(obj);
> +	struct xe_device *xe = xe_bo_device(bo);
> +	int ret;
>   
>   	/*
> -	 * Migrate to TT first to increase the chance of non-p2p clients
> -	 * can attach.
> +	 * For now only support pinning in TT memory, for two reasons:
> +	 * 1) Avoid pinning in a placement not accessible to some importers.
> +	 * 2) Pinning in VRAM requires PIN accounting which is a to-do.
>   	 */
> -	(void)xe_bo_migrate(bo, XE_PL_TT);
> -	xe_bo_pin_external(bo);
> +	if (xe_bo_is_pinned(bo) && bo->ttm.resource->placement != XE_PL_TT) {
> +		drm_dbg(&xe->drm, "Can't migrate pinned bo for dma-buf pin.\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = xe_bo_migrate(bo, XE_PL_TT);
> +	if (ret) {
> +		if (ret != -EINTR && ret != -ERESTARTSYS)
> +			drm_dbg(&xe->drm,
> +				"Failed migrating dma-buf to TT memory: %pe\n",
> +				ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	ret = xe_bo_pin_external(bo);
> +	xe_assert(xe, !ret);
>   
>   	return 0;
>   }