[igt-dev] [RFC PATCH i-g-t v4 2/4] lib: Add minimum GTT alignment helper

[Vanshidhar Konda]

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 at linux.intel.com>
>Cc: Chris Wilson <chris at chris-wilson.co.uk>
>Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>Cc: Stuart Summers <stuart.summers at intel.com>
>---
> 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.

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() */
>-- 
>2.21.0
>