Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 16 May 2013 09:41:27 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        amd64@freebsd.org
Cc:        current@freebsd.org
Subject:   Fix amd64 ddb hardware watchpoints for SMP
Message-ID:  <20130516064127.GC3047@kib.kiev.ua>

next in thread | raw e-mail | index | archive | help

--8Qjoyxf9cqqzZ33K
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

The ddb use of hardware watchpoints on the x86 architectures is known to
be lacking. There are at least two known problems. One is the improper
interaction with the user-mode debuggers which use debug registers.
Another is that ddb only loads the debug registers for the watchpoint
into the CPU which is executing ddb code, not setting up non-current
processors.

Not touching the first problem for now, I want to fix the second issue,
since as is, hardware watchpoints are useless on SMP. Patch below makes
the stopped processors to load the debug registers after resuming from
the stop handler, if directed by ddb.

Also the patch contains two other commands for ddb which made my
exprerience with debugger on amd64 better. The 'show pginfo[/p] addr'
command dumps the vm_page_t information, either by vm_page address, or,
if the /p modifier is specified, by the physical page address. The 'show
phys2dmap addr' command translates physical address into the directly
mapped address, which can be accessed from ddb then.

diff --git a/sys/amd64/amd64/db_trace.c b/sys/amd64/amd64/db_trace.c
index 2c81f87..39297d9 100644
--- a/sys/amd64/amd64/db_trace.c
+++ b/sys/amd64/amd64/db_trace.c
@@ -33,6 +33,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/systm.h>
 #include <sys/kdb.h>
 #include <sys/proc.h>
+#include <sys/smp.h>
 #include <sys/stack.h>
 #include <sys/sysent.h>
=20
@@ -63,6 +64,8 @@ static db_varfcn_t db_frame;
 static db_varfcn_t db_rsp;
 static db_varfcn_t db_ss;
=20
+CTASSERT(sizeof(struct dbreg) =3D=3D sizeof(((struct pcpu *)NULL)->pc_dbre=
g));
+
 /*
  * Machine register set.
  */
@@ -591,64 +594,82 @@ db_md_set_watchpoint(addr, size)
 	db_expr_t addr;
 	db_expr_t size;
 {
-	struct dbreg d;
-	int avail, i, wsize;
+	struct dbreg *d;
+	struct pcpu *pc;
+	int avail, c, cpu, i, wsize;
=20
-	fill_dbregs(NULL, &d);
+	d =3D (struct dbreg *)PCPU_PTR(dbreg);
+	cpu =3D PCPU_GET(cpuid);
+	fill_dbregs(NULL, d);
=20
 	avail =3D 0;
-	for(i =3D 0; i < 4; i++) {
-		if (!DBREG_DR7_ENABLED(d.dr[7], i))
+	for (i =3D 0; i < 4; i++) {
+		if (!DBREG_DR7_ENABLED(d->dr[7], i))
 			avail++;
 	}
=20
 	if (avail * 8 < size)
 		return (-1);
=20
-	for (i =3D 0; i < 4 && (size > 0); i++) {
-		if (!DBREG_DR7_ENABLED(d.dr[7], i)) {
+	for (i =3D 0; i < 4 && size > 0; i++) {
+		if (!DBREG_DR7_ENABLED(d->dr[7], i)) {
 			if (size >=3D 8 || (avail =3D=3D 1 && size > 4))
 				wsize =3D 8;
 			else if (size > 2)
 				wsize =3D 4;
 			else
 				wsize =3D size;
-			amd64_set_watch(i, addr, wsize,
-				       DBREG_DR7_WRONLY, &d);
+			amd64_set_watch(i, addr, wsize, DBREG_DR7_WRONLY, d);
 			addr +=3D wsize;
 			size -=3D wsize;
 			avail--;
 		}
 	}
=20
-	set_dbregs(NULL, &d);
+	set_dbregs(NULL, d);
+	CPU_FOREACH(c) {
+		if (c =3D=3D cpu)
+			continue;
+		pc =3D pcpu_find(c);
+		memcpy(pc->pc_dbreg, d, sizeof(*d));
+		pc->pc_dbreg_cmd =3D PC_DBREG_CMD_LOAD;
+	}
=20
-	return(0);
+	return (0);
 }
=20
-
 int
 db_md_clr_watchpoint(addr, size)
 	db_expr_t addr;
 	db_expr_t size;
 {
-	struct dbreg d;
-	int i;
+	struct dbreg *d;
+	struct pcpu *pc;
+	int i, c, cpu;
=20
-	fill_dbregs(NULL, &d);
+	d =3D (struct dbreg *)PCPU_PTR(dbreg);
+	cpu =3D PCPU_GET(cpuid);
+	fill_dbregs(NULL, d);
=20
-	for(i =3D 0; i < 4; i++) {
-		if (DBREG_DR7_ENABLED(d.dr[7], i)) {
-			if ((DBREG_DRX((&d), i) >=3D addr) &&
-			    (DBREG_DRX((&d), i) < addr+size))
-				amd64_clr_watch(i, &d);
+	for (i =3D 0; i < 4; i++) {
+		if (DBREG_DR7_ENABLED(d->dr[7], i)) {
+			if (DBREG_DRX((d), i) >=3D addr &&
+			    DBREG_DRX((d), i) < addr + size)
+				amd64_clr_watch(i, d);
=20
 		}
 	}
=20
-	set_dbregs(NULL, &d);
+	set_dbregs(NULL, d);
+	CPU_FOREACH(c) {
+		if (c =3D=3D cpu)
+			continue;
+		pc =3D pcpu_find(c);
+		memcpy(pc->pc_dbreg, d, sizeof(*d));
+		pc->pc_dbreg_cmd =3D PC_DBREG_CMD_LOAD;
+	}
=20
-	return(0);
+	return (0);
 }
=20
=20
@@ -699,3 +720,17 @@ db_md_list_watchpoints()
 	}
 	db_printf("\n");
 }
+
+void
+amd64_db_resume_dbreg(void)
+{
+	struct dbreg *d;
+
+	switch (PCPU_GET(dbreg_cmd)) {
+	case PC_DBREG_CMD_LOAD:
+		d =3D (struct dbreg *)PCPU_PTR(dbreg);
+		set_dbregs(NULL, d);
+		PCPU_SET(dbreg_cmd, PC_DBREG_CMD_NONE);
+		break;
+	}
+}
diff --git a/sys/amd64/amd64/mp_machdep.c b/sys/amd64/amd64/mp_machdep.c
index 31dbb3f..5f4aacf 100644
--- a/sys/amd64/amd64/mp_machdep.c
+++ b/sys/amd64/amd64/mp_machdep.c
@@ -28,6 +28,7 @@
 __FBSDID("$FreeBSD$");
=20
 #include "opt_cpu.h"
+#include "opt_ddb.h"
 #include "opt_kstack_pages.h"
 #include "opt_sched.h"
 #include "opt_smp.h"
@@ -1396,6 +1397,10 @@ cpustop_handler(void)
 	CPU_CLR_ATOMIC(cpu, &started_cpus);
 	CPU_CLR_ATOMIC(cpu, &stopped_cpus);
=20
+#ifdef DDB
+	amd64_db_resume_dbreg();
+#endif
+
 	if (cpu =3D=3D 0 && cpustop_restartfunc !=3D NULL) {
 		cpustop_restartfunc();
 		cpustop_restartfunc =3D NULL;
diff --git a/sys/amd64/include/md_var.h b/sys/amd64/include/md_var.h
index 5d7cb74..6ffcf63 100644
--- a/sys/amd64/include/md_var.h
+++ b/sys/amd64/include/md_var.h
@@ -117,5 +117,6 @@ void	minidumpsys(struct dumperinfo *);
 struct savefpu *get_pcb_user_save_td(struct thread *td);
 struct savefpu *get_pcb_user_save_pcb(struct pcb *pcb);
 struct pcb *get_pcb_td(struct thread *td);
+void	amd64_db_resume_dbreg(void);
=20
 #endif /* !_MACHINE_MD_VAR_H_ */
diff --git a/sys/amd64/include/pcpu.h b/sys/amd64/include/pcpu.h
index bb7d339..ba4c618 100644
--- a/sys/amd64/include/pcpu.h
+++ b/sys/amd64/include/pcpu.h
@@ -78,9 +78,14 @@
 	struct system_segment_descriptor *pc_tss;			\
 	u_int	pc_cmci_mask		/* MCx banks for CMCI */	\
 	PCPU_XEN_FIELDS;						\
-	char	__pad[293]		/* be divisor of PAGE_SIZE	\
+	uint64_t pc_dbreg[16];		/* ddb debugging regs */	\
+	int pc_dbreg_cmd;		/* ddb debugging reg cmd */	\
+	char	__pad[161]		/* be divisor of PAGE_SIZE	\
 					   after cache alignment */
=20
+#define	PC_DBREG_CMD_NONE	0
+#define	PC_DBREG_CMD_LOAD	1
+
 #ifdef _KERNEL
=20
 #ifdef lint
diff --git a/sys/amd64/amd64/pmap.c b/sys/amd64/amd64/pmap.c
index 1b1c86c..ebb184e 100644
--- a/sys/amd64/amd64/pmap.c
+++ b/sys/amd64/amd64/pmap.c
@@ -5535,4 +5535,16 @@ DB_SHOW_COMMAND(pte, pmap_print_pte)
 	pte =3D pmap_pde_to_pte(pde, va);
 	db_printf(" pte %#016lx\n", *pte);
 }
+
+DB_SHOW_COMMAND(phys2dmap, pmap_phys2dmap)
+{
+	vm_paddr_t a;
+
+	if (have_addr) {
+		a =3D (vm_paddr_t)addr;
+		db_printf("0x%jx\n", (uintmax_t)PHYS_TO_DMAP(a));
+	} else {
+		db_printf("show phys2dmap addr\n");
+	}
+}
 #endif
diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c
index 2c381b6..1746e54 100644
--- a/sys/vm/vm_page.c
+++ b/sys/vm/vm_page.c
@@ -2884,4 +2884,27 @@ DB_SHOW_COMMAND(pageq, vm_page_print_pageq_info)
 		*vm_pagequeues[PQ_ACTIVE].pq_cnt,
 		*vm_pagequeues[PQ_INACTIVE].pq_cnt);
 }
+
+DB_SHOW_COMMAND(pginfo, vm_page_print_pginfo)
+{
+	vm_page_t m;
+	boolean_t phys;
+
+	if (!have_addr) {
+		db_printf("show pginfo addr\n");
+		return;
+	}
+
+	phys =3D strchr(modif, 'p') !=3D NULL;
+	if (phys)
+		m =3D PHYS_TO_VM_PAGE(addr);
+	else
+		m =3D (vm_page_t)addr;
+	db_printf(
+    "page %p obj %p pidx 0x%jx phys 0x%jx q %d hold %d wire %d\n"
+    "  af 0x%x of 0x%x f 0x%x act %d busy %d valid 0x%x dirty 0x%x\n",
+	    m, m->object, (uintmax_t)m->pindex, (uintmax_t)m->phys_addr,
+	    m->queue, m->hold_count, m->wire_count, m->aflags, m->oflags,
+	    m->flags, m->act_count, m->busy, m->valid, m->dirty);
+}
 #endif /* DDB */

--8Qjoyxf9cqqzZ33K
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.20 (FreeBSD)

iQIcBAEBAgAGBQJRlH+XAAoJEJDCuSvBvK1BM9YQAJ0kuAtCe1iuJLNaZnbsbQgA
BFsLwjTovYTsIYQP860+fQEazwQCabd42xdrByyB4krRflFWYKkDti3wR2ooV+oH
BfqpIDys2H9aJAepoCt8o8hxDLfERUTdRXwdXFijf1/ILrWxlgIFb4+I8ZPM7/98
2DDVkRRa0yLDq66G7y8f//Rf1WJLza3FsMIFhfSwurb17R5PoUYDrRNIYpWUiphN
jQyW75z7fj1yTl5dFDDFqpzHV51KUYB8F5IMFcsKUXl1hJWNd+ueEv60fSIqcsZS
vu0RIJVE4ReYVtCFwyddtM10oPYiyHU5Zkdzt3A8uK63z0TCGhuodskATY0I7R8t
vNURQmNgHdw8kk5idSCYr4AcbdHGyNSGRuqVloH4WXShsY+PxfvUfKBz2inEEwby
eI3cR8zvHgSS7C2TNI+qMUxIUbnCdZYOn4zNT1/91MhKYkBzPGFQ0QIjnAirZ+vt
km27scr5aU/tam33NmLDA4FQORnIalfYP2ztSsx3358jHhUPw1RXXp16GBf+JZVb
W0EybcbVMkx8Rs/OpbQ8vSto0V2Drtql4GwMnnmvofk2dwEB1rUK25P2KYf7NhSV
yYSwa6HBCbw5on+zdxvfPMPhNbiHGF6bcPZCSOmgVYAEXTPgDWNidM+AvNwv8OrN
/STPz0lbq6R0zW+TWT2m
=pEYk
-----END PGP SIGNATURE-----

--8Qjoyxf9cqqzZ33K--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130516064127.GC3047>