target: target_halt_wait and target_check_hw_wp replaced with target_halt_poll.

The new function returns a stop reason which must be translated in gdb server.
In the case of a watchpoint hit, the address is returned by a pointer parameter.
Simplify the extenal interface for set/clear breaki-/watchpoints.

src/gdb_main.c

@@ -34,6 +34,13 @@
 #include "command.h"
 #include "crc32.h"
 
+enum gdb_signal {
+	GDB_SIGINT = 2,
+	GDB_SIGTRAP = 5,
+	GDB_SIGSEGV = 11,
+	GDB_SIGLOST = 29,
+};
+
 #define BUF_SIZE	1024
 
 #define ERROR_IF_NO_TARGET()	\
@@ -156,8 +163,8 @@ int gdb_main_loop(struct target_controller *tc, bool in_syscall)
 		case '?': {	/* '?': Request reason for target halt */
 			/* This packet isn't documented as being mandatory,
 			 * but GDB doesn't work without it. */
-			target_addr watch_addr;
-			int sig;
+			target_addr watch;
+			enum target_halt_reason reason;
 
 			if(!cur_target) {
 				/* Report "target exited" if no target */
@@ -166,7 +173,7 @@ int gdb_main_loop(struct target_controller *tc, bool in_syscall)
 			}
 
 			/* Wait for target halt */
-			while(!(sig = target_halt_wait(cur_target))) {
+			while(!(reason = target_halt_poll(cur_target, &watch))) {
 				unsigned char c = gdb_if_getchar_to(0);
 				if((c == '\x03') || (c == '\x04')) {
 					target_halt_request(cur_target);
@@ -174,22 +181,22 @@ int gdb_main_loop(struct target_controller *tc, bool in_syscall)
 			}
 			SET_RUN_STATE(0);
 
-			/* Negative signal indicates we're in a syscall */
-			if (sig < 0)
+			/* Translate reason to GDB signal */
+			switch (reason) {
+			case TARGET_HALT_ERROR:
+				gdb_putpacket_f("X%02X", GDB_SIGLOST);
 				break;
-
-			/* Target disappeared */
-			if (cur_target == NULL) {
-				gdb_putpacket_f("X%02X", sig);
+			case TARGET_HALT_REQUEST:
+				gdb_putpacket_f("T%02X", GDB_SIGINT);
 				break;
-			}
-
-			/* Report reason for halt */
-			if(target_check_hw_wp(cur_target, &watch_addr)) {
-				/* Watchpoint hit */
-				gdb_putpacket_f("T%02Xwatch:%08X;", sig, watch_addr);
-			} else {
-				gdb_putpacket_f("T%02X", sig);
+			case TARGET_HALT_WATCHPOINT:
+				gdb_putpacket_f("T%02Xwatch:%08X;", GDB_SIGTRAP, watch);
+				break;
+			case TARGET_HALT_FAULT:
+				gdb_putpacket_f("T%02X", GDB_SIGSEGV);
+				break;
+			default:
+				gdb_putpacket_f("T%02X", GDB_SIGTRAP);
 			}
 			break;
 			}
@@ -447,32 +454,18 @@ handle_z_packet(char *packet, int plen)
 	//sscanf(packet, "%*[zZ]%hhd,%08lX,%hhd", &type, &addr, &len);
 	type = packet[1] - '0';
 	sscanf(packet + 2, ",%" PRIx32 ",%d", &addr, &len);
-	switch(type) {
-	case 1: /* Hardware breakpoint */
-		if(set)
-			ret = target_set_hw_bp(cur_target, addr, len);
-		else
-			ret = target_clear_hw_bp(cur_target, addr, len);
-		break;
-
-	case 2:
-	case 3:
-	case 4:
-		if(set)
-			ret = target_set_hw_wp(cur_target, type, addr, len);
-		else
-			ret = target_clear_hw_wp(cur_target, type, addr, len);
-		break;
+	if(set)
+		ret = target_breakwatch_set(cur_target, type, addr, len);
+	else
+		ret = target_breakwatch_clear(cur_target, type, addr, len);
 
-	default:
+	if (ret < 0) {
+		gdb_putpacketz("E01");
+	} else if (ret > 0) {
 		gdb_putpacketz("");
-		return;
-	}
-
-	if(!ret)
+	} else {
 		gdb_putpacketz("OK");
-	else
-		gdb_putpacketz("E01");
+	}
 }
 
 void gdb_main(void)

src/include/target.h

@@ -121,18 +121,31 @@ void target_regs_read(target *t, void *data);
 void target_regs_write(target *t, const void *data);
 
 /* Halt/resume functions */
+enum target_halt_reason {
+	TARGET_HALT_RUNNING = 0, /* Target not halted */
+	TARGET_HALT_ERROR,       /* Failed to read target status */
+	TARGET_HALT_REQUEST,
+	TARGET_HALT_STEPPING,
+	TARGET_HALT_BREAKPOINT,
+	TARGET_HALT_WATCHPOINT,
+	TARGET_HALT_FAULT,
+};
+
 void target_reset(target *t);
 void target_halt_request(target *t);
-int target_halt_wait(target *t);
+enum target_halt_reason target_halt_poll(target *t, target_addr *watch);
 void target_halt_resume(target *t, bool step);
 
 /* Break-/watchpoint functions */
-int target_set_hw_bp(target *t, target_addr addr, uint8_t len);
-int target_clear_hw_bp(target *t, target_addr addr, uint8_t len);
-
-int target_set_hw_wp(target *t, uint8_t type, target_addr addr, uint8_t len);
-int target_clear_hw_wp(target *t, uint8_t type, target_addr addr, uint8_t len);
-int target_check_hw_wp(target *t, target_addr *addr);
+enum target_breakwatch {
+	TARGET_BREAK_SOFT,
+	TARGET_BREAK_HARD,
+	TARGET_WATCH_WRITE,
+	TARGET_WATCH_READ,
+	TARGET_WATCH_ACCESS,
+};
+int target_breakwatch_set(target *t, enum target_breakwatch, target_addr, size_t);
+int target_breakwatch_clear(target *t, enum target_breakwatch, target_addr, size_t);
 
 /* Flash memory access functions */
 int target_flash_erase(target *t, target_addr addr, size_t len);
@@ -140,7 +153,7 @@ int target_flash_write(target *t, target_addr dest, const void *src, size_t len)
 int target_flash_done(target *t);
 
 /* Accessor functions */
-int target_regs_size(target *t);
+size_t target_regs_size(target *t);
 const char *target_tdesc(target *t);
 const char *target_mem_map(target *t);
 const char *target_driver_name(target *t);

src/target/cortexa.c

@@ -35,12 +35,6 @@
 
 static char cortexa_driver_str[] = "ARM Cortex-A";
 
-/* Signals returned by cortexa_halt_wait() */
-#define SIGINT 2
-#define SIGTRAP 5
-#define SIGSEGV 11
-#define SIGLOST 29
-
 static bool cortexa_attach(target *t);
 static void cortexa_detach(target *t);
 static void cortexa_halt_resume(target *t, bool step);
@@ -51,7 +45,7 @@ static void cortexa_regs_read_internal(target *t);
 static void cortexa_regs_write_internal(target *t);
 
 static void cortexa_reset(target *t);
-static int cortexa_halt_wait(target *t);
+static enum target_halt_reason cortexa_halt_poll(target *t, target_addr *watch);
 static void cortexa_halt_request(target *t);
 
 static int cortexa_set_hw_bp(target *t, target_addr addr, uint8_t len);
@@ -395,7 +389,7 @@ bool cortexa_probe(ADIv5_AP_t *apb, uint32_t debug_base)
 
 	t->reset = cortexa_reset;
 	t->halt_request = cortexa_halt_request;
-	t->halt_wait = cortexa_halt_wait;
+	t->halt_poll = cortexa_halt_poll;
 	t->halt_resume = cortexa_halt_resume;
 	t->regs_size = sizeof(priv->reg_cache);
 
@@ -422,7 +416,7 @@ bool cortexa_attach(target *t)
 
 	target_halt_request(t);
 	tries = 10;
-	while(!platform_srst_get_val() && !target_halt_wait(t) && --tries)
+	while(!platform_srst_get_val() && !target_halt_poll(t, NULL) && --tries)
 		platform_delay(200);
 	if(!tries)
 		return false;
@@ -586,8 +580,10 @@ static void cortexa_halt_request(target *t)
 	}
 }
 
-static int cortexa_halt_wait(target *t)
+static enum target_halt_reason cortexa_halt_poll(target *t, target_addr *watch)
 {
+	(void)watch; /* No watchpoint support yet */
+
 	volatile uint32_t dbgdscr = 0;
 	volatile struct exception e;
 	TRY_CATCH (e, EXCEPTION_ALL) {
@@ -599,14 +595,14 @@ static int cortexa_halt_wait(target *t)
 	case EXCEPTION_ERROR:
 		/* Oh crap, there's no recovery from this... */
 		target_list_free();
-		return SIGLOST;
+		return TARGET_HALT_ERROR;
 	case EXCEPTION_TIMEOUT:
 		/* Timeout isn't a problem, target could be in WFI */
-		return 0;
+		return TARGET_HALT_RUNNING;
 	}
 
 	if (!(dbgdscr & DBGDSCR_HALTED)) /* Not halted */
-		return 0;
+		return TARGET_HALT_RUNNING;
 
 	DEBUG("%s: DBGDSCR = 0x%08x\n", __func__, dbgdscr);
 	/* Reenable DBGITR */
@@ -614,18 +610,18 @@ static int cortexa_halt_wait(target *t)
 	apb_write(t, DBGDSCR, dbgdscr);
 
 	/* Find out why we halted */
-	int sig;
+	enum target_halt_reason reason;
 	switch (dbgdscr & DBGDSCR_MOE_MASK) {
 	case DBGDSCR_MOE_HALT_REQ:
-		sig = SIGINT;
+		reason = TARGET_HALT_REQUEST;
 		break;
 	default:
-		sig = SIGTRAP;
+		reason = TARGET_HALT_BREAKPOINT;
 	}
 
 	cortexa_regs_read_internal(t);
 
-	return sig;
+	return reason;
 }
 
 void cortexa_halt_resume(target *t, bool step)

src/target/cortexm.c

@@ -47,18 +47,12 @@ const struct command_s cortexm_cmd_list[] = {
 #define	TOPT_FLAVOUR_V6M	(1<<0)	/* if not set, target is assumed to be v7m */
 #define	TOPT_FLAVOUR_V7MF	(1<<1)	/* if set, floating-point enabled. */
 
-/* Signals returned by cortexm_halt_wait() */
-#define SIGINT 2
-#define SIGTRAP 5
-#define SIGSEGV 11
-#define SIGLOST 29
-
 static void cortexm_regs_read(target *t, void *data);
 static void cortexm_regs_write(target *t, const void *data);
 static uint32_t cortexm_pc_read(target *t);
 
 static void cortexm_reset(target *t);
-static int cortexm_halt_wait(target *t);
+static enum target_halt_reason cortexm_halt_poll(target *t, target_addr *watch);
 static void cortexm_halt_request(target *t);
 static int cortexm_fault_unwind(target *t);
 
@@ -68,7 +62,7 @@ static int cortexm_clear_hw_bp(target *t, target_addr addr, uint8_t len);
 static int cortexm_set_hw_wp(target *t, uint8_t type, target_addr addr, uint8_t len);
 static int cortexm_clear_hw_wp(target *t, uint8_t type, target_addr addr, uint8_t len);
 
-static int cortexm_check_hw_wp(target *t, target_addr *addr);
+static target_addr cortexm_check_watch(target *t);
 
 #define CORTEXM_MAX_WATCHPOINTS	4	/* architecture says up to 15, no implementation has > 4 */
 #define CORTEXM_MAX_BREAKPOINTS	6	/* architecture says up to 127, no implementation has > 6 */
@@ -242,7 +236,7 @@ bool cortexm_probe(ADIv5_AP_t *ap)
 
 	t->reset = cortexm_reset;
 	t->halt_request = cortexm_halt_request;
-	t->halt_wait = cortexm_halt_wait;
+	t->halt_poll = cortexm_halt_poll;
 	t->halt_resume = cortexm_halt_resume;
 	t->regs_size = sizeof(regnum_cortex_m);
 
@@ -295,7 +289,7 @@ bool cortexm_attach(target *t)
 
 	target_halt_request(t);
 	tries = 10;
-	while(!platform_srst_get_val() && !target_halt_wait(t) && --tries)
+	while(!platform_srst_get_val() && !target_halt_poll(t, NULL) && --tries)
 		platform_delay(200);
 	if(!tries)
 		return false;
@@ -338,7 +332,6 @@ bool cortexm_attach(target *t)
 	/* Data Watchpoint and Trace */
 	t->set_hw_wp = cortexm_set_hw_wp;
 	t->clear_hw_wp = cortexm_clear_hw_wp;
-	t->check_hw_wp = cortexm_check_hw_wp;
 
 	platform_srst_set_val(false);
 
@@ -480,7 +473,7 @@ static void cortexm_halt_request(target *t)
 	}
 }
 
-static int cortexm_halt_wait(target *t)
+static enum target_halt_reason cortexm_halt_poll(target *t, target_addr *watch)
 {
 	struct cortexm_priv *priv = t->priv;
 
@@ -495,21 +488,21 @@ static int cortexm_halt_wait(target *t)
 	case EXCEPTION_ERROR:
 		/* Oh crap, there's no recovery from this... */
 		target_list_free();
-		return SIGLOST;
+		return TARGET_HALT_ERROR;
 	case EXCEPTION_TIMEOUT:
 		/* Timeout isn't a problem, target could be in WFI */
-		return 0;
+		return TARGET_HALT_RUNNING;
 	}
 
 	if (!(dhcsr & CORTEXM_DHCSR_S_HALT))
-		return 0;
+		return TARGET_HALT_RUNNING;
 
 	/* We've halted.  Let's find out why. */
 	uint32_t dfsr = target_mem_read32(t, CORTEXM_DFSR);
 	target_mem_write32(t, CORTEXM_DFSR, dfsr); /* write back to reset */
 
 	if ((dfsr & CORTEXM_DFSR_VCATCH) && cortexm_fault_unwind(t))
-		return SIGSEGV;
+		return TARGET_HALT_FAULT;
 
 	/* Remember if we stopped on a breakpoint */
 	priv->on_bkpt = dfsr & (CORTEXM_DFSR_BKPT);
@@ -521,7 +514,7 @@ static int cortexm_halt_wait(target *t)
 		bkpt_instr = target_mem_read16(t, pc);
 		if (bkpt_instr == 0xBEAB) {
 			if (cortexm_hostio_request(t)) {
-				return SIGINT;
+				return TARGET_HALT_REQUEST;
 			} else {
 				target_halt_resume(t, priv->stepping);
 				return 0;
@@ -529,14 +522,18 @@ static int cortexm_halt_wait(target *t)
 		}
 	}
 
-	if (dfsr & (CORTEXM_DFSR_BKPT | CORTEXM_DFSR_DWTTRAP))
-		return SIGTRAP;
+	if (dfsr & CORTEXM_DFSR_DWTTRAP) {
+		if (watch != NULL)
+			*watch = cortexm_check_watch(t);
+		return TARGET_HALT_WATCHPOINT;
+	}
+	if (dfsr & CORTEXM_DFSR_BKPT)
+		return TARGET_HALT_BREAKPOINT;
 
 	if (dfsr & CORTEXM_DFSR_HALTED)
-		return priv->stepping ? SIGTRAP : SIGINT;
-
-	return SIGTRAP;
+		return priv->stepping ? TARGET_HALT_STEPPING : TARGET_HALT_REQUEST;
 
+	return TARGET_HALT_BREAKPOINT;
 }
 
 void cortexm_halt_resume(target *t, bool step)
@@ -642,7 +639,7 @@ int cortexm_run_stub(target *t, uint32_t loadaddr,
 
 	/* Execute the stub */
 	cortexm_halt_resume(t, 0);
-	while (!cortexm_halt_wait(t))
+	while (!cortexm_halt_poll(t, NULL))
 		;
 
 	uint32_t pc = cortexm_pc_read(t);
@@ -779,7 +776,7 @@ cortexm_clear_hw_wp(target *t, uint8_t type, target_addr addr, uint8_t len)
 	return 0;
 }
 
-static int cortexm_check_hw_wp(target *t, target_addr *addr)
+static target_addr cortexm_check_watch(target *t)
 {
 	struct cortexm_priv *priv = t->priv;
 	unsigned i;
@@ -793,8 +790,7 @@ static int cortexm_check_hw_wp(target *t, target_addr *addr)
 
 	if(i == priv->hw_watchpoint_max) return 0;
 
-	*addr = priv->hw_watchpoint[i].addr;
-	return 1;
+	return priv->hw_watchpoint[i].addr;
 }
 
 static bool cortexm_vector_catch(target *t, int argc, char *argv[])

src/target/lpc_common.c

@@ -82,7 +82,7 @@ enum iap_status lpc_iap_call(struct lpc_flash *f, enum iap_cmd cmd, ...)
 
 	/* start the target and wait for it to halt again */
 	target_halt_resume(t, false);
-	while (!target_halt_wait(t));
+	while (!target_halt_poll(t, NULL));
 
 	/* copy back just the parameters structure */
 	target_mem_read(t, &param, f->iap_ram, sizeof(param));

src/target/target.c

@@ -305,47 +305,52 @@ void target_regs_write(target *t, const void *data) { t->regs_write(t, data); }
 /* Halt/resume functions */
 void target_reset(target *t) { t->reset(t); }
 void target_halt_request(target *t) { t->halt_request(t); }
-int target_halt_wait(target *t) { return t->halt_wait(t); }
-void target_halt_resume(target *t, bool step) { t->halt_resume(t, step); }
-
-/* Break-/watchpoint functions */
-int target_set_hw_bp(target *t, target_addr addr, uint8_t len)
+enum target_halt_reason target_halt_poll(target *t, target_addr *watch)
 {
-	if (t->set_hw_bp == NULL)
-		return 0;
-	return t->set_hw_bp(t, addr, len);
+	return t->halt_poll(t, watch);
 }
 
-int target_clear_hw_bp(target *t, target_addr addr, uint8_t len)
-{
-	if (t->clear_hw_bp == NULL)
-		return 0;
-	return t->clear_hw_bp(t, addr, len);
-}
-
-int target_set_hw_wp(target *t, uint8_t type, target_addr addr, uint8_t len)
-{
-	if (t->set_hw_wp == NULL)
-		return 0;
-	return t->set_hw_wp(t, type, addr, len);
-}
-
-int target_clear_hw_wp(target *t, uint8_t type, target_addr addr, uint8_t len)
-{
-	if (t->clear_hw_wp == NULL)
-		return 0;
-	return t->clear_hw_wp(t, type, addr, len);
-}
+void target_halt_resume(target *t, bool step) { t->halt_resume(t, step); }
 
-int target_check_hw_wp(target *t, target_addr *addr)
-{
-	if (t->check_hw_wp == NULL)
-		return 0;
-	return t->check_hw_wp(t, addr);
+/* Break-/watchpoint functions */
+int target_breakwatch_set(target *t,
+                          enum target_breakwatch type, target_addr addr, size_t len)
+{
+	switch (type) {
+	case TARGET_BREAK_HARD:
+		if (t->set_hw_bp)
+			return t->set_hw_bp(t, addr, len);
+	case TARGET_WATCH_WRITE:
+	case TARGET_WATCH_READ:
+	case TARGET_WATCH_ACCESS:
+		if (t->set_hw_wp)
+			return t->set_hw_wp(t, type, addr, len);
+	default:
+		break;
+	}
+	return 1;
+}
+
+int target_breakwatch_clear(target *t,
+                            enum target_breakwatch type, target_addr addr, size_t len)
+{
+	switch (type) {
+	case TARGET_BREAK_HARD:
+		if (t->set_hw_bp)
+			return t->set_hw_bp(t, addr, len);
+	case TARGET_WATCH_WRITE:
+	case TARGET_WATCH_READ:
+	case TARGET_WATCH_ACCESS:
+		if (t->set_hw_wp)
+			return t->set_hw_wp(t, type, addr, len);
+	default:
+		break;
+	}
+	return 1;
 }
 
 /* Accessor functions */
-int target_regs_size(target *t)
+size_t target_regs_size(target *t)
 {
 	return t->regs_size;
 }

src/target/target_internal.h

@@ -84,7 +84,7 @@ struct target_s {
 	                  const void *src, size_t len);
 
 	/* Register access functions */
-	int regs_size;
+	size_t regs_size;
 	const char *tdesc;
 	void (*regs_read)(target *t, void *data);
 	void (*regs_write)(target *t, const void *data);
@@ -92,7 +92,7 @@ struct target_s {
 	/* Halt/resume functions */
 	void (*reset)(target *t);
 	void (*halt_request)(target *t);
-	int (*halt_wait)(target *t);
+	enum target_halt_reason (*halt_poll)(target *t, target_addr *watch);
 	void (*halt_resume)(target *t, bool step);
 
 	/* Break-/watchpoint functions */
@@ -102,8 +102,6 @@ struct target_s {
 	int (*set_hw_wp)(target *t, uint8_t type, target_addr addr, uint8_t len);
 	int (*clear_hw_wp)(target *t, uint8_t type, target_addr addr, uint8_t len);
 
-	int (*check_hw_wp)(target *t, target_addr *addr);
-
 	/* target-defined options */
 	unsigned target_options;
 	uint32_t idcode;