On 15/12/17 16:14, Chris Wilson wrote:
Quoting Antonio Argenziano (2017-12-15 19:01:11)
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.
Signed-off-by: Antonio Argenziano <antonio.argenziano@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
---
tests/gem_ctx_param.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 76 insertions(+), 1 deletion(-)
diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index c20ae1ee..9a222e60 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -25,6 +25,7 @@
*/
#include "igt.h"
+#include <limits.h>
IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
@@ -136,11 +137,85 @@ igt_main
gem_context_set_param(fd, &arg);
}
+#define MAX_PRIO 1023
+#define MIN_PRIO -MAX_PRIO
+#define DEF_PRIO 0
Take these from the uapi header.
+
+ arg.param = I915_CONTEXT_PARAM_PRIORITY;
+
+ igt_subtest("root-set-priority") {
+ arg.ctx_id = ctx;
+ arg.size = 0;
+
Bonus points for CAP_SYS_NICE checking.
arg.size validation checking. > arg.value > u32 (checking for overflows)
What is the expectation for overflows? It looks like we only cast value
to int.
gem_context_get_param() of a new context should return DEF_PRIO
set_param on older machines returns -ENODEV
And that's not even trying to fuzz bad values beyond the sanity checks
of I915_CONTEXT_PARAM_PRIORITY.
+ for (int i = MIN_PRIO; i <= MAX_PRIO; i += 1023) {
It's a couple of ioctls, do all 1024. Do them in a random order, trust
no one.
Do we have a lib for doing that (generate a random permutation) already?
+ arg.value = i;
+ gem_context_set_param(fd, &arg);
+
+ gem_context_get_param(fd, &arg);
+ igt_assert(arg.value == i); /* Verify prio was set */
igt_assert_eq(arg.value, i);
But doesn't verify the priority does anything. Just the RTT in the API,
which is a nice verification nevertheless.
+ }
+ }
+
+ igt_subtest("root-set-priority-invalid-value") {
+ int prio_levels[] = {INT_MIN, MIN_PRIO - 1, MAX_PRIO + 1, INT_MAX};
+ int old_value;
+ arg.ctx_id = ctx;
+
+ gem_context_get_param(fd, &arg);
+ old_value = arg.value;
+
+ for (int i = 0; i < (sizeof(prio_levels) / sizeof(int)); i++) {
ARRAY_SIZE(prio_levels);
+ arg.value = prio_levels[i];
+ igt_assert_eq(__gem_context_set_param(fd, &arg), -EINVAL);
+
+ gem_context_get_param(fd, &arg);
+ igt_assert(arg.value == old_value); /* Verify prio was not set */
+ }
+ }
+
+ igt_subtest("user-set-priority") {
+ arg.size = 0;
+
+ igt_fork(child, 1) {
+ igt_drop_root();
+ for (int i = MIN_PRIO; i <= DEF_PRIO; i += 1023) {
+ arg.value = i;
+ gem_context_set_param(fd, &arg);
+
+ gem_context_get_param(fd, &arg);
+ igt_assert(arg.value == i); /* Verify prio was set */
I wonder if the CAP_SYS_NICE limit might be adjusted in the future.
Certainly preemption rules are flexible, as we haven't told anyone about
them yet.
I guess when the change comes we will have a new constant we could swap
DEF_PRIO with.
-Antonio
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
