From patchwork Mon Dec 8 22:33:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael H. Nguyen" X-Patchwork-Id: 5459451 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 26EBB9F1D4 for ; Mon, 8 Dec 2014 22:30:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id ABA49200FF for ; Mon, 8 Dec 2014 22:29:55 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id E726120160 for ; Mon, 8 Dec 2014 22:29:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1695F6EC0B; Mon, 8 Dec 2014 14:29:52 -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 33C876EBE7 for ; Mon, 8 Dec 2014 14:29:50 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 08 Dec 2014 14:27:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,540,1413270000"; d="scan'208";a="650547579" Received: from mhn-devubuntu2.fm.intel.com ([10.19.123.25]) by orsmga002.jf.intel.com with ESMTP; 08 Dec 2014 14:29:49 -0800 From: michael.h.nguyen@intel.com To: intel-gfx@lists.freedesktop.org Date: Mon, 8 Dec 2014 14:33:48 -0800 Message-Id: <1418078030-17690-4-git-send-email-michael.h.nguyen@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1418078030-17690-1-git-send-email-michael.h.nguyen@intel.com> References: <1418078030-17690-1-git-send-email-michael.h.nguyen@intel.com> Cc: Brad Volkin Subject: [Intel-gfx] [PATCH v6 3/5] drm/i915: Use batch length instead of object size in command parser 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.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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: Brad Volkin Previously we couldn't trust the user-supplied batch length because it came directly from userspace (i.e. untrusted code). It would have affected what commands software parsed without regard to what hardware would actually execute, leaving a potential hole. With the parser now copying the user supplied batch buffer and writing MI_NOP commands to any space after the copied region, we can safely use the batch length input. This should be a performance win as the actual batch length is frequently much smaller than the allocated object size. v2: Fix handling of non-zero batch_start_offset Issue: VIZ-4719 Signed-off-by: Brad Volkin --- drivers/gpu/drm/i915/i915_cmd_parser.c | 48 ++++++++++++++++++++---------- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 + 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index ac5f5af..118079d 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -842,11 +842,19 @@ finish: /* Returns a vmap'd pointer to dest_obj, which the caller must unmap */ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj, - struct drm_i915_gem_object *src_obj) + struct drm_i915_gem_object *src_obj, + u32 batch_start_offset, + u32 batch_len) { int ret = 0; int needs_clflush = 0; - u32 *src_addr, *dest_addr = NULL; + u32 *src_base, *dest_base = NULL; + u32 *src_addr, *dest_addr; + u32 offset = batch_start_offset / sizeof(*dest_addr); + u32 end = batch_start_offset + batch_len; + + if (end > dest_obj->base.size || end > src_obj->base.size) + return ERR_PTR(-E2BIG); ret = i915_gem_obj_prepare_shmem_read(src_obj, &needs_clflush); if (ret) { @@ -854,15 +862,17 @@ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj, return ERR_PTR(ret); } - src_addr = vmap_batch(src_obj); - if (!src_addr) { + src_base = vmap_batch(src_obj); + if (!src_base) { DRM_DEBUG_DRIVER("CMD: Failed to vmap batch\n"); ret = -ENOMEM; goto unpin_src; } + src_addr = src_base + offset; + if (needs_clflush) - drm_clflush_virt_range((char *)src_addr, src_obj->base.size); + drm_clflush_virt_range((char *)src_addr, batch_len); ret = i915_gem_object_set_to_cpu_domain(dest_obj, true); if (ret) { @@ -870,24 +880,27 @@ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj, goto unmap_src; } - dest_addr = vmap_batch(dest_obj); - if (!dest_addr) { + dest_base = vmap_batch(dest_obj); + if (!dest_base) { DRM_DEBUG_DRIVER("CMD: Failed to vmap shadow batch\n"); ret = -ENOMEM; goto unmap_src; } - memcpy(dest_addr, src_addr, src_obj->base.size); - if (dest_obj->base.size > src_obj->base.size) - memset((u8 *)dest_addr + src_obj->base.size, 0, - dest_obj->base.size - src_obj->base.size); + dest_addr = dest_base + offset; + + if (batch_start_offset != 0) + memset((u8 *)dest_base, 0, batch_start_offset); + + memcpy(dest_addr, src_addr, batch_len); + memset((u8 *)dest_addr + batch_len, 0, dest_obj->base.size - end); unmap_src: - vunmap(src_addr); + vunmap(src_base); unpin_src: i915_gem_object_unpin_pages(src_obj); - return ret ? ERR_PTR(ret) : dest_addr; + return ret ? ERR_PTR(ret) : dest_base; } /** @@ -1008,6 +1021,7 @@ static bool check_cmd(const struct intel_engine_cs *ring, * @batch_obj: the batch buffer in question * @shadow_batch_obj: copy of the batch buffer in question * @batch_start_offset: byte offset in the batch at which execution starts + * @batch_len: length of the commands in batch_obj * @is_master: is the submitting process the drm master? * * Parses the specified batch buffer looking for privilege violations as @@ -1020,6 +1034,7 @@ int i915_parse_cmds(struct intel_engine_cs *ring, struct drm_i915_gem_object *batch_obj, struct drm_i915_gem_object *shadow_batch_obj, u32 batch_start_offset, + u32 batch_len, bool is_master) { int ret = 0; @@ -1027,7 +1042,8 @@ int i915_parse_cmds(struct intel_engine_cs *ring, struct drm_i915_cmd_descriptor default_desc = { 0 }; bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */ - batch_base = copy_batch(shadow_batch_obj, batch_obj); + batch_base = copy_batch(shadow_batch_obj, batch_obj, + batch_start_offset, batch_len); if (IS_ERR(batch_base)) { DRM_DEBUG_DRIVER("CMD: Failed to copy batch\n"); return PTR_ERR(batch_base); @@ -1036,11 +1052,11 @@ int i915_parse_cmds(struct intel_engine_cs *ring, cmd = batch_base + (batch_start_offset / sizeof(*cmd)); /* - * We use the source object's size because the shadow object is as + * We use the batch length as size because the shadow object is as * large or larger and copy_batch() will write MI_NOPs to the extra * space. Parsing should be faster in some cases this way. */ - batch_end = cmd + (batch_obj->base.size / sizeof(*batch_end)); + batch_end = cmd + (batch_len / sizeof(*batch_end)); while (cmd < batch_end) { const struct drm_i915_cmd_descriptor *desc; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6e3d354..de0e002 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2906,6 +2906,7 @@ int i915_parse_cmds(struct intel_engine_cs *ring, struct drm_i915_gem_object *batch_obj, struct drm_i915_gem_object *shadow_batch_obj, u32 batch_start_offset, + u32 batch_len, bool is_master); /* i915_suspend.c */ diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a108184..fccfff5 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1418,6 +1418,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, batch_obj, shadow_batch_obj, args->batch_start_offset, + args->batch_len, file->is_master); i915_gem_object_ggtt_unpin(shadow_batch_obj);