| Message ID | 20171012221037.9081-2-antonio.argenziano@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/10/17 15:10, Antonio Argenziano wrote: > When a client gets a fixed amount of its contexts banned, will be > blocked from creating any new contexts itself. This patch adds a test > for this feature. > > Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > --- > tests/gem_ctx_create.c | 10 +++++----- > tests/gem_reset_stats.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+), 5 deletions(-) > > diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c > index ae0825a1..abe9a32e 100644 > --- a/tests/gem_ctx_create.c > +++ b/tests/gem_ctx_create.c > @@ -45,7 +45,7 @@ static unsigned all_nengine; > static unsigned ppgtt_engines[16]; > static unsigned ppgtt_nengine; > > -static int __gem_context_create(int fd, struct drm_i915_gem_context_create *arg) > +static int __gem_context_create_local(int fd, struct drm_i915_gem_context_create *arg) > { > int ret = 0; > if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg)) > @@ -255,7 +255,7 @@ static void maximum(int fd, int ncpus, unsigned mode) > > err = -ENOMEM; > if (avail_mem > (count + 1) * ctx_size) > - err = __gem_context_create(fd, &create); > + err = __gem_context_create_local(fd, &create); > if (err) { > igt_info("Created %lu contexts, before failing with '%s' [%d]\n", > count, strerror(-err), -err); > @@ -322,7 +322,7 @@ igt_main > igt_require_gem(fd); > > memset(&create, 0, sizeof(create)); > - igt_require(__gem_context_create(fd, &create) == 0); > + igt_require(__gem_context_create_local(fd, &create) == 0); > gem_context_destroy(fd, create.ctx_id); > > for_each_engine(fd, engine) { > @@ -347,7 +347,7 @@ igt_main > memset(&create, 0, sizeof(create)); > create.ctx_id = rand(); > create.pad = 0; > - igt_assert_eq(__gem_context_create(fd, &create), 0); > + igt_assert_eq(__gem_context_create_local(fd, &create), 0); > igt_assert(create.ctx_id != 0); > gem_context_destroy(fd, create.ctx_id); > } > @@ -356,7 +356,7 @@ igt_main > memset(&create, 0, sizeof(create)); > create.ctx_id = rand(); > create.pad = 1; > - igt_assert_eq(__gem_context_create(fd, &create), -EINVAL); > + igt_assert_eq(__gem_context_create_local(fd, &create), -EINVAL); > } > > igt_subtest("maximum-mem") Ops, this was supposed to be in patch 1/2.
Quoting Antonio Argenziano (2017-10-12 23:10:37) > When a client gets a fixed amount of its contexts banned, will be > blocked from creating any new contexts itself. This patch adds a test > for this feature. Fixed by whom? The "feature" is a primitive DoS prevention. Atm it is purely implementation defined as we don't have a "service level agreement" to describe exactly what can be expected. It is certainly not an ABI one client can rely on for itself, more a general effect about how much interruption a client can expect from any other. So how do you define such behaviour? Do we want to carve the "three strikes" rule into stone? How about when the malicious client creates new contexts to avoid the strikes? How about when a malicious client decides to try and DoS its common parent (e.g firefox and multiple webgl, with one bad apple trying to upset the barrel)? At the end of the day, limits will be imposed in exactly the same way as for traditional processes, and gpu resources will be managed and scheduled under the same umbrella. From that viewpoint, everything we do today will have to undergo radical change. So consider careful what you want to record as user ABI. Atm, userspace requires support for ARB_context_robustness and we have not been asked for anything more. -Chris
diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c index ae0825a1..abe9a32e 100644 --- a/tests/gem_ctx_create.c +++ b/tests/gem_ctx_create.c @@ -45,7 +45,7 @@ static unsigned all_nengine; static unsigned ppgtt_engines[16]; static unsigned ppgtt_nengine; -static int __gem_context_create(int fd, struct drm_i915_gem_context_create *arg) +static int __gem_context_create_local(int fd, struct drm_i915_gem_context_create *arg) { int ret = 0; if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg)) @@ -255,7 +255,7 @@ static void maximum(int fd, int ncpus, unsigned mode) err = -ENOMEM; if (avail_mem > (count + 1) * ctx_size) - err = __gem_context_create(fd, &create); + err = __gem_context_create_local(fd, &create); if (err) { igt_info("Created %lu contexts, before failing with '%s' [%d]\n", count, strerror(-err), -err); @@ -322,7 +322,7 @@ igt_main igt_require_gem(fd); memset(&create, 0, sizeof(create)); - igt_require(__gem_context_create(fd, &create) == 0); + igt_require(__gem_context_create_local(fd, &create) == 0); gem_context_destroy(fd, create.ctx_id); for_each_engine(fd, engine) { @@ -347,7 +347,7 @@ igt_main memset(&create, 0, sizeof(create)); create.ctx_id = rand(); create.pad = 0; - igt_assert_eq(__gem_context_create(fd, &create), 0); + igt_assert_eq(__gem_context_create_local(fd, &create), 0); igt_assert(create.ctx_id != 0); gem_context_destroy(fd, create.ctx_id); } @@ -356,7 +356,7 @@ igt_main memset(&create, 0, sizeof(create)); create.ctx_id = rand(); create.pad = 1; - igt_assert_eq(__gem_context_create(fd, &create), -EINVAL); + igt_assert_eq(__gem_context_create_local(fd, &create), -EINVAL); } igt_subtest("maximum-mem") diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c index edc40767..f51475a9 100644 --- a/tests/gem_reset_stats.c +++ b/tests/gem_reset_stats.c @@ -438,6 +438,50 @@ static void test_ban_ctx(const struct intel_execution_engine *e) close(fd); } +#define CLIENT_BAN 4 +#define CTX_BAN 5 +static void test_client_ban(const struct intel_execution_engine *e) +{ + int fd_bad,fd_good; + struct local_drm_i915_reset_stats rs_bad, rs_good; + int ctx_bans; + uint32_t ctx_bad[CLIENT_BAN]; + uint32_t test_ctx; + + fd_bad = drm_open_driver(DRIVER_INTEL); + fd_good = drm_open_driver(DRIVER_INTEL); + + assert_reset_status(fd_bad, fd_bad, 0, RS_NO_ERROR); + assert_reset_status(fd_good, fd_good, 0, RS_NO_ERROR); + + for (int i = 0; i < CLIENT_BAN; i++) { + ctx_bad[i] = gem_context_create(fd_bad); + noop(fd_bad, ctx_bad[i], e); + assert_reset_status(fd_bad, fd_bad, ctx_bad[i], RS_NO_ERROR); + + ctx_bans = CTX_BAN; + while (ctx_bans--) + inject_hang(fd_bad, ctx_bad[i], e, BAN); + + igt_assert_eq(noop(fd_bad, ctx_bad[i], e), -EIO); + + assert_reset_status(fd_bad, fd_bad, ctx_bad[i], RS_BATCH_ACTIVE); + igt_assert_eq(gem_reset_stats(fd_bad, ctx_bad[i], &rs_bad), 0); + igt_assert_eq(rs_bad.batch_active, CTX_BAN); + } + + igt_assert_eq(__gem_context_create(fd_bad, &test_ctx), -EIO); + igt_info("Client banned after %d bad contexts\n", CLIENT_BAN); + + igt_assert_lt(0, noop(fd_good, 0, e)); + assert_reset_status(fd_good, fd_good, 0, RS_NO_ERROR); + igt_assert_eq(gem_reset_stats(fd_good, 0, &rs_good), 0); + igt_assert_eq(rs_good.batch_active, 0); + + close(fd_bad); + close(fd_good); +} + static void test_unrelated_ctx(const struct intel_execution_engine *e) { int fd1,fd2; @@ -817,6 +861,9 @@ igt_main igt_subtest_f("ban-ctx-%s", e->name) RUN_CTX_TEST(test_ban_ctx(e)); + igt_subtest_f("ban-client-%s", e->name) + RUN_CTX_TEST(test_client_ban(e)); + igt_subtest_f("reset-count-%s", e->name) RUN_TEST(test_reset_count(e, false));
When a client gets a fixed amount of its contexts banned, will be blocked from creating any new contexts itself. This patch adds a test for this feature. Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> --- tests/gem_ctx_create.c | 10 +++++----- tests/gem_reset_stats.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-)