Re: [PATCH 11/27] drm/i915/selftests: Fix memory corruption in live_lrc_isolation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/18/2021 11:16 PM, Matthew Brost wrote:
GuC submission has exposed an existing memory corruption in
live_lrc_isolation. We believe that some writes to the watchdog offsets
in the LRC (0x178 & 0x17c) can result in trashing of portions of the
address space. With GuC submission there are additional objects which
can move the context redzone into the space that is trashed. To
workaround this avoid poisoning the watchdog.

This is kind of a worrying explanation, as it implies an HW issue. AFAICS we no longer increase the context size with GuC submission, so the redzone should be in the same place relative to the base address of the context; although it is true that we have more objects in memory due to support the GuC, hitting the redzone consistently feels too much like a coincidence. When we write the watchdog regs there is a risk we're triggering a watchdog interrupt, which will cause the GuC to handle that; on a media reset, the GuC overwrites the context with the golden context in the ADS, are we sure that's not what is causing this problem?
Looking in the ADS we set the context memcpy size to:

real_size = intel_engine_context_size(gt, engine_class);

but then we only initialize real_size - SKIP_SIZE(gt->i915), which IMO could be the real cause of the bug as the GuC memcpy starts at SKIP_SIZE().

Daniele


v2:
  (Daniel Vetter)
   - Add VLK ref in code to workaround

Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/selftest_lrc.c | 29 +++++++++++++++++++++++++-
  1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index b0977a3b699b..cdc6ae48a1e1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1074,6 +1074,32 @@ record_registers(struct intel_context *ce,
  	goto err_after;
  }
+static u32 safe_offset(u32 offset, u32 reg)
+{
+	/* XXX skip testing of watchdog - VLK-22772 */
+	if (offset == 0x178 || offset == 0x17c)
+		reg = 0;
+
+	return reg;
+}
+
+static int get_offset_mask(struct intel_engine_cs *engine)
+{
+	if (GRAPHICS_VER(engine->i915) < 12)
+		return 0xfff;
+
+	switch (engine->class) {
+	default:
+	case RENDER_CLASS:
+		return 0x07ff;
+	case COPY_ENGINE_CLASS:
+		return 0x0fff;
+	case VIDEO_DECODE_CLASS:
+	case VIDEO_ENHANCEMENT_CLASS:
+		return 0x3fff;
+	}
+}
+
  static struct i915_vma *load_context(struct intel_context *ce, u32 poison)
  {
  	struct i915_vma *batch;
@@ -1117,7 +1143,8 @@ static struct i915_vma *load_context(struct intel_context *ce, u32 poison)
  		len = (len + 1) / 2;
  		*cs++ = MI_LOAD_REGISTER_IMM(len);
  		while (len--) {
-			*cs++ = hw[dw];
+			*cs++ = safe_offset(hw[dw] & get_offset_mask(ce->engine),
+					    hw[dw]);
  			*cs++ = poison;
  			dw += 2;
  		}




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux