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

[Gustavo Sousa <gustavo.sousa@intel.com>]

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.

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

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