- Add macro constants for the various fields in %dr7 and use them in place

  of various scattered magic values.
- Pretty print the address of hardware watchpoints in 'show watch' rather
  than just displaying hex.
- Expand address field width on amd64 for 64-bit pointers.

3 files changed, 54 insertions, 38 deletions

diff --git a/sys/amd64/amd64/db_trace.c b/sys/amd64/amd64/db_trace.c
index f6b033e..bd737dd 100644
--- a/sys/amd64/amd64/db_trace.c
+++ b/sys/amd64/amd64/db_trace.c
@@ -538,12 +538,11 @@ amd64_set_watch(watchnum, watchaddr, size, access, d)
 	int access;
 	struct dbreg *d;
 {
-	int i;
-	unsigned int mask;
+	int i, len;
 
 	if (watchnum == -1) {
-		for (i = 0, mask = 0x3; i < 4; i++, mask <<= 2)
-			if ((d->dr[7] & mask) == 0)
+		for (i = 0; i < 4; i++)
+			if (!DBREG_DR7_ENABLED(d->dr[7], i))
 				break;
 		if (i < 4)
 			watchnum = i;
@@ -566,22 +565,28 @@ amd64_set_watch(watchnum, watchaddr, size, access, d)
 	 * we can watch a 1, 2, or 4 byte sized location
 	 */
 	switch (size) {
-	case 1	: mask = 0x00; break;
-	case 2	: mask = 0x01 << 2; break;
-	case 4	: mask = 0x03 << 2; break;
-	default : return (-1);
+	case 1:
+		len = DBREG_DR7_LEN_1;
+		break;
+	case 2:
+		len = DBREG_DR7_LEN_2;
+		break;
+	case 4:
+		len = DBREG_DR7_LEN_4;
+		break;
+	default:
+		return (-1);
 	}
 
-	mask |= access;
-
 	/* clear the bits we are about to affect */
-	d->dr[7] &= ~((0x3 << (watchnum*2)) | (0x0f << (watchnum*4+16)));
+	d->dr[7] &= ~DBREG_DR7_MASK(watchnum);
 
 	/* set drN register to the address, N=watchnum */
 	DBREG_DRX(d, watchnum) = watchaddr;
 
 	/* enable the watchpoint */
-	d->dr[7] |= (0x2 << (watchnum*2)) | (mask << (watchnum*4+16));
+	d->dr[7] |= DBREG_DR7_SET(watchnum, len, access,
+	    DBREG_DR7_GLOBAL_ENABLE);
 
 	return (watchnum);
 }
@@ -596,7 +601,7 @@ amd64_clr_watch(watchnum, d)
 	if (watchnum < 0 || watchnum >= 4)
 		return (-1);
 
-	d->dr[7] = d->dr[7] & ~((0x3 << (watchnum*2)) | (0x0f << (watchnum*4+16)));
+	d->dr[7] &= ~DBREG_DR7_MASK(watchnum);
 	DBREG_DRX(d, watchnum) = 0;
 
 	return (0);
@@ -615,21 +620,19 @@ db_md_set_watchpoint(addr, size)
 
 	avail = 0;
 	for(i = 0; i < 4; i++) {
-		if ((d.dr[7] & (3 << (i*2))) == 0)
+		if (!DBREG_DR7_ENABLED(d.dr[7], i))
 			avail++;
 	}
 
 	if (avail * 4 < size)
 		return (-1);
 
-	for (i = 0; i < 4 && (size != 0); i++) {
-		if ((d.dr[7] & (3<<(i*2))) == 0) {
-			if (size > 4)
+	for (i = 0; i < 4 && (size > 0); i++) {
+		if (!DBREG_DR7_ENABLED(d.dr[7], i)) {
+			if (size > 2)
 				wsize = 4;
 			else
 				wsize = size;
-			if (wsize == 3)
-				wsize++;
 			amd64_set_watch(i, addr, wsize,
 				       DBREG_DR7_WRONLY, &d);
 			addr += wsize;
@@ -654,7 +657,7 @@ db_md_clr_watchpoint(addr, size)
 	fill_dbregs(NULL, &d);
 
 	for(i = 0; i < 4; i++) {
-		if (d.dr[7] & (3 << (i*2))) {
+		if (DBREG_DR7_ENABLED(d.dr[7], i)) {
 			if ((DBREG_DRX((&d), i) >= addr) &&
 			    (DBREG_DRX((&d), i) < addr+size))
 				amd64_clr_watch(i, &d);
@@ -690,17 +693,17 @@ db_md_list_watchpoints()
 	fill_dbregs(NULL, &d);
 
 	db_printf("\nhardware watchpoints:\n");
-	db_printf("  watch    status        type  len     address\n");
-	db_printf("  -----  --------  ----------  ---  ----------\n");
+	db_printf("  watch    status        type  len             address\n");
+	db_printf("  -----  --------  ----------  ---  ------------------\n");
 	for (i = 0; i < 4; i++) {
-		if (d.dr[7] & (0x03 << (i*2))) {
-			type = (d.dr[7] >> (16+(i*4))) & 3;
-			len =  (d.dr[7] >> (16+(i*4)+2)) & 3;
-			db_printf("  %-5d  %-8s  %10s  %3d  0x%016lx\n",
-				  i, "enabled", watchtype_str(type),
-				  len + 1, DBREG_DRX((&d), i));
-		}
-		else {
+		if (DBREG_DR7_ENABLED(d.dr[7], i)) {
+			type = DBREG_DR7_ACCESS(d.dr[7], i);
+			len = DBREG_DR7_LEN(d.dr[7], i);
+			db_printf("  %-5d  %-8s  %10s  %3d  ",
+			    i, "enabled", watchtype_str(type), len + 1);
+			db_printsym((db_addr_t)DBREG_DRX((&d), i), DB_STGY_ANY);
+			db_printf("\n");
+		} else {
 			db_printf("  %-5d  disabled\n", i);
 		}
 	}
diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c
index 7804db8..6c98010 100644
--- a/sys/amd64/amd64/machdep.c
+++ b/sys/amd64/amd64/machdep.c
@@ -1692,7 +1692,6 @@ set_dbregs(struct thread *td, struct dbreg *dbregs)
 {
 	struct pcb *pcb;
 	int i;
-	u_int64_t mask1, mask2;
 
 	if (td == NULL) {
 		load_dr0(dbregs->dr[0]);
@@ -1709,10 +1708,12 @@ set_dbregs(struct thread *td, struct dbreg *dbregs)
 		 * TRCTRAP or a general protection fault right here.
 		 * Upper bits of dr6 and dr7 must not be set
 		 */
-		for (i = 0, mask1 = 0x3<<16, mask2 = 0x2<<16; i < 8;
-		     i++, mask1 <<= 2, mask2 <<= 2)
-			if ((dbregs->dr[7] & mask1) == mask2)
+		for (i = 0; i < 4; i++) {
+			if (DBREG_DR7_ACCESS(dbregs->dr[7], i) == 0x02)
 				return (EINVAL);
+			if (DBREG_DR7_LEN(dbregs->dr[7], i) == 0x02)
+				return (EINVAL);
+		}
 		if ((dbregs->dr[6] & 0xffffffff00000000ul) != 0 ||
 		    (dbregs->dr[7] & 0xffffffff00000000ul) != 0)
 			return (EINVAL);
@@ -1733,22 +1734,22 @@ set_dbregs(struct thread *td, struct dbreg *dbregs)
 		 * from within kernel mode?
 		 */
 
-		if (dbregs->dr[7] & 0x3) {
+		if (DBREG_DR7_ENABLED(dbregs->dr[7], 0)) {
 			/* dr0 is enabled */
 			if (dbregs->dr[0] >= VM_MAXUSER_ADDRESS)
 				return (EINVAL);
 		}
-		if (dbregs->dr[7] & 0x3<<2) {
+		if (DBREG_DR7_ENABLED(dbregs->dr[7], 1)) {
 			/* dr1 is enabled */
 			if (dbregs->dr[1] >= VM_MAXUSER_ADDRESS)
 				return (EINVAL);
 		}
-		if (dbregs->dr[7] & 0x3<<4) {
+		if (DBREG_DR7_ENABLED(dbregs->dr[7], 2)) {
 			/* dr2 is enabled */
 			if (dbregs->dr[2] >= VM_MAXUSER_ADDRESS)
 				return (EINVAL);
 		}
-		if (dbregs->dr[7] & 0x3<<6) {
+		if (DBREG_DR7_ENABLED(dbregs->dr[7], 3)) {
 			/* dr3 is enabled */
 			if (dbregs->dr[3] >= VM_MAXUSER_ADDRESS)
 				return (EINVAL);
diff --git a/sys/amd64/include/reg.h b/sys/amd64/include/reg.h
index 6772930..9358f05 100644
--- a/sys/amd64/include/reg.h
+++ b/sys/amd64/include/reg.h
@@ -92,9 +92,21 @@ struct dbreg {
 				/* Index 8-15: reserved */
 };
 
+#define	DBREG_DR7_LOCAL_ENABLE	0x01
+#define	DBREG_DR7_GLOBAL_ENABLE	0x02
+#define	DBREG_DR7_LEN_1		0x00	/* 1 byte length          */
+#define	DBREG_DR7_LEN_2		0x01
+#define	DBREG_DR7_LEN_4		0x03
 #define	DBREG_DR7_EXEC		0x00	/* break on execute       */
 #define	DBREG_DR7_WRONLY	0x01	/* break on write         */
 #define	DBREG_DR7_RDWR		0x03	/* break on read or write */
+#define	DBREG_DR7_MASK(i)	((u_long)0xf << ((i) * 4 + 16) | 0x3 << (i) * 2)
+#define	DBREG_DR7_SET(i, len, access, enable)				\
+	((u_long)((len) << 2 | (access)) << ((i) * 4 + 16) | (enable) << (i) * 2)
+#define	DBREG_DR7_GD		0x2000
+#define	DBREG_DR7_ENABLED(d, i)	(((d) & 0x3 << (i) * 2) != 0)
+#define	DBREG_DR7_ACCESS(d, i)	((d) >> ((i) * 4 + 16) & 0x3)
+#define	DBREG_DR7_LEN(d, i)	((d) >> ((i) * 4 + 18) & 0x3)
 
 #define	DBREG_DRX(d,x)	((d)->dr[(x)])	/* reference dr0 - dr15 by
 					   register number */