[Pdbg] [PATCH] libpdbg: Abstract thread status

Alistair Popple alistair at popple.id.au
Thu May 24 13:49:37 AEST 2018


There is currently no abstraction of thread status between library and
application. It just passes the hardware specific values down. There is
also no interface exported in the library headers to read status.

This makes it difficult to implement more advanced functionality in the
application as there is no way hardware agnostic way to determine if a
thread is in powersave mode or not for example.

Instead introduce a hardware agnostic thread state so that we can implement
more advanced functionality such as automatically stopping threads if
required.

Signed-off-by: Alistair Popple <alistair at popple.id.au>
---
 libpdbg/chip.c    |  2 +-
 libpdbg/libpdbg.h | 28 +++++++----------
 libpdbg/p8chip.c  | 92 +++++++++++++++++++++++++++++++++++++++++++++++--------
 libpdbg/p9chip.c  | 44 ++++++++++++++------------
 libpdbg/target.h  |  2 +-
 src/htm.c         |  7 +++--
 src/thread.c      | 54 ++++++++++++++++----------------
 7 files changed, 147 insertions(+), 82 deletions(-)

diff --git a/libpdbg/chip.c b/libpdbg/chip.c
index 8925494..0e890a6 100644
--- a/libpdbg/chip.c
+++ b/libpdbg/chip.c
@@ -92,7 +92,7 @@ static uint64_t ld(uint64_t rt, uint64_t ds, uint64_t ra)
 	return LD_OPCODE | (rt << 21) | (ra << 16) | (ds << 2);
 }
 
-uint64_t thread_status(struct pdbg_target *target)
+struct thread_state thread_status(struct pdbg_target *target)
 {
 	struct thread *thread;
 
diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
index 51e1523..3b82154 100644
--- a/libpdbg/libpdbg.h
+++ b/libpdbg/libpdbg.h
@@ -146,27 +146,21 @@ int ram_step_thread(struct pdbg_target *target, int steps);
 int ram_stop_thread(struct pdbg_target *target);
 int ram_sreset_thread(struct pdbg_target *target);
 int ram_state_thread(struct pdbg_target *target, struct thread_regs *regs);
-uint64_t thread_status(struct pdbg_target *target);
+struct thread_state thread_status(struct pdbg_target *target);
 int getring(struct pdbg_target *chiplet_target, uint64_t ring_addr, uint64_t ring_len, uint32_t result[]);
 
-#define THREAD_STATUS_DISABLED	PPC_BIT(0)
-#define THREAD_STATUS_ACTIVE	PPC_BIT(63)
+enum pdbg_sleep_state {PDBG_THREAD_STATE_RUN, PDBG_THREAD_STATE_DOZE,
+		       PDBG_THREAD_STATE_NAP, PDBG_THREAD_STATE_SLEEP,
+		       PDBG_THREAD_STATE_STOP};
 
-#define THREAD_STATUS_STATE	PPC_BITMASK(61, 62)
-#define THREAD_STATUS_DOZE	PPC_BIT(62)
-#define THREAD_STATUS_NAP	PPC_BIT(61)
-#define THREAD_STATUS_SLEEP	PPC_BITMASK(61, 62)
+enum pdbg_smt_state {PDBG_SMT_UNKNOWN, PDBG_SMT_1, PDBG_SMT_2, PDBG_SMT_4, PDBG_SMT_8};
 
-#define THREAD_STATUS_QUIESCE	PPC_BIT(60)
-
-#define THREAD_STATUS_SMT	PPC_BITMASK(57, 59)
-#define THREAD_STATUS_SMT_1	PPC_BIT(59)
-#define THREAD_STATUS_SMT_2SH	PPC_BIT(58)
-#define THREAD_STATUS_SMT_2SP	(PPC_BIT(58) | PPC_BIT(59))
-#define THREAD_STATUS_SMT_4	PPC_BIT(57)
-#define THREAD_STATUS_SMT_8	(PPC_BIT(57) | PPC_BIT(59))
-
-#define THREAD_STATUS_STOP	PPC_BIT(56)
+struct thread_state {
+	bool active;
+	bool quiesced;
+	enum pdbg_sleep_state sleep_state;
+	enum pdbg_smt_state smt_state;
+};
 
 int htm_start(struct pdbg_target *target);
 int htm_stop(struct pdbg_target *target);
diff --git a/libpdbg/p8chip.c b/libpdbg/p8chip.c
index 2c799b6..cabb3b5 100644
--- a/libpdbg/p8chip.c
+++ b/libpdbg/p8chip.c
@@ -43,7 +43,19 @@
 #define  RAS_STATUS_TS_QUIESCE		PPC_BIT(49)
 #define POW_STATUS_REG			0x4
 #define  PMC_POW_STATE			PPC_BITMASK(4, 5)
+#define   PMC_POW_STATE_RUN		0x0
+#define   PMC_POW_STATE_DOZE		0x1
+#define   PMC_POW_STATE_NAP		0x2
+#define   PMC_POW_STATE_SLEEP		0x3
 #define  PMC_POW_SMT			PPC_BITMASK(6, 8)
+#define   PMC_POW_SMT_0			0x0
+#define   PMC_POW_SMT_1			0x1
+#define   PMC_POW_SMT_2SH		0x2
+#define   PMC_POW_SMT_2SP		0x3
+#define   PMC_POW_SMT_4_3		0x4
+#define   PMC_POW_SMT_4_4		0x6
+#define   PMC_POW_SMT_8_5		0x5
+#define   PMC_POW_SMT_8_8		0x7
 #define  CORE_POW_STATE			PPC_BITMASK(23, 25)
 #define THREAD_ACTIVE_REG      		0x1310e
 #define  THREAD_ACTIVE			PPC_BITMASK(0, 7)
@@ -108,28 +120,82 @@ static int deassert_special_wakeup(struct core *chip)
 }
 #endif
 
-static uint64_t get_thread_status(struct thread *thread)
+static struct thread_state get_thread_status(struct thread *thread)
 {
-	uint64_t val, mode_reg, thread_status = thread->status;
+	uint64_t val, mode_reg;
+	struct thread_state thread_status;
 
 	/* Need to activete debug mode to get complete status */
-	CHECK_ERR(pib_read(&thread->target, RAS_MODE_REG, &mode_reg));
+	pib_read(&thread->target, RAS_MODE_REG, &mode_reg);
 	mode_reg |= MR_THREAD_IN_DEBUG;
-	CHECK_ERR(pib_write(&thread->target, RAS_MODE_REG, mode_reg));
+	pib_write(&thread->target, RAS_MODE_REG, mode_reg);
 
 	/* Read status */
-	CHECK_ERR(pib_read(&thread->target, RAS_STATUS_REG, &val));
+	pib_read(&thread->target, RAS_STATUS_REG, &val);
 
-	thread_status = SETFIELD(THREAD_STATUS_ACTIVE, thread_status, !!(val & RAS_STATUS_THREAD_ACTIVE));
-	thread_status = SETFIELD(THREAD_STATUS_QUIESCE, thread_status, !!(val & RAS_STATUS_TS_QUIESCE));
+	thread_status.active = !!(val & RAS_STATUS_THREAD_ACTIVE);
+	thread_status.quiesced = !!(val & RAS_STATUS_TS_QUIESCE);
 
 	/* Read POW status */
-	CHECK_ERR(pib_read(&thread->target, POW_STATUS_REG, &val));
-	thread_status = SETFIELD(THREAD_STATUS_STATE, thread_status, GETFIELD(PMC_POW_STATE, val));
-	thread_status = SETFIELD(THREAD_STATUS_SMT, thread_status, GETFIELD(PMC_POW_SMT, val));
+	pib_read(&thread->target, POW_STATUS_REG, &val);
+
+	switch (GETFIELD(PMC_POW_STATE, val)) {
+	case PMC_POW_STATE_RUN:
+		thread_status.sleep_state = PDBG_THREAD_STATE_RUN;
+		break;
+
+	case PMC_POW_STATE_DOZE:
+		thread_status.sleep_state = PDBG_THREAD_STATE_DOZE;
+		break;
+
+	case PMC_POW_STATE_NAP:
+		thread_status.sleep_state = PDBG_THREAD_STATE_NAP;
+		break;
+
+	case PMC_POW_STATE_SLEEP:
+		thread_status.sleep_state = PDBG_THREAD_STATE_SLEEP;
+		break;
+
+	default:
+		/* PMC_POW_STATE is a 2-bit field and we test all values so it
+		 * should be impossible to get here. */
+		assert(0);
+	}
+
+	switch (GETFIELD(PMC_POW_SMT, val)) {
+	case PMC_POW_SMT_0:
+		thread_status.smt_state = PDBG_SMT_UNKNOWN;
+		break;
+
+	case PMC_POW_SMT_1:
+		thread_status.smt_state = PDBG_SMT_1;
+		break;
+
+	case PMC_POW_SMT_2SH:
+	case PMC_POW_SMT_2SP:
+		thread_status.smt_state = PDBG_SMT_2;
+		break;
+
+	/* It's unclear from the documentation what the difference between these
+	 * two are. */
+	case PMC_POW_SMT_4_3:
+	case PMC_POW_SMT_4_4:
+		thread_status.smt_state = PDBG_SMT_4;
+		break;
+
+	/* Ditto */
+	case PMC_POW_SMT_8_5:
+	case PMC_POW_SMT_8_8:
+		thread_status.smt_state = PDBG_SMT_8;
+		break;
+
+	default:
+		assert(0);
+	}
+
 	/* Clear debug mode */
 	mode_reg &= ~MR_THREAD_IN_DEBUG;
-	CHECK_ERR(pib_write(&thread->target, RAS_MODE_REG, mode_reg));
+	pib_write(&thread->target, RAS_MODE_REG, mode_reg);
 
 	return thread_status;
 }
@@ -231,11 +297,11 @@ static int p8_ram_setup(struct thread *thread)
 	dt_for_each_compatible(&chip->target, target, "ibm,power8-thread") {
 		struct thread *tmp;
 		tmp = target_to_thread(target);
-		if (!(get_thread_status(tmp) & THREAD_STATUS_QUIESCE))
+		if (!(get_thread_status(tmp).quiesced))
 			return 1;
 	}
 
-	if (!(thread->status & THREAD_STATUS_ACTIVE))
+	if (!(thread->status.active))
 		return 2;
 
 	/* Activate RAM mode */
diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c
index f0ca142..6e9d3ec 100644
--- a/libpdbg/p9chip.c
+++ b/libpdbg/p9chip.c
@@ -92,23 +92,27 @@ static uint64_t thread_write(struct thread *thread, uint64_t addr, uint64_t data
 	return pib_write(chip, addr, data);
 }
 
-static uint64_t p9_get_thread_status(struct thread *thread)
+static struct thread_state p9_get_thread_status(struct thread *thread)
 {
-	uint64_t value, status = 0;
+	uint64_t value;
+	struct thread_state thread_state;
 
 	thread_read(thread, P9_RAS_STATUS, &value);
-	if (GETFIELD(PPC_BITMASK(8*thread->id, 3 + 8*thread->id), value) == 0xf)
-		status |= THREAD_STATUS_QUIESCE;
+
+	thread_state.quiesced = (GETFIELD(PPC_BITMASK(8*thread->id, 3 + 8*thread->id), value) == 0xf);
 
 	thread_read(thread, P9_THREAD_INFO, &value);
-	if (value & PPC_BIT(thread->id))
-		status |= THREAD_STATUS_ACTIVE;
+	thread_state.active = !!(value & PPC_BIT(thread->id));
 
 	thread_read(thread, P9_CORE_THREAD_STATE, &value);
 	if (value & PPC_BIT(56 + thread->id))
-		status |= THREAD_STATUS_STOP;
+		thread_state.sleep_state = PDBG_THREAD_STATE_STOP;
+	else
+		thread_state.sleep_state = PDBG_THREAD_STATE_RUN;
+
+	thread_state.smt_state = PDBG_SMT_UNKNOWN;
 
-	return status;
+	return thread_state;
 }
 
 static int p9_thread_probe(struct pdbg_target *target)
@@ -123,11 +127,11 @@ static int p9_thread_probe(struct pdbg_target *target)
 
 static int p9_thread_start(struct thread *thread)
 {
-	if (!(thread->status & THREAD_STATUS_QUIESCE))
+	if (!(thread->status.quiesced))
 		return 1;
 
-	if ((!(thread->status & THREAD_STATUS_ACTIVE)) ||
-	    (thread->status & THREAD_STATUS_STOP)) {
+	if ((!(thread->status.active)) ||
+	    (thread->status.sleep_state == PDBG_THREAD_STATE_STOP)) {
 		/* Inactive or active ad stopped: Clear Maint */
 		thread_write(thread, P9_DIRECT_CONTROL, PPC_BIT(3 + 8*thread->id));
 	} else {
@@ -145,7 +149,7 @@ static int p9_thread_stop(struct thread *thread)
 	int i = 0;
 
 	thread_write(thread, P9_DIRECT_CONTROL, PPC_BIT(7 + 8*thread->id));
-	while (!(p9_get_thread_status(thread) & THREAD_STATUS_QUIESCE)) {
+	while (!(p9_get_thread_status(thread).quiesced)) {
 		usleep(1000);
 		if (i++ > RAS_STATUS_TIMEOUT) {
 			PR_ERROR("Unable to quiesce thread\n");
@@ -163,15 +167,15 @@ static int p9_thread_step(struct thread *thread, int count)
 	int i;
 
 	/* Can only step if a thread is quiesced */
-	if (!(thread->status & THREAD_STATUS_QUIESCE))
+	if (!(thread->status.quiesced))
 		return 1;
 
 	/* Core must be active to step */
-	if (!(thread->status & THREAD_STATUS_ACTIVE))
+	if (!(thread->status.active))
 		return 1;
 
 	/* Stepping a stop instruction doesn't really work */
-	if (thread->status & THREAD_STATUS_STOP)
+	if (thread->status.sleep_state == PDBG_THREAD_STATE_STOP)
 		return 1;
 
 	/* Fence interrupts. */
@@ -198,14 +202,14 @@ static int p9_thread_sreset(struct thread *thread)
 {
 	if (!pdbg_expert_mode) {
 		/* Something already quiesced it, fail*/
-		if (thread->status & THREAD_STATUS_QUIESCE)
+		if (thread->status.quiesced)
 			return 1;
 		if (p9_thread_stop(thread))
 			return 1;
 	}
 
 	/* Can only sreset if a thread is quiesced */
-	if (!(thread->status & THREAD_STATUS_QUIESCE))
+	if (!(thread->status.quiesced))
 		return 1;
 
 	thread_write(thread, P9_DIRECT_CONTROL, PPC_BIT(4 + 8*thread->id));
@@ -254,9 +258,9 @@ static int p9_ram_setup(struct thread *thread)
 		p9_thread_probe(target);
 		tmp = target_to_thread(target);
 		/* Something already quiesced it, fail*/
-		if (tmp->status & THREAD_STATUS_QUIESCE)
+		if (tmp->status.quiesced)
 			goto out_fail;
-		if (!(tmp->status & THREAD_STATUS_QUIESCE)) {
+		if (!(tmp->status.quiesced)) {
 			if (p9_thread_stop(tmp))
 				goto out_fail;
 			tmp->ram_did_quiesce = true;
@@ -273,7 +277,7 @@ expert:
 		   so do that now. This will also update the thread status */
 		p9_thread_probe(target);
 		tmp = target_to_thread(target);
-		if (!(tmp->status & THREAD_STATUS_QUIESCE))
+		if (!(tmp->status.quiesced))
 			goto out_fail;
 	}
 
diff --git a/libpdbg/target.h b/libpdbg/target.h
index 5541729..abdc0be 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -137,7 +137,7 @@ struct core {
 
 struct thread {
 	struct pdbg_target target;
-	uint64_t status;
+	struct thread_state status;
 	int id;
 	int (*step)(struct thread *, int);
 	int (*start)(struct thread *);
diff --git a/src/htm.c b/src/htm.c
index 1e65a04..945d8d6 100644
--- a/src/htm.c
+++ b/src/htm.c
@@ -338,9 +338,10 @@ int run_htm(int optind, int argc, char *argv[])
 			if (pdbg_target_status(target) == PDBG_TARGET_NONEXISTENT)
 				continue;
 
-			if ((!(thread_status(target) & THREAD_STATUS_ACTIVE)) ||
-			    (thread_status(target) & THREAD_STATUS_STOP)) {
-				fprintf(stderr, "It appears powersave is on 0x%"  PRIx64 "%p\n", thread_status(target), target);
+			if ((!(thread_status(target).active)) ||
+			    (thread_status(target).sleep_state != PDBG_THREAD_STATE_RUN)) {
+				fprintf(stderr, "It appears powersave is on on %s@%d\n",
+					pdbg_target_name(target), pdbg_target_index(target));
 				fprintf(stderr, "core HTM needs to run with powersave off\n");
 				fprintf(stderr, "Hint: put powersave=off on the kernel commandline\n");
 				return 0;
diff --git a/src/thread.c b/src/thread.c
index 0e4627f..e8b54cb 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -18,58 +18,58 @@
 #include <stdio.h>
 #include <stdlib.h>
 
-#include <bitutils.h>
-
-#include <target.h>
-#include <operations.h>
+#include <libpdbg.h>
 
 #include "main.h"
 #include "mem.h"
 
-static int print_thread_status(struct pdbg_target *target, uint32_t index, uint64_t *status, uint64_t *unused1)
+static int print_thread_status(struct pdbg_target *target, uint32_t index, uint64_t *arg, uint64_t *unused1)
 {
+	struct thread_state *status = (struct thread_state *) arg;
+
 	status[index] = thread_status(target);
 	return 1;
 }
 
 static int print_core_thread_status(struct pdbg_target *core_target, uint32_t index, uint64_t *maxindex, uint64_t *unused1)
 {
-	uint64_t status[8];
+	struct thread_state status[8];
 	int i, rc;
 
-	memset(status, 0xff, sizeof(status));
+	printf("c%02d:  ", index);
 
-	printf("c%02d: ", index);
-	rc = for_each_child_target("thread", core_target, print_thread_status, &status[0], NULL);
+	/* TODO: This cast is gross. Need to rewrite for_each_child_target as an iterator. */
+	rc = for_each_child_target("thread", core_target, print_thread_status, (uint64_t *) &status[0], NULL);
 	for (i = 0; i <= *maxindex; i++) {
-		if (status[i] == -1ULL) {
-			printf("    ");
-			continue;
-		}
-		if (status[i] & ~(THREAD_STATUS_ACTIVE|THREAD_STATUS_DOZE|
-				  THREAD_STATUS_NAP|THREAD_STATUS_SLEEP|
-				  THREAD_STATUS_STOP|THREAD_STATUS_QUIESCE)) {
-			printf("%" PRIx64 " ", status[i]);
-			continue;
-		}
 
-		if (status[i] & THREAD_STATUS_ACTIVE)
+		if (status[i].active)
 			printf("A");
 		else
 			printf(".");
 
-		if (status[i] & THREAD_STATUS_DOZE)
+		switch (status[i].sleep_state) {
+		case PDBG_THREAD_STATE_DOZE:
 			printf("D");
-		else if (status[i] & THREAD_STATUS_NAP)
+			break;
+
+		case PDBG_THREAD_STATE_NAP:
 			printf("N");
-		else if (status[i] & THREAD_STATUS_SLEEP)
-			printf("S");
-		else if (status[i] & THREAD_STATUS_STOP)
+			break;
+
+		case PDBG_THREAD_STATE_SLEEP:
+			printf("Z");
+			break;
+
+		case PDBG_THREAD_STATE_STOP:
 			printf("S");
-		else
+			break;
+
+		default:
 			printf(".");
+			break;
+		}
 
-		if (status[i] & THREAD_STATUS_QUIESCE)
+		if (status[i].quiesced)
 			printf("Q");
 		else
 			printf(".");
-- 
2.11.0



More information about the Pdbg mailing list