From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/selftests: Sabotague the RING_HEAD
Date: Tue, 11 Feb 2020 19:55:02 +0200 [thread overview]
Message-ID: <871rr19fgp.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20200211150951.1026811-1-chris@chris-wilson.co.uk>
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Apply vast quantities of poison and not tell anyone to see if we fall
> for the trap of using a stale RING_HEAD.
>
> References: 42827350f75c ("drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gt/selftest_lrc.c | 75 ++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 82fa0712808e..1421dc2db7cf 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -300,6 +300,80 @@ static int live_unlite_preempt(void *arg)
> return live_unlite_restore(arg, I915_USER_PRIORITY(I915_PRIORITY_MAX));
> }
>
> +static int live_pin_rewind(void *arg)
> +{
> + struct intel_gt *gt = arg;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + int err = 0;
> +
> + /*
> + * We have to be careful not to trust intel_ring too much, for example
> + * ring->head is updated upon retire which is out of sync with pinning
> + * the context. Thus we cannot use ring->head to set CTX_RING_HEAD,
> + * or else we risk writing an older, stale value.
> + *
> + * To simulate this, let's apply a bit of deliberate sabotague.
> + */
> +
> + for_each_engine(engine, gt, id) {
> + struct intel_context *ce;
> + struct i915_request *rq;
> + struct igt_live_test t;
> +
> + if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
> + err = -EIO;
> + break;
> + }
> +
> + ce = intel_context_create(engine);
> + if (IS_ERR(ce)) {
> + err = PTR_ERR(ce);
> + break;
> + }
> +
> + err = intel_context_pin(ce);
> + if (err) {
> + intel_context_put(ce);
> + break;
> + }
> +
> + /* Keep the context awake while we play games */
> + err = i915_active_acquire(&ce->active);
> + if (err) {
> + intel_context_unpin(ce);
> + intel_context_put(ce);
> + break;
> + }
> +
> + /* Poison the head of the ring */
> + memset32(ce->ring->vaddr, STACK_MAGIC, 1024);
I got affidavit that this will change to poisoning the whole
ring according to size.
> + ce->ring->emit = 1024;
> + ce->ring->tail = 1024;
> +
> + intel_context_unpin(ce);
> +
> + /* Submit a simple nop request */
> + GEM_BUG_ON(intel_context_is_pinned(ce));
> + rq = intel_context_create_request(ce);
The request get the head from ring->emit at this point.
Perhaps there are more nyances but this will atleast test
the boundaries of request emission.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> + i915_active_release(&ce->active); /* e.g. async retire */
> + intel_context_put(ce);
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + break;
> + }
> + i915_request_add(rq);
> +
> + /* Expect not to hang! */
> + if (igt_live_test_end(&t)) {
> + err = -EIO;
> + break;
> + }
> + }
> +
> + return err;
> +}
> +
> static int live_hold_reset(void *arg)
> {
> struct intel_gt *gt = arg;
> @@ -3616,6 +3690,7 @@ int intel_execlists_live_selftests(struct drm_i915_private *i915)
> SUBTEST(live_sanitycheck),
> SUBTEST(live_unlite_switch),
> SUBTEST(live_unlite_preempt),
> + SUBTEST(live_pin_rewind),
> SUBTEST(live_hold_reset),
> SUBTEST(live_error_interrupt),
> SUBTEST(live_timeslice_preempt),
> --
> 2.25.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-02-11 17:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-11 15:09 [Intel-gfx] [PATCH] drm/i915/selftests: Sabotague the RING_HEAD Chris Wilson
2020-02-11 17:55 ` Mika Kuoppala [this message]
2020-02-13 19:04 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871rr19fgp.fsf@gaia.fi.intel.com \
--to=mika.kuoppala@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).