Re: [PATCH i-g-t 1/3] tests/xe_eudebug: Keep engine for each client

["Grzegorzek, Dominik" <dominik.grzegorzek@intel.com>]

On Thu, 2024-09-26 at 13:39 +0300, Mika Kuoppala wrote:
> If we only find hardware engine for first client,
> it can be stale for other clients, after we close
> the first client, as it was associated through the fd
> and we should not trust lib/xe internals how it handles
> the engine array.
> 
> I found this when trying to track down:
> 
> [  132.467616] xe 0000:00:02.0: [drm] *ERROR* Ioctl argument check
> failed at drivers/gpu/drm/xe/xe_exec_queue.c:611:
> eci[0].gt_id >= xe->info.gt_count
> 
> transient error when running xe_eudebug@discovery-empty.
> Implying that a provided class_instance was corrupted/stale.
> 
> Rediscover the engine for each opened client so that
> it points to a proper valid entry for exec_queue_create.
> 
> Cc: Christoph Manszewski <christoph.manszewski@intel.com>
> Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Looks good to me.

It is:
Reviewed-by: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> ---
>  tests/intel/xe_eudebug.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/intel/xe_eudebug.c b/tests/intel/xe_eudebug.c
> index 40e07ddf6..a2b2f3596 100644
> --- a/tests/intel/xe_eudebug.c
> +++ b/tests/intel/xe_eudebug.c
> @@ -1007,7 +1007,6 @@ static void test_basic_discovery(int fd, unsigned int flags, bool match_opposite
>  #define DISCOVERY_VM_BIND		(1 << 3)
>  static void run_discovery_client(struct xe_eudebug_client *c)
>  {
> -	struct drm_xe_engine_class_instance *hwe = NULL;
>  	int fd[RESOURCE_COUNT], i;
>  	bool skip_sleep = c->flags & (DISCOVERY_DESTROY_RESOURCES | DISCOVERY_CLOSE_CLIENT);
>  	uint64_t addr = 0x1a0000;
> @@ -1015,20 +1014,15 @@ static void run_discovery_client(struct xe_eudebug_client *c)
>  	srand(getpid());
>  
>  	for (i = 0; i < RESOURCE_COUNT; i++) {
> +		struct drm_xe_engine_class_instance *hwe = NULL;
> +
>  		fd[i] = xe_eudebug_client_open_driver(c);
>  
> -		if (!i) {
> -			bool found = false;
> +		/* Get first */
> +		xe_eudebug_for_each_engine(fd[i], hwe)
> +			break;
>  
> -			xe_for_each_engine(fd[0], hwe) {
> -				if (hwe->engine_class == DRM_XE_ENGINE_CLASS_COMPUTE ||
> -				    hwe->engine_class == DRM_XE_ENGINE_CLASS_RENDER) {
> -					found = true;
> -					break;
> -				}
> -			}
> -			igt_assert(found);
> -		}
> +		igt_assert(hwe);
>  
>  		/*
>  		 * Give the debugger a break in event stream after every