[igt-dev] [PATCH i-g-t] i915/gem_ctx_persistence: Check precision of hostile cancellation

[igt-dev] [PATCH i-g-t] i915/gem_ctx_persistence: Check precision of hostile cancellation

[Chris Wilson]

Check that if we have to remove a hostile request from a non-persistent
context, we do so without harming any other concurrent users.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_ctx_persistence.c | 63 ++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
index 20007f5c4..cd174d263 100644
--- a/tests/i915/gem_ctx_persistence.c
+++ b/tests/i915/gem_ctx_persistence.c
@@ -613,6 +613,62 @@ static void test_process_mixed(int pfd, unsigned int engine)
 	gem_quiescent_gpu(pfd);
 }
 
+static void
+test_saturated_hostile(int i915, const struct intel_execution_engine2 *engine)
+{
+	const struct intel_execution_engine2 *other;
+	igt_spin_t *spin;
+	uint32_t ctx;
+	int fence = -1;
+
+	/*
+	 * Check that if we have to remove a hostile request from a
+	 * non-persistent context, we do so without harming any other
+	 * concurrent users.
+	 */
+
+	__for_each_physical_engine(i915, other) {
+		if (other->flags == engine->flags)
+			continue;
+
+		spin = igt_spin_new(i915,
+				   .engine = other->flags,
+				   .flags = (IGT_SPIN_NO_PREEMPTION |
+					     IGT_SPIN_FENCE_OUT));
+
+		if (fence < 0) {
+			fence = spin->out_fence;
+		} else {
+			int tmp;
+
+			tmp = sync_fence_merge(fence, spin->out_fence);
+			close(fence);
+			close(spin->out_fence);
+
+			fence = tmp;
+		}
+		spin->out_fence = -1;
+	}
+
+	ctx = gem_context_clone_with_engines(i915, 0);
+	gem_context_set_persistence(i915, ctx, false);
+	spin = igt_spin_new(i915, ctx,
+			    .engine = engine->flags,
+			    .flags = (IGT_SPIN_NO_PREEMPTION |
+				      IGT_SPIN_POLL_RUN |
+				      IGT_SPIN_FENCE_OUT));
+	igt_spin_busywait_until_started(spin);
+	gem_context_destroy(i915, ctx);
+
+	igt_assert_eq(sync_fence_wait(spin->out_fence, reset_timeout_ms), 0);
+	igt_assert_eq(sync_fence_status(spin->out_fence), -EIO);
+
+	/* All other spinners should be left unharmed */
+	gem_quiescent_gpu(i915);
+	igt_assert_eq(sync_fence_wait(fence, reset_timeout_ms), 0);
+	igt_assert_eq(sync_fence_status(fence), 1);
+}
+
 static void test_processes(int i915)
 {
 	struct {
@@ -1041,6 +1097,13 @@ igt_main
 			}
 		}
 
+		igt_subtest_with_dynamic_f("saturated-hostile") {
+			__for_each_physical_engine(i915, e) {
+				igt_dynamic_f("%s", e->name)
+					test_saturated_hostile(i915, e);
+			}
+		}
+
 		igt_subtest("smoketest")
 			smoketest(i915);
 	}
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ GitLab.Pipeline: failure for i915/gem_ctx_persistence: Check precision of hostile cancellation

[Patchwork]

== Series Details ==

Series: i915/gem_ctx_persistence: Check precision of hostile cancellation
URL   : https://patchwork.freedesktop.org/series/73874/
State : failure

== Summary ==

ERROR! This series introduces new undocumented tests:

gem_ctx_persistence@saturated-hostile

Can you document them as per the requirement in the [CONTRIBUTING.md]?

[Documentation] has more details on how to do this.

Here are few examples:
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/0316695d03aa46108296b27f3982ec93200c7a6e
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/443cc658e1e6b492ee17bf4f4d891029eb7a205d

Thanks in advance!

[CONTRIBUTING.md]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/CONTRIBUTING.md#L19
[Documentation]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe

Other than that, pipeline status: SUCCESS.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/112211 for the overview.

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/112211
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for i915/gem_ctx_persistence: Check precision of hostile cancellation

[Patchwork]

== Series Details ==

Series: i915/gem_ctx_persistence: Check precision of hostile cancellation
URL   : https://patchwork.freedesktop.org/series/73874/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8000 -> IGTPW_4219
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_4219 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_4219, 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_4219/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - fi-hsw-peppy:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8000/fi-hsw-peppy/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/fi-hsw-peppy/igt@i915_module_load@reload.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-icl-u2:          [PASS][3] -> [FAIL][4] ([fdo#109635] / [i915#217])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8000/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-bsw-kefka:       [DMESG-FAIL][5] -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8000/fi-bsw-kefka/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/fi-bsw-kefka/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cml-s:           [DMESG-FAIL][7] ([i915#877]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8000/fi-cml-s/igt@i915_selftest@live_gem_contexts.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/fi-cml-s/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][9] ([fdo#111407]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8000/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][11] ([i915#44]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8000/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-tgl-y:           [FAIL][13] ([CI#94]) -> [INCOMPLETE][14] ([CI#94] / [i915#460])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8000/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html

  
  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#109635]: https://bugs.freedesktop.org/show_bug.cgi?id=109635
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44
  [i915#460]: https://gitlab.freedesktop.org/drm/intel/issues/460
  [i915#877]: https://gitlab.freedesktop.org/drm/intel/issues/877


Participating hosts (48 -> 45)
------------------------------

  Additional (2): fi-byt-n2820 fi-cfl-8109u 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5463 -> IGTPW_4219

  CI-20190529: 20190529
  CI_DRM_8000: e231691d89abd1b3fff01e75f142e435b44b522f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4219: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/index.html
  IGT_5463: d519c80219ebe558cd2fa378f26f9d73f9e35310 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_ctx_persistence@saturated-hostile

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4219/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_persistence: Check precision of hostile cancellation

[Tvrtko Ursulin]

On 24/02/2020 21:56, Chris Wilson wrote:
> Check that if we have to remove a hostile request from a non-persistent
> context, we do so without harming any other concurrent users.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/i915/gem_ctx_persistence.c | 63 ++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
> index 20007f5c4..cd174d263 100644
> --- a/tests/i915/gem_ctx_persistence.c
> +++ b/tests/i915/gem_ctx_persistence.c
> @@ -613,6 +613,62 @@ static void test_process_mixed(int pfd, unsigned int engine)
>   	gem_quiescent_gpu(pfd);
>   }
>   
> +static void
> +test_saturated_hostile(int i915, const struct intel_execution_engine2 *engine)
> +{
> +	const struct intel_execution_engine2 *other;
> +	igt_spin_t *spin;
> +	uint32_t ctx;
> +	int fence = -1;
> +
> +	/*
> +	 * Check that if we have to remove a hostile request from a
> +	 * non-persistent context, we do so without harming any other
> +	 * concurrent users.
> +	 */
> +
> +	__for_each_physical_engine(i915, other) {
> +		if (other->flags == engine->flags)
> +			continue;
> +
> +		spin = igt_spin_new(i915,
> +				   .engine = other->flags,
> +				   .flags = (IGT_SPIN_NO_PREEMPTION |
> +					     IGT_SPIN_FENCE_OUT));
> +
> +		if (fence < 0) {
> +			fence = spin->out_fence;
> +		} else {
> +			int tmp;
> +
> +			tmp = sync_fence_merge(fence, spin->out_fence);
> +			close(fence);
> +			close(spin->out_fence);
> +
> +			fence = tmp;
> +		}
> +		spin->out_fence = -1;
> +	}
> +
> +	ctx = gem_context_clone_with_engines(i915, 0);
> +	gem_context_set_persistence(i915, ctx, false);
> +	spin = igt_spin_new(i915, ctx,
> +			    .engine = engine->flags,
> +			    .flags = (IGT_SPIN_NO_PREEMPTION |
> +				      IGT_SPIN_POLL_RUN |
> +				      IGT_SPIN_FENCE_OUT));
> +	igt_spin_busywait_until_started(spin);
> +	gem_context_destroy(i915, ctx);
> +
> +	igt_assert_eq(sync_fence_wait(spin->out_fence, reset_timeout_ms), 0);
> +	igt_assert_eq(sync_fence_status(spin->out_fence), -EIO);
> +
> +	/* All other spinners should be left unharmed */
> +	gem_quiescent_gpu(i915);
> +	igt_assert_eq(sync_fence_wait(fence, reset_timeout_ms), 0);
> +	igt_assert_eq(sync_fence_status(fence), 1);

I don't quite get this test. Why would other spinners be unharmed? They 
are non-preemptible as well. And non-persistent spinner is alone on the 
engine. So what aspect you wanted to test?

Regards,

Tvrtko

> +}
> +
>   static void test_processes(int i915)
>   {
>   	struct {
> @@ -1041,6 +1097,13 @@ igt_main
>   			}
>   		}
>   
> +		igt_subtest_with_dynamic_f("saturated-hostile") {
> +			__for_each_physical_engine(i915, e) {
> +				igt_dynamic_f("%s", e->name)
> +					test_saturated_hostile(i915, e);
> +			}
> +		}
> +
>   		igt_subtest("smoketest")
>   			smoketest(i915);
>   	}
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_persistence: Check precision of hostile cancellation

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-02-25 18:08:14)
> 
> On 24/02/2020 21:56, Chris Wilson wrote:
> > Check that if we have to remove a hostile request from a non-persistent
> > context, we do so without harming any other concurrent users.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > +     /* All other spinners should be left unharmed */
> > +     gem_quiescent_gpu(i915);
> > +     igt_assert_eq(sync_fence_wait(fence, reset_timeout_ms), 0);
> > +     igt_assert_eq(sync_fence_status(fence), 1);
> 
> I don't quite get this test. Why would other spinners be unharmed? They 
> are non-preemptible as well. And non-persistent spinner is alone on the 
> engine. So what aspect you wanted to test?

Per-engine reset. Termination of the non-persistent context should be
clean and precise, we don't allow creation of non-persistent contexts
unless we have that level of surgical precision. Otherwise it becomes a
new attack vector.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_persistence: Check precision of hostile cancellation

[Tvrtko Ursulin]

On 25/02/2020 18:11, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-02-25 18:08:14)
>>
>> On 24/02/2020 21:56, Chris Wilson wrote:
>>> Check that if we have to remove a hostile request from a non-persistent
>>> context, we do so without harming any other concurrent users.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> +     /* All other spinners should be left unharmed */
>>> +     gem_quiescent_gpu(i915);
>>> +     igt_assert_eq(sync_fence_wait(fence, reset_timeout_ms), 0);
>>> +     igt_assert_eq(sync_fence_status(fence), 1);
>>
>> I don't quite get this test. Why would other spinners be unharmed? They
>> are non-preemptible as well. And non-persistent spinner is alone on the
>> engine. So what aspect you wanted to test?
> 
> Per-engine reset. Termination of the non-persistent context should be
> clean and precise, we don't allow creation of non-persistent contexts
> unless we have that level of surgical precision. Otherwise it becomes a
> new attack vector.

If it is just engine reset then it does what it says on the tin.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_persistence: Check precision of hostile cancellation

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-02-25 18:18:12)
> 
> On 25/02/2020 18:11, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2020-02-25 18:08:14)
> >>
> >> On 24/02/2020 21:56, Chris Wilson wrote:
> >>> Check that if we have to remove a hostile request from a non-persistent
> >>> context, we do so without harming any other concurrent users.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> ---
> >>> +     /* All other spinners should be left unharmed */
> >>> +     gem_quiescent_gpu(i915);
> >>> +     igt_assert_eq(sync_fence_wait(fence, reset_timeout_ms), 0);
> >>> +     igt_assert_eq(sync_fence_status(fence), 1);
> >>
> >> I don't quite get this test. Why would other spinners be unharmed? They
> >> are non-preemptible as well. And non-persistent spinner is alone on the
> >> engine. So what aspect you wanted to test?
> > 
> > Per-engine reset. Termination of the non-persistent context should be
> > clean and precise, we don't allow creation of non-persistent contexts
> > unless we have that level of surgical precision. Otherwise it becomes a
> > new attack vector.
> 
> If it is just engine reset then it does what it says on the tin.

I shall update the description to clarify that then :)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev