Hi Vanshi,
On Thursday, October 31, 2019 5:58:31 PM CET Vanshidhar Konda wrote:
> On Thu, Oct 31, 2019 at 04:28:55PM +0100, Janusz Krzysztofik wrote:
> >Some tests assume 4kB offset alignment while using softpin. That
> >assumption may be wrong on future GEM backends with possibly larger
> >minimum page sizes. As a result, those tests may either fail on
> >softpin at offsets which are incorrectly aligned, may silently skip
> >such incorrectly aligned addresses assuming them occupied by other
> >users if incorrect detection method is used, or may always succeed
> >when examining invalid use patterns.
> >
> >Provide a helper function that detects minimum GTT alignment. Tests
> >may use it to calculate softpin offsets valid for actually used backing
> >store.
> >
> >v2: Rename the helper, use 'minimum GTT alignment' term across the
> > change (Chris),
> > - use error numbers to distinguish between invalid offsets and
> > addresses occupied by other users, then
> > - simplify the code (Chris).
> >
> >Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx>
> >Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> >Cc: Stuart Summers <stuart.summers@xxxxxxxxx>
> >---
> > lib/ioctl_wrappers.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> > lib/ioctl_wrappers.h | 2 ++
> > 2 files changed, 48 insertions(+)
> >
> >diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> >index 628f8b83..f0ef8b2e 100644
> >--- a/lib/ioctl_wrappers.c
> >+++ b/lib/ioctl_wrappers.c
> >@@ -54,6 +54,7 @@
> > #include "intel_io.h"
> > #include "igt_debugfs.h"
> > #include "igt_sysfs.h"
> >+#include "igt_x86.h"
> > #include "config.h"
> >
> > #ifdef HAVE_VALGRIND
> >@@ -1158,6 +1159,51 @@ bool gem_has_softpin(int fd)
> > return has_softpin;
> > }
> >
> >+/**
> >+ * gem_gtt_min_alignment_order:
> >+ * @fd: open i915 drm file descriptor
> >+ *
> >+ * This function detects the minimum possible alignment of a soft-pinned gem
> >+ * object allocated from a default backing store. It is useful for calculating
> >+ * correctly aligned softpin offsets.
> >+ * Since size order to size conversion (size = 1 << order) is less trivial
> >+ * than the opposite, the function returns the alignment order as more handy.
> >+ *
> >+ * Returns:
> >+ * Size order of the minimum GTT alignment
> >+ */
> >+int gem_gtt_min_alignment_order(int fd)
> >+{
> >+ struct drm_i915_gem_exec_object2 obj;
> >+ struct drm_i915_gem_execbuffer2 eb;
> >+ const uint32_t bbe = MI_BATCH_BUFFER_END;
> >+ int order;
> >+
> >+ /* no softpin => 4kB page size */
> >+ if (!gem_has_softpin(fd))
> >+ return 12;
> >+
> >+ memset(&obj, 0, sizeof(obj));
> >+ memset(&eb, 0, sizeof(eb));
> >+
> >+ obj.handle = gem_create(fd, 4096);
> >+ obj.flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> >+ eb.buffers_ptr = to_user_pointer(&obj);
> >+ eb.buffer_count = 1;
> >+ gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
>
> I think it will be safer to create a new context to execute this
> execbuffer. For a new context the address space should be empty reducing
> the chance that there is another object mapped by the caller of the
> helper function at the address we start testing.
AFAICU, that shouldn't matter. Object attributes are validated and possibly
-EINVAL is returned already before an eviction is possibly attempted. The
algorithm assumes an offset is valid if any error code other than -EINVAL
(including no error) is returned.
Thanks,
Janusz
>
> Otherwise it looks good to me.
>
> Vanshi
>
> >+
> >+ for (order = 12; order < 64; order++) {
> >+ obj.offset = 1ull << order;
> >+ if (__gem_execbuf(fd, &eb) != -EINVAL)
> >+ break;
> >+ }
> >+ igt_assert(obj.offset < gem_aperture_size(fd));
> >+
> >+ gem_close(fd, obj.handle);
> >+ igt_debug("minimum GTT alignment is %#llx\n", (long long)obj.offset);
> >+ return order;
> >+}
> >+
> > /**
> > * gem_has_exec_fence:
> > * @fd: open i915 drm file descriptor
> >diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> >index 03211c97..c8d57a7c 100644
> >--- a/lib/ioctl_wrappers.h
> >+++ b/lib/ioctl_wrappers.h
> >@@ -138,6 +138,8 @@ uint64_t gem_aperture_size(int fd);
> > uint64_t gem_global_aperture_size(int fd);
> > uint64_t gem_mappable_aperture_size(void);
> > bool gem_has_softpin(int fd);
> >+int gem_gtt_min_alignment_order(int fd);
> >+#define gem_gtt_min_alignment(fd) (1ull << gem_gtt_min_alignment_order(fd))
> > bool gem_has_exec_fence(int fd);
> >
> > /* check functions which auto-skip tests by calling igt_skip() */
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
