[i-g-t] tests/gem_bad_reloc: use correct page table size
diff mbox

Message ID 1441296838-14072-1-git-send-email-daniele.ceraolospurio@intel.com
State New
Headers show

Commit Message

Daniele Ceraolo Spurio Sept. 3, 2015, 4:13 p.m. UTC
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

2 subparts of gem_bad_reloc check that the reloc address is below the
global gtt boundary. However, when executing from ppgtt the reloc
address can be greater than that and still be a valid address.

To be sure that we're using the right upper limit, select it based on
the ppgtt mode.

Cc: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 tests/gem_bad_reloc.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

Comments

Michel Thierry Sept. 8, 2015, 12:30 p.m. UTC | #1
On 9/3/2015 5:13 PM, daniele.ceraolospurio@intel.com wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> 2 subparts of gem_bad_reloc check that the reloc address is below the
> global gtt boundary. However, when executing from ppgtt the reloc
> address can be greater than that and still be a valid address.
>
> To be sure that we're using the right upper limit, select it based on
> the ppgtt mode.
>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Yes, we need this, specially when we use top-down allocation.
(you could use %'lld to add thousand separators into these long long 
outputs).

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

> ---
>   tests/gem_bad_reloc.c | 32 +++++++++++++++++++++++++++++---
>   1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/tests/gem_bad_reloc.c b/tests/gem_bad_reloc.c
> index 563571e..d2e0b70 100644
> --- a/tests/gem_bad_reloc.c
> +++ b/tests/gem_bad_reloc.c
> @@ -44,6 +44,32 @@ IGT_TEST_DESCRIPTION("Simulates SNA behaviour using negative self-relocations"
>
>   #define USE_LUT (1 << 12)
>
> +static uint64_t get_page_table_size(int fd)
> +{
> +	struct drm_i915_getparam gp;
> +	int val = 0;
> +
> +	memset(&gp, 0, sizeof(gp));
> +	gp.param = 18; /* HAS_ALIASING_PPGTT */
> +	gp.value = &val;
> +
> +	if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
> +		return 0;
> +	errno = 0;
> +
> +	switch (val) {
> +	case 0:
> +	case 1:
> +		return gem_aperture_size(fd);
> +	case 2:
> +		return 1ULL << 32;
> +	case 3:
> +		return 1ULL << 48;
> +	}
> +
> +	return 0;
> +}
> +
>   /* Simulates SNA behaviour using negative self-relocations for
>    * STATE_BASE_ADDRESS command packets. If they wrap around (to values greater
>    * than the total size of the GTT), the GPU will hang.
> @@ -54,7 +80,7 @@ static int negative_reloc(int fd, unsigned flags)
>   	struct drm_i915_gem_execbuffer2 execbuf;
>   	struct drm_i915_gem_exec_object2 gem_exec[2];
>   	struct drm_i915_gem_relocation_entry gem_reloc[1000];
> -	uint64_t gtt_max = gem_aperture_size(fd);
> +	uint64_t gtt_max = get_page_table_size(fd);
>   	uint32_t buf[1024] = {MI_BATCH_BUFFER_END};
>   	int i;
>
> @@ -86,7 +112,7 @@ static int negative_reloc(int fd, unsigned flags)
>   			   &execbuf));
>   	gem_close(fd, gem_exec[1].handle);
>
> -	igt_info("Found offset %ld for 4k batch\n", (long)gem_exec[0].offset);
> +	igt_info("Found offset %lld for 4k batch\n", (long long)gem_exec[0].offset);
>   	/*
>   	 * Ideally we'd like to be able to control where the kernel is going to
>   	 * place the buffer. We don't SKIP here because it causes the test
> @@ -114,7 +140,7 @@ static int negative_reloc(int fd, unsigned flags)
>   			   DRM_IOCTL_I915_GEM_EXECBUFFER2,
>   			   &execbuf));
>
> -	igt_info("Batch is now at offset %ld\n", (long)gem_exec[0].offset);
> +	igt_info("Batch is now at offset %lld\n", (long long)gem_exec[0].offset);
>
>   	gem_read(fd, gem_exec[0].handle, 0, buf, sizeof(buf));
>   	gem_close(fd, gem_exec[0].handle);
>

Patch
diff mbox

diff --git a/tests/gem_bad_reloc.c b/tests/gem_bad_reloc.c
index 563571e..d2e0b70 100644
--- a/tests/gem_bad_reloc.c
+++ b/tests/gem_bad_reloc.c
@@ -44,6 +44,32 @@  IGT_TEST_DESCRIPTION("Simulates SNA behaviour using negative self-relocations"
 
 #define USE_LUT (1 << 12)
 
+static uint64_t get_page_table_size(int fd)
+{
+	struct drm_i915_getparam gp;
+	int val = 0;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.param = 18; /* HAS_ALIASING_PPGTT */
+	gp.value = &val;
+
+	if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
+		return 0;
+	errno = 0;
+
+	switch (val) {
+	case 0:
+	case 1:
+		return gem_aperture_size(fd);
+	case 2:
+		return 1ULL << 32;
+	case 3:
+		return 1ULL << 48;
+	}
+
+	return 0;
+}
+
 /* Simulates SNA behaviour using negative self-relocations for
  * STATE_BASE_ADDRESS command packets. If they wrap around (to values greater
  * than the total size of the GTT), the GPU will hang.
@@ -54,7 +80,7 @@  static int negative_reloc(int fd, unsigned flags)
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_exec_object2 gem_exec[2];
 	struct drm_i915_gem_relocation_entry gem_reloc[1000];
-	uint64_t gtt_max = gem_aperture_size(fd);
+	uint64_t gtt_max = get_page_table_size(fd);
 	uint32_t buf[1024] = {MI_BATCH_BUFFER_END};
 	int i;
 
@@ -86,7 +112,7 @@  static int negative_reloc(int fd, unsigned flags)
 			   &execbuf));
 	gem_close(fd, gem_exec[1].handle);
 
-	igt_info("Found offset %ld for 4k batch\n", (long)gem_exec[0].offset);
+	igt_info("Found offset %lld for 4k batch\n", (long long)gem_exec[0].offset);
 	/*
 	 * Ideally we'd like to be able to control where the kernel is going to
 	 * place the buffer. We don't SKIP here because it causes the test
@@ -114,7 +140,7 @@  static int negative_reloc(int fd, unsigned flags)
 			   DRM_IOCTL_I915_GEM_EXECBUFFER2,
 			   &execbuf));
 
-	igt_info("Batch is now at offset %ld\n", (long)gem_exec[0].offset);
+	igt_info("Batch is now at offset %lld\n", (long long)gem_exec[0].offset);
 
 	gem_read(fd, gem_exec[0].handle, 0, buf, sizeof(buf));
 	gem_close(fd, gem_exec[0].handle);