public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: rafael.barbalho@intel.com
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] tests/gem_reloc_overflow: Add gen8+ specifc tests
Date: Wed, 6 Nov 2013 19:52:13 +0100	[thread overview]
Message-ID: <20131106185213.GN14082@phenom.ffwll.local> (raw)
In-Reply-To: <1383761532-30805-1-git-send-email-rafael.barbalho@intel.com>

On Wed, Nov 06, 2013 at 06:12:12PM +0000, rafael.barbalho@intel.com wrote:
> From: Rafael Barbalho <rafael.barbalho@intel.com>
> 
> Broadwell introduces 64-bit relocation addresses which add extra
> corner cases. The test was refactored slightly with some tests that
> were in the source offset tests were moved to the more generic reloc
> test area. The source offset tests are now gen aware and called twice to
> test both cpu & gtt relocation paths. In addition 2 new gen8+ test
> were added to the test:
> 
> * Relocation straddling page a page
> * Insufficient space for a relocation at the end of the buffer.
> 
> Signed-off-by: Rafael Barbalho <rafael.barbalho@intel.com>
> 
> Conflicts:
> 	tests/gem_reloc_overflow.c
> ---
>  tests/gem_reloc_overflow.c | 148 +++++++++++++++++++++++++++++++--------------
>  1 file changed, 103 insertions(+), 45 deletions(-)
> 
> diff --git a/tests/gem_reloc_overflow.c b/tests/gem_reloc_overflow.c
> index f7ba1d7..38ad2a5 100644
> --- a/tests/gem_reloc_overflow.c
> +++ b/tests/gem_reloc_overflow.c
> @@ -24,6 +24,7 @@
>   * Authors:
>   *    Kees Cook <keescook@chromium.org>
>   *    Daniel Vetter <daniel.vetter@ffwll.ch>
> + *    Rafael Barbalho <rafael.barbalho@intel.com>
>   *
>   */
>  
> @@ -60,13 +61,14 @@ struct drm_i915_gem_relocation_entry *reloc;
>  uint32_t handle;
>  uint32_t batch_handle;
>  
> -
> -static void source_offset_tests(void)
> +static void source_offset_tests(int devid, bool reloc_gtt)
>  {
>  	struct drm_i915_gem_relocation_entry single_reloc;
> +	char *dst_gtt;
> +	char *relocation_type;
>  
>  	igt_fixture {
> -		handle = gem_create(fd, 4096);
> +		handle = gem_create(fd, 8192);
>  
>  		execobjs[1].handle = batch_handle;
>  		execobjs[1].relocation_count = 0;
> @@ -76,21 +78,70 @@ static void source_offset_tests(void)
>  		execobjs[0].relocation_count = 1;
>  		execobjs[0].relocs_ptr = (uintptr_t) &single_reloc;
>  		execbuf.buffer_count = 2;
> +
> +		if (reloc_gtt) {
> +			dst_gtt = gem_mmap(fd, handle, 8192, PROT_READ | PROT_WRITE);
> +			igt_assert(dst_gtt != MAP_FAILED);
> +			gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> +			memset(dst_gtt, 0, 8192);
> +			munmap(dst_gtt, 8192);
> +			relocation_type = "reloc-gtt";
> +		} else {
> +			relocation_type = "reloc-cpu";
> +		}
>  	}
>  
> -	igt_subtest("source-offset-end") {
> -		single_reloc.offset = 4096 - 4;
> -		single_reloc.delta = 0;
> -		single_reloc.target_handle = handle;
> -		single_reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> -		single_reloc.write_domain = I915_GEM_DOMAIN_RENDER;
> -		single_reloc.presumed_offset = 0;
> +	if (intel_gen(devid) >= 8) {
> +		igt_subtest_f("source-offset-page-stradle-gen8+-%s", relocation_type) {
> +			single_reloc.offset = 4096 - 4;
> +			single_reloc.delta = 0;
> +			single_reloc.target_handle = handle;
> +			single_reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.write_domain = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.presumed_offset = 0;
> +
> +			igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) == 0);
> +			single_reloc.delta = 1024;
> +			igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) == 0);
> +		}
>  
> -		igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) == 0);
> +		igt_subtest_f("source-offset-end-gen8+-%s", relocation_type) {
> +			single_reloc.offset = 8192 - 8;
> +			single_reloc.delta = 0;
> +			single_reloc.target_handle = handle;
> +			single_reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.write_domain = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.presumed_offset = 0;
> +
> +			igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) == 0);
> +		}
> +
> +		igt_subtest_f("source-offset-overflow-gen8+-%s", relocation_type) {
> +			single_reloc.offset = 8192 - 4;
> +			single_reloc.delta = 0;
> +			single_reloc.target_handle = handle;
> +			single_reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.write_domain = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.presumed_offset = 0;
> +
> +			igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) != 0);
> +			igt_assert(errno == EINVAL);
> +		}
> +	} else {
> +		igt_subtest_f("source-offset-end-%s", relocation_type) {
> +			single_reloc.offset = 8192 - 4;
> +			single_reloc.delta = 0;
> +			single_reloc.target_handle = handle;
> +			single_reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.write_domain = I915_GEM_DOMAIN_RENDER;
> +			single_reloc.presumed_offset = 0;
> +
> +			igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) == 0);
> +		}
>  	}
>  
> -	igt_subtest("source-offset-big") {
> -		single_reloc.offset = 4096;
> +	igt_subtest_f("source-offset-big-%s", relocation_type) {
> +		single_reloc.offset = 8192;
>  		single_reloc.delta = 0;
>  		single_reloc.target_handle = handle;
>  		single_reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> @@ -101,7 +152,7 @@ static void source_offset_tests(void)
>  		igt_assert(errno == EINVAL);
>  	}
>  
> -	igt_subtest("source-offset-negative") {
> +	igt_subtest_f("source-offset-negative-%s", relocation_type) {
>  		single_reloc.offset = (int64_t) -4;
>  		single_reloc.delta = 0;
>  		single_reloc.target_handle = handle;
> @@ -113,7 +164,7 @@ static void source_offset_tests(void)
>  		igt_assert(errno == EINVAL);
>  	}
>  
> -	igt_subtest("source-offset-unaligned") {
> +	igt_subtest_f("source-offset-unaligned-%s", relocation_type) {
>  		single_reloc.offset = 1;
>  		single_reloc.delta = 0;
>  		single_reloc.target_handle = handle;
> @@ -126,34 +177,6 @@ static void source_offset_tests(void)
>  	}
>  
>  	igt_fixture {
> -		execobjs[0].handle = batch_handle;
> -		execobjs[0].relocation_count = 0;
> -		execobjs[0].relocs_ptr = 0;
> -
> -		execbuf.buffer_count = 1;
> -	}
> -
> -	igt_subtest("batch-start-unaligend") {
> -		execbuf.batch_start_offset = 1;
> -		execbuf.batch_len = 8;
> -
> -		igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) != 0);
> -		igt_assert(errno == EINVAL);
> -	}
> -
> -	igt_subtest("batch-end-unaligend") {
> -		execbuf.batch_start_offset = 0;
> -		execbuf.batch_len = 7;
> -
> -		igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) != 0);
> -		igt_assert(errno == EINVAL);
> -	}
> -
> -	igt_fixture {
> -		/* Undo damage for next tests. */
> -		execbuf.batch_start_offset = 0;
> -		execbuf.batch_len = 8;
> -
>  		gem_close(fd, handle);
>  	}
>  }
> @@ -185,6 +208,36 @@ static void reloc_tests(void)
>  		igt_assert(errno == EINVAL);
>  	}
>  
> +	igt_fixture {
> +		execobjs[0].handle = batch_handle;
> +		execobjs[0].relocation_count = 0;
> +		execobjs[0].relocs_ptr = 0;
> +
> +		execbuf.buffer_count = 1;
> +	}
> +
> +	igt_subtest("batch-start-unaligned") {
> +		execbuf.batch_start_offset = 1;
> +		execbuf.batch_len = 8;
> +
> +		igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) != 0);
> +		igt_assert(errno == EINVAL);
> +	}
> +
> +	igt_subtest("batch-end-unaligned") {
> +		execbuf.batch_start_offset = 0;
> +		execbuf.batch_len = 7;
> +
> +		igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) != 0);
> +		igt_assert(errno == EINVAL);
> +	}
> +
> +	igt_fixture {
> +		/* Undo damage for next tests. */
> +		execbuf.batch_start_offset = 0;
> +		execbuf.batch_len = 8;
> +	}

Moving these tests around is tricky - they should be carefully constructed
so that the wrong batch start/end is the only thing which is wrong with
the metadata, and that with correct start/len (0, 8) it would execute
perfectly.

This is to make sure that we really exercise this cornercase and don't get
caught in some other check (that originally was done later but then might
have moved around).

tldr; Have you check that this is still true?
-Daniel
> +
>  	igt_subtest("wrapped-overflow") {
>  		/* Attempt wrapped overflow entries. */
>  		for (i = 0; i < num; i++) {
> @@ -252,12 +305,16 @@ static void buffer_count_tests(void)
>  
>  igt_main
>  {
> +	int devid = 0;
> +
>  	igt_fixture {
>  		int ring;
>  		uint32_t batch_data [2] = { MI_NOOP, MI_BATCH_BUFFER_END };
>  
>  		fd = drm_open_any();
>  
> +		devid = intel_get_drm_devid(fd);
> +
>  		/* Create giant reloc buffer area. */
>  		num = 257;
>  		entries = ((1ULL << 32) / (num - 1));
> @@ -271,7 +328,7 @@ igt_main
>  		for (int i = 0; i < num; i++)
>  			handles[i] = gem_create(fd, 4096);
>  
> -		if (intel_gen(intel_get_drm_devid(fd)) >= 6)
> +		if (intel_gen(devid) >= 6)
>  			ring = I915_EXEC_BLT;
>  		else
>  			ring = 0;
> @@ -296,7 +353,8 @@ igt_main
>  
>  	reloc_tests();
>  
> -	source_offset_tests();
> +	source_offset_tests(devid, false);
> +	source_offset_tests(devid, true);
>  
>  	buffer_count_tests();
>  
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2013-11-06 18:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-06 18:12 [PATCH] tests/gem_reloc_overflow: Add gen8+ specifc tests rafael.barbalho
2013-11-06 18:52 ` Daniel Vetter [this message]
2013-11-06 19:52   ` Barbalho, Rafael
2013-11-07 13:05     ` Daniel Vetter
2013-11-07 13:15       ` Daniel Vetter

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=20131106185213.GN14082@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rafael.barbalho@intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox