There's quite a fundamental difference between userspace contexts, and kernel contexts. Latter all share intel_gt->vm, former get their vm from gem_ctx->vm (on full ppgtt at least). By splitting context creation for userspace from kernel-internal ones we can make this all a bit more strict and WARN_ON if there's a vm already set in intel_context_set_gem(). All this is only possible because gem_ctx cannot chance their VM anymore since commit ccbc1b97948ab671335e950271e39766729736c3 Author: Jason Ekstrand <jason@xxxxxxxxxxxxxx> Date: Thu Jul 8 10:48:30 2021 -0500 drm/i915/gem: Don't allow changing the VM on running contexts (v4) v2: I didn't take care of virtual engine creation and on top of that collided pretty good with Matt's abstraction from commit 556120256ecd25aacea2c7e3ad11ec6584de7252 Author: Matthew Brost <matthew.brost@xxxxxxxxx> Date: Mon Jul 26 17:23:16 2021 -0700 drm/i915/guc: GuC virtual engines address this all by also splitting the new intel_engine_create_virtual into a _user variant that doesn't set ce->vm. Cc: Matthew Brost <matthew.brost@xxxxxxxxx> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> Cc: John Harrison <John.C.Harrison@xxxxxxxxx> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> Cc: Jon Bloomfield <jon.bloomfield@xxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> Cc: "Thomas Hellström" <thomas.hellstrom@xxxxxxxxxxxxxxx> Cc: Matthew Auld <matthew.auld@xxxxxxxxx> Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> Cc: Dave Airlie <airlied@xxxxxxxxxx> Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 10 ++++----- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 4 +++- .../gpu/drm/i915/gem/selftests/mock_context.c | 2 +- drivers/gpu/drm/i915/gt/intel_context.c | 22 +++++++++++++++++-- drivers/gpu/drm/i915/gt/intel_context.h | 2 ++ drivers/gpu/drm/i915/gt/intel_engine.h | 4 ++++ drivers/gpu/drm/i915/gt/intel_engine_cs.c | 21 ++++++++++++++++-- 7 files changed, 54 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 2f3cc73d4710..754b9b8d4981 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -746,7 +746,7 @@ static int intel_context_set_gem(struct intel_context *ce, ce->ring_size = SZ_16K; - i915_vm_put(ce->vm); + WARN_ON(ce->vm); ce->vm = i915_gem_context_get_eb_vm(ctx); if (ctx->sched.priority >= I915_PRIORITY_NORMAL && @@ -856,7 +856,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx, GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES); GEM_BUG_ON(e->engines[engine->legacy_idx]); - ce = intel_context_create(engine); + ce = intel_context_create_user(engine); if (IS_ERR(ce)) { err = ERR_CAST(ce); goto free_engines; @@ -897,12 +897,12 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, switch (pe[n].type) { case I915_GEM_ENGINE_TYPE_PHYSICAL: - ce = intel_context_create(pe[n].engine); + ce = intel_context_create_user(pe[n].engine); break; case I915_GEM_ENGINE_TYPE_BALANCED: - ce = intel_engine_create_virtual(pe[n].siblings, - pe[n].num_siblings); + ce = intel_engine_create_virtual_user(pe[n].siblings, + pe[n].num_siblings); break; case I915_GEM_ENGINE_TYPE_INVALID: diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index bdf2b5785a81..54de94433365 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -30,15 +30,17 @@ struct eb_vma { struct i915_vma *vma; + struct drm_i915_gem_object *obj; unsigned int flags; + u32 handle; + /** This vma's place in the execbuf reservation list */ struct drm_i915_gem_exec_object2 *exec; struct list_head bind_link; struct list_head reloc_link; struct hlist_node node; - u32 handle; }; enum { diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c index fee070df1c97..e5efda1058a3 100644 --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c @@ -124,7 +124,7 @@ live_context_for_engine(struct intel_engine_cs *engine, struct file *file) return ctx; } - ce = intel_context_create(engine); + ce = intel_context_create_user(engine); if (IS_ERR(ce)) { __free_engines(engines, 0); return ERR_CAST(ce); diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 745e84c72c90..9e33efb594dd 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -34,6 +34,23 @@ void intel_context_free(struct intel_context *ce) call_rcu(&ce->rcu, rcu_context_free); } +/* for user contexts, callers must set ce->vm correctly */ +struct intel_context * +intel_context_create_user(struct intel_engine_cs *engine) +{ + struct intel_context *ce; + + ce = intel_context_alloc(); + if (!ce) + return ERR_PTR(-ENOMEM); + + intel_context_init(ce, engine); + + trace_intel_context_create(ce); + return ce; +} + +/* for kernel-internal users only, sets ce->vm to intel_gt.vm */ struct intel_context * intel_context_create(struct intel_engine_cs *engine) { @@ -44,6 +61,8 @@ intel_context_create(struct intel_engine_cs *engine) return ERR_PTR(-ENOMEM); intel_context_init(ce, engine); + ce->vm = i915_vm_get(engine->gt->vm); + trace_intel_context_create(ce); return ce; } @@ -368,6 +387,7 @@ static int sw_fence_dummy_notify(struct i915_sw_fence *sf, return NOTIFY_DONE; } +/* callers must set ce->vm for user or kernel vm as needed */ void intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine) { @@ -384,8 +404,6 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine) ewma_runtime_init(&ce->runtime.avg); - ce->vm = i915_vm_get(engine->gt->vm); - /* NB ce->signal_link/lock is used under RCU */ spin_lock_init(&ce->signal_lock); INIT_LIST_HEAD(&ce->signals); diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index c41098950746..a80018d53a36 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -34,6 +34,8 @@ void intel_context_fini(struct intel_context *ce); void i915_context_module_exit(void); int i915_context_module_init(void); +struct intel_context * +intel_context_create_user(struct intel_engine_cs *engine); struct intel_context * intel_context_create(struct intel_engine_cs *engine); diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 87579affb952..6f945e60f6d6 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -279,6 +279,10 @@ intel_engine_has_preempt_reset(const struct intel_engine_cs *engine) return intel_engine_has_preemption(engine); } +struct intel_context * +intel_engine_create_virtual_user(struct intel_engine_cs **siblings, + unsigned int count); + struct intel_context * intel_engine_create_virtual(struct intel_engine_cs **siblings, unsigned int count); diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 0d9105a31d84..34fe9dbfccba 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1879,9 +1879,10 @@ ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine, ktime_t *now) return total; } +/* for user contexts, callers must set ce->vm correctly */ struct intel_context * -intel_engine_create_virtual(struct intel_engine_cs **siblings, - unsigned int count) +intel_engine_create_virtual_user(struct intel_engine_cs **siblings, + unsigned int count) { if (count == 0) return ERR_PTR(-EINVAL); @@ -1893,6 +1894,22 @@ intel_engine_create_virtual(struct intel_engine_cs **siblings, return siblings[0]->cops->create_virtual(siblings, count); } +/* for kernel-internal users only, sets ce->vm to intel_gt.vm */ +struct intel_context * +intel_engine_create_virtual(struct intel_engine_cs **siblings, + unsigned int count) +{ + struct intel_context *ce; + + ce = intel_engine_create_virtual_user(siblings, count); + if (IS_ERR(ce)) + return ce; + + ce->vm = i915_vm_get(siblings[0]->gt->vm); + + return ce; +} + struct i915_request * intel_engine_execlist_find_hung_request(struct intel_engine_cs *engine) { -- 2.32.0