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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jan 30, 2014 at 03:11:08PM +0000, Chris Wilson wrote:
> 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@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@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.

Fine by me. For the series:

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

-- 
Ville Syrjälä
Intel OTC