Page MenuHomeFreeBSD
Differential D16469

fix TRAPF_USERMODE macro
ClosedPublic
Actions

Authored by br on Jul 27 2018, 3:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 18, 2:50 AM
Unknown Object (File)
Thu, Apr 17, 10:51 PM
Unknown Object (File)
Mon, Apr 14, 10:55 AM
Unknown Object (File)
Mon, Apr 14, 5:04 AM
Unknown Object (File)
Sun, Apr 13, 11:25 PM
Unknown Object (File)
Sun, Apr 13, 9:23 PM
Unknown Object (File)
Sat, Apr 12, 7:05 PM
Unknown Object (File)
Sun, Apr 6, 7:12 PM
View All 28 Files
Subscribers
imp

Details

Reviewers
markj
Commits
rS336766: Use SPP (Supervisor Previous Privilege) bit in the sstatus
Summary

Use SPP (supervisor previous privilege) bit in sstatus register to determine if trap is from userspace.

Otherwise if we jump to kernel address from userspace, then TRAPF_USERMODE failed to detect usermode and then do_ast triggers a panic "ast in kernel mode"

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

br created this revision.Jul 27 2018, 3:04 PM
markj accepted this revision.Jul 27 2018, 3:50 PM
This revision is now accepted and ready to land.Jul 27 2018, 3:50 PM
Closed by commit rS336766: Use SPP (Supervisor Previous Privilege) bit in the sstatus (authored by br). · Explain WhyJul 27 2018, 4:13 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptJul 27 2018, 4:13 PM

Revision Contents
Changeset List

PathSize
head/
sys/
riscv/
include/
4 lines

Diff 45912

View Options

head/sys/riscv/include/cpu.h

Show First 20 Lines • Show All 49 Lines • ▼ Show 20 Lines
static int vfp_bounce(u_int, u_int, struct trapframe *, int); static int vfp_bounce(u_int, u_int, struct trapframe *, int);
static void vfp_restore(struct vfp_state *); static void vfp_restore(struct vfp_state *);
extern int vfp_exists; extern int vfp_exists;
static struct undefined_handler vfp10_uh, vfp11_uh; static struct undefined_handler vfp10_uh, vfp11_uh;
/* If true the VFP unit has 32 double registers, otherwise it has 16 */ /* If true the VFP unit has 32 double registers, otherwise it has 16 */
static int is_d32; static int is_d32;
struct fpu_kern_ctx {
struct vfp_state *prev;
#define FPU_KERN_CTX_DUMMY 0x01 /* avoided save for the kern thread */
#define FPU_KERN_CTX_INUSE 0x02
uint32_t flags;
struct vfp_state state;
};
/* /*
* About .fpu directives in this file... * About .fpu directives in this file...
* *
* We should need simply .fpu vfpv3, but clang 3.5 has a quirk where setting * We should need simply .fpu vfpv3, but clang 3.5 has a quirk where setting
* vfpv3 doesn't imply that vfp2 features are also available -- both have to be * vfpv3 doesn't imply that vfp2 features are also available -- both have to be
* explicitly set to get all the features of both. This is probably a bug in * explicitly set to get all the features of both. This is probably a bug in
* clang, so it may get fixed and require changes here some day. Other changes * clang, so it may get fixed and require changes here some day. Other changes
* are probably coming in clang too, because there is email and open PRs * are probably coming in clang too, because there is email and open PRs
Show All 29 Lines
static void static void
set_coprocessorACR(u_int val) set_coprocessorACR(u_int val)
{ {
__asm __volatile("mcr p15, 0, %0, c1, c0, 2\n\t" __asm __volatile("mcr p15, 0, %0, c1, c0, 2\n\t"
: : "r" (val) : "cc"); : : "r" (val) : "cc");
isb(); isb();
} }
static void
vfp_enable(void)
{
uint32_t fpexc;
fpexc = fmrx(fpexc);
fmxr(fpexc, fpexc | VFPEXC_EN);
isb();
}
static void
vfp_disable(void)
{
uint32_t fpexc;
fpexc = fmrx(fpexc);
fmxr(fpexc, fpexc & ~VFPEXC_EN);
isb();
}
/* called for each cpu */ /* called for each cpu */
void void
vfp_init(void) vfp_init(void)
{ {
u_int fpsid, tmp; u_int fpsid, tmp;
u_int coproc, vfp_arch; u_int coproc, vfp_arch;
coproc = get_coprocessorACR(); coproc = get_coprocessorACR();
▲ Show 20 Lines • Show All 107 Lines • ▼ Show 20 Linesvfp_bounce(u_int addr, u_int insn, struct trapframe *frame, int code)
* If the last time this thread used the VFP it was on this core, and * If the last time this thread used the VFP it was on this core, and
* the last thread to use the VFP on this core was this thread, then the * the last thread to use the VFP on this core was this thread, then the
* VFP state is valid, otherwise restore this thread's state to the VFP. * VFP state is valid, otherwise restore this thread's state to the VFP.
*/ */
fmxr(fpexc, fpexc | VFPEXC_EN); fmxr(fpexc, fpexc | VFPEXC_EN);
curpcb = curthread->td_pcb; curpcb = curthread->td_pcb;
cpu = PCPU_GET(cpuid); cpu = PCPU_GET(cpuid);
if (curpcb->pcb_vfpcpu != cpu || curthread != PCPU_GET(fpcurthread)) { if (curpcb->pcb_vfpcpu != cpu || curthread != PCPU_GET(fpcurthread)) {
vfp_restore(&curpcb->pcb_vfpstate); if (curpcb->pcb_vfpsaved == NULL)
curpcb->pcb_vfpsaved = &curpcb->pcb_vfpstate;
vfp_restore(curpcb->pcb_vfpsaved);
curpcb->pcb_vfpcpu = cpu; curpcb->pcb_vfpcpu = cpu;
PCPU_SET(fpcurthread, curthread); PCPU_SET(fpcurthread, curthread);
} }
critical_exit(); critical_exit();
return (0); return (0);
} }
▲ Show 20 Lines • Show All 78 Lines • ▼ Show 20 Linesvfp_discard(struct thread *td)
u_int tmp; u_int tmp;
if (PCPU_GET(fpcurthread) == td) if (PCPU_GET(fpcurthread) == td)
PCPU_SET(fpcurthread, NULL); PCPU_SET(fpcurthread, NULL);
tmp = fmrx(fpexc); tmp = fmrx(fpexc);
if (tmp & VFPEXC_EN) if (tmp & VFPEXC_EN)
fmxr(fpexc, tmp & ~VFPEXC_EN); fmxr(fpexc, tmp & ~VFPEXC_EN);
}
void
vfp_save_state(struct thread *td, struct pcb *pcb)
{
int32_t fpexc;
KASSERT(pcb != NULL, ("NULL vfp pcb"));
KASSERT(td == NULL || td->td_pcb == pcb, ("Invalid vfp pcb"));
/*
* savectx() will be called on panic with dumppcb as an argument,
* dumppcb doesn't have pcb_vfpsaved set, so set it to save
* the VFP registers.
*/
if (pcb->pcb_vfpsaved == NULL)
pcb->pcb_vfpsaved = &pcb->pcb_vfpstate;
if (td == NULL)
td = curthread;
critical_enter();
/*
* Only store the registers if the VFP is enabled,
* i.e. return if we are trapping on FP access.
*/
fpexc = fmrx(fpexc);
if (fpexc & VFPEXC_EN) {
KASSERT(PCPU_GET(fpcurthread) == td,
("Storing an invalid VFP state"));
vfp_store(pcb->pcb_vfpsaved, true);
}
critical_exit();
}
void
fpu_kern_enter(struct thread *td, struct fpu_kern_ctx *ctx, u_int flags)
{
struct pcb *pcb;
pcb = td->td_pcb;
KASSERT((flags & FPU_KERN_NOCTX) != 0 || ctx != NULL,
("ctx is required when !FPU_KERN_NOCTX"));
KASSERT(ctx == NULL || (ctx->flags & FPU_KERN_CTX_INUSE) == 0,
("using inuse ctx"));
KASSERT((pcb->pcb_fpflags & PCB_FP_NOSAVE) == 0,
("recursive fpu_kern_enter while in PCB_FP_NOSAVE state"));
if ((flags & FPU_KERN_NOCTX) != 0) {
critical_enter();
if (curthread == PCPU_GET(fpcurthread)) {
vfp_save_state(curthread, pcb);
}
PCPU_SET(fpcurthread, NULL);
vfp_enable();
mmelUnsubmitted
Not Done
Inline Actions

You cannot simply enable VFP here. It can be in a previous state, for example in the middle of a vectorized instruction of another thread. Before enabling it, a new empty context must be loaded.

mmel: You cannot simply enable VFP here. It can be in a previous state, for example in the middle of…
pcb->pcb_fpflags |= PCB_FP_KERN | PCB_FP_NOSAVE |
PCB_FP_STARTED;
return;
}
if ((flags & FPU_KERN_KTHR) != 0 && is_fpu_kern_thread(0)) {
ctx->flags = FPU_KERN_CTX_DUMMY | FPU_KERN_CTX_INUSE;
return;
}
/*
* Check either we are already using the VFP in the kernel, or
* the the saved state points to the default user space.
*/
KASSERT((pcb->pcb_fpflags & PCB_FP_KERN) != 0 ||
pcb->pcb_vfpsaved == &pcb->pcb_vfpstate,
("Mangled pcb_vfpsaved %x %p %p", pcb->pcb_fpflags, pcb->pcb_vfpsaved,
mwUnsubmitted
Done
Inline Actions

This line is a bit too long, please break it as below:

	    ("Mangled pcb_fpusaved %x %p %p", pcb->pcb_fpflags, pcb->pcb_fpusaved,
            &pcb->pcb_fpustate));
mw: This line is a bit too long, please break it as below: ("Mangled pcb_fpusaved %x %p %p"…
&pcb->pcb_vfpstate));
ctx->flags = FPU_KERN_CTX_INUSE;
vfp_save_state(curthread, pcb);
mmelUnsubmitted
Not Done
Inline Actions

I'm not sure, but if this is called from an interruptible context then we may end with inconsistent VFP state, flags and ctx. Imho this should be covered by the critical section. Same should be applied also to fpu_kern_leave()

mmel: I'm not sure, but if this is called from an interruptible context then we may end with…
ctx->prev = pcb->pcb_vfpsaved;
pcb->pcb_vfpsaved = &ctx->state;
pcb->pcb_fpflags |= PCB_FP_KERN;
pcb->pcb_fpflags &= ~PCB_FP_STARTED;
return;
}
int
fpu_kern_leave(struct thread *td, struct fpu_kern_ctx *ctx)
{
struct pcb *pcb;
pcb = td->td_pcb;
if ((pcb->pcb_fpflags & PCB_FP_NOSAVE) != 0) {
KASSERT(ctx == NULL, ("non-null ctx after FPU_KERN_NOCTX"));
KASSERT(PCPU_GET(fpcurthread) == NULL,
("non-NULL fpcurthread for PCB_FP_NOSAVE"));
CRITICAL_ASSERT(td);
vfp_disable();
pcb->pcb_fpflags &= ~(PCB_FP_NOSAVE | PCB_FP_STARTED);
critical_exit();
} else {
KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) != 0,
("FPU context not inuse"));
ctx->flags &= ~FPU_KERN_CTX_INUSE;
if (is_fpu_kern_thread(0) &&
(ctx->flags & FPU_KERN_CTX_DUMMY) != 0)
return (0);
KASSERT((ctx->flags & FPU_KERN_CTX_DUMMY) == 0, ("dummy ctx"));
critical_enter();
vfp_discard(td);
critical_exit();
pcb->pcb_fpflags &= ~PCB_FP_STARTED;
pcb->pcb_vfpsaved = ctx->prev;
}
if (pcb->pcb_vfpsaved == &pcb->pcb_vfpstate) {
pcb->pcb_fpflags &= ~PCB_FP_KERN;
} else {
KASSERT((pcb->pcb_fpflags & PCB_FP_KERN) != 0,
("unpaired fpu_kern_leave"));
}
return (0);
}
int
fpu_kern_thread(u_int flags __unused)
{
struct pcb *pcb = curthread->td_pcb;
KASSERT((curthread->td_pflags & TDP_KTHREAD) != 0,
("Only kthread may use fpu_kern_thread"));
KASSERT(pcb->pcb_vfpsaved == &pcb->pcb_vfpstate,
("Mangled pcb_vfpsaved"));
KASSERT((pcb->pcb_fpflags & PCB_FP_KERN) == 0,
("Thread already setup for the VFP"));
pcb->pcb_fpflags |= PCB_FP_KERN;
return (0);
}
int
is_fpu_kern_thread(u_int flags __unused)
{
struct pcb *curpcb;
if ((curthread->td_pflags & TDP_KTHREAD) == 0)
return (0);
curpcb = curthread->td_pcb;
return ((curpcb->pcb_fpflags & PCB_FP_KERN) != 0);
} }
#endif #endif