[Intel-gfx] [PATCH] igt/gem_mmap_wc: Add the invalid flags subtest

[Akash Goel]

On Tue, 2014-11-25 at 12:30 +0100, Daniel Vetter wrote:
> On Tue, Nov 25, 2014 at 02:28:52PM +0530, akash.goel at intel.com wrote:
> > From: Akash Goel <akash.goel at intel.com>
> > 
> > A new subtest added to validate the new version of gem_mmap ioctl,
> > for creating the wc mappings, on yet to be supported flags.
> > Older kernel is also checked against the flags field, which should
> > be treated as a don't care by it.
> > 
> > Signed-off-by: Akash Goel <akash.goel at intel.com>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  tests/gem_mmap_wc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 66 insertions(+)
> > 
> > diff --git a/tests/gem_mmap_wc.c b/tests/gem_mmap_wc.c
> > index 6f91a89..f923553 100644
> > --- a/tests/gem_mmap_wc.c
> > +++ b/tests/gem_mmap_wc.c
> > @@ -41,6 +41,17 @@
> >  #include "drmtest.h"
> >  #include "igt_debugfs.h"
> >  
> > +struct local_i915_gem_mmap_v2 {
> > +	uint32_t handle;
> > +	uint32_t pad;
> > +	uint64_t offset;
> > +	uint64_t size;
> > +	uint64_t addr_ptr;
> > +	uint64_t flags;
> > +#define I915_MMAP_WC 0x1
> > +};
> > +#define LOCAL_IOCTL_I915_GEM_MMAP_v2 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct local_i915_gem_mmap_v2)
> > +
> >  static int OBJECT_SIZE = 16*1024*1024;
> >  
> >  static void set_domain(int fd, uint32_t handle)
> > @@ -75,6 +86,59 @@ create_pointer(int fd)
> >  }
> >  
> >  static void
> > +test_invalid_flags(int fd)
> > +{
> > +	struct drm_i915_getparam gp;
> > +	struct local_i915_gem_mmap_v2 arg;
> > +	uint64_t flag = I915_MMAP_WC;
> > +	int val = -1;
> > +
> > +	memset(&arg, 0, sizeof(arg));
> > +	arg.handle = gem_create(fd, 4096);
> > +	arg.offset = 0;
> > +	arg.size = 4096;
> > +
> > +	memset(&gp, 0, sizeof(gp));
> > +	gp.param = 30; /* MMAP_VERSION */
> > +	gp.value = &val;
> > +
> > +	/* Do we have the new mmap_ioctl? */
> > +	do_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > +
> > +	if (val >= 1) {
> > +		/*
> > +		 * Only MMAP_WC flag is supported in version 1, so any other
> > +		 * flag should be rejected.
> > +		 */
> > +		flag <<= 1;
> > +		while (flag) {
> > +			arg.flags = flag;
> > +			igt_assert(drmIoctl(fd,
> > +				   LOCAL_IOCTL_I915_GEM_MMAP_v2,
> > +				   &arg) == -1);
> > +			igt_assert_eq(errno, EINVAL);
> > +			flag <<= 1;
> > +		}
> > +	} else {
> > +		/*
> > +		 * flags field should be ignored by older kernel
> > +		 * and so irrespective of the flag value passed,
> > +		 * mmap call should succeed
> > +		 */
> > +		while (flag) {
> > +			arg.flags = flag;
> > +			igt_assert(drmIoctl(fd,
> > +				   LOCAL_IOCTL_I915_GEM_MMAP_v2,
> > +				   &arg) == 0);
> > +			munmap(arg.addr_ptr, 4096);
> > +			flag <<= 1;
> > +		}
> > +	}
> 
> Imo just skip when the new flag stuff isn't available.

Ok will remove this checking for the flags on older kernels.
Thought that older kernel shouldn't react to flag values in anyway.  

> 
> > +
> > +	gem_close(fd, arg.handle);
> > +}
> > +
> > +static void
> >  test_copy(int fd)
> >  {
> >         void *src, *dst;
> > @@ -331,6 +395,8 @@ igt_main
> >         igt_fixture
> >                 fd = drm_open_any();
> >  
> > +       igt_subtest("invalid flags")
> 
> s/ /-/
> 
> Also can you please add invalid-foo tests for all the other ioctl
> paramters, too? That's an existing gap in our coverage and general rule is
> that the next person to touch it gets to fill it out (it's been a while
> though since that was last required, we have fairly good coverage
> nowadays).

Should a new subtest be added in 'gem_mmap' IGT, for checking on the
other parameters of i915_gem_mmap ioctl. 

Best regards
Akash

> 
> Thanks, Daniel
> 
> 
> > +               test_invalid_flags(fd);
> >         igt_subtest("copy")
> >                 test_copy(fd);
> >         igt_subtest("read")
> > -- 
> > 1.9.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>