Re: [PATCH v2 2/2] drm/i915/bxt: work around HW context corruption due to coherency problem

From: Daniel Vetter <daniel@ffwll.ch>
To: "Chris Wilson" <chris@chris-wilson.co.uk>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Imre Deak" <imre.deak@intel.com>,
	intel-gfx@lists.freedesktop.org,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Subject: Re: [PATCH v2 2/2] drm/i915/bxt: work around HW context corruption due to coherency problem
Date: Wed, 23 Sep 2015 15:58:37 +0200	[thread overview]
Message-ID: <20150923135837.GC3383@phenom.ffwll.local> (raw)
In-Reply-To: <20150923133913.GE6739@nuc-i3427.alporthouse.com>

On Wed, Sep 23, 2015 at 02:39:13PM +0100, Chris Wilson wrote:
> On Wed, Sep 23, 2015 at 03:35:59PM +0200, Daniel Vetter wrote:
> > On Fri, Sep 18, 2015 at 10:02:24AM +0100, Chris Wilson wrote:
> > > On Thu, Sep 17, 2015 at 07:17:44PM +0300, Imre Deak wrote:
> > > > The execlist context object is mapped with a CPU/GPU coherent mapping
> > > > everywhere, but on BXT A stepping due to a HW issue the coherency is not
> > > > guaranteed. To work around this flush the context object after pinning
> > > > it (to flush cache lines left by the context initialization/read-back
> > > > from backing storage) and mark it as uncached so later updates during
> > > > context switching will be coherent.
> > > > 
> > > > I noticed this problem via a GPU hang, where IPEHR pointed to an invalid
> > > > opcode value. I couldn't find this value on the ring but looking at the
> > > > contents of the active context object it turned out to be a parameter
> > > > dword of a bigger command there. The original command opcode itself
> > > > was zeroed out, based on the above I assume due to a CPU writeback of
> > > > the corresponding cacheline. When restoring the context the GPU would
> > > > jump over the zeroed out opcode and hang when trying to execute the
> > > > above parameter dword.
> > > > 
> > > > I could easily reproduce this by running igt/gem_render_copy_redux and
> > > > gem_tiled_blits/basic in parallel, but I guess it could be triggered by
> > > > anything involving frequent switches between two separate contexts. With
> > > > this workaround I couldn't reproduce the problem.
> > > > 
> > > > v2:
> > > > - instead of clflushing after updating the tail and PDP values during
> > > >   context switching, map the corresponding page as uncached to avoid a
> > > >   race between CPU and GPU, both updating the same cacheline at the same
> > > >   time (Ville)
> > > 
> > > No. Changing PAT involves a stop_machine() and is severely detrimental
> > > to performance (context creation overhead does impact userspace).
> > > Mapping it as uncached doesn't remove the race anyway.
> > 
> > Yeah it's not pretty, but otoh it's for A stepping and we'll kill it again
> > once bxt is shipping. I think with a big "IXME: dont ever dare to copy
> > this" comment this is acceptable. It's not really the worst "my gpu
> > crawls" workaround we've seend for early hw ...
> 
> Thinking about this, an incoherent TAIL write cannot cause IPEHR !=
> *ACTHD. The flush is just papering over the absence of a flush elsewhere
> and the root cause remains unfixed.

I thought the incoherence happens in the ring itself, not the TAIL update.
That one might just cause additional fun.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx