From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:52747 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751497AbcDRJun convert rfc822-to-8bit (ORCPT ); Mon, 18 Apr 2016 05:50:43 -0400 From: Jani Nikula To: Daniel Vetter , Chris Wilson Cc: stable@vger.kernel.org, intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [CI-ping 11/15] drm/i915: Prevent machine death on Ivybridge context switching In-Reply-To: <20160413093316.GP2510@phenom.ffwll.local> References: <1460491389-8602-1-git-send-email-chris@chris-wilson.co.uk> <1460491389-8602-11-git-send-email-chris@chris-wilson.co.uk> <20160413093316.GP2510@phenom.ffwll.local> Date: Mon, 18 Apr 2016 12:50:35 +0300 Message-ID: <87inzfp7o4.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: stable-owner@vger.kernel.org List-ID: On Wed, 13 Apr 2016, Daniel Vetter wrote: > On Tue, Apr 12, 2016 at 09:03:05PM +0100, Chris Wilson wrote: >> Two concurrent writes into the same register cacheline has the chance of >> killing the machine on Ivybridge and other gen7. This includes LRI >> emitted from the command parser. The MI_SET_CONTEXT itself serves as >> serialising barrier and prevents the pair of register writes in the first >> packet from triggering the fault. However, if a second switch-context >> immediately occurs then we may have two adjacent blocks of LRI to the >> same registers which may then trigger the hang. To counteract this we >> need to insert a delay after the second register write using SRM. >> >> This is easiest to reproduce with something like >> igt/gem_ctx_switch/interruptible that triggers back-to-back context >> switches (with no operations in between them in the command stream, >> which requires the execbuf operation to be interrupted after the >> MI_SET_CONTEXT) but can be observed sporadically elsewhere when running >> interruptible igt. No reports from the wild though, so it must be of low >> enough frequency that no one has correlated the random machine freezes >> with i915.ko >> >> The issue was introduced with >> commit 2c550183476dfa25641309ae9a28d30feed14379 [v3.19] >> Author: Chris Wilson >> Date: Tue Dec 16 10:02:27 2014 +0000 >> >> drm/i915: Disable PSMI sleep messages on all rings around context switches >> >> Testcase: igt/gem_ctx_switch/render-interruptible #ivb >> Signed-off-by: Chris Wilson >> Cc: Daniel Vetter >> Cc: Ville Syrjälä >> Cc: stable@vger.kernel.org > > Reviewed-by: Daniel Vetter FYI, this (*) does not cherry-pick cleanly to drm-intel-fixes. BR, Jani. (*) Well, not exactly *this* but rather https://patchwork.freedesktop.org/patch/80952/ which was not posted on the list so I can't reply to it. > >> --- >> drivers/gpu/drm/i915/i915_gem_context.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c >> index fe580cb9501a..e5ad7b21e356 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_context.c >> +++ b/drivers/gpu/drm/i915/i915_gem_context.c >> @@ -539,7 +539,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) >> >> len = 4; >> if (INTEL_INFO(engine->dev)->gen >= 7) >> - len += 2 + (num_rings ? 4*num_rings + 2 : 0); >> + len += 2 + (num_rings ? 4*num_rings + 6 : 0); >> >> ret = intel_ring_begin(req, len); >> if (ret) >> @@ -579,6 +579,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) >> if (INTEL_INFO(engine->dev)->gen >= 7) { >> if (num_rings) { >> struct intel_engine_cs *signaller; >> + i915_reg_t last_reg = {}; /* keep gcc quiet */ >> >> intel_ring_emit(engine, >> MI_LOAD_REGISTER_IMM(num_rings)); >> @@ -586,11 +587,19 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) >> if (signaller == engine) >> continue; >> >> - intel_ring_emit_reg(engine, >> - RING_PSMI_CTL(signaller->mmio_base)); >> + last_reg = RING_PSMI_CTL(signaller->mmio_base); >> + intel_ring_emit_reg(engine, last_reg); >> intel_ring_emit(engine, >> _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE)); >> } >> + >> + /* Insert a delay before the next switch! */ >> + intel_ring_emit(engine, >> + MI_STORE_REGISTER_MEM | >> + MI_SRM_LRM_GLOBAL_GTT); >> + intel_ring_emit_reg(engine, last_reg); >> + intel_ring_emit(engine, engine->scratch.gtt_offset); >> + intel_ring_emit(engine, MI_NOOP); >> } >> intel_ring_emit(engine, MI_ARB_ON_OFF | MI_ARB_ENABLE); >> } >> -- >> 2.8.0.rc3 >> -- Jani Nikula, Intel Open Source Technology Center