Re: [PATCH 04/10] drm/xe: Add a new debug focused configfs group

["Summers, Stuart" <stuart.summers@intel.com>]

On Sun, 2026-08-23 at 23:22 +0200, Michal Wajdeczko wrote:
> 
> 
> On 8/22/2026 12:08 AM, Matt Roper wrote:
> > On Thu, Aug 20, 2026 at 09:06:43PM +0000, Stuart Summers wrote:
> > > Add the skeleton code for a new debug specific configfs group.
> > > Just add the structure for now. Actual debug content will be
> > > added in a subsequent patch.
> > > 
> > > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > > Assisted-by: Copilot:claude-sonnet-4.6,claude-opus-4.7
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/Makefile            |  2 ++
> > >  drivers/gpu/drm/xe/xe_configfs.c       |  5 +++++
> > >  drivers/gpu/drm/xe/xe_configfs_debug.c | 14 ++++++++++++++
> > >  drivers/gpu/drm/xe/xe_configfs_debug.h | 13 +++++++++++++
> > >  drivers/gpu/drm/xe/xe_configfs_types.h |  3 +++
> > >  5 files changed, 37 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 92134709d998..0b71f311c9c4 100644
> > > --- a/drivers/gpu/drm/xe/Makefile
> > > +++ b/drivers/gpu/drm/xe/Makefile
> > > @@ -163,6 +163,8 @@ xe-$(CONFIG_HWMON) += xe_hwmon.o
> > >  
> > >  xe-$(CONFIG_PERF_EVENTS) += xe_pmu.o
> > >  xe-$(CONFIG_CONFIGFS_FS) += xe_configfs.o
> > 
> > Not really an issue with your patch here, but since we're messing
> > with
> > kconfig stuff it reminds me --- I'm not sure our pre-existing
> > configfs
> > kconfig handling is entirely correct (and I think I've seen kernel
> > test
> > robot warnings about that too).  The concern is that our header
> > file
> > does a typical IS_ENABLED(CONFIG_CONFIGFS) type of check to decide
> > between extern function declarations or noop stubs.  But if Xe is
> > built
> > into the bzImage, but configfs is configured as a .ko, then Xe
> > can't
> > reach the configfs functions.  I believe there's an IS_REACHABLE()
> > that
> > maybe we should be using instead, although I haven't got around to
> > actually looking closer and trying it out myself.
> 
> I was already trying to use IS_REACHABLE [1] but Jani is against it
> [2]
> 
> [1]
> https://patchwork.freedesktop.org/patch/696103/?series=159395&rev=1
> [2]
> https://lore.kernel.org/intel-xe/e22a535a6405412aaeb0f2251733840b72c93b56@intel.com/

Maybe we just add CONFIG_CONFIGFS as a requirement of CONFIG_DRM_XE?

But also I'd rather have this discussion separate from the patch series
here if possible. I didn't want to change anything fundamental here
other than adding the debug-only pieces.

Let me know what you think here.

Thanks,
Stuart

> 
> > 
> > Your actual changes in this patch look okay to me.
> > 
> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > 
> > 
> > Matt
> > 
> > > +xe_debug_configfs_obj-$(CONFIG_DRM_XE_DEBUG) :=
> > > xe_configfs_debug.o
> > > +xe-$(CONFIG_CONFIGFS_FS) += $(xe_debug_configfs_obj-y)
> > >  
> > >  # 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 029c9f682c8e..33aea1150c86 100644
> > > --- a/drivers/gpu/drm/xe/xe_configfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_configfs.c
> > > @@ -14,6 +14,7 @@
> > >  
> > >  #include "instructions/xe_mi_commands.h"
> > >  #include "xe_configfs.h"
> > > +#include "xe_configfs_debug.h"
> > >  #include "xe_defaults.h"
> > >  #include "xe_gt_types.h"
> > >  #include "xe_module.h"
> > > @@ -1242,6 +1243,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
> > >  
> > >         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..5f938450aed2
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/xe/xe_configfs_debug.h
> > > @@ -0,0 +1,13 @@
> > > +/* SPDX-License-Identifier: MIT */
> > > +/*
> > > + * Copyright © 2026 Intel Corporation
> > > + */
> > > +#ifndef _XE_CONFIGFS_DEBUG_H_
> > > +#define _XE_CONFIGFS_DEBUG_H_
> > > +
> > > +#if IS_ENABLED(CONFIG_DRM_XE_DEBUG) &&
> > > IS_ENABLED(CONFIG_CONFIGFS_FS)
> > > +struct config_item_type;
> > > +extern const struct config_item_type xe_configfs_debug_type;
> > > +#endif
> > > +
> > > +#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 3be7d6160b4c..e22b9424f719 100644
> > > --- a/drivers/gpu/drm/xe/xe_configfs_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_configfs_types.h
> > > @@ -22,6 +22,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
> > >  
> > >         /*
> > >          * Fields sorted by type (largest first) then name:
> > > struct arrays,
> > > -- 
> > > 2.43.0
> > > 
> > 
>