[Intel-gfx] [PATCH] drm/i915: use i915_sg_dma_sizes() for internal backend

[Summers, Stuart]

On Thu, 2022-10-20 at 16:10 +0100, Matthew Auld wrote:
> We rely on page_sizes.sg in setup_scratch_page() reporting the
> correct
> value if the underlying sgl is not contiguous, however in
> get_pages_internal() we are only looking at the layout of the created
> pages when calculating the sg_page_sizes, and not the final sgl,
> which
> could in theory be completely different. In such a situation we might
> incorrectly think we have a 64K scratch page, when it is actually
> only
> 4K or similar split over multiple non-contiguous entries, which could
> lead to broken behaviour when touching the scratch space within the
> padding of a 64K GTT page-table.  Like we already do for other
> backends,
> switch over to calling i915_sg_dma_sizes() after mapping the pages.

Makes sense to me. Is there a bug tracking this though? If so please
add here.

Otherwise:
Reviewed-by: Stuart Summers <stuart.summers at intel.com>

> 
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Andrzej Hajda <andrzej.hajda at intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_internal.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> index c698f95af15f..301cfb127c4c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> @@ -36,7 +36,6 @@ static int
> i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj)
>  	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>  	struct sg_table *st;
>  	struct scatterlist *sg;
> -	unsigned int sg_page_sizes;
>  	unsigned int npages;
>  	int max_order;
>  	gfp_t gfp;
> @@ -75,7 +74,6 @@ static int
> i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj)
>  
>  	sg = st->sgl;
>  	st->nents = 0;
> -	sg_page_sizes = 0;
>  
>  	do {
>  		int order = min(fls(npages) - 1, max_order);
> @@ -94,7 +92,6 @@ static int
> i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj)
>  		} while (1);
>  
>  		sg_set_page(sg, page, PAGE_SIZE << order, 0);
> -		sg_page_sizes |= PAGE_SIZE << order;
>  		st->nents++;
>  
>  		npages -= 1 << order;
> @@ -116,7 +113,7 @@ static int
> i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj)
>  		goto err;
>  	}
>  
> -	__i915_gem_object_set_pages(obj, st, sg_page_sizes);
> +	__i915_gem_object_set_pages(obj, st, i915_sg_dma_sizes(st-
> >sgl));
>  
>  	return 0;
>