[PATCH 36/61] dyndbg: fail modprobe if classmaps have conflicting class-id ranges
Jim Cromie
jim.cromie at gmail.com
Wed Sep 25 18:10:28 UTC 2024
All classes used by a module must share 0..63 class-id space, that is
their respective base,+length reservations cannot overlap. This might
lead to possibly subtle misbehavior of dynamic-debug, which is bad.
Detect these class-id range overlaps and fail modprobe.
ddebug_class_check_range() implements the range check, accumulating
the reserved-ids as it examines each class. It probably should use
bitmaps.
Both ddebug_attach_*module_classes() call it once they've found a
range of classmaps matching the module-name, as they may apply kparam
inputs which control those classmaps. So the reserved-id check
applies to classmaps both (DYNDBG_CLASSMAP)_DEFINEd and $1_USED in the
module.
since all classmaps in a module must share
the 0..62 class-id space, whether they're _DEFINEd or _USEd in the
module.
That bad classmap is detected, and the ddebug-attach is incomplete;
this probably breaks dyndbg classmaps functionality in the submod, but
everything else works, so onward towards failing modprobe properly,
then disarming the bad classmap with an #if-0 block.
To prove the above, the test-modules can be compiled with
-DFORCE_CLASSID_CONFLICT_MODPROBE. This adds 2 bad declares:
DYNDBG_CLASS_DEFINEs, into parent and -submod respectively, which
conflict with one of the good ones in the parent (D2_CORE..etc).
These cause the modules to fail at modprobe when built with the flag.
Ideally we'd fail at compile time, since the DYNDBG_CLASS_DEFINEs are
all file-scoped, with constant args. This is impractical in macros,
or in linker scripts.
The next best thing is to fail modprobe, so that the developer adding
the 2nd class sees the conflict and fixes it.
NB. the reported violator of the rule is dependent upon linkage order
of classes in a module, and on parent/submod, so it cannot report
which class is problematic. This is tolerable.
Signed-off-by: Jim Cromie <jim.cromie at gmail.com>
---
lib/dynamic_debug.c | 31 ++++++++++++++++++++++++++-----
lib/test_dynamic_debug.c | 15 +++++++++++++--
2 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d9666ba72b48..061fcf26c308 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1241,6 +1241,21 @@ static void ddebug_apply_params(const struct ddebug_class_map *cm, const char *m
}
}
+static int ddebug_class_check_range(struct ddebug_class_map *cm,
+ u64 *reserved_ids)
+{
+ u64 range = (((1ULL << cm->length) - 1) << cm->base);
+
+ if (range & *reserved_ids) {
+ pr_err("[%d..%d] on %s conflicts with %llx\n", cm->base,
+ cm->base + cm->length - 1, cm->class_names[0],
+ *reserved_ids);
+ return -1;
+ }
+ *reserved_ids |= range;
+ return 0;
+}
+
/*
* Find this module's classmaps in a sub/whole-range of the builtin/
* modular classmap vector/section. Save the start and length of the
@@ -1266,8 +1281,11 @@ static int ddebug_attach_module_classes(struct ddebug_table *dt,
vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
dt->num_classes = nc;
- for (i = 0, cm = dt->classes; i < dt->num_classes; i++, cm++)
+ for (i = 0, cm = dt->classes; i < dt->num_classes; i++, cm++) {
+ if (ddebug_class_check_range(cm, reserved_ids))
+ return -EINVAL;
ddebug_apply_params(cm, cm->mod_name);
+ }
return 0;
}
@@ -1277,8 +1295,8 @@ static int ddebug_attach_module_classes(struct ddebug_table *dt,
* list to be seen by ddebug_change.
*/
static int ddebug_attach_user_module_classes(struct ddebug_table *dt,
- const struct _ddebug_info *di,
- u64 *reserved_ids)
+ const struct _ddebug_info *di,
+ u64 *reserved_ids)
{
struct ddebug_class_user *cli;
int i, nc = 0;
@@ -1306,9 +1324,11 @@ static int ddebug_attach_user_module_classes(struct ddebug_table *dt,
dt->num_class_users = nc;
/* now iterate dt */
- for (i = 0, cli = dt->class_users; i < dt->num_class_users; i++, cli++)
+ for (i = 0, cli = dt->class_users; i < dt->num_class_users; i++, cli++) {
+ if (ddebug_class_check_range(cli->map, reserved_ids))
+ return -EINVAL;
ddebug_apply_params(cli->map, cli->user_mod_name);
-
+ }
vpr_dt_info(dt, "attach-client-module: ");
return 0;
}
@@ -1350,6 +1370,7 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
if (rc)
return rc;
}
+
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 4d7b1709d055..96855e7ba342 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -74,7 +74,7 @@ enum cat_disjoint_bits {
D2_DRMRES };
/* numeric verbosity, V2 > V1 related */
-enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
+enum cat_level_num { V0 = 16, V1, V2, V3, V4, V5, V6, V7 };
/*
* use/demonstrate multi-module-group classmaps, as for DRM
@@ -86,6 +86,17 @@ enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
* The classmap is exported, so that other modules in the group can
* link to it and control their prdbgs.
*/
+#ifdef FORCE_CLASSID_CONFLICT_MODPROBE
+/*
+ * Enable with -Dflag on compile to test overlapping class-id range
+ * detection. This should break on modprobe.
+ * TLDR: This is declared 1st cuz linker builds sections in LIFO
+ * order, and we want this to be the reported usurper of reserved
+ * class-ids.
+ */
+DYNDBG_CLASSMAP_DEFINE(classid_range_conflict, 0, D2_CORE + 1, "D3_CORE");
+#endif
+
DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
D2_CORE,
"D2_CORE",
@@ -128,7 +139,7 @@ DYNDBG_CLASSMAP_DEFINE(classid_range_conflict_submod, 0, D2_CORE + 1, "D3_SUB");
#if defined(DD_MACRO_ARGCHECK)
/*
- * Exersize compile-time arg-checks in the macro.
+ * Exersize compile-time arg-checks in the _DEFINE macro.
* These will break compilation.
*/
DYNDBG_CLASSMAP_DEFINE(fail_base_neg, 0, -1, "NEGATIVE_BASE_ARG");
--
2.46.1
More information about the Intel-gfx-trybot
mailing list