[v2,00/59] Fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y

Message

Jim Cromie March 20, 2025, 6:51 p.m. UTC

This series fixes dynamic-debug's support for DRM debug-categories.
Classmaps-v1 evaded full review, and got committed in 2 chunks:

  b7b4eebdba7b..6ea3bf466ac6	# core dyndbg changes
  0406faf25fb1..ee7d633f2dfb	# drm adoption

Then DRM-CI found a regression when booting with drm.debug=<initval>;
the pr_debug callsites' static-keys under the drm-dbgs in drm.ko got
enabled, but those in drivers & helpers did not, since they weren't
yet modprobed, and didn't yet exist.

Design Review / Restatemment:

My primary design goal for dyndbg-classmaps was DIRECT support of DRM,
with NO api changes, starting with the enum drm_debug_category typing,
and keeping as much of the existing macro stack as unaltered as possible.

Some UUID-ish thing was OO-scope; pessimizing unseen optimizations on
compile-time constant ints (that could fit in a byte), across DRM's
macro stack, with ~5k expansions, would be ungood.

Immediate consequences:

= unique integers are *hard* to coordinate across the whole kernel.

= 0 is not special - DRM_UT_CORE is already used.
  other users probably want it too
  
= 0..N are special - they're exposed in sysfs knobs.
  ie: echo 0x1ff > /sys/module/drm/parameters/debug
  all classmap-param users get 0..N on their knob.

= DRM_UT_<*> has only 10 categories/classes
  small enough for DIRECT storage inside the _ddebug descriptor

= DRM's enum gives us both class-names and class/category values.
  classnames === stringified DRM_UT_<*>
  class_ids  === bare DRM_UT_<*>
  all future classmap users are expected to follow this model.
  
We need to "privatize" the class_id's, per module.
We can do this in a few steps:

= modules declare their map: classnames => Ids.
  author defined, user oriented
  classnames/strings is wide, expressive interface
  2-levels used, DRM_UT_*, no 3-levels yet;
  _UT could vary, but has no meaning now.

We can use the known (and desired) ordering/contiguity of the 0..Ns
here, and of the array of classnames, to simplify the specification of
the map:

  _CLASSMAP_DEFINE (_base-ID/start-of-range, list-of-classnames)

no sparse maps, no reorder-maps, nothing extra to think about.
Just classnames to manipulate class_ids/categories.

= dyndbg refuses direct selection by class_ids.
  this privatizes them
  "speak my class-name if you expect a response".

= dyndbg requires "grammar access only"
  ie: "class DRM_UT_CORE +p" >control
  ie: "name the class to change it"
  class-param handler feeds cmd-strings thru ddebug_query().

= classname --> class_id lookup "validates" the user.
  unknown classnames can be discarded, per module.
  other modules get the same,
  no other class-user would accidentally know "DRM_UT_CORE"

= with private class_ids per module.
  a small 0..N range is enough.
  32 is a practical limit for a single classmap,
  if you think "echo 0x12345678 > $sysknob" is practical.
  16 is more practical, DRM has 10.
  we have 63. this allows..

= multiple classmaps are supported
  as long as the mapped class_ids dont conflict.
  a fair requirement for a narrow, future use-case.
  this is now verified in this patchset.
  7x 8-bit classes is workable (if you'd like)

= "legacy" class.
  all existing (un-class'd) pr_debugs.
  class_id is a 6-bit integer,
  so all existing pr_debugs need a default.
  63 = 2^6-1 is our _DFLT
  0..62 is then available for named classes. (0 is not special)
  63 is "legacy".


And now on to the nascent disagreement between Jason and myself:
classmaps-v1 "protected" new classes from changes by legacy/_DFLT
queries.  Jason saw this as a "special-case" conferred on the class
keyword; all other keywords have no selectivity until they're queried.

After realizing this was unsettled, I found a technical fix: split the
class-mismatch case to queried-class-mismatch (not special, so skip)
OR class'd-site-vs-DFLT-query.  Now we can make a policy decision,
named: ddebug_client_module_protects_classes().

IIRC @DVetter expressed unconcern about protecting DRM_UT_* classes
from unintended alteration, so the above macro is set false, per
Jason's preference.

Should dispositions change, we could perhaps just set the macro true,
since DRM is the only classmap usage so far.  Or perhaps make it the
using module's choice, with a flag in _ddebug_info, and maybe a new
DD_CLASSMAP_TYPE to convey it to dyndbg wo api tweaks.

Previous Rev(s):

https://lore.kernel.org/lkml/[e-mail]/
Ive incorporated the Reviewed-by:s and Tested-by:s offered there.

DRM-CI runs:

https://patchwork.freedesktop.org/series/135669/
https://patchwork.freedesktop.org/series/139147/

Changes since:

= one more mea-culpa fix
= squash 3 verbose-tweak patches together
= lots of cleanup/refactoring patches brought forward
  or squashed, reducing churn.

= regularize section names & linker symbols.
  any naming conventions I missed ?

= new _DPRINTK flags for LOOKUP, CACHED_PREFIX
  use LOOKUP to refine dynamic_emit_prefix()

= cleanup most spurious name variances:
  s/\bdd_|\bddebug_/_ddebug_/g
  most of s/DYNDBG/DYNAMIC_DEBUG/.
  
= moved doc patches to back of dydnbg-core part.
  more wordsmithing / bikeshedding chances
  the classmaps-info might be over-specifying

= one trailing RFC patch for DRM to consider
  attempt to link in a drm_dbg_client.o classmap-user everwhere
  _CLASSMAP_USE at a distance.

Jim Cromie (59):
  vmlinux.lds.h: fixup HEADERED_SECTION{,_BY} macros
  docs/dyndbg: update examples \012 to \n
  test-dyndbg: fixup CLASSMAP usage error
  dyndbg: reword "class unknown," to "class:_UNKNOWN_"
  dyndbg: make ddebug_class_param union members same size
  dyndbg: drop NUM_TYPE_ARRAY
  dyndbg: reduce verbose/debug clutter
  dyndbg: refactor param_set_dyndbg_classes and below
  dyndbg: tighten fn-sig of ddebug_apply_class_bitmap
  dyndbg: replace classmap list with a vector
  dyndbg: macrofy a 2-index for-loop pattern
  dyndbg,module: make proper substructs in _ddebug_info
  dyndbg: add 2 new _DPRINTK_FLAGS_: INCL_LOOKUP, PREFIX_CACHED
  dyndbg: split _emit_lookup() out of dynamic_emit_prefix()
  dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
  dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code
  dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP
  selftests-dyndbg: add tools/testing/selftests/dynamic_debug/*
  dyndbg: detect class_id reservation conflicts
  dyndbg: check DYNDBG_CLASSMAP_DEFINE args at compile-time
  dyndbg-test: change do_prints testpoint to accept a loopct
  dyndbg-API: promote DYNAMIC_DEBUG_CLASSMAP_PARAM to API
  dyndbg: move .mod_name from/to structs ddebug_table/_ddebug_info
  dyndbg: treat comma as a token separator
  selftests-dyndbg: add comma_terminator_tests
  dyndbg: split multi-query strings with %
  selftests-dyndbg: test_percent_splitting
  selftests-dyndbg: add test_mod_submod
  dyndbg: change __dynamic_func_call_cls* macros into expressions
  dyndbg: drop "protection" of class'd pr_debugs from legacy queries
  docs/dyndbg: explain new delimiters: comma, percent
  docs/dyndbg: explain flags parse 1st
  docs/dyndbg: add classmap info to howto (TBD)
  checkpatch: dont warn about unused macro arg on empty body
  drm: use correct ccflags-y spelling
  drm-dyndbg: adapt drm core to use dyndbg classmaps-v2
  drm-dyndbg: adapt DRM to invoke DYNAMIC_DEBUG_CLASSMAP_PARAM
  drm-print: fix config-dependent unused variable
  drm-dyndbg: DRM_CLASSMAP_USE in amdgpu driver
  drm-dyndbg: DRM_CLASSMAP_USE in i915 driver
  drm-dyndbg: DRM_CLASSMAP_USE in drm_crtc_helper
  drm-dyndbg: DRM_CLASSMAP_USE in drm_dp_helper
  drm-dyndbg: DRM_CLASSMAP_USE in nouveau
  drm-dyndbg: add DRM_CLASSMAP_USE to Xe driver
  drm-dyndbg: add DRM_CLASSMAP_USE to virtio_gpu
  drm-dyndbg: add DRM_CLASSMAP_USE to simpledrm
  drm-dyndbg: add DRM_CLASSMAP_USE to bochs
  drm-dyndbg: add DRM_CLASSMAP_USE to etnaviv
  drm-dyndbg: add DRM_CLASSMAP_USE to gma500 driver
  drm-dyndbg: add DRM_CLASSMAP_USE to radeon
  drm-dyndbg: add DRM_CLASSMAP_USE to vmwgfx driver
  drm-dyndbg: add DRM_CLASSMAP_USE to vkms driver
  drm-dyndbg: add DRM_CLASSMAP_USE to udl driver
  drm-dyndbg: add DRM_CLASSMAP_USE to mgag200 driver
  drm-dyndbg: add DRM_CLASSMAP_USE to the gud driver
  drm-dyndbg: add DRM_CLASSMAP_USE to the qxl driver
  drm-dyndbg: add DRM_CLASSMAP_USE to the drm_gem_shmem_helper driver
  drm: restore CONFIG_DRM_USE_DYNAMIC_DEBUG un-BROKEN
  drm: RFC - make drm_dyndbg_user.o for drm-*_helpers, drivers

 .../admin-guide/dynamic-debug-howto.rst       | 127 +++-
 MAINTAINERS                                   |   3 +-
 drivers/gpu/drm/Kconfig                       |   3 +-
 drivers/gpu/drm/Makefile                      |  12 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  12 +-
 drivers/gpu/drm/display/drm_dp_helper.c       |  12 +-
 drivers/gpu/drm/drm_crtc_helper.c             |  12 +-
 drivers/gpu/drm/drm_dyndbg_user.c             |  11 +
 drivers/gpu/drm/drm_gem_shmem_helper.c        |   1 +
 drivers/gpu/drm/drm_print.c                   |  38 +-
 drivers/gpu/drm/etnaviv/etnaviv_drv.c         |   2 +
 drivers/gpu/drm/gma500/psb_drv.c              |   2 +
 drivers/gpu/drm/gud/gud_drv.c                 |   2 +
 drivers/gpu/drm/i915/i915_params.c            |  12 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c         |   2 +
 drivers/gpu/drm/nouveau/nouveau_drm.c         |  12 +-
 drivers/gpu/drm/qxl/qxl_drv.c                 |   2 +
 drivers/gpu/drm/radeon/radeon_drv.c           |   2 +
 drivers/gpu/drm/tiny/bochs.c                  |   2 +
 drivers/gpu/drm/tiny/simpledrm.c              |   2 +
 drivers/gpu/drm/udl/udl_main.c                |   2 +
 drivers/gpu/drm/virtio/virtgpu_drv.c          |   2 +
 drivers/gpu/drm/vkms/vkms_drv.c               |   2 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c           |   2 +
 drivers/gpu/drm/xe/xe_drm_client.c            |   2 +
 include/asm-generic/vmlinux.lds.h             |  12 +-
 include/drm/drm_print.h                       |  12 +
 include/linux/dynamic_debug.h                 | 225 ++++--
 kernel/module/main.c                          |  15 +-
 lib/Kconfig.debug                             |  24 +-
 lib/Makefile                                  |   3 +
 lib/dynamic_debug.c                           | 656 +++++++++++-------
 lib/test_dynamic_debug.c                      | 203 ++++--
 lib/test_dynamic_debug_submod.c               |  21 +
 scripts/checkpatch.pl                         |   2 +-
 tools/testing/selftests/Makefile              |   1 +
 .../testing/selftests/dynamic_debug/Makefile  |   9 +
 tools/testing/selftests/dynamic_debug/config  |   2 +
 .../dynamic_debug/dyndbg_selftest.sh          | 364 ++++++++++
 39 files changed, 1329 insertions(+), 501 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_dyndbg_user.c
 create mode 100644 lib/test_dynamic_debug_submod.c
 create mode 100644 tools/testing/selftests/dynamic_debug/Makefile
 create mode 100644 tools/testing/selftests/dynamic_debug/config
 create mode 100755 tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh