[Intel-gfx] [PATCH 1/2] Revert "drm/i915: Use uabi engines for the default engine map"
Mathias Krause
minipli at grsecurity.net
Tue Sep 5 11:39:20 UTC 2023
Commit 1ec23ed7126e ("drm/i915: Use uabi engines for the default engine
map") switched from using for_each_engine() to for_each_uabi_engine() to
iterate over the user engines. While this seems to be a sensible change,
it's only safe to do when the engines are actually chained using the
rb-tree structure which is not the case during early driver
initialization where it can be either a lock-less list or regular
double-linked list.
In fact, the modesetting initialization code may end up calling
default_engines() through the fb helper code while the engines list
is still llist_node-based:
i915_driver_probe() ->
intel_modeset_init() ->
intel_fbdev_init() ->
drm_fb_helper_init() ->
drm_client_init() ->
drm_client_open() ->
drm_file_alloc() ->
i915_driver_open() ->
i915_gem_open() ->
i915_gem_context_open() ->
i915_gem_create_context() ->
default_engines()
Using for_each_uabi_engine() in default_engines() is therefore wrong, as
it would try to interpret the llist as rb-tree, making it find no engine
at all, as the rb_left and rb_right members will still be NULL, as they
haven't been iniitalized yet.
Revert back to use for_each_engine() which will look at each individual
engine based on its id, i.e. purely array-index based, avoiding the type
confusion mess.
Reported-by: sanitiy checks in grsecurity
Fixes: 1ec23ed7126e ("drm/i915: Use uabi engines for the default engine map")
Signed-off-by: Mathias Krause <minipli at grsecurity.net>
Cc: Jonathan Cavitt <jonathan.cavitt at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 9a9ff84c90d7..e4f7ebbd2690 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1100,15 +1100,16 @@ static struct i915_gem_engines *alloc_engines(unsigned int count)
static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
struct intel_sseu rcs_sseu)
{
- const unsigned int max = I915_NUM_ENGINES;
+ const struct intel_gt *gt = to_gt(ctx->i915);
struct intel_engine_cs *engine;
struct i915_gem_engines *e, *err;
+ enum intel_engine_id id;
- e = alloc_engines(max);
+ e = alloc_engines(I915_NUM_ENGINES);
if (!e)
return ERR_PTR(-ENOMEM);
- for_each_uabi_engine(engine, ctx->i915) {
+ for_each_engine(engine, gt, id) {
struct intel_context *ce;
struct intel_sseu sseu = {};
int ret;
@@ -1116,7 +1117,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
if (engine->legacy_idx == INVALID_ENGINE)
continue;
- GEM_BUG_ON(engine->legacy_idx >= max);
+ GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
GEM_BUG_ON(e->engines[engine->legacy_idx]);
ce = intel_context_create(engine);
--
2.39.2
More information about the Intel-gfx
mailing list