Re: [PATCH v2 06/16] drm/i915: Ensure gem_contexts selftests work with unbind changes.

On Mon, 29 Nov 2021 at 13:57, Maarten Lankhorst maarten.lankhorst@linux.intel.com wrote:

In the next commit, we don't evict when refcount = 0.

igt_vm_isolation() continuously tries to pin/unpin at same address, but also calls put() on the object, which means the object may not be unpinned in time.

Instead of this, re-use the same object over and over, so they can be unbound as required.

Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

Is this something to be worried about in the real world, outside of the selftests?

---

.../drm/i915/gem/selftests/i915_gem_context.c | 54 +++++++++++-------- 1 file changed, 32 insertions(+), 22 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 b32f7fed2d9c..3fc595b57cf4 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -1481,10 +1481,10 @@ static int check_scratch(struct i915_address_space *vm, u64 offset)

static int write_to_scratch(struct i915_gem_context *ctx, struct intel_engine_cs *engine,

•                       struct drm_i915_gem_object *obj,
                        u64 offset, u32 value)
  

{ struct drm_i915_private *i915 = ctx->i915;

•   struct drm_i915_gem_object *obj;
    struct i915_address_space *vm;
    struct i915_request *rq;
    struct i915_vma *vma;
  

@@ -1497,15 +1497,9 @@ static int write_to_scratch(struct i915_gem_context *ctx, if (err) return err;

•   obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
  

•   if (IS_ERR(obj))
  

•           return PTR_ERR(obj);
  

•   cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB);
  

•   if (IS_ERR(cmd)) {
  

•           err = PTR_ERR(cmd);
  

•           goto out;
  

•   }
  

•   if (IS_ERR(cmd))
  

•           return PTR_ERR(cmd);
  
    *cmd++ = MI_STORE_DWORD_IMM_GEN4;
    if (GRAPHICS_VER(i915) >= 8) {
  

@@ -1569,17 +1563,19 @@ static int write_to_scratch(struct i915_gem_context *ctx, i915_vma_unpin(vma); out_vm: i915_vm_put(vm); -out:

•   i915_gem_object_put(obj);
  

•   if (!err)
  

•           err = i915_gem_object_wait(obj, 0, MAX_SCHEDULE_TIMEOUT);
  

•   return err;
  

}

static int read_from_scratch(struct i915_gem_context *ctx, struct intel_engine_cs *engine,

•                        struct drm_i915_gem_object *obj,
                         u64 offset, u32 *value)
  

{ struct drm_i915_private *i915 = ctx->i915;

•   struct drm_i915_gem_object *obj;
    struct i915_address_space *vm;
    const u32 result = 0x100;
    struct i915_request *rq;
  

@@ -1594,10 +1590,6 @@ static int read_from_scratch(struct i915_gem_context *ctx, if (err) return err;

•   obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
  

•   if (IS_ERR(obj))
  

•           return PTR_ERR(obj);
  

•   if (GRAPHICS_VER(i915) >= 8) {
            const u32 GPR0 = engine->mmio_base + 0x600;
  
  

@@ -1615,7 +1607,7 @@ static int read_from_scratch(struct i915_gem_context *ctx, cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); if (IS_ERR(cmd)) { err = PTR_ERR(cmd);

•                   goto out;
  

•                   goto err_unpin;
            }
  
            memset(cmd, POISON_INUSE, PAGE_SIZE);
  

@@ -1651,7 +1643,7 @@ static int read_from_scratch(struct i915_gem_context *ctx, cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); if (IS_ERR(cmd)) { err = PTR_ERR(cmd);

•                   goto out;
  

•                   goto err_unpin;
            }
  
            memset(cmd, POISON_INUSE, PAGE_SIZE);
  

@@ -1722,8 +1714,10 @@ static int read_from_scratch(struct i915_gem_context *ctx, i915_vma_unpin(vma); out_vm: i915_vm_put(vm); -out:

•   i915_gem_object_put(obj);
  

•   if (!err)
  

•           err = i915_gem_object_wait(obj, 0, MAX_SCHEDULE_TIMEOUT);
  

•   return err;
  

}

@@ -1765,6 +1759,7 @@ static int igt_vm_isolation(void *arg) u64 vm_total; u32 expected; int err;

•   struct drm_i915_gem_object *obj_a, *obj_b;
  

Nit: Christmas tree-ish

    if (GRAPHICS_VER(i915) < 7)
            return 0;

@@ -1810,6 +1805,18 @@ static int igt_vm_isolation(void *arg) vm_total = ctx_a->vm->total; GEM_BUG_ON(ctx_b->vm->total != vm_total);

•   obj_a = i915_gem_object_create_internal(i915, PAGE_SIZE);
  

•   if (IS_ERR(obj_a)) {
  

•           err = PTR_ERR(obj_a);
  

•           goto out_file;
  

•   }
  

•   obj_b = i915_gem_object_create_internal(i915, PAGE_SIZE);
  

•   if (IS_ERR(obj_b)) {
  

•           err = PTR_ERR(obj_b);
  

•           goto put_a;
  

•   }
  

•   count = 0;
    num_engines = 0;
    for_each_uabi_engine(engine, i915) {
  

@@ -1832,10 +1839,10 @@ static int igt_vm_isolation(void *arg) I915_GTT_PAGE_SIZE, vm_total, sizeof(u32), alignof_dword);

•                   err = write_to_scratch(ctx_a, engine,
  

•                   err = write_to_scratch(ctx_a, engine, obj_a,
                                           offset, 0xdeadbeef);
                    if (err == 0)
  

•                           err = read_from_scratch(ctx_b, engine,
  

•                           err = read_from_scratch(ctx_b, engine, obj_b,
                                                    offset, &value);
                    if (err)
                            goto out_file;
  

goto put_b; below also?

Otherwise, Reviewed-by: Matthew Auld matthew.auld@intel.com

@@ -1858,6 +1865,9 @@ static int igt_vm_isolation(void *arg) pr_info("Checked %lu scratch offsets across %lu engines\n", count, num_engines);

•   i915_gem_object_put(obj_b);
  

+put_a:

•   i915_gem_object_put(obj_a);
  

out_file: if (igt_live_test_end(&t)) err = -EIO; -- 2.34.0