From patchwork Fri Jan 4 19:18:53 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Imre Deak X-Patchwork-Id: 1934491 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 2FAC3DFABD for ; Fri, 4 Jan 2013 19:19:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0C1F7E66C5 for ; Fri, 4 Jan 2013 11:19:06 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 6E34AE5DEE for ; Fri, 4 Jan 2013 11:18:57 -0800 (PST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 04 Jan 2013 11:18:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.84,412,1355126400"; d="scan'208";a="272965052" Received: from unknown (HELO [10.252.123.73]) ([10.252.123.73]) by fmsmga002.fm.intel.com with ESMTP; 04 Jan 2013 11:18:54 -0800 Message-ID: <1357327133.4147.9.camel@ideak-mobl> From: Imre Deak To: Chris Wilson Date: Fri, 04 Jan 2013 21:18:53 +0200 In-Reply-To: <6c3329$7v1jn1@orsmga002.jf.intel.com> References: <1357317721-6313-1-git-send-email-imre.deak@intel.com> <1357317721-6313-6-git-send-email-imre.deak@intel.com> <1357320222.2737.9.camel@localhost> <6c3329$7v1jn1@orsmga002.jf.intel.com> X-Mailer: Evolution 3.6.0-0ubuntu3 Mime-Version: 1.0 Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 5/5] drm/i915: fix gtt space allocated for tiled objects X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org On Fri, 2013-01-04 at 17:47 +0000, Chris Wilson wrote: > On Fri, 04 Jan 2013 19:23:42 +0200, Imre Deak wrote: > > On Fri, 2013-01-04 at 17:07 +0000, Chris Wilson wrote: > > > On Fri, 4 Jan 2013 18:42:00 +0200, Imre Deak wrote: > > > > The gtt space needed for tiled objects might be bigger than the linear > > > > size programmed into the correpsonding fence register. For example for > > > > the following buffer on a Gen5+ HW: > > > > > > > > - allocation size: 4096 bytes > > > > - tiling mode: X tiled > > > > - stride: 1536 > > > > > > > > we need (1536 / 512) * 4096 bytes of gtt space to cover all the pixels > > > > in the buffer, but at the moment we allocate only 4096. This means that > > > > any buffer following this tiled buffer in the gtt space will be > > > > corrupted if pixels belonging to the 2nd and 3rd tiles are written. > > > > > > > > Fix this by rounding up the size of the allocated gtt space to the next > > > > tile row address. The page frames beyond the allocation size will be > > > > backed by the single gtt scratch page used already elsewhere for similar > > > > padding. > > > > > > > > Note that this is more of a security/robustness problem and not fixing any > > > > reported issue that I know of. This is because applications will normally > > > > access only the part of the buffer that is tile row size aligned. > > > > > > There should not be any reported issues because all userspace already > > > allocates up to the end of tile-row and stride should be enforced to be > > > a multiple of tile-width. So the use of DIV_ROUND_UP implies a > > > programming error that should have been reported back to userspace > > > earlier. We can extend that by checking to make sure userspace has > > > allocated a valid buffer, that is, it has allocated sufficient pages for > > > the sampler access into the tiled buffer (or reject the set-tiling). > > > -Chris > > > > Ok, I tested this with older UXA that still allocated non-aligned > > buffers. If that's not the case any more then rejecting set-tiling if > > it's called on a non tile-row size aligned buffer would work too. > > Meep. A long time ago we got the calcuations wrong (slightly less for > gen2), but it was and still is a userspace bug with the potential of > handing the GPU. > > A multiple of tile_height tall and a mutiple of tile_width across should > always be an exact number of pages (and an exact multiple of tile-row > pages). And we should have been obeying that since the introduction of > set-tiling (ignoring the aforementioned bugs) - so I'd really like to > see any evidence of userspace getting that wrong. On Ubuntu 12.10, with xf86-video-intel 2.20.9 I get the attached log with the following patch applied: if (stride < tile_width) return false; dmesg | grep unaligned | cut -c 16- | sort | uniq -c 2 unaligned tiling: comm compiz size 10485760 mode X stride 6656 size-mod-tilerow-size 49152 2 unaligned tiling: comm compiz size 10485760 mode Y stride 6400 size-mod-tilerow-size 40960 1 unaligned tiling: comm compiz size 1572864 mode Y stride 2944 size-mod-tilerow-size 65536 1 unaligned tiling: comm compiz size 163840 mode X stride 6144 size-mod-tilerow-size 16384 1 unaligned tiling: comm compiz size 163840 mode Y stride 1536 size-mod-tilerow-size 16384 2 unaligned tiling: comm compiz size 229376 mode Y stride 1152 size-mod-tilerow-size 8192 1 unaligned tiling: comm compiz size 2621440 mode Y stride 4736 size-mod-tilerow-size 45056 1 unaligned tiling: comm compiz size 262144 mode X stride 5120 size-mod-tilerow-size 16384 2 unaligned tiling: comm compiz size 327680 mode X stride 6144 size-mod-tilerow-size 32768 6 unaligned tiling: comm compiz size 327680 mode X stride 6656 size-mod-tilerow-size 8192 1 unaligned tiling: comm compiz size 327680 mode Y stride 3072 size-mod-tilerow-size 32768 1 unaligned tiling: comm compiz size 327680 mode Y stride 4736 size-mod-tilerow-size 24576 1 unaligned tiling: comm compiz size 3670016 mode X stride 3072 size-mod-tilerow-size 8192 1 unaligned tiling: comm compiz size 458752 mode Y stride 6400 size-mod-tilerow-size 49152 1 unaligned tiling: comm compiz size 524288 mode Y stride 384 size-mod-tilerow-size 8192 1 unaligned tiling: comm compiz size 6291456 mode X stride 6656 size-mod-tilerow-size 8192 2 unaligned tiling: comm compiz size 6291456 mode Y stride 6400 size-mod-tilerow-size 147456 6 unaligned tiling: comm compiz size 65536 mode Y stride 640 size-mod-tilerow-size 4096 1 unaligned tiling: comm unity_support_t size 6291456 mode Y stride 6400 size-mod-tilerow-size 147456 69 unaligned tiling: comm Xorg size 114688 mode X stride 2560 size-mod-tilerow-size 12288 152 unaligned tiling: comm Xorg size 1310720 mode X stride 1536 size-mod-tilerow-size 8192 3 unaligned tiling: comm Xorg size 131072 mode X stride 2560 size-mod-tilerow-size 8192 317 unaligned tiling: comm Xorg size 131072 mode X stride 3072 size-mod-tilerow-size 8192 4 unaligned tiling: comm Xorg size 163840 mode X stride 1536 size-mod-tilerow-size 4096 87 unaligned tiling: comm Xorg size 163840 mode X stride 3072 size-mod-tilerow-size 16384 1104 unaligned tiling: comm Xorg size 163840 mode X stride 3584 size-mod-tilerow-size 20480 1 unaligned tiling: comm Xorg size 163840 mode X stride 4608 size-mod-tilerow-size 16384 1 unaligned tiling: comm Xorg size 163840 mode X stride 6656 size-mod-tilerow-size 4096 2 unaligned tiling: comm Xorg size 196608 mode X stride 2560 size-mod-tilerow-size 12288 12 unaligned tiling: comm Xorg size 196608 mode X stride 3584 size-mod-tilerow-size 24576 20 unaligned tiling: comm Xorg size 196608 mode X stride 4608 size-mod-tilerow-size 12288 2 unaligned tiling: comm Xorg size 2097152 mode X stride 2560 size-mod-tilerow-size 8192 10 unaligned tiling: comm Xorg size 229376 mode X stride 3072 size-mod-tilerow-size 8192 193 unaligned tiling: comm Xorg size 2621440 mode X stride 4608 size-mod-tilerow-size 4096 4 unaligned tiling: comm Xorg size 262144 mode X stride 1536 size-mod-tilerow-size 4096 16 unaligned tiling: comm Xorg size 262144 mode X stride 4608 size-mod-tilerow-size 4096 1 unaligned tiling: comm Xorg size 3145728 mode X stride 5632 size-mod-tilerow-size 36864 3 unaligned tiling: comm Xorg size 327680 mode X stride 1536 size-mod-tilerow-size 8192 2 unaligned tiling: comm Xorg size 393216 mode X stride 4608 size-mod-tilerow-size 24576 3 unaligned tiling: comm Xorg size 40960 mode X stride 1536 size-mod-tilerow-size 4096 1 unaligned tiling: comm Xorg size 5242880 mode X stride 5632 size-mod-tilerow-size 16384 116 unaligned tiling: comm Xorg size 6291456 mode X stride 6656 size-mod-tilerow-size 8192 280 unaligned tiling: comm Xorg size 65536 mode X stride 1536 size-mod-tilerow-size 4096 6 unaligned tiling: comm Xorg size 65536 mode X stride 2560 size-mod-tilerow-size 4096 diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index 7b0a3b3..846e96f 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -236,9 +236,17 @@ i915_tiling_ok(struct drm_device *dev, int stride, int size, int tiling_mode) if (INTEL_INFO(dev)->gen >= 4) { if (stride & (tile_width - 1)) return false; - return true; } + if (size % (stride / tile_width * PAGE_SIZE)) + printk("unaligned tiling: comm %.*s size %u mode %c stride %d size-mod-tilerow-size %zd\n", + (int)sizeof(current->comm), current->comm, + size, tiling_mode == I915_TILING_X ? 'X' : 'Y', + stride, size % (stride / tile_width * PAGE_SIZE)); + + if (INTEL_INFO(dev)->gen >= 4) + return true; + /* Pre-965 needs power of two tile widths */