[igt-dev] [PATCH i-g-t 0/4] Add tests for newly invalid cases

[igt-dev] [PATCH i-g-t 0/4] Add tests for newly invalid cases

[Jason Ekstrand]

As part of the ioctl clean-ups, I've made a few unused cases of
get/set_param return -EINVAL.  This little series adds IGT tests for those
cases.  Daniel, please let me know if you see any I missed.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Jason Ekstrand (4):
  tests/i915/gem_ctx_param: Add tests for recently removed params
  tests/i915/gem_ctx_param: Add a couple invalid PARAM_VM cases
  tests/i915/gem_ctx_engines: Add more invalid SET_CONTEXT_PARAM cases
  tests/i915/gem_exec_balancer: Add a test for combind balancing and
    bonding

 tests/i915/gem_ctx_engines.c   | 20 +++++++++--
 tests/i915/gem_ctx_param.c     | 61 ++++++++++++++++++++++++++++++++++
 tests/i915/gem_exec_balancer.c | 26 +++++++++++++++
 3 files changed, 105 insertions(+), 2 deletions(-)

-- 
2.31.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 1/4] tests/i915/gem_ctx_param: Add tests for recently removed params

[Jason Ekstrand]

---
 tests/i915/gem_ctx_param.c | 44 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tests/i915/gem_ctx_param.c b/tests/i915/gem_ctx_param.c
index 3c27c78c..ae802b8a 100644
--- a/tests/i915/gem_ctx_param.c
+++ b/tests/i915/gem_ctx_param.c
@@ -243,6 +243,35 @@ static void test_vm(int i915)
 	gem_close(i915, batch.handle);
 }
 
+static void test_set_invalid_param(int fd, uint64_t param, uint64_t value)
+{
+	/* Create a fresh context */
+	struct drm_i915_gem_context_param arg = {
+		.ctx_id = gem_context_create(fd),
+		.param = param,
+		.value = value,
+	};
+	int err;
+
+	err = __gem_context_set_param(fd, &arg);
+	gem_context_destroy(fd, arg.ctx_id);
+	igt_assert_eq(err, -EINVAL);
+}
+
+static void test_get_invalid_param(int fd, uint64_t param)
+{
+	/* Create a fresh context */
+	struct drm_i915_gem_context_param arg = {
+		.ctx_id = gem_context_create(fd),
+		.param = param,
+	};
+	int err;
+
+	err = __gem_context_get_param(fd, &arg);
+	gem_context_destroy(fd, arg.ctx_id);
+	igt_assert_eq(err, -EINVAL);
+}
+
 igt_main
 {
 	struct drm_i915_gem_context_param arg;
@@ -405,6 +434,21 @@ igt_main
 		igt_assert_eq(__gem_context_set_param(fd, &arg), -EINVAL);
 	}
 
+	igt_subtest("invalid-set-ringsize")
+		test_set_invalid_param(fd, I915_CONTEXT_PARAM_RINGSIZE, 8192);
+
+	igt_subtest("invalid-get-ringsize")
+		test_get_invalid_param(fd, I915_CONTEXT_PARAM_RINGSIZE);
+
+	igt_subtest("invalid-set-no-zeromap")
+		test_set_invalid_param(fd, I915_CONTEXT_PARAM_NO_ZEROMAP, 1);
+
+	igt_subtest("invalid-get-no-zeromap")
+		test_get_invalid_param(fd, I915_CONTEXT_PARAM_NO_ZEROMAP);
+
+	igt_subtest("invalid-get-engines")
+		test_get_invalid_param(fd, I915_CONTEXT_PARAM_ENGINES);
+
 	igt_fixture
 		close(fd);
 }
-- 
2.31.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/4] tests/i915/gem_ctx_param: Add a couple invalid PARAM_VM cases

[Jason Ekstrand]

---
 tests/i915/gem_ctx_param.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tests/i915/gem_ctx_param.c b/tests/i915/gem_ctx_param.c
index ae802b8a..69184bd7 100644
--- a/tests/i915/gem_ctx_param.c
+++ b/tests/i915/gem_ctx_param.c
@@ -180,6 +180,23 @@ static void test_vm(int i915)
 	gem_context_destroy(i915, arg.ctx_id);
 	igt_require(err == -ENOENT);
 
+	/* Test that we can't set the VM on ctx0 */
+	arg.ctx_id = 0;
+	arg.value = gem_vm_create(i915);
+	err = __gem_context_set_param(i915, &arg);
+	gem_vm_destroy(i915, arg.value);
+	igt_assert_eq(err, -EINVAL);
+
+	/* Test that we can't set the VM after we've done an execbuf */
+	arg.ctx_id = gem_context_create(i915);
+	spin = igt_spin_new(i915, .ctx_id = arg.ctx_id);
+	igt_spin_free(i915, spin);
+	arg.value = gem_vm_create(i915);
+	err = __gem_context_set_param(i915, &arg);
+	gem_context_destroy(i915, arg.ctx_id);
+	gem_vm_destroy(i915, arg.value);
+	igt_assert_eq(err, -EINVAL);
+
 	parent = gem_context_create(i915);
 	child = gem_context_create(i915);
 
-- 
2.31.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 3/4] tests/i915/gem_ctx_engines: Add more invalid SET_CONTEXT_PARAM cases

[Jason Ekstrand]

---
 tests/i915/gem_ctx_engines.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
index e8d88aa4..f4ee990b 100644
--- a/tests/i915/gem_ctx_engines.c
+++ b/tests/i915/gem_ctx_engines.c
@@ -48,12 +48,14 @@
 static void invalid_engines(int i915)
 {
 	struct i915_context_param_engines stack = {}, *engines;
+	uint32_t ctx_id = gem_context_create(i915);
 	struct drm_i915_gem_context_param param = {
-		.ctx_id = gem_context_create(i915),
+		.ctx_id = ctx_id,
 		.param = I915_CONTEXT_PARAM_ENGINES,
 		.value = to_user_pointer(&stack),
 	};
 	uint32_t handle;
+	igt_spin_t *spin;
 	void *ptr;
 
 	param.size = 0;
@@ -68,6 +70,9 @@ static void invalid_engines(int i915)
 	param.size = sizeof(stack) + 1;
 	igt_assert_eq(__gem_context_set_param(i915, &param), -EINVAL);
 
+	param.size = sizeof(*engines) + (I915_EXEC_RING_MASK + 2) * sizeof(*engines->engines);
+	igt_assert_eq(__gem_context_set_param(i915, &param), -EINVAL);
+
 	param.size = 0;
 	igt_assert_eq(__gem_context_set_param(i915, &param), 0);
 
@@ -159,9 +164,20 @@ static void invalid_engines(int i915)
 
 	munmap(ptr + 4096, 4096);
 
+	/* Test that we can't set engines after we've done an execbuf */
+	spin = igt_spin_new(i915, .ctx_id = ctx_id);
+	igt_spin_free(i915, spin);
+	param.size = 0;
+	igt_assert_eq(__gem_context_set_param(i915, &param), -EINVAL);
+
+	/* Test that we can't set engines on ctx0 */
+	param.ctx_id = 0;
+	param.size = 0;
+	igt_assert_eq(__gem_context_set_param(i915, &param), -ENOENT);
+
 out:
 	munmap(engines, 4096);
-	gem_context_destroy(i915, param.ctx_id);
+	gem_context_destroy(i915, ctx_id);
 }
 
 static uint32_t batch_create(int i915)
-- 
2.31.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 4/4] tests/i915/gem_exec_balancer: Add a test for combind balancing and bonding

[Jason Ekstrand]

---
 tests/i915/gem_exec_balancer.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index 4f1fa390..b594e2e1 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -167,6 +167,7 @@ static uint32_t batch_create(int i915)
 static void invalid_balancer(int i915)
 {
 	I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, 64);
+	I915_DEFINE_CONTEXT_ENGINES_BOND(bond, 1);
 	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 64);
 	struct drm_i915_gem_context_param p = {
 		.param = I915_CONTEXT_PARAM_ENGINES,
@@ -283,6 +284,31 @@ static void invalid_balancer(int i915)
 
 		munmap(ptr + 4096, 4096);
 
+		if (count >= 2) {
+			/* You can't bond to a balanced engine */
+			memset(&bond, 0, sizeof(bond));
+			bond.base.name = I915_CONTEXT_ENGINES_EXT_BOND;
+			bond.master = ci[0];
+			bond.virtual_index = 0;
+			bond.num_bonds = 1;
+			bond.engines[0] = ci[1];
+
+			balancer.base.next_extension = to_user_pointer(&bond);
+			balancer.engine_index = 0;
+			balancer.num_siblings = count;
+			memcpy(balancer.engines, ci, count * sizeof(*ci));
+
+			memset(&engines, 0, sizeof(engines));
+			engines.engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
+			engines.engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
+			engines.extensions = to_user_pointer(&balancer);
+
+			p.size = (sizeof(struct i915_context_param_engines) +
+				  sizeof(*engines.engines));
+
+			igt_assert_eq(__gem_context_set_param(i915, &p), -EINVAL);
+		}
+
 		gem_context_destroy(i915, p.ctx_id);
 		free(ci);
 	}
-- 
2.31.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] tests/i915/gem_exec_balancer: Add a test for combind balancing and bonding

[Daniel Vetter]

On Wed, Apr 28, 2021 at 03:21:43PM -0500, Jason Ekstrand wrote:

In the subject of this one: s/combind/combined/

I think that's fairly complete sanity check. On the series:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I think best if you include this in your overall igt set (can't be CI'ed
otherwise) and the Test-With: check this together with your kernel set.

Cheers, Daniel

> ---
>  tests/i915/gem_exec_balancer.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> index 4f1fa390..b594e2e1 100644
> --- a/tests/i915/gem_exec_balancer.c
> +++ b/tests/i915/gem_exec_balancer.c
> @@ -167,6 +167,7 @@ static uint32_t batch_create(int i915)
>  static void invalid_balancer(int i915)
>  {
>  	I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, 64);
> +	I915_DEFINE_CONTEXT_ENGINES_BOND(bond, 1);
>  	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 64);
>  	struct drm_i915_gem_context_param p = {
>  		.param = I915_CONTEXT_PARAM_ENGINES,
> @@ -283,6 +284,31 @@ static void invalid_balancer(int i915)
>  
>  		munmap(ptr + 4096, 4096);
>  
> +		if (count >= 2) {
> +			/* You can't bond to a balanced engine */
> +			memset(&bond, 0, sizeof(bond));
> +			bond.base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> +			bond.master = ci[0];
> +			bond.virtual_index = 0;
> +			bond.num_bonds = 1;
> +			bond.engines[0] = ci[1];
> +
> +			balancer.base.next_extension = to_user_pointer(&bond);
> +			balancer.engine_index = 0;
> +			balancer.num_siblings = count;
> +			memcpy(balancer.engines, ci, count * sizeof(*ci));
> +
> +			memset(&engines, 0, sizeof(engines));
> +			engines.engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
> +			engines.engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
> +			engines.extensions = to_user_pointer(&balancer);
> +
> +			p.size = (sizeof(struct i915_context_param_engines) +
> +				  sizeof(*engines.engines));
> +
> +			igt_assert_eq(__gem_context_set_param(i915, &p), -EINVAL);
> +		}
> +
>  		gem_context_destroy(i915, p.ctx_id);
>  		free(ci);
>  	}
> -- 
> 2.31.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] tests/i915/gem_exec_balancer: Add a test for combind balancing and bonding

[Daniel Vetter]

On Thu, Jun 03, 2021 at 01:46:09PM +0200, Daniel Vetter wrote:
> On Wed, Apr 28, 2021 at 03:21:43PM -0500, Jason Ekstrand wrote:
> 
> In the subject of this one: s/combind/combined/
> 
> I think that's fairly complete sanity check. On the series:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I think best if you include this in your overall igt set (can't be CI'ed
> otherwise) and the Test-With: check this together with your kernel set.

Oh and sobs missing.
-Daniel

> 
> Cheers, Daniel
> 
> > ---
> >  tests/i915/gem_exec_balancer.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> > index 4f1fa390..b594e2e1 100644
> > --- a/tests/i915/gem_exec_balancer.c
> > +++ b/tests/i915/gem_exec_balancer.c
> > @@ -167,6 +167,7 @@ static uint32_t batch_create(int i915)
> >  static void invalid_balancer(int i915)
> >  {
> >  	I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, 64);
> > +	I915_DEFINE_CONTEXT_ENGINES_BOND(bond, 1);
> >  	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 64);
> >  	struct drm_i915_gem_context_param p = {
> >  		.param = I915_CONTEXT_PARAM_ENGINES,
> > @@ -283,6 +284,31 @@ static void invalid_balancer(int i915)
> >  
> >  		munmap(ptr + 4096, 4096);
> >  
> > +		if (count >= 2) {
> > +			/* You can't bond to a balanced engine */
> > +			memset(&bond, 0, sizeof(bond));
> > +			bond.base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> > +			bond.master = ci[0];
> > +			bond.virtual_index = 0;
> > +			bond.num_bonds = 1;
> > +			bond.engines[0] = ci[1];
> > +
> > +			balancer.base.next_extension = to_user_pointer(&bond);
> > +			balancer.engine_index = 0;
> > +			balancer.num_siblings = count;
> > +			memcpy(balancer.engines, ci, count * sizeof(*ci));
> > +
> > +			memset(&engines, 0, sizeof(engines));
> > +			engines.engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
> > +			engines.engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
> > +			engines.extensions = to_user_pointer(&balancer);
> > +
> > +			p.size = (sizeof(struct i915_context_param_engines) +
> > +				  sizeof(*engines.engines));
> > +
> > +			igt_assert_eq(__gem_context_set_param(i915, &p), -EINVAL);
> > +		}
> > +
> >  		gem_context_destroy(i915, p.ctx_id);
> >  		free(ci);
> >  	}
> > -- 
> > 2.31.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] tests/i915/gem_exec_balancer: Add a test for combind balancing and bonding

[Daniel Vetter]

On Thu, Jun 03, 2021 at 01:47:03PM +0200, Daniel Vetter wrote:
> On Thu, Jun 03, 2021 at 01:46:09PM +0200, Daniel Vetter wrote:
> > On Wed, Apr 28, 2021 at 03:21:43PM -0500, Jason Ekstrand wrote:
> > 
> > In the subject of this one: s/combind/combined/
> > 
> > I think that's fairly complete sanity check. On the series:
> > 
> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > I think best if you include this in your overall igt set (can't be CI'ed
> > otherwise) and the Test-With: check this together with your kernel set.
> 
> Oh and sobs missing.

One more: For the new tests please add igt_describe(). Also since you
looked at these quite a bit, would be really great to do a follow-up patch
and roll out igt_describe for the ctx tests you're touching. At least
where it's obvious what's getting tested.
-Daniel

> -Daniel
> 
> > 
> > Cheers, Daniel
> > 
> > > ---
> > >  tests/i915/gem_exec_balancer.c | 26 ++++++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > > 
> > > diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> > > index 4f1fa390..b594e2e1 100644
> > > --- a/tests/i915/gem_exec_balancer.c
> > > +++ b/tests/i915/gem_exec_balancer.c
> > > @@ -167,6 +167,7 @@ static uint32_t batch_create(int i915)
> > >  static void invalid_balancer(int i915)
> > >  {
> > >  	I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, 64);
> > > +	I915_DEFINE_CONTEXT_ENGINES_BOND(bond, 1);
> > >  	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 64);
> > >  	struct drm_i915_gem_context_param p = {
> > >  		.param = I915_CONTEXT_PARAM_ENGINES,
> > > @@ -283,6 +284,31 @@ static void invalid_balancer(int i915)
> > >  
> > >  		munmap(ptr + 4096, 4096);
> > >  
> > > +		if (count >= 2) {
> > > +			/* You can't bond to a balanced engine */
> > > +			memset(&bond, 0, sizeof(bond));
> > > +			bond.base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> > > +			bond.master = ci[0];
> > > +			bond.virtual_index = 0;
> > > +			bond.num_bonds = 1;
> > > +			bond.engines[0] = ci[1];
> > > +
> > > +			balancer.base.next_extension = to_user_pointer(&bond);
> > > +			balancer.engine_index = 0;
> > > +			balancer.num_siblings = count;
> > > +			memcpy(balancer.engines, ci, count * sizeof(*ci));
> > > +
> > > +			memset(&engines, 0, sizeof(engines));
> > > +			engines.engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
> > > +			engines.engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
> > > +			engines.extensions = to_user_pointer(&balancer);
> > > +
> > > +			p.size = (sizeof(struct i915_context_param_engines) +
> > > +				  sizeof(*engines.engines));
> > > +
> > > +			igt_assert_eq(__gem_context_set_param(i915, &p), -EINVAL);
> > > +		}
> > > +
> > >  		gem_context_destroy(i915, p.ctx_id);
> > >  		free(ci);
> > >  	}
> > > -- 
> > > 2.31.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BUILD: failure for Add tests for newly invalid cases

[Patchwork]

== Series Details ==

Series: Add tests for newly invalid cases
URL   : https://patchwork.freedesktop.org/series/89599/
State : failure

== Summary ==

Applying: tests/i915/gem_ctx_param: Add tests for recently removed params
Applying: tests/i915/gem_ctx_param: Add a couple invalid PARAM_VM cases
Patch failed at 0002 tests/i915/gem_ctx_param: Add a couple invalid PARAM_VM cases
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev