| Message ID | 20180510190006.31586-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Matthew Auld (2018-05-10 20:00:06) > We write out all PTEs when operating in 64K mode, which is acceptable > given the assertion that the hw only cares about every 16th PTE and so > will ignore everything else. However this may hide potential issues, > for example the hw could be sneakily operating in 4K mode and we would > be none the wiser, so make sure this doesn't escape us in the selftests. For selftesting, don't you want to do the opposite and set the ignored addresses to something that will explode, and not something safe? -Chris
On 10 May 2018 at 20:56, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Matthew Auld (2018-05-10 20:00:06) >> We write out all PTEs when operating in 64K mode, which is acceptable >> given the assertion that the hw only cares about every 16th PTE and so >> will ignore everything else. However this may hide potential issues, >> for example the hw could be sneakily operating in 4K mode and we would >> be none the wiser, so make sure this doesn't escape us in the selftests. > > For selftesting, don't you want to do the opposite and set the ignored > addresses to something that will explode, and not something safe? The tests will fail, when we do write_huge some of our writes will land in scratch, and cpu_check will throw a fit. > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Matthew Auld (2018-05-10 22:12:13) > On 10 May 2018 at 20:56, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Matthew Auld (2018-05-10 20:00:06) > >> We write out all PTEs when operating in 64K mode, which is acceptable > >> given the assertion that the hw only cares about every 16th PTE and so > >> will ignore everything else. However this may hide potential issues, > >> for example the hw could be sneakily operating in 4K mode and we would > >> be none the wiser, so make sure this doesn't escape us in the selftests. > > > > For selftesting, don't you want to do the opposite and set the ignored > > addresses to something that will explode, and not something safe? > > The tests will fail, when we do write_huge some of our writes will > land in scratch, and cpu_check will throw a fit. From reading the comment I was mightily confused, the commitmsg is better for it included an extra clause of explanation. /* * We write all 4K page entries, even when using 64K pages. In order * to verify that the HW isn't cheating by using the 4K PTE instead * of the 64K PTE, we want to remove all the surplus entries. If the HW * skipped the 64K PTE, it will read/write into the scratch page instead - * which we detect as missing results during selftests. */ -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index c879bfd9294f..5deef6044944 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1161,6 +1161,26 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma, vaddr[idx.pde] |= GEN8_PDE_IPS_64K; kunmap_atomic(vaddr); page_size = I915_GTT_PAGE_SIZE_64K; + + /* + * For simplicity we choose to write out all PTEs when + * operating in 64K mode, which is acceptable given the + * assertion that the hw only cares about every 16th PTE + * and so will ignore everything else. However this may + * hide potential issues, so make sure this doesn't + * escape us in the selftests. + */ + if (I915_SELFTEST_ONLY(vma->vm->scrub_64K)) { + u16 i; + + encode = pte_encode | vma->vm->scratch_page.daddr; + vaddr = kmap_atomic_px(pd->page_table[idx.pde]); + + for (i = 1; i < index; i += 16) + memset64(vaddr + i, encode, 15); + + kunmap_atomic(vaddr); + } } vma->page_sizes.gtt |= page_size; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 1db0dedb4059..aec4f73574f4 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -342,6 +342,7 @@ struct i915_address_space { void (*clear_pages)(struct i915_vma *vma); I915_SELFTEST_DECLARE(struct fault_attr fault_attr); + I915_SELFTEST_DECLARE(bool scrub_64K); }; #define i915_is_ggtt(V) (!(V)->file) diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c index d7c8ef8e6764..91c72911be3c 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -1757,6 +1757,9 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *dev_priv) goto out_unlock; } + if (ctx->ppgtt) + ctx->ppgtt->base.scrub_64K = true; + err = i915_subtests(tests, ctx); out_unlock:
We write out all PTEs when operating in 64K mode, which is acceptable given the assertion that the hw only cares about every 16th PTE and so will ignore everything else. However this may hide potential issues, for example the hw could be sneakily operating in 4K mode and we would be none the wiser, so make sure this doesn't escape us in the selftests. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Changbin Du <changbin.du@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 20 ++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + drivers/gpu/drm/i915/selftests/huge_pages.c | 3 +++ 3 files changed, 24 insertions(+)