[Intel-gfx] [PATCH 2/2] drm/i915: Don't access snooped pages through the GTT (even for error capture)

[Chris Wilson]

On Thu, Jan 30, 2014 at 05:08:09PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 30, 2014 at 02:38:16PM +0000, Chris Wilson wrote:
> > We want to use the GTT for reading back objects upon an error so that we
> > have exactly the information that the GPU saw. However, it is verboten
> > to access snoopable pages through the GTT and causes my PineView GPU to
> > throw a page fault instead.
> > 
> > This has not been a problem in the past as we only dumped ringbuffers
> > and batchbuffers, both of which must be uncached. However, the
> > introduction of HWS page dumping leads to a read of a snooped object
> > through the GTT. This was introduced by
> > 
> > commit f3ce3821393e31a3f1a8ca6c24eb2d735a428445
> > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > Date:   Thu Jan 23 22:40:36 2014 +0000
> > 
> >     drm/i915: Include HW status page in error capture
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 6d5ab945132c..3b18c2dff3b8 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -527,7 +527,8 @@ i915_error_object_create_sized(struct drm_i915_private *dev_priv,
> >  			goto unwind;
> >  
> >  		local_irq_save(flags);
> > -		if (reloc_offset < dev_priv->gtt.mappable_end &&
> > +		if (src->cache_level == I915_CACHE_NONE &&
> > +		    reloc_offset < dev_priv->gtt.mappable_end &&
> >  		    src->has_global_gtt_mapping &&
> >  		    i915_is_ggtt(vm)) {
> >  			void __iomem *s;
> 
> So you don't want to make it (cache_level == NONE || HAS_LLC)?

That llc is coherent between the physical pages and GTT is deep rooted
in our code by now, and Ben is working hard to make sure we never take
that path on llc anyway. Hence I let it go.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre