[igt-dev] [Intel-gfx] [PATCH i-g-t] igt/gem_userptr: Check read-only mappings

Chris Wilson chris at chris-wilson.co.uk
Thu Jun 28 14:37:05 UTC 2018


Quoting Mika Kuoppala (2018-06-28 15:25:49)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Setup a userptr object that only has a read-only mapping back to a file
> > store (memfd). Then attempt to write into that mapping using the GPU and
> > assert that those writes do not land (while also writing via a writable
> > userptr mapping into the same memfd to verify that the GPU is working!)
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > ---
> >  configure.ac              |   1 +
> >  lib/ioctl_wrappers.c      |   4 +-
> >  lib/ioctl_wrappers.h      |   4 +-
> >  lib/meson.build           |   1 +
> >  meson.build               |   1 +
> >  tests/Makefile.am         |   4 +-
> >  tests/gem_userptr_blits.c | 337 +++++++++++++++++++++++++++++++++++++-
> >  7 files changed, 342 insertions(+), 10 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 1ee4e90e9..195963d4f 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -125,6 +125,7 @@ PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
> >  PKG_CHECK_MODULES(KMOD, [libkmod])
> >  PKG_CHECK_MODULES(PROCPS, [libprocps])
> >  PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
> > +PKG_CHECK_MODULES(SSL, [openssl])
> >  PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
> >  
> >  if test x$have_valgrind = xyes; then
> > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > index 79db44a8c..d5d2a4e4c 100644
> > --- a/lib/ioctl_wrappers.c
> > +++ b/lib/ioctl_wrappers.c
> > @@ -869,7 +869,7 @@ int gem_madvise(int fd, uint32_t handle, int state)
> >       return madv.retained;
> >  }
> >  
> > -int __gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t flags, uint32_t *handle)
> > +int __gem_userptr(int fd, void *ptr, uint64_t size, int read_only, uint32_t flags, uint32_t *handle)
> >  {
> >       struct drm_i915_gem_userptr userptr;
> >  
> > @@ -898,7 +898,7 @@ int __gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t flags, ui
> >   *
> >   * Returns userptr handle for the GEM object.
> >   */
> > -void gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t flags, uint32_t *handle)
> > +void gem_userptr(int fd, void *ptr, uint64_t size, int read_only, uint32_t flags, uint32_t *handle)
> >  {
> >       igt_assert_eq(__gem_userptr(fd, ptr, size, read_only, flags, handle), 0);
> >  }
> > diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> > index b966f72c9..8e2cd380b 100644
> > --- a/lib/ioctl_wrappers.h
> > +++ b/lib/ioctl_wrappers.h
> > @@ -133,8 +133,8 @@ struct local_i915_gem_userptr {
> >  #define LOCAL_I915_USERPTR_UNSYNCHRONIZED (1<<31)
> >       uint32_t handle;
> >  };
> > -void gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t flags, uint32_t *handle);
> > -int __gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t flags, uint32_t *handle);
> > +void gem_userptr(int fd, void *ptr, uint64_t size, int read_only, uint32_t flags, uint32_t *handle);
> > +int __gem_userptr(int fd, void *ptr, uint64_t size, int read_only, uint32_t flags, uint32_t *handle);
> >  
> >  void gem_sw_finish(int fd, uint32_t handle);
> >  
> > diff --git a/lib/meson.build b/lib/meson.build
> > index 1a355414e..939167f91 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -62,6 +62,7 @@ lib_deps = [
> >       pthreads,
> >       math,
> >       realtime,
> > +     ssl,
> >  ]
> >  
> >  if libdrm_intel.found()
> > diff --git a/meson.build b/meson.build
> > index 4d15d6238..638c01066 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -98,6 +98,7 @@ pciaccess = dependency('pciaccess', version : '>=0.10')
> >  libkmod = dependency('libkmod')
> >  libprocps = dependency('libprocps', required : true)
> >  libunwind = dependency('libunwind', required : true)
> > +ssl = dependency('openssl', required : true)
> >  
> >  valgrind = null_dep
> >  valgrindinfo = 'No'
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index f41ad5096..ba307b220 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -126,8 +126,8 @@ gem_tiled_swapping_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
> >  gem_tiled_swapping_LDADD = $(LDADD) -lpthread
> >  prime_self_import_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
> >  prime_self_import_LDADD = $(LDADD) -lpthread
> > -gem_userptr_blits_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
> > -gem_userptr_blits_LDADD = $(LDADD) -lpthread
> > +gem_userptr_blits_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) $(SSL_CFLAGS)
> > +gem_userptr_blits_LDADD = $(LDADD) $(SSL_LIBS) -lpthread
> >  perf_pmu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
> >  
> >  gem_eio_LDADD = $(LDADD) -lrt
> > diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
> > index 7e3b6ef38..30c6bc48c 100644
> > --- a/tests/gem_userptr_blits.c
> > +++ b/tests/gem_userptr_blits.c
> > @@ -43,13 +43,17 @@
> >  #include <fcntl.h>
> >  #include <inttypes.h>
> >  #include <errno.h>
> > +#include <setjmp.h>
> >  #include <sys/stat.h>
> >  #include <sys/time.h>
> >  #include <sys/mman.h>
> > +#include <openssl/sha.h>
> >  #include <signal.h>
> >  #include <pthread.h>
> >  #include <time.h>
> >  
> > +#include <linux/memfd.h>
> > +
> >  #include "drm.h"
> >  #include "i915_drm.h"
> >  
> > @@ -238,6 +242,57 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
> >       return ret;
> >  }
> >  
> > +static void store_dword(int fd, uint32_t target,
> > +                     uint32_t offset, uint32_t value)
> > +{
> > +     const int gen = intel_gen(intel_get_drm_devid(fd));
> > +     struct drm_i915_gem_exec_object2 obj[2];
> > +     struct drm_i915_gem_relocation_entry reloc;
> > +     struct drm_i915_gem_execbuffer2 execbuf;
> > +     uint32_t batch[16];
> > +     int i;
> > +
> > +     memset(&execbuf, 0, sizeof(execbuf));
> > +     execbuf.buffers_ptr = to_user_pointer(obj);
> > +     execbuf.buffer_count = ARRAY_SIZE(obj);
> > +     execbuf.flags = 0;
> > +     if (gen < 6)
> > +             execbuf.flags |= I915_EXEC_SECURE;
> > +
> > +     memset(obj, 0, sizeof(obj));
> > +     obj[0].handle = target;
> > +     obj[1].handle = gem_create(fd, 4096);
> > +
> > +     memset(&reloc, 0, sizeof(reloc));
> > +     reloc.target_handle = obj[0].handle;
> > +     reloc.presumed_offset = 0;
> > +     reloc.offset = sizeof(uint32_t);
> > +     reloc.delta = offset;
> > +     reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> > +     reloc.write_domain = I915_GEM_DOMAIN_RENDER;
> > +     obj[1].relocs_ptr = to_user_pointer(&reloc);
> > +     obj[1].relocation_count = 1;
> > +
> > +     i = 0;
> > +     batch[i] = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
> > +     if (gen >= 8) {
> > +             batch[++i] = offset;
> > +             batch[++i] = 0;
> > +     } else if (gen >= 4) {
> > +             batch[++i] = 0;
> > +             batch[++i] = offset;
> > +             reloc.offset += sizeof(uint32_t);
> > +     } else {
> > +             batch[i]--;
> > +             batch[++i] = offset;
> > +     }
> > +     batch[++i] = value;
> > +     batch[++i] = MI_BATCH_BUFFER_END;
> > +     gem_write(fd, obj[1].handle, 0, batch, sizeof(batch));
> > +     gem_execbuf(fd, &execbuf);
> > +     gem_close(fd, obj[1].handle);
> > +}
> > +
> >  static uint32_t
> >  create_userptr(int fd, uint32_t val, uint32_t *ptr)
> >  {
> > @@ -941,6 +996,275 @@ static int test_dmabuf(void)
> >       return 0;
> >  }
> >  
> > +static void test_readonly(int i915)
> > +{
> > +     unsigned char orig[SHA_DIGEST_LENGTH];
> > +     uint64_t aperture_size;
> > +     uint32_t whandle, rhandle;
> > +     size_t sz, total;
> > +     void *pages, *space;
> > +     int memfd;
> > +
> > +     /*
> > +      * A small batch of pages; small enough to cheaply check for stray
> > +      * writes but large enough that we don't create too many VMA pointing
> > +      * back to this set from the large arena. The limit on total number
> > +      * of VMA for a process is 65,536 (at least on this kernel).
> > +      */
> > +     sz = 16 << 12;
> > +     memfd = memfd_create("pages", 0);
> > +     igt_require(memfd != -1);
> > +     igt_require(ftruncate(memfd, sz) == 0);
> > +
> > +     pages = mmap(NULL, sz, PROT_WRITE, MAP_SHARED, memfd, 0);
> > +     igt_assert(pages != MAP_FAILED);
> > +
> > +     igt_require(__gem_userptr(i915, pages, sz, true, userptr_flags, &rhandle) == 0);
> > +     gem_close(i915, rhandle);
> > +
> > +     gem_userptr(i915, pages, sz, false, userptr_flags, &whandle);
> > +
> > +     total = 2048ull << 20;
> > +     aperture_size = gem_aperture_size(i915) / 2;
> > +     if (aperture_size < total)
> > +             total = aperture_size;
> > +     total = total / sz * sz;
> > +     igt_info("Using a %'zuB (%'zu pages) arena onto %zu pages\n",
> > +              total, total >> 12, sz >> 12);
> > +
> > +     /* Create an arena all pointing to the same set of pages */
> > +     space = mmap(NULL, total, PROT_READ, MAP_ANON | MAP_SHARED, -1, 0);
> > +     igt_require(space != MAP_FAILED);
> > +     for (size_t offset = 0; offset < total; offset += sz) {
> > +             igt_assert(mmap(space + offset, sz,
> > +                             PROT_WRITE, MAP_SHARED | MAP_FIXED,
> > +                             memfd, 0) != MAP_FAILED);
> > +             *(uint32_t *)(space + offset) = offset;
> > +     }
> > +     igt_assert_eq_u32(*(uint32_t *)pages, (uint32_t)(total - sz));
> > +     igt_assert(mlock(space, total) == 0);
> > +     close(memfd);
> > +
> > +     /* Check we can create a normal userptr bo wrapping the wrapper */
> > +     gem_userptr(i915, space, total, false, userptr_flags, &rhandle);
> > +     gem_set_domain(i915, rhandle, I915_GEM_DOMAIN_CPU, 0);
> > +     for (size_t offset = 0; offset < total; offset += sz)
> > +             store_dword(i915, rhandle, offset + 4, offset / sz);
> > +     gem_sync(i915, rhandle);
> > +     igt_assert_eq_u32(*(uint32_t *)(pages + 0), (uint32_t)(total - sz));
> > +     igt_assert_eq_u32(*(uint32_t *)(pages + 4), (uint32_t)(total / sz - 1));
> > +     gem_close(i915, rhandle);
> > +
> > +     /* Now enforce read-only henceforth */
> > +     igt_assert(mprotect(space, total, PROT_READ) == 0);
> > +
> > +     SHA1(pages, sz, orig);
> > +     igt_fork(child, 1) {
> > +             const int gen = intel_gen(intel_get_drm_devid(i915));
> > +             const int nreloc = 1024;
> > +             struct drm_i915_gem_relocation_entry *reloc;
> > +             struct drm_i915_gem_exec_object2 obj[2];
> > +             struct drm_i915_gem_execbuffer2 exec;
> > +             unsigned char ref[SHA_DIGEST_LENGTH], result[SHA_DIGEST_LENGTH];
> > +             uint32_t *batch;
> > +             int i;
> > +
> > +             reloc = calloc(sizeof(*reloc), nreloc);
> > +             gem_userptr(i915, space, total, true, userptr_flags, &rhandle);
> > +
> > +
> > +             memset(obj, 0, sizeof(obj));
> > +             obj[0].flags = LOCAL_EXEC_OBJECT_SUPPORTS_48B;
> > +             obj[1].handle = gem_create(i915, 4096*16);
> > +             obj[1].relocation_count = nreloc;
> > +             obj[1].relocs_ptr = to_user_pointer(reloc);
> > +
> > +             batch = gem_mmap__wc(i915, obj[1].handle, 0, 4096*16, PROT_WRITE);
> > +
> > +             memset(&exec, 0, sizeof(exec));
> > +             exec.buffer_count =2;
> > +             exec.buffers_ptr = to_user_pointer(obj);
> > +
> > +             for_each_engine(i915, exec.flags) {
> > +                     /* First tweak the backing store through the write */
> > +                     i = 0;
> > +                     obj[0].handle = whandle;
> > +                     for (int n = 0; n < nreloc; n++) {
> > +                             uint64_t offset;
> > +
> > +                             reloc[n].target_handle = obj[0].handle;
> > +                             reloc[n].delta = 4*(rand() % (sz/4));
> > +                             reloc[n].offset = (i+1) * sizeof(uint32_t);
> > +                             reloc[n].presumed_offset = obj[0].offset;
> > +                             reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
> > +                             reloc[n].write_domain = I915_GEM_DOMAIN_RENDER;
> > +
> > +                             offset = reloc[n].presumed_offset + reloc[n].delta;
> > +
> > +                             batch[i] = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
> > +                             if (gen >= 8) {
> > +                                     batch[++i] = offset;
> > +                                     batch[++i] = offset >> 32;
> > +                             } else if (gen >= 4) {
> > +                                     batch[++i] = 0;
> > +                                     batch[++i] = offset;
> > +                                     reloc[n].offset += sizeof(uint32_t);
> > +                             } else {
> > +                                     batch[i]--;
> > +                                     batch[++i] = offset;
> > +                             }
> > +                             batch[++i] = rand();
> > +                             i++;
> > +                     }
> > +                     batch[i] = MI_BATCH_BUFFER_END;
> > +
> > +                     gem_execbuf(i915, &exec);
> > +                     gem_sync(i915, obj[0].handle);
> > +                     SHA1(pages, sz, ref);
> > +
> > +                     igt_assert(memcmp(ref, orig, sizeof(ref)));
> > +                     memcpy(orig, ref, sizeof(orig));
> 
> This memcpy seems obsolete.

We use the current state for the next engine. After we write to the
buffer, we want to compare against the current, not original, checksum
to be sure that writes are landing.
-Chris


More information about the igt-dev mailing list