NetBSD Problem Report #52603

From bouyer@antioche.eu.org  Sun Oct  8 16:18:58 2017
Return-Path: <bouyer@antioche.eu.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id CA8647A1BC
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  8 Oct 2017 16:18:58 +0000 (UTC)
Message-Id: <20171008161851.6E1552807@rochebonne.antioche.eu.org>
Date: Sun,  8 Oct 2017 18:18:51 +0200 (CEST)
From: bouyer@antioche.eu.org
Reply-To: bouyer@antioche.eu.org
To: gnats-bugs@NetBSD.org
Subject: arm(v7?) vfp register corruption
X-Send-Pr-Version: 3.95

>Number:         52603
>Category:       port-arm
>Synopsis:       arm(v7?) vfp register corruption
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    bouyer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Oct 08 16:20:00 +0000 2017
>Closed-Date:    Tue Oct 24 09:20:40 +0000 2017
>Last-Modified:  Tue Oct 24 09:20:40 +0000 2017
>Originator:     Manuel Bouyer
>Release:        NetBSD 8.0_BETA
>Organization:
>Environment:
System: NetBSD chartplotter 8.0_BETA NetBSD 8.0_BETA (CHARTPLOTTER) #1: Sat Sep 9 13:55:40 CEST 2017 bouyer@bop.soc.lip6.fr:/dsk/l1/misc/bouyer/tmp/earmv7hf/obj/dsk/l1/misc/bouyer/netbsd-8/src/sys/arch/evbarm/compile/CHARTPLOTTER evbarm
Architecture: earmv7hf
Machine: evbarm
>Description:
	running pkgsrc/geography/opencpn on a olimex lime2, and a cubieboard2
	(both Allinner A20), I got evidence of occasional
	floating-point register corruption (a printf at a strategic point
	shows that a variable computed from other values a few lines before
	has the wrong value).
	I then tried this test program:
cat > fptest.c << EOF
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <err.h>
#include <sys/time.h>

#define NREGS 32

void do_test(int);
void do_wait(int);

double foo = 0;


void
do_wait(int id) {
	struct timeval start, now;
	if (id != 0)
		return;
	// sleep(1);
	gettimeofday(&start, NULL);
	while (1) {
		if (id == 0) {
			foo = foo * 0.1;
		} else {
			int i;
			for (i = 0; i < 1000000000; i++) {
				if (id == 0)
				;
			}
		}
		gettimeofday(&now, NULL);
		if (now.tv_sec - start.tv_sec > 1)
			return;
	}
}

void
do_test(int id)
{
	double *src = malloc(sizeof(double) * NREGS);
	double *dst = malloc(sizeof(double) * NREGS);
	int i;

	printf("start job %d for %d registers\n", id, NREGS);

	for (i = 0; i < NREGS; i++) {
		src[i] = id * 100 + i * 1.1;
	}

	while (1) {
		foo = foo * 0.1;
		__asm __volatile("vldmia %0, {d0-d15}" :: "r" (src) : "memory");
#if NREGS > 16
		__asm __volatile("vldmia %0, {d16-d31}" :: "r" (&src[16]) : "memory");
#endif
		memset(dst, 0, sizeof(double) * NREGS);
		do_wait(id);
		__asm __volatile("vstmia %0, {d0-d15}" :: "r" (dst) : "memory");
#if NREGS > 16
		__asm __volatile("vstmia\t%0, {d16-d31}" :: "r" (&dst[16]) : "memory");
#endif
		if (id == 0)
			continue;
		for (i = 0; i < NREGS; i++) {
			double v = id * 100 + i * 1.1;
			if (dst[i] != v) {
				printf("%d: %lf %lf %lf\n", i, src[i], dst[i], v);
			}
		}
	}
}

int
main(int argc, const char *argv[])
{
	int n;
	int i;

	if (argc != 2) {
		errx(1, "usage: fptest <n>");
	}

	n = atoi(argv[1]);

	for (i = 1; i < n; i++) {
		switch(fork()) {
		case -1:
			err(1, "fork");

		case 0:
			do_test(i);
			exit(0);

		default:
			break;
		}
	}
	do_test(0);
	exit(0);
}
EOF
	compile with
gcc -g -mfpu=neon-vfpv4 -o fptest fptest.c

	running 
./fptest 2
	in parallel with opencpn, after a few hours I got
0: 100.000000 100.000000 28.000000

	and then, less than a day later:
0: 100.000000 11.672723 100.000000
1: 101.100000 16.850291 101.100000
2: 102.200000 6.424029 102.200000
3: 103.300000 16.679222 103.300000
4: 104.400000 255.000000 104.400000
5: 105.500000 255.000000 105.500000
6: 106.600000 255.000000 106.600000
7: 107.700000 0.002048 107.700000
8: 108.800000 0.087582 108.800000
9: 109.900000 0.500000 109.900000

	so we have rare but obvious vfp register corruption.
	I suspect it's related an to interrupt occuring at the wrong
	time, but couldn't track it down more than that.

>How-To-Repeat:
	see above. It you're not running opencpn, you may need to run
	other heavy FP application, or start more than 2 process when
	invoking the test program.
>Fix:
	please ...

>Release-Note:

>Audit-Trail:
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: port-arm-maintainer@NetBSD.org, gnats-bugs@NetBSD.org,
        netbsd-bugs@NetBSD.org
Cc: 
Subject: Re: port-arm/52603: arm(v7?) vfp register corruption
Date: Mon, 9 Oct 2017 20:51:36 +0200

 On Sun, Oct 08, 2017 at 04:20:00PM +0000, bouyer@antioche.eu.org wrote:
 > 	running 
 > ./fptest 2
 > 	in parallel with opencpn, after a few hours I got
 > 0: 100.000000 100.000000 28.000000
 > 
 > 	and then, less than a day later:
 > 0: 100.000000 11.672723 100.000000
 > 1: 101.100000 16.850291 101.100000
 > 2: 102.200000 6.424029 102.200000
 > 3: 103.300000 16.679222 103.300000
 > 4: 104.400000 255.000000 104.400000
 > 5: 105.500000 255.000000 105.500000
 > 6: 106.600000 255.000000 106.600000
 > 7: 107.700000 0.002048 107.700000
 > 8: 108.800000 0.087582 108.800000
 > 9: 109.900000 0.500000 109.900000
 > 
 > 	so we have rare but obvious vfp register corruption.
 > 	I suspect it's related an to interrupt occuring at the wrong
 > 	time, but couldn't track it down more than that.

 running
 ./fptest 3
 without opencpn, on an otherwise idle board didn't show corruption after
 24h. But starting a ping -f against the board caused the corruption to
 show up in less than half an hour.

 So it looks like interrupt activity (other than the CPU ticks) is needed to
 trigger the problem.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: port-arm-maintainer@NetBSD.org, gnats-bugs@NetBSD.org,
        netbsd-bugs@NetBSD.org
Cc: 
Subject: Re: port-arm/52603: arm(v7?) vfp register corruption
Date: Mon, 9 Oct 2017 21:07:11 +0200

 On Mon, Oct 09, 2017 at 08:51:36PM +0200, Manuel Bouyer wrote:
 > running
 > ./fptest 3
 > without opencpn, on an otherwise idle board didn't show corruption after
 > 24h. But starting a ping -f against the board caused the corruption to
 > show up in less than half an hour.
 > 
 > So it looks like interrupt activity (other than the CPU ticks) is needed to
 > trigger the problem.

 Some interresting patterns:
 7: 107.700000 0.000000 107.700000
 17: 118.700000 117.600000 118.700000
 18: 119.800000 1.100000 119.800000

 the '17' line looks just as if a previous value of the register from the
 same process had been restored

 27: 129.700000 129.700000 217.600000

 here the wrong value looks like comming from another instance of the
 test program.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: port-arm-maintainer@NetBSD.org, gnats-bugs@NetBSD.org,
        netbsd-bugs@NetBSD.org
Cc: 
Subject: Re: port-arm/52603: arm(v7?) vfp register corruption
Date: Mon, 9 Oct 2017 22:04:29 +0200

 On Mon, Oct 09, 2017 at 09:07:11PM +0200, Manuel Bouyer wrote:
 > Some interresting patterns:
 > 7: 107.700000 0.000000 107.700000
 > 17: 118.700000 117.600000 118.700000
 > 18: 119.800000 1.100000 119.800000
 > 
 > the '17' line looks just as if a previous value of the register from the
 > same process had been restored
 > 
 > 27: 129.700000 129.700000 217.600000
 > 
 > here the wrong value looks like comming from another instance of the
 > test program.

 Some other interesting patterns:
 16: 217.600000 218.700000 217.600000

 I can't make much sense of this one


 0: 100.000000 0.000000 100.000000  
 1: 101.100000 1.100000 101.100000  
 2: 102.200000 2.200000 102.200000  
 3: 103.300000 3.300000 103.300000  
 4: 104.400000 4.400000 104.400000  
 5: 105.500000 5.500000 105.500000  
 6: 106.600000 6.600000 106.600000  
 7: 107.700000 7.700000 107.700000  
 8: 108.800000 8.800000 108.800000  
 9: 109.900000 9.900000 109.900000  
 10: 111.000000 11.000000 111.000000
 11: 112.100000 12.100000 112.100000
 12: 113.200000 13.200000 113.200000
 13: 114.300000 14.300000 114.300000
 14: 115.400000 15.400000 115.400000
 15: 116.500000 16.500000 116.500000
 16: 117.600000 0.000000 117.600000
 17: 118.700000 0.100000 118.700000
 18: 119.800000 19.800000 119.800000
 19: 120.900000 20.900000 120.900000
 20: 122.000000 22.000000 122.000000
 21: 123.100000 23.100000 123.100000
 22: 124.200000 24.200000 124.200000
 23: 125.300000 25.300000 125.300000
 24: 126.400000 26.400000 126.400000
 25: 127.500000 27.500000 127.500000
 26: 128.600000 28.600000 128.600000
 27: 129.700000 29.700000 129.700000
 28: 130.800000 30.800000 130.800000
 29: 131.900000 31.900000 131.900000
 30: 133.000000 33.000000 133.000000
 31: 134.100000 34.100000 134.100000

 the whole set of registers is from another process.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: port-arm-maintainer@NetBSD.org, gnats-bugs@NetBSD.org,
        netbsd-bugs@NetBSD.org
Cc: 
Subject: Re: port-arm/52603: arm(v7?) vfp register corruption
Date: Wed, 11 Oct 2017 15:00:06 +0200

 I tested on a RPI3, running
 ./fptest 5
 shows the problematic behavior much faster here: in less than a minute
 I got more than 30 failures:

 $ ./fptest 5                                                            [34/130]
 start job 0 for 32 registers        
 start job 2 for 32 registers        
 start job 3 for 32 registers        
 start job 4 for 32 registers        
 start job 1 for 32 registers        

 5: 205.500000 205.500000 205.500000 

 7: 107.700000 0.000000 107.700000   
 16: 117.600000 105.500000 117.600000
 17: 118.700000 105.500000 118.700000
 18: 119.800000 100.000000 119.800000
 19: 120.900000 1.100000 120.900000  
 0: 400.000000 100.000000 400.000000 
 1: 401.100000 101.100000 401.100000
 2: 402.200000 102.200000 402.200000
 3: 403.300000 103.300000 403.300000
 4: 404.400000 104.400000 404.400000
 5: 405.500000 105.500000 405.500000
 6: 406.600000 106.600000 406.600000
 7: 407.700000 107.700000 407.700000
 8: 408.800000 108.800000 408.800000
 9: 409.900000 109.900000 409.900000
 10: 411.000000 111.000000 411.000000
 11: 412.100000 112.100000 412.100000
 12: 413.200000 113.200000 413.200000
 13: 414.300000 114.300000 414.300000
 14: 415.400000 115.400000 415.400000
 15: 416.500000 116.500000 416.500000
 16: 417.600000 117.600000 417.600000
 17: 418.700000 118.700000 418.700000
 18: 419.800000 119.800000 419.800000
 19: 420.900000 120.900000 420.900000
 20: 422.000000 122.000000 422.000000
 21: 423.100000 123.100000 423.100000
 22: 424.200000 124.200000 424.200000
 23: 425.300000 125.300000 425.300000
 24: 426.400000 126.400000 426.400000
 25: 427.500000 127.500000 427.500000
 26: 428.600000 128.600000 428.600000
 27: 429.700000 129.700000 429.700000
 28: 430.800000 130.800000 430.800000
 29: 431.900000 131.900000 431.900000
 30: 433.000000 133.000000 433.000000
 0: 100.000000 417.600000 100.000000
 3: 103.300000 0.000000 103.300000
 4: 104.400000 10.000000 104.400000
 5: 105.500000 0.500000 105.500000
 6: 106.600000 0.500000 106.600000
 7: 107.700000 0.000000 107.700000
 16: 117.600000 417.600000 117.600000
 17: 118.700000 400.000000 118.700000
 18: 119.800000 1.100000 119.800000
 19: 120.900000 1.100000 120.900000

 The first one is strange, it looks like registers have the correct values,
 yet the compare failed.

 I looked at subr_pcu.c and vfp_init.c but I didn't see anything obvious ...
 I added a few KASSERT() in both files but they didn't fire.

 idea welcome ...

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: port-arm-maintainer@NetBSD.org, gnats-bugs@NetBSD.org,
        netbsd-bugs@NetBSD.org
Cc: 
Subject: Re: port-arm/52603: arm(v7?) vfp register corruption
Date: Sun, 15 Oct 2017 10:02:53 +0200

 Some progress. I added various KASSERTs, especially this one:

 Index: subr_pcu.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/subr_pcu.c,v
 retrieving revision 1.20
 diff -u -p -u -r1.20 subr_pcu.c
 --- subr_pcu.c	16 Mar 2017 16:13:21 -0000	1.20
 +++ subr_pcu.c	15 Oct 2017 07:55:08 -0000
 @@ -59,6 +59,8 @@ __KERNEL_RCSID(0, "$NetBSD: subr_pcu.c,v
  #include <sys/lwp.h>
  #include <sys/pcu.h>
  #include <sys/ipi.h>
 +#include <arm/pcb.h>
 +#include <arm/vfpreg.h>

  #if PCU_UNIT_COUNT > 0

 @@ -92,25 +94,42 @@ extern const pcu_ops_t * const pcu_ops_m
   * pcu_switchpoint: release PCU state if the LWP is being run on another CPU.
   * This routine is called on each context switch by by mi_switch().
   */
 +__asm(".fpu\tvfpv4");
  void
  pcu_switchpoint(lwp_t *l)
  {
  	const uint32_t pcu_valid = l->l_pcu_valid;
  	int s;
 +	struct pcb * const pcb = lwp_getpcb(l);

  	KASSERTMSG(l == curlwp, "l %p != curlwp %p", l, curlwp);

  	if (__predict_true(pcu_valid == 0)) {
  		/* PCUs are not in use. */
 +		KASSERT((pcb->pcb_vfp.vfp_fpexc & VFP_FPEXC_EN) == 0);
  		return;
  	}
  	s = splpcu();
  	for (u_int id = 0; id < PCU_UNIT_COUNT; id++) {
  		if ((pcu_valid & (1U << id)) == 0) {
 +			if (id == PCU_FPU) {
 +				KASSERT((pcb->pcb_vfp.vfp_fpexc & VFP_FPEXC_EN) == 0);
 +				KASSERTMSG((armreg_fpexc_read() & VFP_FPEXC_EN) == 0,
 +				    "fpexc 0x%x/0x%x vfpid 0x%x",
 +				    pcb->pcb_vfp.vfp_fpexc, armreg_fpexc_read(), curcpu()->ci_vfp_id);
 +			}
  			continue;
  		}
  		struct cpu_info * const pcu_ci = l->l_pcu_cpu[id];
  		if (pcu_ci == NULL || pcu_ci == l->l_cpu) {
 +			if (pcu_ci == NULL && id == PCU_FPU) {
 +				KASSERT((pcb->pcb_vfp.vfp_fpexc & VFP_FPEXC_EN) == 0);
 +				KASSERTMSG((armreg_fpexc_read() & VFP_FPEXC_EN) == 0,
 +				    "fpexc 0x%x/0x%x vfpid 0x%x",
 +				    pcb->pcb_vfp.vfp_fpexc, armreg_fpexc_read(), curcpu()->ci_vfp_id);
 +			} else {
 +				KASSERT(pcu_ci->ci_pcu_curlwp[id] == l);
 +			}
  			continue;
  		}
  		const pcu_ops_t * const pcu = pcu_ops_md_defs[id];

 And it fired:
 panic: kernel diagnostic assertion "(armreg_fpexc_read() & VFP_FPEXC_EN) == 0" failed: file "/dsk/l1/misc/bouyer/netbsd-8/src/sys/kern/subr_pcu.c", line 129 fpexc 0x0/0x40000000 vfpid 0x41023074
 Stopped in pid 713.1 (fptest) at        netbsd:cpu_Debugger+0x4:        bx      r14
 0xbb397eac: netbsd:vpanic+0x10
 0xbb397ec4: netbsd:__udivmoddi4
 0xbb397f04: netbsd:pcu_switchpoint+0x19c
 0xbb397f64: netbsd:mi_switch+0x2a4
 0xbb397f84: netbsd:preempt+0x80
 0xbb397fac: netbsd:ast+0x58

 So the lwp's pcb has the FPU disabled in it's fpexc copy, but it's enabled
 on the CPU (so the lwp will use the FPU with the wrong state)
 This would suggest that cpu_switchto() failed to properly
 reload fpexc, or that there is another code path that can cause a lwp
 to return to mi_switch(), which is not cpu_switchto. I didn't find
 this other path yet ...

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: port-arm-maintainer@NetBSD.org, gnats-bugs@NetBSD.org,
        netbsd-bugs@NetBSD.org
Cc: 
Subject: Re: port-arm/52603: arm(v7?) vfp register corruption
Date: Sun, 15 Oct 2017 20:58:24 +0200

 More KASSERT(), this time in mi_switch():

 Index: kern/kern_synch.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/kern_synch.c,v
 retrieving revision 1.311
 diff -u -p -u -r1.311 kern_synch.c
 --- kern/kern_synch.c	3 Jul 2016 14:24:58 -0000	1.311
 +++ kern/kern_synch.c	15 Oct 2017 18:54:34 -0000
 @@ -97,6 +97,9 @@ __KERNEL_RCSID(0, "$NetBSD: kern_synch.c
  #include <sys/atomic.h>
  #include <sys/syslog.h>

 +#include <arm/pcb.h>
 +#include <arm/vfpreg.h>
 +
  #include <uvm/uvm_extern.h>

  #include <dev/lockstat.h>
 @@ -503,6 +506,7 @@ nextlwp(struct cpu_info *ci, struct sche
   *
   * Returns 1 if another LWP was actually run.
   */
 +__asm(".fpu\tvfpv4");
  int
  mi_switch(lwp_t *l)
  {
 @@ -724,13 +728,44 @@ mi_switch(lwp_t *l)
  		KASSERTMSG(l == curlwp, "l %p curlwp %p prevlwp %p",
  		    l, curlwp, prevlwp);

 +		KASSERTMSG(l->l_switchto == NULL, "l %p curlwp %p prevlwp %p l->l_switchto %p", l, curlwp, prevlwp, l->l_switchto);
 +		if ((l->l_flag & LW_SYSTEM) == 0) {
 +			KASSERTMSG(((struct pcb *)lwp_getpcb(l))->pcb_vfp.vfp_fpexc == armreg_fpexc_read(),
 +			    "fpexc 0x%x prev 0x%x reg 0x%x l %p/%p", 
 +			    ((struct pcb *)lwp_getpcb(l))->pcb_vfp.vfp_fpexc,
 +			    ((struct pcb *)lwp_getpcb(prevlwp))->pcb_vfp.vfp_fpexc,
 +			    armreg_fpexc_read(),
 +			    l, prevlwp);
 +		} else {
 +			KASSERT((l->l_pcu_valid & (1 << PCU_FPU)) == 0);
 +		}
 +
  		/*
  		 * Switched away - we have new curlwp.
  		 * Restore VM context and IPL.
  		 */
  		pmap_activate(l);
  		uvm_emap_switch(l);
 +		if ((l->l_flag & LW_SYSTEM) == 0) {
 +			KASSERTMSG(((struct pcb *)lwp_getpcb(l))->pcb_vfp.vfp_fpexc == armreg_fpexc_read(),
 +			    "XXX fpexc 0x%x prev 0x%x reg 0x%x l %p/%p", 
 +			    ((struct pcb *)lwp_getpcb(l))->pcb_vfp.vfp_fpexc,
 +			    ((struct pcb *)lwp_getpcb(prevlwp))->pcb_vfp.vfp_fpexc,
 +			    armreg_fpexc_read(),
 +			    l, prevlwp);
 +		} else {
 +			KASSERT((l->l_pcu_valid & (1 << PCU_FPU)) == 0);
 +		}
  		pcu_switchpoint(l);
 +		if ((l->l_flag & LW_SYSTEM) == 0) {
 +			KASSERTMSG(((struct pcb *)lwp_getpcb(l))->pcb_vfp.vfp_fpexc == armreg_fpexc_read(),
 +			    "fpexc 0x%x/0x%x l %p/%p", 
 +			    ((struct pcb *)lwp_getpcb(l))->pcb_vfp.vfp_fpexc,
 +			    armreg_fpexc_read(),
 +			    l, prevlwp);
 +		} else {
 +			KASSERT((l->l_pcu_valid & (1 << PCU_FPU)) == 0);
 +		}

  		if (prevlwp != NULL) {
  			/* Normalize the count of the spin-mutexes */


 note that the KASSERTMSG() before and after pmap_activate() are the same.
 The KASSERTMSG() after pmap_activate() fired twice, this means that
 actually cpu_switchto() works as expected, and the FPU is reenabled
 after, probably by an interrupt.

 Another KASSERT proved me that IPIs are allowed in this portion of code,
 but I can't see where we could fail to disable the fpu before exiting the
 handler ...

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

Responsible-Changed-From-To: port-arm-maintainer->bouyer
Responsible-Changed-By: bouyer@NetBSD.org
Responsible-Changed-When: Mon, 16 Oct 2017 14:05:06 +0000
Responsible-Changed-Why:
.


State-Changed-From-To: open->analyzed
State-Changed-By: bouyer@NetBSD.org
State-Changed-When: Mon, 16 Oct 2017 14:05:06 +0000
State-Changed-Why:
The race senario is the following:

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.

I see the following way to fix this:
a) go to splhigh() before cpu_switchto() and splx() after pcu_switchpoint().
   I'm not sure it's a good idea to block all interrupts here.
b) in pcu_switchpoint() call the release callback even if l_pcu_cpu is NULL,
   to make sure the PCU of the current CPU is released too.
c) call pcu_switchpoint() for newl before cpu_switchto() in mi_switch()
d) in cpu_switchto() always set fpexc to 0
e) in cpu_switchto() check l_pcu_cpu before using the PCB fpexc copy,
   and set fpexc to NULL if the PCB copy is not from this CPU.

I think e is too exensive for cpu_switchto().
d will always cause a FPU trap, even if the FPU state is loaded on the CPU

I tested b) and it works, and it looks like the cleanest way to fix this.


From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52603 CVS commit: src/sys/kern
Date: Mon, 16 Oct 2017 15:03:57 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Mon Oct 16 15:03:57 UTC 2017

 Modified Files:
 	src/sys/kern: subr_pcu.c

 Log Message:
 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.


 To generate a diff of this commit:
 cvs rdiff -u -r1.20 -r1.21 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.

State-Changed-From-To: analyzed->pending-pullups
State-Changed-By: bouyer@NetBSD.org
State-Changed-When: Wed, 18 Oct 2017 11:30:35 +0000
State-Changed-Why:
pullup-8 #326


From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52603 CVS commit: [netbsd-8] src/sys
Date: Tue, 24 Oct 2017 09:14:59 +0000

 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.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: bouyer@NetBSD.org
State-Changed-When: Tue, 24 Oct 2017 09:20:40 +0000
State-Changed-Why:
fix pulled up to netbsd-8.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.