The global page that holds ARM_TP_ADDRESS and the RAS start/end addresses is allocated using the VM_ALLOC_ZERO flag, but that flag only indicates a preference for a pre-zeroed page if one is available, it doesn't guarantee the returned page has been zeroed. It appears that often (perhaps always?) there are no pre-zeroed pages available early in startup, and the page will contain random values. Some combinations of these random values may be misinterpreted by the RAS code in PUSHFRAMEINSVC, causing it to use the garbage value from RAS_START when returning from the first trap/interrupt, typically causing a panic due to supervisor-mode access fault. Fix: This patch zeroes the page if necessary after allocating it, and also initializes ARM_RAS_END to 0xffffffff, since that's one of the preconditions of the RAS scheme when no atomic sequence is running. How-To-Repeat: Naturally a failure scenario based on inopportune values of random data is pretty rare and hard to repeat.
Author: cognet Date: Sun Oct 16 17:37:54 2011 New Revision: 226441 URL: http://svn.freebsd.org/changeset/base/226441 Log: Explicitely set ARM_RAS_START and ARM_RAS_END once the cacheline or the page has been allocated, or we could end up using random values, and bad things could happen. PR: arm/161492 Submitted by: Ian Lepore <freebsd AT damnhippie dot dyndns DOT org> MFC after: 1 week Modified: head/sys/arm/arm/machdep.c Modified: head/sys/arm/arm/machdep.c ============================================================================== --- head/sys/arm/arm/machdep.c Sun Oct 16 16:58:28 2011 (r226440) +++ head/sys/arm/arm/machdep.c Sun Oct 16 17:37:54 2011 (r226441) @@ -312,6 +312,8 @@ cpu_startup(void *dummy) m = vm_page_alloc(NULL, 0, VM_ALLOC_NOOBJ | VM_ALLOC_ZERO); pmap_kenter_user(ARM_TP_ADDRESS, VM_PAGE_TO_PHYS(m)); #endif + *(uint32_t *)ARM_RAS_START = 0; + *(uint32_t *)ARM_RAS_END = 0xffffffff; } SYSINIT(cpu, SI_SUB_CPU, SI_ORDER_FIRST, cpu_startup, NULL); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->closed Committed, with minor changes. Thanks!
Author: cognet Date: Tue Feb 7 15:50:14 2012 New Revision: 231131 URL: http://svn.freebsd.org/changeset/base/231131 Log: MFC r226441 and r226443 r226441: Explicitely set ARM_RAS_START and ARM_RAS_END once the cacheline or the page has been allocated, or we could end up using random values, and bad things could happen. PR: arm/161492 Submitted by: Ian Lepore <freebsd AT damnhippie dot dyndns DOT org> r226443: Fix 2 bugs : - A race condition could happen if two threads were using RAS at the same time as the code didn't reset RAS_END, the RAS code could believe we were not in a RAS, when we were in fact. - Using signed value logic to compare addresses wasn't such a good idea. Many thanks to Ian to investigate on these issues. Pointy hat to: cognet PR: arm/161498 Submitted by: Ian Lepore <freebsd At damnhippie DOT dyndns dot org Modified: stable/8/sys/arm/arm/machdep.c stable/8/sys/arm/include/asmacros.h stable/8/sys/arm/include/sysarch.h Modified: stable/8/sys/arm/arm/machdep.c ============================================================================== --- stable/8/sys/arm/arm/machdep.c Tue Feb 7 14:50:33 2012 (r231130) +++ stable/8/sys/arm/arm/machdep.c Tue Feb 7 15:50:14 2012 (r231131) @@ -312,6 +312,8 @@ cpu_startup(void *dummy) m = vm_page_alloc(NULL, 0, VM_ALLOC_NOOBJ | VM_ALLOC_ZERO); pmap_kenter_user(ARM_TP_ADDRESS, VM_PAGE_TO_PHYS(m)); #endif + *(uint32_t *)ARM_RAS_START = 0; + *(uint32_t *)ARM_RAS_END = 0xffffffff; } SYSINIT(cpu, SI_SUB_CPU, SI_ORDER_FIRST, cpu_startup, NULL); Modified: stable/8/sys/arm/include/asmacros.h ============================================================================== --- stable/8/sys/arm/include/asmacros.h Tue Feb 7 14:50:33 2012 (r231130) +++ stable/8/sys/arm/include/asmacros.h Tue Feb 7 15:50:14 2012 (r231131) @@ -71,9 +71,8 @@ ldr r0, =ARM_RAS_START; \ mov r1, #0; \ str r1, [r0]; \ - ldr r0, =ARM_RAS_END; \ mov r1, #0xffffffff; \ - str r1, [r0]; + str r1, [r0, #4]; /* * PULLFRAME - macro to pull a trap frame from the stack in the current mode @@ -120,20 +119,19 @@ stmia r0, {r13-r14}^; /* Push the user mode registers */ \ mov r0, r0; /* NOP for previous instruction */ \ ldr r5, =ARM_RAS_START; /* Check if there's any RAS */ \ - ldr r3, [r5]; \ - cmp r3, #0; /* Is the update needed ? */ \ - ldrgt lr, [r0, #16]; \ - ldrgt r1, =ARM_RAS_END; \ - ldrgt r4, [r1]; /* Get the end of the RAS */ \ - movgt r2, #0; /* Reset the magic addresses */ \ - strgt r2, [r5]; \ - movgt r2, #0xffffffff; \ - strgt r2, [r1]; \ - cmpgt lr, r3; /* Were we in the RAS ? */ \ - cmpgt r4, lr; \ - strgt r3, [r0, #16]; /* Yes, update the pc */ \ - mrs r0, spsr_all; /* Put the SPSR on the stack */ \ - str r0, [sp, #-4]! + ldr r4, [r5, #4]; /* reset it to point at the */ \ + cmp r4, #0xffffffff; /* end of memory if necessary; */ \ + movne r1, #0xffffffff; /* leave value in r4 for later */ \ + strne r1, [r5, #4]; /* comparision against PC. */ \ + ldr r3, [r5]; /* Retrieve global RAS_START */ \ + cmp r3, #0; /* and reset it if non-zero. */ \ + movne r1, #0; /* If non-zero RAS_START and */ \ + strne r1, [r5]; /* PC was lower than RAS_END, */ \ + ldrne r1, [r0, #16]; /* adjust the saved PC so that */ \ + cmpne r4, r1; /* execution later resumes at */ \ + strhi r3, [r0, #16]; /* the RAS_START location. */ \ + mrs r0, spsr_all; \ + str r0, [sp, #-4]! /* * PULLFRAMEFROMSVCANDEXIT - macro to pull a trap frame from the stack Modified: stable/8/sys/arm/include/sysarch.h ============================================================================== --- stable/8/sys/arm/include/sysarch.h Tue Feb 7 14:50:33 2012 (r231130) +++ stable/8/sys/arm/include/sysarch.h Tue Feb 7 15:50:14 2012 (r231131) @@ -42,9 +42,13 @@ * The ARM_TP_ADDRESS points to a special purpose page, which is used as local * store for the ARM per-thread data and Restartable Atomic Sequences support. * Put it just above the "high" vectors' page. - * the cpu_switch() code assumes ARM_RAS_START is ARM_TP_ADDRESS + 4, and + * The cpu_switch() code assumes ARM_RAS_START is ARM_TP_ADDRESS + 4, and * ARM_RAS_END is ARM_TP_ADDRESS + 8, so if that ever changes, be sure to * update the cpu_switch() (and cpu_throw()) code as well. + * In addition, code in arm/include/atomic.h and arm/include/asmacros.h + * assumes that ARM_RAS_END is at ARM_RAS_START+4, so be sure to update those + * if ARM_RAS_END moves in relation to ARM_RAS_START (look for occurrances + * of ldr/str rm,[rn, #4]). */ #define ARM_TP_ADDRESS (ARM_VECTORS_HIGH + 0x1000) #define ARM_RAS_START (ARM_TP_ADDRESS + 4) _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: cognet Date: Tue Feb 7 16:07:29 2012 New Revision: 231132 URL: http://svn.freebsd.org/changeset/base/231132 Log: MFC r226441 and r226443 r226441: Explicitely set ARM_RAS_START and ARM_RAS_END once the cacheline or the page has been allocated, or we could end up using random values, and bad things could happen. PR: arm/161492 Submitted by: Ian Lepore <freebsd AT damnhippie dot dyndns DOT org> r226443: Fix 2 bugs : - A race condition could happen if two threads were using RAS at the same time as the code didn't reset RAS_END, the RAS code could believe we were not in a RAS, when we were in fact. - Using signed value logic to compare addresses wasn't such a good idea. Many thanks to Ian to investigate on these issues. Pointy hat to: cognet PR: arm/161498 Submitted by: Ian Lepore <freebsd At damnhippie DOT dyndns dot org Modified: stable/9/sys/arm/arm/machdep.c stable/9/sys/arm/include/asmacros.h stable/9/sys/arm/include/sysarch.h Modified: stable/9/sys/arm/arm/machdep.c ============================================================================== --- stable/9/sys/arm/arm/machdep.c Tue Feb 7 15:50:14 2012 (r231131) +++ stable/9/sys/arm/arm/machdep.c Tue Feb 7 16:07:29 2012 (r231132) @@ -312,6 +312,8 @@ cpu_startup(void *dummy) m = vm_page_alloc(NULL, 0, VM_ALLOC_NOOBJ | VM_ALLOC_ZERO); pmap_kenter_user(ARM_TP_ADDRESS, VM_PAGE_TO_PHYS(m)); #endif + *(uint32_t *)ARM_RAS_START = 0; + *(uint32_t *)ARM_RAS_END = 0xffffffff; } SYSINIT(cpu, SI_SUB_CPU, SI_ORDER_FIRST, cpu_startup, NULL); Modified: stable/9/sys/arm/include/asmacros.h ============================================================================== --- stable/9/sys/arm/include/asmacros.h Tue Feb 7 15:50:14 2012 (r231131) +++ stable/9/sys/arm/include/asmacros.h Tue Feb 7 16:07:29 2012 (r231132) @@ -71,9 +71,8 @@ ldr r0, =ARM_RAS_START; \ mov r1, #0; \ str r1, [r0]; \ - ldr r0, =ARM_RAS_END; \ mov r1, #0xffffffff; \ - str r1, [r0]; + str r1, [r0, #4]; /* * PULLFRAME - macro to pull a trap frame from the stack in the current mode @@ -120,20 +119,19 @@ stmia r0, {r13-r14}^; /* Push the user mode registers */ \ mov r0, r0; /* NOP for previous instruction */ \ ldr r5, =ARM_RAS_START; /* Check if there's any RAS */ \ - ldr r3, [r5]; \ - cmp r3, #0; /* Is the update needed ? */ \ - ldrgt lr, [r0, #16]; \ - ldrgt r1, =ARM_RAS_END; \ - ldrgt r4, [r1]; /* Get the end of the RAS */ \ - movgt r2, #0; /* Reset the magic addresses */ \ - strgt r2, [r5]; \ - movgt r2, #0xffffffff; \ - strgt r2, [r1]; \ - cmpgt lr, r3; /* Were we in the RAS ? */ \ - cmpgt r4, lr; \ - strgt r3, [r0, #16]; /* Yes, update the pc */ \ - mrs r0, spsr_all; /* Put the SPSR on the stack */ \ - str r0, [sp, #-4]! + ldr r4, [r5, #4]; /* reset it to point at the */ \ + cmp r4, #0xffffffff; /* end of memory if necessary; */ \ + movne r1, #0xffffffff; /* leave value in r4 for later */ \ + strne r1, [r5, #4]; /* comparision against PC. */ \ + ldr r3, [r5]; /* Retrieve global RAS_START */ \ + cmp r3, #0; /* and reset it if non-zero. */ \ + movne r1, #0; /* If non-zero RAS_START and */ \ + strne r1, [r5]; /* PC was lower than RAS_END, */ \ + ldrne r1, [r0, #16]; /* adjust the saved PC so that */ \ + cmpne r4, r1; /* execution later resumes at */ \ + strhi r3, [r0, #16]; /* the RAS_START location. */ \ + mrs r0, spsr_all; \ + str r0, [sp, #-4]! /* * PULLFRAMEFROMSVCANDEXIT - macro to pull a trap frame from the stack Modified: stable/9/sys/arm/include/sysarch.h ============================================================================== --- stable/9/sys/arm/include/sysarch.h Tue Feb 7 15:50:14 2012 (r231131) +++ stable/9/sys/arm/include/sysarch.h Tue Feb 7 16:07:29 2012 (r231132) @@ -42,9 +42,13 @@ * The ARM_TP_ADDRESS points to a special purpose page, which is used as local * store for the ARM per-thread data and Restartable Atomic Sequences support. * Put it just above the "high" vectors' page. - * the cpu_switch() code assumes ARM_RAS_START is ARM_TP_ADDRESS + 4, and + * The cpu_switch() code assumes ARM_RAS_START is ARM_TP_ADDRESS + 4, and * ARM_RAS_END is ARM_TP_ADDRESS + 8, so if that ever changes, be sure to * update the cpu_switch() (and cpu_throw()) code as well. + * In addition, code in arm/include/atomic.h and arm/include/asmacros.h + * assumes that ARM_RAS_END is at ARM_RAS_START+4, so be sure to update those + * if ARM_RAS_END moves in relation to ARM_RAS_START (look for occurrances + * of ldr/str rm,[rn, #4]). */ #define ARM_TP_ADDRESS (ARM_VECTORS_HIGH + 0x1000) #define ARM_RAS_START (ARM_TP_ADDRESS + 4) _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"