[igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array

[igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array

[Antonio Argenziano]

Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
---
 lib/igt_aux.c | 19 +++++++++++++++++++
 lib/igt_aux.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 8ca0b60d..8012d8ae 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
 	int_arr[j] = tmp;
 }
 
+/**
+ * igt_exchange_int64:
+ * @array: pointer to the array of int64_t
+ * @i: first position
+ * @j: second position
+ *
+ * Exchanges the two values at array indices @i and @j. Useful as an exchange
+ * function for igt_permute_array().
+ */
+void igt_exchange_int64(void *array, unsigned i, unsigned j)
+{
+	int64_t *int_arr, tmp;
+	int_arr = array;
+
+	tmp = int_arr[i];
+	int_arr[i] = int_arr[j];
+	int_arr[j] = tmp;
+}
+
 /**
  * igt_permute_array:
  * @array: pointer to array
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 02e70126..60d4de71 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -117,6 +117,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
 	for (struct timespec t__={}; igt_nsec_elapsed(&t__)>>20 < (t); )
 
 void igt_exchange_int(void *array, unsigned i, unsigned j);
+void igt_exchange_int64(void *array, unsigned i, unsigned j);
 void igt_permute_array(void *array, unsigned size,
 			   void (*exchange_func)(void *array,
 						 unsigned i,
-- 
2.14.2

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

[igt-dev] [PATCH i-g-t v4 2/3] tests/gem_ctx_param: Update invalid param

[Antonio Argenziano]

Since commit: drm/i915/scheduler: Support user-defined priorities, the
driver support an extra context param to set context's priority. Add
tests for that interface and update invalid tests.

v2:
	- Add arg size validation test. (Chris)
	- Add arg value overflow test. (Chris)
	- Add test for unsupported platforms. (Chris)
	- Feed interface with all priority values and in random order. (Chris)

v3:
	- Parametrize tests. (Chris)

v4:
	- Code-style refactoring. (Chris)

Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
---
 tests/gem_ctx_param.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 188 insertions(+), 1 deletion(-)

diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index c20ae1ee..180202b9 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -25,9 +25,147 @@
  */
 
 #include "igt.h"
+#include <limits.h>
 
 IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
 
+#define MAX_USER_SET_PRIO I915_CONTEXT_DEFAULT_PRIORITY /* Current max prio for non-root users */
+#define PRIO_RANGE (I915_CONTEXT_MAX_USER_PRIORITY - I915_CONTEXT_MIN_USER_PRIORITY)
+#define USER_PRIO_RANGE (MAX_USER_SET_PRIO - I915_CONTEXT_MIN_USER_PRIORITY)
+
+#define ROOT (0x1 << 3)
+#define NEW_CTX	(0x1 << 2)
+#define VALID_PRIO (0x1 << 1)
+#define OVERFLOW_PRIO (0x1 << 0)
+
+#define IS_ROOT(flags) (flags & ROOT)
+
+static int is_priority_valid(int64_t value, unsigned flags)
+{
+	if (IS_ROOT(flags)) {
+		if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
+			(value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
+			return 0;
+
+		return -EINVAL;
+	} else {
+		if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= USER_PRIO_RANGE &&
+			(value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
+			return 0;
+
+		if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
+			(value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
+			return -EPERM;
+
+		return -EINVAL;
+	}
+}
+
+static void
+get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
+{
+	const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
+	int64_t *values;
+
+	*size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
+	values = (int64_t*) calloc(*size, sizeof(int64_t));
+	igt_assert(values);
+
+	for (int i = 0; i < *size; i++) {
+		values[i] = overflow ^ (i + I915_CONTEXT_MIN_USER_PRIORITY);
+	}
+
+	*prio_values = values;
+}
+
+static void
+get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
+{
+	const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
+	int64_t *values;
+
+	if (IS_ROOT(flags)) {
+		int64_t test_values[] = { /* Test space too big pick significant values */
+				INT_MIN,
+				I915_CONTEXT_MIN_USER_PRIORITY - 1,
+				I915_CONTEXT_MAX_USER_PRIORITY + 1,
+				INT_MAX
+		};
+
+		*size = ARRAY_SIZE(test_values);
+		values = (int64_t*) calloc(*size, sizeof(int64_t));
+
+		for (int i = 0; i < *size; i++)
+			values[i] = overflow ^ test_values[i];
+	} else {
+		*size = PRIO_RANGE - USER_PRIO_RANGE;
+		values = (int64_t*) calloc(*size, sizeof(int64_t));
+
+		for (int i = 0; i < *size; i++)
+			values[i] = overflow ^ (i + (MAX_USER_SET_PRIO + 1));
+	}
+
+	*prio_values = values;
+}
+
+static void
+get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
+{
+	if (flags & VALID_PRIO)
+		get_prio_values_valid(prio_values, size, flags);
+	else
+		get_prio_values_invalid(prio_values, size, flags);
+
+	igt_permute_array(*prio_values, *size, igt_exchange_int64);
+}
+
+static void
+set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
+{
+	uint32_t ctx = 0;
+	int expected_ret;
+	int64_t old_value;
+	int64_t *prio_values;
+	unsigned size;
+
+	igt_fork(child, 1) {
+		get_prio_values(&prio_values, &size, flags);
+
+		if (flags & NEW_CTX)
+			ctx = gem_context_create(fd);
+		arg.ctx_id = ctx;
+
+		if (!(flags & ROOT)) {
+			igt_debug("Dropping root privilege\n");
+			igt_drop_root();
+		}
+
+		gem_context_get_param(fd, &arg);
+		old_value = arg.value;
+
+		for (int i = 0; i < size; i++) {
+			arg.value = prio_values[i];
+			expected_ret = is_priority_valid(arg.value, flags);
+
+			igt_assert_eq(__gem_context_set_param(fd, &arg), expected_ret);
+
+			gem_context_get_param(fd, &arg);
+			if (expected_ret)
+				igt_assert_eq(arg.value, old_value); /* unchanged */
+			else
+				igt_assert_eq(arg.value, prio_values[i]); /* updated */
+		}
+
+		arg.value = 0;
+		gem_context_set_param(fd, &arg);
+
+		if (flags & NEW_CTX)
+			gem_context_destroy(fd, ctx);
+	}
+
+	igt_waitchildren();
+}
+
 igt_main
 {
 	struct drm_i915_gem_context_param arg;
@@ -136,11 +274,60 @@ igt_main
 		gem_context_set_param(fd, &arg);
 	}
 
+	arg.param = I915_CONTEXT_PARAM_PRIORITY;
+
+	igt_subtest("set-priority-not-supported") {
+		igt_require(!gem_scheduler_has_ctx_priority(fd));
+
+		arg.ctx_id = ctx;
+		arg.size = 0;
+
+		igt_assert_eq(__gem_context_set_param(fd, &arg), -ENODEV);
+	}
+
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(gem_scheduler_has_ctx_priority(fd));
+		}
+
+		igt_subtest("get-priority-new-ctx") {
+			struct drm_i915_gem_context_param local_arg = arg;
+			uint32_t local_ctx = gem_context_create(fd);
+
+			local_arg.ctx_id = local_ctx;
+
+			gem_context_get_param(fd, &local_arg);
+			igt_assert_eq(local_arg.value, 0);
+
+			gem_context_destroy(fd, local_ctx);
+		}
+
+		igt_subtest("set-priority-invalid-size") {
+			struct drm_i915_gem_context_param local_arg = arg;
+			local_arg.ctx_id = ctx;
+			local_arg.value = 0;
+			local_arg.size = ~0;
+
+			igt_assert_eq(__gem_context_set_param(fd, &local_arg), -EINVAL);
+		}
+
+		for (unsigned flags = 0;
+				flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
+				flags++) {
+			igt_subtest_f("set-priority%s%s%s%s",
+					(flags & ROOT) ? "-root" : "-user",
+					(flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
+					(flags & VALID_PRIO) ? "" : "-invalid",
+					(flags & OVERFLOW_PRIO) ? "-overflow" : "")
+				set_priority(fd, arg, flags);
+		}
+	}
+
 	/* NOTE: This testcase intentionally tests for the next free parameter
 	 * to catch ABI extensions. Don't "fix" this testcase without adding all
 	 * the tests for the new param first.
 	 */
-	arg.param = I915_CONTEXT_PARAM_BANNABLE + 1;
+	arg.param = I915_CONTEXT_PARAM_PRIORITY + 1;
 
 	igt_subtest("invalid-param-get") {
 		arg.ctx_id = ctx;
-- 
2.14.2

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

[igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Antonio Argenziano]

Adds tests for !SYS_NICE users trying to change the priority level of a
context using the provided IOCTL interface.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
---
 README                 |  1 +
 configure.ac           |  8 ++++++++
 meson.build            |  3 +++
 tests/Makefile.am      |  7 +++++++
 tests/Makefile.sources |  1 -
 tests/gem_ctx_param.c  | 42 ++++++++++++++++++++++++++++++++++++------
 6 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/README b/README
index 7cd78e44..89c10beb 100644
--- a/README
+++ b/README
@@ -147,6 +147,7 @@ the default configuration (package names may vary):
 	libkmod-dev
 	libpciaccess-dev
 	libprocps-dev
+	libcap-dev
 	libunwind-dev
 	python-docutils
 	x11proto-dri2-dev
diff --git a/configure.ac b/configure.ac
index e13a3b74..2d67e3f1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -81,6 +81,13 @@ AC_CHECK_FUNCS(timer_create, [], [
 ])
 AC_SUBST(TIMER_LIBS)
 
+AC_CHECK_HEADER(sys/capability.h, [have_libcap=yes])
+if test x$have_libcap = xyes; then
+	AC_DEFINE(HAVE_LIBCAP, 1, [Enable libcap support.])
+fi
+
+AM_CONDITIONAL(HAVE_LIBCAP, [test "x$have_libcap" = xyes])
+
 dnl Check for CPUID
 cpuid="yes"
 AC_TRY_LINK([
@@ -126,6 +133,7 @@ PKG_CHECK_MODULES(KMOD, [libkmod])
 PKG_CHECK_MODULES(PROCPS, [libprocps])
 PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
 PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
+#PKG_CHECK_MODULES(LIBCAP, [libcap-dev], [have_libcap=yes], [have_libcap=no])
 
 if test x$have_valgrind = xyes; then
 	AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
diff --git a/meson.build b/meson.build
index 9036feb1..937edf52 100644
--- a/meson.build
+++ b/meson.build
@@ -106,6 +106,9 @@ endif
 if cc.has_header('sys/io.h')
 	config.set('HAVE_SYS_IO_H', 1)
 endif
+if cc.has_header('sys/capability.h')
+	config.set('HAVE_LIBCAP', 1)
+endif
 if cc.has_header('cpuid.h')
 	# FIXME: Do we need the example link test from configure.ac?
 	config.set('HAVE_CPUID_H', 1)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1b9a7b0a..c482e970 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -14,6 +14,12 @@ if BUILD_VC4
     TESTS_progs += $(VC4_TESTS)
 endif
 
+if HAVE_LIBCAP
+TESTS_progs += \
+	gem_ctx_param \
+	$(NULL)
+endif
+
 if HAVE_CHAMELIUM
 TESTS_progs += \
 	kms_chamelium \
@@ -106,6 +112,7 @@ gem_close_race_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_close_race_LDADD = $(LDADD) -lpthread
 gem_ctx_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_ctx_thrash_LDADD = $(LDADD) -lpthread
+gem_ctx_param_LDADD = $(LDADD) -lcap
 gem_exec_parallel_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_exec_parallel_LDADD = $(LDADD) -lpthread
 gem_fence_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index e4e06d01..4fd3c7e8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -57,7 +57,6 @@ TESTS_progs = \
 	gem_ctx_bad_exec \
 	gem_ctx_create \
 	gem_ctx_exec \
-	gem_ctx_param \
 	gem_ctx_switch \
 	gem_ctx_thrash \
 	gem_double_irq_loop \
diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index 180202b9..c51b3290 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -26,6 +26,7 @@
 
 #include "igt.h"
 #include <limits.h>
+#include <sys/capability.h>
 
 IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
 
@@ -33,16 +34,17 @@ IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
 #define PRIO_RANGE (I915_CONTEXT_MAX_USER_PRIORITY - I915_CONTEXT_MIN_USER_PRIORITY)
 #define USER_PRIO_RANGE (MAX_USER_SET_PRIO - I915_CONTEXT_MIN_USER_PRIORITY)
 
+#define NICE (0x1 << 4)
 #define ROOT (0x1 << 3)
 #define NEW_CTX	(0x1 << 2)
 #define VALID_PRIO (0x1 << 1)
 #define OVERFLOW_PRIO (0x1 << 0)
 
-#define IS_ROOT(flags) (flags & ROOT)
+#define IS_ROOT_AND_NICE(flags) ((flags & ROOT) && (flags & NICE))
 
 static int is_priority_valid(int64_t value, unsigned flags)
 {
-	if (IS_ROOT(flags)) {
+	if (IS_ROOT_AND_NICE(flags)) {
 		if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
 			(value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
 			return 0;
@@ -67,7 +69,7 @@ get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
 	const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
 	int64_t *values;
 
-	*size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
+	*size = (IS_ROOT_AND_NICE(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
 	values = (int64_t*) calloc(*size, sizeof(int64_t));
 	igt_assert(values);
 
@@ -84,7 +86,7 @@ get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
 	const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
 	int64_t *values;
 
-	if (IS_ROOT(flags)) {
+	if (IS_ROOT_AND_NICE(flags)) {
 		int64_t test_values[] = { /* Test space too big pick significant values */
 				INT_MIN,
 				I915_CONTEXT_MIN_USER_PRIORITY - 1,
@@ -119,6 +121,25 @@ get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
 	igt_permute_array(*prio_values, *size, igt_exchange_int64);
 }
 
+static void lower_sys_nice(void)
+{
+	cap_t caps;
+	cap_value_t cap_list = CAP_SYS_NICE;
+	pid_t pid = getpid();
+	cap_flag_value_t cap_val;
+
+	caps = cap_get_pid(pid);
+	igt_require(caps);
+
+	cap_get_flag(caps, cap_list, CAP_EFFECTIVE, &cap_val);
+	if (cap_val == CAP_CLEAR)
+		return; /* CAP_SYS_NICE already unset */
+
+	igt_assert(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_list, CAP_CLEAR) == 0);
+	igt_assert(cap_set_proc(caps) == 0);
+	cap_free(caps);
+}
+
 static void
 set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
 {
@@ -140,6 +161,11 @@ set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
 			igt_drop_root();
 		}
 
+		if (!(flags & NICE)) {
+			igt_debug("Dropping SYS_NICE capability\n");
+			lower_sys_nice();
+		}
+
 		gem_context_get_param(fd, &arg);
 		old_value = arg.value;
 
@@ -312,9 +338,13 @@ igt_main
 		}
 
 		for (unsigned flags = 0;
-				flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
+				flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
 				flags++) {
-			igt_subtest_f("set-priority%s%s%s%s",
+			if (!(flags & NICE) && !(flags & ROOT))
+				continue; /* Needs to be rot to set properties */
+
+			igt_subtest_f("set-priority%s%s%s%s%s",
+					(flags & NICE) ? "" : "-not-nice",
 					(flags & ROOT) ? "-root" : "-user",
 					(flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
 					(flags & VALID_PRIO) ? "" : "-invalid",
-- 
2.14.2

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array
URL   : https://patchwork.freedesktop.org/series/37082/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
ec7d71660694157af698741eac3764203bf338ec igt/kms_frontbuffer_tracking: Show FBC status during the wait

with latest DRM-Tip kernel build CI_DRM_3683
9d8467fe5626 drm-tip: 2018y-01m-24d-19h-59m-41s UTC integration manifest

Testlist changes:
-igt@gem_ctx_param@basic
-igt@gem_ctx_param@basic-default
-igt@gem_ctx_param@invalid-ctx-get
-igt@gem_ctx_param@invalid-ctx-set
-igt@gem_ctx_param@invalid-param-get
-igt@gem_ctx_param@invalid-param-set
-igt@gem_ctx_param@invalid-size-get
-igt@gem_ctx_param@invalid-size-set
-igt@gem_ctx_param@non-root-set
-igt@gem_ctx_param@non-root-set-no-zeromap
-igt@gem_ctx_param@root-set
-igt@gem_ctx_param@root-set-no-zeromap-disabled
-igt@gem_ctx_param@root-set-no-zeromap-enabled

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:21  time:421s
fi-bdw-gvtdvm    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:24  time:432s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:62  time:373s
fi-bsw-n3050     total:288  pass:240  dwarn:0   dfail:0   fail:0   skip:46  time:492s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:103 time:285s
fi-bxt-dsi       total:288  pass:256  dwarn:0   dfail:0   fail:0   skip:30  time:486s
fi-bxt-j4205     total:288  pass:257  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-byt-j1900     total:288  pass:251  dwarn:0   dfail:0   fail:0   skip:35  time:472s
fi-byt-n2820     total:288  pass:247  dwarn:0   dfail:0   fail:0   skip:39  time:458s
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:43 
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:106 time:279s
fi-glk-1         total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:28  time:515s
fi-hsw-4770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:27  time:393s
fi-hsw-4770r     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:27  time:403s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:58  time:413s
fi-ivb-3520m     total:288  pass:257  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-ivb-3770      total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:33  time:413s
fi-kbl-7500u     total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:24  time:461s
fi-kbl-7560u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:19  time:497s
fi-kbl-7567u     total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-kbl-r         total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:27  time:500s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:63  time:578s
fi-skl-6260u     total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:20  time:427s
fi-skl-6600u     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:27  time:511s
fi-skl-6700hq    total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:26  time:528s
fi-skl-6700k2    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:24  time:492s
fi-skl-6770hq    total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:20  time:486s
fi-skl-guc       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:28  time:418s
fi-skl-gvtdvm    total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:23  time:434s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:246  dwarn:0   dfail:0   fail:0   skip:40  time:403s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:26  time:568s
fi-glk-dsi       total:288  pass:256  dwarn:0   dfail:0   fail:0   skip:30  time:477s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_822/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array
URL   : https://patchwork.freedesktop.org/series/37082/
State : failure

== Summary ==

Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                skip       -> PASS       (shard-snb)
Test kms_flip:
        Subgroup 2x-plain-flip-fb-recreate:
                pass       -> FAIL       (shard-hsw) fdo#100368 +2
        Subgroup 2x-flip-vs-modeset-interruptible:
                dmesg-warn -> PASS       (shard-hsw)
        Subgroup flip-vs-expired-vblank-interruptible:
                pass       -> FAIL       (shard-apl) fdo#102887
        Subgroup 2x-wf_vblank-ts-check-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103928
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047
Test gem_pwrite:
        Subgroup big-gtt-forwards:
                pass       -> SKIP       (shard-apl)
Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-crc-atomic:
                fail       -> PASS       (shard-apl)
Test gem_eio:
        Subgroup in-flight-contexts:
                fail       -> PASS       (shard-hsw) fdo#104676
Test perf_pmu:
        Subgroup busy-double-start-vcs0:
                pass       -> INCOMPLETE (shard-apl)

fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#104676 https://bugs.freedesktop.org/show_bug.cgi?id=104676

shard-apl        total:2807 pass:1725 dwarn:1   dfail:0   fail:21  skip:1058 time:12333s
shard-hsw        total:2825 pass:1723 dwarn:1   dfail:0   fail:10  skip:1090 time:12022s
shard-snb        total:2825 pass:1319 dwarn:1   dfail:0   fail:9   skip:1496 time:6554s
Blacklisted hosts:
shard-kbl        total:2825 pass:1826 dwarn:36  dfail:0   fail:23  skip:940 time:9533s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_822/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array

[Ville Syrjälä]

On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> ---
>  lib/igt_aux.c | 19 +++++++++++++++++++
>  lib/igt_aux.h |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 8ca0b60d..8012d8ae 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
>  	int_arr[j] = tmp;
>  }
>  
> +/**
> + * igt_exchange_int64:
> + * @array: pointer to the array of int64_t
> + * @i: first position
> + * @j: second position
> + *
> + * Exchanges the two values at array indices @i and @j. Useful as an exchange
> + * function for igt_permute_array().
> + */
> +void igt_exchange_int64(void *array, unsigned i, unsigned j)
> +{
> +	int64_t *int_arr, tmp;

Why void* when you really mean int64_t*?

> +	int_arr = array;
> +
> +	tmp = int_arr[i];
> +	int_arr[i] = int_arr[j];
> +	int_arr[j] = tmp;

igt_swap()

> +}
> +
>  /**
>   * igt_permute_array:
>   * @array: pointer to array
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 02e70126..60d4de71 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -117,6 +117,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
>  	for (struct timespec t__={}; igt_nsec_elapsed(&t__)>>20 < (t); )
>  
>  void igt_exchange_int(void *array, unsigned i, unsigned j);
> +void igt_exchange_int64(void *array, unsigned i, unsigned j);
>  void igt_permute_array(void *array, unsigned size,
>  			   void (*exchange_func)(void *array,
>  						 unsigned i,
> -- 
> 2.14.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array

[Chris Wilson]

Quoting Ville Syrjälä (2018-01-25 11:21:50)
> On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
> > Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > ---
> >  lib/igt_aux.c | 19 +++++++++++++++++++
> >  lib/igt_aux.h |  1 +
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index 8ca0b60d..8012d8ae 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
> >       int_arr[j] = tmp;
> >  }
> >  
> > +/**
> > + * igt_exchange_int64:
> > + * @array: pointer to the array of int64_t
> > + * @i: first position
> > + * @j: second position
> > + *
> > + * Exchanges the two values at array indices @i and @j. Useful as an exchange
> > + * function for igt_permute_array().
> > + */
> > +void igt_exchange_int64(void *array, unsigned i, unsigned j)
> > +{
> > +     int64_t *int_arr, tmp;
> 
> Why void* when you really mean int64_t*?

To fit the function prototype for igt_permute_array().
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array

[Jani Nikula]

On Thu, 25 Jan 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>> ---
>>  lib/igt_aux.c | 19 +++++++++++++++++++
>>  lib/igt_aux.h |  1 +
>>  2 files changed, 20 insertions(+)
>> 
>> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
>> index 8ca0b60d..8012d8ae 100644
>> --- a/lib/igt_aux.c
>> +++ b/lib/igt_aux.c
>> @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
>>  	int_arr[j] = tmp;
>>  }
>>  
>> +/**
>> + * igt_exchange_int64:
>> + * @array: pointer to the array of int64_t
>> + * @i: first position
>> + * @j: second position
>> + *
>> + * Exchanges the two values at array indices @i and @j. Useful as an exchange
>> + * function for igt_permute_array().
>> + */
>> +void igt_exchange_int64(void *array, unsigned i, unsigned j)
>> +{
>> +	int64_t *int_arr, tmp;
>
> Why void* when you really mean int64_t*?
>
>> +	int_arr = array;
>> +
>> +	tmp = int_arr[i];
>> +	int_arr[i] = int_arr[j];
>> +	int_arr[j] = tmp;
>
> igt_swap()


All of the igt_exchange_whatever functions could be replaced with a
single macro that does typeof on the array parameter.

BR,
Jani.

>
>> +}
>> +
>>  /**
>>   * igt_permute_array:
>>   * @array: pointer to array
>> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
>> index 02e70126..60d4de71 100644
>> --- a/lib/igt_aux.h
>> +++ b/lib/igt_aux.h
>> @@ -117,6 +117,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
>>  	for (struct timespec t__={}; igt_nsec_elapsed(&t__)>>20 < (t); )
>>  
>>  void igt_exchange_int(void *array, unsigned i, unsigned j);
>> +void igt_exchange_int64(void *array, unsigned i, unsigned j);
>>  void igt_permute_array(void *array, unsigned size,
>>  			   void (*exchange_func)(void *array,
>>  						 unsigned i,
>> -- 
>> 2.14.2
>> 
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Chris Wilson]

Quoting Antonio Argenziano (2018-01-25 01:00:03)
> Adds tests for !SYS_NICE users trying to change the priority level of a
> context using the provided IOCTL interface.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> ---
>  README                 |  1 +
>  configure.ac           |  8 ++++++++
>  meson.build            |  3 +++
>  tests/Makefile.am      |  7 +++++++
>  tests/Makefile.sources |  1 -
>  tests/gem_ctx_param.c  | 42 ++++++++++++++++++++++++++++++++++++------
>  6 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/README b/README
> index 7cd78e44..89c10beb 100644
> --- a/README
> +++ b/README
> @@ -147,6 +147,7 @@ the default configuration (package names may vary):
>         libkmod-dev
>         libpciaccess-dev
>         libprocps-dev
> +       libcap-dev
>         libunwind-dev
>         python-docutils
>         x11proto-dri2-dev
> diff --git a/configure.ac b/configure.ac
> index e13a3b74..2d67e3f1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -81,6 +81,13 @@ AC_CHECK_FUNCS(timer_create, [], [
>  ])
>  AC_SUBST(TIMER_LIBS)
>  
> +AC_CHECK_HEADER(sys/capability.h, [have_libcap=yes])
> +if test x$have_libcap = xyes; then
> +       AC_DEFINE(HAVE_LIBCAP, 1, [Enable libcap support.])
> +fi
> +
> +AM_CONDITIONAL(HAVE_LIBCAP, [test "x$have_libcap" = xyes])
> +
>  dnl Check for CPUID
>  cpuid="yes"
>  AC_TRY_LINK([
> @@ -126,6 +133,7 @@ PKG_CHECK_MODULES(KMOD, [libkmod])
>  PKG_CHECK_MODULES(PROCPS, [libprocps])
>  PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
>  PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
> +#PKG_CHECK_MODULES(LIBCAP, [libcap-dev], [have_libcap=yes], [have_libcap=no])
>  
>  if test x$have_valgrind = xyes; then
>         AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
> diff --git a/meson.build b/meson.build
> index 9036feb1..937edf52 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -106,6 +106,9 @@ endif
>  if cc.has_header('sys/io.h')
>         config.set('HAVE_SYS_IO_H', 1)
>  endif
> +if cc.has_header('sys/capability.h')
> +       config.set('HAVE_LIBCAP', 1)
> +endif
>  if cc.has_header('cpuid.h')
>         # FIXME: Do we need the example link test from configure.ac?
>         config.set('HAVE_CPUID_H', 1)
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 1b9a7b0a..c482e970 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -14,6 +14,12 @@ if BUILD_VC4
>      TESTS_progs += $(VC4_TESTS)
>  endif
>  
> +if HAVE_LIBCAP
> +TESTS_progs += \
> +       gem_ctx_param \
> +       $(NULL)
> +endif

CI doesn't have libcap-dev so this nerfed the tests.
Only one subtest requires LIBCAP...

> +#define NICE (0x1 << 4)
>  #define ROOT (0x1 << 3)
>  #define NEW_CTX        (0x1 << 2)
>  #define VALID_PRIO (0x1 << 1)
>  #define OVERFLOW_PRIO (0x1 << 0)

Hmm, this is opposite to the usual ordering :)
> 
> -#define IS_ROOT(flags) (flags & ROOT)
> +#define IS_ROOT_AND_NICE(flags) ((flags & ROOT) && (flags & NICE))

Nah, you meant IS_ROOT_OR_NICE in most places.

>  static int is_priority_valid(int64_t value, unsigned flags)
>  {
> -       if (IS_ROOT(flags)) {
> +       if (IS_ROOT_AND_NICE(flags)) {
>                 if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
>                         (value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
>                         return 0;
> @@ -67,7 +69,7 @@ get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
>         const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
>         int64_t *values;
>  
> -       *size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
> +       *size = (IS_ROOT_AND_NICE(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
>         values = (int64_t*) calloc(*size, sizeof(int64_t));
>         igt_assert(values);
>  
> @@ -84,7 +86,7 @@ get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
>         const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
>         int64_t *values;
>  
> -       if (IS_ROOT(flags)) {
> +       if (IS_ROOT_AND_NICE(flags)) {
>                 int64_t test_values[] = { /* Test space too big pick significant values */
>                                 INT_MIN,
>                                 I915_CONTEXT_MIN_USER_PRIORITY - 1,
> @@ -119,6 +121,25 @@ get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
>         igt_permute_array(*prio_values, *size, igt_exchange_int64);
>  }
>  
> +static void lower_sys_nice(void)
> +{

#if HAVE_LIBCAP

> +       cap_t caps;
> +       cap_value_t cap_list = CAP_SYS_NICE;
> +       pid_t pid = getpid();
> +       cap_flag_value_t cap_val;
> +
> +       caps = cap_get_pid(pid);
> +       igt_require(caps);
> +
> +       cap_get_flag(caps, cap_list, CAP_EFFECTIVE, &cap_val);
> +       if (cap_val == CAP_CLEAR)
> +               return; /* CAP_SYS_NICE already unset */
> +
> +       igt_assert(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_list, CAP_CLEAR) == 0);
> +       igt_assert(cap_set_proc(caps) == 0);
> +       cap_free(caps);

#else
	return false;
#endif

> +}
> +
>  static void
>  set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
>  {
> @@ -140,6 +161,11 @@ set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
>                         igt_drop_root();
>                 }
>  
> +               if (!(flags & NICE)) {
> +                       igt_debug("Dropping SYS_NICE capability\n");
> +                       lower_sys_nice();

Rewrite as igt_require(drop_sys_nice());

> +               }
> +
>                 gem_context_get_param(fd, &arg);
>                 old_value = arg.value;
>  
> @@ -312,9 +338,13 @@ igt_main
>                 }
>  
>                 for (unsigned flags = 0;
> -                               flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> +                               flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>                                 flags++) {
> -                       igt_subtest_f("set-priority%s%s%s%s",
> +                       if (!(flags & NICE) && !(flags & ROOT))
> +                               continue; /* Needs to be rot to set properties */

Nope, just use igt_require as above.

> +
> +                       igt_subtest_f("set-priority%s%s%s%s%s",
> +                                       (flags & NICE) ? "" : "-not-nice",
>                                         (flags & ROOT) ? "-root" : "-user",
>                                         (flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
>                                         (flags & VALID_PRIO) ? "" : "-invalid",
> -- 
> 2.14.2
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array

[Antonio Argenziano]

On 25/01/18 03:35, Jani Nikula wrote:
> On Thu, 25 Jan 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
>>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>>> ---
>>>   lib/igt_aux.c | 19 +++++++++++++++++++
>>>   lib/igt_aux.h |  1 +
>>>   2 files changed, 20 insertions(+)
>>>
>>> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
>>> index 8ca0b60d..8012d8ae 100644
>>> --- a/lib/igt_aux.c
>>> +++ b/lib/igt_aux.c
>>> @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
>>>   	int_arr[j] = tmp;
>>>   }
>>>   
>>> +/**
>>> + * igt_exchange_int64:
>>> + * @array: pointer to the array of int64_t
>>> + * @i: first position
>>> + * @j: second position
>>> + *
>>> + * Exchanges the two values at array indices @i and @j. Useful as an exchange
>>> + * function for igt_permute_array().
>>> + */
>>> +void igt_exchange_int64(void *array, unsigned i, unsigned j)
>>> +{
>>> +	int64_t *int_arr, tmp;
>>
>> Why void* when you really mean int64_t*?
>>
>>> +	int_arr = array;
>>> +
>>> +	tmp = int_arr[i];
>>> +	int_arr[i] = int_arr[j];
>>> +	int_arr[j] = tmp;
>>
>> igt_swap()
> 
> 
> All of the igt_exchange_whatever functions could be replaced with a
> single macro that does typeof on the array parameter.

Basically that would be igt_swap(). You would have to replace the 
permute interface to make it not take a void pointer but a generic array.

I tried the following:

#define igt_permute_array(array, size) do { \
	int _i; \
	typeof(array) _array; \
	_array = array; \
  \
	for (_i = size - 1; _i > 0; _i--) { \
		/* yes, not perfectly uniform, who cares */ \
		long _l = hars_petruska_f54_1_random_unsafe() % (_i +1); \
		if (_i != _l) \
			igt_swap(_array[_i], _array[_l]); \
	} \
} while (0)

The problem I had is that typeof() doesn't seems to play nice with 
arrays but wants a pointer type.
So it would be a pervasive change and I would prefer to make it into a 
separate patchset.

Thanks,
Antonio

> 
> BR,
> Jani.
> 
>>
>>> +}
>>> +
>>>   /**
>>>    * igt_permute_array:
>>>    * @array: pointer to array
>>> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
>>> index 02e70126..60d4de71 100644
>>> --- a/lib/igt_aux.h
>>> +++ b/lib/igt_aux.h
>>> @@ -117,6 +117,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
>>>   	for (struct timespec t__={}; igt_nsec_elapsed(&t__)>>20 < (t); )
>>>   
>>>   void igt_exchange_int(void *array, unsigned i, unsigned j);
>>> +void igt_exchange_int64(void *array, unsigned i, unsigned j);
>>>   void igt_permute_array(void *array, unsigned size,
>>>   			   void (*exchange_func)(void *array,
>>>   						 unsigned i,
>>> -- 
>>> 2.14.2
>>>
>>> _______________________________________________
>>> igt-dev mailing list
>>> igt-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Antonio Argenziano]

On 25/01/18 03:37, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-01-25 01:00:03)
>> Adds tests for !SYS_NICE users trying to change the priority level of a
>> context using the provided IOCTL interface.
>>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>> ---
>>   README                 |  1 +
>>   configure.ac           |  8 ++++++++
>>   meson.build            |  3 +++
>>   tests/Makefile.am      |  7 +++++++
>>   tests/Makefile.sources |  1 -
>>   tests/gem_ctx_param.c  | 42 ++++++++++++++++++++++++++++++++++++------
>>   6 files changed, 55 insertions(+), 7 deletions(-)
>>
>> diff --git a/README b/README
>> index 7cd78e44..89c10beb 100644
>> --- a/README
>> +++ b/README
>> @@ -147,6 +147,7 @@ the default configuration (package names may vary):
>>          libkmod-dev
>>          libpciaccess-dev
>>          libprocps-dev
>> +       libcap-dev
>>          libunwind-dev
>>          python-docutils
>>          x11proto-dri2-dev
>> diff --git a/configure.ac b/configure.ac
>> index e13a3b74..2d67e3f1 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -81,6 +81,13 @@ AC_CHECK_FUNCS(timer_create, [], [
>>   ])
>>   AC_SUBST(TIMER_LIBS)
>>   
>> +AC_CHECK_HEADER(sys/capability.h, [have_libcap=yes])
>> +if test x$have_libcap = xyes; then
>> +       AC_DEFINE(HAVE_LIBCAP, 1, [Enable libcap support.])
>> +fi
>> +
>> +AM_CONDITIONAL(HAVE_LIBCAP, [test "x$have_libcap" = xyes])
>> +
>>   dnl Check for CPUID
>>   cpuid="yes"
>>   AC_TRY_LINK([
>> @@ -126,6 +133,7 @@ PKG_CHECK_MODULES(KMOD, [libkmod])
>>   PKG_CHECK_MODULES(PROCPS, [libprocps])
>>   PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
>>   PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
>> +#PKG_CHECK_MODULES(LIBCAP, [libcap-dev], [have_libcap=yes], [have_libcap=no])
>>   
>>   if test x$have_valgrind = xyes; then
>>          AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
>> diff --git a/meson.build b/meson.build
>> index 9036feb1..937edf52 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -106,6 +106,9 @@ endif
>>   if cc.has_header('sys/io.h')
>>          config.set('HAVE_SYS_IO_H', 1)
>>   endif
>> +if cc.has_header('sys/capability.h')
>> +       config.set('HAVE_LIBCAP', 1)
>> +endif
>>   if cc.has_header('cpuid.h')
>>          # FIXME: Do we need the example link test from configure.ac?
>>          config.set('HAVE_CPUID_H', 1)
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index 1b9a7b0a..c482e970 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -14,6 +14,12 @@ if BUILD_VC4
>>       TESTS_progs += $(VC4_TESTS)
>>   endif
>>   
>> +if HAVE_LIBCAP
>> +TESTS_progs += \
>> +       gem_ctx_param \
>> +       $(NULL)
>> +endif
> 
> CI doesn't have libcap-dev so this nerfed the tests.
> Only one subtest requires LIBCAP...
> 
>> +#define NICE (0x1 << 4)
>>   #define ROOT (0x1 << 3)
>>   #define NEW_CTX        (0x1 << 2)
>>   #define VALID_PRIO (0x1 << 1)
>>   #define OVERFLOW_PRIO (0x1 << 0)
> 
> Hmm, this is opposite to the usual ordering :)
>>
>> -#define IS_ROOT(flags) (flags & ROOT)
>> +#define IS_ROOT_AND_NICE(flags) ((flags & ROOT) && (flags & NICE))
> 
> Nah, you meant IS_ROOT_OR_NICE in most places.

Or even just IS_NICE if I filter flags with NICE and !ROOT when creating 
sub-tests.

> 
>>   static int is_priority_valid(int64_t value, unsigned flags)
>>   {
>> -       if (IS_ROOT(flags)) {
>> +       if (IS_ROOT_AND_NICE(flags)) {
>>                  if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
>>                          (value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
>>                          return 0;
>> @@ -67,7 +69,7 @@ get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
>>          const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
>>          int64_t *values;
>>   
>> -       *size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
>> +       *size = (IS_ROOT_AND_NICE(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
>>          values = (int64_t*) calloc(*size, sizeof(int64_t));
>>          igt_assert(values);
>>   
>> @@ -84,7 +86,7 @@ get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
>>          const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
>>          int64_t *values;
>>   
>> -       if (IS_ROOT(flags)) {
>> +       if (IS_ROOT_AND_NICE(flags)) {
>>                  int64_t test_values[] = { /* Test space too big pick significant values */
>>                                  INT_MIN,
>>                                  I915_CONTEXT_MIN_USER_PRIORITY - 1,
>> @@ -119,6 +121,25 @@ get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
>>          igt_permute_array(*prio_values, *size, igt_exchange_int64);
>>   }
>>   
>> +static void lower_sys_nice(void)
>> +{
> 
> #if HAVE_LIBCAP
> 
>> +       cap_t caps;
>> +       cap_value_t cap_list = CAP_SYS_NICE;
>> +       pid_t pid = getpid();
>> +       cap_flag_value_t cap_val;
>> +
>> +       caps = cap_get_pid(pid);
>> +       igt_require(caps);
>> +
>> +       cap_get_flag(caps, cap_list, CAP_EFFECTIVE, &cap_val);
>> +       if (cap_val == CAP_CLEAR)
>> +               return; /* CAP_SYS_NICE already unset */
>> +
>> +       igt_assert(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_list, CAP_CLEAR) == 0);
>> +       igt_assert(cap_set_proc(caps) == 0);
>> +       cap_free(caps);
> 
> #else
> 	return false;
> #endif
> 
>> +}
>> +
>>   static void
>>   set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
>>   {
>> @@ -140,6 +161,11 @@ set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
>>                          igt_drop_root();
>>                  }
>>   
>> +               if (!(flags & NICE)) {
>> +                       igt_debug("Dropping SYS_NICE capability\n");
>> +                       lower_sys_nice();
> 
> Rewrite as igt_require(drop_sys_nice());
> 
>> +               }
>> +
>>                  gem_context_get_param(fd, &arg);
>>                  old_value = arg.value;
>>   
>> @@ -312,9 +338,13 @@ igt_main
>>                  }
>>   
>>                  for (unsigned flags = 0;
>> -                               flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>> +                               flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>>                                  flags++) {
>> -                       igt_subtest_f("set-priority%s%s%s%s",
>> +                       if (!(flags & NICE) && !(flags & ROOT))
>> +                               continue; /* Needs to be rot to set properties */
> 
> Nope, just use igt_require as above.

I didn't want any "*-not-nice-user*" subtests to appear in the list at 
all, with igt_require they would just skip but still be there right?

Thanks,
Antonio

> 
>> +
>> +                       igt_subtest_f("set-priority%s%s%s%s%s",
>> +                                       (flags & NICE) ? "" : "-not-nice",
>>                                          (flags & ROOT) ? "-root" : "-user",
>>                                          (flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
>>                                          (flags & VALID_PRIO) ? "" : "-invalid",
>> -- 
>> 2.14.2
>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Daniel Vetter]

On Tue, Jan 30, 2018 at 04:47:57PM -0800, Antonio Argenziano wrote:
> On 25/01/18 03:37, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2018-01-25 01:00:03)
> > > +               }
> > > +
> > >                  gem_context_get_param(fd, &arg);
> > >                  old_value = arg.value;
> > > @@ -312,9 +338,13 @@ igt_main
> > >                  }
> > >                  for (unsigned flags = 0;
> > > -                               flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> > > +                               flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> > >                                  flags++) {
> > > -                       igt_subtest_f("set-priority%s%s%s%s",
> > > +                       if (!(flags & NICE) && !(flags & ROOT))
> > > +                               continue; /* Needs to be rot to set properties */
> > 
> > Nope, just use igt_require as above.
> 
> I didn't want any "*-not-nice-user*" subtests to appear in the list at all,
> with igt_require they would just skip but still be there right?

subtest lists shouldn't be dynamic, it confuses CI infrastructure badly.
Hence why igt_require/igt_skip result in a special subtest result and will
not supress the subtest outright.

I know that vpg is struggling with heaps of skipped tests in their
reporting, but as soon as you do cross-platform/kernel/OS testing, it's
the only reasonable way to handle things.

So yeah, this needs to be an igt_require, and probably moved into the
subtest stanza (or wrapped with an igt_subtest_group depending upon the
overall test logic).


Aside: The igt intro docs should explain this, patch would be great if
it's unclear/missing.
-Daniel
-- 
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 v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Antonio Argenziano]

On 31/01/18 00:01, Daniel Vetter wrote:
> On Tue, Jan 30, 2018 at 04:47:57PM -0800, Antonio Argenziano wrote:
>> On 25/01/18 03:37, Chris Wilson wrote:
>>> Quoting Antonio Argenziano (2018-01-25 01:00:03)
>>>> +               }
>>>> +
>>>>                   gem_context_get_param(fd, &arg);
>>>>                   old_value = arg.value;
>>>> @@ -312,9 +338,13 @@ igt_main
>>>>                   }
>>>>                   for (unsigned flags = 0;
>>>> -                               flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>>>> +                               flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>>>>                                   flags++) {
>>>> -                       igt_subtest_f("set-priority%s%s%s%s",
>>>> +                       if (!(flags & NICE) && !(flags & ROOT))
>>>> +                               continue; /* Needs to be rot to set properties */
>>>
>>> Nope, just use igt_require as above.
>>
>> I didn't want any "*-not-nice-user*" subtests to appear in the list at all,
>> with igt_require they would just skip but still be there right?
> 
> subtest lists shouldn't be dynamic, it confuses CI infrastructure badly.
> Hence why igt_require/igt_skip result in a special subtest result and will
> not supress the subtest outright.

In this case the combination I am trying to suppress is actually invalid 
for all platforms/kernels that is why I wanted to avoid skipping as it 
would hint that the test can actually run if some conditions are met 
which is not the case here. That is why I was confused about the first 
suggestion (which I might be completely misinterpreting). I'll send the 
new revision so we can continue the discussion over the new code.

> 
> I know that vpg is struggling with heaps of skipped tests in their

(whispers) I actually agree with running everything and letting the test 
skip when not applicable ;).

Thanks,
Antonio

> reporting, but as soon as you do cross-platform/kernel/OS testing, it's
> the only reasonable way to handle things.
> 
> So yeah, this needs to be an igt_require, and probably moved into the
> subtest stanza (or wrapped with an igt_subtest_group depending upon the
> overall test logic).
> 
> 
> Aside: The igt intro docs should explain this, patch would be great if
> it's unclear/missing.
> -Daniel
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Chris Wilson]

Quoting Antonio Argenziano (2018-02-02 22:24:44)
> 
> 
> On 31/01/18 00:01, Daniel Vetter wrote:
> > On Tue, Jan 30, 2018 at 04:47:57PM -0800, Antonio Argenziano wrote:
> >> On 25/01/18 03:37, Chris Wilson wrote:
> >>> Quoting Antonio Argenziano (2018-01-25 01:00:03)
> >>>> +               }
> >>>> +
> >>>>                   gem_context_get_param(fd, &arg);
> >>>>                   old_value = arg.value;
> >>>> @@ -312,9 +338,13 @@ igt_main
> >>>>                   }
> >>>>                   for (unsigned flags = 0;
> >>>> -                               flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> >>>> +                               flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> >>>>                                   flags++) {
> >>>> -                       igt_subtest_f("set-priority%s%s%s%s",
> >>>> +                       if (!(flags & NICE) && !(flags & ROOT))
> >>>> +                               continue; /* Needs to be rot to set properties */
> >>>
> >>> Nope, just use igt_require as above.
> >>
> >> I didn't want any "*-not-nice-user*" subtests to appear in the list at all,
> >> with igt_require they would just skip but still be there right?
> > 
> > subtest lists shouldn't be dynamic, it confuses CI infrastructure badly.
> > Hence why igt_require/igt_skip result in a special subtest result and will
> > not supress the subtest outright.
> 
> In this case the combination I am trying to suppress is actually invalid 
> for all platforms/kernels that is why I wanted to avoid skipping as it 
> would hint that the test can actually run if some conditions are met 
> which is not the case here. That is why I was confused about the first 
> suggestion (which I might be completely misinterpreting). I'll send the 
> new revision so we can continue the discussion over the new code.

Then I'm missing something as those tests should still work for the
user, just the valid range is different. (Still vital to probe overflows
against the restricted range etc.) I think if you call them USER, NICE
it's clearer (as the first drops-all-caps, the second drops-all-caps-but-nice).
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Arkadiusz Hiler]

On Wed, Jan 24, 2018 at 05:00:03PM -0800, Antonio Argenziano wrote:
> Adds tests for !SYS_NICE users trying to change the priority level of a
> context using the provided IOCTL interface.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>

Hey, what's up with this patch? Rest of the series was already merged
through inclusion in other series (AFAIUI). Are you going to do a
followup or can I mark it as not applicable?
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users

[Chris Wilson]

Quoting Arkadiusz Hiler (2018-02-28 14:10:14)
> On Wed, Jan 24, 2018 at 05:00:03PM -0800, Antonio Argenziano wrote:
> > Adds tests for !SYS_NICE users trying to change the priority level of a
> > context using the provided IOCTL interface.
> > 
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> 
> Hey, what's up with this patch? Rest of the series was already merged
> through inclusion in other series (AFAIUI). Are you going to do a
> followup or can I mark it as not applicable?

It's a good test that just needed a bit more work, to sort out the
libcap dependency problem and cleaning up the (root, nice, user) test
modes.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev