Re: [PATCH 00/16] Add new debug infrastructure for configfs

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

On Sun, 2026-09-27 at 20:34 -0700, Matthew Brost wrote:
> On Thu, Sep 24, 2026 at 11:01:19PM +0000, Stuart Summers wrote:
> 
> Thanks for the work here. I was going to look in depth but
> immediately
> have a question.
> 
> > Add a new configfs debug group. The intent of this structure is
> > to allow us to separate ABI facing configfs entries from those
> > which are purely for debug purposes. And it allows us more
> > flexibility
> 
> Why not use debugfs for debug knobs and keep configfs as a strict
> ABI? Is
> the reasoning that everything in configfs is considered a boot-time
> configuration?
> 
> In the past, I've added debug tunables as module parameters that
> probably should have lived in either configfs or debugfs.
> 
> I'm just struggling to understand the distinction here. What really
> defines a "normal" configfs setting versus a "debug" configfs
> setting?
> For example, let's say we have a tuning parameter that a
> knowledgeable
> customer may want to adjust. Where should that live?
> 
> I'm not opposed to this; I'm just a bit fuzzy on the rules and the
> reasoning behind them. That said, if we can't get clear rules then
> I'd
> say lump everything together.

Yeah at least from my perspective, module parameters are things that
apply to all devices on the system at probe time (yes we can change
some of these at runtime, but typically they are probe-time
parameters).

debugfs is used for either information reporting or for runtime
configuration for debug purposes (vs sysfs for runtime configuration
for "production" purposes - ABI)

configfs is used for probe-time configurations that need to be per
device.

I know in i915 we had overloaded the modparams with a debugfs layer
that looks a like what I have here for the X-params. At least when we
were discussing for Xe, we wanted to move more towards configfs for
these kinds of parameters.

I just want a way to be able to quickly and easily add new parameters
upstream that are needed for low level software/hardware debug with a
clear way to implement those. If we want to go the debugfs route, I'm
sure we can make that work too, but I want it to be really clear where
that should live. We do already have things like enable_psmi and
enable_multi_queue here that are doing exactly this - probe time, per-
device parameters.

In terms of how we define what is production and what is debug... I
think that's going to be a case-by-case basis. Essentially what I was
thinking is a production parameter is something a customer will be
using as part of a "normal" runtime flow (like firmware update for
survivability_mode). Whereas a debug parameter is something we don't
want a customer to use without explicitly understanding what they are
doing - they are wrapped in a debug kconfig and have explicit
documentation. One example we don't have here now but could add in the
future is something like the enable_rc6 where we really don't want
customers to be setting this unless they are trying to debug something
with us - it would at least take a kernel rebuild from what is provided
in the distros.

I'm open to discussion. I just want to make sure we have a clear
process here so we can facilitate the parameters we need for debug.

Thanks,
Stuart

> 
> Matt
> 
> > in how we define those parameters used for debug.
> > 
> > Add a new infrastructure to this debug configfs group that lets us
> > easily define the parameters in a quick list. This is primarily
> > useful for simple, single-type parameters such as enable/disable
> > features or simple values passed. For more complex parameters,
> > we will still need to define these separately.
> > 
> > Pull the GuC target related changes from [1] to fit within
> > this new structure and add a new definition for guc_log_level
> > on top of the existing module parameter (to ensure we aren't
> > impacting existing users of the module parameter).
> > 
> > Note that the debug parameters here are all to be used "at your
> > own risk". Without having in depth knowledge of how these impact
> > the software and hardware, there could be unforeseen consequences
> > of setting them. As such, they are all wrapped in a
> > CONFIG_DRM_XE_DEBUG configfs option.
> > 
> > In terms of the patches here, I'm sorting the existing parameters
> > by name/type. I know we have a few other module parameters that
> > could migrate here, but I didn't want to overload this series
> > too much, so the focus for now is on the existing configfs entries
> > and demonstrating the new structures with the GuC log level and
> > target parameters.
> > 
> > I used GitHub Copilot with Claude pretty extensively through the
> > process here and attributed as such. Happy to answer any questions
> > around this. Took a bit of time getting back to this series around
> > other work, and in that time I was playing around with a few
> > different
> > models, hence some of the patches are showing multiple of them. I
> > tried to attribute each as I was implementing the changes.
> > 
> > I also decided to drop John Harrison from the NPK patch. It has
> > been modified quite a bit from the original, but more importantly
> > John is no longer with Intel and that email address isn't available
> > any more. If it makes a difference here, John and I had both
> > separately
> > implemented this same change at different occasions for debug. The
> > one I used to start that initial series was cherry-picked from his
> > latest variant.
> > 
> > v2:
> >  - In this second revision I did confirm that the guc_log_level
> >    module parameter is taking precedence over the configfs
> > parameter
> >    and ensured the other parameters seem to be autogenerating and
> >    working as expected.
> >  - I tried to address all the review feedback from the first
> >    revision, [2].
> >  - I also did another pass on the sorting since there were a few
> >    discrepancies I noticed in the first revision. I kept Gustavo's
> >    R-B on that one, but would like an ack before merging at least
> >    to confirm the patch is sane.
> >  - And finally I moved the getter functions into the X-macro
> >    generators so we can autogenerate more of the similar functions
> >    between the different parameters in that debug param list.
> > v3:
> >  - Address a couple of comments from Sashiko around GuC log level
> >    input checking and proper guard implementation.
> > v4:
> >  - More review feedback from Sashiko addressed...
> > v5:
> >  - Move the goto to a return (more Sashiko feedback) in the GuC
> >    log level setter before moving to the X-macro solution.
> > v6:
> >  - Make the autogenerated X-macro function names more specific to
> >    avoid naming collisions (Sashiko again).
> > v7:
> >  - Fix the couple of pre-existing bugs called out by Sashiko in
> >    the prior rev...
> >  - Make CONFIGFS_FS a required config for xe to avoid issues with
> >    stale values in the fallback getters.
> >  - Renamed disable_vram_page_offline to enable_vram_page_offline
> >    for a more consistent naming scheme (this was a new configfs
> >    entry added since the prior rev).
> >  - Converted survivability_mode to a u8 bitmap to allow for
> >    extendability in the future. Only bit 0 is defined, so the
> > behavior
> >    should be the same.
> >  - Added an enable_media module parameter at the end of the series.
> >    gt_types_allowed is debug-only, so this gives production builds
> > a
> >    supported way to leave the media GT alone. The modparam takes
> >    precedence over configfs.
> >  - Adjust the sorting to be alphabetical for the documentation
> >    specifically (Matt)
> > 
> > [1]: https://patchwork.freedesktop.org/series/162087/
> > [2]: https://patchwork.freedesktop.org/series/165879/
> > 
> > Stuart Summers (16):
> >   drm/xe: Guard configfs attribute reads in getters
> >   drm/xe/configfs: Fix out-of-bounds read in parse_wa_bb_lines()
> >   drm/xe/configfs: Copy wa_bb out under the configfs lock
> >   drm/xe: Require CONFIGFS_FS
> >   drm/xe: Invert vram_page_offline configfs attribute
> >   drm/xe: Make survivability_mode configfs attribute a bitmap
> >   drm/xe: Sort xe_config_device fields
> >   drm/xe: Split out configfs data structures
> >   drm/xe: Add a new debug focused configfs group
> >   drm/xe: Move debug configfs entries to xe_configfs_debug.c
> >   drm/xe/guc: Add configfs support for guc_log_level
> >   drm/xe/guc: Add support for NPK as a GuC log target
> >   drm/xe: Add infrastructure for debug configfs parameters
> >   drm/xe: Migrate existing debug configfs entries to params
> >     infrastructure
> >   drm/xe: Taint kernel when debug configfs parameters are set
> >   drm/xe: Add enable_media module parameter
> > 
> >  drivers/gpu/drm/xe/Kconfig                    |    1 +
> >  drivers/gpu/drm/xe/Makefile                   |    3 +-
> >  drivers/gpu/drm/xe/abi/guc_log_abi.h          |    8 +
> >  drivers/gpu/drm/xe/xe_configfs.c              | 1056 ++-----------
> > ----
> >  drivers/gpu/drm/xe/xe_configfs.h              |  124 +-
> >  drivers/gpu/drm/xe/xe_configfs_debug.c        |  899
> > ++++++++++++++
> >  drivers/gpu/drm/xe/xe_configfs_debug.h        |   48 +
> >  drivers/gpu/drm/xe/xe_configfs_debug_params.c |  158 +++
> >  drivers/gpu/drm/xe/xe_configfs_debug_params.h |  194 +++
> >  drivers/gpu/drm/xe/xe_configfs_types.h        |   60 +
> >  drivers/gpu/drm/xe/xe_defaults.h              |    6 +
> >  drivers/gpu/drm/xe/xe_drm_ras_types.h         |    4 +-
> >  drivers/gpu/drm/xe/xe_guc.c                   |   14 +-
> >  drivers/gpu/drm/xe/xe_guc_ads.c               |    1 +
> >  drivers/gpu/drm/xe/xe_guc_log.c               |    3 +-
> >  drivers/gpu/drm/xe/xe_hw_engine.c             |    1 +
> >  drivers/gpu/drm/xe/xe_lrc.c                   |   39 +-
> >  drivers/gpu/drm/xe/xe_module.c                |    6 +
> >  drivers/gpu/drm/xe/xe_module.h                |    1 +
> >  drivers/gpu/drm/xe/xe_pci.c                   |   20 +-
> >  drivers/gpu/drm/xe/xe_psmi.c                  |    3 +-
> >  drivers/gpu/drm/xe/xe_ras.c                   |    9 +-
> >  drivers/gpu/drm/xe/xe_rtp.c                   |    3 +-
> >  drivers/gpu/drm/xe/xe_survivability_mode.c    |    7 +-
> >  drivers/gpu/drm/xe/xe_ttm_vram_mgr.c          |    2 +-
> >  25 files changed, 1615 insertions(+), 1055 deletions(-)
> >  create mode 100644 drivers/gpu/drm/xe/xe_configfs_debug.c
> >  create mode 100644 drivers/gpu/drm/xe/xe_configfs_debug.h
> >  create mode 100644 drivers/gpu/drm/xe/xe_configfs_debug_params.c
> >  create mode 100644 drivers/gpu/drm/xe/xe_configfs_debug_params.h
> >  create mode 100644 drivers/gpu/drm/xe/xe_configfs_types.h
> > 
> > -- 
> > 2.43.0
> >