diff mbox series

[RFC,v2,1/3] tests/gem_exec_reloc: Don't filter out addresses on full PPGTT

Message ID 20191023152917.647-1-janusz.krzysztofik@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [RFC,v2,1/3] tests/gem_exec_reloc: Don't filter out addresses on full PPGTT | expand

Commit Message

Janusz Krzysztofik Oct. 23, 2019, 3:29 p.m. UTC
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
is unconditional, no matter if running on old shared GTT or not.  When
running on full (non-aliasing) PPGTT, that may result in errors other
than those intended to be skipped over being silently ignored.

Skip over unavailable addresses only when not running on full PPGTT.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_reloc.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

Comments

Chris Wilson Oct. 23, 2019, 3:41 p.m. UTC | #1
Quoting Janusz Krzysztofik (2019-10-23 16:29:15)
> 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
> is unconditional, no matter if running on old shared GTT or not.  When
> running on full (non-aliasing) PPGTT, that may result in errors other
> than those intended to be skipped over being silently ignored.
> 
> Skip over unavailable addresses only when not running on full PPGTT.
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/i915/gem_exec_reloc.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
> index fdd9661d..f7fc0ea7 100644
> --- a/tests/i915/gem_exec_reloc.c
> +++ b/tests/i915/gem_exec_reloc.c
> @@ -539,12 +539,13 @@ static void basic_range(int fd, unsigned flags)
>                 obj[n].offset = (1ull << (i + 12)) - 4096;
>                 obj[n].offset = gen8_canonical_address(obj[n].offset);
>                 obj[n].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> -               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))
> -                       continue;
> -
> +               if (!gem_uses_full_ppgtt(fd)) {

Oh boy, I can think of an [unmentionable] instance where this isn't true
either.

It should be true indeed unless the kernel is interfering with the
user's GTT...
-Chris
diff mbox series

Patch

diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index fdd9661d..f7fc0ea7 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -539,12 +539,13 @@  static void basic_range(int fd, unsigned flags)
 		obj[n].offset = (1ull << (i + 12)) - 4096;
 		obj[n].offset = gen8_canonical_address(obj[n].offset);
 		obj[n].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-		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))
-			continue;
-
+		if (!gem_uses_full_ppgtt(fd)) {
+			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))
+				continue;
+		}
 		igt_debug("obj[%d] handle=%d, address=%llx\n",
 			  n, obj[n].handle, (long long)obj[n].offset);
 
@@ -559,12 +560,13 @@  static void basic_range(int fd, unsigned flags)
 		obj[n].offset = 1ull << (i + 12);
 		obj[n].offset = gen8_canonical_address(obj[n].offset);
 		obj[n].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-		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))
-			continue;
-
+		if (!gem_uses_full_ppgtt(fd)) {
+			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))
+				continue;
+		}
 		igt_debug("obj[%d] handle=%d, address=%llx\n",
 			  n, obj[n].handle, (long long)obj[n].offset);