All of lore.kernel.org
 help / color / mirror / Atom feed
From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
To: igt-dev@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: [PATCH i-g-t v5 2/4] tests/gem_exec_reloc: Don't filter out invalid addresses
Date: Mon,  4 Nov 2019 18:13:28 +0100	[thread overview]
Message-ID: <20191104171330.24821-3-janusz.krzysztofik@linux.intel.com> (raw)
In-Reply-To: <20191104171330.24821-1-janusz.krzysztofik@linux.intel.com>

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.  If incorrect GTT alignment will be 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 reported as occupied.

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.

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

diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index fdd9661d..e46a4df7 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -23,6 +23,7 @@
 
 #include "igt.h"
 #include "igt_dummyload.h"
+#include "i915/gem_gtt_topology.c"
 
 IGT_TEST_DESCRIPTION("Basic sanity check of execbuf-ioctl relocations.");
 
@@ -520,7 +521,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));
 
@@ -539,11 +540,12 @@ 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))
+		err = gem_gtt_validate_object(fd, &obj[n]);
+		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);
@@ -559,11 +561,12 @@ 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))
+		err = gem_gtt_validate_object(fd, &obj[n]);
+		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

WARNING: multiple messages have this Message-ID (diff)
From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
To: igt-dev@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH i-g-t v5 2/4] tests/gem_exec_reloc: Don't filter out invalid addresses
Date: Mon,  4 Nov 2019 18:13:28 +0100	[thread overview]
Message-ID: <20191104171330.24821-3-janusz.krzysztofik@linux.intel.com> (raw)
Message-ID: <20191104171328.9gSUsHfgCXpAReEWLxR2PJwJ_L38HLKpNAbNOI_Lfog@z> (raw)
In-Reply-To: <20191104171330.24821-1-janusz.krzysztofik@linux.intel.com>

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.  If incorrect GTT alignment will be 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 reported as occupied.

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.

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

diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index fdd9661d..e46a4df7 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -23,6 +23,7 @@
 
 #include "igt.h"
 #include "igt_dummyload.h"
+#include "i915/gem_gtt_topology.c"
 
 IGT_TEST_DESCRIPTION("Basic sanity check of execbuf-ioctl relocations.");
 
@@ -520,7 +521,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));
 
@@ -539,11 +540,12 @@ 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))
+		err = gem_gtt_validate_object(fd, &obj[n]);
+		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);
@@ -559,11 +561,12 @@ 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))
+		err = gem_gtt_validate_object(fd, &obj[n]);
+		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

WARNING: multiple messages have this Message-ID (diff)
From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
To: igt-dev@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: [igt-dev] [PATCH i-g-t v5 2/4] tests/gem_exec_reloc: Don't filter out invalid addresses
Date: Mon,  4 Nov 2019 18:13:28 +0100	[thread overview]
Message-ID: <20191104171330.24821-3-janusz.krzysztofik@linux.intel.com> (raw)
In-Reply-To: <20191104171330.24821-1-janusz.krzysztofik@linux.intel.com>

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.  If incorrect GTT alignment will be 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 reported as occupied.

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.

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

diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index fdd9661d..e46a4df7 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -23,6 +23,7 @@
 
 #include "igt.h"
 #include "igt_dummyload.h"
+#include "i915/gem_gtt_topology.c"
 
 IGT_TEST_DESCRIPTION("Basic sanity check of execbuf-ioctl relocations.");
 
@@ -520,7 +521,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));
 
@@ -539,11 +540,12 @@ 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))
+		err = gem_gtt_validate_object(fd, &obj[n]);
+		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);
@@ -559,11 +561,12 @@ 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))
+		err = gem_gtt_validate_object(fd, &obj[n]);
+		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

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  parent reply	other threads:[~2019-11-04 17:13 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-04 17:13 [PATCH i-g-t v5 0/4] Calculate softpin offsets from minimum GTT alignment Janusz Krzysztofik
2019-11-04 17:13 ` [igt-dev] " Janusz Krzysztofik
2019-11-04 17:13 ` [Intel-gfx] " Janusz Krzysztofik
2019-11-04 17:13 ` [PATCH i-g-t v5 1/4] lib/i915: Add minimum GTT alignment helper Janusz Krzysztofik
2019-11-04 17:13   ` [igt-dev] " Janusz Krzysztofik
2019-11-04 17:13   ` [Intel-gfx] " Janusz Krzysztofik
2019-11-05  9:14   ` [igt-dev] " Joonas Lahtinen
2019-11-05  9:14     ` [Intel-gfx] " Joonas Lahtinen
2019-11-06  8:18     ` Chris Wilson
2019-11-06  8:18       ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-11-06  8:18       ` [Intel-gfx] [igt-dev] " Chris Wilson
2019-11-08 16:51     ` Janusz Krzysztofik
2019-11-08 16:51       ` Janusz Krzysztofik
2019-11-08 16:51       ` [Intel-gfx] " Janusz Krzysztofik
2019-11-04 17:13 ` Janusz Krzysztofik [this message]
2019-11-04 17:13   ` [igt-dev] [PATCH i-g-t v5 2/4] tests/gem_exec_reloc: Don't filter out invalid addresses Janusz Krzysztofik
2019-11-04 17:13   ` [Intel-gfx] " Janusz Krzysztofik
2019-11-04 20:46   ` Vanshidhar Konda
2019-11-04 20:46     ` [igt-dev] " Vanshidhar Konda
2019-11-04 20:46     ` [Intel-gfx] " Vanshidhar Konda
2019-11-05 15:49     ` Janusz Krzysztofik
2019-11-05 15:49       ` [igt-dev] " Janusz Krzysztofik
2019-11-05 15:49       ` [Intel-gfx] " Janusz Krzysztofik
2019-11-04 17:13 ` [PATCH i-g-t v5 3/4] tests/gem_exec_reloc: Calculate offsets from minimum GTT alignment Janusz Krzysztofik
2019-11-04 17:13   ` [igt-dev] " Janusz Krzysztofik
2019-11-04 17:13   ` [Intel-gfx] " Janusz Krzysztofik
2019-11-04 20:48   ` Vanshidhar Konda
2019-11-04 20:48     ` [igt-dev] " Vanshidhar Konda
2019-11-04 20:48     ` [Intel-gfx] " Vanshidhar Konda
2019-11-04 17:13 ` [PATCH i-g-t v5 4/4] tests/gem_ctx_shared: Align objects using " Janusz Krzysztofik
2019-11-04 17:13   ` [Intel-gfx] " Janusz Krzysztofik
2019-11-04 20:49   ` Vanshidhar Konda
2019-11-04 20:49     ` [Intel-gfx] " Vanshidhar Konda
2019-11-04 19:13 ` [igt-dev] ✓ Fi.CI.BAT: success for Calculate softpin offsets from minimum GTT alignment (rev2) Patchwork
2019-11-05  4:23 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191104171330.24821-3-janusz.krzysztofik@linux.intel.com \
    --to=janusz.krzysztofik@linux.intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.