linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] More FCSR handling fixes
@ 2016-10-28  7:19 Maciej W. Rozycki
  2016-10-28  7:19 ` Maciej W. Rozycki
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-10-28  7:19 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: Paul Burton, James Hogan, linux-mips, linux-kernel

Hi,

 Here are some further fixes to our FCSR handling.  Just 2 changes at this 
time.

 The first, very small one, closes an issue where a write made with 
ptrace(PTRACE_POKEUSR, ...) to FCSR does not mark the FP context as used.  
This is the legacy interface, seldom used these days, having been largely 
replaced with ptrace(PTRACE_SETFPREGS, ...), so the problem may have 
escaped easily.  Fixed now.

 The second, larger one, addresses the handling of the Cause bits, by 
letting them remain set in some cases, making their semantics more 
consistent with what hardware does when undisturbed; a SIGFPE signal is 
sent if appropriate where previously it was missed.

 This change is not final as in some cases, specifically in the FP context 
of SIGFPE's signal frame, active Cause bits that match Enable bits will 
still be recorded as clear even though they were originally set, being the 
reason for the signal.  Consequently the signal will not retrigger if the 
handler returns with the FP context unchanged in the signal frame.  This 
is unlike with other signals triggered by a hardware exception which do 
require a corrective action if a return is intended rather than an escape 
via `longjmp' or suchlike and which is what one would expect here as well.

 I plan to address this final issue with a further change in the future, 
however I have not started this effort yet and I don't have a schedule 
set.  Meanwhile this change is I believe a step in the right direction.

 Details of both changes have been provided with individual patch 
descriptions.

 While making these changes I have noticed a bunch of bitrot issues we 
have with the handling of the FP context in signal frames with MIPS I-II 
processors.  I will submit corrections as a separate patch series.

 Questions or comments are welcome, as usually, and otherwise please queue
for the next merge cycle.

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 0/2] More FCSR handling fixes
  2016-10-28  7:19 [PATCH 0/2] More FCSR handling fixes Maciej W. Rozycki
@ 2016-10-28  7:19 ` Maciej W. Rozycki
  2016-10-28  7:20 ` [PATCH 1/2] MIPS: ptrace: Also initialize the FP context on individual FCSR writes Maciej W. Rozycki
  2016-10-28  7:21 ` [PATCH 2/2] MIPS: Fix FCSR Cause bit handling for correct SIGFPE issue Maciej W. Rozycki
  2 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-10-28  7:19 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: Paul Burton, James Hogan, linux-mips, linux-kernel

Hi,

 Here are some further fixes to our FCSR handling.  Just 2 changes at this 
time.

 The first, very small one, closes an issue where a write made with 
ptrace(PTRACE_POKEUSR, ...) to FCSR does not mark the FP context as used.  
This is the legacy interface, seldom used these days, having been largely 
replaced with ptrace(PTRACE_SETFPREGS, ...), so the problem may have 
escaped easily.  Fixed now.

 The second, larger one, addresses the handling of the Cause bits, by 
letting them remain set in some cases, making their semantics more 
consistent with what hardware does when undisturbed; a SIGFPE signal is 
sent if appropriate where previously it was missed.

 This change is not final as in some cases, specifically in the FP context 
of SIGFPE's signal frame, active Cause bits that match Enable bits will 
still be recorded as clear even though they were originally set, being the 
reason for the signal.  Consequently the signal will not retrigger if the 
handler returns with the FP context unchanged in the signal frame.  This 
is unlike with other signals triggered by a hardware exception which do 
require a corrective action if a return is intended rather than an escape 
via `longjmp' or suchlike and which is what one would expect here as well.

 I plan to address this final issue with a further change in the future, 
however I have not started this effort yet and I don't have a schedule 
set.  Meanwhile this change is I believe a step in the right direction.

 Details of both changes have been provided with individual patch 
descriptions.

 While making these changes I have noticed a bunch of bitrot issues we 
have with the handling of the FP context in signal frames with MIPS I-II 
processors.  I will submit corrections as a separate patch series.

 Questions or comments are welcome, as usually, and otherwise please queue
for the next merge cycle.

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] MIPS: ptrace: Also initialize the FP context on individual FCSR writes
  2016-10-28  7:19 [PATCH 0/2] More FCSR handling fixes Maciej W. Rozycki
  2016-10-28  7:19 ` Maciej W. Rozycki
@ 2016-10-28  7:20 ` Maciej W. Rozycki
  2016-10-28  7:20   ` Maciej W. Rozycki
  2016-10-28  7:21 ` [PATCH 2/2] MIPS: Fix FCSR Cause bit handling for correct SIGFPE issue Maciej W. Rozycki
  2 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-10-28  7:20 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: Paul Burton, James Hogan, linux-mips, linux-kernel

Complement commit ac9ad83bc318 ("MIPS: prevent FP context set via ptrace 
being discarded") and also initialize the FP context whenever FCSR alone 
is written with a PTRACE_POKEUSR request addressing FPC_CSR, rather than
along with the full FPU register set in the case of the PTRACE_SETFPREGS
request.

Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Hi,

 This is tricky to verify with modern user software as these days it all 
uses PTRACE_SETFPREGS.  I suppose I could tweak and rebuild `gdbserver' to 
disable modern code and let it use fallback legacy support still present 
there, but frankly I think the change is obviously correct.

 Please apply.

  Maciej

linux-mips-ptrace-fcsr-init-fp-ctx.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2016-10-22 01:29:44.000000000 +0100
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2016-10-22 01:44:38.740202000 +0100
@@ -817,6 +818,7 @@ long arch_ptrace(struct task_struct *chi
 			break;
 #endif
 		case FPC_CSR:
+			init_fp_ctx(child);
 			ptrace_setfcr31(child, data);
 			break;
 		case DSP_BASE ... DSP_BASE + 5: {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] MIPS: ptrace: Also initialize the FP context on individual FCSR writes
  2016-10-28  7:20 ` [PATCH 1/2] MIPS: ptrace: Also initialize the FP context on individual FCSR writes Maciej W. Rozycki
@ 2016-10-28  7:20   ` Maciej W. Rozycki
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-10-28  7:20 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: Paul Burton, James Hogan, linux-mips, linux-kernel

Complement commit ac9ad83bc318 ("MIPS: prevent FP context set via ptrace 
being discarded") and also initialize the FP context whenever FCSR alone 
is written with a PTRACE_POKEUSR request addressing FPC_CSR, rather than
along with the full FPU register set in the case of the PTRACE_SETFPREGS
request.

Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Hi,

 This is tricky to verify with modern user software as these days it all 
uses PTRACE_SETFPREGS.  I suppose I could tweak and rebuild `gdbserver' to 
disable modern code and let it use fallback legacy support still present 
there, but frankly I think the change is obviously correct.

 Please apply.

  Maciej

linux-mips-ptrace-fcsr-init-fp-ctx.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2016-10-22 01:29:44.000000000 +0100
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2016-10-22 01:44:38.740202000 +0100
@@ -817,6 +818,7 @@ long arch_ptrace(struct task_struct *chi
 			break;
 #endif
 		case FPC_CSR:
+			init_fp_ctx(child);
 			ptrace_setfcr31(child, data);
 			break;
 		case DSP_BASE ... DSP_BASE + 5: {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] MIPS: Fix FCSR Cause bit handling for correct SIGFPE issue
  2016-10-28  7:19 [PATCH 0/2] More FCSR handling fixes Maciej W. Rozycki
  2016-10-28  7:19 ` Maciej W. Rozycki
  2016-10-28  7:20 ` [PATCH 1/2] MIPS: ptrace: Also initialize the FP context on individual FCSR writes Maciej W. Rozycki
@ 2016-10-28  7:21 ` Maciej W. Rozycki
  2016-10-28  7:21   ` Maciej W. Rozycki
  2 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-10-28  7:21 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: Paul Burton, James Hogan, linux-mips, linux-kernel

Sanitize FCSR Cause bit handling, following a trail of past attempts:

* commit 4249548454f7 ("MIPS: ptrace: Fix FP context restoration FCSR 
regression"),

* commit 443c44032a54 ("MIPS: Always clear FCSR cause bits after 
emulation"),

* commit 64bedffe4968 ("MIPS: Clear [MSA]FPE CSR.Cause after 
notify_die()"),

* commit b1442d39fac2 ("MIPS: Prevent user from setting FCSR cause 
bits"),

* commit b54d2901517d ("Properly handle branch delay slots in connection 
with signals.").

Specifically do not mask these bits out in ptrace(2) processing and send 
a SIGFPE signal instead whenever a matching pair of an FCSR Cause and 
Enable bit is seen as execution of an affected context is about to 
resume.  Only then clear Cause bits, and even then do not clear any bits 
that are set but masked with the respective Enable bits.  Adjust Cause 
bit clearing throughout code likewise, except within the FPU emulator 
proper where they are set according to IEEE 754 exceptions raised as the 
operation emulated executed.  Do so so that any IEEE 754 exceptions 
subject to their default handling are recorded like with operations 
executed by FPU hardware.

Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Hi,

 This for a change I actually verified, by poking at FCSR with GDB, via 
`gdbserver'.  With this change you can now set any Cause bits and at the 
resumption of the debuggee it will receive a SIGFPE with the right code, 
according to the priority set in `force_fcr31_sig'.  Any transient FCSR 
state does not matter of course.  The R6 emulation bits have only been 
verified by building an R6 configuration though.  They are consistent 
with the rest of the changes so they should be good.  Please apply.

  Maciej

linux-mips-ptrace-fcsr-set-cause.diff
Index: linux-sfr-test/arch/mips/include/asm/fpu_emulator.h
===================================================================
--- linux-sfr-test.orig/arch/mips/include/asm/fpu_emulator.h	2016-10-22 07:10:07.271290000 +0100
+++ linux-sfr-test/arch/mips/include/asm/fpu_emulator.h	2016-10-22 07:10:47.019621000 +0100
@@ -63,6 +63,8 @@ do {									\
 extern int fpu_emulator_cop1Handler(struct pt_regs *xcp,
 				    struct mips_fpu_struct *ctx, int has_fpu,
 				    void *__user *fault_addr);
+void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr,
+		     struct task_struct *tsk);
 int process_fpemu_return(int sig, void __user *fault_addr,
 			 unsigned long fcr31);
 int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
@@ -81,4 +83,15 @@ static inline void fpu_emulator_init_fpu
 		set_fpr64(&t->thread.fpu.fpr[i], 0, SIGNALLING_NAN);
 }
 
+/*
+ * Mask the FCSR Cause bits according to the Enable bits, observing
+ * that Unimplemented is always enabled.
+ */
+static inline unsigned long mask_fcr31_x(unsigned long fcr31)
+{
+	return fcr31 & (FPU_CSR_UNI_X |
+			((fcr31 & FPU_CSR_ALL_E) <<
+			 (ffs(FPU_CSR_ALL_X) - ffs(FPU_CSR_ALL_E))));
+}
+
 #endif /* _ASM_FPU_EMULATOR_H */
Index: linux-sfr-test/arch/mips/include/asm/switch_to.h
===================================================================
--- linux-sfr-test.orig/arch/mips/include/asm/switch_to.h	2016-10-22 07:10:07.275287000 +0100
+++ linux-sfr-test/arch/mips/include/asm/switch_to.h	2016-10-22 07:10:47.037633000 +0100
@@ -76,6 +76,22 @@ do {	if (cpu_has_rw_llb) {						\
 } while (0)
 
 /*
+ * Check FCSR for any unmasked exceptions pending set with `ptrace',
+ * clear them and send a signal.
+ */
+#define __sanitize_fcr31(next)						\
+do {									\
+	unsigned long fcr31 = mask_fcr31_x(next->thread.fpu.fcr31);	\
+	void __user *pc;						\
+									\
+	if (unlikely(fcr31)) {						\
+		pc = (void __user *)task_pt_regs(next)->cp0_epc;	\
+		next->thread.fpu.fcr31 &= ~fcr31;			\
+		force_fcr31_sig(fcr31, pc, next);			\
+	}								\
+} while (0)
+
+/*
  * For newly created kernel threads switch_to() will return to
  * ret_from_kernel_thread, newly created user threads to ret_from_fork.
  * That is, everything following resume() will be skipped for new threads.
@@ -85,6 +101,8 @@ do {	if (cpu_has_rw_llb) {						\
 do {									\
 	__mips_mt_fpaff_switch_to(prev);				\
 	lose_fpu_inatomic(1, prev);					\
+	if (tsk_used_math(next))					\
+		__sanitize_fcr31(next);					\
 	if (cpu_has_dsp) {						\
 		__save_dsp(prev);					\
 		__restore_dsp(next);					\
Index: linux-sfr-test/arch/mips/kernel/mips-r2-to-r6-emul.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/mips-r2-to-r6-emul.c	2016-10-22 07:10:07.312291000 +0100
+++ linux-sfr-test/arch/mips/kernel/mips-r2-to-r6-emul.c	2016-10-22 07:10:47.065621000 +0100
@@ -899,7 +899,7 @@ static inline int mipsr2_find_op_func(st
  * mipsr2_decoder: Decode and emulate a MIPS R2 instruction
  * @regs: Process register set
  * @inst: Instruction to decode and emulate
- * @fcr31: Floating Point Control and Status Register returned
+ * @fcr31: Floating Point Control and Status Register Cause bits returned
  */
 int mipsr2_decoder(struct pt_regs *regs, u32 inst, unsigned long *fcr31)
 {
@@ -1172,13 +1172,13 @@ int mipsr2_decoder(struct pt_regs *regs,
 
 		err = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 0,
 					       &fault_addr);
-		*fcr31 = current->thread.fpu.fcr31;
 
 		/*
-		 * We can't allow the emulated instruction to leave any of
-		 * the cause bits set in $fcr31.
+		 * We can't allow the emulated instruction to leave any
+		 * enabled Cause bits set in $fcr31.
 		 */
-		current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+		*fcr31 = res = mask_fcr31_x(current->thread.fpu.fcr31);
+		current->thread.fpu.fcr31 &= ~res;
 
 		/*
 		 * this is a tricky issue - lose_fpu() uses LL/SC atomics
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2016-10-22 07:10:45.047625000 +0100
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2016-10-22 07:10:47.068623000 +0100
@@ -79,16 +79,15 @@ void ptrace_disable(struct task_struct *
 }
 
 /*
- * Poke at FCSR according to its mask.  Don't set the cause bits as
- * this is currently not handled correctly in FP context restoration
- * and will cause an oops if a corresponding enable bit is set.
+ * Poke at FCSR according to its mask.  Set the Cause bits even
+ * if a corresponding Enable bit is set.  This will be noticed at
+ * the time the thread is switched to and SIGFPE thrown accordingly.
  */
 static void ptrace_setfcr31(struct task_struct *child, u32 value)
 {
 	u32 fcr31;
 	u32 mask;
 
-	value &= ~FPU_CSR_ALL_X;
 	fcr31 = child->thread.fpu.fcr31;
 	mask = boot_cpu_data.fpu_msk31;
 	child->thread.fpu.fcr31 = (value & ~mask) | (fcr31 & mask);
Index: linux-sfr-test/arch/mips/kernel/traps.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/traps.c	2016-10-22 07:10:07.375283000 +0100
+++ linux-sfr-test/arch/mips/kernel/traps.c	2016-10-22 07:10:47.076623000 +0100
@@ -705,6 +705,32 @@ asmlinkage void do_ov(struct pt_regs *re
 	exception_exit(prev_state);
 }
 
+/*
+ * Send SIGFPE according to FCSR Cause bits, which must have already
+ * been masked against Enable bits.  This is impotant as Inexact can
+ * happen together with Overflow or Underflow, and `ptrace' can set
+ * any bits.
+ */
+void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr,
+		     struct task_struct *tsk)
+{
+	struct siginfo si = { .si_addr = fault_addr, .si_signo = SIGFPE };
+
+	if (fcr31 & FPU_CSR_INV_X)
+		si.si_code = FPE_FLTINV;
+	else if (fcr31 & FPU_CSR_DIV_X)
+		si.si_code = FPE_FLTDIV;
+	else if (fcr31 & FPU_CSR_OVF_X)
+		si.si_code = FPE_FLTOVF;
+	else if (fcr31 & FPU_CSR_UDF_X)
+		si.si_code = FPE_FLTUND;
+	else if (fcr31 & FPU_CSR_INE_X)
+		si.si_code = FPE_FLTRES;
+	else
+		si.si_code = __SI_FAULT;
+	force_sig_info(SIGFPE, &si, tsk);
+}
+
 int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31)
 {
 	struct siginfo si = { 0 };
@@ -715,27 +741,7 @@ int process_fpemu_return(int sig, void _
 		return 0;
 
 	case SIGFPE:
-		si.si_addr = fault_addr;
-		si.si_signo = sig;
-		/*
-		 * Inexact can happen together with Overflow or Underflow.
-		 * Respect the mask to deliver the correct exception.
-		 */
-		fcr31 &= (fcr31 & FPU_CSR_ALL_E) <<
-			 (ffs(FPU_CSR_ALL_X) - ffs(FPU_CSR_ALL_E));
-		if (fcr31 & FPU_CSR_INV_X)
-			si.si_code = FPE_FLTINV;
-		else if (fcr31 & FPU_CSR_DIV_X)
-			si.si_code = FPE_FLTDIV;
-		else if (fcr31 & FPU_CSR_OVF_X)
-			si.si_code = FPE_FLTOVF;
-		else if (fcr31 & FPU_CSR_UDF_X)
-			si.si_code = FPE_FLTUND;
-		else if (fcr31 & FPU_CSR_INE_X)
-			si.si_code = FPE_FLTRES;
-		else
-			si.si_code = __SI_FAULT;
-		force_sig_info(sig, &si, current);
+		force_fcr31_sig(fcr31, fault_addr, current);
 		return 1;
 
 	case SIGBUS:
@@ -799,13 +805,13 @@ static int simulate_fp(struct pt_regs *r
 	/* Run the emulator */
 	sig = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 1,
 				       &fault_addr);
-	fcr31 = current->thread.fpu.fcr31;
 
 	/*
-	 * We can't allow the emulated instruction to leave any of
-	 * the cause bits set in $fcr31.
+	 * We can't allow the emulated instruction to leave any
+	 * enabled Cause bits set in $fcr31.
 	 */
-	current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+	fcr31 = mask_fcr31_x(current->thread.fpu.fcr31);
+	current->thread.fpu.fcr31 &= ~fcr31;
 
 	/* Restore the hardware register state */
 	own_fpu(1);
@@ -831,7 +837,7 @@ asmlinkage void do_fpe(struct pt_regs *r
 		goto out;
 
 	/* Clear FCSR.Cause before enabling interrupts */
-	write_32bit_cp1_register(CP1_STATUS, fcr31 & ~FPU_CSR_ALL_X);
+	write_32bit_cp1_register(CP1_STATUS, fcr31 & ~mask_fcr31_x(fcr31));
 	local_irq_enable();
 
 	die_if_kernel("FP exception in kernel code", regs);
@@ -853,13 +859,13 @@ asmlinkage void do_fpe(struct pt_regs *r
 		/* Run the emulator */
 		sig = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 1,
 					       &fault_addr);
-		fcr31 = current->thread.fpu.fcr31;
 
 		/*
-		 * We can't allow the emulated instruction to leave any of
-		 * the cause bits set in $fcr31.
+		 * We can't allow the emulated instruction to leave any
+		 * enabled Cause bits set in $fcr31.
 		 */
-		current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+		fcr31 = mask_fcr31_x(current->thread.fpu.fcr31);
+		current->thread.fpu.fcr31 &= ~fcr31;
 
 		/* Restore the hardware register state */
 		own_fpu(1);	/* Using the FPU again.	 */
@@ -1424,13 +1430,13 @@ asmlinkage void do_cpu(struct pt_regs *r
 
 		sig = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 0,
 					       &fault_addr);
-		fcr31 = current->thread.fpu.fcr31;
 
 		/*
 		 * We can't allow the emulated instruction to leave
-		 * any of the cause bits set in $fcr31.
+		 * any enabled Cause bits set in $fcr31.
 		 */
-		current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+		fcr31 = mask_fcr31_x(current->thread.fpu.fcr31);
+		current->thread.fpu.fcr31 &= ~fcr31;
 
 		/* Send a signal if required.  */
 		if (!process_fpemu_return(sig, fault_addr, fcr31) && !err)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] MIPS: Fix FCSR Cause bit handling for correct SIGFPE issue
  2016-10-28  7:21 ` [PATCH 2/2] MIPS: Fix FCSR Cause bit handling for correct SIGFPE issue Maciej W. Rozycki
@ 2016-10-28  7:21   ` Maciej W. Rozycki
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-10-28  7:21 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: Paul Burton, James Hogan, linux-mips, linux-kernel

Sanitize FCSR Cause bit handling, following a trail of past attempts:

* commit 4249548454f7 ("MIPS: ptrace: Fix FP context restoration FCSR 
regression"),

* commit 443c44032a54 ("MIPS: Always clear FCSR cause bits after 
emulation"),

* commit 64bedffe4968 ("MIPS: Clear [MSA]FPE CSR.Cause after 
notify_die()"),

* commit b1442d39fac2 ("MIPS: Prevent user from setting FCSR cause 
bits"),

* commit b54d2901517d ("Properly handle branch delay slots in connection 
with signals.").

Specifically do not mask these bits out in ptrace(2) processing and send 
a SIGFPE signal instead whenever a matching pair of an FCSR Cause and 
Enable bit is seen as execution of an affected context is about to 
resume.  Only then clear Cause bits, and even then do not clear any bits 
that are set but masked with the respective Enable bits.  Adjust Cause 
bit clearing throughout code likewise, except within the FPU emulator 
proper where they are set according to IEEE 754 exceptions raised as the 
operation emulated executed.  Do so so that any IEEE 754 exceptions 
subject to their default handling are recorded like with operations 
executed by FPU hardware.

Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Hi,

 This for a change I actually verified, by poking at FCSR with GDB, via 
`gdbserver'.  With this change you can now set any Cause bits and at the 
resumption of the debuggee it will receive a SIGFPE with the right code, 
according to the priority set in `force_fcr31_sig'.  Any transient FCSR 
state does not matter of course.  The R6 emulation bits have only been 
verified by building an R6 configuration though.  They are consistent 
with the rest of the changes so they should be good.  Please apply.

  Maciej

linux-mips-ptrace-fcsr-set-cause.diff
Index: linux-sfr-test/arch/mips/include/asm/fpu_emulator.h
===================================================================
--- linux-sfr-test.orig/arch/mips/include/asm/fpu_emulator.h	2016-10-22 07:10:07.271290000 +0100
+++ linux-sfr-test/arch/mips/include/asm/fpu_emulator.h	2016-10-22 07:10:47.019621000 +0100
@@ -63,6 +63,8 @@ do {									\
 extern int fpu_emulator_cop1Handler(struct pt_regs *xcp,
 				    struct mips_fpu_struct *ctx, int has_fpu,
 				    void *__user *fault_addr);
+void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr,
+		     struct task_struct *tsk);
 int process_fpemu_return(int sig, void __user *fault_addr,
 			 unsigned long fcr31);
 int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
@@ -81,4 +83,15 @@ static inline void fpu_emulator_init_fpu
 		set_fpr64(&t->thread.fpu.fpr[i], 0, SIGNALLING_NAN);
 }
 
+/*
+ * Mask the FCSR Cause bits according to the Enable bits, observing
+ * that Unimplemented is always enabled.
+ */
+static inline unsigned long mask_fcr31_x(unsigned long fcr31)
+{
+	return fcr31 & (FPU_CSR_UNI_X |
+			((fcr31 & FPU_CSR_ALL_E) <<
+			 (ffs(FPU_CSR_ALL_X) - ffs(FPU_CSR_ALL_E))));
+}
+
 #endif /* _ASM_FPU_EMULATOR_H */
Index: linux-sfr-test/arch/mips/include/asm/switch_to.h
===================================================================
--- linux-sfr-test.orig/arch/mips/include/asm/switch_to.h	2016-10-22 07:10:07.275287000 +0100
+++ linux-sfr-test/arch/mips/include/asm/switch_to.h	2016-10-22 07:10:47.037633000 +0100
@@ -76,6 +76,22 @@ do {	if (cpu_has_rw_llb) {						\
 } while (0)
 
 /*
+ * Check FCSR for any unmasked exceptions pending set with `ptrace',
+ * clear them and send a signal.
+ */
+#define __sanitize_fcr31(next)						\
+do {									\
+	unsigned long fcr31 = mask_fcr31_x(next->thread.fpu.fcr31);	\
+	void __user *pc;						\
+									\
+	if (unlikely(fcr31)) {						\
+		pc = (void __user *)task_pt_regs(next)->cp0_epc;	\
+		next->thread.fpu.fcr31 &= ~fcr31;			\
+		force_fcr31_sig(fcr31, pc, next);			\
+	}								\
+} while (0)
+
+/*
  * For newly created kernel threads switch_to() will return to
  * ret_from_kernel_thread, newly created user threads to ret_from_fork.
  * That is, everything following resume() will be skipped for new threads.
@@ -85,6 +101,8 @@ do {	if (cpu_has_rw_llb) {						\
 do {									\
 	__mips_mt_fpaff_switch_to(prev);				\
 	lose_fpu_inatomic(1, prev);					\
+	if (tsk_used_math(next))					\
+		__sanitize_fcr31(next);					\
 	if (cpu_has_dsp) {						\
 		__save_dsp(prev);					\
 		__restore_dsp(next);					\
Index: linux-sfr-test/arch/mips/kernel/mips-r2-to-r6-emul.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/mips-r2-to-r6-emul.c	2016-10-22 07:10:07.312291000 +0100
+++ linux-sfr-test/arch/mips/kernel/mips-r2-to-r6-emul.c	2016-10-22 07:10:47.065621000 +0100
@@ -899,7 +899,7 @@ static inline int mipsr2_find_op_func(st
  * mipsr2_decoder: Decode and emulate a MIPS R2 instruction
  * @regs: Process register set
  * @inst: Instruction to decode and emulate
- * @fcr31: Floating Point Control and Status Register returned
+ * @fcr31: Floating Point Control and Status Register Cause bits returned
  */
 int mipsr2_decoder(struct pt_regs *regs, u32 inst, unsigned long *fcr31)
 {
@@ -1172,13 +1172,13 @@ int mipsr2_decoder(struct pt_regs *regs,
 
 		err = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 0,
 					       &fault_addr);
-		*fcr31 = current->thread.fpu.fcr31;
 
 		/*
-		 * We can't allow the emulated instruction to leave any of
-		 * the cause bits set in $fcr31.
+		 * We can't allow the emulated instruction to leave any
+		 * enabled Cause bits set in $fcr31.
 		 */
-		current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+		*fcr31 = res = mask_fcr31_x(current->thread.fpu.fcr31);
+		current->thread.fpu.fcr31 &= ~res;
 
 		/*
 		 * this is a tricky issue - lose_fpu() uses LL/SC atomics
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2016-10-22 07:10:45.047625000 +0100
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2016-10-22 07:10:47.068623000 +0100
@@ -79,16 +79,15 @@ void ptrace_disable(struct task_struct *
 }
 
 /*
- * Poke at FCSR according to its mask.  Don't set the cause bits as
- * this is currently not handled correctly in FP context restoration
- * and will cause an oops if a corresponding enable bit is set.
+ * Poke at FCSR according to its mask.  Set the Cause bits even
+ * if a corresponding Enable bit is set.  This will be noticed at
+ * the time the thread is switched to and SIGFPE thrown accordingly.
  */
 static void ptrace_setfcr31(struct task_struct *child, u32 value)
 {
 	u32 fcr31;
 	u32 mask;
 
-	value &= ~FPU_CSR_ALL_X;
 	fcr31 = child->thread.fpu.fcr31;
 	mask = boot_cpu_data.fpu_msk31;
 	child->thread.fpu.fcr31 = (value & ~mask) | (fcr31 & mask);
Index: linux-sfr-test/arch/mips/kernel/traps.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/traps.c	2016-10-22 07:10:07.375283000 +0100
+++ linux-sfr-test/arch/mips/kernel/traps.c	2016-10-22 07:10:47.076623000 +0100
@@ -705,6 +705,32 @@ asmlinkage void do_ov(struct pt_regs *re
 	exception_exit(prev_state);
 }
 
+/*
+ * Send SIGFPE according to FCSR Cause bits, which must have already
+ * been masked against Enable bits.  This is impotant as Inexact can
+ * happen together with Overflow or Underflow, and `ptrace' can set
+ * any bits.
+ */
+void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr,
+		     struct task_struct *tsk)
+{
+	struct siginfo si = { .si_addr = fault_addr, .si_signo = SIGFPE };
+
+	if (fcr31 & FPU_CSR_INV_X)
+		si.si_code = FPE_FLTINV;
+	else if (fcr31 & FPU_CSR_DIV_X)
+		si.si_code = FPE_FLTDIV;
+	else if (fcr31 & FPU_CSR_OVF_X)
+		si.si_code = FPE_FLTOVF;
+	else if (fcr31 & FPU_CSR_UDF_X)
+		si.si_code = FPE_FLTUND;
+	else if (fcr31 & FPU_CSR_INE_X)
+		si.si_code = FPE_FLTRES;
+	else
+		si.si_code = __SI_FAULT;
+	force_sig_info(SIGFPE, &si, tsk);
+}
+
 int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31)
 {
 	struct siginfo si = { 0 };
@@ -715,27 +741,7 @@ int process_fpemu_return(int sig, void _
 		return 0;
 
 	case SIGFPE:
-		si.si_addr = fault_addr;
-		si.si_signo = sig;
-		/*
-		 * Inexact can happen together with Overflow or Underflow.
-		 * Respect the mask to deliver the correct exception.
-		 */
-		fcr31 &= (fcr31 & FPU_CSR_ALL_E) <<
-			 (ffs(FPU_CSR_ALL_X) - ffs(FPU_CSR_ALL_E));
-		if (fcr31 & FPU_CSR_INV_X)
-			si.si_code = FPE_FLTINV;
-		else if (fcr31 & FPU_CSR_DIV_X)
-			si.si_code = FPE_FLTDIV;
-		else if (fcr31 & FPU_CSR_OVF_X)
-			si.si_code = FPE_FLTOVF;
-		else if (fcr31 & FPU_CSR_UDF_X)
-			si.si_code = FPE_FLTUND;
-		else if (fcr31 & FPU_CSR_INE_X)
-			si.si_code = FPE_FLTRES;
-		else
-			si.si_code = __SI_FAULT;
-		force_sig_info(sig, &si, current);
+		force_fcr31_sig(fcr31, fault_addr, current);
 		return 1;
 
 	case SIGBUS:
@@ -799,13 +805,13 @@ static int simulate_fp(struct pt_regs *r
 	/* Run the emulator */
 	sig = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 1,
 				       &fault_addr);
-	fcr31 = current->thread.fpu.fcr31;
 
 	/*
-	 * We can't allow the emulated instruction to leave any of
-	 * the cause bits set in $fcr31.
+	 * We can't allow the emulated instruction to leave any
+	 * enabled Cause bits set in $fcr31.
 	 */
-	current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+	fcr31 = mask_fcr31_x(current->thread.fpu.fcr31);
+	current->thread.fpu.fcr31 &= ~fcr31;
 
 	/* Restore the hardware register state */
 	own_fpu(1);
@@ -831,7 +837,7 @@ asmlinkage void do_fpe(struct pt_regs *r
 		goto out;
 
 	/* Clear FCSR.Cause before enabling interrupts */
-	write_32bit_cp1_register(CP1_STATUS, fcr31 & ~FPU_CSR_ALL_X);
+	write_32bit_cp1_register(CP1_STATUS, fcr31 & ~mask_fcr31_x(fcr31));
 	local_irq_enable();
 
 	die_if_kernel("FP exception in kernel code", regs);
@@ -853,13 +859,13 @@ asmlinkage void do_fpe(struct pt_regs *r
 		/* Run the emulator */
 		sig = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 1,
 					       &fault_addr);
-		fcr31 = current->thread.fpu.fcr31;
 
 		/*
-		 * We can't allow the emulated instruction to leave any of
-		 * the cause bits set in $fcr31.
+		 * We can't allow the emulated instruction to leave any
+		 * enabled Cause bits set in $fcr31.
 		 */
-		current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+		fcr31 = mask_fcr31_x(current->thread.fpu.fcr31);
+		current->thread.fpu.fcr31 &= ~fcr31;
 
 		/* Restore the hardware register state */
 		own_fpu(1);	/* Using the FPU again.	 */
@@ -1424,13 +1430,13 @@ asmlinkage void do_cpu(struct pt_regs *r
 
 		sig = fpu_emulator_cop1Handler(regs, &current->thread.fpu, 0,
 					       &fault_addr);
-		fcr31 = current->thread.fpu.fcr31;
 
 		/*
 		 * We can't allow the emulated instruction to leave
-		 * any of the cause bits set in $fcr31.
+		 * any enabled Cause bits set in $fcr31.
 		 */
-		current->thread.fpu.fcr31 &= ~FPU_CSR_ALL_X;
+		fcr31 = mask_fcr31_x(current->thread.fpu.fcr31);
+		current->thread.fpu.fcr31 &= ~fcr31;
 
 		/* Send a signal if required.  */
 		if (!process_fpemu_return(sig, fault_addr, fcr31) && !err)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-10-28  7:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-28  7:19 [PATCH 0/2] More FCSR handling fixes Maciej W. Rozycki
2016-10-28  7:19 ` Maciej W. Rozycki
2016-10-28  7:20 ` [PATCH 1/2] MIPS: ptrace: Also initialize the FP context on individual FCSR writes Maciej W. Rozycki
2016-10-28  7:20   ` Maciej W. Rozycki
2016-10-28  7:21 ` [PATCH 2/2] MIPS: Fix FCSR Cause bit handling for correct SIGFPE issue Maciej W. Rozycki
2016-10-28  7:21   ` Maciej W. Rozycki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).