Quoting Antonio Argenziano (2017-12-18 18:15:35)
>
>
> On 15/12/17 16:14, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2017-12-15 19:01:11)
> >> + 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.
That was my point. We could create 1<<32 + prio, so we really should be
failing with -EINVAL rather than setting the parameter to prio. Trust
nothing, least of all the kernel.
> > 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?
See igt_permute_array. Maybe igt_random_array(count, prng_state).
> >> + 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.
The question is whether it is ABI, as it may be changed. If its not,
don't enshrine it into law.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
