Re: [PATCH v4 19/33] drm/xe/uapi: Add DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Feb 07, 2025 at 06:43:11AM -0700, Upadhyay, Tejas wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Thomas
> > Hellström
> > Sent: Friday, February 7, 2025 6:35 PM
> > To: Brost, Matthew <matthew.brost@intel.com>; intel-
> > xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> > Cc: Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com>;
> > apopple@nvidia.com; airlied@gmail.com; simona.vetter@ffwll.ch;
> > felix.kuehling@amd.com; dakr@kernel.org
> > Subject: Re: [PATCH v4 19/33] drm/xe/uapi: Add
> > DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR
> > 
> > On Wed, 2025-01-29 at 11:51 -0800, Matthew Brost wrote:
> > > Add the DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR device
> > query
> > > flag, which indicates whether the device supports CPU address
> > > mirroring.
> > > The
> > > intent is for UMDs to use this query to determine if a VM can be set
> > > up with CPU address mirroring. This flag is implemented by checking if
> > > the device supports GPU faults.
> > >
> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > 
> > > ---
> > >  drivers/gpu/drm/xe/xe_query.c | 5 ++++-
> > >  include/uapi/drm/xe_drm.h     | 3 +++
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_query.c
> > > b/drivers/gpu/drm/xe/xe_query.c index c059639613f7..40f56eaf98fa
> > > 100644
> > > --- a/drivers/gpu/drm/xe/xe_query.c
> > > +++ b/drivers/gpu/drm/xe/xe_query.c
> > > @@ -333,8 +333,11 @@ static int query_config(struct xe_device *xe,
> > > struct drm_xe_device_query *query)
> > >  	config->info[DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID] =
> > >  		xe->info.devid | (xe->info.revid << 16);
> > >  	if (xe_device_get_root_tile(xe)->mem.vram.usable_size)
> > > -		config->info[DRM_XE_QUERY_CONFIG_FLAGS] =
> > > +		config->info[DRM_XE_QUERY_CONFIG_FLAGS] |=
> > >  			DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM;
> > > +	if (xe->info.has_usm)
> > > +		config->info[DRM_XE_QUERY_CONFIG_FLAGS] |=
> > > +
> > 	DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR
> > > ;
> > >  	config->info[DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT] =
> > >  		xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K
> > > : SZ_4K;
> > >  	config->info[DRM_XE_QUERY_CONFIG_VA_BITS] = xe-
> > > >info.va_bits;
> > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > > index b86dc1b4c2fe..37e54ca6ffe9 100644
> > > --- a/include/uapi/drm/xe_drm.h
> > > +++ b/include/uapi/drm/xe_drm.h
> > > @@ -393,6 +393,8 @@ struct drm_xe_query_mem_regions {
> > >   *
> > >   *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the
> > > device
> > >   *      has usable VRAM
> > > + *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR - Flag is
> > set
> > > if the
> > > + *      device has CPU address mirroring support
> > >   *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory
> > alignment
> > >   *    required by this device, typically SZ_4K or SZ_64K
> > >   *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual
> > > address @@ -409,6 +411,7 @@ struct drm_xe_query_config {
> > >  #define DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID	0
> > >  #define DRM_XE_QUERY_CONFIG_FLAGS			1
> > >  	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM	(1 << 0)
> > > +	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR
> > 	(1
> > > << 1)
> 
> I don’t know how we handle this, but https://patchwork.freedesktop.org/patch/635834/ is getting merged soon, will conflict with (1 << 1). If its like whoever merges first then it should be ok to keep it this way and you can add my r-o-b. Or else if we should adjust now!

Thanks for the heads up.

I think whoever gets in first gets the lowest bit and fixup the later
series in the following post or at merge time.

Matt

> 
> Anyways,
> Reviewed-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
> 
> > >  #define DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT		2
> > >  #define DRM_XE_QUERY_CONFIG_VA_BITS			3
> > >  #define DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY	4
>