[Intel-xe] [PATCH] xe/xe_bo: Reject bo creation of unaligned size

[Thomas Hellström]

+Maarten

Hi, Zbigniew, Mauro

On 4/13/23 08:56, Zbigniew Kempczyński wrote:
> From: Mauro Carvalho Chehab <mauro.chehab at linux.intel.com>
>
> For xe bo creation we request passing size which matches system or
> vram minimum page alignment. This way we want to ensure userspace
> is aware of region constraints and not aligned allocations will be
> rejected returning EINVAL.

Personally, I think the alignment constraints is a property / 
restriction of the Xe GPU binding that should not interfere with bo 
creation / size that should instead reflect the size of the data we need 
to store. So the alignment constraints should apply only to the GPU VA 
space allocation. This would also make the semantics of importing 
dma-bufs of unaligned size straightforward.

Although admittedly I'm not fully up-to-date with the discussion 
preceding the choice to return -EINVAL if the bo size is not aligned to 
our GPU binding constraints. (Maarten, could you perhaps fill in?)

Thanks,

Thomas




>
> Signed-off-by: Mauro Carvalho Chehab <mauro.chehab at linux.intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> ---
>   drivers/gpu/drm/xe/xe_bo.c | 23 ++++++++++++++++-------
>   include/uapi/drm/xe_drm.h  |  5 ++---
>   2 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 3ab404e33fae..d3972c84bcce 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1090,6 +1090,7 @@ struct xe_bo *__xe_bo_create_locked(struct xe_device *xe, struct xe_bo *bo,
>   	};
>   	struct ttm_placement *placement;
>   	uint32_t alignment;
> +	size_t aligned_size;
>   	int err;
>   
>   	/* Only kernel objects should set GT */
> @@ -1098,22 +1099,30 @@ struct xe_bo *__xe_bo_create_locked(struct xe_device *xe, struct xe_bo *bo,
>   	if (XE_WARN_ON(!size))
>   		return ERR_PTR(-EINVAL);
>   
> -	if (!bo) {
> -		bo = xe_bo_alloc();
> -		if (IS_ERR(bo))
> -			return bo;
> -	}
> -
>   	if (flags & (XE_BO_CREATE_VRAM_MASK | XE_BO_CREATE_STOLEN_BIT) &&
>   	    !(flags & XE_BO_CREATE_IGNORE_MIN_PAGE_SIZE_BIT) &&
>   	    xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K) {
> -		size = ALIGN(size, SZ_64K);
> +		aligned_size = ALIGN(size, SZ_64K);
> +		if (type != ttm_bo_type_device)
> +			size = ALIGN(size, SZ_64K);
>   		flags |= XE_BO_INTERNAL_64K;
>   		alignment = SZ_64K >> PAGE_SHIFT;
> +
>   	} else {
> +		aligned_size = ALIGN(size, SZ_4K);
> +		flags &= ~XE_BO_INTERNAL_64K;
>   		alignment = SZ_4K >> PAGE_SHIFT;
>   	}
>   
> +	if (type == ttm_bo_type_device && aligned_size != size)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!bo) {
> +		bo = xe_bo_alloc();
> +		if (IS_ERR(bo))
> +			return bo;
> +	}
> +
>   	bo->gt = gt;
>   	bo->size = size;
>   	bo->flags = flags;
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index b0b80aae3ee8..d3e64af7eabb 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -259,9 +259,8 @@ struct drm_xe_gem_create {
>   	__u64 extensions;
>   
>   	/**
> -	 * @size: Requested size for the object
> -	 *
> -	 * The (page-aligned) allocated size for the object will be returned.
> +	 * @size: Size of the object to be created, must match region
> +	 * (system or vram) minimum alignment.
>   	 */
>   	__u64 size;
>