On 12/20/21 13:00, Tejas Upadhyay wrote:
During repeated wedged-unwedged, it is
found that i915_request_retire zaps the old
request with 0x6b6b6b6b.
On unwedged, we write a new request at RING_TAIL,
expecting to start executuing from that position,
but execution resumes from RING_HEAD (preserved
from an earlier wakeup before wedging) and
consumes the 0x6b.
Resetting kernel/user context setup enables
RING_HEAD to use RING_TAIL for submitting new
requests which resolves issue. Normally this reset
is applied when unpinning a user context, or for
kernel_contexts upon waking up the device. But fast
wedged-unwedged sequence will keep the device awake,
preserving the RING_HEAD from before.
Testcase: igt@gem_eio@unwedge-stress
Note : Current user impact is assessed to be low, as
this only affects intel_gt_unset_wedged which is
currently only used during testing and upon suspend
resume (where the device was already flushed and will
reset the kernel_contexts on waking up). In the
future though, this will present an issue for PCI
error recovery.
Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 ++++++++
drivers/gpu/drm/i915/gt/intel_reset.c | 3 +++
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 352254e001b4..7e1c561bce69 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1455,9 +1455,17 @@ void intel_engines_reset_default_submission(struct intel_gt *gt)
enum intel_engine_id id;
for_each_engine(engine, gt, id) {
+ struct intel_context *ce = engine->kernel_context;
+
if (engine->sanitize)
engine->sanitize(engine);
+ /* Reset RING_HEAD so we don't consume the old
+ * poisoned request on unwedging
+ */
+ if (ce)
+ ce->ops->reset(ce);
+
Would it make sense to instead stop excluding the kernel context in
intel_engine_reset_pinned_contexts, which is called from
engine->sanitize(). That would lead to a double kernel context reset in
some situations, though, but I figure the above would as well if the
engine was parked before unwedging?
How is the PCI error recovery affected? Do we have have a plan to
address that?
Thanks,
/Thomas
engine->set_default_submission(engine);
}
}
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 7be0002d9d70..1c26e936e699 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -961,6 +961,9 @@ static bool __intel_gt_unset_wedged(struct intel_gt *gt)
}
spin_unlock(&timelines->lock);
+ /* Ensure that all non-kernel contexts are unpinned as well */
+ intel_gt_retire_requests(gt);
+
/* We must reset pending GPU events before restoring our submission */
ok = !HAS_EXECLISTS(gt->i915); /* XXX better agnosticism desired */
if (!INTEL_INFO(gt->i915)->gpu_reset_clobbers_display)
