Re: [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

On Fri, 2015-08-21 at 16:26 +0300, Ander Conselvan De Oliveira wrote:
> On Fri, 2015-08-07 at 15:53 +0300, David Weinehall wrote:
> > On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> > > This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
> > > 
> > > The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> > > despite our best efforts we really suck at properly reviewing for test
> > > coverage when extending ABI.
> > > 
> > > The real bug here is that David Weinhall hasn't submitted updated igts
> > > for the NO_ZEROMAP feature yet. Imo the right course of action is to
> > > revert that feature if the testcase don't show up within a few days.
> > 
> > The reason I never submitted it was probably because of Chris's strong
> > opposition to the feature in the first place; I've had the testcase
> > laying around on my computer for quite a while.
> > 
> > Anyhow, here's a slightly modified version of that test -- hopefully
> > not breaking anything.
> > 
> > 
> > Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
> > 
> > diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> > index bc5d4bd827cf..f4deca6bd79e 100644
> > --- a/lib/ioctl_wrappers.h
> > +++ b/lib/ioctl_wrappers.h
> > @@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
> >  	uint32_t size;
> >  	uint64_t param;
> >  #define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
> > +#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
> >  	uint64_t value;
> >  };
> >  void gem_context_require_ban_period(int fd);
> > diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> > index b44b37cf0538..1e7e8ff40703 100644
> > --- a/tests/gem_ctx_param_basic.c
> > +++ b/tests/gem_ctx_param_basic.c
> > @@ -57,7 +57,7 @@ igt_main
> >  		ctx = gem_context_create(fd);
> >  	}
> >  
> > -	ctx_param.param  = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> > +	ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> >  
> >  	igt_subtest("basic") {
> >  		ctx_param.context = ctx;
> > @@ -98,21 +98,31 @@ igt_main
> 
> [...]
> 
> > -	ctx_param.param  = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> > +	ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
> >  
> > -	igt_subtest("non-root-set") {
> > +	igt_subtest("non-root-set-no-zeromap") {
> >  		igt_fork(child, 1) {
> >  			igt_drop_root();
> 
>  		ctx_param.context = ctx;
> 		TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
> 		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> 		ctx_param.value--;
> 		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> 
> (I've added the code missing from the context)

Except I added the wrong code. Here's what is in i-g-t now:

        ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;

        igt_subtest("non-root-set-no-zeromap") {
                igt_fork(child, 1) {
                        igt_drop_root();

                        ctx_param.context = ctx;
                        TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
                        ctx_param.value--;
                        TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM); 
                }

                igt_waitchildren();
        }

> The code in i915_gem_context_setparam_ioctl() that handles CONTEXT_PARAM_NO_ZEROMAP never returns
> EPERM, so this test always fails.

Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx