Module Name: src
Committed By: snj
Date: Tue Oct 24 09:14:59 UTC 2017
Modified Files:
src/sys/arch/arm/vfp [netbsd-8]: vfp_init.c
src/sys/kern [netbsd-8]: subr_pcu.c
Log Message:
Pull up following revision(s) (requested by bouyer in ticket #326):
sys/arch/arm/vfp/vfp_init.c: revision 1.54-1.55
sys/kern/subr_pcu.c: revision 1.21
PR port-arm/52603:
There is a race here, as seen on arm with FPU:
LWP L is running but not on CPU, has its FPU state on CPU2 which
has not been released yet, so fpexc still has VFP_FPEXC_EN set in the PCB copy.
LWP L is scheduled on CPU1, CPU1 calls cpu_switchto() for L in mi_switch().
cpu_switchto() will set VFP_FPEXC_EN in the FPU's fpexc register per the
PCB fpexc copy.
Before CPU1 calls pcu_switchpoint() for L, CPU2 calls
pcu_do_op(PCU_CMD_SAVE | PCU_CMD_RELEASE) for L because it still holds its
FPU state and wants to load another lwp. This cause VFP_FPEXC_EN to
be cleared in the PCB copy, but not in CPU1's register. L's l_pcu_cpu is
set to NULL.
When CPU1 calls pcu_switchpoint() for L it see l_pcu_cpu is NULL, and doesn't
call the release callback.
Now CPU1 has its FPU enabled but with the wrong FPU state.
Fix by releasing the PCU even if l_pcu_cpu is NULL.
--
In the REENABLE case, make sur the fpexc copy in the pcb also has
VFP_FPEXC_EN set. Otherwise we could trap on every context switch even if
the CPU already has the VFP state.
--
We KASSERT((fregs->vfp_fpexc & VFP_FPEXC_EN) == 0) just before, so
enabled is always false. remove.
To generate a diff of this commit:
cvs rdiff -u -r1.53 -r1.53.2.1 src/sys/arch/arm/vfp/vfp_init.c
cvs rdiff -u -r1.20 -r1.20.6.1 src/sys/kern/subr_pcu.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/arch/arm/vfp/vfp_init.c
diff -u src/sys/arch/arm/vfp/vfp_init.c:1.53 src/sys/arch/arm/vfp/vfp_init.c:1.53.2.1
--- src/sys/arch/arm/vfp/vfp_init.c:1.53 Fri May 26 21:17:46 2017
+++ src/sys/arch/arm/vfp/vfp_init.c Tue Oct 24 09:14:59 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: vfp_init.c,v 1.53 2017/05/26 21:17:46 jmcneill Exp $ */
+/* $NetBSD: vfp_init.c,v 1.53.2.1 2017/10/24 09:14:59 snj Exp $ */
/*
* Copyright (c) 2008 ARM Ltd
@@ -520,29 +520,25 @@ vfp_state_load(lwp_t *l, u_int flags)
curcpu()->ci_vfp_evs[1].ev_count++;
}
+ KASSERT((armreg_fpexc_read() & VFP_FPEXC_EN) == 0);
/*
* If the VFP is already enabled we must be bouncing an instruction.
*/
if (flags & PCU_REENABLE) {
uint32_t fpexc = armreg_fpexc_read();
armreg_fpexc_write(fpexc | VFP_FPEXC_EN);
+ fregs->vfp_fpexc |= VFP_FPEXC_EN;
return;
}
+ KASSERT((fregs->vfp_fpexc & VFP_FPEXC_EN) == 0);
/*
* Load and Enable the VFP (so that we can write the registers).
*/
- bool enabled = fregs->vfp_fpexc & VFP_FPEXC_EN;
fregs->vfp_fpexc |= VFP_FPEXC_EN;
armreg_fpexc_write(fregs->vfp_fpexc);
- if (enabled) {
- /*
- * If we think the VFP is enabled, it must have be
- * disabled by vfp_state_release for another LWP so
- * we can now just return.
- */
- return;
- }
+ KASSERT(curcpu()->ci_pcu_curlwp[PCU_FPU] == NULL);
+ KASSERT(l->l_pcu_cpu[PCU_FPU] == NULL);
load_vfpregs(fregs);
armreg_fpscr_write(fregs->vfp_fpscr);
@@ -562,6 +558,9 @@ vfp_state_save(lwp_t *l)
struct vfpreg * const fregs = &pcb->pcb_vfp;
uint32_t fpexc = armreg_fpexc_read();
+ KASSERT(curcpu()->ci_pcu_curlwp[PCU_FPU] == l);
+ KASSERT(curcpu() == l->l_pcu_cpu[PCU_FPU]);
+ KASSERT(curlwp == l || curlwp->l_pcu_cpu[PCU_FPU] != curcpu());
/*
* Enable the VFP (so we can read the registers).
* Make sure the exception bit is cleared so that we can
Index: src/sys/kern/subr_pcu.c
diff -u src/sys/kern/subr_pcu.c:1.20 src/sys/kern/subr_pcu.c:1.20.6.1
--- src/sys/kern/subr_pcu.c:1.20 Thu Mar 16 16:13:21 2017
+++ src/sys/kern/subr_pcu.c Tue Oct 24 09:14:59 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: subr_pcu.c,v 1.20 2017/03/16 16:13:21 chs Exp $ */
+/* $NetBSD: subr_pcu.c,v 1.20.6.1 2017/10/24 09:14:59 snj Exp $ */
/*-
* Copyright (c) 2011, 2014 The NetBSD Foundation, Inc.
@@ -52,7 +52,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_pcu.c,v 1.20 2017/03/16 16:13:21 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_pcu.c,v 1.20.6.1 2017/10/24 09:14:59 snj Exp $");
#include <sys/param.h>
#include <sys/cpu.h>
@@ -110,7 +110,8 @@ pcu_switchpoint(lwp_t *l)
continue;
}
struct cpu_info * const pcu_ci = l->l_pcu_cpu[id];
- if (pcu_ci == NULL || pcu_ci == l->l_cpu) {
+ if (pcu_ci == l->l_cpu) {
+ KASSERT(pcu_ci->ci_pcu_curlwp[id] == l);
continue;
}
const pcu_ops_t * const pcu = pcu_ops_md_defs[id];
