On Mon, 2026-06-22 at 13:30 +0300, Joonas Lahtinen wrote:
> (+ 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.
There is a kernel context switch cost to consider.
Also, because this only checks one feature, we would eventually need
multiple uAPI calls to check all the features support.
We could avoid this by adding a single flag to an existing query,
allowing us to check support for all features in a single uAPI call.
>
> 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?
VM_GET_PROPERTY_FAULTS call would work in any platform but would only
only return fault information in platforms with has_usm support.
"
int xe_pagefault_init(struct xe_device *xe)
{
int err, i;
if (!xe->info.has_usm)
return 0;
"
>
> 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.
Someone just reading xe_drm.h would understand that they need to check
for DRM_XE_QUERY_CONFIG_FLAG_HAS before using a feature. Without that,
a person lacking historical context might blindly call
VM_GET_PROPERTY_FAULTS, unaware that some KMD versions don't even
support it.
>
> 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
