[Pdbg] [PATCH v2 6/8] libpdbg: Lazy probing

Cyril Bur cyrilbur at gmail.com
Tue Mar 27 11:30:24 AEDT 2018


When libpdbg runs though the device tree and calls probe() on all the
targets it marks any node (and all its children) which fails to probe
as "disabled".

When pdbg recieves arguments to specifiy a pib, core or thread (-p -c
-t) it runs though the device tree and marks nodes which do not match
the specified -p -c or -t as "disabled".

These two scenarios are related - that is we don't want to be operating
on any disabled nodes. However, they are not the same.

Consider HTM wanting to know if the machine has powersave disabled. A
simple way would be to loop through all the threads and if any of them
are not in "active" state then powersave must be enabled. In this
scenario HTM want to be able to get the status of every thread on the
machine regardless of -p -c -t flags but if the machine isn't fully
speced out and has cores which didn't probe, HTM wants to know to
ignore those threads.

This patch introduces a "nonexistant" pdbg target status which is set
during libpdbg probe() time. Later pdbg can mark nodes to targets that
do 'exist' but it would like to avoid as "disabled".

Attempting to solve the above problem with only a "nonexistant" flag
would also require everything to be probed before selecting out chips
and threads that the user is not interested in. This would be highly
inefficient.

This patch probes the system lazily

Signed-off-by: Cyril Bur <cyrilbur at gmail.com>
---
 libpdbg/libpdbg.c |  2 ++
 libpdbg/libpdbg.h |  4 ++--
 libpdbg/target.c  | 30 +++++++++++++++++++++---------
 libpdbg/target.h  |  1 +
 src/htm.c         |  5 +++++
 src/main.c        | 39 ++++++++++++++++++++++-----------------
 src/main.h        |  3 ++-
 7 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c
index 6194048..7f8a935 100644
--- a/libpdbg/libpdbg.c
+++ b/libpdbg/libpdbg.c
@@ -69,6 +69,8 @@ enum pdbg_target_status pdbg_target_status(struct pdbg_target *target)
 		return PDBG_TARGET_DISABLED;
 	else if (!strcmp(p->prop, "hidden"))
 		return PDBG_TARGET_HIDDEN;
+	else if (!strcmp(p->prop, "nonexistant"))
+		return PDBG_TARGET_NONEXISTANT;
 	else
 		assert(0);
 }
diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
index 8dec016..c7706eb 100644
--- a/libpdbg/libpdbg.h
+++ b/libpdbg/libpdbg.h
@@ -9,7 +9,7 @@ struct pdbg_taget *pdbg_root_target;
 /* loops/iterators */
 struct pdbg_target *__pdbg_next_target(const char *klass, struct pdbg_target *parent, struct pdbg_target *last);
 struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last);
-enum pdbg_target_status {PDBG_TARGET_ENABLED, PDBG_TARGET_DISABLED, PDBG_TARGET_HIDDEN};
+enum pdbg_target_status {PDBG_TARGET_ENABLED, PDBG_TARGET_DISABLED, PDBG_TARGET_HIDDEN, PDBG_TARGET_NONEXISTANT};
 
 #define pdbg_for_each_target(class, parent, target)			\
 	for (target = __pdbg_next_target(class, parent, NULL);		\
@@ -36,7 +36,7 @@ uint64_t pdbg_get_address(struct pdbg_target *target, uint64_t *size);
 
 /* Misc. */
 void pdbg_targets_init(void *fdt);
-void pdbg_target_probe(void);
+void pdbg_target_probe(struct pdbg_target *target);
 void pdbg_enable_target(struct pdbg_target *target);
 void pdbg_disable_target(struct pdbg_target *target);
 enum pdbg_target_status pdbg_target_status(struct pdbg_target *target);
diff --git a/libpdbg/target.c b/libpdbg/target.c
index 1eb85bb..acdd762 100644
--- a/libpdbg/target.c
+++ b/libpdbg/target.c
@@ -300,7 +300,7 @@ void pdbg_targets_init(void *fdt)
 }
 
 /* Disable a node and all it's children */
-static void disable_node(struct dt_node *dn)
+static void deprobe(struct dt_node *dn)
 {
 	struct dt_node *next;
 	struct dt_property *p;
@@ -309,9 +309,9 @@ static void disable_node(struct dt_node *dn)
 	if (p)
 		dt_del_property(dn, p);
 
-	dt_add_property_string(dn, "status", "disabled");
+	dt_add_property_string(dn, "status", "nonexistant");
 	dt_for_each_child(dn, next)
-		disable_node(next);
+		deprobe(next);
 }
 
 static void _target_probe(struct dt_node *dn)
@@ -326,26 +326,38 @@ static void _target_probe(struct dt_node *dn)
 	}
 
 	p = dt_find_property(dn, "status");
-	if ((p && !strcmp(p->prop, "disabled")) || (dn->target->probe && (rc = dn->target->probe(dn->target)))) {
+	if ((p && !strcmp(p->prop, "nonexistant")) || (dn->target->probe && (rc = dn->target->probe(dn->target)))) {
 		if (rc)
 			PR_DEBUG("not found\n");
 		else
 			PR_DEBUG("disabled\n");
 
-		disable_node(dn);
+		deprobe(dn);
 	} else {
+		if (!p)
+			dt_add_property_string(dn, "status", "enabled");
+		dn->target->probed = true;
 		PR_DEBUG("success\n");
 	}
 }
 
 /* We walk the tree root down disabling targets which might/should
  * exist but don't */
-void pdbg_target_probe(void)
+void pdbg_target_probe(struct pdbg_target *target)
 {
-	struct dt_node *dn;
+	struct pdbg_target *parent;
+	struct dt_property *p;
+
+	if (!target || target->probed)
+		return;
+
+	parent = require_target_parent(target);
+	if (parent && !parent->probed)
+		pdbg_target_probe(parent);
 
-	dt_for_each_node(dt_root, dn)
-		_target_probe(dn);
+	p = dt_find_property(target->dn, "status");
+	if (!p || strcmp(p->prop, "nonexistant") != 0)
+		_target_probe(target->dn);
 }
 
 bool pdbg_target_is_class(struct pdbg_target *target, const char *class)
diff --git a/libpdbg/target.h b/libpdbg/target.h
index 0a618e2..8324179 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -44,6 +44,7 @@ struct pdbg_target {
 	char *class;
 	int (*probe)(struct pdbg_target *target);
 	int index;
+	bool probed;
 	struct dt_node *dn;
 	struct list_node class_link;
 };
diff --git a/src/htm.c b/src/htm.c
index 1032a5c..80501c5 100644
--- a/src/htm.c
+++ b/src/htm.c
@@ -80,6 +80,7 @@ static int run_start(enum htm_type type, int optind, int argc, char *argv[])
 	int rc = 0;
 
 	pdbg_for_each_class_target(HTM_ENUM_TO_STRING(type), target) {
+		pdbg_target_probe(target);
 		if (target_is_disabled(target))
 			continue;
 
@@ -101,6 +102,7 @@ static int run_stop(enum htm_type type, int optind, int argc, char *argv[])
 	int rc = 0;
 
 	pdbg_for_each_class_target(HTM_ENUM_TO_STRING(type), target) {
+		pdbg_target_probe(target);
 		if (target_is_disabled(target))
 			continue;
 
@@ -122,6 +124,7 @@ static int run_status(enum htm_type type, int optind, int argc, char *argv[])
 	int rc = 0;
 
 	pdbg_for_each_class_target(HTM_ENUM_TO_STRING(type), target) {
+		pdbg_target_probe(target);
 		if (target_is_disabled(target))
 			continue;
 
@@ -145,6 +148,7 @@ static int run_reset(enum htm_type type, int optind, int argc, char *argv[])
 	int rc = 0;
 
 	pdbg_for_each_class_target(HTM_ENUM_TO_STRING(type), target) {
+		pdbg_target_probe(target);
 		if (target_is_disabled(target))
 			continue;
 
@@ -181,6 +185,7 @@ static int run_dump(enum htm_type type, int optind, int argc, char *argv[])
 	/* size = 0 will dump everything */
 	printf("Dumping HTM trace to file [chip].[#]%s\n", filename);
 	pdbg_for_each_class_target(HTM_ENUM_TO_STRING(type), target) {
+		pdbg_target_probe(target);
 		if (target_is_disabled(target))
 			continue;
 
diff --git a/src/main.c b/src/main.c
index c6b645e..6cadb09 100644
--- a/src/main.c
+++ b/src/main.c
@@ -268,8 +268,9 @@ int for_each_child_target(char *class, struct pdbg_target *parent,
 	pdbg_for_each_target(class, parent, target) {
 		index = pdbg_target_index(target);
 		assert(index != -1);
+		pdbg_target_probe(target);
 		status = pdbg_target_status(target);
-		if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN)
+		if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN || status == PDBG_TARGET_NONEXISTANT)
 			continue;
 
 		rc += cb(target, index, arg1, arg2);
@@ -288,8 +289,9 @@ int for_each_target(char *class, int (*cb)(struct pdbg_target *, uint32_t, uint6
 	pdbg_for_each_class_target(class, target) {
 		index = pdbg_target_index(target);
 		assert(index != -1);
+		pdbg_target_probe(target);
 		status = pdbg_target_status(target);
-		if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN)
+		if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN || status == PDBG_TARGET_NONEXISTANT)
 			continue;
 
 		rc += cb(target, index, arg1, arg2);
@@ -317,6 +319,16 @@ extern unsigned char _binary_p8_host_dtb_o_start;
 extern unsigned char _binary_p8_host_dtb_o_end;
 extern unsigned char _binary_p9_host_dtb_o_start;
 extern unsigned char _binary_p9_host_dtb_o_end;
+
+static void mark_disabled(struct pdbg_target *target)
+{
+	struct pdbg_target *child;
+
+	pdbg_disable_target(target);
+
+	pdbg_for_each_child_target(target, child)
+		pdbg_disable_target(child);
+}
 static int target_select(void)
 {
 	struct pdbg_target *fsi, *pib, *chip, *thread;
@@ -383,31 +395,25 @@ static int target_select(void)
 			pdbg_set_target_property(pib, "bus", device_node, strlen(device_node) + 1);
 
 		if (processorsel[proc_index]) {
-			pdbg_enable_target(pib);
 			pdbg_for_each_target("core", pib, chip) {
 				int chip_index = pdbg_target_index(chip);
 				if (chipsel[proc_index][chip_index]) {
-					pdbg_enable_target(chip);
 					pdbg_for_each_target("thread", chip, thread) {
 						int thread_index = pdbg_target_index(thread);
-						if (threadsel[proc_index][chip_index][thread_index])
-							pdbg_enable_target(thread);
-						else
-							pdbg_disable_target(thread);
+						if (!threadsel[proc_index][chip_index][thread_index])
+							mark_disabled(thread);
 					}
 				} else
-					pdbg_disable_target(chip);
+					mark_disabled(chip);
 			}
 		} else
-			pdbg_disable_target(pib);
+			mark_disabled(pib);
 	}
 
 	pdbg_for_each_class_target("fsi", fsi) {
 		int index = pdbg_target_index(fsi);
-		if (processorsel[index])
-			pdbg_enable_target(fsi);
-		else
-			pdbg_disable_target(fsi);
+		if (!processorsel[index])
+			mark_disabled(fsi);
 	}
 
 	return 0;
@@ -419,8 +425,9 @@ void print_target(struct pdbg_target *target, int level)
 	struct pdbg_target *next;
 	enum pdbg_target_status status;
 
+	pdbg_target_probe(target);
 	status = pdbg_target_status(target);
-	if (status == PDBG_TARGET_DISABLED)
+	if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_NONEXISTANT)
 		return;
 
 	if (status == PDBG_TARGET_ENABLED) {
@@ -493,8 +500,6 @@ int main(int argc, char *argv[])
 	if (target_select())
 		return 1;
 
-	pdbg_target_probe();
-
 	for (i = 0; i < ARRAY_SIZE(actions); i++) {
 		if (strcmp(argv[optind], actions[i].name) == 0) {
 			rc = actions[i].fn(optind, argc, argv);
diff --git a/src/main.h b/src/main.h
index 71599ee..f9ea77c 100644
--- a/src/main.h
+++ b/src/main.h
@@ -21,7 +21,8 @@ enum backend { FSI, I2C, KERNEL, FAKE, HOST };
 
 static inline bool target_is_disabled(struct pdbg_target *target)
 {
-	return pdbg_target_status(target) == PDBG_TARGET_DISABLED;
+	return pdbg_target_status(target) == PDBG_TARGET_DISABLED ||
+		pdbg_target_status(target) == PDBG_TARGET_NONEXISTANT;
 }
 
 /* Returns the sum of return codes. This can be used to count how many targets the callback was run on. */
-- 
2.16.3



More information about the Pdbg mailing list