[Intel-gfx] [RFC PATCH i-g-t v4 1/4] tests/gem_exec_reloc: Don't filter out invalid addresses

[Janusz Krzysztofik]

Hi Vanshi,

On Thursday, October 31, 2019 5:59:50 PM CET Vanshidhar Konda wrote:
> May be this patch can just be merged with the other patch in this series
> that changes gem_exec_reloc.

Even if both patches are closely related to possibly incorrect alignment in 
use, I think each one resolves it own distinct issue, that's why I think they 
should be kept separate.

Thanks,
Janusz


> Vanshi
> 
> On Thu, Oct 31, 2019 at 04:28:54PM +0100, Janusz Krzysztofik wrote:
> >Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable
> >addresses for !ppgtt") introduced filtering of addresses possibly
> >occupied by other users of shared GTT.  Unfortunately, that filtering
> >doesn't distinguish between actually occupied addresses and otherwise
> >invalid softpin offsets.  As soon as incorrect GTT alignment is assumed
> >when running on future backends with possibly larger minimum page
> >sizes, a half of calculated offsets to be tested will be incorrectly
> >detected as occupied by other users and silently skipped instead of
> >reported as a problem.  That will significantly distort the intended
> >test pattern.
> >
> >Filter out failing addresses only if not reported as invalid.
> >
> >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.
> >
> >Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> >Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >---
> > tests/i915/gem_exec_reloc.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> >diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
> >index 5f59fe99..423fed8b 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,11 @@ 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) {
> >+			igt_assert(err != -EINVAL);
> > 			continue;
> >+		}
> >
> > 		igt_debug("obj[%d] handle=%d, address=%llx\n",
> > 			  n, obj[n].handle, (long long)obj[n].offset);
> >@@ -562,8 +565,11 @@ 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) {
> >+			igt_assert(err != -EINVAL);
> > 			continue;
> >+		}
> >
> > 		igt_debug("obj[%d] handle=%d, address=%llx\n",
> > 			  n, obj[n].handle, (long long)obj[n].offset);
>