From patchwork Tue Aug 4 21:30:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Zanoni, Paulo R" X-Patchwork-Id: 6944541 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B9C69C05AC for ; Tue, 4 Aug 2015 21:30:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9434320546 for ; Tue, 4 Aug 2015 21:30:21 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 34D972053C for ; Tue, 4 Aug 2015 21:30:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 676866E523; Tue, 4 Aug 2015 14:30:19 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 8468F6E523 for ; Tue, 4 Aug 2015 14:30:17 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 04 Aug 2015 14:30:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,611,1432623600"; d="scan'208";a="761975484" Received: from hmmeyer-mobl2.amr.corp.intel.com (HELO panetone.amr.corp.intel.com) ([10.252.133.90]) by fmsmga001.fm.intel.com with ESMTP; 04 Aug 2015 14:30:16 -0700 From: Paulo Zanoni To: intel-gfx@lists.freedesktop.org Date: Tue, 4 Aug 2015 18:30:08 -0300 Message-Id: <1438723808-19643-1-git-send-email-paulo.r.zanoni@intel.com> X-Mailer: git-send-email 2.4.6 Subject: [Intel-gfx] [PATCH] drm/i915: fix stolen bios_reserved checks X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I started digging this when I noticed that the BDW code was just reserving 1mb by coincidence since it was reading reserved fields. Then I noticed we didn't have any values set for SNB and earlier, and that the HSW sizes were wrong. After that, I noticed that the reserved area has a specific start, and may not exactly end where the stolen memory ends. I also noticed the base pointer can be zero. So I decided to just write a single patch fixing everything instead of 20 patches that would be much harder to review. This patch may solve random stolen memory corruption/problems on almost all platforms. Notice that since this is always dealing with the top of the stolen memory, the problems are not so easy to reproduce - especially since FBC is still disabled by default. One of the major differences of this patch is that we now look at both the size and base address. By only looking at the size we were assuming that the bios reserved area was always at the very top of stolen, which is not always true: I have a HSW machine that falls into this category. After we merge the patch series that allows user space to allocate stolen memory we'll be able to write IGT tests that maybe catch the bugs fixed by this patch. Signed-off-by: Paulo Zanoni Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) --- drivers/gpu/drm/i915/i915_gem_stolen.c | 156 ++++++++++++++++++++++++++++++--- drivers/gpu/drm/i915/i915_reg.h | 19 ++-- 2 files changed, 157 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index ed682a9..cc8b26a 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -186,11 +186,103 @@ void i915_gem_cleanup_stolen(struct drm_device *dev) drm_mm_takedown(&dev_priv->mm.stolen); } +static void gen6_get_bios_reserved(struct drm_i915_private *dev_priv, + unsigned long *base, unsigned long *size) +{ + uint32_t reg_val = I915_READ(GEN6_BIOS_RESERVED); + + *base = reg_val & GEN6_BIOS_RESERVED_ADDR_MASK; + + switch (reg_val & GEN6_BIOS_RESERVED_SIZE_MASK) { + case GEN6_BIOS_RESERVED_1M: + *size = 1024 * 1024; + break; + case GEN6_BIOS_RESERVED_512K: + *size = 512 * 1024; + break; + case GEN6_BIOS_RESERVED_256K: + *size = 256 * 1024; + break; + case GEN6_BIOS_RESERVED_128K: + *size = 128 * 1024; + break; + default: + *size = 1024 * 1024; + MISSING_CASE(reg_val & GEN6_BIOS_RESERVED_SIZE_MASK); + } +} + +static void gen7_get_bios_reserved(struct drm_i915_private *dev_priv, + unsigned long *base, unsigned long *size) +{ + uint32_t reg_val = I915_READ(GEN6_BIOS_RESERVED); + + *base = reg_val & GEN7_BIOS_RESERVED_ADDR_MASK; + + switch (reg_val & GEN7_BIOS_RESERVED_SIZE_MASK) { + case GEN7_BIOS_RESERVED_1M: + *size = 1024 * 1024; + break; + case GEN7_BIOS_RESERVED_256K: + *size = 256 * 1024; + break; + default: + *size = 1024 * 1024; + MISSING_CASE(reg_val & GEN7_BIOS_RESERVED_SIZE_MASK); + } +} + +static void gen8_get_bios_reserved(struct drm_i915_private *dev_priv, + unsigned long *base, unsigned long *size) +{ + uint32_t reg_val = I915_READ(GEN6_BIOS_RESERVED); + + *base = reg_val & GEN6_BIOS_RESERVED_ADDR_MASK; + + switch (reg_val & GEN8_BIOS_RESERVED_SIZE_MASK) { + case GEN8_BIOS_RESERVED_1M: + *size = 1024 * 1024; + break; + case GEN8_BIOS_RESERVED_2M: + *size = 2 * 1024 * 1024; + break; + case GEN8_BIOS_RESERVED_4M: + *size = 4 * 1024 * 1024; + break; + case GEN8_BIOS_RESERVED_8M: + *size = 8 * 1024 * 1024; + break; + default: + *size = 8 * 1024 * 1024; + MISSING_CASE(reg_val & GEN8_BIOS_RESERVED_SIZE_MASK); + } +} + +static void bdw_get_bios_reserved(struct drm_i915_private *dev_priv, + unsigned long *base, unsigned long *size) +{ + uint32_t reg_val = I915_READ(GEN6_BIOS_RESERVED); + unsigned long stolen_top; + + stolen_top = dev_priv->mm.stolen_base + dev_priv->gtt.stolen_size; + + *base = reg_val & GEN6_BIOS_RESERVED_ADDR_MASK; + + /* On these platforms, the register doesn't have a size field, so the + * size is the distance between the base and the top of the stolen + * memory. We also have the genuine case where base is zero and there's + * nothing reserved. */ + if (*base == 0) + *size = 0; + else + *size = stolen_top - *base; +} + int i915_gem_init_stolen(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - u32 tmp; - int bios_reserved = 0; + unsigned long bios_rsvd_total, bios_rsvd_base, bios_rsvd_size; + unsigned long stolen_top; mutex_init(&dev_priv->mm.stolen_lock); @@ -211,23 +303,61 @@ int i915_gem_init_stolen(struct drm_device *dev) DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n", dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base); - if (INTEL_INFO(dev)->gen >= 8) { - tmp = I915_READ(GEN7_BIOS_RESERVED); - tmp >>= GEN8_BIOS_RESERVED_SHIFT; - tmp &= GEN8_BIOS_RESERVED_MASK; - bios_reserved = (1024*1024) << tmp; - } else if (IS_GEN7(dev)) { - tmp = I915_READ(GEN7_BIOS_RESERVED); - bios_reserved = tmp & GEN7_BIOS_RESERVED_256K ? - 256*1024 : 1024*1024; + stolen_top = dev_priv->mm.stolen_base + dev_priv->gtt.stolen_size; + + switch (INTEL_INFO(dev_priv)->gen) { + case 2: + case 3: + case 4: + case 5: + /* Assume the gen6 maximum for the older platforms. */ + bios_rsvd_size = 1024 * 1024; + bios_rsvd_base = stolen_top - bios_rsvd_size; + break; + case 6: + gen6_get_bios_reserved(dev_priv, &bios_rsvd_base, + &bios_rsvd_size); + break; + case 7: + if (IS_HASWELL(dev_priv)) + gen6_get_bios_reserved(dev_priv, &bios_rsvd_base, + &bios_rsvd_size); + else + gen7_get_bios_reserved(dev_priv, &bios_rsvd_base, + &bios_rsvd_size); + break; + default: + if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv)) + bdw_get_bios_reserved(dev_priv, &bios_rsvd_base, + &bios_rsvd_size); + else + gen8_get_bios_reserved(dev_priv, &bios_rsvd_base, + &bios_rsvd_size); + break; + } + + /* It is possible for the BIOS reserved base to be zero, but the + * register field for size doesn't have a zero option. */ + if (bios_rsvd_base == 0) { + bios_rsvd_size = 0; + bios_rsvd_base = stolen_top; } - if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size)) + if (bios_rsvd_base < dev_priv->mm.stolen_base || + bios_rsvd_base + bios_rsvd_size > stolen_top) { + DRM_ERROR("BIOS reserved area outside stolen memory\n"); return 0; + } + + /* It is possible for the BIOS reserved area to end before the end of + * stolen memory, so just consider the start. */ + bios_rsvd_total = stolen_top - bios_rsvd_base; + + DRM_DEBUG_KMS("%lu bytes of stolen memory reserved\n", bios_rsvd_total); /* Basic memrange allocator for stolen space */ drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size - - bios_reserved); + bios_rsvd_total); return 0; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8cf7756..539127e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -178,13 +178,22 @@ #define GAB_CTL 0x24000 #define GAB_CTL_CONT_AFTER_PAGEFAULT (1<<8) -#define GEN7_BIOS_RESERVED 0x1082C0 +#define GEN6_BIOS_RESERVED 0x1082C0 +#define GEN6_BIOS_RESERVED_ADDR_MASK (0xFFF << 20) +#define GEN7_BIOS_RESERVED_ADDR_MASK (0x3FFF << 18) +#define GEN6_BIOS_RESERVED_SIZE_MASK (3 << 4) +#define GEN6_BIOS_RESERVED_1M (0 << 4) +#define GEN6_BIOS_RESERVED_512K (1 << 4) +#define GEN6_BIOS_RESERVED_256K (2 << 4) +#define GEN6_BIOS_RESERVED_128K (3 << 4) +#define GEN7_BIOS_RESERVED_SIZE_MASK (1 << 5) #define GEN7_BIOS_RESERVED_1M (0 << 5) #define GEN7_BIOS_RESERVED_256K (1 << 5) -#define GEN8_BIOS_RESERVED_SHIFT 7 -#define GEN7_BIOS_RESERVED_MASK 0x1 -#define GEN8_BIOS_RESERVED_MASK 0x3 - +#define GEN8_BIOS_RESERVED_SIZE_MASK (3 << 7) +#define GEN8_BIOS_RESERVED_1M (0 << 7) +#define GEN8_BIOS_RESERVED_2M (1 << 7) +#define GEN8_BIOS_RESERVED_4M (2 << 7) +#define GEN8_BIOS_RESERVED_8M (3 << 7) /* VGA stuff */