Hi Krzysztof,
On Mon, Nov 18, 2024 at 12:19:22PM +0000, Krzysztof Karas wrote:
> Commit 255fc1703e42 ("drm/i915/gem: Calculate object page offset for
> partial memory mapping") introduced a new offset, which accounts for
> userspace mapping not starting from the beginning of object's scatterlist.
>
> This works fine for cases where first object pte is larger than the new
> offset - "r->sgt.curr" counter is set to the offset to match the difference
> in the number of total pages. However, if object's first pte's size is
> equal to or smaller than the offset, then information about the offset
> in userspace is covered up by moving "r->sgt" pointer in remap_sg():
>
> r->sgt.curr += PAGE_SIZE;
> if (r->sgt.curr >= r->sgt.max)
> r->sgt = __sgt_iter(__sg_next(r->sgt.sgp), use_dma(r->iobase));
>
> This means that two or more pages from virtual memory are counted for
> only one page in object's memory, because after moving "r->sgt" pointer
> "r->sgt.curr" will be 0.
>
> We should account for this mismatch by moving "r->sgt" pointer to the
> next pte. For that we may use "r.sgt.max", which already holds the max
> allowed size. This change also eliminates possible confusion, when
> looking at i915_scatterlist.h and remap_io_sg() code: former has
> scatterlist pointer definition, which differentiates "s.max" value
> based on "dma" flag (sg_dma_len() is used only when the flag is
> enabled), while latter uses sg_dma_len() indiscriminately.
>
> This patch aims to resolve issue:
> https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12031
>
>
> v3:
> - instead of checking if r.sgt.curr would exceed allowed max, changed
> the value in the while loop to be aligned with `dma` value
>
> v4:
> - remove unnecessary parent relation
>
> v5:
> - update commit message with explanation about page counting mismatch
> and link to the issue
>
> Signed-off-by: Krzysztof Karas <krzysztof.karas@xxxxxxxxx>
thanks for your patch, merged to drm-intel-gt-next.
Thanks,
Andi
