(+ Sima and dri-devel)
This goes very much against the previously discussed and agreed
direction[1].
The QUERY style IOCTL (like xe_vm_get_property_ioctl here) were specifically
designed to have built-in feature-checking to avoid need for anything
like this check.
Executing the query with size zero would be a pretty much no-overhead
call (there's one spinlock there) as we can see from the body of
xe_vm_get_property_helper:
if (!args->size) {
args->size = size;
return 0;
}
That trait got added to the QUERY style IOCTLs exactly to address the
requirement for low-overhead feature checking by Mesa folks back then.
So you execute all the validity checks, but avoid actual data copying.
Quoting José Roberto de Souza (2026-06-17 18:25:00)
> Not all Xe versions or Xe-supported devices support this feature, so this
> flag is required to make the UMD's life easier.
>
> v2:
> - DRM_XE_QUERY_CONFIG_FLAG_HAS_VM_GET_PROPERTY_FAULTS should be set on
> info[DRM_XE_QUERY_CONFIG_FLAGS].
>
> Signed-off-by: José Roberto de Souza <[email protected]>
> ---
> drivers/gpu/drm/xe/xe_query.c | 4 ++++
> include/uapi/drm/xe_drm.h | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index dc975f595368..f187a264756a 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -354,6 +354,10 @@ static int query_config(struct xe_device *xe, struct
> drm_xe_device_query *query)
> DRM_XE_QUERY_CONFIG_FLAG_HAS_DISABLE_STATE_CACHE_PERF_FIX;
> config->info[DRM_XE_QUERY_CONFIG_FLAGS] |=
> DRM_XE_QUERY_CONFIG_FLAG_HAS_PURGING_SUPPORT;
> + if (xe->info.has_usm) {
> + config->info[DRM_XE_QUERY_CONFIG_FLAGS] |=
> + DRM_XE_QUERY_CONFIG_FLAG_HAS_VM_GET_PROPERTY_FAULTS;
> + }
I think it kind of proves the validness of concerns in [1] that the
has_usm check you add here doesn't have much relevance to the actual
VM_GET_PROPERTY_FAULTS IOCTL being available or functional.
If you think the VM_GET_PROPERTY_FAULTS should be gated by the has_usm
condition, then you should have updated the actual IOCTL, no?
The whole point of the original discussions resulting in [1] was that
it is such an easy error to make the capability flag check and the
actual IOCTL availability differ that it should be avoided.
The fact that such an error is in this very patch still after reading
through the original reasoning from Sima kind of underlines why the
point still stands strongly.
Regards, Joonas
[1] https://www.spinics.net/lists/intel-gfx/msg325342.html
