From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] tests/gem_reloc_overflow: Add gen8+ specifc tests Date: Thu, 7 Nov 2013 14:05:55 +0100 Message-ID: <20131107130555.GR14082@phenom.ffwll.local> References: <1383761532-30805-1-git-send-email-rafael.barbalho@intel.com> <20131106185213.GN14082@phenom.ffwll.local> <4B498744C37F034EA16F6FBC6AB9FAD201414F58@IRSMSX103.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f51.google.com (mail-ee0-f51.google.com [74.125.83.51]) by gabe.freedesktop.org (Postfix) with ESMTP id 2750CFABEF for ; Thu, 7 Nov 2013 05:05:26 -0800 (PST) Received: by mail-ee0-f51.google.com with SMTP id t10so288231eei.10 for ; Thu, 07 Nov 2013 05:05:26 -0800 (PST) Content-Disposition: inline In-Reply-To: <4B498744C37F034EA16F6FBC6AB9FAD201414F58@IRSMSX103.ger.corp.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: "Barbalho, Rafael" Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Wed, Nov 06, 2013 at 07:52:23PM +0000, Barbalho, Rafael wrote: > > > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel > > Vetter > > Sent: Wednesday, November 06, 2013 6:52 PM > > To: Barbalho, Rafael > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] tests/gem_reloc_overflow: Add gen8+ > > specifc tests > > > > * snip * > > > > @@ -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? > > When I first moved the test I did try to make sure to move the igt_fixture > that set up the state for the tests did the correct thing. Just to make sure that I > didn't screw things up I've re-run tests with an annotated kernel and it is > behaving as expected with the driver returning false from > i915_gem_check_execbuffer due to buffer misalignment. > > tldr; Yes. Cool, thanks for testing. Since I'm paranoid about this I've added an assert into the relevant fixture block to make sure the batch would really work safe for the condition we're testing. That way we're maximally robust against kernel changes that move the tests around. Also fixed some compile warnings in a follow-up. Thanks for your patch, applied to i-g-t. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch