[igt-dev] [PATCH i-g-t] lib/ioctl_wrappers: Keep IGT working without pread/pwrite ioctls

[Dixit, Ashutosh]

On Tue, 16 Mar 2021 02:16:14 -0700, Tvrtko Ursulin wrote:
> On 15/03/2021 23:19, Dixit, Ashutosh wrote:
> > On Mon, 15 Mar 2021 09:51:04 -0700, Tvrtko Ursulin wrote:
> >>> @@ -324,6 +325,70 @@ void gem_close(int fd, uint32_t handle)
> >>>	do_ioctl(fd, DRM_IOCTL_GEM_CLOSE, &close_bo);
> >>>    }
> >>>    +static bool is_cache_coherent(int fd, uint32_t handle)
> >>> +{
> >>> +	return gem_get_caching(fd, handle) != I915_CACHING_NONE;
> >>> +}
> >>> +
> >>> +static void mmap_write(int fd, uint32_t handle, uint64_t offset,
> >>> +		       const void *buf, uint64_t length)
> >>> +{
> >>> +	void *map = NULL;
> >>> +
> >>> +	if (!length)
> >>> +		return;
> >>> +
> >>> +	if (is_cache_coherent(fd, handle)) {
> >>> +		/* offset arg for mmap functions must be 0 */
> >>
> >> Offset and lenght I think. So I guess none of the tests used non-aligned
> >> access? Might be worth putting those two as asserts at the top of
> >> mmap_write and mmap_read.
> >
> > Hi Tvrtko, sorry I think you missed the logic. offset and length coming
> > into mmap_write() and mmap_read() are non-zero (so tests are using
> > non-aligned access). However __gem_mmap_offset() (called from
> > __gem_mmap__cpu_coherent() etc.) has an assert that needs the offset to be
> > 0 (which is what the comment above refers to). Therefore instead of doing
> > mmap(offset, length) below we do mmap(0, offset + length).
>
> But is offset + length is guaranteed to be page aligned? I thought mmap(2)
> requires it. Well, if it works obviously I was thinking wrong..

Yes, seems to be working. Offset for mmap(2) needs to be page aligned (the
length is probably bumped up to be page aligned by the kernel) and in the
case of __gem_mmap_offset() the offset supplied to mmap is the page aligned
offset returned by DRM_IOCTL_I915_GEM_MMAP_OFFSET.