Re: [PATCH] igt/gem_mmap_wc: Add the invalid flags subtest

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Nov 25, 2014 at 02:28:52PM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@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@intel.com>
> Signed-off-by: Chris Wilson <chris@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.

> +
> +	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).

Thanks, Daniel


> +               test_invalid_flags(fd);
>         igt_subtest("copy")
>                 test_copy(fd);
>         igt_subtest("read")
> -- 
> 1.9.2
> 
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx