Merge bde improvements for ddb on x86, mostly for single-stepping and

vm86 mode.

MFC r304085 (by bde):
Fix the variables $esp, $ds, $es, $fs, $gs and $ss in vm86 mode.  Fix
PC_REGS() so that printing of instructions works in some useful cases.

MFC r304962 (by bde):
Expand error messages: print symbol names, parentheses and shift tokens,
and negative shift counts.  Fix error messages.

MFC r305612 (by bde):
Fix single-stepping of instructions emulated by vm86.

MFC r305661 (by bde):
Give the full syntax of the 'count' arg for all commmands that support
it. Give the full syntax of the 'addr' arg for these commands and some
others.  Rename it from 'address' for the generic command. Fix
description of how 'count' is supposed to work for the 'break'
command.

Don't (mis)describe the syntax of the comma for the 'step' command.

Expand the description for the generic command.

Give the full syntax for the 'examine' command.  It was also missing
the possible values for the modifier.

MFC r305663 (by bde):
Fix stopping when the specified breakpoint count is reached.

MFC r305665 (by bde):
Pass the trap type and code down from db_trap() to db_stop_at_pc() so
that the latter can easily determine what the trap type actually is
after callers are fixed to encode the type unambigously.

MFC r305807 (by bde):
Use the MI macro TRAPF_USERMODE() instead of open-coded checks for
SEL_UPL and sometimes PSL_VM.  Fix logic errors in treating vm86
bioscall mode as kernel mode.  The main place checked all the
necessary flags, but put the necessary parentheses for the PSL_VM and
PCB_VM86CALL checks in the wrong place.

MFC r305811 (by bz):
Try to fix LINT builds after r305807.

MFC r305840 (by bde):
Abort single stepping in ddb if the trap is not for single-stepping.

MFC r305862 (by bde):
Ifdef the new dr6 variable for KDB.

MFC r305864 (by bde):
Statically initialize the run mode to the one that will become current
on first entry. Don't reset to the run mode to STEP_NONE when
stopping, and remove STEP_NONE.

MFC r305865 (by bde):
Fix decoding of tf_rsp on amd64, and move TF_HAS_STACKREGS() to the
i386-only section, and fix a comment about the amd64 kernel trapframe
not having stackregs.

MFC r305897 (by bde):
Silently ignore unexpected single-step traps.

MFC r306311 (by bde):
Determine the operand/address size of %cs in a new function
db_segsize().  Use db_segsize() to set the default operand/address
size for disassembling.

Fix db_print_loc_and_inst() to ask for the normal format and not the
alternate in normal operation. Use db_segsize() to avoid trying to
print a garbage stack trace if %cs is 16 bits.

14 files changed, 227 insertions, 85 deletions

diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
index 04c5dcc..8d2b399 100644
--- a/sys/amd64/amd64/trap.c
+++ b/sys/amd64/amd64/trap.c
@@ -176,7 +176,10 @@ trap(struct trapframe *frame)
 #endif
 	struct thread *td = curthread;
 	struct proc *p = td->td_proc;
-	int i = 0, ucode = 0, code;
+#ifdef KDB
+	register_t dr6;
+#endif
+	int i = 0, ucode = 0;
 	u_int type;
 	register_t addr = 0;
 	ksiginfo_t ksi;
@@ -236,7 +239,7 @@ trap(struct trapframe *frame)
 		 * interrupts disabled until they are accidentally
 		 * enabled later.
 		 */
-		if (ISPL(frame->tf_cs) == SEL_UPL)
+		if (TRAPF_USERMODE(frame))
 			uprintf(
 			    "pid %ld (%s): trap %d with interrupts disabled\n",
 			    (long)curproc->p_pid, curthread->td_name, type);
@@ -258,9 +261,7 @@ trap(struct trapframe *frame)
 		}
 	}
 
-	code = frame->tf_err;
-
-        if (ISPL(frame->tf_cs) == SEL_UPL) {
+	if (TRAPF_USERMODE(frame)) {
 		/* user trap */
 
 		td->td_pticks = 0;
@@ -377,7 +378,7 @@ trap(struct trapframe *frame)
 #ifdef DEV_ISA
 		case T_NMI:
 			/* machine/parity/power fail/"kitchen sink" faults */
-			if (isa_nmi(code) == 0) {
+			if (isa_nmi(frame->tf_err) == 0) {
 #ifdef KDB
 				/*
 				 * NMI can be hooked up to a pushbutton
@@ -540,8 +541,7 @@ trap(struct trapframe *frame)
 				 * Reset breakpoint bits because the
 				 * processor doesn't
 				 */
-				/* XXX check upper bits here */
-				load_dr6(rdr6() & 0xfffffff0);
+				load_dr6(rdr6() & ~0xf);
 				goto out;
 			}
 			/*
@@ -553,7 +553,10 @@ trap(struct trapframe *frame)
 			 * Otherwise, debugger traps "can't happen".
 			 */
 #ifdef KDB
-			if (kdb_trap(type, 0, frame))
+			/* XXX %dr6 is not quite reentrant. */
+			dr6 = rdr6();
+			load_dr6(dr6 & ~0x4000);
+			if (kdb_trap(type, dr6, frame))
 				goto out;
 #endif
 			break;
@@ -561,7 +564,7 @@ trap(struct trapframe *frame)
 #ifdef DEV_ISA
 		case T_NMI:
 			/* machine/parity/power fail/"kitchen sink" faults */
-			if (isa_nmi(code) == 0) {
+			if (isa_nmi(frame->tf_err) == 0) {
 #ifdef KDB
 				/*
 				 * NMI can be hooked up to a pushbutton
@@ -773,7 +776,6 @@ trap_fatal(frame, eva)
 {
 	int code, ss;
 	u_int type;
-	long esp;
 	struct soft_segment_descriptor softseg;
 	char *msg;
 
@@ -787,7 +789,7 @@ trap_fatal(frame, eva)
 	else
 		msg = "UNKNOWN";
 	printf("\n\nFatal trap %d: %s while in %s mode\n", type, msg,
-	    ISPL(frame->tf_cs) == SEL_UPL ? "user" : "kernel");
+	    TRAPF_USERMODE(frame) ? "user" : "kernel");
 #ifdef SMP
 	/* two separate prints in case of a trap on an unmapped page */
 	printf("cpuid = %d; ", PCPU_GET(cpuid));
@@ -804,14 +806,8 @@ trap_fatal(frame, eva)
 	}
 	printf("instruction pointer	= 0x%lx:0x%lx\n",
 	       frame->tf_cs & 0xffff, frame->tf_rip);
-        if (ISPL(frame->tf_cs) == SEL_UPL) {
-		ss = frame->tf_ss & 0xffff;
-		esp = frame->tf_rsp;
-	} else {
-		ss = GSEL(GDATA_SEL, SEL_KPL);
-		esp = (long)&frame->tf_rsp;
-	}
-	printf("stack pointer	        = 0x%x:0x%lx\n", ss, esp);
+	ss = frame->tf_ss & 0xffff;
+	printf("stack pointer	        = 0x%x:0x%lx\n", ss, frame->tf_rsp);
 	printf("frame pointer	        = 0x%x:0x%lx\n", ss, frame->tf_rbp);
 	printf("code segment		= base 0x%lx, limit 0x%lx, type 0x%x\n",
 	       softseg.ssd_base, softseg.ssd_limit, softseg.ssd_type);
@@ -934,7 +930,7 @@ amd64_syscall(struct thread *td, int traced)
 	ksiginfo_t ksi;
 
 #ifdef DIAGNOSTIC
-	if (ISPL(td->td_frame->tf_cs) != SEL_UPL) {
+	if (!TRAPF_USERMODE(td->td_frame)) {
 		panic("syscall");
 		/* NOT REACHED */
 	}
diff --git a/sys/amd64/include/db_machdep.h b/sys/amd64/include/db_machdep.h
index 29e385e..722a7b3 100644
--- a/sys/amd64/include/db_machdep.h
+++ b/sys/amd64/include/db_machdep.h
@@ -56,12 +56,16 @@ do {						\
 #define	db_clear_single_step	kdb_cpu_clear_singlestep
 #define	db_set_single_step	kdb_cpu_set_singlestep
 
-#define	IS_BREAKPOINT_TRAP(type, code)	((type) == T_BPTFLT)
 /*
- * Watchpoints are not supported.  The debug exception type is in %dr6
- * and not yet in the args to this macro.
+ * The debug exception type is copied from %dr6 to 'code' and used to
+ * disambiguate single step traps.  Watchpoints have no special support.
+ * Our hardware breakpoints are not well integrated with ddb and are too
+ * different from watchpoints.  ddb treats them as unknown traps with
+ * unknown addresses and doesn't turn them off while it is running.
  */
-#define IS_WATCHPOINT_TRAP(type, code)	0
+#define	IS_BREAKPOINT_TRAP(type, code)	((type) == T_BPTFLT)
+#define	IS_SSTEP_TRAP(type, code)	((type) == T_TRCTRAP && (code) & 0x4000)
+#define	IS_WATCHPOINT_TRAP(type, code)	0
 
 #define	I_CALL		0xe8
 #define	I_CALLI		0xff
diff --git a/sys/ddb/db_examine.c b/sys/ddb/db_examine.c
index de2bbe4..a1e5a28 100644
--- a/sys/ddb/db_examine.c
+++ b/sys/ddb/db_examine.c
@@ -241,7 +241,7 @@ db_print_loc_and_inst(db_addr_t loc)
 	db_printsym(loc, DB_STGY_PROC);
 	if (db_search_symbol(loc, DB_STGY_PROC, &off) != C_DB_SYM_NULL) {
 		db_printf(":\t");
-		(void)db_disasm(loc, true);
+		(void)db_disasm(loc, false);
 	}
 }
 
diff --git a/sys/ddb/db_expr.c b/sys/ddb/db_expr.c
index db17f36..c206a57 100644
--- a/sys/ddb/db_expr.c
+++ b/sys/ddb/db_expr.c
@@ -57,7 +57,8 @@ db_term(db_expr_t *valuep)
 	    if (!db_value_of_name(db_tok_string, valuep) &&
 		!db_value_of_name_pcpu(db_tok_string, valuep) &&
 		!db_value_of_name_vnet(db_tok_string, valuep)) {
-		db_error("Symbol not found\n");
+		db_printf("Symbol '%s' not found\n", db_tok_string);
+		db_error(NULL);
 		/*NOTREACHED*/
 	    }
 	    return (true);
@@ -89,12 +90,14 @@ db_term(db_expr_t *valuep)
 	}
 	if (t == tLPAREN) {
 	    if (!db_expression(valuep)) {
-		db_error("Syntax error\n");
+		db_printf("Expression syntax error after '%c'\n", '(');
+		db_error(NULL);
 		/*NOTREACHED*/
 	    }
 	    t = db_read_token();
 	    if (t != tRPAREN) {
-		db_error("Syntax error\n");
+		db_printf("Expression syntax error -- expected '%c'\n", ')');
+		db_error(NULL);
 		/*NOTREACHED*/
 	    }
 	    return (true);
@@ -164,7 +167,9 @@ db_mult_expr(db_expr_t *valuep)
 	while (t == tSTAR || t == tSLASH || t == tPCT || t == tHASH ||
 	    t == tBIT_AND ) {
 	    if (!db_term(&rhs)) {
-		db_printf("Expression syntax error after '%c'\n", '!');
+		db_printf("Expression syntax error after '%c'\n",
+		    t == tSTAR ? '*' : t == tSLASH ? '/' : t == tPCT ? '%' :
+		    t == tHASH ? '#' : '&');
 		db_error(NULL);
 		/*NOTREACHED*/
 	    }
@@ -177,7 +182,7 @@ db_mult_expr(db_expr_t *valuep)
 		    break;
 		default:
 		    if (rhs == 0) {
-			db_error("Divide by 0\n");
+			db_error("Division by 0\n");
 			/*NOTREACHED*/
 		    }
 		    if (t == tSLASH)
@@ -199,7 +204,6 @@ db_add_expr(db_expr_t *valuep)
 {
 	db_expr_t	lhs, rhs;
 	int		t;
-	char		c;
 
 	if (!db_mult_expr(&lhs))
 	    return (false);
@@ -207,8 +211,8 @@ db_add_expr(db_expr_t *valuep)
 	t = db_read_token();
 	while (t == tPLUS || t == tMINUS || t == tBIT_OR) {
 	    if (!db_mult_expr(&rhs)) {
-		c = db_tok_string[0];
-		db_printf("Expression syntax error after '%c'\n", c);
+		db_printf("Expression syntax error after '%c'\n",
+		    t == tPLUS ? '+' : t == tMINUS ? '-' : '|');
 		db_error(NULL);
 		/*NOTREACHED*/
 	    }
@@ -243,11 +247,14 @@ db_shift_expr(db_expr_t *valuep)
 	t = db_read_token();
 	while (t == tSHIFT_L || t == tSHIFT_R) {
 	    if (!db_add_expr(&rhs)) {
-		db_error("Syntax error\n");
+		db_printf("Expression syntax error after '%s'\n",
+		    t == tSHIFT_L ? "<<" : ">>");
+		db_error(NULL);
 		/*NOTREACHED*/
 	    }
 	    if (rhs < 0) {
-		db_error("Negative shift amount\n");
+		db_printf("Negative shift amount %jd\n", (intmax_t)rhs);
+		db_error(NULL);
 		/*NOTREACHED*/
 	    }
 	    if (t == tSHIFT_L)
@@ -269,7 +276,6 @@ db_logical_relation_expr(
 {
 	db_expr_t	lhs, rhs;
 	int		t;
-	char		op[3];
 
 	if (!db_shift_expr(&lhs))
 	    return (false);
@@ -277,11 +283,11 @@ db_logical_relation_expr(
 	t = db_read_token();
 	while (t == tLOG_EQ || t == tLOG_NOT_EQ || t == tGREATER ||
 	    t == tGREATER_EQ || t == tLESS || t == tLESS_EQ) {
-	    op[0] = db_tok_string[0];
-	    op[1] = db_tok_string[1];
-	    op[2] = 0;
 	    if (!db_shift_expr(&rhs)) {
-		db_printf("Expression syntax error after \"%s\"\n", op);
+		db_printf("Expression syntax error after '%s'\n",
+		    t == tLOG_EQ ? "==" : t == tLOG_NOT_EQ ? "!=" :
+		    t == tGREATER ? ">" : t == tGREATER_EQ ? ">=" :
+		    t == tLESS ? "<" : "<=");
 		db_error(NULL);
 		/*NOTREACHED*/
 	    }
diff --git a/sys/ddb/db_main.c b/sys/ddb/db_main.c
index 3345bb6..da6a513 100644
--- a/sys/ddb/db_main.c
+++ b/sys/ddb/db_main.c
@@ -226,10 +226,7 @@ db_trap(int type, int code)
 	if (cnunavailable())
 		return (0);
 
-	bkpt = IS_BREAKPOINT_TRAP(type, code);
-	watchpt = IS_WATCHPOINT_TRAP(type, code);
-
-	if (db_stop_at_pc(&bkpt)) {
+	if (db_stop_at_pc(type, code, &bkpt, &watchpt)) {
 		if (db_inst_count) {
 			db_printf("After %d instructions (%d loads, %d stores),\n",
 			    db_inst_count, db_load_count, db_store_count);
diff --git a/sys/ddb/db_run.c b/sys/ddb/db_run.c
index 5250651..4d8d3c7 100644
--- a/sys/ddb/db_run.c
+++ b/sys/ddb/db_run.c
@@ -48,15 +48,15 @@ __FBSDID("$FreeBSD$");
 #include <ddb/db_break.h>
 #include <ddb/db_access.h>
 
-static int	db_run_mode;
-#define	STEP_NONE	0
 #define	STEP_ONCE	1
 #define	STEP_RETURN	2
 #define	STEP_CALLT	3
 #define	STEP_CONTINUE	4
 #define	STEP_INVISIBLE	5
 #define	STEP_COUNT	6
+static int	db_run_mode = STEP_CONTINUE;
 
+static bool		db_sstep_multiple;
 static bool		db_sstep_print;
 static int		db_loop_count;
 static int		db_call_depth;
@@ -90,13 +90,14 @@ db_pc_is_singlestep(db_addr_t pc)
 #endif
 
 bool
-db_stop_at_pc(bool *is_breakpoint)
+db_stop_at_pc(int type, int code, bool *is_breakpoint, bool *is_watchpoint)
 {
 	db_addr_t	pc;
 	db_breakpoint_t bkpt;
 
+	*is_breakpoint = IS_BREAKPOINT_TRAP(type, code);
+	*is_watchpoint = IS_WATCHPOINT_TRAP(type, code);
 	pc = PC_REGS();
-
 	if (db_pc_is_singlestep(pc))
 		*is_breakpoint = false;
 
@@ -125,13 +126,39 @@ db_stop_at_pc(bool *is_breakpoint)
 		*is_breakpoint = true;
 		return (true);	/* stop here */
 	    }
+	    return (false);	/* continue the countdown */
 	} else if (*is_breakpoint) {
 #ifdef BKPT_SKIP
 		BKPT_SKIP;
 #endif
 	}
 
-	*is_breakpoint = false;
+	*is_breakpoint = false;	/* might be a breakpoint, but not ours */
+
+	/*
+	 * If not stepping, then silently ignore single-step traps
+	 * (except for clearing the single-step-flag above).
+	 *
+	 * If stepping, then abort if the trap type is unexpected.
+	 * Breakpoints owned by us are expected and were handled above.
+	 * Single-steps are expected and are handled below.  All others
+	 * are unexpected.
+	 *
+	 * Only do either of these if the MD layer claims to classify
+	 * single-step traps unambiguously (by defining IS_SSTEP_TRAP).
+	 * Otherwise, fall through to the bad historical behaviour
+	 * given by turning unexpected traps into expected traps: if not
+	 * stepping, then expect only breakpoints and stop, and if
+	 * stepping, then expect only single-steps and step.
+	 */
+#ifdef IS_SSTEP_TRAP
+	if (db_run_mode == STEP_CONTINUE && IS_SSTEP_TRAP(type, code))
+	    return (false);
+	if (db_run_mode != STEP_CONTINUE && !IS_SSTEP_TRAP(type, code)) {
+	    printf("Stepping aborted\n");
+	    return (true);
+	}
+#endif
 
 	if (db_run_mode == STEP_INVISIBLE) {
 	    db_run_mode = STEP_CONTINUE;
@@ -184,7 +211,6 @@ db_stop_at_pc(bool *is_breakpoint)
 		return (false);	/* continue */
 	    }
 	}
-	db_run_mode = STEP_NONE;
 	return (true);
 }
 
@@ -194,6 +220,7 @@ db_restart_at_pc(bool watchpt)
 	db_addr_t	pc = PC_REGS();
 
 	if ((db_run_mode == STEP_COUNT) ||
+	    ((db_run_mode == STEP_ONCE) && db_sstep_multiple) ||
 	    (db_run_mode == STEP_RETURN) ||
 	    (db_run_mode == STEP_CALLT)) {
 	    /*
@@ -321,6 +348,7 @@ db_single_step_cmd(db_expr_t addr, bool have_addr, db_expr_t count, char *modif)
 
 	db_run_mode = STEP_ONCE;
 	db_loop_count = count;
+	db_sstep_multiple = (count != 1);
 	db_sstep_print = print;
 	db_inst_count = 0;
 	db_load_count = 0;
diff --git a/sys/ddb/ddb.h b/sys/ddb/ddb.h
index a2adcdf..fcf772b 100644
--- a/sys/ddb/ddb.h
+++ b/sys/ddb/ddb.h
@@ -215,7 +215,8 @@ void		db_restart_at_pc(bool watchpt);
 int		db_set_variable(db_expr_t value);
 void		db_set_watchpoints(void);
 void		db_skip_to_eol(void);
-bool		db_stop_at_pc(bool *is_breakpoint);
+bool		db_stop_at_pc(int type, int code, bool *is_breakpoint,
+		    bool *is_watchpoint);
 #define		db_strcpy	strcpy
 void		db_trace_self(void);
 int		db_trace_thread(struct thread *, int);
diff --git a/sys/i386/i386/db_disasm.c b/sys/i386/i386/db_disasm.c
index 2b398da..5728fe5 100644
--- a/sys/i386/i386/db_disasm.c
+++ b/sys/i386/i386/db_disasm.c
@@ -31,6 +31,7 @@ __FBSDID("$FreeBSD$");
  * Instruction disassembler.
  */
 #include <sys/param.h>
+#include <sys/kdb.h>
 
 #include <ddb/ddb.h>
 #include <ddb/db_access.h>
@@ -1168,9 +1169,17 @@ db_disasm(db_addr_t loc, bool altfmt)
 	int	len;
 	struct i_addr	address;
 
+	if (db_segsize(kdb_frame) == 16)
+	   altfmt = !altfmt;
 	get_value_inc(inst, loc, 1, FALSE);
-	short_addr = FALSE;
-	size = LONG;
+	if (altfmt) {
+	    short_addr = TRUE;
+	    size = WORD;
+	}
+	else {
+	    short_addr = FALSE;
+	    size = LONG;
+	}
 	seg = NULL;
 
 	/*
diff --git a/sys/i386/i386/db_interface.c b/sys/i386/i386/db_interface.c
index 7079e56..3c0ac81 100644
--- a/sys/i386/i386/db_interface.c
+++ b/sys/i386/i386/db_interface.c
@@ -135,6 +135,30 @@ db_write_bytes(vm_offset_t addr, size_t size, char *data)
 	return (ret);
 }
 
+int
+db_segsize(struct trapframe *tfp)
+{
+	struct proc_ldt *plp;
+	struct segment_descriptor *sdp;
+	int sel;
+
+	if (tfp == NULL)
+	    return (32);
+	if (tfp->tf_eflags & PSL_VM)
+	    return (16);
+	sel = tfp->tf_cs & 0xffff;
+	if (sel == GSEL(GCODE_SEL, SEL_KPL))
+	    return (32);
+	/* Rare cases follow.  User mode cases are currently unreachable. */
+	if (ISLDT(sel)) {
+	    plp = curthread->td_proc->p_md.md_ldt;
+	    sdp = (plp != NULL) ? &plp->ldt_sd : &ldt[0].sd;
+	} else {
+	    sdp = &gdt[PCPU_GET(cpuid) * NGDT].sd;
+	}
+	return (sdp[IDXSEL(sel)].sd_def32 == 0 ? 16 : 32);
+}
+
 void
 db_show_mdpcpu(struct pcpu *pc)
 {
diff --git a/sys/i386/i386/db_trace.c b/sys/i386/i386/db_trace.c
index 3563579..4f5f413 100644
--- a/sys/i386/i386/db_trace.c
+++ b/sys/i386/i386/db_trace.c
@@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/sysent.h>
 
 #include <machine/cpu.h>
+#include <machine/frame.h>
 #include <machine/md_var.h>
 #include <machine/pcb.h>
 #include <machine/reg.h>
@@ -81,8 +82,7 @@ struct db_variable *db_eregs = db_regs + nitems(db_regs);
 static __inline int
 get_esp(struct trapframe *tf)
 {
-	return ((ISPL(tf->tf_cs)) ? tf->tf_esp :
-	    (db_expr_t)tf + (uintptr_t)DB_OFFSET(tf_esp));
+	return (TF_HAS_STACKREGS(tf) ? tf->tf_esp : (intptr_t)&tf->tf_esp);
 }
 
 static int
@@ -104,12 +104,32 @@ db_frame(struct db_variable *vp, db_expr_t *valuep, int op)
 static int
 db_frame_seg(struct db_variable *vp, db_expr_t *valuep, int op)
 {
+	struct trapframe_vm86 *tfp;
+	int off;
 	uint16_t *reg;
 
 	if (kdb_frame == NULL)
 		return (0);
 
-	reg = (uint16_t *)((uintptr_t)kdb_frame + (db_expr_t)vp->valuep);
+	off = (intptr_t)vp->valuep;
+	if (kdb_frame->tf_eflags & PSL_VM) {
+		tfp = (void *)kdb_frame;
+		switch ((intptr_t)vp->valuep) {
+		case (intptr_t)DB_OFFSET(tf_cs):
+			reg = (uint16_t *)&tfp->tf_cs;
+			break;
+		case (intptr_t)DB_OFFSET(tf_ds):
+			reg = (uint16_t *)&tfp->tf_vm86_ds;
+			break;
+		case (intptr_t)DB_OFFSET(tf_es):
+			reg = (uint16_t *)&tfp->tf_vm86_es;
+			break;
+		case (intptr_t)DB_OFFSET(tf_fs):
+			reg = (uint16_t *)&tfp->tf_vm86_fs;
+			break;
+		}
+	} else
+		reg = (uint16_t *)((uintptr_t)kdb_frame + off);
 	if (op == DB_VAR_GET)
 		*valuep = *reg;
 	else
@@ -126,7 +146,7 @@ db_esp(struct db_variable *vp, db_expr_t *valuep, int op)
 
 	if (op == DB_VAR_GET)
 		*valuep = get_esp(kdb_frame);
-	else if (ISPL(kdb_frame->tf_cs))
+	else if (TF_HAS_STACKREGS(kdb_frame))
 		kdb_frame->tf_esp = *valuep;
 	return (1);
 }
@@ -134,7 +154,16 @@ db_esp(struct db_variable *vp, db_expr_t *valuep, int op)
 static int
 db_gs(struct db_variable *vp, db_expr_t *valuep, int op)
 {
+	struct trapframe_vm86 *tfp;
 
+	if (kdb_frame != NULL && kdb_frame->tf_eflags & PSL_VM) {
+		tfp = (void *)kdb_frame;
+		if (op == DB_VAR_GET)
+			*valuep = tfp->tf_vm86_gs;
+		else
+			tfp->tf_vm86_gs = *valuep;
+		return (1);
+	}
 	if (op == DB_VAR_GET)
 		*valuep = rgs();
 	else
@@ -150,8 +179,9 @@ db_ss(struct db_variable *vp, db_expr_t *valuep, int op)
 		return (0);
 
 	if (op == DB_VAR_GET)
-		*valuep = (ISPL(kdb_frame->tf_cs)) ? kdb_frame->tf_ss : rss();
-	else if (ISPL(kdb_frame->tf_cs))
+		*valuep = TF_HAS_STACKREGS(kdb_frame) ? kdb_frame->tf_ss :
+		    rss();
+	else if (TF_HAS_STACKREGS(kdb_frame))
 		kdb_frame->tf_ss = *valuep;
 	return (1);
 }
@@ -391,6 +421,17 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct i386_frame *frame,
 	int instr, narg;
 	boolean_t first;
 
+	if (db_segsize(tf) == 16) {
+		db_printf(
+"--- 16-bit%s, cs:eip = %#x:%#x, ss:esp = %#x:%#x, ebp = %#x, tf = %p ---\n",
+		    (tf->tf_eflags & PSL_VM) ? " (vm86)" : "",
+		    tf->tf_cs, tf->tf_eip,
+		    TF_HAS_STACKREGS(tf) ? tf->tf_ss : rss(),
+		    TF_HAS_STACKREGS(tf) ? tf->tf_esp : (intptr_t)&tf->tf_esp,
+		    tf->tf_ebp, tf);
+		return (0);
+	}
+
 	/*
 	 * If an indirect call via an invalid pointer caused a trap,
 	 * %pc contains the invalid address while the return address
@@ -408,7 +449,7 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct i386_frame *frame,
 		 * Find where the trap frame actually ends.
 		 * It won't contain tf_esp or tf_ss unless crossing rings.
 		 */
-		if (ISPL(kdb_frame->tf_cs))
+		if (TF_HAS_STACKREGS(kdb_frame))
 			instr = (int)(kdb_frame + 1);
 		else
 			instr = (int)&kdb_frame->tf_esp;
diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c
index c540a49..3decfd43 100644
--- a/sys/i386/i386/trap.c
+++ b/sys/i386/i386/trap.c
@@ -189,7 +189,10 @@ trap(struct trapframe *frame)
 #endif
 	struct thread *td = curthread;
 	struct proc *p = td->td_proc;
-	int i = 0, ucode = 0, code;
+#ifdef KDB
+	register_t dr6;
+#endif
+	int i = 0, ucode = 0;
 	u_int type;
 	register_t addr = 0;
 	vm_offset_t eva;
@@ -267,7 +270,8 @@ trap(struct trapframe *frame)
 		 * interrupts disabled until they are accidentally
 		 * enabled later.
 		 */
-		if (ISPL(frame->tf_cs) == SEL_UPL || (frame->tf_eflags & PSL_VM))
+		if (TRAPF_USERMODE(frame) &&
+		    (curpcb->pcb_flags & PCB_VM86CALL) == 0)
 			uprintf(
 			    "pid %ld (%s): trap %d with interrupts disabled\n",
 			    (long)curproc->p_pid, curthread->td_name, type);
@@ -291,7 +295,6 @@ trap(struct trapframe *frame)
 		}
 	}
 	eva = 0;
-	code = frame->tf_err;
 	if (type == T_PAGEFLT) {
 		/*
 		 * For some Cyrix CPUs, %cr2 is clobbered by
@@ -307,9 +310,7 @@ trap(struct trapframe *frame)
 			enable_intr();
 	}
 
-        if ((ISPL(frame->tf_cs) == SEL_UPL) ||
-	    ((frame->tf_eflags & PSL_VM) && 
-		!(curpcb->pcb_flags & PCB_VM86CALL))) {
+        if (TRAPF_USERMODE(frame) && (curpcb->pcb_flags & PCB_VM86CALL) == 0) {
 		/* user trap */
 
 		td->td_pticks = 0;
@@ -335,6 +336,7 @@ trap(struct trapframe *frame)
 					goto out;
 			}
 #endif
+user_trctrap_out:
 			frame->tf_eflags &= ~PSL_T;
 			i = SIGTRAP;
 			ucode = (type == T_TRCTRAP ? TRAP_TRACE : TRAP_BRKPT);
@@ -360,6 +362,11 @@ trap(struct trapframe *frame)
 		case T_STKFLT:		/* stack fault */
 			if (frame->tf_eflags & PSL_VM) {
 				i = vm86_emulate((struct vm86frame *)frame);
+				if (i == SIGTRAP) {
+					type = T_TRCTRAP;
+					load_dr6(rdr6() | 0x4000);
+					goto user_trctrap_out;
+				}
 				if (i == 0)
 					goto user;
 				break;
@@ -461,7 +468,7 @@ trap(struct trapframe *frame)
 			goto userout;
 #else /* !POWERFAIL_NMI */
 			/* machine/parity/power fail/"kitchen sink" faults */
-			if (isa_nmi(code) == 0) {
+			if (isa_nmi(frame->tf_err) == 0) {
 #ifdef KDB
 				/*
 				 * NMI can be hooked up to a pushbutton
@@ -566,6 +573,11 @@ trap(struct trapframe *frame)
 		case T_STKFLT:		/* stack fault */
 			if (frame->tf_eflags & PSL_VM) {
 				i = vm86_emulate((struct vm86frame *)frame);
+				if (i == SIGTRAP) {
+					type = T_TRCTRAP;
+					load_dr6(rdr6() | 0x4000);
+					goto kernel_trctrap;
+				}
 				if (i != 0)
 					/*
 					 * returns to original process
@@ -654,6 +666,7 @@ trap(struct trapframe *frame)
 			break;
 
 		case T_TRCTRAP:	 /* trace trap */
+kernel_trctrap:
 			if (frame->tf_eip == (int)IDTVEC(lcall_syscall)) {
 				/*
 				 * We've just entered system mode via the
@@ -687,7 +700,7 @@ trap(struct trapframe *frame)
 				 * Reset breakpoint bits because the
 				 * processor doesn't
 				 */
-				load_dr6(rdr6() & 0xfffffff0);
+				load_dr6(rdr6() & ~0xf);
 				goto out;
 			}
 			/*
@@ -699,7 +712,10 @@ trap(struct trapframe *frame)
 			 * Otherwise, debugger traps "can't happen".
 			 */
 #ifdef KDB
-			if (kdb_trap(type, 0, frame))
+			/* XXX %dr6 is not quite reentrant. */
+			dr6 = rdr6();
+			load_dr6(dr6 & ~0x4000);
+			if (kdb_trap(type, dr6, frame))
 				goto out;
 #endif
 			break;
@@ -715,7 +731,7 @@ trap(struct trapframe *frame)
 			goto out;
 #else /* !POWERFAIL_NMI */
 			/* machine/parity/power fail/"kitchen sink" faults */
-			if (isa_nmi(code) == 0) {
+			if (isa_nmi(frame->tf_err) == 0) {
 #ifdef KDB
 				/*
 				 * NMI can be hooked up to a pushbutton
@@ -953,7 +969,7 @@ trap_fatal(frame, eva)
 	}
 	printf("instruction pointer	= 0x%x:0x%x\n",
 	       frame->tf_cs & 0xffff, frame->tf_eip);
-        if ((ISPL(frame->tf_cs) == SEL_UPL) || (frame->tf_eflags & PSL_VM)) {
+        if (TF_HAS_STACKREGS(frame)) {
 		ss = frame->tf_ss & 0xffff;
 		esp = frame->tf_esp;
 	} else {
@@ -1107,7 +1123,8 @@ syscall(struct trapframe *frame)
 	ksiginfo_t ksi;
 
 #ifdef DIAGNOSTIC
-	if (ISPL(frame->tf_cs) != SEL_UPL) {
+	if (!(TRAPF_USERMODE(frame) &&
+	    (curpcb->pcb_flags & PCB_VM86CALL) == 0)) {
 		panic("syscall");
 		/* NOT REACHED */
 	}
diff --git a/sys/i386/i386/vm86.c b/sys/i386/i386/vm86.c
index 93ff855..baa1912 100644
--- a/sys/i386/i386/vm86.c
+++ b/sys/i386/i386/vm86.c
@@ -171,7 +171,7 @@ vm86_emulate(vmf)
 					PUSHL((vmf->vmf_eflags & PUSH_MASK)
 					    | PSL_IOPL, vmf);
 				vmf->vmf_ip += inc_ip;
-				return (0);
+				return (retcode);
 
 			case POPF:
 				temp_flags = POPL(vmf) & POP_MASK;
@@ -185,7 +185,7 @@ vm86_emulate(vmf)
 				} else {
 					vmf->vmf_eflags &= ~PSL_VIF;
 				}
-				return (0);
+				return (retcode);
 			}
 			break;
 
@@ -203,7 +203,7 @@ vm86_emulate(vmf)
 		case INTn:
 			break;
 
-		/* VME if trying to set PSL_TF, or PSL_I when VIP is set */
+		/* VME if trying to set PSL_T, or PSL_I when VIP is set */
 		case POPF:
 			temp_flags = POP(vmf) & POP_MASK;
 			vmf->vmf_flags = (vmf->vmf_flags & ~POP_MASK)
@@ -218,7 +218,7 @@ vm86_emulate(vmf)
 			}
 			return (retcode);
 
-		/* VME if trying to set PSL_TF, or PSL_I when VIP is set */
+		/* VME if trying to set PSL_T, or PSL_I when VIP is set */
 		case IRET:
 			vmf->vmf_ip = POP(vmf);
 			vmf->vmf_cs = POP(vmf);
diff --git a/sys/i386/include/db_machdep.h b/sys/i386/include/db_machdep.h
index 27207a8..e8d1d8e 100644
--- a/sys/i386/include/db_machdep.h
+++ b/sys/i386/include/db_machdep.h
@@ -35,7 +35,10 @@
 typedef	vm_offset_t	db_addr_t;	/* address - unsigned */
 typedef	int		db_expr_t;	/* expression - signed */
 
-#define	PC_REGS()	((db_addr_t)kdb_thrctx->pcb_eip)
+#define	PC_REGS()	((db_addr_t)(kdb_frame->tf_eflags & PSL_VM ?	\
+			    (kdb_frame->tf_eip & 0xffff) +		\
+			    ((kdb_frame->tf_cs & 0xffff) << 4) :	\
+			    kdb_frame->tf_eip))
 
 #define	BKPT_INST	0xcc		/* breakpoint instruction */
 #define	BKPT_SIZE	(1)		/* size of breakpoint inst */
@@ -56,12 +59,16 @@ do {						\
 #define	db_clear_single_step	kdb_cpu_clear_singlestep
 #define	db_set_single_step	kdb_cpu_set_singlestep
 
-#define	IS_BREAKPOINT_TRAP(type, code)	((type) == T_BPTFLT)
 /*
- * Watchpoints are not supported.  The debug exception type is in %dr6
- * and not yet in the args to this macro.
+ * The debug exception type is copied from %dr6 to 'code' and used to
+ * disambiguate single step traps.  Watchpoints have no special support.
+ * Our hardware breakpoints are not well integrated with ddb and are too
+ * different from watchpoints.  ddb treats them as unknown traps with
+ * unknown addresses and doesn't turn them off while it is running.
  */
-#define IS_WATCHPOINT_TRAP(type, code)	0
+#define	IS_BREAKPOINT_TRAP(type, code)	((type) == T_BPTFLT)
+#define	IS_SSTEP_TRAP(type, code)	((type) == T_TRCTRAP && (code) & 0x4000)
+#define	IS_WATCHPOINT_TRAP(type, code)	0
 
 #define	I_CALL		0xe8
 #define	I_CALLI		0xff
@@ -91,4 +98,6 @@ do {						\
 #define	DB_SMALL_VALUE_MAX	0x7fffffff
 #define	DB_SMALL_VALUE_MIN	(-0x400001)
 
+int	db_segsize(struct trapframe *tfp);
+
 #endif /* !_MACHINE_DB_MACHDEP_H_ */
diff --git a/sys/x86/include/frame.h b/sys/x86/include/frame.h
index a32ef7c..7418736 100644
--- a/sys/x86/include/frame.h
+++ b/sys/x86/include/frame.h
@@ -64,7 +64,7 @@ struct trapframe {
 	int	tf_eip;
 	int	tf_cs;
 	int	tf_eflags;
-	/* below only when crossing rings (e.g. user to kernel) */
+	/* below only when crossing rings (user to kernel) */
 	int	tf_esp;
 	int	tf_ss;
 };
@@ -89,15 +89,24 @@ struct trapframe_vm86 {
 	int	tf_eip;
 	int	tf_cs;
 	int	tf_eflags;
-	/* below only when crossing rings (e.g. user to kernel) */
+	/* below only when crossing rings (user (including vm86) to kernel) */
 	int	tf_esp;
 	int	tf_ss;
-	/* below only when switching out of VM86 mode */
+	/* below only when crossing from vm86 mode to kernel */
 	int	tf_vm86_es;
 	int	tf_vm86_ds;
 	int	tf_vm86_fs;
 	int	tf_vm86_gs;
 };
+
+/*
+ * This alias for the MI TRAPF_USERMODE() should be used when we don't
+ * care about user mode itself, but need to know if a frame has stack
+ * registers.  The difference is only logical, but on i386 the logic
+ * for using TRAPF_USERMODE() is complicated by sometimes treating vm86
+ * bioscall mode (which is a special ring 3 user mode) as kernel mode.
+ */
+#define	TF_HAS_STACKREGS(tf)	TRAPF_USERMODE(tf)
 #endif /* __i386__ */
 
 #ifdef __amd64__
@@ -136,6 +145,7 @@ struct trapframe {
 	register_t	tf_rip;
 	register_t	tf_cs;
 	register_t	tf_rflags;
+	/* the amd64 frame always has the stack registers */
 	register_t	tf_rsp;
 	register_t	tf_ss;
 };