[igt-dev] [PATCH i-g-t v4] tests/i915/gem_mmap_offset: add new tests to extend coverage

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Tue Dec 10 10:15:19 UTC 2019


Extend coverity by adding:
 * coherence test - verify mappings coherency
 * test descriptions
 * some minor changes within existing tests

v2: Changes according to the review:
 * allocation strategy is now more adaptive in 'clear' test
 * removing not important flags in 'bad-flags' test
 * coherency test supports now more strategies

v3: Decrease minimum memory requirement for 'clear' test.

v4: Removing 'big-bo' test, fixing gem_create / mmap behavior in
    'clear' test.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
---
 tests/i915/gem_mmap_offset.c | 189 +++++++++++++++++++++++++++++++----
 1 file changed, 171 insertions(+), 18 deletions(-)

diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
index 95e1e3e6..59d3c808 100644
--- a/tests/i915/gem_mmap_offset.c
+++ b/tests/i915/gem_mmap_offset.c
@@ -31,7 +31,7 @@
 #include "igt.h"
 #include "igt_x86.h"
 
-IGT_TEST_DESCRIPTION("Basic MMAP_OFFSET IOCTL tests for mem regions\n");
+IGT_TEST_DESCRIPTION("Basic MMAP_OFFSET IOCTL tests\n");
 
 static const struct mmap_offset {
 	const char *name;
@@ -119,7 +119,6 @@ static void bad_flags(int i915)
 		.handle = gem_create(i915, 4096),
 		.flags = -1ull,
 	};
-
 	igt_assert_eq(mmap_offset_ioctl(i915, &arg), -EINVAL);
 	gem_close(i915, arg.handle);
 }
@@ -147,7 +146,7 @@ static void basic_uaf(int i915)
 
 	for_each_mmap_offset_type(t) {
 		uint32_t handle = gem_create(i915, obj_size);
-		uint8_t *expected, *buf, *addr;
+		uint8_t *buf, *addr;
 
 		addr = __mmap_offset(i915, handle, 0, obj_size,
 				     PROT_READ | PROT_WRITE,
@@ -157,14 +156,12 @@ static void basic_uaf(int i915)
 			continue;
 		}
 
-		expected = calloc(obj_size, sizeof(*expected));
+		buf = calloc(obj_size, sizeof(*buf));
 		gem_set_domain(i915, handle, t->domain, 0);
-		igt_assert_f(memcmp(addr, expected, obj_size) == 0,
+		igt_assert_f(memcmp(addr, buf, obj_size) == 0,
 			     "mmap(%s) not clear on gem_create()\n",
 			     t->name);
-		free(expected);
 
-		buf = calloc(obj_size, sizeof(*buf));
 		memset(buf + 1024, 0x01, 1024);
 		gem_write(i915, handle, 0, buf, obj_size);
 		gem_set_domain(i915, handle, t->domain, 0);
@@ -184,9 +181,8 @@ static void basic_uaf(int i915)
 		igt_assert_f(memcmp(buf, addr, obj_size) == 0,
 			     "mmap(%s) not resident after gem_close()\n",
 			     t->name);
-		free(buf);
 
-		igt_debug("Testing unmapping\n");
+		free(buf);
 		munmap(addr, obj_size);
 	}
 }
@@ -218,10 +214,10 @@ static void isolation(int i915)
 		igt_assert_eq(mmap_offset_ioctl(B, &mmap_arg), 0);
 		offset_b = mmap_arg.offset;
 
-		igt_info("A[%s]: {fd:%d, handle:%d, offset:%"PRIx64"}\n",
-			 t->name, A, a, offset_a);
-		igt_info("B[%s]: {fd:%d, handle:%d, offset:%"PRIx64"}\n",
-			 t->name, B, b, offset_b);
+		igt_debug("A[%s]: {fd:%d, handle:%d, offset:%"PRIx64"}\n",
+			  t->name, A, a, offset_a);
+		igt_debug("B[%s]: {fd:%d, handle:%d, offset:%"PRIx64"}\n",
+			  t->name, B, b, offset_b);
 
 		errno = 0;
 		ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, i915, offset_a);
@@ -349,6 +345,8 @@ static uint64_t get_npages(_Atomic(uint64_t) *global, uint64_t npages)
 
 struct thread_clear {
 	_Atomic(uint64_t) max;
+	_Atomic(uint64_t) current;
+	uint64_t min;
 	int timeout;
 	int i915;
 };
@@ -366,6 +364,31 @@ static int create_ioctl(int i915, struct drm_i915_gem_create *create)
 	return err;
 }
 
+static bool __is_mmap_type_supported(int i915, unsigned int type)
+{
+	uint64_t size = 4096;
+	struct drm_i915_gem_create create = {
+		.size = size,
+	};
+	void *ptr;
+	int ret;
+
+	ret = create_ioctl(i915, &create);
+	if (ret)
+		return false;
+
+	ptr = __mmap_offset(i915, create.handle, 0, create.size,
+			    PROT_READ | PROT_WRITE,
+			    type);
+	gem_close(i915, create.handle);
+
+	if (!ptr)
+		return false;
+	munmap(ptr, size);
+
+	return true;
+}
+
 static void *thread_clear(void *data)
 {
 	struct thread_clear *arg = data;
@@ -374,10 +397,12 @@ static void *thread_clear(void *data)
 	int i915 = arg->i915;
 
 	t = mmap_offset_types;
+
 	igt_until_timeout(arg->timeout) {
 		struct drm_i915_gem_create create = {};
 		uint64_t npages;
 		void *ptr;
+		int ret;
 
 		npages = random();
 		npages <<= 32;
@@ -385,10 +410,48 @@ static void *thread_clear(void *data)
 		npages = get_npages(&arg->max, npages);
 		create.size = npages << 12;
 
-		create_ioctl(i915, &create);
-		ptr = __mmap_offset(i915, create.handle, 0, create.size,
-				    PROT_READ | PROT_WRITE,
-				    t->type);
+		if (!__is_mmap_type_supported(i915, t->type)) {
+			goto next;
+		}
+
+		ret = create_ioctl(i915, &create);
+		if (!ret) {
+			ptr = __mmap_offset(i915, create.handle, 0, create.size,
+					    PROT_READ | PROT_WRITE,
+					    t->type);
+			/*
+			 * gem_create succeed, but we're not able to mmap bo.
+			 * We mark path as failed and we have to change memory
+			 * allocation constraints.
+			 */
+			if (!ptr) {
+				ret = -1;
+				gem_close(i915, create.handle);
+			}
+		}
+
+		if (ret) {
+			uint64_t current;
+
+			/*
+			 * We're executing on hardware with different memory
+			 * regions and sizes so we try do adopt our needs.
+			 * Thus for each unsuccessful allocation size we reduce
+			 * by half.
+			 */
+			current = atomic_load(&arg->current);
+			igt_debug("Decreasing pages %lu -> %lu, current: %lu, "
+				  "min: %lu\n",
+				  npages, npages / 2,
+				  current - npages/2, arg->min);
+			atomic_fetch_add(&arg->max, npages/2);
+			atomic_fetch_sub(&arg->current, npages/2);
+			current = atomic_load(&arg->current);
+			igt_assert(current > arg->min);
+
+			continue;
+		}
+
 		/* No set-domains as we are being as naughty as possible */
 		for (uint64_t page = 0; ptr && page < npages; page++) {
 			uint64_t x[8] = {
@@ -409,6 +472,7 @@ static void *thread_clear(void *data)
 		gem_close(i915, create.handle);
 		checked += npages;
 
+next:
 		atomic_fetch_add(&arg->max, npages);
 
 		if (!(++t)->name)
@@ -424,6 +488,9 @@ static void always_clear(int i915, int timeout)
 		.i915 = i915,
 		.timeout = timeout,
 		.max = intel_get_avail_ram_mb() << (20 - 12), /* in pages */
+		.min = 32ULL << (20 - 12),
+		.current = intel_get_avail_ram_mb() << (20 - 12),
+
 	};
 	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
 	unsigned long checked;
@@ -438,7 +505,81 @@ static void always_clear(int i915, int timeout)
 		pthread_join(thread[i], &result);
 		checked += (uintptr_t)result;
 	}
-	igt_info("Checked %'lu page allocations\n", checked);
+	igt_debug("Checked %'lu page allocations\n", checked);
+}
+
+static const struct coherency_pair {
+	const char *name;
+	unsigned int type1;
+	unsigned int type2;
+	unsigned int domain;
+	bool invalidate_cpu;
+} coherency_pair_types[] = {
+	{ "wc-wb", I915_MMAP_OFFSET_WC, I915_MMAP_OFFSET_WB,
+	  I915_GEM_DOMAIN_WC, true },
+	{ "uc-wb", I915_MMAP_OFFSET_UC, I915_MMAP_OFFSET_WB,
+	  I915_GEM_DOMAIN_WC, false },
+	{ "uc-wc", I915_MMAP_OFFSET_UC, I915_MMAP_OFFSET_WC,
+	  I915_GEM_DOMAIN_WC, false },
+	{ "gtt-wb", I915_MMAP_OFFSET_GTT, I915_MMAP_OFFSET_WB,
+	  I915_GEM_DOMAIN_CPU, true },
+	{ "gtt-wc", I915_MMAP_OFFSET_GTT, I915_MMAP_OFFSET_WC,
+	  I915_GEM_DOMAIN_WC, false },
+	{ "gtt-uc", I915_MMAP_OFFSET_GTT, I915_MMAP_OFFSET_UC,
+	  I915_GEM_DOMAIN_WC, false },
+	{},
+};
+
+#define for_each_coherency_pair(__cp) \
+	for (const struct coherency_pair *__cp = coherency_pair_types; \
+	     (__cp)->name; \
+	     (__cp)++)
+
+static void test_coherency(int i915)
+{
+	uint64_t size = 1 << 20; /* 1/16 is enough */
+	uint32_t handle;
+	uint32_t *map1, *map2;
+	int i;
+
+	igt_require(igt_setup_clflush());
+
+	handle = gem_create(i915, size);
+
+	for_each_coherency_pair(cp) {
+		igt_debug("Checking coherency pair: %s - ", cp->name);
+		map1 = __mmap_offset(i915, handle, 0, size,
+				     PROT_READ | PROT_WRITE,
+				     cp->type1);
+		if (!map1) {
+			igt_debug("skip\n");
+			continue;
+		}
+
+		map2 = __mmap_offset(i915, handle, 0, size,
+				     PROT_READ | PROT_WRITE,
+				     cp->type2);
+		if (!map2) {
+			igt_debug("skip\n");
+			goto skip;
+		}
+
+		gem_set_domain(i915, handle, cp->domain, cp->domain);
+
+		for (i = 0; i < size / 64; i++) {
+			int x = 16*i + (i%16);
+
+			map1[x] = i;
+			if (cp->invalidate_cpu)
+				igt_clflush_range(&map2[x], sizeof(map2[x]));
+			igt_assert_eq(map2[x], i);
+
+		}
+		igt_debug("ok\n");
+		munmap(map1, size);
+skip:		munmap(map2, size);
+	}
+	gem_close(i915, handle);
 }
 
 static int mmap_gtt_version(int i915)
@@ -470,8 +611,12 @@ igt_main
 	igt_describe("Verify mapping to invalid gem objects won't be created");
 	igt_subtest_f("bad-object")
 		bad_object(i915);
+
+	igt_describe("Verify mapping is not possible on wrong flags");
 	igt_subtest_f("bad-flags")
 		bad_flags(i915);
+
+	igt_describe("Verify mapping is not possible on wrong extensions");
 	igt_subtest_f("bad-extensions")
 		bad_extensions(i915);
 
@@ -479,8 +624,11 @@ igt_main
 	igt_subtest_f("basic-uaf")
 		basic_uaf(i915);
 
+	igt_describe("Check BOs are isolated between contexts");
 	igt_subtest_f("isolation")
 		isolation(i915);
+
+	igt_describe("Verify pagefault is async");
 	igt_subtest_f("pf-nonblock")
 		pf_nonblock(i915);
 
@@ -488,9 +636,14 @@ igt_main
 	igt_subtest_f("close-race")
 		close_race(i915, 20);
 
+	igt_describe("Verify BOs allocated in many threads are zeroed");
 	igt_subtest_f("clear")
 		always_clear(i915, 20);
 
+	igt_describe("Check coherency between WC and WB mappings");
+	igt_subtest_f("coherency")
+		test_coherency(i915);
+
 	igt_fixture {
 		close(i915);
 	}
-- 
2.23.0



More information about the igt-dev mailing list