diff options
| author |   Chris Wilson <chris@chris-wilson.co.uk> | 2019-11-25 09:43:18 +0000 |
|---|---|---|
| committer |   Joonas Lahtinen <joonas.lahtinen@linux.intel.com> | 2019-11-25 16:36:40 +0200 |
| commit | 97f9af78f38d43cd058cfd40220647be67aca9f7 (patch) | |
| tree | da29df02c0975f90d9aac52d72d4ef525db3d1e1 /drivers/gpu/drm/i915/gt/intel_lrc.c | |
| parent | drm/i915/gt: Unlock engine-pm after queuing the kernel context switch (diff) | |
| download | linux-dev-97f9af78f38d43cd058cfd40220647be67aca9f7.tar.xz linux-dev-97f9af78f38d43cd058cfd40220647be67aca9f7.zip | |
drm/i915/gt: Mark the execlists->active as the primary volatile access
Since we want to do a lockless read of the current active request, and
that request is written to by process_csb also without serialisation, we
need to instruct gcc to take care in reading the pointer itself.
Otherwise, we have observed execlists_active() to report 0x40.
[ 2400.760381] igt/para-4098 1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
[ 2400.760826] igt/para-4098 1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
[ 2400.761271] igt/para-4098 1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
[ 2400.761726] igt/para-4097 0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940
which is impossible!
The answer is that as we keep the existing execlists->active pointing
into the array as we copy over that array, the unserialised read may see
a partial pointer value.
Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191125094318.1630806-1-chris@chris-wilson.co.uk
(cherry picked from commit 331bf90591573dfe6c8e892239713ef9702f1396)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Diffstat (limited to '')
| -rw-r--r-- | drivers/gpu/drm/i915/gt/intel_lrc.c | 27 |
1 files changed, 17 insertions, 10 deletions
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 37fe72aa8e27..a692e6c9ea75 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1943,6 +1943,9 @@ cancel_port_requests(struct intel_engine_execlists * const execlists) execlists_schedule_out(rq); memset(execlists->pending, 0, sizeof(execlists->pending)); + /* Mark the end of active before we overwrite *active */ + WRITE_ONCE(execlists->active, execlists->pending); + for (port = execlists->active; (rq = *port); port++) execlists_schedule_out(rq); execlists->active = @@ -2099,23 +2102,27 @@ static void process_csb(struct intel_engine_cs *engine) else promote = gen8_csb_parse(execlists, buf + 2 * head); if (promote) { + struct i915_request * const *old = execlists->active; + + /* Point active to the new ELSP; prevent overwriting */ + WRITE_ONCE(execlists->active, execlists->pending); + set_timeslice(engine); + if (!inject_preempt_hang(execlists)) ring_set_paused(engine, 0); /* cancel old inflight, prepare for switch */ - trace_ports(execlists, "preempted", execlists->active); - while (*execlists->active) - execlists_schedule_out(*execlists->active++); + trace_ports(execlists, "preempted", old); + while (*old) + execlists_schedule_out(*old++); /* switch pending to inflight */ GEM_BUG_ON(!assert_pending_valid(execlists, "promote")); - execlists->active = - memcpy(execlists->inflight, - execlists->pending, - execlists_num_ports(execlists) * - sizeof(*execlists->pending)); - - set_timeslice(engine); + WRITE_ONCE(execlists->active, + memcpy(execlists->inflight, + execlists->pending, + execlists_num_ports(execlists) * + sizeof(*execlists->pending))); WRITE_ONCE(execlists->pending[0], NULL); } else { |