[Intel-gfx] [PATCH] drm/i915: Don't allow binding objects into the last page of the aperture.

[Eric Anholt]

On Tue, 2009-05-12 at 15:34 -0700, Jesse Barnes wrote:
> On Tue, 12 May 2009 15:29:56 -0700
> Eric Anholt <eric at anholt.net> wrote:
> 
> > This should avoid a class of bugs where the hardware prefetches past
> > the end of the object, and walks into unallocated memory when the
> > object is bound to the last page of the aperture.
> > 
> > fd.o bug #21488
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c |   12 ++++++++++--
> >  1 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c index 051134c..3133f99 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1011,8 +1011,16 @@ static int i915_load_modeset_init(struct
> > drm_device *dev) /* Basic memrange allocator for stolen space (aka
> > vram) */ drm_mm_init(&dev_priv->vram, 0, prealloc_size);
> >  
> > -	/* Let GEM Manage from end of prealloc space to end of
> > aperture */
> > -	i915_gem_do_init(dev, prealloc_size, agp_size);
> > +	/* Let GEM Manage from end of prealloc space to end of
> > aperture.
> > +	 *
> > +	 * However, leave one page at the end still bound to the
> > scratch page.
> > +	 * There are a number of places where the hardware apparently
> > +	 * prefetches past the end of the object, and we've seen
> > multiple
> > +	 * hangs with the GPU head pointer stuck in a batchbuffer
> > bound
> > +	 * at the last page of the aperture.  One page should be
> > enough to
> > +	 * keep any prefetching inside of the aperture.
> > +	 */
> > +	i915_gem_do_init(dev, prealloc_size, agp_size - 4096);
> 
> What about objects in the middle of the aperture that are immediately
> followed by unbound pages?  Do we already handle that case by adding a
> padding page to objects (iirc one of the docs mentions that this is
> necessary).

We never have unbound pages inside of the aperture once AGP has
initialized -- they're always mapped to the scratch page, like the docs
recommend.  Thus why this patch should work -- it leaves the last page
unused and just pointing at scratch.

Note that we've talked about a quick and dirty hack taking advantage of
the scratch -- make a sysfs knob that says "operate execbuffers
synchronously, and compare the contents of the scratch page before and
after each execbuf".  This would let us catch some failures with writing
beyond the boundaries of objects at a very low code cost.

-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090512/6a5df08c/attachment.sig>