Commit eca15360 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Don't dereference request if it may have been retired when printing

This has caught me out on countless occasions, when we retrieve a pointer
from the submission/execlists backend, it does not carry a reference to
the context or ring. Those are only pinned while the request is active,
so if we see the request is already completed, it may be in the process
of being retired and those pointers defunct.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110938
Fixes: 3a068721 ("drm/i915: Show ring->start for the ELSP context/request queue")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190618161951.28820-2-chris@chris-wilson.co.uk
parent 1422768f
...@@ -1311,12 +1311,13 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len) ...@@ -1311,12 +1311,13 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
} }
} }
static void intel_engine_print_registers(const struct intel_engine_cs *engine, static void intel_engine_print_registers(struct intel_engine_cs *engine,
struct drm_printer *m) struct drm_printer *m)
{ {
struct drm_i915_private *dev_priv = engine->i915; struct drm_i915_private *dev_priv = engine->i915;
const struct intel_engine_execlists * const execlists = const struct intel_engine_execlists * const execlists =
&engine->execlists; &engine->execlists;
unsigned long flags;
u64 addr; u64 addr;
if (engine->id == RCS0 && IS_GEN_RANGE(dev_priv, 4, 7)) if (engine->id == RCS0 && IS_GEN_RANGE(dev_priv, 4, 7))
...@@ -1397,15 +1398,16 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine, ...@@ -1397,15 +1398,16 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
idx, hws[idx * 2], hws[idx * 2 + 1]); idx, hws[idx * 2], hws[idx * 2 + 1]);
} }
rcu_read_lock(); spin_lock_irqsave(&engine->active.lock, flags);
for (idx = 0; idx < execlists_num_ports(execlists); idx++) { for (idx = 0; idx < execlists_num_ports(execlists); idx++) {
struct i915_request *rq; struct i915_request *rq;
unsigned int count; unsigned int count;
char hdr[80];
rq = port_unpack(&execlists->port[idx], &count); rq = port_unpack(&execlists->port[idx], &count);
if (rq) { if (!rq) {
char hdr[80]; drm_printf(m, "\t\tELSP[%d] idle\n", idx);
} else if (!i915_request_signaled(rq)) {
snprintf(hdr, sizeof(hdr), snprintf(hdr, sizeof(hdr),
"\t\tELSP[%d] count=%d, ring:{start:%08x, hwsp:%08x, seqno:%08x}, rq: ", "\t\tELSP[%d] count=%d, ring:{start:%08x, hwsp:%08x, seqno:%08x}, rq: ",
idx, count, idx, count,
...@@ -1414,11 +1416,11 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine, ...@@ -1414,11 +1416,11 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
hwsp_seqno(rq)); hwsp_seqno(rq));
print_request(m, rq, hdr); print_request(m, rq, hdr);
} else { } else {
drm_printf(m, "\t\tELSP[%d] idle\n", idx); print_request(m, rq, "\t\tELSP[%d] rq: ");
} }
} }
drm_printf(m, "\t\tHW active? 0x%x\n", execlists->active); drm_printf(m, "\t\tHW active? 0x%x\n", execlists->active);
rcu_read_unlock(); spin_unlock_irqrestore(&engine->active.lock, flags);
} else if (INTEL_GEN(dev_priv) > 6) { } else if (INTEL_GEN(dev_priv) > 6) {
drm_printf(m, "\tPP_DIR_BASE: 0x%08x\n", drm_printf(m, "\tPP_DIR_BASE: 0x%08x\n",
ENGINE_READ(engine, RING_PP_DIR_BASE)); ENGINE_READ(engine, RING_PP_DIR_BASE));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment