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 <[email protected]> > > Signed-off-by: Matthew Auld <[email protected]> > Cc: Andrzej Hajda <[email protected]> > --- > 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; >
