[PATCH i-g-t 7/8] lib/intel_bufops: keep bo_size separate

[Zbigniew Kempczyński]

On Fri, Feb 09, 2024 at 06:34:32PM +0000, Matthew Auld wrote:
> If the caller provides some bo_size, like for an existing object handle,
> then we currently trample the buf->size here, which seems quite
> inconsistent. Rather treat bo_size and size separate, just ensuring that
> bo_size is large enough.
> 
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> ---
>  lib/intel_bufops.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
> index 7f72f4762..2ef2c1c70 100644
> --- a/lib/intel_bufops.c
> +++ b/lib/intel_bufops.c
> @@ -903,11 +903,6 @@ static void __intel_buf_init(struct buf_ops *bops,
>  		size = buf->surface[0].stride * ALIGN(height, align_h);
>  	}
>  
> -	if (bo_size > 0) {
> -		igt_assert(bo_size >= size);
> -		size = bo_size;
> -	}
> -
>  	/* Store buffer size to avoid mistakes in calculating it again */
>  	buf->size = size;
>  	buf->handle = handle;
> @@ -918,17 +913,22 @@ static void __intel_buf_init(struct buf_ops *bops,
>  	buf->region = region;
>  
>  	if (!handle) {
> +		if (!bo_size)
> +			bo_size = size;
> +
>  		if (bops->driver == INTEL_DRIVER_I915) {
> -			if (__gem_create_in_memory_regions(bops->fd, &buf->handle, &size, region))
> -				igt_assert_eq(__gem_create(bops->fd, &size, &buf->handle), 0);
> +			if (__gem_create_in_memory_regions(bops->fd, &buf->handle, &bo_size, region))
> +				igt_assert_eq(__gem_create(bops->fd, &bo_size, &buf->handle), 0);
>  		} else {
> -			size = ALIGN(size, xe_get_default_alignment(bops->fd));
> -			buf->handle = xe_bo_create(bops->fd, 0, size, region, 0);
> +			bo_size = ALIGN(bo_size, xe_get_default_alignment(bops->fd));
> +			buf->handle = xe_bo_create(bops->fd, 0, bo_size, region, 0);
>  		}
> +	} else {
> +		igt_assert(bo_size >= size);

I think this assertion should be outside and cover all cases.
If user will pass handle and due to bug some positive bo_size
we still can catch situation where according to size calculation
it will be bigger than bo_size what's wrong.

With this minor nits above:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

--
Zbigniew

>  	}
>  
>  	/* Store gem bo size */
> -	buf->bo_size = size;
> +	buf->bo_size = bo_size;
>  
>  	if (bops->driver == INTEL_DRIVER_I915)
>  		set_hw_tiled(bops, buf);
> -- 
> 2.43.0
>