* [Intel-gfx] [PATCH] drm/i915/gt: Add selftests for TLB invalidation
@ 2023-01-16 15:59 Andrzej Hajda
2023-01-16 17:36 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Andrzej Hajda @ 2023-01-16 15:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi, Andrzej Hajda, Chris Wilson
From: Chris Wilson <chris@chris-wilson.co.uk>
Check that we invalidate the TLB cache, the updated physical addresses
are immediately visible to the HW, and there is no retention of the old
physical address for concurrent HW access.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
[ahajda: adjust to upstream driver]
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 1 +
drivers/gpu/drm/i915/gt/intel_gt.c | 4 +
drivers/gpu/drm/i915/gt/selftest_tlb.c | 405 ++++++++++++++++++
.../drm/i915/selftests/i915_live_selftests.h | 1 +
4 files changed, 411 insertions(+)
create mode 100644 drivers/gpu/drm/i915/gt/selftest_tlb.c
diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
index 2af1ae3831df98..e10507fa71ce63 100644
--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
@@ -394,6 +394,7 @@
#define MI_LOAD_URB_MEM MI_INSTR(0x2C, 0)
#define MI_STORE_URB_MEM MI_INSTR(0x2D, 0)
#define MI_CONDITIONAL_BATCH_BUFFER_END MI_INSTR(0x36, 0)
+#define MI_DO_COMPARE REG_BIT(21)
#define STATE_BASE_ADDRESS \
((0x3 << 29) | (0x0 << 27) | (0x1 << 24) | (0x1 << 16))
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 7eeee5a7cb33cb..e6358373eb9c92 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -1196,3 +1196,7 @@ void intel_gt_invalidate_tlb(struct intel_gt *gt, u32 seqno)
mutex_unlock(>->tlb.invalidate_lock);
}
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_tlb.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_tlb.c b/drivers/gpu/drm/i915/gt/selftest_tlb.c
new file mode 100644
index 00000000000000..a5082a70f06a77
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_tlb.c
@@ -0,0 +1,405 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+
+#include "gem/i915_gem_internal.h"
+#include "gem/i915_gem_region.h"
+
+#include "gen8_engine_cs.h"
+#include "i915_gem_ww.h"
+#include "intel_engine_regs.h"
+#include "intel_gpu_commands.h"
+#include "intel_context.h"
+#include "intel_gt.h"
+#include "intel_ring.h"
+
+#include "selftests/igt_flush_test.h"
+#include "selftests/i915_random.h"
+
+static void clear_dw(struct i915_vma *vma, u64 addr, u32 val)
+{
+ GEM_BUG_ON(addr < i915_vma_offset(vma));
+ GEM_BUG_ON(addr >= i915_vma_offset(vma) + i915_vma_size(vma));
+ memset32(page_mask_bits(vma->obj->mm.mapping) +
+ (addr - i915_vma_offset(vma)), val, 1);
+}
+
+static int
+pte_tlbinv(struct intel_context *ce,
+ struct i915_vma *va,
+ struct i915_vma *vb,
+ u64 align,
+ void (*tlbinv)(struct i915_address_space *vm, u64 addr, u64 length),
+ u64 length,
+ struct rnd_state *prng)
+{
+ const int use_64b = GRAPHICS_VER(ce->vm->i915) >= 8;
+ struct drm_i915_gem_object *batch;
+ struct i915_request *rq;
+ struct i915_vma *vma;
+ int retries;
+ u64 addr;
+ int err;
+ u32 *cs;
+
+ batch = i915_gem_object_create_internal(ce->vm->i915, 4096);
+ if (IS_ERR(batch))
+ return PTR_ERR(batch);
+
+ vma = i915_vma_instance(batch, ce->vm, NULL);
+ if (IS_ERR(vma)) {
+ err = PTR_ERR(vma);
+ goto out;
+ }
+
+ err = i915_vma_pin(vma, 0, 0, PIN_USER);
+ if (err)
+ goto out;
+
+ retries = 5;
+ do {
+ addr = igt_random_offset(prng,
+ i915_vma_offset(vma),
+ /* upper limit for MI_BB_START */
+ min(ce->vm->total, BIT_ULL(48)),
+ va->size, 4);
+
+ err = i915_vma_pin(va, 0, 0, (addr & -align) | PIN_OFFSET_FIXED | PIN_USER);
+ } while (err == -ENOSPC && --retries);
+ if (err) {
+ err = 0;
+ goto out;
+ }
+ GEM_BUG_ON(i915_vma_offset(va) != (addr & -align));
+ vb->node = va->node; /* overwrites the _same_ PTE */
+
+ if (align == SZ_64K) {
+ u64 end = addr + va->size;
+
+ /*
+ * SZ_64K pages on dg1 require that the whole PT be marked
+ * containing 64KiB entries. So we make sure that our vma
+ * covers the whole PT, despite being randomly aligned to 64KiB
+ * and restrict our sampling to the 2MiB PT within where
+ * we know that we will be using 64KiB pages.
+ */
+ addr = round_up(addr & -align, SZ_2M);
+ addr |= igt_random_offset(prng, 0, end - addr, 4, 4);
+ }
+
+ if (addr - i915_vma_offset(va) >= i915_vma_size(va))
+ addr = igt_random_offset(prng,
+ i915_vma_offset(va),
+ i915_vma_offset(va) + i915_vma_size(va),
+ 4, 4);
+
+ pr_info("%s(%s): Sampling %llx, with alignment %llx, using PTE size %x (phys %x, sg %x), invalidate:%llx+%llx\n",
+ ce->engine->name, va->obj->mm.region->name ?: "smem",
+ addr, align, va->resource->page_sizes_gtt, va->page_sizes.phys, va->page_sizes.sg,
+ addr & -length, length);
+
+ cs = i915_gem_object_pin_map_unlocked(batch, I915_MAP_WC);
+ *cs++ = MI_NOOP; /* for later termination */
+
+ /* Sample the target to see if we spot an incorrect page */
+ *cs++ = MI_CONDITIONAL_BATCH_BUFFER_END | MI_DO_COMPARE | (1 + use_64b);
+ *cs++ = -2; /* break if *addr < -1 */
+ *cs++ = lower_32_bits(addr);
+ *cs++ = upper_32_bits(addr);
+ clear_dw(va, addr, -1);
+ clear_dw(vb, addr, 0);
+
+ /* Keep sampling until we get bored */
+ *cs++ = MI_BATCH_BUFFER_START | BIT(8) | use_64b;
+ *cs++ = lower_32_bits(i915_vma_offset(vma));
+ *cs++ = upper_32_bits(i915_vma_offset(vma));
+
+ i915_gem_object_flush_map(batch);
+
+ rq = i915_request_create(ce);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto out_va;
+ }
+
+ err = rq->engine->emit_bb_start(rq, i915_vma_offset(vma), 0, 0);
+ if (err) {
+ i915_request_add(rq);
+ goto out_va;
+ }
+
+ i915_request_get(rq);
+ i915_request_add(rq);
+
+ /* Short sleep to sanitycheck the batch is spinning before we begin */
+ msleep(10);
+ if (va == vb) {
+ if (!i915_request_completed(rq)) {
+ pr_err("Semaphore sanitycheck failed\n");
+ err = -EIO;
+ }
+ } else if (!i915_request_completed(rq)) {
+ struct i915_vma_resource vb_res = {
+ .bi.pages = vb->obj->mm.pages,
+ .bi.page_sizes = vb->obj->mm.page_sizes,
+ .start = i915_vma_offset(vb),
+ .vma_size = i915_vma_size(vb)
+ };
+ unsigned int pte_flags = 0;
+
+ /* Flip the PTE between A and B */
+ if (i915_gem_object_is_lmem(vb->obj))
+ pte_flags |= PTE_LM;
+ ce->vm->insert_entries(ce->vm, &vb_res, 0, pte_flags);
+
+ /* Flush the PTE update to concurrent HW */
+ tlbinv(ce->vm, addr & -length, length);
+
+ if (wait_for(i915_request_completed(rq), HZ / 2)) {
+ pr_err("%s: Request did not complete; the COND_BBE did not read the updated PTE\n",
+ ce->engine->name);
+ err = -EINVAL;
+ }
+ } else {
+ pr_err("Spinner sanitycheck failed\n");
+ err = -EIO;
+ }
+ i915_request_put(rq);
+
+ cs = page_mask_bits(batch->mm.mapping);
+ *cs = MI_BATCH_BUFFER_END;
+ wmb();
+
+out_va:
+ if (vb != va)
+ memset(&vb->node, 0, sizeof(vb->node));
+ i915_vma_unpin(va);
+ if (i915_vma_unbind_unlocked(va))
+ err = -EIO;
+out:
+ i915_gem_object_put(batch);
+ return err;
+}
+
+static struct drm_i915_gem_object *create_lmem(struct intel_gt *gt)
+{
+ return i915_gem_object_create_lmem(gt->i915, SZ_1G, I915_BO_ALLOC_CONTIGUOUS);
+}
+
+static struct drm_i915_gem_object *create_smem(struct intel_gt *gt)
+{
+ /*
+ * SZ_64K pages require covering the whole 2M PT (gen8 to tgl/dg1).
+ * While that does not require the whole 2M block to be contiguous
+ * it is easier to make it so, since we need that for SZ_2M pagees.
+ * Since we randomly offset the start of the vma, we need a 4M object
+ * so that there is a 2M range within it is suitable for SZ_64K PTE.
+ */
+ return i915_gem_object_create_internal(gt->i915, SZ_4M);
+}
+
+static int
+mem_tlbinv(struct intel_gt *gt,
+ struct drm_i915_gem_object *(*create_fn)(struct intel_gt *),
+ void (*tlbinv)(struct i915_address_space *vm, u64 addr, u64 length))
+{
+ struct intel_engine_cs *engine;
+ struct drm_i915_gem_object *A, *B;
+ struct i915_ppgtt *ppgtt;
+ struct i915_vma *va, *vb;
+ enum intel_engine_id id;
+ I915_RND_STATE(prng);
+ LIST_HEAD(discard);
+ void *vaddr;
+ int err;
+
+ if (GRAPHICS_VER(gt->i915) < 6) /* MI_CONDITIONAL_BB_END & bcs */
+ return 0;
+
+ /*
+ * Check that the TLB invalidate is able to revoke an active
+ * page. We load a page into a spinning COND_BBE loop and then
+ * remap that page to a new physical address. The old address, and
+ * so the loop keeps spinning, is retained in the TLB cache until
+ * we issue an invalidate.
+ */
+
+ A = create_fn(gt);
+ if (IS_ERR(A))
+ return PTR_ERR(A);
+
+ vaddr = i915_gem_object_pin_map_unlocked(A, I915_MAP_WC);
+ if (IS_ERR(vaddr)) {
+ err = PTR_ERR(vaddr);
+ goto out_a;
+ }
+
+ /* Allocate a second physical address significantly different from A */
+ do {
+ B = create_fn(gt);
+ if (IS_ERR(B)) {
+ err = PTR_ERR(B);
+ goto out_a;
+ }
+
+ err = i915_gem_object_pin_pages_unlocked(B);
+ if (err)
+ goto out_b;
+
+ if (upper_32_bits(i915_gem_object_get_dma_address(A, 0)) !=
+ upper_32_bits(i915_gem_object_get_dma_address(B, 0)))
+ break;
+
+ list_add(&B->st_link, &discard);
+ } while (1);
+
+ vaddr = i915_gem_object_pin_map_unlocked(B, I915_MAP_WC);
+ if (IS_ERR(vaddr)) {
+ err = PTR_ERR(vaddr);
+ goto out_b;
+ }
+
+ GEM_BUG_ON(A->base.size != B->base.size);
+ if ((A->mm.page_sizes.phys | B->mm.page_sizes.phys) & (A->base.size - 1))
+ pr_warn("Failed to allocate contiguous pages for size %zx\n",
+ A->base.size);
+
+ ppgtt = i915_ppgtt_create(gt, 0);
+ if (IS_ERR(ppgtt)) {
+ err = PTR_ERR(ppgtt);
+ goto out_b;
+ }
+
+ va = i915_vma_instance(A, &ppgtt->vm, NULL);
+ if (IS_ERR(va)) {
+ err = PTR_ERR(va);
+ goto out_vm;
+ }
+
+ vb = i915_vma_instance(B, &ppgtt->vm, NULL);
+ if (IS_ERR(vb)) {
+ err = PTR_ERR(vb);
+ goto out_vm;
+ }
+
+ err = 0;
+ for_each_engine(engine, gt, id) {
+ struct i915_gem_ww_ctx ww;
+ struct intel_context *ce;
+ int bit;
+
+ ce = intel_context_create(engine);
+ if (IS_ERR(ce)) {
+ err = PTR_ERR(ce);
+ break;
+ }
+
+ i915_vm_put(ce->vm);
+ ce->vm = i915_vm_get(&ppgtt->vm);
+
+ for_i915_gem_ww(&ww, err, true)
+ err = intel_context_pin_ww(ce, &ww);
+ if (err == 0) {
+ for_each_set_bit(bit,
+ (unsigned long *)&RUNTIME_INFO(gt->i915)->page_sizes,
+ BITS_PER_TYPE(RUNTIME_INFO(gt->i915)->page_sizes)) {
+ int len;
+
+ /* sanitycheck the semaphore wake up */
+ err = pte_tlbinv(ce, va, va,
+ BIT_ULL(bit),
+ NULL, SZ_4K,
+ &prng);
+ if (err)
+ goto err_unpin;
+
+ for (len = 2; len <= RUNTIME_INFO(gt->i915)->ppgtt_size; len *= 2) {
+ err = pte_tlbinv(ce, va, vb,
+ BIT_ULL(bit),
+ tlbinv,
+ BIT_ULL(len),
+ &prng);
+ if (err)
+ goto err_unpin;
+ }
+
+ if (len != RUNTIME_INFO(gt->i915)->ppgtt_size) {
+ len = RUNTIME_INFO(gt->i915)->ppgtt_size;
+ err = pte_tlbinv(ce, va, vb,
+ BIT_ULL(bit),
+ tlbinv,
+ BIT_ULL(len),
+ &prng);
+ if (err)
+ goto err_unpin;
+ }
+ }
+err_unpin:
+ intel_context_unpin(ce);
+ }
+
+ intel_context_put(ce);
+ if (err)
+ break;
+ }
+
+ if (igt_flush_test(gt->i915))
+ err = -EIO;
+
+out_vm:
+ i915_vm_put(&ppgtt->vm);
+out_b:
+ i915_gem_object_put(B);
+out_a:
+ i915_gem_object_put(A);
+ list_for_each_entry_safe(A, B, &discard, st_link)
+ i915_gem_object_put(A);
+ return err;
+}
+
+static void tlbinv_full(struct i915_address_space *vm, u64 addr, u64 length)
+{
+ intel_gt_invalidate_tlb(vm->gt, intel_gt_tlb_seqno(vm->gt) | 1);
+}
+
+static int invalidate_full(void *arg)
+{
+ struct intel_gt *gt = arg;
+ int err;
+
+ if (GRAPHICS_VER(gt->i915) < 8)
+ return 0; /* TLB invalidate not implemented */
+
+ err = mem_tlbinv(gt, create_smem, tlbinv_full);
+ if (err == 0)
+ err = mem_tlbinv(gt, create_lmem, tlbinv_full);
+ if (err == -ENODEV || err == -ENXIO)
+ err = 0;
+
+ return err;
+}
+
+int intel_tlb_live_selftests(struct drm_i915_private *i915)
+{
+ static const struct i915_subtest tests[] = {
+ SUBTEST(invalidate_full),
+ };
+ struct intel_gt *gt;
+ unsigned int i;
+
+ for_each_gt(gt, i915, i) {
+ int err;
+
+ if (intel_gt_is_wedged(gt))
+ continue;
+
+ err = intel_gt_live_subtests(tests, gt);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index aaf8a380e5c789..5aee6c9a8295ce 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -25,6 +25,7 @@ selftest(gt_lrc, intel_lrc_live_selftests)
selftest(gt_mocs, intel_mocs_live_selftests)
selftest(gt_pm, intel_gt_pm_live_selftests)
selftest(gt_heartbeat, intel_heartbeat_live_selftests)
+selftest(gt_tlb, intel_tlb_live_selftests)
selftest(requests, i915_request_live_selftests)
selftest(migrate, intel_migrate_live_selftests)
selftest(active, i915_active_live_selftests)
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Add selftests for TLB invalidation
2023-01-16 15:59 [Intel-gfx] [PATCH] drm/i915/gt: Add selftests for TLB invalidation Andrzej Hajda
@ 2023-01-16 17:36 ` Patchwork
2023-01-16 19:27 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-01-17 10:51 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2023-01-16 17:36 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 10554 bytes --]
== Series Details ==
Series: drm/i915/gt: Add selftests for TLB invalidation
URL : https://patchwork.freedesktop.org/series/112894/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12587 -> Patchwork_112894v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/index.html
Participating hosts (41 -> 41)
------------------------------
Additional (3): fi-ctg-p8600 fi-rkl-11600 bat-adls-5
Missing (3): bat-rpls-2 bat-atsm-1 fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_112894v1:
### IGT changes ###
#### Possible regressions ####
* {igt@i915_selftest@live@gt_tlb} (NEW):
- {bat-dg2-11}: NOTRUN -> [DMESG-FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/bat-dg2-11/igt@i915_selftest@live@gt_tlb.html
- bat-dg1-6: NOTRUN -> [DMESG-FAIL][2]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/bat-dg1-6/igt@i915_selftest@live@gt_tlb.html
- fi-bsw-nick: NOTRUN -> [INCOMPLETE][3]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-bsw-nick/igt@i915_selftest@live@gt_tlb.html
- fi-bsw-kefka: NOTRUN -> [DMESG-FAIL][4]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-bsw-kefka/igt@i915_selftest@live@gt_tlb.html
- fi-bsw-n3050: NOTRUN -> [DMESG-FAIL][5]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-bsw-n3050/igt@i915_selftest@live@gt_tlb.html
- bat-dg1-5: NOTRUN -> [DMESG-FAIL][6]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/bat-dg1-5/igt@i915_selftest@live@gt_tlb.html
- {bat-dg1-7}: NOTRUN -> [DMESG-FAIL][7]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/bat-dg1-7/igt@i915_selftest@live@gt_tlb.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@i915_selftest@live@gt_engines:
- {bat-dg2-8}: [PASS][8] -> [FAIL][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/bat-dg2-8/igt@i915_selftest@live@gt_engines.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/bat-dg2-8/igt@i915_selftest@live@gt_engines.html
* igt@i915_selftest@live@migrate:
- {bat-dg2-11}: [PASS][10] -> [DMESG-WARN][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/bat-dg2-11/igt@i915_selftest@live@migrate.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/bat-dg2-11/igt@i915_selftest@live@migrate.html
New tests
---------
New tests have been introduced between CI_DRM_12587 and Patchwork_112894v1:
### New IGT tests (1) ###
* igt@i915_selftest@live@gt_tlb:
- Statuses : 6 dmesg-fail(s) 1 incomplete(s) 32 pass(s)
- Exec time: [0.0] s
Known issues
------------
Here are the changes found in Patchwork_112894v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@debugfs_test@basic-hwmon:
- fi-rkl-11600: NOTRUN -> [SKIP][12] ([i915#7456])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@debugfs_test@basic-hwmon.html
* igt@gem_huc_copy@huc-copy:
- fi-rkl-11600: NOTRUN -> [SKIP][13] ([i915#2190])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-rkl-11600: NOTRUN -> [SKIP][14] ([i915#4613]) +3 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@gem_lmem_swapping@basic.html
* igt@gem_tiled_pread_basic:
- fi-rkl-11600: NOTRUN -> [SKIP][15] ([i915#3282])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@gem_tiled_pread_basic.html
* igt@i915_pm_backlight@basic-brightness:
- fi-rkl-11600: NOTRUN -> [SKIP][16] ([i915#7561])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html
* igt@i915_suspend@basic-s3-without-i915:
- fi-rkl-11600: NOTRUN -> [INCOMPLETE][17] ([i915#4817])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_chamelium_frames@dp-crc-fast:
- fi-ctg-p8600: NOTRUN -> [SKIP][18] ([fdo#109271]) +31 similar issues
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-ctg-p8600/igt@kms_chamelium_frames@dp-crc-fast.html
* igt@kms_chamelium_hpd@dp-hpd-fast:
- fi-rkl-11600: NOTRUN -> [SKIP][19] ([i915#7828]) +7 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@kms_chamelium_hpd@dp-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
- fi-rkl-11600: NOTRUN -> [SKIP][20] ([i915#4103])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
- fi-bsw-n3050: [PASS][21] -> [FAIL][22] ([i915#6298])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
* igt@kms_force_connector_basic@force-load-detect:
- fi-rkl-11600: NOTRUN -> [SKIP][23] ([fdo#109285] / [i915#4098])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_psr@primary_page_flip:
- fi-rkl-11600: NOTRUN -> [SKIP][24] ([i915#1072]) +3 similar issues
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@kms_psr@primary_page_flip.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-rkl-11600: NOTRUN -> [SKIP][25] ([i915#3555] / [i915#4098])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-read:
- fi-rkl-11600: NOTRUN -> [SKIP][26] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@prime_vgem@basic-read.html
* igt@prime_vgem@basic-userptr:
- fi-rkl-11600: NOTRUN -> [SKIP][27] ([fdo#109295] / [i915#3301] / [i915#3708])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-rkl-11600/igt@prime_vgem@basic-userptr.html
#### Possible fixes ####
* igt@i915_pm_rpm@basic-rte:
- {bat-adlp-6}: [DMESG-WARN][28] ([i915#7077]) -> [PASS][29]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/bat-adlp-6/igt@i915_pm_rpm@basic-rte.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/bat-adlp-6/igt@i915_pm_rpm@basic-rte.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [DMESG-FAIL][30] ([i915#5334]) -> [PASS][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_suspend@basic-s3-without-i915:
- fi-apl-guc: [DMESG-WARN][32] ([i915#1982]) -> [PASS][33]
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/fi-apl-guc/igt@i915_suspend@basic-s3-without-i915.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/fi-apl-guc/igt@i915_suspend@basic-s3-without-i915.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
[i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#7077]: https://gitlab.freedesktop.org/drm/intel/issues/7077
[i915#7443]: https://gitlab.freedesktop.org/drm/intel/issues/7443
[i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7834]: https://gitlab.freedesktop.org/drm/intel/issues/7834
Build changes
-------------
* Linux: CI_DRM_12587 -> Patchwork_112894v1
CI-20190529: 20190529
CI_DRM_12587: e4d10a72766332a0ca94db1fb9b4b2aa8b319172 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7120: dffabf00c79c55e0ae23b75d0a7922d55251ee5e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_112894v1: e4d10a72766332a0ca94db1fb9b4b2aa8b319172 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
5b3f563d8e8a drm/i915/gt: Add selftests for TLB invalidation
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/index.html
[-- Attachment #2: Type: text/html, Size: 11697 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: Add selftests for TLB invalidation
2023-01-16 15:59 [Intel-gfx] [PATCH] drm/i915/gt: Add selftests for TLB invalidation Andrzej Hajda
2023-01-16 17:36 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
@ 2023-01-16 19:27 ` Patchwork
2023-01-17 10:51 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2023-01-16 19:27 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 20114 bytes --]
== Series Details ==
Series: drm/i915/gt: Add selftests for TLB invalidation
URL : https://patchwork.freedesktop.org/series/112894/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_12587_full -> Patchwork_112894v1_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_112894v1_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_112894v1_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/index.html
Participating hosts (13 -> 10)
------------------------------
Missing (3): pig-skl-6260u pig-kbl-iris pig-glk-j5005
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_112894v1_full:
### IGT changes ###
#### Possible regressions ####
* {igt@i915_selftest@live@gt_tlb} (NEW):
- {shard-dg1}: NOTRUN -> [DMESG-FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-dg1-17/igt@i915_selftest@live@gt_tlb.html
* igt@kms_ccs@pipe-a-crc-primary-basic-y_tiled_ccs:
- shard-glk: [PASS][2] -> [FAIL][3]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-glk7/igt@kms_ccs@pipe-a-crc-primary-basic-y_tiled_ccs.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk8/igt@kms_ccs@pipe-a-crc-primary-basic-y_tiled_ccs.html
New tests
---------
New tests have been introduced between CI_DRM_12587_full and Patchwork_112894v1_full:
### New IGT tests (1) ###
* igt@i915_selftest@live@gt_tlb:
- Statuses : 1 dmesg-fail(s) 3 pass(s)
- Exec time: [0.0] s
Known issues
------------
Here are the changes found in Patchwork_112894v1_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_lmem_swapping@heavy-verify-random:
- shard-glk: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk3/igt@gem_lmem_swapping@heavy-verify-random.html
* igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs:
- shard-glk: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#3886]) +3 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk3/igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs.html
* igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size:
- shard-glk: [PASS][6] -> [FAIL][7] ([i915#2346])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-glk6/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk3/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt:
- shard-glk: NOTRUN -> [SKIP][8] ([fdo#109271]) +35 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt.html
* igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area:
- shard-glk: NOTRUN -> [SKIP][9] ([fdo#109271] / [i915#658])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk3/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html
#### Possible fixes ####
* igt@drm_fdinfo@virtual-idle:
- {shard-rkl}: [FAIL][10] ([i915#7742]) -> [PASS][11] +1 similar issue
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-2/igt@drm_fdinfo@virtual-idle.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@drm_fdinfo@virtual-idle.html
* igt@fbdev@pan:
- {shard-rkl}: [SKIP][12] ([i915#2582]) -> [PASS][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@fbdev@pan.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@fbdev@pan.html
* igt@gem_exec_fair@basic-deadline:
- shard-glk: [FAIL][14] ([i915#2846]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-glk1/igt@gem_exec_fair@basic-deadline.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk2/igt@gem_exec_fair@basic-deadline.html
* igt@gem_exec_fair@basic-none-vip@rcs0:
- {shard-rkl}: [FAIL][16] ([i915#2842]) -> [PASS][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@gem_exec_fair@basic-none-vip@rcs0.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-5/igt@gem_exec_fair@basic-none-vip@rcs0.html
* igt@gem_exec_reloc@basic-write-read-active:
- {shard-rkl}: [SKIP][18] ([i915#3281]) -> [PASS][19] +7 similar issues
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-3/igt@gem_exec_reloc@basic-write-read-active.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-5/igt@gem_exec_reloc@basic-write-read-active.html
* igt@gem_mmap_gtt@coherency:
- {shard-rkl}: [SKIP][20] ([fdo#111656]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@gem_mmap_gtt@coherency.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-5/igt@gem_mmap_gtt@coherency.html
* igt@gem_set_tiling_vs_pwrite:
- {shard-rkl}: [SKIP][22] ([i915#3282]) -> [PASS][23] +6 similar issues
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@gem_set_tiling_vs_pwrite.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-5/igt@gem_set_tiling_vs_pwrite.html
* igt@gen9_exec_parse@unaligned-jump:
- {shard-rkl}: [SKIP][24] ([i915#2527]) -> [PASS][25]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@gen9_exec_parse@unaligned-jump.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-5/igt@gen9_exec_parse@unaligned-jump.html
* igt@i915_pm_dc@dc6-psr:
- {shard-rkl}: [SKIP][26] ([i915#658]) -> [PASS][27]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@i915_pm_dc@dc6-psr.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@i915_pm_dc@dc6-psr.html
* igt@i915_pm_rc6_residency@rc6-idle@rcs0:
- {shard-dg1}: [FAIL][28] ([i915#3591]) -> [PASS][29]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-dg1-19/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-dg1-14/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html
* igt@i915_pm_rpm@drm-resources-equal:
- {shard-rkl}: [SKIP][30] ([fdo#109308]) -> [PASS][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-2/igt@i915_pm_rpm@drm-resources-equal.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@i915_pm_rpm@drm-resources-equal.html
* igt@kms_ccs@pipe-b-bad-aux-stride-y_tiled_gen12_rc_ccs:
- {shard-rkl}: [SKIP][32] ([i915#1845] / [i915#4098]) -> [PASS][33] +21 similar issues
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-2/igt@kms_ccs@pipe-b-bad-aux-stride-y_tiled_gen12_rc_ccs.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@kms_ccs@pipe-b-bad-aux-stride-y_tiled_gen12_rc_ccs.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a1:
- shard-glk: [FAIL][34] ([i915#79]) -> [PASS][35]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-glk2/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a1.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk4/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a1.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt:
- {shard-rkl}: [SKIP][36] ([i915#1849] / [i915#4098]) -> [PASS][37] +17 similar issues
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt.html
* igt@kms_hdmi_inject@inject-audio:
- {shard-rkl}: [SKIP][38] ([i915#433]) -> [PASS][39]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-3/igt@kms_hdmi_inject@inject-audio.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-2/igt@kms_hdmi_inject@inject-audio.html
* igt@kms_psr@primary_mmap_cpu:
- {shard-rkl}: [SKIP][40] ([i915#1072]) -> [PASS][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@kms_psr@primary_mmap_cpu.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@kms_psr@primary_mmap_cpu.html
* igt@perf@gen12-mi-rpc:
- {shard-rkl}: [SKIP][42] ([fdo#109289]) -> [PASS][43] +1 similar issue
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-5/igt@perf@gen12-mi-rpc.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-1/igt@perf@gen12-mi-rpc.html
* igt@perf@stress-open-close:
- shard-glk: [INCOMPLETE][44] ([i915#5213]) -> [PASS][45]
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-glk6/igt@perf@stress-open-close.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-glk3/igt@perf@stress-open-close.html
* igt@perf_pmu@idle@rcs0:
- {shard-rkl}: [FAIL][46] ([i915#4349]) -> [PASS][47]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-1/igt@perf_pmu@idle@rcs0.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-5/igt@perf_pmu@idle@rcs0.html
* igt@testdisplay:
- {shard-rkl}: [SKIP][48] ([i915#4098]) -> [PASS][49] +1 similar issue
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12587/shard-rkl-2/igt@testdisplay.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/shard-rkl-6/igt@testdisplay.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
[fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
[fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
[fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#109314]: https://bugs.freedesktop.org/show_bug.cgi?id=109314
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
[fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
[fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
[fdo#111656]: https://bugs.freedesktop.org/show_bug.cgi?id=111656
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
[i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
[i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2434]: https://gitlab.freedesktop.org/drm/intel/issues/2434
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
[i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
[i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
[i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
[i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
[i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
[i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
[i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
[i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
[i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
[i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3528]: https://gitlab.freedesktop.org/drm/intel/issues/3528
[i915#3536]: https://gitlab.freedesktop.org/drm/intel/issues/3536
[i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
[i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
[i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
[i915#3825]: https://gitlab.freedesktop.org/drm/intel/issues/3825
[i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
[i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
[i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
[i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
[i915#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#433]: https://gitlab.freedesktop.org/drm/intel/issues/433
[i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
[i915#4877]: https://gitlab.freedesktop.org/drm/intel/issues/4877
[i915#5030]: https://gitlab.freedesktop.org/drm/intel/issues/5030
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
[i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
[i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
[i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
[i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#6230]: https://gitlab.freedesktop.org/drm/intel/issues/6230
[i915#6245]: https://gitlab.freedesktop.org/drm/intel/issues/6245
[i915#6247]: https://gitlab.freedesktop.org/drm/intel/issues/6247
[i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
[i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
[i915#6334]: https://gitlab.freedesktop.org/drm/intel/issues/6334
[i915#6335]: https://gitlab.freedesktop.org/drm/intel/issues/6335
[i915#6355]: https://gitlab.freedesktop.org/drm/intel/issues/6355
[i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
[i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
[i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
[i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
[i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
[i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
[i915#7037]: https://gitlab.freedesktop.org/drm/intel/issues/7037
[i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
[i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
[i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
[i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
[i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
[i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
Build changes
-------------
* Linux: CI_DRM_12587 -> Patchwork_112894v1
* Piglit: piglit_4509 -> None
CI-20190529: 20190529
CI_DRM_12587: e4d10a72766332a0ca94db1fb9b4b2aa8b319172 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7120: dffabf00c79c55e0ae23b75d0a7922d55251ee5e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_112894v1: e4d10a72766332a0ca94db1fb9b4b2aa8b319172 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112894v1/index.html
[-- Attachment #2: Type: text/html, Size: 14723 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Add selftests for TLB invalidation
2023-01-16 15:59 [Intel-gfx] [PATCH] drm/i915/gt: Add selftests for TLB invalidation Andrzej Hajda
2023-01-16 17:36 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2023-01-16 19:27 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2023-01-17 10:51 ` Tvrtko Ursulin
2023-01-24 15:39 ` Andrzej Hajda
2 siblings, 1 reply; 5+ messages in thread
From: Tvrtko Ursulin @ 2023-01-17 10:51 UTC (permalink / raw)
To: Andrzej Hajda, intel-gfx; +Cc: Rodrigo Vivi, Chris Wilson
Hi,
Thanks for sending this on! Lets see below..
On 16/01/2023 15:59, Andrzej Hajda wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> Check that we invalidate the TLB cache, the updated physical addresses
> are immediately visible to the HW, and there is no retention of the old
> physical address for concurrent HW access.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> [ahajda: adjust to upstream driver]
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 1 +
> drivers/gpu/drm/i915/gt/intel_gt.c | 4 +
> drivers/gpu/drm/i915/gt/selftest_tlb.c | 405 ++++++++++++++++++
> .../drm/i915/selftests/i915_live_selftests.h | 1 +
> 4 files changed, 411 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/gt/selftest_tlb.c
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
> index 2af1ae3831df98..e10507fa71ce63 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
> @@ -394,6 +394,7 @@
> #define MI_LOAD_URB_MEM MI_INSTR(0x2C, 0)
> #define MI_STORE_URB_MEM MI_INSTR(0x2D, 0)
> #define MI_CONDITIONAL_BATCH_BUFFER_END MI_INSTR(0x36, 0)
> +#define MI_DO_COMPARE REG_BIT(21)
>
> #define STATE_BASE_ADDRESS \
> ((0x3 << 29) | (0x0 << 27) | (0x1 << 24) | (0x1 << 16))
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 7eeee5a7cb33cb..e6358373eb9c92 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -1196,3 +1196,7 @@ void intel_gt_invalidate_tlb(struct intel_gt *gt, u32 seqno)
> mutex_unlock(>->tlb.invalidate_lock);
> }
> }
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftest_tlb.c"
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/selftest_tlb.c b/drivers/gpu/drm/i915/gt/selftest_tlb.c
> new file mode 100644
> index 00000000000000..a5082a70f06a77
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_tlb.c
> @@ -0,0 +1,405 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "i915_selftest.h"
> +
> +#include "gem/i915_gem_internal.h"
> +#include "gem/i915_gem_region.h"
> +
> +#include "gen8_engine_cs.h"
> +#include "i915_gem_ww.h"
> +#include "intel_engine_regs.h"
> +#include "intel_gpu_commands.h"
> +#include "intel_context.h"
> +#include "intel_gt.h"
> +#include "intel_ring.h"
> +
> +#include "selftests/igt_flush_test.h"
> +#include "selftests/i915_random.h"
> +
> +static void clear_dw(struct i915_vma *vma, u64 addr, u32 val)
> +{
> + GEM_BUG_ON(addr < i915_vma_offset(vma));
> + GEM_BUG_ON(addr >= i915_vma_offset(vma) + i915_vma_size(vma));
> + memset32(page_mask_bits(vma->obj->mm.mapping) +
> + (addr - i915_vma_offset(vma)), val, 1);
> +}
> +
> +static int
> +pte_tlbinv(struct intel_context *ce,
> + struct i915_vma *va,
> + struct i915_vma *vb,
> + u64 align,
> + void (*tlbinv)(struct i915_address_space *vm, u64 addr, u64 length),
> + u64 length,
> + struct rnd_state *prng)
> +{
> + const int use_64b = GRAPHICS_VER(ce->vm->i915) >= 8;
> + struct drm_i915_gem_object *batch;
> + struct i915_request *rq;
> + struct i915_vma *vma;
> + int retries;
> + u64 addr;
> + int err;
> + u32 *cs;
> +
> + batch = i915_gem_object_create_internal(ce->vm->i915, 4096);
> + if (IS_ERR(batch))
> + return PTR_ERR(batch);
> +
> + vma = i915_vma_instance(batch, ce->vm, NULL);
> + if (IS_ERR(vma)) {
> + err = PTR_ERR(vma);
> + goto out;
> + }
> +
> + err = i915_vma_pin(vma, 0, 0, PIN_USER);
> + if (err)
> + goto out;
> +
> + retries = 5;
> + do {
> + addr = igt_random_offset(prng,
> + i915_vma_offset(vma),
Why start is set to i915_vma_offset(vma) ?
> + /* upper limit for MI_BB_START */
> + min(ce->vm->total, BIT_ULL(48)),
> + va->size, 4);
> +
> + err = i915_vma_pin(va, 0, 0, (addr & -align) | PIN_OFFSET_FIXED | PIN_USER);
> + } while (err == -ENOSPC && --retries);
> + if (err) {
> + err = 0;
> + goto out;
> + }
This loop should basically never fail since the VM is pristine and
local, right? It can only clash with the the batch buffer so I am
wondering if it would be better not to eat the error condition.
Or evicting previous engine runs does not work? But then five retries
wouldn't be enough..
> + GEM_BUG_ON(i915_vma_offset(va) != (addr & -align));
> + vb->node = va->node; /* overwrites the _same_ PTE */
> +
> + if (align == SZ_64K) {
> + u64 end = addr + va->size;
> +
> + /*
> + * SZ_64K pages on dg1 require that the whole PT be marked
> + * containing 64KiB entries. So we make sure that our vma
> + * covers the whole PT, despite being randomly aligned to 64KiB
> + * and restrict our sampling to the 2MiB PT within where
> + * we know that we will be using 64KiB pages.
> + */
> + addr = round_up(addr & -align, SZ_2M);
> + addr |= igt_random_offset(prng, 0, end - addr, 4, 4);
I don't quite get what this is for. Can it even be that PTs are not all
SZ_64K when the backing store was verified to be contigous already?
> + }
> +
> + if (addr - i915_vma_offset(va) >= i915_vma_size(va))
> + addr = igt_random_offset(prng,
> + i915_vma_offset(va),
> + i915_vma_offset(va) + i915_vma_size(va),
> + 4, 4);
What's this for? Seemingly can override all the "complicated" placement
calculations from above.
> +
> + pr_info("%s(%s): Sampling %llx, with alignment %llx, using PTE size %x (phys %x, sg %x), invalidate:%llx+%llx\n",
> + ce->engine->name, va->obj->mm.region->name ?: "smem",
> + addr, align, va->resource->page_sizes_gtt, va->page_sizes.phys, va->page_sizes.sg,
> + addr & -length, length);
> +
> + cs = i915_gem_object_pin_map_unlocked(batch, I915_MAP_WC);
> + *cs++ = MI_NOOP; /* for later termination */
> +
> + /* Sample the target to see if we spot an incorrect page */
> + *cs++ = MI_CONDITIONAL_BATCH_BUFFER_END | MI_DO_COMPARE | (1 + use_64b);
> + *cs++ = -2; /* break if *addr < -1 */
> + *cs++ = lower_32_bits(addr);
> + *cs++ = upper_32_bits(addr);
> + clear_dw(va, addr, -1);
> + clear_dw(vb, addr, 0);
Is the "break if *addr < -1" comment correct? Because in the sanity
check case where va == vb, we have 0 in the target dword - and code
implies that exists the batch.
> +
> + /* Keep sampling until we get bored */
> + *cs++ = MI_BATCH_BUFFER_START | BIT(8) | use_64b;
Strange that we don't have MI_BATCH_PPGTT for Gen8+.
> + *cs++ = lower_32_bits(i915_vma_offset(vma));
> + *cs++ = upper_32_bits(i915_vma_offset(vma));
> +
> + i915_gem_object_flush_map(batch);
> +
> + rq = i915_request_create(ce);
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + goto out_va;
> + }
> +
> + err = rq->engine->emit_bb_start(rq, i915_vma_offset(vma), 0, 0);
> + if (err) {
> + i915_request_add(rq);
> + goto out_va;
> + }
> +
> + i915_request_get(rq);
> + i915_request_add(rq);
> +
> + /* Short sleep to sanitycheck the batch is spinning before we begin */
> + msleep(10);
> + if (va == vb) {
> + if (!i915_request_completed(rq)) {
> + pr_err("Semaphore sanitycheck failed\n");
> + err = -EIO;
> + }
> + } else if (!i915_request_completed(rq)) {
> + struct i915_vma_resource vb_res = {
> + .bi.pages = vb->obj->mm.pages,
> + .bi.page_sizes = vb->obj->mm.page_sizes,
> + .start = i915_vma_offset(vb),
> + .vma_size = i915_vma_size(vb)
> + };
> + unsigned int pte_flags = 0;
> +
> + /* Flip the PTE between A and B */
> + if (i915_gem_object_is_lmem(vb->obj))
> + pte_flags |= PTE_LM;
> + ce->vm->insert_entries(ce->vm, &vb_res, 0, pte_flags);
> +
> + /* Flush the PTE update to concurrent HW */
> + tlbinv(ce->vm, addr & -length, length);
> +
> + if (wait_for(i915_request_completed(rq), HZ / 2)) {
> + pr_err("%s: Request did not complete; the COND_BBE did not read the updated PTE\n",
> + ce->engine->name);
> + err = -EINVAL;
> + }
> + } else {
> + pr_err("Spinner sanitycheck failed\n");
> + err = -EIO;
This doesn't appear to be the sanity check branch, as referred in
mem_tlbinv, but more like batch unexpectedly terminated in the full test
scenario.
> + }
> + i915_request_put(rq);
> +
> + cs = page_mask_bits(batch->mm.mapping);
> + *cs = MI_BATCH_BUFFER_END;
> + wmb();
> +
> +out_va:
> + if (vb != va)
> + memset(&vb->node, 0, sizeof(vb->node));
> + i915_vma_unpin(va);
> + if (i915_vma_unbind_unlocked(va))
> + err = -EIO;
> +out:
> + i915_gem_object_put(batch);
> + return err;
> +}
> +
> +static struct drm_i915_gem_object *create_lmem(struct intel_gt *gt)
> +{
> + return i915_gem_object_create_lmem(gt->i915, SZ_1G, I915_BO_ALLOC_CONTIGUOUS);
> +}
Is there a special significance in allocating 1G and if so lets explain
it with a comment similar to one in create_smem below.
> +
> +static struct drm_i915_gem_object *create_smem(struct intel_gt *gt)
> +{
> + /*
> + * SZ_64K pages require covering the whole 2M PT (gen8 to tgl/dg1).
> + * While that does not require the whole 2M block to be contiguous
> + * it is easier to make it so, since we need that for SZ_2M pagees.
> + * Since we randomly offset the start of the vma, we need a 4M object
> + * so that there is a 2M range within it is suitable for SZ_64K PTE.
> + */
> + return i915_gem_object_create_internal(gt->i915, SZ_4M);
> +}
> +
> +static int
> +mem_tlbinv(struct intel_gt *gt,
> + struct drm_i915_gem_object *(*create_fn)(struct intel_gt *),
> + void (*tlbinv)(struct i915_address_space *vm, u64 addr, u64 length))
> +{
> + struct intel_engine_cs *engine;
> + struct drm_i915_gem_object *A, *B;
> + struct i915_ppgtt *ppgtt;
> + struct i915_vma *va, *vb;
> + enum intel_engine_id id;
> + I915_RND_STATE(prng);
> + LIST_HEAD(discard);
> + void *vaddr;
> + int err;
> +
> + if (GRAPHICS_VER(gt->i915) < 6) /* MI_CONDITIONAL_BB_END & bcs */
> + return 0;
> +
> + /*
> + * Check that the TLB invalidate is able to revoke an active
> + * page. We load a page into a spinning COND_BBE loop and then
> + * remap that page to a new physical address. The old address, and
> + * so the loop keeps spinning, is retained in the TLB cache until
> + * we issue an invalidate.
> + */
> +
> + A = create_fn(gt);
> + if (IS_ERR(A))
> + return PTR_ERR(A);
> +
> + vaddr = i915_gem_object_pin_map_unlocked(A, I915_MAP_WC);
> + if (IS_ERR(vaddr)) {
> + err = PTR_ERR(vaddr);
> + goto out_a;
> + }
> +
> + /* Allocate a second physical address significantly different from A */
Could we expand this with a why please?
> + do {
> + B = create_fn(gt);
> + if (IS_ERR(B)) {
> + err = PTR_ERR(B);
> + goto out_a;
> + }
> +
> + err = i915_gem_object_pin_pages_unlocked(B);
> + if (err)
> + goto out_b;
> +
> + if (upper_32_bits(i915_gem_object_get_dma_address(A, 0)) !=
> + upper_32_bits(i915_gem_object_get_dma_address(B, 0)))
> + break;
> +
> + list_add(&B->st_link, &discard);
Something prevents releasing the object immediately here?
> + } while (1);
> +
> + vaddr = i915_gem_object_pin_map_unlocked(B, I915_MAP_WC);
> + if (IS_ERR(vaddr)) {
> + err = PTR_ERR(vaddr);
> + goto out_b;
> + }
> +
> + GEM_BUG_ON(A->base.size != B->base.size);
> + if ((A->mm.page_sizes.phys | B->mm.page_sizes.phys) & (A->base.size - 1))
> + pr_warn("Failed to allocate contiguous pages for size %zx\n",
> + A->base.size);
> +
> + ppgtt = i915_ppgtt_create(gt, 0);
> + if (IS_ERR(ppgtt)) {
> + err = PTR_ERR(ppgtt);
> + goto out_b;
> + }
> +
> + va = i915_vma_instance(A, &ppgtt->vm, NULL);
> + if (IS_ERR(va)) {
> + err = PTR_ERR(va);
> + goto out_vm;
> + }
> +
> + vb = i915_vma_instance(B, &ppgtt->vm, NULL);
> + if (IS_ERR(vb)) {
> + err = PTR_ERR(vb);
> + goto out_vm;
> + }
> +
> + err = 0;
> + for_each_engine(engine, gt, id) {
> + struct i915_gem_ww_ctx ww;
> + struct intel_context *ce;
> + int bit;
> +
> + ce = intel_context_create(engine);
> + if (IS_ERR(ce)) {
> + err = PTR_ERR(ce);
> + break;
> + }
> +
> + i915_vm_put(ce->vm);
> + ce->vm = i915_vm_get(&ppgtt->vm);
> +
> + for_i915_gem_ww(&ww, err, true)
> + err = intel_context_pin_ww(ce, &ww);
> + if (err == 0) {
Could invert and save one indentation level at the cost of having two
intel_context_put's, but your choice.
> + for_each_set_bit(bit,
> + (unsigned long *)&RUNTIME_INFO(gt->i915)->page_sizes,
> + BITS_PER_TYPE(RUNTIME_INFO(gt->i915)->page_sizes)) {
> + int len;
> +
> + /* sanitycheck the semaphore wake up */
> + err = pte_tlbinv(ce, va, va,
> + BIT_ULL(bit),
> + NULL, SZ_4K,
> + &prng);
> + if (err)
> + goto err_unpin;
> +
> + for (len = 2; len <= RUNTIME_INFO(gt->i915)->ppgtt_size; len *= 2) {
> + err = pte_tlbinv(ce, va, vb,
> + BIT_ULL(bit),
> + tlbinv,
> + BIT_ULL(len),
> + &prng);
> + if (err)
> + goto err_unpin;
> + }
> +
> + if (len != RUNTIME_INFO(gt->i915)->ppgtt_size) {
> + len = RUNTIME_INFO(gt->i915)->ppgtt_size;
> + err = pte_tlbinv(ce, va, vb,
> + BIT_ULL(bit),
> + tlbinv,
> + BIT_ULL(len),
> + &prng);
> + if (err)
> + goto err_unpin;
> + }
> + }
> +err_unpin:
> + intel_context_unpin(ce);
Is
if (err)
break;
missing from here?
> + }
> +
> + intel_context_put(ce);
> + if (err)
> + break;
> + }
> +
> + if (igt_flush_test(gt->i915))
> + err = -EIO;
> +
> +out_vm:
> + i915_vm_put(&ppgtt->vm);
> +out_b:
> + i915_gem_object_put(B);
> +out_a:
> + i915_gem_object_put(A);
> + list_for_each_entry_safe(A, B, &discard, st_link)
> + i915_gem_object_put(A);
> + return err;
> +}
> +
> +static void tlbinv_full(struct i915_address_space *vm, u64 addr, u64 length)
> +{
> + intel_gt_invalidate_tlb(vm->gt, intel_gt_tlb_seqno(vm->gt) | 1);
> +}
> +
> +static int invalidate_full(void *arg)
> +{
> + struct intel_gt *gt = arg;
> + int err;
> +
> + if (GRAPHICS_VER(gt->i915) < 8)
> + return 0; /* TLB invalidate not implemented */
> +
> + err = mem_tlbinv(gt, create_smem, tlbinv_full);
> + if (err == 0)
> + err = mem_tlbinv(gt, create_lmem, tlbinv_full);
> + if (err == -ENODEV || err == -ENXIO)
ENODEV presumably will come out from platforms not supporting something,
like the create_lmem test.
What about ENXIO? Wondering if it is future proof to eat that error here.
> + err = 0;
> +
> + return err;
> +}
> +
> +int intel_tlb_live_selftests(struct drm_i915_private *i915)
> +{
> + static const struct i915_subtest tests[] = {
> + SUBTEST(invalidate_full),
> + };
> + struct intel_gt *gt;
> + unsigned int i;
> +
> + for_each_gt(gt, i915, i) {
> + int err;
> +
> + if (intel_gt_is_wedged(gt))
> + continue;
> +
> + err = intel_gt_live_subtests(tests, gt);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> index aaf8a380e5c789..5aee6c9a8295ce 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> @@ -25,6 +25,7 @@ selftest(gt_lrc, intel_lrc_live_selftests)
> selftest(gt_mocs, intel_mocs_live_selftests)
> selftest(gt_pm, intel_gt_pm_live_selftests)
> selftest(gt_heartbeat, intel_heartbeat_live_selftests)
> +selftest(gt_tlb, intel_tlb_live_selftests)
> selftest(requests, i915_request_live_selftests)
> selftest(migrate, intel_migrate_live_selftests)
> selftest(active, i915_active_live_selftests)
Okay general idea looks simple - just some details which I perhaps did
not get on the first try to explain.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Add selftests for TLB invalidation
2023-01-17 10:51 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
@ 2023-01-24 15:39 ` Andrzej Hajda
0 siblings, 0 replies; 5+ messages in thread
From: Andrzej Hajda @ 2023-01-24 15:39 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Chris Wilson, Rodrigo Vivi
Hi Tvrtko,
Thx for looking at it.
On 17.01.2023 11:51, Tvrtko Ursulin wrote:
>
> Hi,
>
> Thanks for sending this on! Lets see below..
>
> On 16/01/2023 15:59, Andrzej Hajda wrote:
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Check that we invalidate the TLB cache, the updated physical addresses
>> are immediately visible to the HW, and there is no retention of the old
>> physical address for concurrent HW access.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> [ahajda: adjust to upstream driver]
>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 1 +
>> drivers/gpu/drm/i915/gt/intel_gt.c | 4 +
>> drivers/gpu/drm/i915/gt/selftest_tlb.c | 405 ++++++++++++++++++
>> .../drm/i915/selftests/i915_live_selftests.h | 1 +
>> 4 files changed, 411 insertions(+)
>> create mode 100644 drivers/gpu/drm/i915/gt/selftest_tlb.c
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
>> b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
>> index 2af1ae3831df98..e10507fa71ce63 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
>> @@ -394,6 +394,7 @@
>> #define MI_LOAD_URB_MEM MI_INSTR(0x2C, 0)
>> #define MI_STORE_URB_MEM MI_INSTR(0x2D, 0)
>> #define MI_CONDITIONAL_BATCH_BUFFER_END MI_INSTR(0x36, 0)
>> +#define MI_DO_COMPARE REG_BIT(21)
>> #define STATE_BASE_ADDRESS \
>> ((0x3 << 29) | (0x0 << 27) | (0x1 << 24) | (0x1 << 16))
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
>> b/drivers/gpu/drm/i915/gt/intel_gt.c
>> index 7eeee5a7cb33cb..e6358373eb9c92 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -1196,3 +1196,7 @@ void intel_gt_invalidate_tlb(struct intel_gt
>> *gt, u32 seqno)
>> mutex_unlock(>->tlb.invalidate_lock);
>> }
>> }
>> +
>> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>> +#include "selftest_tlb.c"
>> +#endif
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_tlb.c
>> b/drivers/gpu/drm/i915/gt/selftest_tlb.c
>> new file mode 100644
>> index 00000000000000..a5082a70f06a77
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gt/selftest_tlb.c
>> @@ -0,0 +1,405 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#include "i915_selftest.h"
>> +
>> +#include "gem/i915_gem_internal.h"
>> +#include "gem/i915_gem_region.h"
>> +
>> +#include "gen8_engine_cs.h"
>> +#include "i915_gem_ww.h"
>> +#include "intel_engine_regs.h"
>> +#include "intel_gpu_commands.h"
>> +#include "intel_context.h"
>> +#include "intel_gt.h"
>> +#include "intel_ring.h"
>> +
>> +#include "selftests/igt_flush_test.h"
>> +#include "selftests/i915_random.h"
>> +
>> +static void clear_dw(struct i915_vma *vma, u64 addr, u32 val)
>> +{
>> + GEM_BUG_ON(addr < i915_vma_offset(vma));
>> + GEM_BUG_ON(addr >= i915_vma_offset(vma) + i915_vma_size(vma));
>> + memset32(page_mask_bits(vma->obj->mm.mapping) +
>> + (addr - i915_vma_offset(vma)), val, 1);
>> +}
>> +
>> +static int
>> +pte_tlbinv(struct intel_context *ce,
>> + struct i915_vma *va,
>> + struct i915_vma *vb,
>> + u64 align,
>> + void (*tlbinv)(struct i915_address_space *vm, u64 addr, u64
>> length),
>> + u64 length,
>> + struct rnd_state *prng)
>> +{
>> + const int use_64b = GRAPHICS_VER(ce->vm->i915) >= 8;
>> + struct drm_i915_gem_object *batch;
>> + struct i915_request *rq;
>> + struct i915_vma *vma;
>> + int retries;
>> + u64 addr;
>> + int err;
>> + u32 *cs;
>> +
>> + batch = i915_gem_object_create_internal(ce->vm->i915, 4096);
>> + if (IS_ERR(batch))
>> + return PTR_ERR(batch);
>> +
>> + vma = i915_vma_instance(batch, ce->vm, NULL);
>> + if (IS_ERR(vma)) {
>> + err = PTR_ERR(vma);
>> + goto out;
>> + }
>> +
>> + err = i915_vma_pin(vma, 0, 0, PIN_USER);
>> + if (err)
>> + goto out;
>> +
>> + retries = 5;
>> + do {
>> + addr = igt_random_offset(prng,
>> + i915_vma_offset(vma),
>
> Why start is set to i915_vma_offset(vma) ?
I guess it can be probably better to start after vma (since vma is
allocated at the beginning of vm).
>
>> + /* upper limit for MI_BB_START */
>> + min(ce->vm->total, BIT_ULL(48)),
>> + va->size, 4);
>> +
>> + err = i915_vma_pin(va, 0, 0, (addr & -align) |
>> PIN_OFFSET_FIXED | PIN_USER);
>> + } while (err == -ENOSPC && --retries);
>> + if (err) {
>> + err = 0;
>> + goto out;
>> + }
>
> This loop should basically never fail since the VM is pristine and
> local, right? It can only clash with the the batch buffer so I am
> wondering if it would be better not to eat the error condition.
>
> Or evicting previous engine runs does not work? But then five retries
> wouldn't be enough..
My tests shows the bb (vma) is usually pinned either to offset 0, ether
to 4096, so the pristinity is disputable.
Anyway I will try to pin in the area after end of vma plus align just once.
>
>> + GEM_BUG_ON(i915_vma_offset(va) != (addr & -align));
>> + vb->node = va->node; /* overwrites the _same_ PTE */
>> +
>> + if (align == SZ_64K) {
>> + u64 end = addr + va->size;
>> +
>> + /*
>> + * SZ_64K pages on dg1 require that the whole PT be marked
>> + * containing 64KiB entries. So we make sure that our vma
>> + * covers the whole PT, despite being randomly aligned to 64KiB
>> + * and restrict our sampling to the 2MiB PT within where
>> + * we know that we will be using 64KiB pages.
>> + */
>> + addr = round_up(addr & -align, SZ_2M);
>> + addr |= igt_random_offset(prng, 0, end - addr, 4, 4);
>
> I don't quite get what this is for. Can it even be that PTs are not all
> SZ_64K when the backing store was verified to be contigous already?
This is, I belive, how 64KB pages works on DG1[1] - all 32 entries (only
every 16th counts) in PT should point to 64KB pages - 32*64KB=2MB. In
short if we want to excersise 64KB pages, we need to pin vma at 64KB
boundary, ptes up to 2MB boundary will be 4KB, then we should have 32
64KB ptes.
[1]: https://gfxspecs.intel.com/Predator/Home/Index/45026
>
>> + }
>> +
>> + if (addr - i915_vma_offset(va) >= i915_vma_size(va))
>> + addr = igt_random_offset(prng,
>> + i915_vma_offset(va),
>> + i915_vma_offset(va) + i915_vma_size(va),
>> + 4, 4);
>
> What's this for? Seemingly can override all the "complicated" placement
> calculations from above.
This apparently handles cases in which complicated calculation could
went wrong. I will try to remove it.
>
>> +
>> + pr_info("%s(%s): Sampling %llx, with alignment %llx, using PTE
>> size %x (phys %x, sg %x), invalidate:%llx+%llx\n",
>> + ce->engine->name, va->obj->mm.region->name ?: "smem",
>> + addr, align, va->resource->page_sizes_gtt,
>> va->page_sizes.phys, va->page_sizes.sg,
>> + addr & -length, length);
>> +
>> + cs = i915_gem_object_pin_map_unlocked(batch, I915_MAP_WC);
>> + *cs++ = MI_NOOP; /* for later termination */
>> +
>> + /* Sample the target to see if we spot an incorrect page */
>> + *cs++ = MI_CONDITIONAL_BATCH_BUFFER_END | MI_DO_COMPARE | (1 +
>> use_64b);
>> + *cs++ = -2; /* break if *addr < -1 */
>> + *cs++ = lower_32_bits(addr);
>> + *cs++ = upper_32_bits(addr);
>> + clear_dw(va, addr, -1);
>> + clear_dw(vb, addr, 0);
>
> Is the "break if *addr < -1" comment correct? Because in the sanity
> check case where va == vb, we have 0 in the target dword - and code
> implies that exists the batch.
This is unsigned:
(u32)0 < (u32)(-2)
(u32)(-1) > (u32)(-2)
So the comment should be fixed "/* break if *addr < -2 */"
or different values choosen.
>
>> +
>> + /* Keep sampling until we get bored */
>> + *cs++ = MI_BATCH_BUFFER_START | BIT(8) | use_64b;
>
> Strange that we don't have MI_BATCH_PPGTT for Gen8+.
>
>> + *cs++ = lower_32_bits(i915_vma_offset(vma));
>> + *cs++ = upper_32_bits(i915_vma_offset(vma));
>> +
>> + i915_gem_object_flush_map(batch);
>> +
>> + rq = i915_request_create(ce);
>> + if (IS_ERR(rq)) {
>> + err = PTR_ERR(rq);
>> + goto out_va;
>> + }
>> +
>> + err = rq->engine->emit_bb_start(rq, i915_vma_offset(vma), 0, 0);
>> + if (err) {
>> + i915_request_add(rq);
>> + goto out_va;
>> + }
>> +
>> + i915_request_get(rq);
>> + i915_request_add(rq);
>> +
>> + /* Short sleep to sanitycheck the batch is spinning before we
>> begin */
>> + msleep(10);
>> + if (va == vb) {
>> + if (!i915_request_completed(rq)) {
>> + pr_err("Semaphore sanitycheck failed\n");
>> + err = -EIO;
>> + }
>> + } else if (!i915_request_completed(rq)) {
>> + struct i915_vma_resource vb_res = {
>> + .bi.pages = vb->obj->mm.pages,
>> + .bi.page_sizes = vb->obj->mm.page_sizes,
>> + .start = i915_vma_offset(vb),
>> + .vma_size = i915_vma_size(vb)
>> + };
>> + unsigned int pte_flags = 0;
>> +
>> + /* Flip the PTE between A and B */
>> + if (i915_gem_object_is_lmem(vb->obj))
>> + pte_flags |= PTE_LM;
>> + ce->vm->insert_entries(ce->vm, &vb_res, 0, pte_flags);
>> +
>> + /* Flush the PTE update to concurrent HW */
>> + tlbinv(ce->vm, addr & -length, length);
>> +
>> + if (wait_for(i915_request_completed(rq), HZ / 2)) {
>> + pr_err("%s: Request did not complete; the COND_BBE did
>> not read the updated PTE\n",
>> + ce->engine->name);
>> + err = -EINVAL;
>> + }
>> + } else {
>> + pr_err("Spinner sanitycheck failed\n");
>> + err = -EIO;
>
> This doesn't appear to be the sanity check branch, as referred in
> mem_tlbinv, but more like batch unexpectedly terminated in the full test
> scenario.
>
>> + }
>> + i915_request_put(rq);
>> +
>> + cs = page_mask_bits(batch->mm.mapping);
>> + *cs = MI_BATCH_BUFFER_END;
>> + wmb();
>> +
>> +out_va:
>> + if (vb != va)
>> + memset(&vb->node, 0, sizeof(vb->node));
>> + i915_vma_unpin(va);
>> + if (i915_vma_unbind_unlocked(va))
>> + err = -EIO;
>> +out:
>> + i915_gem_object_put(batch);
>> + return err;
>> +}
>> +
>> +static struct drm_i915_gem_object *create_lmem(struct intel_gt *gt)
>> +{
>> + return i915_gem_object_create_lmem(gt->i915, SZ_1G,
>> I915_BO_ALLOC_CONTIGUOUS);
>> +}
>
> Is there a special significance in allocating 1G and if so lets explain
> it with a comment similar to one in create_smem below.
Ok, will be added (1GB pages).
>
>> +
>> +static struct drm_i915_gem_object *create_smem(struct intel_gt *gt)
>> +{
>> + /*
>> + * SZ_64K pages require covering the whole 2M PT (gen8 to tgl/dg1).
>> + * While that does not require the whole 2M block to be contiguous
>> + * it is easier to make it so, since we need that for SZ_2M pagees.
>> + * Since we randomly offset the start of the vma, we need a 4M
>> object
>> + * so that there is a 2M range within it is suitable for SZ_64K PTE.
>> + */
>> + return i915_gem_object_create_internal(gt->i915, SZ_4M);
>> +}
>> +
>> +static int
>> +mem_tlbinv(struct intel_gt *gt,
>> + struct drm_i915_gem_object *(*create_fn)(struct intel_gt *),
>> + void (*tlbinv)(struct i915_address_space *vm, u64 addr, u64
>> length))
>> +{
>> + struct intel_engine_cs *engine;
>> + struct drm_i915_gem_object *A, *B;
>> + struct i915_ppgtt *ppgtt;
>> + struct i915_vma *va, *vb;
>> + enum intel_engine_id id;
>> + I915_RND_STATE(prng);
>> + LIST_HEAD(discard);
>> + void *vaddr;
>> + int err;
>> +
>> + if (GRAPHICS_VER(gt->i915) < 6) /* MI_CONDITIONAL_BB_END & bcs */
>> + return 0;
>> +
>> + /*
>> + * Check that the TLB invalidate is able to revoke an active
>> + * page. We load a page into a spinning COND_BBE loop and then
>> + * remap that page to a new physical address. The old address, and
>> + * so the loop keeps spinning, is retained in the TLB cache until
>> + * we issue an invalidate.
>> + */
>> +
>> + A = create_fn(gt);
>> + if (IS_ERR(A))
>> + return PTR_ERR(A);
>> +
>> + vaddr = i915_gem_object_pin_map_unlocked(A, I915_MAP_WC);
>> + if (IS_ERR(vaddr)) {
>> + err = PTR_ERR(vaddr);
>> + goto out_a;
>> + }
>> +
>> + /* Allocate a second physical address significantly different
>> from A */
>
> Could we expand this with a why please?
As I understand from Chris, this is to assure we have 'phys' addresses
as different as possible, to avoid influence of possible other
caches/prefetch/...
>
>> + do {
>> + B = create_fn(gt);
>> + if (IS_ERR(B)) {
>> + err = PTR_ERR(B);
>> + goto out_a;
>> + }
>> +
>> + err = i915_gem_object_pin_pages_unlocked(B);
>> + if (err)
>> + goto out_b;
>> +
>> + if (upper_32_bits(i915_gem_object_get_dma_address(A, 0)) !=
>> + upper_32_bits(i915_gem_object_get_dma_address(B, 0)))
>> + break;
>> +
>> + list_add(&B->st_link, &discard);
>
> Something prevents releasing the object immediately here?
Avoid the same address allocation in the next iteration.
>
>> + } while (1);
>> +
>> + vaddr = i915_gem_object_pin_map_unlocked(B, I915_MAP_WC);
>> + if (IS_ERR(vaddr)) {
>> + err = PTR_ERR(vaddr);
>> + goto out_b;
>> + }
>> +
>> + GEM_BUG_ON(A->base.size != B->base.size);
>> + if ((A->mm.page_sizes.phys | B->mm.page_sizes.phys) &
>> (A->base.size - 1))
>> + pr_warn("Failed to allocate contiguous pages for size %zx\n",
>> + A->base.size);
>> +
>> + ppgtt = i915_ppgtt_create(gt, 0);
>> + if (IS_ERR(ppgtt)) {
>> + err = PTR_ERR(ppgtt);
>> + goto out_b;
>> + }
>> +
>> + va = i915_vma_instance(A, &ppgtt->vm, NULL);
>> + if (IS_ERR(va)) {
>> + err = PTR_ERR(va);
>> + goto out_vm;
>> + }
>> +
>> + vb = i915_vma_instance(B, &ppgtt->vm, NULL);
>> + if (IS_ERR(vb)) {
>> + err = PTR_ERR(vb);
>> + goto out_vm;
>> + }
>> +
>> + err = 0;
>> + for_each_engine(engine, gt, id) {
>> + struct i915_gem_ww_ctx ww;
>> + struct intel_context *ce;
>> + int bit;
>> +
>> + ce = intel_context_create(engine);
>> + if (IS_ERR(ce)) {
>> + err = PTR_ERR(ce);
>> + break;
>> + }
>> +
>> + i915_vm_put(ce->vm);
>> + ce->vm = i915_vm_get(&ppgtt->vm);
>> +
>> + for_i915_gem_ww(&ww, err, true)
>> + err = intel_context_pin_ww(ce, &ww);
>> + if (err == 0) {
>
> Could invert and save one indentation level at the cost of having two
> intel_context_put's, but your choice.
Will do it.
>
>> + for_each_set_bit(bit,
>> + (unsigned long
>> *)&RUNTIME_INFO(gt->i915)->page_sizes,
>> +
>> BITS_PER_TYPE(RUNTIME_INFO(gt->i915)->page_sizes)) {
>> + int len;
>> +
>> + /* sanitycheck the semaphore wake up */
>> + err = pte_tlbinv(ce, va, va,
>> + BIT_ULL(bit),
>> + NULL, SZ_4K,
>> + &prng);
>> + if (err)
>> + goto err_unpin;
>> +
>> + for (len = 2; len <=
>> RUNTIME_INFO(gt->i915)->ppgtt_size; len *= 2) {
>> + err = pte_tlbinv(ce, va, vb,
>> + BIT_ULL(bit),
>> + tlbinv,
>> + BIT_ULL(len),
>> + &prng);
>> + if (err)
>> + goto err_unpin;
>> + }
>> +
>> + if (len != RUNTIME_INFO(gt->i915)->ppgtt_size) {
>> + len = RUNTIME_INFO(gt->i915)->ppgtt_size;
>> + err = pte_tlbinv(ce, va, vb,
>> + BIT_ULL(bit),
>> + tlbinv,
>> + BIT_ULL(len),
>> + &prng);
>> + if (err)
>> + goto err_unpin;
>> + }
>> + }
>> +err_unpin:
>> + intel_context_unpin(ce);
>
> Is
>
> if (err)
> break;
>
> missing from here?
No, the loop ends below and there is break on error.
>
>> + }
>> +
>> + intel_context_put(ce);
>> + if (err)
>> + break;
>> + }
>> +
>> + if (igt_flush_test(gt->i915))
>> + err = -EIO;
>> +
>> +out_vm:
>> + i915_vm_put(&ppgtt->vm);
>> +out_b:
>> + i915_gem_object_put(B);
>> +out_a:
>> + i915_gem_object_put(A);
>> + list_for_each_entry_safe(A, B, &discard, st_link)
>> + i915_gem_object_put(A);
>> + return err;
>> +}
>> +
>> +static void tlbinv_full(struct i915_address_space *vm, u64 addr, u64
>> length)
>> +{
>> + intel_gt_invalidate_tlb(vm->gt, intel_gt_tlb_seqno(vm->gt) | 1);
>> +}
>> +
>> +static int invalidate_full(void *arg)
>> +{
>> + struct intel_gt *gt = arg;
>> + int err;
>> +
>> + if (GRAPHICS_VER(gt->i915) < 8)
>> + return 0; /* TLB invalidate not implemented */
>> +
>> + err = mem_tlbinv(gt, create_smem, tlbinv_full);
>> + if (err == 0)
>> + err = mem_tlbinv(gt, create_lmem, tlbinv_full);
>> + if (err == -ENODEV || err == -ENXIO)
>
> ENODEV presumably will come out from platforms not supporting something,
> like the create_lmem test.
>
> What about ENXIO? Wondering if it is future proof to eat that error here.
From i915_gem_object_pin_map in case of no iommu?
>
>> + err = 0;
>> +
>> + return err;
>> +}
>> +
>> +int intel_tlb_live_selftests(struct drm_i915_private *i915)
>> +{
>> + static const struct i915_subtest tests[] = {
>> + SUBTEST(invalidate_full),
>> + };
>> + struct intel_gt *gt;
>> + unsigned int i;
>> +
>> + for_each_gt(gt, i915, i) {
>> + int err;
>> +
>> + if (intel_gt_is_wedged(gt))
>> + continue;
>> +
>> + err = intel_gt_live_subtests(tests, gt);
>> + if (err)
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
>> b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
>> index aaf8a380e5c789..5aee6c9a8295ce 100644
>> --- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
>> +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
>> @@ -25,6 +25,7 @@ selftest(gt_lrc, intel_lrc_live_selftests)
>> selftest(gt_mocs, intel_mocs_live_selftests)
>> selftest(gt_pm, intel_gt_pm_live_selftests)
>> selftest(gt_heartbeat, intel_heartbeat_live_selftests)
>> +selftest(gt_tlb, intel_tlb_live_selftests)
>> selftest(requests, i915_request_live_selftests)
>> selftest(migrate, intel_migrate_live_selftests)
>> selftest(active, i915_active_live_selftests)
>
> Okay general idea looks simple - just some details which I perhaps did
> not get on the first try to explain.
That was quite good excersise for me as well.
I will send v2 in 1-2 days.
Regards
Andrzej
>
> Regards,
>
> Tvrtko
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-24 15:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-16 15:59 [Intel-gfx] [PATCH] drm/i915/gt: Add selftests for TLB invalidation Andrzej Hajda
2023-01-16 17:36 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2023-01-16 19:27 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-01-17 10:51 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
2023-01-24 15:39 ` Andrzej Hajda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox