On 27/12/2024 11:11, apoorva.singh@xxxxxxxxx wrote:
From: Apoorva Singh <apoorva.singh@xxxxxxxxx>
Ensure GEM_BUG_ON verifies at least one engine supports migration.
Additionally, it prevents passing an uninitialized variable to
intel_context_create().
Under what circumstances can this happen ie. does it need a Fixes: tag?
Also consider that if it can happen in production on some real hardware
then GEM_BUG_ON does not prevent anything (GEM_BUG_ON is compiled out).
On debug builds it also strictly doesn't prevent anything but crashes
the machine.
Consider this as an alternative (if the error scenario is real):
diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c
b/drivers/gpu/drm/i915/gt/intel_migrate.c
index 6f7af4077135..83e6c20cb750 100644
--- a/drivers/gpu/drm/i915/gt/intel_migrate.c
+++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
@@ -296,7 +296,9 @@ static struct intel_context
*__migrate_engines(struct intel_gt *gt)
engines[count++] = engine;
}
- return intel_context_create(engines[random_index(count)]);
+ return count ?
+ intel_context_create(engines[random_index(count)]) :
+ ERR_PTR(-ENOENT);
}
struct intel_context *intel_migrate_create_context(struct
intel_migrate *m)
But check if the whole error propagation chain is fine to handle it.
Regards,
Tvrtko
Cc: Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx>
Signed-off-by: Apoorva Singh <apoorva.singh@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_migrate.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c
index 6f7af4077135..528a7cba3623 100644
--- a/drivers/gpu/drm/i915/gt/intel_migrate.c
+++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
@@ -296,6 +296,9 @@ static struct intel_context *__migrate_engines(struct intel_gt *gt)
engines[count++] = engine;
}
+ /* At least one engine must support migration! */
+ GEM_BUG_ON(!count);
+
return intel_context_create(engines[random_index(count)]);
}
