Module Name:    src
Committed By:   ryo
Date:           Tue Mar  9 16:44:27 UTC 2021

Modified Files:
        src/sys/arch/aarch64/aarch64: cpu.c cpuswitch.S db_machdep.c trap.c
            vectors.S
        src/sys/arch/aarch64/include: db_machdep.h

Log Message:
Add support hardware breakpoint and watchpoint again.

Limited support for hardware watchpoint has been available for some time, but it
has not been working properly. In addition, it stopped working at the time of
the PTRACE support commit on 2018-12-13. This has been fixed to work correctly,
and also fixed to be practical by sharing hardware watchpoints and breakpoints
between CPUs on MULTIPROCESSOR.

Also fixed a bug that causes a malfunction when switching CPUs with
"machine cpu N" when entering ddb mode from other than cpu_Debugger().

I have confirmed that the CPU can be switched by "machine cpu N" and return from
ddb properly in each case where ddb is called triggered by ddb break/watchpoint,
hardware break/watchpoint, and cpu_Debugger().


To generate a diff of this commit:
cvs rdiff -u -r1.58 -r1.59 src/sys/arch/aarch64/aarch64/cpu.c
cvs rdiff -u -r1.32 -r1.33 src/sys/arch/aarch64/aarch64/cpuswitch.S
cvs rdiff -u -r1.36 -r1.37 src/sys/arch/aarch64/aarch64/db_machdep.c
cvs rdiff -u -r1.44 -r1.45 src/sys/arch/aarch64/aarch64/trap.c
cvs rdiff -u -r1.21 -r1.22 src/sys/arch/aarch64/aarch64/vectors.S
cvs rdiff -u -r1.11 -r1.12 src/sys/arch/aarch64/include/db_machdep.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/arch/aarch64/aarch64/cpu.c
diff -u src/sys/arch/aarch64/aarch64/cpu.c:1.58 src/sys/arch/aarch64/aarch64/cpu.c:1.59
--- src/sys/arch/aarch64/aarch64/cpu.c:1.58	Mon Jan 11 21:58:31 2021
+++ src/sys/arch/aarch64/aarch64/cpu.c	Tue Mar  9 16:44:27 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: cpu.c,v 1.58 2021/01/11 21:58:31 skrll Exp $ */
+/* $NetBSD: cpu.c,v 1.59 2021/03/09 16:44:27 ryo Exp $ */
 
 /*
  * Copyright (c) 2017 Ryo Shimizu <r...@nerv.org>
@@ -27,10 +27,11 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(1, "$NetBSD: cpu.c,v 1.58 2021/01/11 21:58:31 skrll Exp $");
+__KERNEL_RCSID(1, "$NetBSD: cpu.c,v 1.59 2021/03/09 16:44:27 ryo Exp $");
 
 #include "locators.h"
 #include "opt_arm_debug.h"
+#include "opt_ddb.h"
 #include "opt_fdt.h"
 #include "opt_multiprocessor.h"
 
@@ -53,6 +54,9 @@ __KERNEL_RCSID(1, "$NetBSD: cpu.c,v 1.58
 #include <aarch64/armreg.h>
 #include <aarch64/cpu.h>
 #include <aarch64/cpu_counter.h>
+#ifdef DDB
+#include <aarch64/db_machdep.h>
+#endif
 #include <aarch64/machdep.h>
 
 #include <arm/cpufunc.h>
@@ -681,7 +685,9 @@ cpu_hatch(struct cpu_info *ci)
 	aarch64_getcacheinfo(device_unit(ci->ci_dev));
 	aarch64_printcacheinfo(ci->ci_dev);
 	cpu_identify2(ci->ci_dev, ci);
-
+#ifdef DDB
+	db_machdep_init();
+#endif
 	mutex_exit(&cpu_hatch_lock);
 
 	cpu_init_counter(ci);

Index: src/sys/arch/aarch64/aarch64/cpuswitch.S
diff -u src/sys/arch/aarch64/aarch64/cpuswitch.S:1.32 src/sys/arch/aarch64/aarch64/cpuswitch.S:1.33
--- src/sys/arch/aarch64/aarch64/cpuswitch.S:1.32	Sat Dec 26 00:55:26 2020
+++ src/sys/arch/aarch64/aarch64/cpuswitch.S	Tue Mar  9 16:44:27 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: cpuswitch.S,v 1.32 2020/12/26 00:55:26 jmcneill Exp $ */
+/* $NetBSD: cpuswitch.S,v 1.33 2021/03/09 16:44:27 ryo Exp $ */
 
 /*-
  * Copyright (c) 2014, 2020 The NetBSD Foundation, Inc.
@@ -38,7 +38,7 @@
 #include "opt_ddb.h"
 #include "opt_kasan.h"
 
-RCSID("$NetBSD: cpuswitch.S,v 1.32 2020/12/26 00:55:26 jmcneill Exp $")
+RCSID("$NetBSD: cpuswitch.S,v 1.33 2021/03/09 16:44:27 ryo Exp $")
 
 	ARMV8_DEFINE_OPTIONS
 
@@ -310,7 +310,7 @@ END(lwp_trampoline)
 
 #ifdef DDB
 ENTRY_NP(cpu_Debugger)
-	brk	#0
+	brk	#0xffff
 	ret
 END(cpu_Debugger)
 #endif /* DDB */

Index: src/sys/arch/aarch64/aarch64/db_machdep.c
diff -u src/sys/arch/aarch64/aarch64/db_machdep.c:1.36 src/sys/arch/aarch64/aarch64/db_machdep.c:1.37
--- src/sys/arch/aarch64/aarch64/db_machdep.c:1.36	Tue Mar  9 16:43:13 2021
+++ src/sys/arch/aarch64/aarch64/db_machdep.c	Tue Mar  9 16:44:27 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: db_machdep.c,v 1.36 2021/03/09 16:43:13 ryo Exp $ */
+/* $NetBSD: db_machdep.c,v 1.37 2021/03/09 16:44:27 ryo Exp $ */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_machdep.c,v 1.36 2021/03/09 16:43:13 ryo Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_machdep.c,v 1.37 2021/03/09 16:44:27 ryo Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd32.h"
@@ -75,6 +75,7 @@ void db_md_reset_cmd(db_expr_t, bool, db
 void db_md_tlbi_cmd(db_expr_t, bool, db_expr_t, const char *);
 void db_md_ttbr_cmd(db_expr_t, bool, db_expr_t, const char *);
 void db_md_sysreg_cmd(db_expr_t, bool, db_expr_t, const char *);
+void db_md_break_cmd(db_expr_t, bool, db_expr_t, const char *);
 void db_md_watch_cmd(db_expr_t, bool, db_expr_t, const char *);
 #if defined(_KERNEL) && defined(MULTIPROCESSOR)
 void db_md_switch_cpu_cmd(db_expr_t, bool, db_expr_t, const char *);
@@ -83,6 +84,26 @@ void db_md_switch_cpu_cmd(db_expr_t, boo
 static void db_md_meminfo_cmd(db_expr_t, bool, db_expr_t, const char *);
 #endif
 
+#ifdef _KERNEL
+#define MAX_BREAKPOINT	15
+#define MAX_WATCHPOINT	15
+/* The number varies depending on the CPU core (e.g. big.LITTLE) */
+static int max_breakpoint = MAX_BREAKPOINT;
+static int max_watchpoint = MAX_WATCHPOINT;
+
+struct breakpoint_info {
+	db_addr_t addr;
+};
+static struct breakpoint_info breakpoint_buf[MAX_BREAKPOINT + 1];
+
+struct watchpoint_info {
+	db_addr_t addr;
+	int size;
+	int accesstype;
+};
+static struct watchpoint_info watchpoint_buf[MAX_WATCHPOINT + 1];
+#endif
+
 const struct db_command db_machine_command_table[] = {
 #if defined(_KERNEL) && defined(MULTIPROCESSOR)
 	{
@@ -95,6 +116,14 @@ const struct db_command db_machine_comma
 #if defined(_KERNEL)
 	{
 		DDB_ADD_CMD(
+		    "break", db_md_break_cmd, 0,
+		    "set or clear breakpoint",
+		    "[address|#]",
+		    "\taddress: breakpoint address to set\n"
+		    "\t#: breakpoint number to remove\n")
+	},
+	{
+		DDB_ADD_CMD(
 		    "cpuinfo", db_md_cpuinfo_cmd, 0,
 		    "Displays the current cpuinfo",
 		    NULL, NULL)
@@ -661,70 +690,27 @@ aarch64_set_wcr_wvr(int n, uint64_t wcr,
 	}
 }
 
-static uint64_t
-aarch64_get_dbgwcr(int n)
-{
-#define DBGWCR_READ(regno)	(reg_dbgwcr ## regno ## _el1_read())
-
-	switch (n) {
-	case 0:		return DBGWCR_READ(0);
-	case 1:		return DBGWCR_READ(1);
-	case 2:		return DBGWCR_READ(2);
-	case 3:		return DBGWCR_READ(3);
-	case 4:		return DBGWCR_READ(4);
-	case 5:		return DBGWCR_READ(5);
-	case 6:		return DBGWCR_READ(6);
-	case 7:		return DBGWCR_READ(7);
-	case 8:		return DBGWCR_READ(8);
-	case 9:		return DBGWCR_READ(9);
-	case 10:	return DBGWCR_READ(10);
-	case 11:	return DBGWCR_READ(11);
-	case 12:	return DBGWCR_READ(12);
-	case 13:	return DBGWCR_READ(13);
-	case 14:	return DBGWCR_READ(14);
-	case 15:	return DBGWCR_READ(15);
-	}
-
-	return 0;
-}
-
-static uint64_t
-aarch64_get_dbgwvr(int n)
+void
+aarch64_breakpoint_set(int n, vaddr_t addr)
 {
-#define DBGWVR_READ(regno)	(reg_dbgwvr ## regno ## _el1_read())
+	uint64_t bcr, bvr;
 
-	switch (n) {
-	case 0:		return DBGWVR_READ(0);
-	case 1:		return DBGWVR_READ(1);
-	case 2:		return DBGWVR_READ(2);
-	case 3:		return DBGWVR_READ(3);
-	case 4:		return DBGWVR_READ(4);
-	case 5:		return DBGWVR_READ(5);
-	case 6:		return DBGWVR_READ(6);
-	case 7:		return DBGWVR_READ(7);
-	case 8:		return DBGWVR_READ(8);
-	case 9:		return DBGWVR_READ(9);
-	case 10:	return DBGWVR_READ(10);
-	case 11:	return DBGWVR_READ(11);
-	case 12:	return DBGWVR_READ(12);
-	case 13:	return DBGWVR_READ(13);
-	case 14:	return DBGWVR_READ(14);
-	case 15:	return DBGWVR_READ(15);
+	if (addr == 0) {
+		bvr = 0;
+		bcr = 0;
+	} else {
+		bvr = addr;
+		bcr =
+		    __SHIFTIN(0, DBGBCR_BT) |
+		    __SHIFTIN(0, DBGBCR_LBN) |
+		    __SHIFTIN(0, DBGBCR_SSC) |
+		    __SHIFTIN(0, DBGBCR_HMC) |
+		    __SHIFTIN(15, DBGBCR_BAS) |
+		    __SHIFTIN(3, DBGBCR_PMC) |
+		    __SHIFTIN(1, DBGBCR_E);
 	}
 
-	return 0;
-}
-
-void
-aarch64_breakpoint_clear(int n)
-{
-	aarch64_set_bcr_bvr(n, 0, 0);
-}
-
-void
-aarch64_watchpoint_clear(int n)
-{
-	aarch64_set_wcr_wvr(n, 0, 0);
+	aarch64_set_bcr_bvr(n, bcr, bvr);
 }
 
 void
@@ -750,7 +736,7 @@ aarch64_watchpoint_set(int n, vaddr_t ad
 		wvr = 0;
 		wcr = 0;
 	} else {
-		wvr = addr & DBGWVR_MASK;
+		wvr = addr;
 		wcr =
 		    __SHIFTIN(0, DBGWCR_MASK) |		/* MASK: no mask */
 		    __SHIFTIN(0, DBGWCR_WT) |		/* WT: 0 */
@@ -766,70 +752,135 @@ aarch64_watchpoint_set(int n, vaddr_t ad
 	aarch64_set_wcr_wvr(n, wcr, wvr);
 }
 
-#define MAX_BREAKPOINT	15
-static int max_breakpoint = MAX_BREAKPOINT;
+static void
+db_md_breakpoint_set(int n, vaddr_t addr)
+{
+	if (n >= __arraycount(breakpoint_buf))
+		return;
 
-#define MAX_WATCHPOINT	15
-static int max_watchpoint = MAX_WATCHPOINT;
+	breakpoint_buf[n].addr = addr;
+}
+
+static void
+db_md_watchpoint_set(int n, vaddr_t addr, int size, int accesstype)
+{
+	if (n >= __arraycount(watchpoint_buf))
+		return;
+
+	watchpoint_buf[n].addr = addr;
+	watchpoint_buf[n].size = size;
+	watchpoint_buf[n].accesstype = accesstype;
+}
+
+static void
+db_md_breakwatchpoints_clear(void)
+{
+	int i;
+
+	for (i = 0; i <= max_breakpoint; i++)
+		aarch64_breakpoint_set(i, 0);
+	for (i = 0; i <= max_watchpoint; i++)
+		aarch64_watchpoint_set(i, 0, 0, 0);
+}
+
+static void
+db_md_breakwatchpoints_reload(void)
+{
+	int i;
+
+	for (i = 0; i <= max_breakpoint; i++) {
+		aarch64_breakpoint_set(i,
+		    breakpoint_buf[i].addr);
+	}
+	for (i = 0; i <= max_watchpoint; i++) {
+		aarch64_watchpoint_set(i,
+		    watchpoint_buf[i].addr,
+		    watchpoint_buf[i].size,
+		    watchpoint_buf[i].accesstype);
+	}
+}
 
 void
 db_machdep_init(void)
 {
 	uint64_t dfr, mdscr;
-	int i;
+	int i, cpu_max_breakpoint, cpu_max_watchpoint;
 
 	dfr = reg_id_aa64dfr0_el1_read();
-	max_breakpoint = __SHIFTOUT(dfr, ID_AA64DFR0_EL1_BRPS);
-	max_watchpoint = __SHIFTOUT(dfr, ID_AA64DFR0_EL1_WRPS);
+	cpu_max_breakpoint = __SHIFTOUT(dfr, ID_AA64DFR0_EL1_BRPS);
+	cpu_max_watchpoint = __SHIFTOUT(dfr, ID_AA64DFR0_EL1_WRPS);
 
-	for (i = 0; i <= max_breakpoint; i++) {
+	for (i = 0; i <= cpu_max_breakpoint; i++) {
 		/* clear all breakpoints */
-		aarch64_breakpoint_clear(i);
+		aarch64_breakpoint_set(i, 0);
 	}
-	for (i = 0; i <= max_watchpoint; i++) {
+	for (i = 0; i <= cpu_max_watchpoint; i++) {
 		/* clear all watchpoints */
-		aarch64_watchpoint_clear(i);
+		aarch64_watchpoint_set(i, 0, 0, 0);
 	}
 
+	/* enable watchpoint and breakpoint */
 	mdscr = reg_mdscr_el1_read();
-	mdscr |= MDSCR_MDE;	/* enable watchpoint and breakpoint */
+	mdscr |= MDSCR_MDE | MDSCR_KDE;
 	reg_mdscr_el1_write(mdscr);
 	reg_oslar_el1_write(0);
+
+	/* num of {watch,break}point may be different depending on the core */
+	membar_consumer();
+	if (max_breakpoint > cpu_max_breakpoint)
+		max_breakpoint = cpu_max_breakpoint;
+	if (max_watchpoint > cpu_max_watchpoint)
+		max_watchpoint = cpu_max_watchpoint;
+	membar_producer();
+}
+
+static void
+show_breakpoints(void)
+{
+	uint64_t addr;
+	unsigned int i, nused;
+
+	for (nused = 0, i = 0; i <= max_breakpoint; i++) {
+		addr = breakpoint_buf[i].addr;
+		if (addr == 0) {
+			db_printf("%d: disabled\n", i);
+		} else {
+			db_printf("%d: breakpoint %016" PRIx64 " (", i,
+			    addr);
+			db_printsym(addr, DB_STGY_ANY, db_printf);
+			db_printf(")\n");
+			nused++;
+		}
+	}
+	db_printf("breakpoint used %d/%d\n", nused, max_breakpoint + 1);
 }
 
 static void
 show_watchpoints(void)
 {
-	uint64_t wcr, addr;
-	unsigned int i, bas, offset, size, nused;
+	uint64_t addr;
+	unsigned int i, nused;
 
 	for (nused = 0, i = 0; i <= max_watchpoint; i++) {
-		addr = aarch64_get_dbgwvr(i) & DBGWVR_MASK;
-		wcr = aarch64_get_dbgwcr(i);
-		if (wcr & DBGWCR_E) {
-			bas = __SHIFTOUT(wcr, DBGWCR_BAS);
-			if (bas == 0) {
-				db_printf("%d: disabled %016" PRIx64, i, addr);
-			} else {
-				offset = ffs(bas) - 1;
-				addr += offset;
-				bas >>= offset;
-				size = ffs(~bas) - 1;
-
-				db_printf("%d: watching %016" PRIx64 ", %d bytes", i,
-				    addr, size);
-
-				switch (__SHIFTOUT(wcr, DBGWCR_LSC)) {
-				case WATCHPOINT_ACCESS_LOAD:
-					db_printf(", load");
-					break;
-				case WATCHPOINT_ACCESS_STORE:
-					db_printf(", store");
-					break;
-				case WATCHPOINT_ACCESS_LOADSTORE:
-					db_printf(", load/store");
-					break;
-				}
+		addr = watchpoint_buf[i].addr;
+		if (addr == 0) {
+			db_printf("%d: disabled\n", i);
+		} else {
+			db_printf("%d: watching %016" PRIx64 " (", i,
+			    addr);
+			db_printsym(addr, DB_STGY_ANY, db_printf);
+			db_printf("), %d bytes", watchpoint_buf[i].size);
+
+			switch (watchpoint_buf[i].accesstype) {
+			case WATCHPOINT_ACCESS_LOAD:
+				db_printf(", load");
+				break;
+			case WATCHPOINT_ACCESS_STORE:
+				db_printf(", store");
+				break;
+			case WATCHPOINT_ACCESS_LOADSTORE:
+				db_printf(", load/store");
+				break;
 			}
 			db_printf("\n");
 			nused++;
@@ -839,6 +890,57 @@ show_watchpoints(void)
 }
 
 void
+db_md_break_cmd(db_expr_t addr, bool have_addr, db_expr_t count,
+    const char *modif)
+{
+	int i;
+	int added, cleared;
+
+	if (!have_addr) {
+		show_breakpoints();
+		return;
+	}
+
+	addr &= DBGBVR_MASK;
+	added = -1;
+	cleared = -1;
+	if (0 <= addr && addr <= max_breakpoint) {
+		i = addr;
+		if (breakpoint_buf[i].addr != 0) {
+			db_md_breakpoint_set(i, 0);
+			cleared = i;
+		}
+	} else {
+		for (i = 0; i <= max_breakpoint; i++) {
+			if (breakpoint_buf[i].addr == addr) {
+				db_md_breakpoint_set(i, 0);
+				cleared = i;
+			}
+		}
+		if (cleared == -1) {
+			for (i = 0; i <= max_breakpoint; i++) {
+				if (breakpoint_buf[i].addr == 0) {
+					db_md_breakpoint_set(i, addr);
+					added = i;
+					break;
+				}
+			}
+			if (i > max_breakpoint) {
+				db_printf("no more available breakpoint\n");
+			}
+		}
+	}
+
+	if (added >= 0)
+		db_printf("add breakpoint %d as %016"DDB_EXPR_FMT"x\n",
+		    added, addr);
+	if (cleared >= 0)
+		db_printf("clear breakpoint %d\n", cleared);
+
+	show_breakpoints();
+}
+
+void
 db_md_watch_cmd(db_expr_t addr, bool have_addr, db_expr_t count,
     const char *modif)
 {
@@ -851,6 +953,7 @@ db_md_watch_cmd(db_expr_t addr, bool hav
 		return;
 	}
 
+	addr &= DBGWVR_MASK;
 	accesstype = watchsize = 0;
 	if ((modif != NULL) && (*modif != '\0')) {
 		int ch;
@@ -886,26 +989,22 @@ db_md_watch_cmd(db_expr_t addr, bool hav
 	cleared = -1;
 	if (0 <= addr && addr <= max_watchpoint) {
 		i = addr;
-		if (aarch64_get_dbgwcr(i) & DBGWCR_E) {
-			/* clear watch */
-			aarch64_watchpoint_set(i, 0, 0, 0);
+		if (watchpoint_buf[i].addr != 0) {
+			db_md_watchpoint_set(i, 0, 0, 0);
 			cleared = i;
 		}
 	} else {
 		for (i = 0; i <= max_watchpoint; i++) {
-			if ((aarch64_get_dbgwvr(i) & DBGWVR_MASK) ==
-			    (addr & DBGWVR_MASK)) {
-				/* clear watch */
-				aarch64_watchpoint_set(i, 0, 0, 0);
+			if (watchpoint_buf[i].addr == addr) {
+				db_md_watchpoint_set(i, 0, 0, 0);
 				cleared = i;
 			}
 		}
 		if (cleared == -1) {
 			for (i = 0; i <= max_watchpoint; i++) {
-				if (!(aarch64_get_dbgwcr(i) & DBGWCR_E)) {
-					/* add watch */
-					aarch64_watchpoint_set(i, addr,
-					    watchsize, accesstype);
+				if (watchpoint_buf[i].addr == 0) {
+					db_md_watchpoint_set(i, addr, watchsize,
+					    accesstype);
 					added = i;
 					break;
 				}
@@ -917,7 +1016,8 @@ db_md_watch_cmd(db_expr_t addr, bool hav
 	}
 
 	if (added >= 0)
-		db_printf("add watchpoint %d as %016"DDB_EXPR_FMT"x\n", added, addr);
+		db_printf("add watchpoint %d as %016"DDB_EXPR_FMT"x\n",
+		    added, addr);
 	if (cleared >= 0)
 		db_printf("clear watchpoint %d\n", cleared);
 
@@ -941,15 +1041,14 @@ db_md_switch_cpu_cmd(db_expr_t addr, boo
 	int i;
 
 	membar_consumer();
-
 	if (!have_addr) {
 		for (i = 0; i < ncpu; i++) {
 			if (db_readytoswitch[i] != NULL) {
 				db_printf("cpu%d: ready. tf=%p, pc=%016lx ", i,
-					db_readytoswitch[i],
-					db_readytoswitch[i]->tf_pc);
+				    db_readytoswitch[i],
+				    db_readytoswitch[i]->tf_pc);
 				db_printsym(db_readytoswitch[i]->tf_pc,
-					DB_STGY_ANY, db_printf);
+				    DB_STGY_ANY, db_printf);
 				db_printf("\n");
 			} else {
 				db_printf("cpu%d: not responding\n", i);
@@ -972,14 +1071,6 @@ db_md_switch_cpu_cmd(db_expr_t addr, boo
 	if (new_ci == curcpu())
 		return;
 
-	/* XXX */
-	membar_consumer();
-	if (db_trigger == curcpu()) {
-		DDB_REGS->tf_pc -= 4;
-		db_trigger = NULL;
-		membar_producer();
-	}
-
 	db_newcpu = new_ci;
 	db_continue_cmd(0, false, 0, "");
 }
@@ -993,15 +1084,34 @@ kdb_trap(int type, struct trapframe *tf)
 {
 #ifdef MULTIPROCESSOR
 	struct cpu_info * const ci = curcpu();
+	bool static_brk = false;
 #endif
 	int s;
+	bool restore_hw_watchpoints = true;
 
 	switch (type) {
-	case DB_TRAP_UNKNOWN:
+	case DB_TRAP_WATCHPOINT:
 	case DB_TRAP_BREAKPOINT:
+		/*
+		 * In the case of a hardware watchpoint or breakpoint,
+		 * even if cpu return from ddb as is, it will be trapped again,
+		 * so clear it all once.
+		 *
+		 * breakpoint and watchpoint will be restored at the end of
+		 * next DB_TRAP_BKPT_INSN (ddb's STEP_INVISIBLE mode).
+		 */
+		db_md_breakwatchpoints_clear();
+		restore_hw_watchpoints = false;
+		break;
 	case DB_TRAP_BKPT_INSN:
-	case DB_TRAP_WATCHPOINT:
+#ifdef MULTIPROCESSOR
+		/* brk #0xffff in cpu_Debugger() ? */
+		if (__SHIFTOUT(tf->tf_esr, ESR_ISS) == 0xffff)
+			static_brk = true;
+		/* FALLTHRU */
+#endif
 	case DB_TRAP_SW_STEP:
+	case DB_TRAP_UNKNOWN:
 	case -1:	/* from pic_ipi_ddb() */
 		break;
 	default:
@@ -1021,25 +1131,32 @@ kdb_trap(int type, struct trapframe *tf)
 	    (atomic_cas_ptr(&db_onproc, NULL, ci) == NULL)) {
 		intr_ipi_send(NULL, IPI_DDB);
 		db_trigger = ci;
-		membar_producer();
+	} else {
+		/*
+		 * If multiple CPUs catch kdb_trap() that is not IPI_DDB derived
+		 * at the same time, only the CPU that was able to get db_onproc
+		 * first will execute db_trap.
+		 * The CPU that could not get db_onproc will be set to type = -1
+		 * once, and kdb_trap will be called again with the correct type
+		 * after kdb_trap returns.
+		 */
+		type = -1;
+		restore_hw_watchpoints = true;
 	}
 	db_readytoswitch[ci->ci_index] = tf;
-	membar_producer();
 #endif
 
 	for (;;) {
 #ifdef MULTIPROCESSOR
 		if (ncpu > 1) {
-
 			/* waiting my turn, or exit */
-			membar_consumer();
+			dsb(ishld);
 			while (db_onproc != ci) {
 				__asm __volatile ("wfe");
 
-				membar_consumer();
-				if (db_onproc == NULL) {
-					return 1;
-				}
+				dsb(ishld);
+				if (db_onproc == NULL)
+					goto kdb_trap_done;
 			}
 			/* It's my turn! */
 		}
@@ -1059,12 +1176,46 @@ kdb_trap(int type, struct trapframe *tf)
 		*tf = ddb_regs;
 
 #ifdef MULTIPROCESSOR
-		if ((ncpu > 1) && (db_newcpu != NULL)) {
+		if (ncpu < 2)
+			break;
+
+		if (db_newcpu == NULL && db_onproc != db_trigger) {
+			/*
+			 * If the "machine cpu" switches CPUs after entering
+			 * ddb from a breakpoint or watchpoint, it will return
+			 * control to the CPU that triggered the ddb in the
+			 * first place in order to correctly reset the
+			 * breakpoint or watchpoint.
+			 * If db_trap() returns further from here,
+			 * watchpoints and breakpoints will be reset.
+			 * (db_run_mode = STEP_INVISIBLE)
+			 */
+			db_newcpu = db_trigger;
+		}
+
+		if (db_newcpu != NULL) {
+			/* XXX: override sys/ddb/db_run.c:db_run_mode */
+			db_continue_cmd(0, false, 0, "");
+
+			/*
+			 * When static BRK instruction (cpu_Debugger()),
+			 * db_trap() advance the $PC to the next instruction of
+			 * BRK. If db_trap() will be called twice by
+			 * "machine cpu" command, the $PC will be advanced to
+			 * the after the next instruction.
+			 * To avoid this, change 'type' so that the second call
+			 * to db_trap() will not change the PC.
+			 */
+			if (static_brk)
+				type = -1;
+
 			db_onproc = db_newcpu;
 			db_newcpu = NULL;
 			dsb(ishst);
+			/* waking up the CPU waiting for its turn to db_trap */
 			sev();
-			continue;	/* redo DDB on new cpu */
+
+			continue;	/* go to waiting my turn */
 		}
 #endif /* MULTIPROCESSOR */
 
@@ -1072,15 +1223,35 @@ kdb_trap(int type, struct trapframe *tf)
 	}
 
 #ifdef MULTIPROCESSOR
-	if (ncpu > 1) {
+	if (ncpu > 1 && db_onproc == ci) {
 		db_onproc = NULL;
 		dsb(ishst);
+		/* waking up the CPU waiting for its turn to exit */
+		sev();
+
+		db_readytoswitch[cpu_index(ci)] = NULL;
+		/* wait for all other CPUs are ready to exit */
+		for (;;) {
+			int i;
+			dsb(ishld);
+			for (i = 0; i < ncpu; i++) {
+				if (db_readytoswitch[i] != NULL)
+					break;
+			}
+			if (i == ncpu)
+				break;
+		}
+		db_trigger = NULL;
 		sev();
+	} else {
+ kdb_trap_done:
+		db_readytoswitch[cpu_index(ci)] = NULL;
+		dsb(ishst);
+		__asm __volatile ("wfe");
 	}
-	db_trigger = NULL;
-	db_readytoswitch[ci->ci_index] = NULL;
-	membar_producer();
 #endif
+	if (restore_hw_watchpoints)
+		db_md_breakwatchpoints_reload();
 
 	return 1;
 }

Index: src/sys/arch/aarch64/aarch64/trap.c
diff -u src/sys/arch/aarch64/aarch64/trap.c:1.44 src/sys/arch/aarch64/aarch64/trap.c:1.45
--- src/sys/arch/aarch64/aarch64/trap.c:1.44	Mon Feb 22 02:18:33 2021
+++ src/sys/arch/aarch64/aarch64/trap.c	Tue Mar  9 16:44:27 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: trap.c,v 1.44 2021/02/22 02:18:33 jmcneill Exp $ */
+/* $NetBSD: trap.c,v 1.45 2021/03/09 16:44:27 ryo Exp $ */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
 
 #include <sys/cdefs.h>
 
-__KERNEL_RCSID(1, "$NetBSD: trap.c,v 1.44 2021/02/22 02:18:33 jmcneill Exp $");
+__KERNEL_RCSID(1, "$NetBSD: trap.c,v 1.45 2021/03/09 16:44:27 ryo Exp $");
 
 #include "opt_arm_intr_impl.h"
 #include "opt_compat_netbsd32.h"
@@ -416,8 +416,14 @@ trap_el0_sync(struct trapframe *tf)
 	const uint32_t esr = tf->tf_esr;
 	const uint32_t eclass = __SHIFTOUT(esr, ESR_EC); /* exception class */
 
+#ifdef DDB
+	/* disable trace, and enable hardware breakpoint/watchpoint */
+	reg_mdscr_el1_write(
+	    (reg_mdscr_el1_read() & ~MDSCR_SS) | MDSCR_KDE);
+#else
 	/* disable trace */
 	reg_mdscr_el1_write(reg_mdscr_el1_read() & ~MDSCR_SS);
+#endif
 	/* enable traps and interrupts */
 	daif_enable(DAIF_D|DAIF_A|DAIF_I|DAIF_F);
 
@@ -511,9 +517,14 @@ interrupt(struct trapframe *tf)
 	}
 #endif
 
+#ifdef DDB
+	/* disable trace, and enable hardware breakpoint/watchpoint */
+	reg_mdscr_el1_write(
+	    (reg_mdscr_el1_read() & ~MDSCR_SS) | MDSCR_KDE);
+#else
 	/* disable trace */
 	reg_mdscr_el1_write(reg_mdscr_el1_read() & ~MDSCR_SS);
-
+#endif
 	/* enable traps */
 	daif_enable(DAIF_D|DAIF_A);
 
@@ -761,8 +772,14 @@ trap_el0_32sync(struct trapframe *tf)
 	const uint32_t esr = tf->tf_esr;
 	const uint32_t eclass = __SHIFTOUT(esr, ESR_EC); /* exception class */
 
+#ifdef DDB
+	/* disable trace, and enable hardware breakpoint/watchpoint */
+	reg_mdscr_el1_write(
+	    (reg_mdscr_el1_read() & ~MDSCR_SS) | MDSCR_KDE);
+#else
 	/* disable trace */
 	reg_mdscr_el1_write(reg_mdscr_el1_read() & ~MDSCR_SS);
+#endif
 	/* enable traps and interrupts */
 	daif_enable(DAIF_D|DAIF_A|DAIF_I|DAIF_F);
 

Index: src/sys/arch/aarch64/aarch64/vectors.S
diff -u src/sys/arch/aarch64/aarch64/vectors.S:1.21 src/sys/arch/aarch64/aarch64/vectors.S:1.22
--- src/sys/arch/aarch64/aarch64/vectors.S:1.21	Thu Oct 15 08:37:20 2020
+++ src/sys/arch/aarch64/aarch64/vectors.S	Tue Mar  9 16:44:27 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: vectors.S,v 1.21 2020/10/15 08:37:20 ryo Exp $	*/
+/*	$NetBSD: vectors.S,v 1.22 2021/03/09 16:44:27 ryo Exp $	*/
 
 #include <aarch64/asm.h>
 #include <aarch64/locore.h>
@@ -10,7 +10,7 @@
 #include "opt_ddb.h"
 #include "opt_dtrace.h"
 
-RCSID("$NetBSD: vectors.S,v 1.21 2020/10/15 08:37:20 ryo Exp $")
+RCSID("$NetBSD: vectors.S,v 1.22 2021/03/09 16:44:27 ryo Exp $")
 
 	ARMV8_DEFINE_OPTIONS
 
@@ -313,6 +313,9 @@ ENTRY_NP(el0_trap_exit)
 	tbz	x1, #SPSR_SS_SHIFT, 1f
 	mrs	x0, mdscr_el1
 	orr	x0, x0, #MDSCR_SS
+#ifdef DDB
+	bic	x0, x0, #MDSCR_KDE
+#endif
 	msr	mdscr_el1, x0
 1:
 	unwind_x0_x2

Index: src/sys/arch/aarch64/include/db_machdep.h
diff -u src/sys/arch/aarch64/include/db_machdep.h:1.11 src/sys/arch/aarch64/include/db_machdep.h:1.12
--- src/sys/arch/aarch64/include/db_machdep.h:1.11	Mon Sep 14 10:53:02 2020
+++ src/sys/arch/aarch64/include/db_machdep.h	Tue Mar  9 16:44:27 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: db_machdep.h,v 1.11 2020/09/14 10:53:02 ryo Exp $ */
+/* $NetBSD: db_machdep.h,v 1.12 2021/03/09 16:44:27 ryo Exp $ */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -96,9 +96,9 @@ int kdb_trap(int, struct trapframe *);
 #define DB_TRAP_SW_STEP		4
 
 #define IS_BREAKPOINT_TRAP(type, code) \
-	((type) == DB_TRAP_BREAKPOINT || (type) == DB_TRAP_BKPT_INSN)
+	((type) == DB_TRAP_BKPT_INSN)
 #define IS_WATCHPOINT_TRAP(type, code) \
-	((type) == DB_TRAP_WATCHPOINT)
+	((type) == DB_TRAP_BREAKPOINT || (type) == DB_TRAP_WATCHPOINT)
 
 static inline bool
 inst_return(db_expr_t insn)
@@ -217,9 +217,7 @@ const char *strdisasm(vaddr_t, uint64_t)
 void db_machdep_init(void);
 
 /* hardware breakpoint/watchpoint functions */
-void aarch64_breakpoint_clear(int);
 void aarch64_breakpoint_set(int, vaddr_t);
-void aarch64_watchpoint_clear(int);
 void aarch64_watchpoint_set(int, vaddr_t, int, int);
 #define WATCHPOINT_ACCESS_LOAD		0x01
 #define WATCHPOINT_ACCESS_STORE		0x02

Reply via email to