[Intel-gfx] [PATCH 2/5] drm/i915: Do not access stolen memory directly by the CPU, even for error capture

[Chris Wilson]

On Thu, Aug 14, 2014 at 05:51:48PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > For stolen pages, since it is verboten to access them directly on many
> > architectures, we have to read them through the GTT aperture. If they
> > are not accessible through the aperture, then we have to abort.
> >
> > This was complicated by
> >
> > commit 8b6124a633d8095b0c8364f585edff9c59568a96
> > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > Date:   Thu Jan 30 14:38:16 2014 +0000
> >
> >     drm/i915: Don't access snooped pages through the GTT (even for error capture)
> >
> > and the desire to use stolen memory for ringbuffers, contexts and
> > batches in the future.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 50 ++++++++++++++++++++++-------------
> >  1 file changed, 31 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 35e70d5..6d280c07 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -561,10 +561,11 @@ static struct drm_i915_error_object *
> >  i915_error_object_create_sized(struct drm_i915_private *dev_priv,
> >  			       struct drm_i915_gem_object *src,
> >  			       struct i915_address_space *vm,
> > -			       const int num_pages)
> > +			       int num_pages)
> >  {
> >  	struct drm_i915_error_object *dst;
> > -	int i;
> > +	bool use_ggtt;
> > +	int i = 0;
> >  	u32 reloc_offset;
> >  
> >  	if (src == NULL || src->pages == NULL)
> > @@ -574,8 +575,32 @@ i915_error_object_create_sized(struct drm_i915_private *dev_priv,
> >  	if (dst == NULL)
> >  		return NULL;
> >  
> > -	reloc_offset = dst->gtt_offset = i915_gem_obj_offset(src, vm);
> > -	for (i = 0; i < num_pages; i++) {
> > +	dst->gtt_offset = i915_gem_obj_offset(src, vm);
> > +
> > +	reloc_offset = dst->gtt_offset;
> > +	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
> > +		    i915_is_ggtt(vm) &&
> > +		    src->has_global_gtt_mapping &&
> > +		    reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> > +
> > +	/* Cannot access stolen address directly, try to use the aperture */
> > +	if (src->stolen) {
> > +		use_ggtt = true;
> > +
> > +		if (!src->has_global_gtt_mapping)
> > +			goto unwind;
> > +
> > +		reloc_offset = i915_gem_obj_ggtt_offset(src);
> > +		if (reloc_offset + num_pages * PAGE_SIZE > dev_priv->gtt.mappable_end)
> > +			goto unwind;
> > +	}
> > +
> > +	/* Cannot access snooped pages through the aperture */
> > +	if (use_ggtt && src->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv->dev))
> > +		goto unwind;
> 
> Why do we need to bail out if we dont have LLC ?

It is verboten to access snooped PTEs through the GTT. (As in it will
hang some machines.)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre