[PATCH 35/61] dyndbg: fail modprobe if classmaps have conflicting class-id ranges

Jim Cromie jim.cromie at gmail.com
Wed Oct 16 18:13:18 UTC 2024


All classes used by a module (declared DYNDBG_CLASSMAP_{DEFINE,USE} by
module code) must share 0..63 class-id space, that is their respective
base,+length reservations shouldn't overlap.  Overlaps would lead to
unintended changes in ddebug enablements.

Detecting these class-id range overlaps at compile-time would be ideal
but is not practical; failing at modprobe at least insures that the
developer sees and fixes the conflict.

TBD: failing modprobe is kinda harsh, maybe warn and proceed ?

But the call-chain is already updated to allow failing modprobe, so
now lets use it to signal classid conflicts.

ddebug_class_range_overlap() implements the range check, accumulating
the reserved-ids as it examines each class.  It probably should use
bitmaps.

Since the reserved-ids are accumulated in ddebug_add_module() so the
check applies to both (DYNDBG_CLASSMAP)_DEFINEd and $1_USEd classmaps
in the module.

test_dynamic_debug*.ko:

If built with -DFORCE_CLASSID_CONFLICT_MODPROBE, the modules get 2 bad
DYNDBG_CLASS_DEFINE declarations, into parent and the _submod.  These
conflict with one of the good ones in the parent (D2_CORE..etc),
causing the modprobe(s) to fail.  TODO: do in submod only, since fail
of parent prevents submod from ever trying.

Signed-off-by: Jim Cromie <jim.cromie at gmail.com>
---
-9p: fix failing classid-conflict code.
---
 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 d60a709a43f0..7555d18e9d94 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1251,6 +1251,21 @@ static void ddebug_apply_params(const struct ddebug_class_map *cm, const char *m
 	}
 }
 
+static int ddebug_class_range_overlap(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 -EINVAL;
+	}
+	*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
@@ -1276,8 +1291,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_subvec(i, cm, dt, classes)
+	for_subvec(i, cm, dt, classes) {
+		if (ddebug_class_range_overlap(cm, reserved_ids))
+			return -EINVAL;
 		ddebug_apply_params(cm, cm->mod_name);
+	}
 	return 0;
 }
 
@@ -1287,8 +1305,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;
@@ -1314,9 +1332,11 @@ static int ddebug_attach_user_module_classes(struct ddebug_table *dt,
 	dt->num_class_users = nc;
 
 	/* now iterate dt */
-	for_subvec(i, cli, di, class_users)
+	for_subvec(i, cli, di, class_users) {
+		if (ddebug_class_range_overlap(cli->map, reserved_ids))
+			return -EINVAL;
 		ddebug_apply_params(cli->map, cli->mod_name);
-
+	}
 	vpr_dt_info(dt, "attach-client-module: ");
 	return 0;
 }
@@ -1360,6 +1380,7 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
 			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 9eb5c3296443..5806495ff51f 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.47.0



More information about the Intel-gfx-trybot mailing list