Date: Wed, 14 Sep 2016 12:57:40 +0000 (UTC) From: Bruce Evans <bde@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r305807 - in head/sys: amd64/amd64 i386/i386 x86/include Message-ID: <201609141257.u8ECveAs020350@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bde Date: Wed Sep 14 12:57:40 2016 New Revision: 305807 URL: https://svnweb.freebsd.org/changeset/base/305807 Log: Use the MI macro TRAPF_USERMODE() instead of open-coded checks for SEL_UPL and sometimes PSL_VM. This is just a style change on amd64, but on i386 it fixes 1 unimportant place where the PSL_VM check was missing and starts fixing 1 important place where the PSL_VM check had a logic error. 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. The broken case is only reached if a vm86 bioscall uses a %cs which is nonzero mod 4, but that is unusual -- most bios calls start with %cs = 0xc000 or 0xf000 and rarely change it. Another place was missing the check for PCB_VM86CALL, but was only reachable if there are bugs virtualizing PSL_I. Add a macro TF_HAS_STACKREGS() and use this instead of converting open-coded checks of SEL_UPL, etc. to TRAPF_USERMODE() when we only care about whether the frame has stack registers. This fixes 3 places in my recent fix for register variables in vm86 mode where I messed up the PSL_VM check and cleans up other places. Modified: head/sys/amd64/amd64/trap.c head/sys/i386/i386/db_trace.c head/sys/i386/i386/trap.c head/sys/x86/include/frame.h Modified: head/sys/amd64/amd64/trap.c ============================================================================== --- head/sys/amd64/amd64/trap.c Wed Sep 14 12:07:34 2016 (r305806) +++ head/sys/amd64/amd64/trap.c Wed Sep 14 12:57:40 2016 (r305807) @@ -236,7 +236,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); @@ -260,7 +260,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; @@ -787,7 +787,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,7 +804,7 @@ 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) { + if (TF_HAS_STACKREGS(frame)) { ss = frame->tf_ss & 0xffff; esp = frame->tf_rsp; } else { @@ -934,7 +934,7 @@ amd64_syscall(struct thread *td, int tra ksiginfo_t ksi; #ifdef DIAGNOSTIC - if (ISPL(td->td_frame->tf_cs) != SEL_UPL) { + if (!TRAPF_USERMODE(frame)) { panic("syscall"); /* NOT REACHED */ } Modified: head/sys/i386/i386/db_trace.c ============================================================================== --- head/sys/i386/i386/db_trace.c Wed Sep 14 12:07:34 2016 (r305806) +++ head/sys/i386/i386/db_trace.c Wed Sep 14 12:57:40 2016 (r305807) @@ -82,8 +82,7 @@ struct db_variable *db_eregs = db_regs + static __inline int get_esp(struct trapframe *tf) { - return ((ISPL(tf->tf_cs) || kdb_frame->tf_eflags & PSL_VM) ? - tf->tf_esp : (intptr_t)&tf->tf_esp); + return (TF_HAS_STACKREGS(tf) ? tf->tf_esp : (intptr_t)&tf->tf_esp); } static int @@ -147,7 +146,7 @@ db_esp(struct db_variable *vp, db_expr_t 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); } @@ -180,9 +179,9 @@ db_ss(struct db_variable *vp, db_expr_t return (0); if (op == DB_VAR_GET) - *valuep = (ISPL(kdb_frame->tf_cs) || - kdb_frame->tf_eflags & PSL_VM) ? kdb_frame->tf_ss : rss(); - else if (ISPL(kdb_frame->tf_cs) || kdb_frame->tf_eflags & PSL_VM) + *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); } @@ -439,7 +438,7 @@ db_backtrace(struct thread *td, struct t * 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; Modified: head/sys/i386/i386/trap.c ============================================================================== --- head/sys/i386/i386/trap.c Wed Sep 14 12:07:34 2016 (r305806) +++ head/sys/i386/i386/trap.c Wed Sep 14 12:57:40 2016 (r305807) @@ -267,7 +267,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); @@ -307,9 +308,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; @@ -963,7 +962,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 { @@ -1117,7 +1116,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 */ } Modified: head/sys/x86/include/frame.h ============================================================================== --- head/sys/x86/include/frame.h Wed Sep 14 12:07:34 2016 (r305806) +++ head/sys/x86/include/frame.h Wed Sep 14 12:57:40 2016 (r305807) @@ -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,10 +89,10 @@ 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; @@ -136,6 +136,7 @@ struct trapframe { register_t tf_rip; register_t tf_cs; register_t tf_rflags; + /* below only when crossing rings (user to kernel) */ register_t tf_rsp; register_t tf_ss; }; @@ -145,4 +146,13 @@ struct trapframe { #define TF_HASFPXSTATE 0x4 #endif /* __amd64__ */ +/* + * 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 /* _MACHINE_FRAME_H_ */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201609141257.u8ECveAs020350>