If we run out of engines, intel_get_current_physical_engine() degrades
into an infinite loop as although it advanced the iterator, it did not
update its local engine pointer.
Reported-by: Petri Latvala <petri.latvala@xxxxxxxxx>
Fixes: 17c77e7b0c3c ("lib/i915: add gem_engine_topology library and for_each loop definition")
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Andi Shyti <andi.shyti@xxxxxxxxx>
Cc: Petri Latvala <petri.latvala@xxxxxxxxx>
---
lib/i915/gem_engine_topology.c | 49 +++++-----------------------------
lib/i915/gem_engine_topology.h | 36 ++++++++++++++++---------
2 files changed, 29 insertions(+), 56 deletions(-)
diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
index fdd1b9516..17f67786f 100644
--- a/lib/i915/gem_engine_topology.c
+++ b/lib/i915/gem_engine_topology.c
@@ -81,11 +81,10 @@ static void ctx_map_engines(int fd, struct intel_engine_data *ed,
struct drm_i915_gem_context_param *param)
{
struct i915_context_param_engines *engines =
- from_user_pointer(param->value);
+ from_user_pointer(param->value);
int i = 0;
- for (typeof(engines->engines[0]) *p =
- &engines->engines[0];
+ for (struct i915_engine_class_instance *p = &engines->engines[0];
i < ed->nengines; i++, p++) {
p->engine_class = ed->engines[i].class;
p->engine_instance = ed->engines[i].instance;
@@ -136,7 +135,7 @@ static void query_engine_list(int fd, struct intel_engine_data *ed)
{
uint8_t buff[SIZEOF_QUERY] = { };
struct drm_i915_query_engine_info *query_engine =
- (struct drm_i915_query_engine_info *) buff;
+ (struct drm_i915_query_engine_info *)buff;
int i;
query_engines(fd, query_engine, SIZEOF_QUERY);
@@ -149,41 +148,6 @@ static void query_engine_list(int fd, struct intel_engine_data *ed)
ed->nengines = query_engine->num_engines;
}
-struct intel_execution_engine2 *
-intel_get_current_engine(struct intel_engine_data *ed)
-{
- if (!ed->n)
- ed->current_engine = &ed->engines[0];
- else if (ed->n >= ed->nengines)
- ed->current_engine = NULL;
-
- return ed->current_engine;
-}
-
-void intel_next_engine(struct intel_engine_data *ed)
-{
- if (ed->n + 1 < ed->nengines) {
- ed->n++;
- ed->current_engine = &ed->engines[ed->n];
- } else {
- ed->n = ed->nengines;
- ed->current_engine = NULL;
- }
-}
-
-struct intel_execution_engine2 *
-intel_get_current_physical_engine(struct intel_engine_data *ed)
-{
- struct intel_execution_engine2 *e;
-
- for (e = intel_get_current_engine(ed);
- e && e->is_virtual;
- intel_next_engine(ed))
- ;
-
- return e;
-}
-
static int gem_topology_get_param(int fd,
struct drm_i915_gem_context_param *p)
{
@@ -197,10 +161,9 @@ static int gem_topology_get_param(int fd,
return 0;
/* size will store the engine count */
- p->size = (p->size - sizeof(struct i915_context_param_engines)) /
- (offsetof(struct i915_context_param_engines,
- engines[1]) -
- sizeof(struct i915_context_param_engines));
+ igt_assert(p->size >= sizeof(struct i915_context_param_engines));
+ p->size -= sizeof(struct i915_context_param_engines);
+ p->size /= sizeof(struct i915_engine_class_instance);
igt_assert_f(p->size <= GEM_MAX_ENGINES, "unsupported engine count\n");
diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h
index 2415fd1e3..a1018afca 100644
--- a/lib/i915/gem_engine_topology.h
+++ b/lib/i915/gem_engine_topology.h
@@ -30,9 +30,7 @@
#define GEM_MAX_ENGINES I915_EXEC_RING_MASK + 1
struct intel_engine_data {
- uint32_t nengines;
- uint32_t n;
- struct intel_execution_engine2 *current_engine;
+ uint32_t nengines, cur;
struct intel_execution_engine2 engines[GEM_MAX_ENGINES];
};
@@ -40,31 +38,43 @@ bool gem_has_engine_topology(int fd);
struct intel_engine_data intel_init_engine_list(int fd, uint32_t ctx_id);
/* iteration functions */
-struct intel_execution_engine2 *
-intel_get_current_engine(struct intel_engine_data *ed);
-
-struct intel_execution_engine2 *
-intel_get_current_physical_engine(struct intel_engine_data *ed);
-
-void intel_next_engine(struct intel_engine_data *ed);
-
int gem_context_lookup_engine(int fd, uint64_t engine, uint32_t ctx_id,
struct intel_execution_engine2 *e);
void gem_context_set_all_engines(int fd, uint32_t ctx);
+static inline struct intel_execution_engine2 *
+intel_get_current_engine(struct intel_engine_data *ed)
+{
+ if (ed->cur >= ed->nengines)
+ return NULL;
+
+ return &ed->engines[ed->cur];
+}
+
+static inline struct intel_execution_engine2 *
+intel_get_current_physical_engine(struct intel_engine_data *ed)
+{
+ struct intel_execution_engine2 *e;
+
+ for (; (e = intel_get_current_engine(ed)) && e->is_virtual; ed->cur++)
+ ;
+
+ return e;
+}
+
#define __for_each_static_engine(e__) \
for ((e__) = intel_execution_engines2; (e__)->name; (e__)++)
#define for_each_context_engine(fd__, ctx__, e__) \
for (struct intel_engine_data i__ = intel_init_engine_list(fd__, ctx__); \
((e__) = intel_get_current_engine(&i__)); \
- intel_next_engine(&i__))
+ i__.cur++)
/* needs to replace "for_each_physical_engine" when conflicts are fixed */
#define __for_each_physical_engine(fd__, e__) \
for (struct intel_engine_data i__ = intel_init_engine_list(fd__, 0); \
((e__) = intel_get_current_physical_engine(&i__)); \
- intel_next_engine(&i__))
+ i__.cur++)
#endif /* GEM_ENGINE_TOPOLOGY_H */
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
