From 250f8c8140ac0a5e5acb91891d6813f12778b224 Mon Sep 17 00:00:00 2001 From: Jon Bloomfield Date: Thu, 12 Jul 2018 19:53:11 +0100 Subject: drm/i915/gtt: Read-only pages for insert_entries on bdw+ Hook up the flags to allow read-only ppGTT mappings for gen8+ v2: Include a selftest to check that writes to a readonly PTE are dropped v3: Don't duplicate cpu_check() as we can just reuse it, and even worse don't wholesale copy the theory-of-operation comment from igt_ctx_exec without changing it to explain the intention behind the new test! v4: Joonas really likes magic mystery values Signed-off-by: Jon Bloomfield Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Matthew Auld Reviewed-by: Joonas Lahtinen Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20180712185315.3288-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/selftests/i915_gem_context.c | 112 +++++++++++++++++++++- 1 file changed, 109 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/selftests/i915_gem_context.c') diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c index ab2590242033..2e2a3c44b8e9 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c @@ -23,6 +23,7 @@ */ #include "../i915_selftest.h" +#include "i915_random.h" #include "igt_flush_test.h" #include "mock_drm.h" @@ -252,9 +253,9 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max) } for (; m < DW_PER_PAGE; m++) { - if (map[m] != 0xdeadbeef) { + if (map[m] != STACK_MAGIC) { pr_err("Invalid value at page %d, offset %d: found %x expected %x\n", - n, m, map[m], 0xdeadbeef); + n, m, map[m], STACK_MAGIC); err = -EINVAL; goto out_unmap; } @@ -310,7 +311,7 @@ create_test_object(struct i915_gem_context *ctx, if (err) return ERR_PTR(err); - err = cpu_fill(obj, 0xdeadbeef); + err = cpu_fill(obj, STACK_MAGIC); if (err) { pr_err("Failed to fill object with cpu, err=%d\n", err); @@ -432,6 +433,110 @@ out_unlock: return err; } +static int igt_ctx_readonly(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct drm_i915_gem_object *obj = NULL; + struct drm_file *file; + I915_RND_STATE(prng); + IGT_TIMEOUT(end_time); + LIST_HEAD(objects); + struct i915_gem_context *ctx; + struct i915_hw_ppgtt *ppgtt; + unsigned long ndwords, dw; + int err = -ENODEV; + + /* + * Create a few read-only objects (with the occasional writable object) + * and try to write into these object checking that the GPU discards + * any write to a read-only object. + */ + + file = mock_file(i915); + if (IS_ERR(file)) + return PTR_ERR(file); + + mutex_lock(&i915->drm.struct_mutex); + + ctx = i915_gem_create_context(i915, file->driver_priv); + if (IS_ERR(ctx)) { + err = PTR_ERR(ctx); + goto out_unlock; + } + + ppgtt = ctx->ppgtt ?: i915->mm.aliasing_ppgtt; + if (!ppgtt || !ppgtt->vm.has_read_only) { + err = 0; + goto out_unlock; + } + + ndwords = 0; + dw = 0; + while (!time_after(jiffies, end_time)) { + struct intel_engine_cs *engine; + unsigned int id; + + for_each_engine(engine, i915, id) { + if (!intel_engine_can_store_dword(engine)) + continue; + + if (!obj) { + obj = create_test_object(ctx, file, &objects); + if (IS_ERR(obj)) { + err = PTR_ERR(obj); + goto out_unlock; + } + + obj->gt_ro = prandom_u32_state(&prng); + } + + intel_runtime_pm_get(i915); + err = gpu_fill(obj, ctx, engine, dw); + intel_runtime_pm_put(i915); + if (err) { + pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) in ctx %u [full-ppgtt? %s], err=%d\n", + ndwords, dw, max_dwords(obj), + engine->name, ctx->hw_id, + yesno(!!ctx->ppgtt), err); + goto out_unlock; + } + + if (++dw == max_dwords(obj)) { + obj = NULL; + dw = 0; + } + ndwords++; + } + } + pr_info("Submitted %lu dwords (across %u engines)\n", + ndwords, INTEL_INFO(i915)->num_rings); + + dw = 0; + list_for_each_entry(obj, &objects, st_link) { + unsigned int rem = + min_t(unsigned int, ndwords - dw, max_dwords(obj)); + unsigned int num_writes; + + num_writes = rem; + if (obj->gt_ro) + num_writes = 0; + + err = cpu_check(obj, num_writes); + if (err) + break; + + dw += rem; + } + +out_unlock: + if (igt_flush_test(i915, I915_WAIT_LOCKED)) + err = -EIO; + mutex_unlock(&i915->drm.struct_mutex); + + mock_file_free(i915, file); + return err; +} + static __maybe_unused const char * __engine_name(struct drm_i915_private *i915, unsigned int engines) { @@ -608,6 +713,7 @@ int i915_gem_context_live_selftests(struct drm_i915_private *dev_priv) static const struct i915_subtest tests[] = { SUBTEST(igt_switch_to_kernel_context), SUBTEST(igt_ctx_exec), + SUBTEST(igt_ctx_readonly), }; bool fake_alias = false; int err; -- cgit From 3e977ac6179b39faa3c0eda5fce4f00663ae298d Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 12 Jul 2018 19:53:13 +0100 Subject: drm/i915: Prevent writing into a read-only object via a GGTT mmap If the user has created a read-only object, they should not be allowed to circumvent the write protection by using a GGTT mmapping. Deny it. Also most machines do not support read-only GGTT PTEs, so again we have to reject attempted writes. Fortunately, this is known a priori, so we can at least reject in the call to create the mmap (with a sanity check in the fault handler). v2: Check the vma->vm_flags during mmap() to allow readonly access. v3: Remove VM_MAYWRITE to curtail mprotect() Testcase: igt/gem_userptr_blits/readonly_mmap* Signed-off-by: Chris Wilson Cc: Jon Bloomfield Cc: Joonas Lahtinen Cc: Matthew Auld Cc: David Herrmann Reviewed-by: Matthew Auld #v1 Reviewed-by: Jon Bloomfield Reviewed-by: Joonas Lahtinen Link: https://patchwork.freedesktop.org/patch/msgid/20180712185315.3288-4-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/selftests/i915_gem_context.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/selftests/i915_gem_context.c') diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c index 2e2a3c44b8e9..1c92560d35da 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c @@ -487,7 +487,8 @@ static int igt_ctx_readonly(void *arg) goto out_unlock; } - obj->gt_ro = prandom_u32_state(&prng); + if (prandom_u32_state(&prng) & 1) + i915_gem_object_set_readonly(obj); } intel_runtime_pm_get(i915); @@ -518,7 +519,7 @@ static int igt_ctx_readonly(void *arg) unsigned int num_writes; num_writes = rem; - if (obj->gt_ro) + if (i915_gem_object_is_readonly(obj)) num_writes = 0; err = cpu_check(obj, num_writes); -- cgit