<div dir="ltr">Petri,<div> </div><div>Got it. I just resubmitted the mmap64() -> mmap() patch:</div><div><a href="https://patchwork.freedesktop.org/patch/506163/">https://patchwork.freedesktop.org/patch/506163/</a> </div><div> </div><div>Jake Freeland</div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 6, 2022 at 5:45 AM Petri Latvala <<a href="mailto:petri.latvala@intel.com">petri.latvala@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Oct 05, 2022 at 07:02:01PM -0500, Jake Freeland wrote: > The mmap64() function exists to point 32-bit architectures to an mmap > interface with the capability to address a 64-bit memory address space. > This patch replaces all instances of mmap64() with drm_mmap() to > ensure that the correct mmap interface is used on varying architectures > and operating systems. > > Signed-off-by: Jake Freeland <<a href="mailto:jfree@freebsd.org" target="_blank">jfree@freebsd.org</a>> > --- > include/drm-uapi/libdrm.h | 89 ++++++++++++++++++++++++++++++++++++ > lib/i915/gem_mman.c | 5 +- > lib/igt_vgem.c | 3 +- > tests/i915/gem_mmap_gtt.c | 23 +++++----- > tests/i915/gem_mmap_offset.c | 13 +++--- > 5 files changed, 113 insertions(+), 20 deletions(-) > create mode 100644 include/drm-uapi/libdrm.h Ah yeah, that's one snag with just using drm_mmap. The function is private. We can't copy it to include/drm-uapi/, as it is not kernel uapi. And definitely not with a misleading name. Let's go with the original patch that changes mmap64 to mmap. If bug reports arise that someone's distro doesn't default to _FILE_OFFSET_BITS=64 on 32bit platforms, we can tune the build system then. -- Petri Latvala > > diff --git a/include/drm-uapi/libdrm.h b/include/drm-uapi/libdrm.h > new file mode 100644 > index 00000000..acfada5c > --- /dev/null > +++ b/include/drm-uapi/libdrm.h > @@ -0,0 +1,89 @@ > +/* > + * Copyright © 2014 NVIDIA Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + */ > + > +#ifndef LIBDRM_LIBDRM_H > +#define LIBDRM_LIBDRM_H > + > +#if defined(HAVE_VISIBILITY) > +# define drm_private __attribute__((visibility("hidden"))) > +# define drm_public __attribute__((visibility("default"))) > +#else > +# define drm_private > +# define drm_public > +#endif > + > + > +/** > + * Static (compile-time) assertion. > + * Basically, use COND to dimension an array. If COND is false/zero the > + * array size will be -1 and we'll get a compilation error. > + */ > +#define STATIC_ASSERT(COND) \ > + do { \ > + (void) sizeof(char [1 - 2*!(COND)]); \ > + } while (0) > + > + > +#include <sys/mman.h> > + > +#if defined(ANDROID) > +#include <errno.h> /* for EINVAL */ > + > +extern void *__mmap2(void *, size_t, int, int, int, size_t); > + > +static inline void *drm_mmap(void *addr, size_t length, int prot, int flags, > + int fd, loff_t offset) > +{ > + /* offset must be aligned to 4096 (not necessarily the page size) */ > + if (offset & 4095) { > + errno = EINVAL; > + return MAP_FAILED; > + } > + > + return __mmap2(addr, length, prot, flags, fd, (size_t) (offset >> 12)); > +} > + > +# define drm_munmap(addr, length) \ > + munmap(addr, length) > + > + > +#else > + > +/* assume large file support exists */ > +# define drm_mmap(addr, length, prot, flags, fd, offset) \ > + mmap(addr, length, prot, flags, fd, offset) > + > + > +static inline int drm_munmap(void *addr, size_t length) > +{ > + /* Copied from configure code generated by AC_SYS_LARGEFILE */ > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + \ > + (((off_t) 1 << 31) << 31)) > + STATIC_ASSERT(LARGE_OFF_T % 2147483629 == 721 && > + LARGE_OFF_T % 2147483647 == 1); > +#undef LARGE_OFF_T > + > + return munmap(addr, length); > +} > +#endif > + > +#endif > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c > index aa9ac6f3..4734f2af 100644 > --- a/lib/i915/gem_mman.c > +++ b/lib/i915/gem_mman.c > @@ -31,6 +31,7 @@ > #include "igt_device.h" > #include "ioctl_wrappers.h" > #include "intel_chipset.h" > +#include "libdrm.h" > > #include "gem_create.h" > #include "gem_mman.h" > @@ -118,7 +119,7 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot) > if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg)) > return NULL; > > - ptr = mmap64(0, size, prot, MAP_SHARED, fd, mmap_arg.offset); > + ptr = drm_mmap(0, size, prot, MAP_SHARED, fd, mmap_arg.offset); > if (ptr == MAP_FAILED) > ptr = NULL; > else > @@ -331,7 +332,7 @@ void *__gem_mmap_offset(int fd, uint32_t handle, uint64_t offset, uint64_t size, > if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_OFFSET, &arg)) > return NULL; > > - ptr = mmap64(0, size, prot, MAP_SHARED, fd, arg.offset + offset); > + ptr = drm_mmap(0, size, prot, MAP_SHARED, fd, arg.offset + offset); > > if (ptr == MAP_FAILED) > ptr = NULL; > diff --git a/lib/igt_vgem.c b/lib/igt_vgem.c > index 7f933b23..a35b3cd3 100644 > --- a/lib/igt_vgem.c > +++ b/lib/igt_vgem.c > @@ -29,6 +29,7 @@ > #include "igt_vgem.h" > #include "igt_core.h" > #include "ioctl_wrappers.h" > +#include "libdrm.h" > > /** > * SECTION:igt_vgem > @@ -76,7 +77,7 @@ void *__vgem_mmap(int fd, struct vgem_bo *bo, unsigned prot) > if (drmIoctl(fd, DRM_IOCTL_MODE_MAP_DUMB, &arg)) > return NULL; > > - ptr = mmap64(0, bo->size, prot, MAP_SHARED, fd, arg.offset); > + ptr = drm_mmap(0, bo->size, prot, MAP_SHARED, fd, arg.offset); > if (ptr == MAP_FAILED) > return NULL; > > diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c > index e39b9047..643f2f30 100644 > --- a/tests/i915/gem_mmap_gtt.c > +++ b/tests/i915/gem_mmap_gtt.c > @@ -46,6 +46,7 @@ > #include "igt.h" > #include "igt_sysfs.h" > #include "igt_x86.h" > +#include "libdrm.h" > #include "sw_sync.h" > > #ifndef PAGE_SIZE > @@ -113,11 +114,11 @@ test_access(int fd) > mmap_arg.handle = handle; > do_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg); > > - igt_assert(mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE, > + igt_assert(drm_mmap(0, OBJECT_SIZE, PROT_READ | PROT_WRITE, > MAP_SHARED, fd, mmap_arg.offset)); > > /* Check that the same offset on the other fd doesn't work. */ > - igt_assert(mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE, > + igt_assert(drm_mmap(0, OBJECT_SIZE, PROT_READ | PROT_WRITE, > MAP_SHARED, fd2, mmap_arg.offset) == MAP_FAILED); > igt_assert(errno == EACCES); > > @@ -128,7 +129,7 @@ test_access(int fd) > > /* Recheck that it works after flink. */ > /* Check that the same offset on the other fd doesn't work. */ > - igt_assert(mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE, > + igt_assert(drm_mmap(0, OBJECT_SIZE, PROT_READ | PROT_WRITE, > MAP_SHARED, fd2, mmap_arg.offset)); > } > > @@ -159,11 +160,11 @@ test_short(int fd) > for (pages = 1; pages <= OBJECT_SIZE / PAGE_SIZE; pages <<= 1) { > uint8_t *r, *w; > > - w = mmap64(0, pages * PAGE_SIZE, PROT_READ | PROT_WRITE, > + w = drm_mmap(0, pages * PAGE_SIZE, PROT_READ | PROT_WRITE, > MAP_SHARED, fd, mmap_arg.offset); > igt_assert(w != MAP_FAILED); > > - r = mmap64(0, pages * PAGE_SIZE, PROT_READ, > + r = drm_mmap(0, pages * PAGE_SIZE, PROT_READ, > MAP_SHARED, fd, mmap_arg.offset); > igt_assert(r != MAP_FAILED); > > @@ -384,13 +385,13 @@ test_isolation(int i915) > > close(B); > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > + ptr = drm_mmap(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > igt_assert(ptr != MAP_FAILED); > munmap(ptr, 4096); > > close(A); > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > + ptr = drm_mmap(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > igt_assert(ptr == MAP_FAILED); > } > > @@ -400,7 +401,7 @@ test_close_race(int i915) > const int ncpus = sysconf(_SC_NPROCESSORS_ONLN); > _Atomic uint32_t *handles; > > - handles = mmap64(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > + handles = drm_mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > igt_assert(handles != MAP_FAILED); > > igt_fork(child, ncpus + 1) { > @@ -418,7 +419,7 @@ test_close_race(int i915) > &mmap_arg) != -1) { > void *ptr; > > - ptr = mmap64(0, 4096, > + ptr = drm_mmap(0, 4096, > PROT_WRITE, MAP_SHARED, i915, > mmap_arg.offset); > if (ptr != MAP_FAILED) { > @@ -444,7 +445,7 @@ test_flink_race(int i915) > const int ncpus = sysconf(_SC_NPROCESSORS_ONLN); > _Atomic uint32_t *handles; > > - handles = mmap64(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > + handles = drm_mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > igt_assert(handles != MAP_FAILED); > > igt_fork(child, ncpus + 1) { > @@ -469,7 +470,7 @@ test_flink_race(int i915) > &mmap_arg) != -1) { > void *ptr; > > - ptr = mmap64(0, 4096, > + ptr = drm_mmap(0, 4096, > PROT_WRITE, MAP_SHARED, fd, > mmap_arg.offset); > if (ptr != MAP_FAILED) { > diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c > index 2b416edd..0a6d41aa 100644 > --- a/tests/i915/gem_mmap_offset.c > +++ b/tests/i915/gem_mmap_offset.c > @@ -38,6 +38,7 @@ > #include "i915/intel_memory_region.h" > #include "igt.h" > #include "igt_x86.h" > +#include "libdrm.h" > > IGT_TEST_DESCRIPTION("Basic MMAP_OFFSET IOCTL tests for mem regions\n"); > > @@ -67,7 +68,7 @@ __mmap_offset(int i915, uint32_t handle, uint64_t offset, uint64_t size, > if (mmap_offset_ioctl(i915, &arg)) > return NULL; > > - ptr = mmap64(0, size, prot, MAP_SHARED, i915, arg.offset + offset); > + ptr = drm_mmap(0, size, prot, MAP_SHARED, i915, arg.offset + offset); > if (ptr == MAP_FAILED) > ptr = NULL; > else > @@ -321,12 +322,12 @@ static void isolation(int i915) > t->name, B, b, offset_b); > > errno = 0; > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, i915, offset_a); > + ptr = drm_mmap(0, 4096, PROT_READ, MAP_SHARED, i915, offset_a); > igt_assert(ptr == MAP_FAILED); > igt_assert_eq(errno, EACCES); > > errno = 0; > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, i915, offset_b); > + ptr = drm_mmap(0, 4096, PROT_READ, MAP_SHARED, i915, offset_b); > igt_assert(ptr == MAP_FAILED); > igt_assert_eq(errno, EACCES); > > @@ -344,13 +345,13 @@ static void isolation(int i915) > > close(B); > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > + ptr = drm_mmap(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > igt_assert(ptr != MAP_FAILED); > munmap(ptr, 4096); > > close(A); > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > + ptr = drm_mmap(0, 4096, PROT_READ, MAP_SHARED, A, offset_a); > igt_assert(ptr == MAP_FAILED); > } > } > @@ -553,7 +554,7 @@ static void close_race(int i915, int timeout) > _Atomic uint32_t *handles; > size_t len = ALIGN((ncpus + 1) * sizeof(uint32_t), 4096); > > - handles = mmap64(0, len, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > + handles = drm_mmap(0, len, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > igt_assert(handles != MAP_FAILED); > > igt_fork(child, ncpus + 1) { > -- > 2.37.0 (Apple Git-136) > </blockquote></div>