From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4D09B6E420 for ; Tue, 16 Mar 2021 18:46:10 +0000 (UTC) Date: Tue, 16 Mar 2021 11:46:08 -0700 Message-ID: <87a6r3hr7z.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: References: <20210121083742.46592-1-ashutosh.dixit@intel.com> <87lfaohuol.wl-ashutosh.dixit@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t] lib/ioctl_wrappers: Keep IGT working without pread/pwrite ioctls List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Tvrtko Ursulin Cc: igt-dev@lists.freedesktop.org List-ID: 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. _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev