All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shane McDonald <mcdonald.shane@gmail.com>
To: anemo@mba.ocn.ne.jp, kevink@paralogos.com,
	linux-mips@linux-mips.org, ralf@linux-mips.org,
	sshtylyov@mvista.com
Subject: [PATCH v3] MIPS FPU emulator: allow Cause bits of FCSR to be writeable by ctc1
Date: Thu, 06 May 2010 23:26:57 -0600	[thread overview]
Message-ID: <E1OAG5R-0003hc-AY@localhost> (raw)

In the FPU emulator code of the MIPS, the Cause bits of the FCSR
register are not currently writeable by the ctc1 instruction.
In odd corner cases, this can cause problems.  For example,
a case existed where a divide-by-zero exception was generated
by the FPU, and the signal handler attempted to restore the FPU
registers to their state before the exception occurred.  In this
particular setup, writing the old value to the FCSR register
would cause another divide-by-zero exception to occur immediately.
The solution is to change the ctc1 instruction emulator code to
allow the Cause bits of the FCSR register to be writeable.
This is the behaviour of the hardware that the code is emulating.

This problem was found by Shane McDonald, but the credit for the
fix goes to Kevin Kissell.  In Kevin's words:

I submit that the bug is indeed in that ctc_op:  case of the emulator.  The
Cause bits (17:12) are supposed to be writable by that instruction, but the
CTC1 emulation won't let them be updated by the instruction.  I think that
actually if you just completely removed lines 387-388 [...]
things would work a good deal better.  At least, it would be a more accurate
emulation of the architecturally defined FPU.  If I wanted to be really,
really pedantic (which I sometimes do), I'd also protect the reserved bits
that aren't necessarily writable.

Signed-off-by: Shane McDonald <mcdonald.shane@gmail.com>
---
v3: More coding style cleanups, and removed the 0x3 constant, replacing
it with FPU_CSR_RM and modeindex().

v2: Replaced an ugly magic number with a constant for the reserved
bits of the FPU CSR.

 arch/mips/include/asm/mipsregs.h |    9 ++++++++-
 arch/mips/math-emu/cp1emu.c      |   15 +++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 49382d5..c6e3c93 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -135,6 +135,12 @@
 #define FPU_CSR_COND7   0x80000000      /* $fcc7 */
 
 /*
+ * Bits 18 - 20 of the FPU Status Register will be read as 0,
+ * and should be written as zero.
+ */
+#define FPU_CSR_RSVD	0x001c0000
+
+/*
  * X the exception cause indicator
  * E the exception enable
  * S the sticky/flag bit
@@ -161,7 +167,8 @@
 #define FPU_CSR_UDF_S   0x00000008
 #define FPU_CSR_INE_S   0x00000004
 
-/* rounding mode */
+/* Bits 0 and 1 of FPU Status Register specify the rounding mode */
+#define FPU_CSR_RM	0x00000003
 #define FPU_CSR_RN      0x0     /* nearest */
 #define FPU_CSR_RZ      0x1     /* towards zero */
 #define FPU_CSR_RU      0x2     /* towards +Infinity */
diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c
index 8f2f8e9..f2338d1 100644
--- a/arch/mips/math-emu/cp1emu.c
+++ b/arch/mips/math-emu/cp1emu.c
@@ -78,6 +78,9 @@ DEFINE_PER_CPU(struct mips_fpu_emulator_stats, fpuemustats);
 #define FPCREG_RID	0	/* $0  = revision id */
 #define FPCREG_CSR	31	/* $31 = csr */
 
+/* Determine rounding mode from the RM bits of the FCSR */
+#define modeindex(v) ((v) & FPU_CSR_RM)
+
 /* Convert Mips rounding mode (0..3) to IEEE library modes. */
 static const unsigned char ieee_rm[4] = {
 	[FPU_CSR_RN] = IEEE754_RN,
@@ -384,10 +387,14 @@ static int cop1Emulate(struct pt_regs *xcp, struct mips_fpu_struct *ctx)
 					(void *) (xcp->cp0_epc),
 					MIPSInst_RT(ir), value);
 #endif
-				value &= (FPU_CSR_FLUSH | FPU_CSR_ALL_E | FPU_CSR_ALL_S | 0x03);
-				ctx->fcr31 &= ~(FPU_CSR_FLUSH | FPU_CSR_ALL_E | FPU_CSR_ALL_S | 0x03);
-				/* convert to ieee library modes */
-				ctx->fcr31 |= (value & ~0x3) | ieee_rm[value & 0x3];
+
+				/*
+				 * Don't write reserved bits,
+				 * and convert to ieee library modes
+				 */
+				ctx->fcr31 = (value &
+						~(FPU_CSR_RSVD | FPU_CSR_RM)) |
+						ieee_rm[modeindex(value)];
 			}
 			if ((ctx->fcr31 >> 5) & ctx->fcr31 & FPU_CSR_ALL_E) {
 				return SIGFPE;
-- 
1.6.2.4

             reply	other threads:[~2010-05-07  5:27 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-07  5:26 Shane McDonald [this message]
2010-05-07 13:33 ` [PATCH v3] MIPS FPU emulator: allow Cause bits of FCSR to be writeable by ctc1 Ralf Baechle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1OAG5R-0003hc-AY@localhost \
    --to=mcdonald.shane@gmail.com \
    --cc=anemo@mba.ocn.ne.jp \
    --cc=kevink@paralogos.com \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    --cc=sshtylyov@mvista.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.