From patchwork Thu Jan 23 10:12:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11347215 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4DC59159A for ; Thu, 23 Jan 2020 10:13:18 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 35DA22467B for ; Thu, 23 Jan 2020 10:13:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 35DA22467B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4886B6FBA7; Thu, 23 Jan 2020 10:13:17 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8BC2E6FBA7 for ; Thu, 23 Jan 2020 10:13:15 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 19979854-1500050 for multiple; Thu, 23 Jan 2020 10:12:14 +0000 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Thu, 23 Jan 2020 10:12:12 +0000 Message-Id: <20200123101212.1281264-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.25.0 In-Reply-To: <20200122201822.889250-1-chris@chris-wilson.co.uk> References: <20200122201822.889250-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v2] drm/i915/gt: Poison GTT scratch pages X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 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" Using a clear page for scratch means that we have relatively benign errors in case it is accidentally used, but that can be rather too benign for debugging. If we poison the scratch, ideally it quickly results in an obvious error. v2: Set each page individual just in case we are using highmem for our scratch page. Suggested-by: Mika Kuoppala Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Matthew Auld --- .../drm/i915/gem/selftests/i915_gem_context.c | 48 ++++++++++++++++--- drivers/gpu/drm/i915/gt/intel_gtt.c | 30 ++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c index 7fc46861a54d..e23280dd8a98 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -1575,7 +1575,7 @@ static int read_from_scratch(struct i915_gem_context *ctx, struct drm_i915_private *i915 = ctx->i915; struct drm_i915_gem_object *obj; struct i915_address_space *vm; - const u32 RCS_GPR0 = 0x2600; /* not all engines have their own GPR! */ + const u32 GPR0 = engine->mmio_base + 0x600; const u32 result = 0x100; struct i915_request *rq; struct i915_vma *vma; @@ -1597,19 +1597,19 @@ static int read_from_scratch(struct i915_gem_context *ctx, memset(cmd, POISON_INUSE, PAGE_SIZE); if (INTEL_GEN(i915) >= 8) { *cmd++ = MI_LOAD_REGISTER_MEM_GEN8; - *cmd++ = RCS_GPR0; + *cmd++ = GPR0; *cmd++ = lower_32_bits(offset); *cmd++ = upper_32_bits(offset); *cmd++ = MI_STORE_REGISTER_MEM_GEN8; - *cmd++ = RCS_GPR0; + *cmd++ = GPR0; *cmd++ = result; *cmd++ = 0; } else { *cmd++ = MI_LOAD_REGISTER_MEM; - *cmd++ = RCS_GPR0; + *cmd++ = GPR0; *cmd++ = offset; *cmd++ = MI_STORE_REGISTER_MEM; - *cmd++ = RCS_GPR0; + *cmd++ = GPR0; *cmd++ = result; } *cmd = MI_BATCH_BUFFER_END; @@ -1686,6 +1686,28 @@ static int read_from_scratch(struct i915_gem_context *ctx, return err; } +static int check_scratch_page(struct i915_gem_context *ctx, u32 *out) +{ + struct page *page = ctx_vm(ctx)->scratch[0].base.page; + u32 *vaddr; + int err = 0; + + if (!page) { + pr_err("No scratch page!\n"); + return -EINVAL; + } + + vaddr = kmap(page); + memcpy(out, vaddr, sizeof(*out)); + if (memchr_inv(vaddr, *out, PAGE_SIZE)) { + pr_err("Inconsistent initial state of scratch page!\n"); + err = -EINVAL; + } + kunmap(page); + + return err; +} + static int igt_vm_isolation(void *arg) { struct drm_i915_private *i915 = arg; @@ -1696,6 +1718,7 @@ static int igt_vm_isolation(void *arg) I915_RND_STATE(prng); struct file *file; u64 vm_total; + u32 expected; int err; if (INTEL_GEN(i915) < 7) @@ -1720,12 +1743,21 @@ static int igt_vm_isolation(void *arg) goto out_file; } + /* Read the initial state of the scratch page */ + err = check_scratch_page(ctx_a, &expected); + if (err) + goto out_file; + ctx_b = live_context(i915, file); if (IS_ERR(ctx_b)) { err = PTR_ERR(ctx_b); goto out_file; } + err = check_scratch_page(ctx_b, &expected); + if (err) + goto out_file; + /* We can only test vm isolation, if the vm are distinct */ if (ctx_vm(ctx_a) == ctx_vm(ctx_b)) goto out_file; @@ -1743,6 +1775,10 @@ static int igt_vm_isolation(void *arg) if (!intel_engine_can_store_dword(engine)) continue; + /* Not all engines have their own GPR! */ + if (INTEL_GEN(i915) < 8 && engine->class != RENDER_CLASS) + continue; + while (!__igt_timeout(end_time, NULL)) { u32 value = 0xc5c5c5c5; u64 offset; @@ -1760,7 +1796,7 @@ static int igt_vm_isolation(void *arg) if (err) goto out_file; - if (value) { + if (value != expected) { pr_err("%s: Read %08x from scratch (offset 0x%08x_%08x), after %lu reads!\n", engine->name, value, upper_32_bits(offset), diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index 45d8e0019a8e..bb9a6e638175 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -299,6 +299,25 @@ fill_page_dma(const struct i915_page_dma *p, const u64 val, unsigned int count) kunmap_atomic(memset64(kmap_atomic(p->page), val, count)); } +static void poison_scratch_page(struct page *page, unsigned long size) +{ + if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) + return; + + GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE)); + + do { + void *vaddr; + + vaddr = kmap(page); + memset(vaddr, POISON_FREE, PAGE_SIZE); + kunmap(page); + + page = pfn_to_page(page_to_pfn(page) + 1); + size -= PAGE_SIZE; + } while (size); +} + int setup_scratch_page(struct i915_address_space *vm, gfp_t gfp) { unsigned long size; @@ -331,6 +350,17 @@ int setup_scratch_page(struct i915_address_space *vm, gfp_t gfp) if (unlikely(!page)) goto skip; + /* + * Use a non-zero scratch page for debugging. + * + * We want a value that should be reasonably obvious + * to spot in the error state, while also causing a GPU hang + * if executed. We prefer using a clear page in production, so + * should it ever be accidentally used, the effect should be + * fairly benign. + */ + poison_scratch_page(page, size); + addr = dma_map_page_attrs(vm->dma, page, 0, size, PCI_DMA_BIDIRECTIONAL,