[Intel-gfx] [PATCH] drm/i915: Don't allow binding objects into the last page of the aperture.
jbarnes at virtuousgeek.org
Wed May 13 02:25:00 CEST 2009
On Tue, 12 May 2009 17:12:29 -0700
Eric Anholt <eric at anholt.net> wrote:
> 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.
Yeah forgot about that bit, good.
> 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.
Yeah that would be great.
Jesse Barnes, Intel Open Source Technology Center
More information about the Intel-gfx