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:df40306902
("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 commit331bf90591
) Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
This commit is contained in:
parent
bf201f5eda
commit
97f9af78f3
|
@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
|
|||
static inline struct i915_request *
|
||||
execlists_active(const struct intel_engine_execlists *execlists)
|
||||
{
|
||||
GEM_BUG_ON(execlists->active - execlists->inflight >
|
||||
execlists_num_ports(execlists));
|
||||
return READ_ONCE(*execlists->active);
|
||||
return *READ_ONCE(execlists->active);
|
||||
}
|
||||
|
||||
static inline void
|
||||
|
|
|
@ -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 {
|
||||
|
|
Loading…
Reference in New Issue