[PATCH] drm/i915/gt: Add GEM_BUG_ON to ensure at least one engine supports migration

[Tvrtko Ursulin]

On 27/12/2024 11:11, apoorva.singh at intel.com wrote:
> From: Apoorva Singh <apoorva.singh at intel.com>
> 
> 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 at linux.intel.com>
> Signed-off-by: Apoorva Singh <apoorva.singh at intel.com>
> ---
>   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)]);
>   }
>