Attachment 95822 Details for Bug 74100 – fix up semaphore hangcheck code

[patch] fix up semaphore hangcheck code

0001-drm-i915-fix-up-semaphore_waits_for.patch (text/plain), 3.48 KB, created by Daniel Vetter on 2014-03-14 23:46:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Vetter

Created: 2014-03-14 23:46:18 UTC

Size: 3.48 KB

patch

obsolete

>From ce1f9e40b4fc6b8d220215b7094044e9640860ae Mon Sep 17 00:00:00 2001
>From: Daniel Vetter <daniel.vetter@ffwll.ch>
>Date: Fri, 14 Mar 2014 23:12:21 +0100
>Subject: [PATCH] drm/i915: fix up semaphore_waits_for
>
>There's an entire pile of issues in here:
>
>- Use the main RING_HEAD register, not ACTHD. ACTHD points at the gtt
>  offset of the batch buffer when a batch is executed. Semaphores are
>  always emitted to the main ring, so we always want to look at that.
>
>- Mask the obtained HEAD pointer with the actual ring size, which is
>  much smaller. Together with the above issue this resulted us in
>  trying to dereference a pointer way outside of the ring mmio
>  mapping. The resulting invalid access in interrupt context
>  (hangcheck is executed from timers) lead to a full blown kernel
>  panic. The fbcon panic handler then tried to frob our driver harder,
>  resulting in a full machine hang at least on my snb here where I've
>  stumbled over this.
>
>- Handle ring wrapping correctly and be a bit more explicit about how
>  many dwords we're scanning. We probably should also scan more than
>  just 4 ...
>
>- Space out some of teh computations for readability.
>
>This prevents hard-hangs on my snb here. Verdict from QA is still
>pending, but the symptoms match.
>
>Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>Cc: Ben Widawsky <ben@bwidawsk.net>
>Cc: Chris Wilson <chris@chris-wilson.co.uk>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74100
>Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>---
> drivers/gpu/drm/i915/i915_irq.c | 38 ++++++++++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>index be2713f12e08..473372a6c97d 100644
>--- a/drivers/gpu/drm/i915/i915_irq.c
>+++ b/drivers/gpu/drm/i915/i915_irq.c
>@@ -2529,29 +2529,43 @@ static struct intel_ring_buffer *
> semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno)
> {
> 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
>-	u32 cmd, ipehr, acthd, acthd_min;
>+	u32 cmd, ipehr, head;
>+	int i;
> 
> 	ipehr = I915_READ(RING_IPEHR(ring->mmio_base));
> 	if ((ipehr & ~(0x3 << 16)) !=
> 	    (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE | MI_SEMAPHORE_REGISTER))
> 		return NULL;
> 
>-	/* ACTHD is likely pointing to the dword after the actual command,
>-	 * so scan backwards until we find the MBOX.
>+	/*
>+	 * HEAD is likely pointing to the dword after the actual command,
>+	 * so scan backwards until we find the MBOX. But limit it to just 3
>+	 * dwords. Note that we don't care about ACTHD here since that might
>+	 * point at at batch, and semaphores are always emitted into the
>+	 * ringbuffer itself.
> 	 */
>-	acthd = intel_ring_get_active_head(ring) & HEAD_ADDR;
>-	acthd_min = max((int)acthd - 3 * 4, 0);
>-	do {
>-		cmd = ioread32(ring->virtual_start + acthd);
>+	head = I915_READ_HEAD(ring) & HEAD_ADDR;
>+
>+	for (i = 4; i; --i) {
>+		/*
>+		 * Be paranoid and presume the hw has gone off into the wild -
>+		 * our ring is smaller than what the hardware (and hence
>+		 * HEAD_ADDR) allows. Also handles wrap-around.
>+		 */
>+		head &= ring->size - 1;
>+
>+		/* This here seems to blow up */
>+		cmd = ioread32(ring->virtual_start + head);
> 		if (cmd == ipehr)
> 			break;
> 
>-		acthd -= 4;
>-		if (acthd < acthd_min)
>-			return NULL;
>-	} while (1);
>+		head -= 4;
>+	}
>+
>+	if (!i)
>+		return NULL;
> 
>-	*seqno = ioread32(ring->virtual_start+acthd+4)+1;
>+	*seqno = ioread32(ring->virtual_start + head + 4) + 1;
> 	return &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3];
> }
> 
>-- 
>1.8.1.4
>

Actions: View | Splinter Review

Attachments on bug 74100: 92828 | 93589 | 95821 | 95822 | 95913

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.