From patchwork Fri Jun 12 21:30:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paulo Zanoni X-Patchwork-Id: 6600901 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 F2568C0020 for ; Fri, 12 Jun 2015 21:31:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0F0F820661 for ; Fri, 12 Jun 2015 21:31:57 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id B474C2065D for ; Fri, 12 Jun 2015 21:31:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 16EC86E5E1; Fri, 12 Jun 2015 14:31:54 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-qg0-f46.google.com (mail-qg0-f46.google.com [209.85.192.46]) by gabe.freedesktop.org (Postfix) with ESMTP id 27E576E017 for ; Fri, 12 Jun 2015 14:31:53 -0700 (PDT) Received: by qgfa66 with SMTP id a66so15314816qgf.0 for ; Fri, 12 Jun 2015 14:31:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=0AWhwi8e6/j0y3RnEkjmn0Jt/xR5+XyahBZLvj/q21o=; b=ltt4hqEaQK4sP3o1hZFLMH9+1lVOh73C7ledvAlpSC/EwAb6CB6q4WJqRglnc20Opr ZYBFFdwP7recvPZA5+ATsXYEAff2zBdyIroq3hawd5l61KQ9muRm2MTfnUvKqs0R/zVq Td7BbEyzPwZ3xFevDerXSY79DLHYlWAhIBZLYdbI727zUGwRDW9tMP3uUDYgvg/Qsddk vm6UuiJMKBwXdzf/op/HOMTDkevNYBfqcnekA7IOnZslFQmkhe/2RPudDayBN64Lje1x KhEKbKIX+HhYca19OhAYDduTp4fk4tkOqnN24RzReoY2enNz8HGZ+RzTZm3e5mUDC1HS cnHg== X-Received: by 10.140.234.84 with SMTP id f81mr22587116qhc.49.1434144712775; Fri, 12 Jun 2015 14:31:52 -0700 (PDT) Received: from localhost.localdomain ([187.95.117.237]) by mx.google.com with ESMTPSA id c73sm2217303qka.24.2015.06.12.14.31.51 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 12 Jun 2015 14:31:52 -0700 (PDT) From: Paulo Zanoni To: intel-gfx@lists.freedesktop.org Date: Fri, 12 Jun 2015 18:30:56 -0300 Message-Id: <1434144656-1557-1-git-send-email-przanoni@gmail.com> X-Mailer: git-send-email 2.1.4 Cc: Paulo Zanoni Subject: [Intel-gfx] [RFC] drm/i915: prevent out of range pt in the PDE macros (take 2) 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.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_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 From: Paulo Zanoni We tried to fix this in the following commit: commit fdc454c1484a20e1345cf4e4d7a9feaee814147f Author: Michel Thierry Date: Tue Mar 24 15:46:19 2015 +0000 drm/i915: Prevent out of range pt in gen6_for_each_pde but the static analyzer still complains that, just before we break due to "iter < I915_PDES", we do "pt = (pd)->page_table[iter]" with an iter value that is bigger than I915_PDES. Of course, this isn't really a problem since no one uses pt outside the macro. Still, every single new usage of the macro will create a new issue for us to mark as a false possitive. After the commit mentioned above we also created some new versions of the macros, so they carry the same "problem". In order to "solve" this "problem", let's leave the macro with a NULL value for pt. So if somebody uses it, we're more likely to get a big error message instead of some silent failure. I hope the static analyzer won't complain about the new solution (I don't have a way to check this!). I know, the solution looks really ugly. I am hoping the reviewers will help us decide if we prefer this patch or if we prefer to keep marking things as false positives. Cc: Michel Thierry Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_gem_gtt.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) I sent this as an RFC because I really don't know if complicating the macro even more will help us in any way. I won't really be surprised if I see NACKs on this patch, so don't hesitate if you want to. Also, all I did was boot a Kernel with this patch and make sure it shows the desktop. So consider this as untested, possibly broken. diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 0d46dd2..b202ca0 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -352,7 +352,8 @@ struct i915_hw_ppgtt { */ #define gen6_for_each_pde(pt, pd, start, length, temp, iter) \ for (iter = gen6_pde_index(start); \ - pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES; \ + pt = iter < I915_PDES ? (pd)->page_table[iter] : NULL, \ + length > 0 && iter < I915_PDES; \ iter++, \ temp = ALIGN(start+1, 1 << GEN6_PDE_SHIFT) - start, \ temp = min_t(unsigned, temp, length), \ @@ -360,7 +361,8 @@ struct i915_hw_ppgtt { #define gen6_for_all_pdes(pt, ppgtt, iter) \ for (iter = 0; \ - pt = ppgtt->pd.page_table[iter], iter < I915_PDES; \ + pt = iter < I915_PDES ? ppgtt->pd.page_table[iter] : NULL, \ + iter < I915_PDES; \ iter++) static inline uint32_t i915_pte_index(uint64_t address, uint32_t pde_shift) @@ -417,7 +419,8 @@ static inline uint32_t gen6_pde_index(uint32_t addr) */ #define gen8_for_each_pde(pt, pd, start, length, temp, iter) \ for (iter = gen8_pde_index(start); \ - pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES; \ + pt = iter < I915_PDES ? (pd)->page_table[iter] : NULL, \ + length > 0 && iter < I915_PDES; \ iter++, \ temp = ALIGN(start+1, 1 << GEN8_PDE_SHIFT) - start, \ temp = min(temp, length), \ @@ -425,7 +428,9 @@ static inline uint32_t gen6_pde_index(uint32_t addr) #define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter) \ for (iter = gen8_pdpe_index(start); \ - pd = (pdp)->page_directory[iter], length > 0 && iter < GEN8_LEGACY_PDPES; \ + pd = iter < GEN8_LEGACY_PDPES ? \ + (pdp)->page_directory[iter] : NULL, \ + length > 0 && iter < GEN8_LEGACY_PDPES; \ iter++, \ temp = ALIGN(start+1, 1 << GEN8_PDPE_SHIFT) - start, \ temp = min(temp, length), \