[PATCH] drm/xe/uapi: Add DRM_XE_QUERY_CONFIG_FLAG_HAS_DIS_NULL_QUERY

[Gote, Nitin R]

Hi Matthew,
> 
> On Mon, Jun 23, 2025 at 12:10:36PM +0530, Nitin Gote wrote:
> > Add the DRM_XE_QUERY_CONFIG_FLAG_HAS_DIS_NULL_QUERY device query
> flag
> > which indicates whether a device supports DIS_NULL_QUERY (Disable null
> 
> s/DIS_NULL_QUERY/DISABLE_NULL_QUERY

The DIS_NULL_QUERY naming convention is followed as per Bspec 57496.

> 
> > anyhit shader query mechanism). The intent is for UMDs to use this
> > query and opt-in DRM_XE_EXEC_QUEUE_DIS_NULL_QUERY flag to disable null
> > query mechanism for anyhit shader by setting DIS_NULL_QUERY bit of
> > RT_CTRL register for Xe2 IP.
> >
> > Signed-off-by: Nitin Gote <nitin.r.gote at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++++++++
> >  drivers/gpu/drm/xe/xe_query.c      | 3 ++-
> >  include/uapi/drm/xe_drm.h          | 8 ++++++++
> >  3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c
> > b/drivers/gpu/drm/xe/xe_exec_queue.c
> > index fee22358cc09..519f36db7cd0 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > @@ -26,6 +26,8 @@
> >  #include "xe_trace.h"
> >  #include "xe_vm.h"
> >  #include "xe_pxp.h"
> > +#include "xe_gt_mcr.h"
> > +#include "regs/xe_gt_regs.h"
> >
> >  enum xe_exec_queue_sched_prop {
> >  	XE_EXEC_QUEUE_JOB_TIMEOUT = 0,
> > @@ -693,6 +695,13 @@ int xe_exec_queue_create_ioctl(struct drm_device
> *dev, void *data,
> >  		}
> >  	}
> >
> > +	if (((GRAPHICS_VER(xe) >= 20) && (GRAPHICS_VER(xe) < 30)) &&
> > +	    (args->flags & DRM_XE_EXEC_QUEUE_DIS_NULL_QUERY) &&
> > +	    (eci[0].engine_class == DRM_XE_ENGINE_CLASS_RENDER ||
> > +	    eci[0].engine_class == DRM_XE_ENGINE_CLASS_COMPUTE)) {
> > +		xe_gt_mcr_multicast_write(q->gt, RT_CTRL, DIS_NULL_QUERY);
> > +	}
> 
> You will hit an error before you get here because of the if statement above (not
> shown in the diff) that sanitizes args->flags.
> 

Yes. Agree with you. I will add it.

> You don’t need {} here.
> 
Thanks, I will remove it.

> Also, a register write implies this would affect the entire GT, not just the exec
> queue flagged with “disable NULL query.” Is that really the intent? It doesn’t
> seem correct to me, but honestly, I’m not entirely sure what this patch is trying to
> do. If the intent is for this to be per-exec queue, then I think this needs to be
> implemented as some ring instructions in the LRC WA BB.
> 

Here, the intention is that for the already released platforms Xe2, 
we need to add an opt-in flag via uapi to disable null query for anyhit shader.
So that userspace can use this flag to disable anyhit shader query mechanism, and I think
that it is not possible in the LRC WA BB. And to me, this placement looks ok as this setting is 
effective post xe_exec call.

> > +
> >  	q->xef = xe_file_get(xef);
> >
> >  	/* user id alloc must always be last in ioctl to prevent UAF */ diff
> > --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > index e8e1743dcb1e..c5002b8c9ac0 100644
> > --- a/drivers/gpu/drm/xe/xe_query.c
> > +++ b/drivers/gpu/drm/xe/xe_query.c
> > @@ -344,7 +344,8 @@ static int query_config(struct xe_device *xe, struct
> drm_xe_device_query *query)
> >  		config->info[DRM_XE_QUERY_CONFIG_FLAGS] |=
> >
> 	DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR;
> >  	config->info[DRM_XE_QUERY_CONFIG_FLAGS] |=
> > -			DRM_XE_QUERY_CONFIG_FLAG_HAS_LOW_LATENCY;
> > +			(DRM_XE_QUERY_CONFIG_FLAG_HAS_LOW_LATENCY
> |
> > +
> 	DRM_XE_QUERY_CONFIG_FLAG_HAS_DIS_NULL_QUERY);
> >  	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
> > 8e8bbdec8c5c..a5bfba121360 100644
> > --- a/include/uapi/drm/xe_drm.h
> > +++ b/include/uapi/drm/xe_drm.h
> > @@ -397,6 +397,8 @@ struct drm_xe_query_mem_regions {
> >   *      has low latency hint support
> >   *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR - Flag is
> set if the
> >   *      device has CPU address mirroring support
> > + *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_DIS_NULL_QUERY - Flag is set if
> the
> > + *      device has null query support for anyhit shader.
> >   *  - %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 @@ -415,6 +417,7 @@ struct drm_xe_query_config {
> >  	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM	(1 << 0)
> >  	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_LOW_LATENCY	(1 <<
> 1)
> >  	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR
> 	(1 << 2)
> > +	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_DIS_NULL_QUERY	(1 <<
> 3)
> >  #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
> > @@ -1271,6 +1274,11 @@ struct drm_xe_exec_queue_create {
> >  	__u32 vm_id;
> >
> >  #define DRM_XE_EXEC_QUEUE_LOW_LATENCY_HINT	(1 << 0)
> > +	/*
> > +	 * DRM_XE_EXEC_QUEUE_DIS_NULL_QUERY flag is
> > +	 * use to disable null query check for Anyhit shader
> > +	 */
> > +#define DRM_XE_EXEC_QUEUE_DIS_NULL_QUERY	(1 << 1)
> 
> We should add proper kernel-doc for this flag.
> 
> It looks like we’re also missing proper kernel-doc for
> DRM_XE_EXEC_QUEUE_LOW_LATENCY_HINT. While you’re at it, could you add
> that as well?
> 
Sure. I will add kernel-doc for both macros.

> Matt
> 
> >  	/** @flags: flags to use for this exec queue */
> >  	__u32 flags;
> >
> > --
> > 2.25.1
> >