[igt-dev] [PATCH i-g-t 0/2] Add Xe lib support for hang & GT reset

[igt-dev] [PATCH i-g-t 0/2] Add Xe lib support for hang & GT reset

[janga.rahul.kumar]

From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>

Add library support for hang and resetting all the GT's.

Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
Cc: Kunal Joshi <kunal1.joshi@intel.com>
Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>

Janga Rahul Kumar (2):
  lib/xe: Add support to reset all GT's
  lib/xe: Add hang library support

 lib/igt_gt.c      | 28 ++++++++++++++++++
 lib/xe/xe_ioctl.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_ioctl.h |  8 ++++++
 3 files changed, 108 insertions(+)

-- 
2.25.1

[igt-dev] [PATCH i-g-t 1/2] lib/xe: Add support to reset all GT's

[janga.rahul.kumar]

From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>

Add library support to check support for GT reset and
force reset all GT's.

Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
Cc: Kunal Joshi <kunal1.joshi@intel.com>
Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
---
 lib/igt_gt.c      |  7 +++++++
 lib/xe/xe_ioctl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_ioctl.h |  2 ++
 3 files changed, 61 insertions(+)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index d4a825e66..2ce464ba6 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -45,6 +45,7 @@
 #include "intel_chipset.h"
 #include "igt_dummyload.h"
 
+#include "xe/xe_ioctl.h"
 /**
  * SECTION:igt_gt
  * @short_description: GT support library
@@ -60,6 +61,9 @@ static int reset_query_once = -1;
 
 static bool has_gpu_reset(int fd)
 {
+	if (is_xe_device(fd))
+		has_xe_gt_reset(fd);
+
 	if (reset_query_once < 0) {
 		reset_query_once = gem_gpu_reset_type(fd);
 
@@ -395,6 +399,9 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
  */
 void igt_force_gpu_reset(int drm_fd)
 {
+	if (is_xe_device(drm_fd))
+		xe_force_gt_reset_all(drm_fd);
+
 	int dir, wedged;
 
 	igt_debug("Triggering GPU reset\n");
diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
index 1f9240cd9..488aa218e 100644
--- a/lib/xe/xe_ioctl.c
+++ b/lib/xe/xe_ioctl.c
@@ -41,6 +41,7 @@
 #include "config.h"
 #include "drmtest.h"
 #include "igt_syncobj.h"
+#include "igt_sysfs.h"
 #include "ioctl_wrappers.h"
 #include "xe_ioctl.h"
 #include "xe_query.h"
@@ -455,6 +456,57 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
 	return ts.tv_sec * 1e9 + ts.tv_nsec;
 }
 
+
+/**
+ * has_xe_gt_reset:
+ * @fd: open xe drm file descriptor
+ *
+ * Check gt force reset syfs entry is available or not
+ *
+ * Returns: reset sysfs entry available
+ */
+bool has_xe_gt_reset(int fd)
+{
+	char reset_sysfs_path[100];
+	struct stat st;
+	int gt;
+	int reset_sysfs_fd = -1;
+	int sysfs_fd = -1;
+	igt_assert_eq(fstat(fd, &st), 0);
+
+	sysfs_fd = igt_sysfs_open(fd);
+	igt_assert(sysfs_fd != -1);
+
+	xe_for_each_gt(fd, gt) {
+		sprintf(reset_sysfs_path, "/sys/kernel/debug/dri/%d/gt%d/force_reset", minor(st.st_rdev), gt);
+		reset_sysfs_fd = openat(sysfs_fd, reset_sysfs_path, O_RDONLY);
+
+		if(reset_sysfs_fd == -1) {
+			close(sysfs_fd);
+			return 0;
+		}
+
+		close(reset_sysfs_fd);
+       }
+
+	close(sysfs_fd);
+	return 1;
+}
+
+/**
+ * xe_force_gt_reset_all:
+ *
+ * Forces reset of all the GT's.
+ *
+ */
+void xe_force_gt_reset_all(int xe_fd)
+{
+	int gt;
+	xe_for_each_gt(xe_fd, gt)
+		xe_force_gt_reset(xe_fd, gt);
+}
+
+
 void xe_force_gt_reset(int fd, int gt)
 {
 	char reset_string[128];
diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
index 320e0f9f6..5a528b345 100644
--- a/lib/xe/xe_ioctl.h
+++ b/lib/xe/xe_ioctl.h
@@ -87,6 +87,8 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
 			       struct drm_xe_engine_class_instance *eci,
 			       int64_t timeout);
 void xe_force_gt_reset(int fd, int gt);
+void xe_force_gt_reset_all(int fd);
+bool has_xe_gt_reset(int fd);
 void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
 		   uint32_t property, uint32_t value);
 
-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/2] lib/xe: Add hang library support

[janga.rahul.kumar]

From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>

Add hang library support using spinners.

Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
Cc: Kunal Joshi <kunal1.joshi@intel.com>
Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
---
 lib/igt_gt.c      | 21 +++++++++++++++++++++
 lib/xe/xe_ioctl.c | 20 ++++++++++++++++++++
 lib/xe/xe_ioctl.h |  6 ++++++
 3 files changed, 47 insertions(+)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 2ce464ba6..6821179ca 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -170,6 +170,15 @@ static void context_set_ban(int fd, unsigned ctx, unsigned ban)
 
 igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
 {
+	if (is_xe_device(fd)) {
+		if (!igt_check_boolean_env_var("IGT_HANG", true))
+			igt_skip("hang injection disabled by user [IGT_HANG=0]\n");
+
+		igt_require(has_gpu_reset(fd));
+
+		return (struct igt_hang){ 0, ctx, 0, flags };
+       }
+
 	struct drm_i915_gem_context_param param = {
 		.ctx_id = ctx,
 	};
@@ -219,6 +228,9 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
 
 void igt_disallow_hang(int fd, igt_hang_t arg)
 {
+	if (is_xe_device(fd))
+		return;
+
 	context_set_ban(fd, arg.ctx, arg.ban);
 
 	if ((arg.flags & HANG_ALLOW_CAPTURE) == 0) {
@@ -290,6 +302,9 @@ static bool has_ctx_exec(int fd, unsigned ring, uint32_t ctx)
 static igt_hang_t __igt_hang_ctx(int fd, uint64_t ahnd, uint32_t ctx, int ring,
 				 unsigned flags)
 {
+	if (is_xe_device(fd))
+		return xe_hang_engine(fd, ahnd, ctx, ring, flags);
+
 	struct drm_i915_gem_context_param param;
 	igt_spin_t *spin;
 	unsigned ban;
@@ -372,6 +387,12 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
 	if (!arg.spin)
 		return;
 
+	if (is_xe_device(fd)) {
+		igt_spin_free(fd, arg.spin);
+		xe_post_hang_ring(fd, arg);
+		return;
+	}
+
 	gem_sync(fd, arg.spin->handle); /* Wait until it hangs */
 	igt_spin_free(fd, arg.spin);
 
diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
index 488aa218e..5ec85239a 100644
--- a/lib/xe/xe_ioctl.c
+++ b/lib/xe/xe_ioctl.c
@@ -520,6 +520,26 @@ void xe_force_gt_reset(int fd, int gt)
 	system(reset_string);
 }
 
+
+igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
+				unsigned flags)
+{
+	uint32_t vm;
+	unsigned int engine;
+	igt_spin_t *spin_t;
+	vm = xe_vm_create(fd, 0, 0);
+	engine = xe_engine_create_class(fd, vm, DRM_XE_ENGINE_CLASS_COPY);
+
+	spin_t = igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm, .flags = IGT_SPIN_NO_PREEMPTION);
+	return (igt_hang_t){ spin_t, engine, 0, flags };
+}
+
+void xe_post_hang_ring(int fd, igt_hang_t arg)
+{
+	xe_engine_destroy(fd, arg.ctx);
+	xe_vm_destroy(fd, arg.spin->vm);
+}
+
 void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
 		   uint32_t property, uint32_t value)
 {
diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
index 5a528b345..0e9708194 100644
--- a/lib/xe/xe_ioctl.h
+++ b/lib/xe/xe_ioctl.h
@@ -15,6 +15,9 @@
 #include <stdint.h>
 #include <xe_drm.h>
 
+#include "lib/igt_gt.h"
+#include "xe_spin.h"
+
 uint32_t xe_cs_prefetch_size(int fd);
 uint32_t xe_vm_create(int fd, uint32_t flags, uint64_t ext);
 int  __xe_vm_bind(int fd, uint32_t vm, uint32_t engine, uint32_t bo,
@@ -89,6 +92,9 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
 void xe_force_gt_reset(int fd, int gt);
 void xe_force_gt_reset_all(int fd);
 bool has_xe_gt_reset(int fd);
+igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
+				unsigned flags);
+void xe_post_hang_ring(int fd, igt_hang_t arg);
 void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
 		   uint32_t property, uint32_t value);
 
-- 
2.25.1

[igt-dev] ✗ Fi.CI.BAT: failure for Add Xe lib support for hang & GT reset

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 11224 bytes --]

== Series Details ==

Series: Add Xe lib support for hang & GT reset
URL   : https://patchwork.freedesktop.org/series/121292/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13418 -> IGTPW_9455
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_9455 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_9455, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/index.html

Participating hosts (41 -> 43)
------------------------------

  Additional (2): fi-kbl-soraka fi-rkl-11600 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_9455:

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@basic-hwmon:
    - fi-kbl-soraka:      NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-kbl-soraka/igt@debugfs_test@basic-hwmon.html

  
Known issues
------------

  Here are the changes found in IGTPW_9455 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - fi-rkl-11600:       NOTRUN -> [SKIP][2] ([i915#7456])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@debugfs_test@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][4] ([i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][6] ([i915#4613]) +3 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][7] ([i915#3282])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-rkl-11600:       NOTRUN -> [SKIP][8] ([i915#7561])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-cfl-8109u:       [PASS][9] -> [FAIL][10] ([i915#7940])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/fi-cfl-8109u/igt@i915_pm_rpm@basic-pci-d3-state.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-cfl-8109u/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@basic-rte:
    - fi-cfl-guc:         [PASS][11] -> [FAIL][12] ([i915#7940])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/fi-cfl-guc/igt@i915_pm_rpm@basic-rte.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-cfl-guc/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_selftest@live@gt_mocs:
    - bat-mtlp-6:         [PASS][13] -> [DMESG-FAIL][14] ([i915#7059])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][15] ([i915#1886] / [i915#7913])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [PASS][16] -> [ABORT][17] ([i915#4983] / [i915#7461] / [i915#8347] / [i915#8384])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/bat-rpls-1/igt@i915_selftest@live@reset.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-rpls-1/igt@i915_selftest@live@reset.html
    - bat-rpls-2:         NOTRUN -> [ABORT][18] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#8347])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-rpls-2/igt@i915_selftest@live@reset.html

  * igt@i915_selftest@live@slpc:
    - bat-mtlp-6:         [PASS][19] -> [DMESG-WARN][20] ([i915#6367])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/bat-mtlp-6/igt@i915_selftest@live@slpc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-mtlp-6/igt@i915_selftest@live@slpc.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-dg2-11:         NOTRUN -> [SKIP][21] ([i915#7828])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-dg2-11/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_chamelium_hpd@dp-hpd-fast:
    - fi-rkl-11600:       NOTRUN -> [SKIP][22] ([i915#7828]) +8 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@kms_chamelium_hpd@dp-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][23] ([fdo#109271]) +14 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-kbl-soraka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-rkl-11600:       NOTRUN -> [SKIP][24] ([i915#4103]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][25] ([fdo#109285] / [i915#4098])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [PASS][26] -> [ABORT][27] ([i915#8442] / [i915#8668])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  * igt@kms_psr@primary_page_flip:
    - fi-rkl-11600:       NOTRUN -> [SKIP][28] ([i915#1072]) +3 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@kms_psr@primary_page_flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-11600:       NOTRUN -> [SKIP][29] ([i915#3555] / [i915#4098])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][30] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/fi-rkl-11600/igt@prime_vgem@basic-read.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_lrc:
    - bat-dg2-11:         [INCOMPLETE][31] ([i915#7609] / [i915#7913]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@requests:
    - bat-rpls-2:         [ABORT][33] ([i915#4983] / [i915#7913]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/bat-rpls-2/igt@i915_selftest@live@requests.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-rpls-2/igt@i915_selftest@live@requests.html

  
#### Warnings ####

  * igt@i915_selftest@live@requests:
    - bat-mtlp-8:         [DMESG-FAIL][35] ([i915#8497]) -> [ABORT][36] ([i915#7982])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13418/bat-mtlp-8/igt@i915_selftest@live@requests.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/bat-mtlp-8/igt@i915_selftest@live@requests.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7609]: https://gitlab.freedesktop.org/drm/intel/issues/7609
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7940]: https://gitlab.freedesktop.org/drm/intel/issues/7940
  [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8384]: https://gitlab.freedesktop.org/drm/intel/issues/8384
  [i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
  [i915#8497]: https://gitlab.freedesktop.org/drm/intel/issues/8497
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7401 -> IGTPW_9455

  CI-20190529: 20190529
  CI_DRM_13418: e31a5b300385ef52e6db1cda820518cb2da089ca @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_9455: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/index.html
  IGT_7401: 0c66a6560eda687effa9088659577a520d913908 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

-igt@syncobj_sync_file@binary-import-export
-igt@syncobj_sync_file@invalid-fd-import
-igt@syncobj_sync_file@invalid-handle-export
-igt@syncobj_sync_file@invalid-handle-import
-igt@syncobj_sync_file@timeline-import-export
-igt@syncobj_sync_file@unsubmitted-export

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9455/index.html

[-- Attachment #2: Type: text/html, Size: 13434 bytes --]

Re: [igt-dev] [PATCH i-g-t 1/2] lib/xe: Add support to reset all GT's

[Kamil Konieczny]

Hi Janga,

On 2023-07-25 at 12:46:21 +0530, janga.rahul.kumar@intel.com wrote:
> From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> 
> Add library support to check support for GT reset and
> force reset all GT's.
> 
> Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> Cc: Kunal Joshi <kunal1.joshi@intel.com>
> Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> ---
>  lib/igt_gt.c      |  7 +++++++
>  lib/xe/xe_ioctl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_ioctl.h |  2 ++
>  3 files changed, 61 insertions(+)
> 
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index d4a825e66..2ce464ba6 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -45,6 +45,7 @@
>  #include "intel_chipset.h"
>  #include "igt_dummyload.h"
>  
> +#include "xe/xe_ioctl.h"

Add newline here.

>  /**
>   * SECTION:igt_gt
>   * @short_description: GT support library
> @@ -60,6 +61,9 @@ static int reset_query_once = -1;
>  
>  static bool has_gpu_reset(int fd)
>  {
> +	if (is_xe_device(fd))
> +		has_xe_gt_reset(fd);
> +
>  	if (reset_query_once < 0) {
>  		reset_query_once = gem_gpu_reset_type(fd);
>  
> @@ -395,6 +399,9 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
>   */
>  void igt_force_gpu_reset(int drm_fd)
>  {
> +	if (is_xe_device(drm_fd))
> +		xe_force_gt_reset_all(drm_fd);
> +

Move above code after igt_debug below.

>  	int dir, wedged;
>  
>  	igt_debug("Triggering GPU reset\n");
> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
> index 1f9240cd9..488aa218e 100644
> --- a/lib/xe/xe_ioctl.c
> +++ b/lib/xe/xe_ioctl.c
> @@ -41,6 +41,7 @@
>  #include "config.h"
>  #include "drmtest.h"
>  #include "igt_syncobj.h"
> +#include "igt_sysfs.h"
>  #include "ioctl_wrappers.h"
>  #include "xe_ioctl.h"
>  #include "xe_query.h"
> @@ -455,6 +456,57 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
>  	return ts.tv_sec * 1e9 + ts.tv_nsec;
>  }
>  
> +
> +/**
> + * has_xe_gt_reset:
> + * @fd: open xe drm file descriptor
> + *
> + * Check gt force reset syfs entry is available or not
> + *
> + * Returns: reset sysfs entry available
> + */
> +bool has_xe_gt_reset(int fd)
> +{
> +	char reset_sysfs_path[100];
> +	struct stat st;
> +	int gt;
> +	int reset_sysfs_fd = -1;
> +	int sysfs_fd = -1;

Add newline here.

> +	igt_assert_eq(fstat(fd, &st), 0);
> +
> +	sysfs_fd = igt_sysfs_open(fd);
> +	igt_assert(sysfs_fd != -1);
> +
> +	xe_for_each_gt(fd, gt) {
> +		sprintf(reset_sysfs_path, "/sys/kernel/debug/dri/%d/gt%d/force_reset", minor(st.st_rdev), gt);
> +		reset_sysfs_fd = openat(sysfs_fd, reset_sysfs_path, O_RDONLY);
> +
> +		if(reset_sysfs_fd == -1) {
> +			close(sysfs_fd);
> +			return 0;
> +		}
> +
> +		close(reset_sysfs_fd);
> +       }
> +
> +	close(sysfs_fd);
> +	return 1;
> +}
> +
> +/**
> + * xe_force_gt_reset_all:
> + *
> + * Forces reset of all the GT's.
> + *
> + */
> +void xe_force_gt_reset_all(int xe_fd)
> +{
> +	int gt;

Add newline.

Regards,
Kamil

> +	xe_for_each_gt(xe_fd, gt)
> +		xe_force_gt_reset(xe_fd, gt);
> +}
> +
> +
>  void xe_force_gt_reset(int fd, int gt)
>  {
>  	char reset_string[128];
> diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
> index 320e0f9f6..5a528b345 100644
> --- a/lib/xe/xe_ioctl.h
> +++ b/lib/xe/xe_ioctl.h
> @@ -87,6 +87,8 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
>  			       struct drm_xe_engine_class_instance *eci,
>  			       int64_t timeout);
>  void xe_force_gt_reset(int fd, int gt);
> +void xe_force_gt_reset_all(int fd);
> +bool has_xe_gt_reset(int fd);
>  void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
>  		   uint32_t property, uint32_t value);
>  
> -- 
> 2.25.1
> 

Re: [igt-dev] [PATCH i-g-t 2/2] lib/xe: Add hang library support

[Kamil Konieczny]

Hi Janga,

On 2023-07-25 at 12:46:22 +0530, janga.rahul.kumar@intel.com wrote:
> From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> 
> Add hang library support using spinners.
> 
> Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> Cc: Kunal Joshi <kunal1.joshi@intel.com>
> Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> ---
>  lib/igt_gt.c      | 21 +++++++++++++++++++++
------ ^^^^^^
Move this change into separate patch.

>  lib/xe/xe_ioctl.c | 20 ++++++++++++++++++++
>  lib/xe/xe_ioctl.h |  6 ++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index 2ce464ba6..6821179ca 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -170,6 +170,15 @@ static void context_set_ban(int fd, unsigned ctx, unsigned ban)
>  
>  igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
>  {
> +	if (is_xe_device(fd)) {
> +		if (!igt_check_boolean_env_var("IGT_HANG", true))
> +			igt_skip("hang injection disabled by user [IGT_HANG=0]\n");
> +
> +		igt_require(has_gpu_reset(fd));
> +
> +		return (struct igt_hang){ 0, ctx, 0, flags };
> +       }
> +

Move code after declarations of vars.

>  	struct drm_i915_gem_context_param param = {
>  		.ctx_id = ctx,
>  	};
> @@ -219,6 +228,9 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
>  
>  void igt_disallow_hang(int fd, igt_hang_t arg)
>  {
> +	if (is_xe_device(fd))
> +		return;
> +
>  	context_set_ban(fd, arg.ctx, arg.ban);
>  
>  	if ((arg.flags & HANG_ALLOW_CAPTURE) == 0) {
> @@ -290,6 +302,9 @@ static bool has_ctx_exec(int fd, unsigned ring, uint32_t ctx)
>  static igt_hang_t __igt_hang_ctx(int fd, uint64_t ahnd, uint32_t ctx, int ring,
>  				 unsigned flags)
>  {
> +	if (is_xe_device(fd))
> +		return xe_hang_engine(fd, ahnd, ctx, ring, flags);
> +

Move this after var declatarions.

>  	struct drm_i915_gem_context_param param;
>  	igt_spin_t *spin;
>  	unsigned ban;
> @@ -372,6 +387,12 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
>  	if (!arg.spin)
>  		return;
>  
> +	if (is_xe_device(fd)) {
> +		igt_spin_free(fd, arg.spin);
> +		xe_post_hang_ring(fd, arg);

It looks like spinner is not used here?

> +		return;
> +	}
> +
>  	gem_sync(fd, arg.spin->handle); /* Wait until it hangs */
>  	igt_spin_free(fd, arg.spin);
>  
> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
> index 488aa218e..5ec85239a 100644
> --- a/lib/xe/xe_ioctl.c
> +++ b/lib/xe/xe_ioctl.c
> @@ -520,6 +520,26 @@ void xe_force_gt_reset(int fd, int gt)
>  	system(reset_string);
>  }
>  
> +
> +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> +				unsigned flags)
> +{
> +	uint32_t vm;
> +	unsigned int engine;
> +	igt_spin_t *spin_t;

Add newline.

> +	vm = xe_vm_create(fd, 0, 0);
> +	engine = xe_engine_create_class(fd, vm, DRM_XE_ENGINE_CLASS_COPY);
----------------------------------------------- ^
Only one engine class here?

> +
> +	spin_t = igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm, .flags = IGT_SPIN_NO_PREEMPTION);

Add newline.

> +	return (igt_hang_t){ spin_t, engine, 0, flags };

Unused ring var?

> +}
> +
> +void xe_post_hang_ring(int fd, igt_hang_t arg)
> +{
> +	xe_engine_destroy(fd, arg.ctx);
> +	xe_vm_destroy(fd, arg.spin->vm);
> +}
> +
>  void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
>  		   uint32_t property, uint32_t value)
>  {
> diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
> index 5a528b345..0e9708194 100644
> --- a/lib/xe/xe_ioctl.h
> +++ b/lib/xe/xe_ioctl.h
> @@ -15,6 +15,9 @@
>  #include <stdint.h>
>  #include <xe_drm.h>
>  
> +#include "lib/igt_gt.h"
> +#include "xe_spin.h"
> +

Are you using new ioctl? If not maybe add xe_hang and xe_post_hang
to other library? Or create new one?

Regards,
Kamil

>  uint32_t xe_cs_prefetch_size(int fd);
>  uint32_t xe_vm_create(int fd, uint32_t flags, uint64_t ext);
>  int  __xe_vm_bind(int fd, uint32_t vm, uint32_t engine, uint32_t bo,
> @@ -89,6 +92,9 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
>  void xe_force_gt_reset(int fd, int gt);
>  void xe_force_gt_reset_all(int fd);
>  bool has_xe_gt_reset(int fd);
> +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> +				unsigned flags);
> +void xe_post_hang_ring(int fd, igt_hang_t arg);
>  void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
>  		   uint32_t property, uint32_t value);
>  
> -- 
> 2.25.1
> 

Re: [igt-dev] [PATCH i-g-t 1/2] lib/xe: Add support to reset all GT's

[Karas, Anna]

Hi all,
I have a few minor remarks to add.

On 25.07.2023 19:44, Kamil Konieczny wrote:
> Hi Janga,
> 
> On 2023-07-25 at 12:46:21 +0530, janga.rahul.kumar@intel.com wrote:
>> From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
>>
>> Add library support to check support for GT reset and
>> force reset all GT's.
>>
>> Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
>> Cc: Kunal Joshi <kunal1.joshi@intel.com>
>> Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
>> ---
>>   lib/igt_gt.c      |  7 +++++++
>>   lib/xe/xe_ioctl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/xe/xe_ioctl.h |  2 ++
>>   3 files changed, 61 insertions(+)
>>
>> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
>> index d4a825e66..2ce464ba6 100644
>> --- a/lib/igt_gt.c
>> +++ b/lib/igt_gt.c
>> @@ -45,6 +45,7 @@
>>   #include "intel_chipset.h"
>>   #include "igt_dummyload.h"
>>   
>> +#include "xe/xe_ioctl.h"
> 
> Add newline here.
> 
>>   /**
>>    * SECTION:igt_gt
>>    * @short_description: GT support library
>> @@ -60,6 +61,9 @@ static int reset_query_once = -1;
>>   
>>   static bool has_gpu_reset(int fd)
>>   {
>> +	if (is_xe_device(fd))
>> +		has_xe_gt_reset(fd);
>> +
>>   	if (reset_query_once < 0) {
>>   		reset_query_once = gem_gpu_reset_type(fd);
>>   
>> @@ -395,6 +399,9 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
>>    */
>>   void igt_force_gpu_reset(int drm_fd)
>>   {
>> +	if (is_xe_device(drm_fd))
>> +		xe_force_gt_reset_all(drm_fd);
>> +
> 
> Move above code after igt_debug below.
> 
>>   	int dir, wedged;
>>   
>>   	igt_debug("Triggering GPU reset\n");
>> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
>> index 1f9240cd9..488aa218e 100644
>> --- a/lib/xe/xe_ioctl.c
>> +++ b/lib/xe/xe_ioctl.c
>> @@ -41,6 +41,7 @@
>>   #include "config.h"
>>   #include "drmtest.h"
>>   #include "igt_syncobj.h"
>> +#include "igt_sysfs.h"
>>   #include "ioctl_wrappers.h"
>>   #include "xe_ioctl.h"
>>   #include "xe_query.h"
>> @@ -455,6 +456,57 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
>>   	return ts.tv_sec * 1e9 + ts.tv_nsec;
>>   }
>>   
>> +
>> +/**
>> + * has_xe_gt_reset:
>> + * @fd: open xe drm file descriptor
>> + *
>> + * Check gt force reset syfs entry is available or not
Typo: sysfs.

>> + *
>> + * Returns: reset sysfs entry available
>> + */
>> +bool has_xe_gt_reset(int fd)
>> +{
>> +	char reset_sysfs_path[100];
>> +	struct stat st;
>> +	int gt;
>> +	int reset_sysfs_fd = -1;
>> +	int sysfs_fd = -1;
> 
> Add newline here.
> 
>> +	igt_assert_eq(fstat(fd, &st), 0);
>> +
>> +	sysfs_fd = igt_sysfs_open(fd);
>> +	igt_assert(sysfs_fd != -1);
>> +
>> +	xe_for_each_gt(fd, gt) {
>> +		sprintf(reset_sysfs_path, "/sys/kernel/debug/dri/%d/gt%d/force_reset", minor(st.st_rdev), gt);
>> +		reset_sysfs_fd = openat(sysfs_fd, reset_sysfs_path, O_RDONLY);
>> +
>> +		if(reset_sysfs_fd == -1) {

Separate if and ( with space.

>> +			close(sysfs_fd);
>> +			return 0;
>> +		}
>> +
>> +		close(reset_sysfs_fd);
>> +       }
>> +
>> +	close(sysfs_fd);
>> +	return 1;
>> +}
>> +
>> +/**
>> + * xe_force_gt_reset_all:
>> + *
>> + * Forces reset of all the GT's.
>> + *
This line is not necessary.

>> + */
>> +void xe_force_gt_reset_all(int xe_fd)
>> +{
>> +	int gt;
> 
> Add newline.
> 
> Regards,
> Kamil
> 
>> +	xe_for_each_gt(xe_fd, gt)
>> +		xe_force_gt_reset(xe_fd, gt);
>> +}
>> +
>> +
>>   void xe_force_gt_reset(int fd, int gt)
>>   {
>>   	char reset_string[128];
>> diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
>> index 320e0f9f6..5a528b345 100644
>> --- a/lib/xe/xe_ioctl.h
>> +++ b/lib/xe/xe_ioctl.h
>> @@ -87,6 +87,8 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
>>   			       struct drm_xe_engine_class_instance *eci,
>>   			       int64_t timeout);
>>   void xe_force_gt_reset(int fd, int gt);
>> +void xe_force_gt_reset_all(int fd);
>> +bool has_xe_gt_reset(int fd);
>>   void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
>>   		   uint32_t property, uint32_t value);
>>   
>> -- 
>> 2.25.1
>>

Apart from my comments, please check your patch with checkpatch before 
sending v2.
Regards,
Anna

Re: [igt-dev] [PATCH i-g-t 2/2] lib/xe: Add hang library support

[Karas, Anna]

Hi all,

On 25.07.2023 20:04, Kamil Konieczny wrote:
> Hi Janga,
> 
> On 2023-07-25 at 12:46:22 +0530, janga.rahul.kumar@intel.com wrote:
>> From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
>>
>> Add hang library support using spinners.
>>
>> Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
>> Cc: Kunal Joshi <kunal1.joshi@intel.com>
>> Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
>> ---
>>   lib/igt_gt.c      | 21 +++++++++++++++++++++
> ------ ^^^^^^
> Move this change into separate patch.
> 
>>   lib/xe/xe_ioctl.c | 20 ++++++++++++++++++++
>>   lib/xe/xe_ioctl.h |  6 ++++++
>>   3 files changed, 47 insertions(+)
>>
>> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
>> index 2ce464ba6..6821179ca 100644
>> --- a/lib/igt_gt.c
>> +++ b/lib/igt_gt.c
>> @@ -170,6 +170,15 @@ static void context_set_ban(int fd, unsigned ctx, unsigned ban)
>>   
>>   igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
>>   {
>> +	if (is_xe_device(fd)) {
>> +		if (!igt_check_boolean_env_var("IGT_HANG", true))
>> +			igt_skip("hang injection disabled by user [IGT_HANG=0]\n");
>> +
>> +		igt_require(has_gpu_reset(fd));
>> +
>> +		return (struct igt_hang){ 0, ctx, 0, flags };
>> +       }
>> +
> 
> Move code after declarations of vars.
> 
>>   	struct drm_i915_gem_context_param param = {
>>   		.ctx_id = ctx,
>>   	};
>> @@ -219,6 +228,9 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
>>   
>>   void igt_disallow_hang(int fd, igt_hang_t arg)
>>   {
>> +	if (is_xe_device(fd))
>> +		return;
>> +
>>   	context_set_ban(fd, arg.ctx, arg.ban);
>>   
>>   	if ((arg.flags & HANG_ALLOW_CAPTURE) == 0) {
>> @@ -290,6 +302,9 @@ static bool has_ctx_exec(int fd, unsigned ring, uint32_t ctx)
>>   static igt_hang_t __igt_hang_ctx(int fd, uint64_t ahnd, uint32_t ctx, int ring,
>>   				 unsigned flags)
>>   {
>> +	if (is_xe_device(fd))
>> +		return xe_hang_engine(fd, ahnd, ctx, ring, flags);
>> +
> 
> Move this after var declatarions.
One more thing: with your change the doc of __igt_hang_ctx is no longer 
valid. Please delete i915 reference in the doc, as now fd can be both 
i915 and xe.
You can also fix already existing typo: @ctx: the contxt -> context

> 
>>   	struct drm_i915_gem_context_param param;
>>   	igt_spin_t *spin;
>>   	unsigned ban;
>> @@ -372,6 +387,12 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
>>   	if (!arg.spin)
>>   		return;
>>   
>> +	if (is_xe_device(fd)) {
>> +		igt_spin_free(fd, arg.spin);
>> +		xe_post_hang_ring(fd, arg);
> 
> It looks like spinner is not used here?
Please update the doc also for igt_post_hang_ring - @fd.

> 
>> +		return;
>> +	}
>> +
>>   	gem_sync(fd, arg.spin->handle); /* Wait until it hangs */
>>   	igt_spin_free(fd, arg.spin);
>>   
>> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
>> index 488aa218e..5ec85239a 100644
>> --- a/lib/xe/xe_ioctl.c
>> +++ b/lib/xe/xe_ioctl.c
>> @@ -520,6 +520,26 @@ void xe_force_gt_reset(int fd, int gt)
>>   	system(reset_string);
>>   }
>>   
>> +
>> +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
>> +				unsigned flags)
>> +{
>> +	uint32_t vm;
>> +	unsigned int engine;
>> +	igt_spin_t *spin_t;
> 
> Add newline.
> 
>> +	vm = xe_vm_create(fd, 0, 0);
>> +	engine = xe_engine_create_class(fd, vm, DRM_XE_ENGINE_CLASS_COPY);
> ----------------------------------------------- ^
> Only one engine class here?
> 
>> +
>> +	spin_t = igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm, .flags = IGT_SPIN_NO_PREEMPTION);
> 
> Add newline.
> 
>> +	return (igt_hang_t){ spin_t, engine, 0, flags };
> 
> Unused ring var?
Ctx also seems to be unused.

> 
>> +}
>> +
>> +void xe_post_hang_ring(int fd, igt_hang_t arg)
>> +{
>> +	xe_engine_destroy(fd, arg.ctx);
>> +	xe_vm_destroy(fd, arg.spin->vm);
>> +}
>> +
>>   void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
>>   		   uint32_t property, uint32_t value)
>>   {
>> diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
>> index 5a528b345..0e9708194 100644
>> --- a/lib/xe/xe_ioctl.h
>> +++ b/lib/xe/xe_ioctl.h
>> @@ -15,6 +15,9 @@
>>   #include <stdint.h>
>>   #include <xe_drm.h>
>>   
>> +#include "lib/igt_gt.h"
>> +#include "xe_spin.h"
>> +
> 
> Are you using new ioctl? If not maybe add xe_hang and xe_post_hang
> to other library? Or create new one?
> 
> Regards,
> Kamil
> 
>>   uint32_t xe_cs_prefetch_size(int fd);
>>   uint32_t xe_vm_create(int fd, uint32_t flags, uint64_t ext);
>>   int  __xe_vm_bind(int fd, uint32_t vm, uint32_t engine, uint32_t bo,
>> @@ -89,6 +92,9 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
>>   void xe_force_gt_reset(int fd, int gt);
>>   void xe_force_gt_reset_all(int fd);
>>   bool has_xe_gt_reset(int fd);
>> +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
>> +				unsigned flags);
>> +void xe_post_hang_ring(int fd, igt_hang_t arg);
>>   void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
>>   		   uint32_t property, uint32_t value);
>>   
>> -- 
>> 2.25.1
>>

As with previous patch, please remember to check it with checkpatch.pl 
before sending v2.

Ania

Re: [igt-dev] [PATCH i-g-t 0/2] Add Xe lib support for hang & GT reset

[Joshi, Kunal1]

Tested-by: Kunal Joshi <kunal1.joshi@intel.com>

Re: [igt-dev] [PATCH i-g-t 2/2] lib/xe: Add hang library support

[Kumar, Janga Rahul]

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Sent: 25 July 2023 23:34
> To: igt-dev@lists.freedesktop.org
> Cc: Kumar, Janga Rahul <janga.rahul.kumar@intel.com>; Gandi, Ramadevi
> <ramadevi.gandi@intel.com>; Ch, Sai Gowtham <sai.gowtham.ch@intel.com>;
> Joshi, Kunal1 <kunal1.joshi@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 2/2] lib/xe: Add hang library support
> 
> Hi Janga,
> 
> On 2023-07-25 at 12:46:22 +0530, janga.rahul.kumar@intel.com wrote:
> > From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> >
> > Add hang library support using spinners.
> >
> > Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > Cc: Kunal Joshi <kunal1.joshi@intel.com>
> > Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> > ---
> >  lib/igt_gt.c      | 21 +++++++++++++++++++++
> ------ ^^^^^^
> Move this change into separate patch.
> 
> >  lib/xe/xe_ioctl.c | 20 ++++++++++++++++++++  lib/xe/xe_ioctl.h |  6
> > ++++++
> >  3 files changed, 47 insertions(+)
> >
> > diff --git a/lib/igt_gt.c b/lib/igt_gt.c index 2ce464ba6..6821179ca
> > 100644
> > --- a/lib/igt_gt.c
> > +++ b/lib/igt_gt.c
> > @@ -170,6 +170,15 @@ static void context_set_ban(int fd, unsigned ctx,
> > unsigned ban)
> >
> >  igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)  {
> > +	if (is_xe_device(fd)) {
> > +		if (!igt_check_boolean_env_var("IGT_HANG", true))
> > +			igt_skip("hang injection disabled by user
> [IGT_HANG=0]\n");
> > +
> > +		igt_require(has_gpu_reset(fd));
> > +
> > +		return (struct igt_hang){ 0, ctx, 0, flags };
> > +       }
> > +
> 
> Move code after declarations of vars.
> 
> >  	struct drm_i915_gem_context_param param = {
> >  		.ctx_id = ctx,
> >  	};
> > @@ -219,6 +228,9 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx,
> > unsigned flags)
> >
> >  void igt_disallow_hang(int fd, igt_hang_t arg)  {
> > +	if (is_xe_device(fd))
> > +		return;
> > +
> >  	context_set_ban(fd, arg.ctx, arg.ban);
> >
> >  	if ((arg.flags & HANG_ALLOW_CAPTURE) == 0) { @@ -290,6 +302,9 @@
> > static bool has_ctx_exec(int fd, unsigned ring, uint32_t ctx)  static
> > igt_hang_t __igt_hang_ctx(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> >  				 unsigned flags)
> >  {
> > +	if (is_xe_device(fd))
> > +		return xe_hang_engine(fd, ahnd, ctx, ring, flags);
> > +
> 
> Move this after var declatarions.
> 
> >  	struct drm_i915_gem_context_param param;
> >  	igt_spin_t *spin;
> >  	unsigned ban;
> > @@ -372,6 +387,12 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
> >  	if (!arg.spin)
> >  		return;
> >
> > +	if (is_xe_device(fd)) {
> > +		igt_spin_free(fd, arg.spin);
> > +		xe_post_hang_ring(fd, arg);
> 
> It looks like spinner is not used here?
In post hang, we free the spinner.
> 
> > +		return;
> > +	}
> > +
> >  	gem_sync(fd, arg.spin->handle); /* Wait until it hangs */
> >  	igt_spin_free(fd, arg.spin);
> >
> > diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c index
> > 488aa218e..5ec85239a 100644
> > --- a/lib/xe/xe_ioctl.c
> > +++ b/lib/xe/xe_ioctl.c
> > @@ -520,6 +520,26 @@ void xe_force_gt_reset(int fd, int gt)
> >  	system(reset_string);
> >  }
> >
> > +
> > +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> > +				unsigned flags)
> > +{
> > +	uint32_t vm;
> > +	unsigned int engine;
> > +	igt_spin_t *spin_t;
> 
> Add newline.
> 
> > +	vm = xe_vm_create(fd, 0, 0);
> > +	engine = xe_engine_create_class(fd, vm,
> DRM_XE_ENGINE_CLASS_COPY);
> ----------------------------------------------- ^ Only one engine class here?
> 
> > +
> > +	spin_t = igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm,
> > +.flags = IGT_SPIN_NO_PREEMPTION);
> 
> Add newline.
> 
> > +	return (igt_hang_t){ spin_t, engine, 0, flags };
> 
> Unused ring var?
Updated it in v2. 
> 
> > +}
> > +
> > +void xe_post_hang_ring(int fd, igt_hang_t arg) {
> > +	xe_engine_destroy(fd, arg.ctx);
> > +	xe_vm_destroy(fd, arg.spin->vm);
> > +}
> > +
> >  void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
> >  		   uint32_t property, uint32_t value)  { diff --git
> > a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h index 5a528b345..0e9708194
> > 100644
> > --- a/lib/xe/xe_ioctl.h
> > +++ b/lib/xe/xe_ioctl.h
> > @@ -15,6 +15,9 @@
> >  #include <stdint.h>
> >  #include <xe_drm.h>
> >
> > +#include "lib/igt_gt.h"
> > +#include "xe_spin.h"
> > +
> 
> Are you using new ioctl? If not maybe add xe_hang and xe_post_hang to other
> library? Or create new one?
Introduced xe_gt library and moved hang and rest helpers of xe into it.
> 
> Regards,
> Kamil
> 
> >  uint32_t xe_cs_prefetch_size(int fd);  uint32_t xe_vm_create(int fd,
> > uint32_t flags, uint64_t ext);  int  __xe_vm_bind(int fd, uint32_t vm,
> > uint32_t engine, uint32_t bo, @@ -89,6 +92,9 @@ int64_t
> > xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,  void
> > xe_force_gt_reset(int fd, int gt);  void xe_force_gt_reset_all(int
> > fd);  bool has_xe_gt_reset(int fd);
> > +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> > +				unsigned flags);
> > +void xe_post_hang_ring(int fd, igt_hang_t arg);
> >  void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
> >  		   uint32_t property, uint32_t value);
> >
> > --
> > 2.25.1
> >

Re: [igt-dev] [PATCH i-g-t 2/2] lib/xe: Add hang library support

[Kumar, Janga Rahul]

> -----Original Message-----
> From: Karas, Anna <anna.karas@intel.com>
> Sent: 27 July 2023 02:15
> To: igt-dev@lists.freedesktop.org
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>; Kumar, Janga Rahul
> <janga.rahul.kumar@intel.com>; Gandi, Ramadevi
> <ramadevi.gandi@intel.com>; Ch, Sai Gowtham <sai.gowtham.ch@intel.com>;
> Joshi, Kunal1 <kunal1.joshi@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 2/2] lib/xe: Add hang library support
> 
> Hi all,
> 
> On 25.07.2023 20:04, Kamil Konieczny wrote:
> > Hi Janga,
> >
> > On 2023-07-25 at 12:46:22 +0530, janga.rahul.kumar@intel.com wrote:
> >> From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> >>
> >> Add hang library support using spinners.
> >>
> >> Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> >> Cc: Kunal Joshi <kunal1.joshi@intel.com>
> >> Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> >> ---
> >>   lib/igt_gt.c      | 21 +++++++++++++++++++++
> > ------ ^^^^^^
> > Move this change into separate patch.
> >
> >>   lib/xe/xe_ioctl.c | 20 ++++++++++++++++++++
> >>   lib/xe/xe_ioctl.h |  6 ++++++
> >>   3 files changed, 47 insertions(+)
> >>
> >> diff --git a/lib/igt_gt.c b/lib/igt_gt.c index 2ce464ba6..6821179ca
> >> 100644
> >> --- a/lib/igt_gt.c
> >> +++ b/lib/igt_gt.c
> >> @@ -170,6 +170,15 @@ static void context_set_ban(int fd, unsigned
> >> ctx, unsigned ban)
> >>
> >>   igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
> >>   {
> >> +	if (is_xe_device(fd)) {
> >> +		if (!igt_check_boolean_env_var("IGT_HANG", true))
> >> +			igt_skip("hang injection disabled by user
> [IGT_HANG=0]\n");
> >> +
> >> +		igt_require(has_gpu_reset(fd));
> >> +
> >> +		return (struct igt_hang){ 0, ctx, 0, flags };
> >> +       }
> >> +
> >
> > Move code after declarations of vars.
> >
> >>   	struct drm_i915_gem_context_param param = {
> >>   		.ctx_id = ctx,
> >>   	};
> >> @@ -219,6 +228,9 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx,
> >> unsigned flags)
> >>
> >>   void igt_disallow_hang(int fd, igt_hang_t arg)
> >>   {
> >> +	if (is_xe_device(fd))
> >> +		return;
> >> +
> >>   	context_set_ban(fd, arg.ctx, arg.ban);
> >>
> >>   	if ((arg.flags & HANG_ALLOW_CAPTURE) == 0) { @@ -290,6 +302,9 @@
> >> static bool has_ctx_exec(int fd, unsigned ring, uint32_t ctx)
> >>   static igt_hang_t __igt_hang_ctx(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> >>   				 unsigned flags)
> >>   {
> >> +	if (is_xe_device(fd))
> >> +		return xe_hang_engine(fd, ahnd, ctx, ring, flags);
> >> +
> >
> > Move this after var declatarions.
> One more thing: with your change the doc of __igt_hang_ctx is no longer valid.
> Please delete i915 reference in the doc, as now fd can be both
> i915 and xe.
> You can also fix already existing typo: @ctx: the contxt -> context
Done.
> 
> >
> >>   	struct drm_i915_gem_context_param param;
> >>   	igt_spin_t *spin;
> >>   	unsigned ban;
> >> @@ -372,6 +387,12 @@ void igt_post_hang_ring(int fd, igt_hang_t arg)
> >>   	if (!arg.spin)
> >>   		return;
> >>
> >> +	if (is_xe_device(fd)) {
> >> +		igt_spin_free(fd, arg.spin);
> >> +		xe_post_hang_ring(fd, arg);
> >
> > It looks like spinner is not used here?
> Please update the doc also for igt_post_hang_ring - @fd.
> 
> >
> >> +		return;
> >> +	}
> >> +
> >>   	gem_sync(fd, arg.spin->handle); /* Wait until it hangs */
> >>   	igt_spin_free(fd, arg.spin);
> >>
> >> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c index
> >> 488aa218e..5ec85239a 100644
> >> --- a/lib/xe/xe_ioctl.c
> >> +++ b/lib/xe/xe_ioctl.c
> >> @@ -520,6 +520,26 @@ void xe_force_gt_reset(int fd, int gt)
> >>   	system(reset_string);
> >>   }
> >>
> >> +
> >> +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> >> +				unsigned flags)
> >> +{
> >> +	uint32_t vm;
> >> +	unsigned int engine;
> >> +	igt_spin_t *spin_t;
> >
> > Add newline.
> >
> >> +	vm = xe_vm_create(fd, 0, 0);
> >> +	engine = xe_engine_create_class(fd, vm,
> DRM_XE_ENGINE_CLASS_COPY);
> > ----------------------------------------------- ^ Only one engine
> > class here?
Updated it in v2
> >
> >> +
> >> +	spin_t = igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm,
> >> +.flags = IGT_SPIN_NO_PREEMPTION);
> >
> > Add newline.
> >
> >> +	return (igt_hang_t){ spin_t, engine, 0, flags };
> >
> > Unused ring var?
> Ctx also seems to be unused.
In Xe, we need to create engine(exec_queue) hence not used here.

Thanks,
Rahul
> 
> >
> >> +}
> >> +
> >> +void xe_post_hang_ring(int fd, igt_hang_t arg) {
> >> +	xe_engine_destroy(fd, arg.ctx);
> >> +	xe_vm_destroy(fd, arg.spin->vm);
> >> +}
> >> +
> >>   void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
> >>   		   uint32_t property, uint32_t value)
> >>   {
> >> diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h index
> >> 5a528b345..0e9708194 100644
> >> --- a/lib/xe/xe_ioctl.h
> >> +++ b/lib/xe/xe_ioctl.h
> >> @@ -15,6 +15,9 @@
> >>   #include <stdint.h>
> >>   #include <xe_drm.h>
> >>
> >> +#include "lib/igt_gt.h"
> >> +#include "xe_spin.h"
> >> +
> >
> > Are you using new ioctl? If not maybe add xe_hang and xe_post_hang to
> > other library? Or create new one?
> >
> > Regards,
> > Kamil
> >
> >>   uint32_t xe_cs_prefetch_size(int fd);
> >>   uint32_t xe_vm_create(int fd, uint32_t flags, uint64_t ext);
> >>   int  __xe_vm_bind(int fd, uint32_t vm, uint32_t engine, uint32_t
> >> bo, @@ -89,6 +92,9 @@ int64_t xe_wait_ufence_abstime(int fd, uint64_t
> *addr, uint64_t value,
> >>   void xe_force_gt_reset(int fd, int gt);
> >>   void xe_force_gt_reset_all(int fd);
> >>   bool has_xe_gt_reset(int fd);
> >> +igt_hang_t xe_hang_engine(int fd, uint64_t ahnd, uint32_t ctx, int ring,
> >> +				unsigned flags);
> >> +void xe_post_hang_ring(int fd, igt_hang_t arg);
> >>   void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t size,
> >>   		   uint32_t property, uint32_t value);
> >>
> >> --
> >> 2.25.1
> >>
> 
> As with previous patch, please remember to check it with checkpatch.pl
> before sending v2.
> 
> Ania