[igt-dev] [PATCH i-g-t] tests/intel/xe_vm : Validate tile mask

[Kumar, Janga Rahul]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Friday, September 15, 2023 1:58 AM
> To: Kumar, Janga Rahul <janga.rahul.kumar at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Gandi, Ramadevi
> <ramadevi.gandi at intel.com>; Summers, Stuart
> <stuart.summers at intel.com>
> Subject: Re: [PATCH i-g-t] tests/intel/xe_vm : Validate tile mask
> 
> On Thu, Sep 14, 2023 at 03:51:56PM +0530, janga.rahul.kumar at intel.com
> wrote:
> > From: Janga Rahul Kumar <janga.rahul.kumar at intel.com>
> >
> > Tile mask should be always less than or equal to tile count.
> >
> > Cc: Stuart Summers <stuart.summers at intel.com>
> > Cc: Matt Roper <matthew.d.roper at intel.com>
> > Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar at intel.com>
> > ---
> >  tests/intel/xe_vm.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/intel/xe_vm.c b/tests/intel/xe_vm.c index
> > 4952ea786..3b11b8a68 100644
> > --- a/tests/intel/xe_vm.c
> > +++ b/tests/intel/xe_vm.c
> > @@ -10,6 +10,7 @@
> >   */
> >
> >  #include "igt.h"
> > +#include "igt_sysfs.h"
> >  #include "lib/igt_syncobj.h"
> >  #include "lib/intel_reg.h"
> >  #include "xe_drm.h"
> > @@ -905,10 +906,11 @@ test_bind_array(int fd, struct
> drm_xe_engine_class_instance *eci, int n_execs,
> >  		uint64_t pad;
> >  		uint32_t data;
> >  	} *data;
> > -	int i, b;
> > +	int i, b, tile_count;
> >
> >  	igt_assert(n_execs <= BIND_ARRAY_MAX_N_EXEC);
> >
> > +	tile_count = xe_sysfs_get_num_tiles(fd);
> >  	vm = xe_vm_create(fd, DRM_XE_VM_CREATE_ASYNC_BIND_OPS, 0);
> >  	bo_size = sizeof(*data) * n_execs;
> >  	bo_size = ALIGN(bo_size + xe_cs_prefetch_size(fd), @@ -928,6
> +930,10
> > @@ test_bind_array(int fd, struct drm_xe_engine_class_instance *eci, int
> n_execs,
> >  		bind_ops[i].range = bo_size;
> >  		bind_ops[i].addr = addr;
> >  		bind_ops[i].tile_mask = 0x1 << eci->gt_id;
> 
> The problem is here --- you shouldn't be building a tile mask from a GT ID
> (you'd want to use the tile ID instead).  Tiles and GTs are different things and
> you can't use them interchangeably.
Agree. 	I have explored the options to break this derivation of tile id from GT id. Currently KMD doesn't provide a framework to get tile id based on GT id or doesn't advertise the tile count as part of XE_QUERY(Which am planning to add). Currently KMD share tile_count as part of debugfs "info" and IGT is counting tiles based on sysfs path of tiles.

@Roper, Matthew D
Is there any other way to UMD/IGT can get the tile info on which a GT is present so that it can bind a VMA on that particular tile?

> 
> > +
> > +		if (bind_ops[i].tile_mask >= tile_count)
> > +			bind_ops[i].tile_mask = tile_count - 1;
> 
> This check doesn't seem to make sense.  If you're trying to target the second
> tile, then your tile mask would be 0x2, which is equal to tile_count on a 2-tile
> system (so following this condition would just force you back to a mask
> representing the first tile no matter what).
> 
> And if you were on a 4-tile system, then trying to target either the third or
> fourth tiles would mean the mask is 0x8 or 0x4, both of which match the
> condition here and cause you to update the mask to 0x3.  And even higher
> tile counts would similarly be broken.
Understood. Thanks for detailed explanation. Will update the tile_mask to '0' (bind VMA to all tiles present). Test behaviour won't be impacted.

Thanks,
Rahul
> 
> 
> Matt
> 
> > +
> >  		bind_ops[i].op = XE_VM_BIND_OP_MAP |
> XE_VM_BIND_FLAG_ASYNC;
> >  		bind_ops[i].region = 0;
> >  		bind_ops[i].reserved[0] = 0;
> > --
> > 2.25.1
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation