[PATCH i-g-t v5] tests/gem_exec_reloc: Don't filter out invalid addresses

* [PATCH i-g-t v5] tests/gem_exec_reloc: Don't filter out invalid addresses
@ 2019-11-05 15:37 ` Janusz Krzysztofik
  0 siblings, 0 replies; 4+ messages in thread
From: Janusz Krzysztofik @ 2019-11-05 15:37 UTC (permalink / raw)
  To: Chris Wilson, Vanshidhar Konda, Joonas Lahtinen; +Cc: intel-gfx, igt-dev

Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable
addresses for !ppgtt") introduced filtering of shared GTT addresses
possibly in use.  Unfortunately, that filtering doesn't distinguish
between addresses actually in use and otherwise invalid softpin
offsets.  If for example incorrect GTT alignment is assumed while
softpin offsets are calculated, a half of those offsets to be tested
may be incorrectly detected as reserved and silently skipped instead
of reported as a problem.  That can significantly distort the intended
test pattern.

Filter out unavailable addresses only if reported as reserved.

v2: Skip unavailable addresses only when not running on full PPGTT.
v3: Replace the not on full PPGTT requirement for skipping with error
    code checking.
v4: Silently skip only those offsets which have been explicitly
    reported as overlapping with shared GTT reserved space, not simply
    all which raise failures other than -EINVAL (Chris),
  - as an implementation of moving the probe out of line so it's not
    easily confused with the central point of the test (Chris), use
    object validation library helper just introduced.
v5: Detach from the series (rejected by Joonas) and submit once more as
    an independent (though related) standalone patch,
  - don't use relocations for anything new (Joonas),
  - don't depend on such quirk behavior as to kernel validating
    parameters in certain order (Joonas),
  - it's not straightforward that we are just checking the offset
    unless I check the code (of the helper) (Vanshi);
    the above three changes effectively revert most of v4 changes,
    leaving only that mentioned as the first one under v4 above,
  - use 'in use' or 'reserved' wording instead of 'occupied by other
    users' in commit description, they better correspond to the
    original commit being fixed as well as the comment proposed by
    Chris.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 tests/i915/gem_exec_reloc.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index fdd9661d..238fa88f 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -520,7 +520,7 @@ static void basic_range(int fd, unsigned flags)
 	uint64_t gtt_size = gem_aperture_size(fd);
 	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	igt_spin_t *spin = NULL;
-	int count, n;
+	int count, n, err;
 
 	igt_require(gem_has_softpin(fd));
 
@@ -542,8 +542,12 @@ static void basic_range(int fd, unsigned flags)
 		gem_write(fd, obj[n].handle, 0, &bbe, sizeof(bbe));
 		execbuf.buffers_ptr = to_user_pointer(&obj[n]);
 		execbuf.buffer_count = 1;
-		if (__gem_execbuf(fd, &execbuf))
+		err = __gem_execbuf(fd, &execbuf);
+		if (err) {
+			/* Iff using a shared GTT, some of it may be reserved */
+			igt_assert_eq(err, -ENOSPC);
 			continue;
+		}
 
 		igt_debug("obj[%d] handle=%d, address=%llx\n",
 			  n, obj[n].handle, (long long)obj[n].offset);
@@ -562,8 +566,12 @@ static void basic_range(int fd, unsigned flags)
 		gem_write(fd, obj[n].handle, 0, &bbe, sizeof(bbe));
 		execbuf.buffers_ptr = to_user_pointer(&obj[n]);
 		execbuf.buffer_count = 1;
-		if (__gem_execbuf(fd, &execbuf))
+		err = __gem_execbuf(fd, &execbuf);
+		if (err) {
+			/* Iff using a shared GTT, some of it may be reserved */
+			igt_assert_eq(err, -ENOSPC);
 			continue;
+		}
 
 		igt_debug("obj[%d] handle=%d, address=%llx\n",
 			  n, obj[n].handle, (long long)obj[n].offset);
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread