From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: Detect invalid pages for SandyBridge Date: Thu, 27 Sep 2012 13:22:18 +0100 Message-ID: <6c3329$6cct5d@orsmga002.jf.intel.com> References: <1348734477-23369-1-git-send-email-chris@chris-wilson.co.uk> <20120927121422.GC2098@bremse> <20120927121611.GD2098@bremse> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id D30579E96C for ; Thu, 27 Sep 2012 05:22:45 -0700 (PDT) In-Reply-To: <20120927121611.GD2098@bremse> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 27 Sep 2012 14:16:11 +0200, Daniel Vetter wrote: > On Thu, Sep 27, 2012 at 02:14:22PM +0200, Daniel Vetter wrote: > > On Thu, Sep 27, 2012 at 09:27:57AM +0100, Chris Wilson wrote: > > > As SandyBridge returns garbage when decoding certain addresses through > > > the GTT (all memory below 1MiB and a very small number of individual > > > pages) we need to prevent the GPU from utilizing those pages. The > > > ultimate goal would be to prevent our allocator from handing us those > > > pages, but that is a longer term project. In the short term, we can > > > detect when we attempt to bind those pages to the GPU and return an > > > error to the application rather than hang the GPU and potentially the > > > system. > > > > > > Signed-off-by: Chris Wilson > > > --- > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 40 ++++++++++++++++++++++++++++++++--- > > > 1 file changed, 37 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > index d9d3fc7..5c7ccfd 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > @@ -306,17 +306,51 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev) > > > intel_gtt_chipset_flush(); > > > } > > > > > > +static bool > > > +gen6_valid_addresses(struct drm_i915_gem_object *obj) > > > +{ > > > + struct scatterlist *sg; > > > + int i; > > > + > > > + for_each_sg(obj->pages->sgl, sg, obj->pages->nents, i) { > > > + dma_addr_t addr = sg_dma_address(sg); > > > + if (WARN(addr < 0x100000 || > > > + addr == 0x20050000 || > > > + addr == 0x20110000 || > > > + addr == 0x20130000 || > > > + addr == 0x20138000 || > > > + addr == 0x40004000, > > > + "object references unaddressable physical pages: addr=%x", > > > + (u32)addr)) > > > + return false; > > > > Iirc the bug is about the physical address, no the remapped one after > > dmar. We'd need to check the windows code for that though ... Hmm, we pass the remapped addresses to the GTT so I presumed that would be what the decoder threw up over. > Also maybe don't fail, but only smash some dirt into dmesg - otherwise > we'll score a neat regression report and probably angry users. I wouldn't consider that it was a regression, we go from either render corruption, GPU hang or full system hang to absent rendering and lots of noise. I'd much rather take the latter. All in all though, I really want to see if this is ever an issue and when. -Chris -- Chris Wilson, Intel Open Source Technology Centre