From patchwork Fri May 16 11:08:22 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: oscar.mateo@intel.com X-Patchwork-Id: 4190201 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.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 933B7BFF02 for ; Fri, 16 May 2014 11:13:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CA7B7202E9 for ; Fri, 16 May 2014 11:13:38 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 5ADAB20295 for ; Fri, 16 May 2014 11:13:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A0F3B6E2D5; Fri, 16 May 2014 04:13:36 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 912066E2D5 for ; Fri, 16 May 2014 04:13:35 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 16 May 2014 04:13:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="4.97,1066,1389772800"; d="scan'208"; a="541540661" Received: from omateolo-linux2.iwi.intel.com ([172.28.253.145]) by orsmga002.jf.intel.com with ESMTP; 16 May 2014 04:13:33 -0700 From: oscar.mateo@intel.com To: intel-gfx@lists.freedesktop.org Date: Fri, 16 May 2014 12:08:22 +0100 Message-Id: <1400238502-10299-1-git-send-email-oscar.mateo@intel.com> X-Mailer: git-send-email 1.9.0 In-Reply-To: <1396462861-16396-1-git-send-email-oscar.mateo@intel.com> References: <1396462861-16396-1-git-send-email-oscar.mateo@intel.com> Subject: [Intel-gfx] [PATCH v2] drm/i915: Gracefully handle obj not bound to GGTT in is_pin_display X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 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.8 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 From: Oscar Mateo Otherwise, we do a NULL pointer dereference. I've seen this happen while handling an error in i915_gem_object_pin_to_display_plane(): If i915_gem_object_set_cache_level() fails, we call is_pin_display() to handle the error. At this point, the object is still not pinned to GGTT and maybe not even bound, so we have to check before we dereference its GGTT vma. v2: Chris Wilson says restoring the old value is easier, but that is_pin_display is useful as a theory of operation. Take the solomonic decision: at least this way is_pin_display is a little more robust (until Chris can kill it off). Issue: VIZ-3772 Signed-off-by: Oscar Mateo Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 034ba2c..211b778 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3641,6 +3641,15 @@ unlock: static bool is_pin_display(struct drm_i915_gem_object *obj) { + struct i915_vma *vma; + + if (list_empty(&obj->vma_list)) + return false; + + vma = i915_gem_obj_to_ggtt(obj); + if (!vma) + return false; + /* There are 3 sources that pin objects: * 1. The display engine (scanouts, sprites, cursors); * 2. Reservations for execbuffer; @@ -3652,7 +3661,7 @@ static bool is_pin_display(struct drm_i915_gem_object *obj) * subtracting the potential reference by the user, any pin_count * remains, it must be due to another use by the display engine. */ - return i915_gem_obj_to_ggtt(obj)->pin_count - !!obj->user_pin_count; + return vma->pin_count - !!obj->user_pin_count; } /* @@ -3666,6 +3675,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, struct intel_ring_buffer *pipelined) { u32 old_read_domains, old_write_domain; + bool was_pin_display; int ret; if (pipelined != obj->ring) { @@ -3677,6 +3687,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, /* Mark the pin_display early so that we account for the * display coherency whilst setting up the cache domains. */ + was_pin_display = obj->pin_display; obj->pin_display = true; /* The display engine is not coherent with the LLC cache on gen6. As @@ -3719,7 +3730,8 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, return 0; err_unpin_display: - obj->pin_display = is_pin_display(obj); + WARN_ON(was_pin_display != is_pin_display(obj)); + obj->pin_display = was_pin_display; return ret; }