Re: [PATCH] drm/xe/uapi: Add DRM_XE_QUERY_CONFIG_FLAG_HAS_DIS_NULL_QUERY

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

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

> 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@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.

You don’t need {} here.

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.

> +
>  	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?

Matt

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