PATCH: fork_trampoline returning to an incorrect address

[Neelkanth Natu]

Fix a problem seen when a new process was returning to userland
through fork_trampoline.

This was caused because we were clearing the SR_INT_IE and setting
SR_EXL bits of the status register at the same time. This means
that if an interrupt happened while this MTC0 was making its way
through the pipeline the exception processing would see the
status register with SR_EXL bit set. This in turn would mean that
the COP_0_EXC_PC would not be updated so the return from exception
would be to an incorrect address.

It is easy to verify this fix by a program that forks in a loop
and the child just exits:

while (1) {
    pid_t pid = vfork();
    if (pid == 0)
       _exit(0);
    if (pid != -1)
       waitpid(pid, NULL, 0);
}

Affected files ...

... //depot/user/neelnatu/projects_mips_sibyte/src/sys/mips/include/cpuregs.h#2 edit
... //depot/user/neelnatu/projects_mips_sibyte/src/sys/mips/mips/swtch.S#2 edit

Differences ...

==== //depot/user/neelnatu/projects_mips_sibyte/src/sys/mips/include/cpuregs.h#2 (text) ====

@@ -106,7 +106,7 @@
 #elif defined(CPU_SB1)
 #define COP0_SYNC  ssnop; ssnop; ssnop; ssnop; ssnop; ssnop; ssnop; ssnop; ssnop
 #else
-#define	COP0_SYNC		/* nothing */
+#define	COP0_SYNC  nop; nop; nop; nop; nop
 #endif
 #define	COP0_HAZARD_FPUENABLE	nop; nop; nop; nop;
 

==== //depot/user/neelnatu/projects_mips_sibyte/src/sys/mips/mips/swtch.S#2 (text) ====

@@ -161,25 +161,37 @@
 
 	DO_AST
 
-/*
- * Since interrupts are enabled at this point, we use a1 instead of
- * k0 or k1 to store the PCB pointer.  This is because k0 and k1
- * are not preserved across interrupts.
- */
-	GET_CPU_PCPU(a1)
-	lw	a1, PC_CURPCB(a1)
-1:
+	/*
+	 * Clear the SR_INT_ENAB bit before setting the SR_EXL bit.
+	 *
+	 * The problem with doing them together is if an interrupt happens
+	 * while the change makes its way through the pipeline.
+	 *
+	 * According to the MIPS32 Architecture for Programmers Vol2:
+	 * "an exception or interrupt also clears both execution and
+	 *  instruction hazards"
+	 *
+	 * This implies that the exception will see the status register
+	 * with the IE bit clear and EXL bit set. Since the EXL bit is set
+	 * the COP_0_EXC_PC will not be updated and the return from exception
+	 * will be to an incorrect address.
+	 */
+	mfc0	v0, COP_0_STATUS_REG
+	and     v0, ~(SR_INT_ENAB)	# disable interrupts
+	mtc0	v0, COP_0_STATUS_REG
+	COP0_SYNC
+
+	or	v0, SR_EXL		# set exception level bit
+	mtc0	v0, COP_0_STATUS_REG
+	COP0_SYNC
 
-	mfc0	v0, COP_0_STATUS_REG	# set exeption level bit.
-	or	v0, SR_EXL
-	and     v0, ~(SR_INT_ENAB)
-	mtc0	v0, COP_0_STATUS_REG	# set exeption level bit.
-	nop
-	nop
-	nop
-	nop
+	/*
+	 * Since 'k1' is trashed during exception processing its use is safe
+	 * only after the point where we are sure not to be interrupted.
+	 */
 	.set	noat
-	move	k1, a1	
+	GET_CPU_PCPU(k1)
+	lw	k1, PC_CURPCB(k1)
 	RESTORE_U_PCB_REG(t0, MULLO, k1)
 	RESTORE_U_PCB_REG(t1, MULHI, k1)
 	mtlo	t0