Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-03-13 Thread Andi Shyti
Hi Chris,

> We [will] expose various per-engine scheduling controls. One of which,
> 'preempt_timeout_ms', defines how we wait for a preemption request to be
> honoured by the currently executing context. If it fails to relieve the
> GPU within the required timeout, the engine is reset and the miscreant
> forcibly evicted.
> 
> Signed-off-by: Chris Wilson 

for the next three patches (3, 4 and 5) I'm not going to repeat
myself :P

Reviewed-by: Andi Shyti 

Andi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-03-11 Thread Chris Wilson
We [will] expose various per-engine scheduling controls. One of which,
'preempt_timeout_ms', defines how we wait for a preemption request to be
honoured by the currently executing context. If it fails to relieve the
GPU within the required timeout, the engine is reset and the miscreant
forcibly evicted.

Signed-off-by: Chris Wilson 
---
 tests/Makefile.sources |   3 +
 tests/i915/sysfs_preempt_timeout.c | 311 +
 tests/intel-ci/blacklist.txt   |   1 +
 tests/meson.build  |   1 +
 4 files changed, 316 insertions(+)
 create mode 100644 tests/i915/sysfs_preempt_timeout.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 4ac6e35ca..c1711b336 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -104,6 +104,9 @@ TESTS_progs = \
vgem_slow \
$(NULL)
 
+TESTS_progs += sysfs_preempt_timeout
+sysfs_preempt_timeout_SOURCES = i915/sysfs_preempt_timeout.c
+
 TESTS_progs += gem_bad_reloc
 gem_bad_reloc_SOURCES = i915/gem_bad_reloc.c
 
diff --git a/tests/i915/sysfs_preempt_timeout.c 
b/tests/i915/sysfs_preempt_timeout.c
new file mode 100644
index 0..be1905bcd
--- /dev/null
+++ b/tests/i915/sysfs_preempt_timeout.c
@@ -0,0 +1,311 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "drmtest.h" /* gem_quiescent_gpu()! */
+#include "i915/gem_engine_topology.h"
+#include "igt_dummyload.h"
+#include "igt_sysfs.h"
+#include "ioctl_wrappers.h" /* igt_require_gem()! */
+#include "sw_sync.h"
+
+#include "igt_debugfs.h"
+
+#define ATTR "preempt_timeout_ms"
+#define RESET_TIMEOUT 20 /* milliseconds, at least one jiffie for kworker */
+
+static bool __enable_hangcheck(int dir, bool state)
+{
+   return igt_sysfs_set(dir, "enable_hangcheck", state ? "1" : "0");
+}
+
+static bool enable_hangcheck(int i915, bool state)
+{
+   bool success;
+   int dir;
+
+   dir = igt_sysfs_open_parameters(i915);
+   if (dir < 0) /* no parameters, must be default! */
+   return false;
+
+   success = __enable_hangcheck(dir, state);
+   close(dir);
+
+   return success;
+}
+
+static void set_preempt_timeout(int engine, unsigned int value)
+{
+   unsigned int delay;
+
+   igt_sysfs_printf(engine, ATTR, "%u", value);
+   igt_sysfs_scanf(engine, ATTR, "%u", );
+   igt_assert_eq(delay, value);
+}
+
+static void test_idempotent(int i915, int engine)
+{
+   unsigned int delays[] = { 0, 1, 1000, 1234, 654321 };
+   unsigned int saved;
+
+   /* Quick test that store/show reports the same values */
+
+   igt_assert(igt_sysfs_scanf(engine, ATTR, "%u", ) == 1);
+   igt_debug("Initial %s:%u\n", ATTR, saved);
+
+   for (int i = 0; i < ARRAY_SIZE(delays); i++)
+   set_preempt_timeout(engine, delays[i]);
+
+   set_preempt_timeout(engine, saved);
+}
+
+static void test_invalid(int i915, int engine)
+{
+   unsigned int saved, delay;
+
+   /* Quick test that values that are not representable are rejected */
+
+   igt_assert(igt_sysfs_scanf(engine, ATTR, "%u", ) == 1);
+   igt_debug("Initial %s:%u\n", ATTR, saved);
+
+   igt_sysfs_printf(engine, ATTR, PRIu64, -1);
+   igt_sysfs_scanf(engine, ATTR, "%u", );
+   igt_assert_eq(delay, saved);
+
+   igt_sysfs_printf(engine, ATTR, "%d", -1);
+   igt_sysfs_scanf(engine, ATTR, "%u", );
+   igt_assert_eq(delay, saved);
+
+   igt_sysfs_printf(engine, ATTR, PRIu64, 40ull << 32);
+   igt_sysfs_scanf(engine, ATTR, "%u", );
+   igt_assert_eq(delay, saved);
+}
+
+static void set_unbannable(int i915, uint32_t ctx)
+{
+   struct drm_i915_gem_context_param p = {
+   .ctx_id = ctx,
+   .param = I915_CONTEXT_PARAM_BANNABLE,
+   };
+
+   

Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-29 Thread Chris Wilson
Quoting Chris Wilson (2020-02-29 18:34:49)
> Quoting Andi Shyti (2020-02-29 12:45:27)
> > > > > > > + char buf[512];
> > > > > > > + int len;
> > > > > > > +
> > > > > > > + lseek(engines, 0, SEEK_SET);
> > > > > > > + while ((len = syscall(SYS_getdents64, engines, buf, 
> > > > > > > sizeof(buf))) > 0) {
> > > > > > > + void *ptr = buf;
> > > > > > > +
> > > > > > > + while (len) {
> > > > > > > + struct linux_dirent64 {
> > > > > > > + ino64_td_ino;
> > > > > > > + off64_td_off;
> > > > > > > + unsigned short d_reclen;
> > > > > > > + unsigned char  d_type;
> > > > > > > + char   d_name[];
> > > > > > > + } *de = ptr;
> > > > > > 
> > > > > > what is the need for having your own linux_dirent64?
> > > > > 
> > > > > fdopendir() takes ownership of the fd, preventing reuse. And
> > > > > fdopendir(dup()) is getting ridiculous.
> > > > 
> > > > why not using dirent64?
> > > 
> > > It's still the same problem that it takes a DIR, assuming ownership of
> > > the fd. Why using linux_dirent64 because the manpage says so -- if you
> > > are going to use the syscall, you need to match it's calling
> > > conventions, not a middleman's.
> > 
> > I understand, but in bits/dirent.h there is, with some
> > assumptions, this part:
> > 
> > #ifdef __USE_LARGEFILE64
> > struct dirent64
> >   {
> > __ino64_t d_ino;
> > __off64_t d_off;
> > unsigned short int d_reclen;
> > unsigned char d_type;
> > char d_name[256];   /* We must not include limits.h! */
> >   };
> > #endif
> > 
> > why redefine a struct linux_dirent64?
> 
> Because the manpage didn't mention that!

Though it is still a libc structure rather than the syscall definition
:(
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-29 Thread Chris Wilson
Quoting Andi Shyti (2020-02-29 12:45:27)
> > > > > > + char buf[512];
> > > > > > + int len;
> > > > > > +
> > > > > > + lseek(engines, 0, SEEK_SET);
> > > > > > + while ((len = syscall(SYS_getdents64, engines, buf, 
> > > > > > sizeof(buf))) > 0) {
> > > > > > + void *ptr = buf;
> > > > > > +
> > > > > > + while (len) {
> > > > > > + struct linux_dirent64 {
> > > > > > + ino64_td_ino;
> > > > > > + off64_td_off;
> > > > > > + unsigned short d_reclen;
> > > > > > + unsigned char  d_type;
> > > > > > + char   d_name[];
> > > > > > + } *de = ptr;
> > > > > 
> > > > > what is the need for having your own linux_dirent64?
> > > > 
> > > > fdopendir() takes ownership of the fd, preventing reuse. And
> > > > fdopendir(dup()) is getting ridiculous.
> > > 
> > > why not using dirent64?
> > 
> > It's still the same problem that it takes a DIR, assuming ownership of
> > the fd. Why using linux_dirent64 because the manpage says so -- if you
> > are going to use the syscall, you need to match it's calling
> > conventions, not a middleman's.
> 
> I understand, but in bits/dirent.h there is, with some
> assumptions, this part:
> 
> #ifdef __USE_LARGEFILE64
> struct dirent64
>   {
> __ino64_t d_ino;
> __off64_t d_off;
> unsigned short int d_reclen;
> unsigned char d_type;
> char d_name[256];   /* We must not include limits.h! */
>   };
> #endif
> 
> why redefine a struct linux_dirent64?

Because the manpage didn't mention that!
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-29 Thread Andi Shyti
> > > > > + char buf[512];
> > > > > + int len;
> > > > > +
> > > > > + lseek(engines, 0, SEEK_SET);
> > > > > + while ((len = syscall(SYS_getdents64, engines, buf, 
> > > > > sizeof(buf))) > 0) {
> > > > > + void *ptr = buf;
> > > > > +
> > > > > + while (len) {
> > > > > + struct linux_dirent64 {
> > > > > + ino64_td_ino;
> > > > > + off64_td_off;
> > > > > + unsigned short d_reclen;
> > > > > + unsigned char  d_type;
> > > > > + char   d_name[];
> > > > > + } *de = ptr;
> > > > 
> > > > what is the need for having your own linux_dirent64?
> > > 
> > > fdopendir() takes ownership of the fd, preventing reuse. And
> > > fdopendir(dup()) is getting ridiculous.
> > 
> > why not using dirent64?
> 
> It's still the same problem that it takes a DIR, assuming ownership of
> the fd. Why using linux_dirent64 because the manpage says so -- if you
> are going to use the syscall, you need to match it's calling
> conventions, not a middleman's.

I understand, but in bits/dirent.h there is, with some
assumptions, this part:

#ifdef __USE_LARGEFILE64
struct dirent64
  {
__ino64_t d_ino;
__off64_t d_off;
unsigned short int d_reclen;
unsigned char d_type;
char d_name[256];   /* We must not include limits.h! */
  };
#endif

why redefine a struct linux_dirent64?

Andi

PS We have time until Monday morning to discuss this, right? :)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-29 Thread Chris Wilson
Quoting Andi Shyti (2020-02-28 23:51:24)
> > > > +void dyn_sysfs_engines(int i915, int engines, const char *file,
> > > > +void (*test)(int, int))
> > > > +{
> > > > + char buf[512];
> > > > + int len;
> > > > +
> > > > + lseek(engines, 0, SEEK_SET);
> > > > + while ((len = syscall(SYS_getdents64, engines, buf, sizeof(buf))) 
> > > > > 0) {
> > > > + void *ptr = buf;
> > > > +
> > > > + while (len) {
> > > > + struct linux_dirent64 {
> > > > + ino64_td_ino;
> > > > + off64_td_off;
> > > > + unsigned short d_reclen;
> > > > + unsigned char  d_type;
> > > > + char   d_name[];
> > > > + } *de = ptr;
> > > 
> > > what is the need for having your own linux_dirent64?
> > 
> > fdopendir() takes ownership of the fd, preventing reuse. And
> > fdopendir(dup()) is getting ridiculous.
> 
> why not using dirent64?

It's still the same problem that it takes a DIR, assuming ownership of
the fd. Why using linux_dirent64 because the manpage says so -- if you
are going to use the syscall, you need to match it's calling
conventions, not a middleman's.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-28 Thread Andi Shyti
> > > +void dyn_sysfs_engines(int i915, int engines, const char *file,
> > > +void (*test)(int, int))
> > > +{
> > > + char buf[512];
> > > + int len;
> > > +
> > > + lseek(engines, 0, SEEK_SET);
> > > + while ((len = syscall(SYS_getdents64, engines, buf, sizeof(buf))) > 
> > > 0) {
> > > + void *ptr = buf;
> > > +
> > > + while (len) {
> > > + struct linux_dirent64 {
> > > + ino64_td_ino;
> > > + off64_td_off;
> > > + unsigned short d_reclen;
> > > + unsigned char  d_type;
> > > + char   d_name[];
> > > + } *de = ptr;
> > 
> > what is the need for having your own linux_dirent64?
> 
> fdopendir() takes ownership of the fd, preventing reuse. And
> fdopendir(dup()) is getting ridiculous.

why not using dirent64?

Andi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-28 Thread Andi Shyti
Hi Chris,

> +static int create_ext_ioctl(int i915,
> + struct drm_i915_gem_context_create_ext *arg)
> +{
> + int err;
> +
> + err = 0;
> + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> + err = -errno;
> + igt_assume(err);
> + }
> +
> + errno = 0;
> + return err;
> +}
> +
>  /**
>   * gem_has_contexts:
>   * @fd: open i915 drm file descriptor
> @@ -324,17 +339,14 @@ __gem_context_clone(int i915,
>   .flags = flags | I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
>   .extensions = to_user_pointer(),
>   };
> - int err = 0;
> + int err;
>  
> - if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, )) {
> - err = -errno;
> - igt_assume(err);
> - }
> + err = create_ext_ioctl(i915, );
> + if (err)
> + return err;
>  
>   *out = arg.ctx_id;
> -
> - errno = 0;
> - return err;
> + return 0;
>  }
>  
>  static bool __gem_context_has(int i915, uint32_t share, unsigned int flags)
> @@ -382,16 +394,8 @@ bool gem_has_context_clone(int i915)
>   .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
>   .extensions = to_user_pointer(),
>   };
> - int err;
> -
> - err = 0;
> - if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, )) {
> - err = -errno;
> - igt_assume(err);
> - }
> - errno = 0;
>  
> - return err == -ENOENT;
> + return create_ext_ioctl(i915, ) == -ENOENT;
>  }

I'd like to see the above in a separate patch.

> +void dyn_sysfs_engines(int i915, int engines, const char *file,
> +void (*test)(int, int))
> +{
> + char buf[512];
> + int len;
> +
> + lseek(engines, 0, SEEK_SET);
> + while ((len = syscall(SYS_getdents64, engines, buf, sizeof(buf))) > 0) {
> + void *ptr = buf;
> +
> + while (len) {
> + struct linux_dirent64 {
> + ino64_td_ino;
> + off64_td_off;
> + unsigned short d_reclen;
> + unsigned char  d_type;
> + char   d_name[];
> + } *de = ptr;

what is the need for having your own linux_dirent64?

All the rest look good.

Andi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-28 Thread Chris Wilson
Quoting Andi Shyti (2020-02-28 23:27:04)
> Hi Chris,
> 
> > +static int create_ext_ioctl(int i915,
> > + struct drm_i915_gem_context_create_ext *arg)
> > +{
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> > + err = -errno;
> > + igt_assume(err);
> > + }
> > +
> > + errno = 0;
> > + return err;
> > +}
> > +
> >  /**
> >   * gem_has_contexts:
> >   * @fd: open i915 drm file descriptor
> > @@ -324,17 +339,14 @@ __gem_context_clone(int i915,
> >   .flags = flags | I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> >   .extensions = to_user_pointer(),
> >   };
> > - int err = 0;
> > + int err;
> >  
> > - if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, )) {
> > - err = -errno;
> > - igt_assume(err);
> > - }
> > + err = create_ext_ioctl(i915, );
> > + if (err)
> > + return err;
> >  
> >   *out = arg.ctx_id;
> > -
> > - errno = 0;
> > - return err;
> > + return 0;
> >  }
> >  
> >  static bool __gem_context_has(int i915, uint32_t share, unsigned int flags)
> > @@ -382,16 +394,8 @@ bool gem_has_context_clone(int i915)
> >   .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> >   .extensions = to_user_pointer(),
> >   };
> > - int err;
> > -
> > - err = 0;
> > - if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, )) {
> > - err = -errno;
> > - igt_assume(err);
> > - }
> > - errno = 0;
> >  
> > - return err == -ENOENT;
> > + return create_ext_ioctl(i915, ) == -ENOENT;
> >  }
> 
> I'd like to see the above in a separate patch.

It's part of the test, I can put it back inside each .c if you prefer.

> > +void dyn_sysfs_engines(int i915, int engines, const char *file,
> > +void (*test)(int, int))
> > +{
> > + char buf[512];
> > + int len;
> > +
> > + lseek(engines, 0, SEEK_SET);
> > + while ((len = syscall(SYS_getdents64, engines, buf, sizeof(buf))) > 
> > 0) {
> > + void *ptr = buf;
> > +
> > + while (len) {
> > + struct linux_dirent64 {
> > + ino64_td_ino;
> > + off64_td_off;
> > + unsigned short d_reclen;
> > + unsigned char  d_type;
> > + char   d_name[];
> > + } *de = ptr;
> 
> what is the need for having your own linux_dirent64?

fdopendir() takes ownership of the fd, preventing reuse. And
fdopendir(dup()) is getting ridiculous.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-02-28 Thread Chris Wilson
We [will] expose various per-engine scheduling controls. One of which,
'preempt_timeout_ms', defines how we wait for a preemption request to be
honoured by the currently executing context. If it fails to relieve the
GPU within the required timeout, the engine is reset and the miscreant
forcibly evicted.

Signed-off-by: Chris Wilson 
---
 lib/i915/gem_context.c |  64 --
 lib/i915/gem_context.h |   2 +
 lib/i915/gem_engine_topology.c |  48 +
 lib/i915/gem_engine_topology.h |   3 +
 tests/Makefile.sources |   3 +
 tests/i915/sysfs_preempt_timeout.c | 309 +
 tests/intel-ci/blacklist.txt   |   1 +
 tests/meson.build  |   1 +
 8 files changed, 414 insertions(+), 17 deletions(-)
 create mode 100644 tests/i915/sysfs_preempt_timeout.c

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index 50dfee3d1..169e43d2e 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -43,6 +43,21 @@
  * software features improving submission model (context priority).
  */
 
+static int create_ext_ioctl(int i915,
+   struct drm_i915_gem_context_create_ext *arg)
+{
+   int err;
+
+   err = 0;
+   if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
+   err = -errno;
+   igt_assume(err);
+   }
+
+   errno = 0;
+   return err;
+}
+
 /**
  * gem_has_contexts:
  * @fd: open i915 drm file descriptor
@@ -324,17 +339,14 @@ __gem_context_clone(int i915,
.flags = flags | I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
.extensions = to_user_pointer(),
};
-   int err = 0;
+   int err;
 
-   if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, )) {
-   err = -errno;
-   igt_assume(err);
-   }
+   err = create_ext_ioctl(i915, );
+   if (err)
+   return err;
 
*out = arg.ctx_id;
-
-   errno = 0;
-   return err;
+   return 0;
 }
 
 static bool __gem_context_has(int i915, uint32_t share, unsigned int flags)
@@ -382,16 +394,8 @@ bool gem_has_context_clone(int i915)
.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
.extensions = to_user_pointer(),
};
-   int err;
-
-   err = 0;
-   if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, )) {
-   err = -errno;
-   igt_assume(err);
-   }
-   errno = 0;
 
-   return err == -ENOENT;
+   return create_ext_ioctl(i915, ) == -ENOENT;
 }
 
 /**
@@ -492,3 +496,29 @@ gem_context_copy_engines(int src_fd, uint32_t src, int 
dst_fd, uint32_t dst)
param.ctx_id = dst;
gem_context_set_param(dst_fd, );
 }
+
+uint32_t gem_context_create_for_engine(int i915, unsigned int class, unsigned 
int inst)
+{
+   I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = {
+   .engines = { { .engine_class = class, .engine_instance = inst } 
}
+   };
+   struct drm_i915_gem_context_create_ext_setparam p_engines = {
+   .base = {
+   .name = I915_CONTEXT_CREATE_EXT_SETPARAM,
+   .next_extension = 0, /* end of chain */
+   },
+   .param = {
+   .param = I915_CONTEXT_PARAM_ENGINES,
+   .value = to_user_pointer(),
+   .size = sizeof(engines),
+   },
+   };
+   struct drm_i915_gem_context_create_ext create = {
+   .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
+   .extensions = to_user_pointer(_engines),
+   };
+
+   igt_assert_eq(create_ext_ioctl(i915, ), 0);
+   igt_assert_neq(create.ctx_id, 0);
+   return create.ctx_id;
+}
diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
index 15e5db281..b478c47a1 100644
--- a/lib/i915/gem_context.h
+++ b/lib/i915/gem_context.h
@@ -34,6 +34,8 @@ int __gem_context_create(int fd, uint32_t *ctx_id);
 void gem_context_destroy(int fd, uint32_t ctx_id);
 int __gem_context_destroy(int fd, uint32_t ctx_id);
 
+uint32_t gem_context_create_for_engine(int fd, unsigned int class, unsigned 
int inst);
+
 int __gem_context_clone(int i915,
uint32_t src, unsigned int share,
unsigned int flags,
diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
index 640777ae4..64a6b0472 100644
--- a/lib/i915/gem_engine_topology.c
+++ b/lib/i915/gem_engine_topology.c
@@ -22,6 +22,8 @@
  */
 
 #include 
+#include 
+#include 
 #include 
 
 #include "drmtest.h"
@@ -411,3 +413,49 @@ uint32_t gem_engine_mmio_base(int i915, const char *engine)
 
return mmio;
 }
+
+void dyn_sysfs_engines(int i915, int engines, const char *file,
+  void (*test)(int, int))
+{
+   char buf[512];
+   int len;
+
+   lseek(engines, 0, SEEK_SET);
+   while ((len = syscall(SYS_getdents64, 

[Intel-gfx] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs

2020-01-27 Thread Chris Wilson
We [will] expose various per-engine scheduling controls. One of which,
'preempt_timeout_ms', defines how we wait for a preemption request to be
honoured by the currently executing context. If it fails to relieve the
GPU within the required timeout, the engine is reset and the miscreant
forcibly evicted.

Signed-off-by: Chris Wilson 
---
 lib/i915/gem_context.c |  41 
 lib/i915/gem_context.h |   2 +
 lib/i915/gem_engine_topology.c |  48 +
 lib/i915/gem_engine_topology.h |   3 +
 tests/Makefile.sources |   3 +
 tests/i915/sysfs_preempt_timeout.c | 309 +
 tests/meson.build  |   1 +
 7 files changed, 407 insertions(+)
 create mode 100644 tests/i915/sysfs_preempt_timeout.c

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index 0b6a554df..fc874a187 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -462,3 +462,44 @@ bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t 
engine)
 
return __gem_execbuf(fd, ) == -ENOENT;
 }
+
+static int create_ext_ioctl(int i915,
+   struct drm_i915_gem_context_create_ext *arg)
+{
+   int err;
+
+   err = 0;
+   if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
+   err = -errno;
+   igt_assume(err);
+   }
+
+   errno = 0;
+   return err;
+}
+
+uint32_t gem_context_create_for_engine(int i915, unsigned int class, unsigned 
int inst)
+{
+   I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = {
+   .engines = { { .engine_class = class, .engine_instance = inst } 
}
+   };
+   struct drm_i915_gem_context_create_ext_setparam p_engines = {
+   .base = {
+   .name = I915_CONTEXT_CREATE_EXT_SETPARAM,
+   .next_extension = 0, /* end of chain */
+   },
+   .param = {
+   .param = I915_CONTEXT_PARAM_ENGINES,
+   .value = to_user_pointer(),
+   .size = sizeof(engines),
+   },
+   };
+   struct drm_i915_gem_context_create_ext create = {
+   .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
+   .extensions = to_user_pointer(_engines),
+   };
+
+   igt_assert_eq(create_ext_ioctl(i915, ), 0);
+   igt_assert_neq(create.ctx_id, 0);
+   return create.ctx_id;
+}
diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
index cf2ba33fe..ded75bb9c 100644
--- a/lib/i915/gem_context.h
+++ b/lib/i915/gem_context.h
@@ -34,6 +34,8 @@ int __gem_context_create(int fd, uint32_t *ctx_id);
 void gem_context_destroy(int fd, uint32_t ctx_id);
 int __gem_context_destroy(int fd, uint32_t ctx_id);
 
+uint32_t gem_context_create_for_engine(int fd, unsigned int class, unsigned 
int inst);
+
 int __gem_context_clone(int i915,
uint32_t src, unsigned int share,
unsigned int flags,
diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
index 058983123..81faf3c15 100644
--- a/lib/i915/gem_engine_topology.c
+++ b/lib/i915/gem_engine_topology.c
@@ -22,6 +22,8 @@
  */
 
 #include 
+#include 
+#include 
 #include 
 
 #include "drmtest.h"
@@ -415,3 +417,49 @@ uint32_t gem_engine_mmio_base(int i915, const char *engine)
 
return mmio;
 }
+
+void dyn_sysfs_engines(int i915, int engines, const char *file,
+  void (*test)(int, int))
+{
+   char buf[512];
+   int len;
+
+   lseek(engines, 0, SEEK_SET);
+   while ((len = syscall(SYS_getdents64, engines, buf, sizeof(buf))) > 0) {
+   void *ptr = buf;
+
+   while (len) {
+   struct linux_dirent64 {
+   ino64_td_ino;
+   off64_td_off;
+   unsigned short d_reclen;
+   unsigned char  d_type;
+   char   d_name[];
+   } *de = ptr;
+   char *name;
+   int engine;
+
+   ptr += de->d_reclen;
+   len -= de->d_reclen;
+
+   engine = openat(engines, de->d_name, O_RDONLY);
+   name = igt_sysfs_get(engine, "name");
+   if (!name) {
+   close(engine);
+   continue;
+   }
+
+   igt_dynamic(name) {
+   if (file) {
+   struct stat st;
+
+   igt_require(fstatat(engine, file, , 
0) == 0);
+   }
+
+   test(i915, engine);
+   }
+
+   close(engine);
+   }
+   }
+}
diff --git