From patchwork Tue Dec 22 11:00:24 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Winiarski?= X-Patchwork-Id: 7904071 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 22E70BEEE5 for ; Tue, 22 Dec 2015 11:00:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0B22E205B3 for ; Tue, 22 Dec 2015 11:00:36 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 3C2E02057F for ; Tue, 22 Dec 2015 11:00:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7D91272007; Tue, 22 Dec 2015 03:00:33 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 0985072007 for ; Tue, 22 Dec 2015 03:00:32 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP; 22 Dec 2015 03:00:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,464,1444719600"; d="scan'208";a="17513576" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga004.fm.intel.com with ESMTP; 22 Dec 2015 03:00:30 -0800 Received: from mwiniars-desk1.ger.corp.intel.com (172.28.173.39) by IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server id 14.3.248.2; Tue, 22 Dec 2015 11:00:30 +0000 From: =?UTF-8?q?Micha=C5=82=20Winiarski?= To: Date: Tue, 22 Dec 2015 12:00:24 +0100 Message-ID: <1450782024-21707-1-git-send-email-michal.winiarski@intel.com> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1450468863-9268-1-git-send-email-michal.winiarski@intel.com> References: <1450468863-9268-1-git-send-email-michal.winiarski@intel.com> MIME-Version: 1.0 X-Originating-IP: [172.28.173.39] Subject: [Intel-gfx] [PATCH v5] drm/i915: Avoid writing relocs with addresses in non-canonical form 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 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 According to bspec, some parts of HW require the addresses to be in a canonical form, where bits [63:48] == [47]. Let's convert addresses to canonical form prior to relocating and return converted offsets to userspace. We also need to make sure that userspace is using addresses in canonical form in case of softpin. v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville) v3: Rebase on top of softpin, fix a hole in relocate_entry, s/expect/require (Chris) v4: Handle softpin in validate_exec_list (Chris) v5: Convert back to canonical form at copy_to_user time (Chris) Cc: Chris Wilson Cc: Michel Thierry Cc: Ville Syrjälä Signed-off-by: Micha? Winiarski --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 107 ++++++++++++++++++----------- 1 file changed, 66 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 5d01ea6..c906232 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -249,6 +249,25 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj) obj->cache_level != I915_CACHE_NONE); } +/* Used to convert any address to canonical form. + * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS, + * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the + * addresses to be in a canonical form: + * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct + * canonical form [63:48] == [47]." + */ +static inline uint64_t gen8_canonical_addr(uint64_t address) +{ + return ((int64_t)address << 16) >> 16; +} + +static inline uint64_t +relocation_target(struct drm_i915_gem_relocation_entry *reloc, + uint64_t target_offset) +{ + return gen8_canonical_addr((int)reloc->delta + target_offset); +} + static int relocate_entry_cpu(struct drm_i915_gem_object *obj, struct drm_i915_gem_relocation_entry *reloc, @@ -256,7 +275,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; uint32_t page_offset = offset_in_page(reloc->offset); - uint64_t delta = reloc->delta + target_offset; + uint64_t delta = relocation_target(reloc, target_offset); char *vaddr; int ret; @@ -292,7 +311,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - uint64_t delta = reloc->delta + target_offset; + uint64_t delta = relocation_target(reloc, target_offset); uint64_t offset; void __iomem *reloc_page; int ret; @@ -347,7 +366,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; uint32_t page_offset = offset_in_page(reloc->offset); - uint64_t delta = (int)reloc->delta + target_offset; + uint64_t delta = relocation_target(reloc, target_offset); char *vaddr; int ret; @@ -395,7 +414,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, target_i915_obj = target_vma->obj; target_obj = &target_vma->obj->base; - target_offset = target_vma->node.start; + target_offset = gen8_canonical_addr(target_vma->node.start); /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and * pipe_control writes because the gpu doesn't properly redirect them @@ -994,6 +1013,18 @@ validate_exec_list(struct drm_device *dev, if (exec[i].flags & invalid_flags) return -EINVAL; + /* Offset can be used as input (EXEC_OBJECT_PINNED), since + * userspace has to use canonical format, we need to reject all + * non-canonical addresses. + */ + if (exec[i].offset != gen8_canonical_addr(exec[i].offset)) + return -EINVAL; + + /* On the other hand, from drm_mm perspective address space is + * continuous, so we're converting to non-canonical form + */ + exec[i].offset &= (1ULL << 48) - 1; + if (exec[i].alignment && !is_power_of_2(exec[i].alignment)) return -EINVAL; @@ -1617,6 +1648,29 @@ pre_mutex_err: return ret; } +static inline int +__i915_gem_execlist_copy_to_user(struct drm_i915_gem_execbuffer2 *args, + struct drm_i915_gem_exec_object2 *exec2_list) +{ + struct drm_i915_gem_exec_object2 __user *user_exec_list = + to_user_ptr(args->buffers_ptr); + int i; + + for (i = 0; i < args->buffer_count; i++) { + /* Userspace uses addresses in canonical form */ + exec2_list[i].offset = gen8_canonical_addr(exec2_list[i].offset); + if (__copy_to_user(&user_exec_list[i].offset, + &exec2_list[i].offset, + sizeof(user_exec_list[i].offset))) { + DRM_DEBUG("failed to copy %d exec entries back to user\n", + args->buffer_count); + return -EFAULT; + } + } + + return 0; +} + /* * Legacy execbuffer just creates an exec2 list from the original exec object * list array and passes it to the real function. @@ -1681,24 +1735,10 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, i915_execbuffer2_set_context_id(exec2, 0); ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list); - if (!ret) { - struct drm_i915_gem_exec_object __user *user_exec_list = - to_user_ptr(args->buffers_ptr); - - /* Copy the new buffer offsets back to the user's exec list. */ - for (i = 0; i < args->buffer_count; i++) { - ret = __copy_to_user(&user_exec_list[i].offset, - &exec2_list[i].offset, - sizeof(user_exec_list[i].offset)); - if (ret) { - ret = -EFAULT; - DRM_DEBUG("failed to copy %d exec entries " - "back to user (%d)\n", - args->buffer_count, ret); - break; - } - } - } + + /* Copy the new buffer offsets back to the user's exec list. */ + if (!ret) + ret = __i915_gem_execlist_copy_to_user(&exec2, exec2_list); drm_free_large(exec_list); drm_free_large(exec2_list); @@ -1745,25 +1785,10 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, } ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list); - if (!ret) { - /* Copy the new buffer offsets back to the user's exec list. */ - struct drm_i915_gem_exec_object2 __user *user_exec_list = - to_user_ptr(args->buffers_ptr); - int i; - - for (i = 0; i < args->buffer_count; i++) { - ret = __copy_to_user(&user_exec_list[i].offset, - &exec2_list[i].offset, - sizeof(user_exec_list[i].offset)); - if (ret) { - ret = -EFAULT; - DRM_DEBUG("failed to copy %d exec entries " - "back to user\n", - args->buffer_count); - break; - } - } - } + + /* Copy the new buffer offsets back to the user's exec list. */ + if (!ret) + ret = __i915_gem_execlist_copy_to_user(args, exec2_list); drm_free_large(exec2_list); return ret;