[Intel-xe] [PATCH] drm/xe/bo: Return object bo size on create

[Maarten Lankhorst]

On 2023-04-03 18:48, Mauro Carvalho Chehab wrote:
> On Tue, 28 Mar 2023 15:08:26 +0200
> Zbigniew Kempczyński <zbigniew.kempczynski at intel.com> wrote:
>
>> Driver may alter bo size requested by the user. Return real object
>> size to make userspace aware how to arrange vm bindings.
>>
>> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_bo.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>> index e4d079b61d52..410e797931ac 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.c
>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>> @@ -1564,6 +1564,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>>   		return err;
>>   
>>   	args->handle = handle;
>> +	args->size = bo->size;
>>   
>>   	return 0;
>>   }
> An alternative approach would be instead to return -EINVAL if the
> buffers aren't aligned. Something like this (untested) patch.
>
> Regards,
> Mauro
>
> [PATCH] xe/xe_bo: Don't allow creating unaligned buffers
>
> Ensure that bo will always be properly aligned.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab at kernel.org>
>
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index f25257bbfc65..ecc04b887790 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -985,6 +985,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 */
> @@ -993,24 +994,28 @@ 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;
> -	}
> -
> -	bo->requested_size = size;
>   	if (flags & (XE_BO_CREATE_VRAM0_BIT | XE_BO_CREATE_VRAM1_BIT |
>   		     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);
>   		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 (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;

Hey,

This looks much better. Can we also make sure that if the kernel tries 
to allocate a 4k buffer we get a louder warning?

~Maarten

> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index a430d1568890..f8a55ef9e789 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -273,10 +273,8 @@ void xe_ggtt_map_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>   		xe_ggtt_set_pte(ggtt, start + offset, pte);
>   	}
>   
> -	if (bo->size == bo->requested_size) {
> -		pte = xe_ggtt_pte_encode(ggtt->scratch ?: bo, 0);
> -		xe_ggtt_set_pte(ggtt, start + bo->size, pte);
> -	}
> +	pte = xe_ggtt_pte_encode(ggtt->scratch ?: bo, 0);
> +	xe_ggtt_set_pte(ggtt, start + bo->size, pte);
>   
>   	if (ggtt->invalidate) {
>   		xe_ggtt_invalidate(ggtt->gt);
> @@ -309,11 +307,9 @@ static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
>   	 * fact we have programmed this GGTT entry to a valid entry. BO aligned
>   	 * to 64k already have padding so no need to add an extra page.
>   	 */
> -	if (bo->size == bo->requested_size) {
> -		size += SZ_4K;
> -		if (end != U64_MAX)
> -			end += SZ_4K;
> -	}
> +        size += alignment;
> +        if (end != U64_MAX)
> +                end += alignment;