Re: [PATCH 4/9] drm/xe: Add a new debug focused configfs group

Gustavo Sousa
2026-05-04T17:29:09Z

"Summers, Stuart" <stuart.summers@intel.com> writes:

> On Mon, 2026-05-04 at 12:42 -0300, Gustavo Sousa wrote:
>> Stuart Summers <stuart.summers@intel.com> writes:
>>
>> > Add the skeleton code for a new debug specific configfs group.
>> >
>> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
>> > Assisted-by: Copilot:claude-opus-4.7
>> > ---
>> >  drivers/gpu/drm/xe/Makefile            |  2 ++
>> >  drivers/gpu/drm/xe/xe_configfs.c       |  4 ++++
>> >  drivers/gpu/drm/xe/xe_configfs_debug.c | 14 ++++++++++++++
>> >  drivers/gpu/drm/xe/xe_configfs_debug.h |  8 ++++++++
>> >  drivers/gpu/drm/xe/xe_configfs_types.h |  7 +++++++
>> >  5 files changed, 35 insertions(+)
>> >  create mode 100644 drivers/gpu/drm/xe/xe_configfs_debug.c
>> >  create mode 100644 drivers/gpu/drm/xe/xe_configfs_debug.h
>> >
>> > diff --git a/drivers/gpu/drm/xe/Makefile
>> > b/drivers/gpu/drm/xe/Makefile
>> > index 09661f079d03..b58667b0b18e 100644
>> > --- a/drivers/gpu/drm/xe/Makefile
>> > +++ b/drivers/gpu/drm/xe/Makefile
>> > @@ -161,6 +161,8 @@ xe-$(CONFIG_HWMON) += xe_hwmon.o
>> >
>> >  xe-$(CONFIG_PERF_EVENTS) += xe_pmu.o
>> >  xe-$(CONFIG_CONFIGFS_FS) += xe_configfs.o
>> > +xe_debug_configfs_obj-$(CONFIG_DRM_XE_DEBUG) :=
>> > xe_configfs_debug.o
>> > +xe-$(CONFIG_CONFIGFS_FS) += $(xe_debug_configfs_obj-y)
>>
>> What are the downsides of keeping this available for non-debug
>> builds?
>>
>> I imagine that the debug configfs could be useful for someone not
>> involved in kernel development working together with the developers
>> to
>> debug issues without needing to rebuild their kernel.
>
> So the focal parameter we had in i915 was enable_rc6. Something like
> this is what we explicitly don't want to expose to a general user as it
> can have unforeseen consequences (like having very high power usage).
> But, having the ability to disable rc6 for debug purposes as a
> developer is extremely useful at narrowing down certain types of bugs
> at the hardware/software interface.

We could probably keep the debug stuff available but cause a taint if
they ever get used?

The scenario I had in mind is someone who is not familiar with building
their own kernel working together with the development community on a
hard-to-reproduce issue and some of the debug configfs attributes being
useful to gather more relevant data.

--
Gustavo Sousa

>
> That's just one example, but there are a bunch of things that could fit
> this category and I expect we'll have even more use cases over time.
>
> Right now we have to hold these types of changes in a side branch not
> exposed externally and I really want to converge our processes so we
> can more easily make these kinds of changes and let us debug directly
> out of drm-tip.
>
>>
>> >
>> >  # graphics virtualization (SR-IOV) support
>> >  xe-y += \
>> > diff --git a/drivers/gpu/drm/xe/xe_configfs.c
>> > b/drivers/gpu/drm/xe/xe_configfs.c
>> > index 12b7fe65446d..85df8ce5cf2a 100644
>> > --- a/drivers/gpu/drm/xe/xe_configfs.c
>> > +++ b/drivers/gpu/drm/xe/xe_configfs.c
>> > @@ -1006,6 +1006,10 @@ static struct config_group
>> > *xe_config_make_device_group(struct config_group *gro
>> >                 config_group_init_type_name(&dev->sriov, "sriov",
>> > &xe_config_sriov_type);
>> >                 configfs_add_default_group(&dev->sriov, &dev-
>> > >group);
>> >         }
>> > +#if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
>> > +       config_group_init_type_name(&dev->debug, "debug",
>> > &xe_configfs_debug_type);
>> > +       configfs_add_default_group(&dev->debug, &dev->group);
>> > +#endif
>>
>> I guess there isn't much we can do about it today, but, based on what
>> I
>> read in configfs documentation a while ago, I suspect it wasn't
>> really
>> designed with arbitrary hierarchies of config items in mind.  It
>> would
>> be nice if there was an API for that though.
>>
>> Basically, a config group is expected to be a directory to contain a
>> set
>> of config items of the same type, which is not the case on our
>> current
>> usage (i.e. the "sriov" and now the "debug" group).
>>
>> I guess we are able to get our desired behavior (basically just a
>> subdirectory behaving as a config item) by not implementing the
>> "make_item" hook.  Not sure if that's an expected use of the API or
>> if
>> that's more of a hack on our side and could come back to bite us in
>> the
>> future.
>
> Hm.. ok that's good feedback. I do like bucketing these in groups and
> subdirectories. It also makes it easy to show or not show based on the
> Kconfig. But let me go through and make sure there isn't something more
> suited here. Happy to hear any other feedback as well here.
>
> Thanks,
> Stuart
>
>>
>> --
>> Gustavo Sousa
>>
>> >
>> >         mutex_init(&dev->lock);
>> >
>> > diff --git a/drivers/gpu/drm/xe/xe_configfs_debug.c
>> > b/drivers/gpu/drm/xe/xe_configfs_debug.c
>> > new file mode 100644
>> > index 000000000000..45617282cec5
>> > --- /dev/null
>> > +++ b/drivers/gpu/drm/xe/xe_configfs_debug.c
>> > @@ -0,0 +1,14 @@
>> > +// SPDX-License-Identifier: MIT
>> > +/*
>> > + * Copyright © 2026 Intel Corporation
>> > + */
>> > +
>> > +#include <linux/configfs.h>
>> > +#include <linux/module.h>
>> > +
>> > +#include "xe_configfs_debug.h"
>> > +#include "xe_configfs_types.h"
>> > +
>> > +const struct config_item_type xe_configfs_debug_type = {
>> > +       .ct_owner       = THIS_MODULE,
>> > +};
>> > diff --git a/drivers/gpu/drm/xe/xe_configfs_debug.h
>> > b/drivers/gpu/drm/xe/xe_configfs_debug.h
>> > new file mode 100644
>> > index 000000000000..01170dc2f97e
>> > --- /dev/null
>> > +++ b/drivers/gpu/drm/xe/xe_configfs_debug.h
>> > @@ -0,0 +1,8 @@
>> > +/* SPDX-License-Identifier: MIT */
>> > +/*
>> > + * Copyright © 2026 Intel Corporation
>> > + */
>> > +#ifndef _XE_CONFIGFS_DEBUG_H_
>> > +#define _XE_CONFIGFS_DEBUG_H_
>> > +
>> > +#endif /* _XE_CONFIGFS_DEBUG_H_ */
>> > diff --git a/drivers/gpu/drm/xe/xe_configfs_types.h
>> > b/drivers/gpu/drm/xe/xe_configfs_types.h
>> > index 935097aafa96..c9d94a3c26a7 100644
>> > --- a/drivers/gpu/drm/xe/xe_configfs_types.h
>> > +++ b/drivers/gpu/drm/xe/xe_configfs_types.h
>> > @@ -24,6 +24,9 @@ struct wa_bb {
>> >  struct xe_config_group_device {
>> >         struct config_group group;
>> >         struct config_group sriov;
>> > +#if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
>> > +       struct config_group debug;
>> > +#endif
>> >
>> >         struct xe_config_device {
>> >                 struct wa_bb
>> > ctx_restore_mid_bb[XE_ENGINE_CLASS_MAX];
>> > @@ -56,4 +59,8 @@ static inline struct xe_config_device
>> > *xe_configfs_to_device(struct config_item
>> >         return &xe_configfs_to_group_device(item)->config;
>> >  }
>> >
>> > +#if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
>> > +extern const struct config_item_type xe_configfs_debug_type;
>> > +#endif
>> > +
>> >  #endif /* _XE_CONFIGFS_TYPES_H_ */
>> > --
>> > 2.43.0