* Re: [PATCH] drm/i915: Hang counting is now always per-fd, so relax the ioctl for DEFAULT_CONTEXT
2014-05-01 7:18 [PATCH] drm/i915: Hang counting is now always per-fd, so relax the ioctl for DEFAULT_CONTEXT Chris Wilson
@ 2014-05-01 18:02 ` Ben Widawsky
2014-05-02 11:15 ` Mika Kuoppala
2014-05-02 11:16 ` [PATCH 1/1] tests/gem_reset_stats: EPERM check not needed Mika Kuoppala
2 siblings, 0 replies; 4+ messages in thread
From: Ben Widawsky @ 2014-05-01 18:02 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, Mika Kuoppala
On Thu, May 01, 2014 at 08:18:44AM +0100, Chris Wilson wrote:
> Since we only count hangs towards the owner of the fd issuing the
> command, we can allow that fd to inspect its own default context without
> leaking global information. We introduced per-fd accounting with
>
> commit 0eea67eb26000657079b7fc41079097942339452
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date: Fri Dec 6 14:11:19 2013 -0800
>
> drm/i915: Create a per file_priv default context
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
Acked-by: Ben Widawsky <ben@bwidawsk.net>
I can bump it to a r-b if Mika doesn't
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 2adf6aa..f8f13ae 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -930,9 +930,6 @@ int i915_get_reset_stats_ioctl(struct drm_device *dev,
> if (args->flags || args->pad)
> return -EINVAL;
>
> - if (args->ctx_id == DEFAULT_CONTEXT_ID && !capable(CAP_SYS_ADMIN))
> - return -EPERM;
> -
> ret = mutex_lock_interruptible(&dev->struct_mutex);
> if (ret)
> return ret;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] tests/gem_reset_stats: EPERM check not needed
2014-05-01 7:18 [PATCH] drm/i915: Hang counting is now always per-fd, so relax the ioctl for DEFAULT_CONTEXT Chris Wilson
2014-05-01 18:02 ` Ben Widawsky
2014-05-02 11:15 ` Mika Kuoppala
@ 2014-05-02 11:16 ` Mika Kuoppala
2 siblings, 0 replies; 4+ messages in thread
From: Mika Kuoppala @ 2014-05-02 11:16 UTC (permalink / raw)
To: intel-gfx
Without root we used to get EPERM if we tried to aquire
default context statistics. Now we have per fd statistics
so only thing which is hidden from us without root access,
is the global reset count.
Drop EPERM and check that global reset count is always zero
for nonroot.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
tests/gem_reset_stats.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index c227798..898f696 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -913,11 +913,7 @@ static void test_reset_count(const bool create_ctx)
igt_drop_root();
c2 = get_reset_count(fd, ctx);
-
- if (ctx == 0)
- igt_assert(c2 == -EPERM);
- else
- igt_assert(c2 == 0);
+ igt_assert(c2 == 0);
}
igt_waitchildren();
@@ -952,45 +948,37 @@ static int _test_params(int fd, int ctx, uint32_t flags, uint32_t pad)
return 0;
}
-typedef enum { root = 0, user } cap_t;
-
-static void _check_param_ctx(const int fd, const int ctx, const cap_t cap)
+static void _check_param_ctx(const int fd, const int ctx)
{
const uint32_t bad = rand() + 1;
- if (ctx == 0) {
- if (cap == root)
- igt_assert(_test_params(fd, ctx, 0, 0) == 0);
- else
- igt_assert(_test_params(fd, ctx, 0, 0) == -EPERM);
- }
-
+ igt_assert(_test_params(fd, ctx, 0, 0) == 0);
igt_assert(_test_params(fd, ctx, 0, bad) == -EINVAL);
igt_assert(_test_params(fd, ctx, bad, 0) == -EINVAL);
igt_assert(_test_params(fd, ctx, bad, bad) == -EINVAL);
}
-static void check_params(const int fd, const int ctx, cap_t cap)
+static void check_params(const int fd, const int ctx)
{
igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
- _check_param_ctx(fd, ctx, cap);
+ _check_param_ctx(fd, ctx);
}
static void _test_param(const int fd, const int ctx)
{
- check_params(fd, ctx, root);
+ check_params(fd, ctx);
igt_fork(child, 1) {
- check_params(fd, ctx, root);
+ check_params(fd, ctx);
igt_drop_root();
- check_params(fd, ctx, user);
+ check_params(fd, ctx);
}
- check_params(fd, ctx, root);
+ check_params(fd, ctx);
igt_waitchildren();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread