Re: [PATCH v2] MIPS FPU emulator: allow Cause bits of FCSR to be writeable by ctc1

[Sergei Shtylyov <sshtylyov@mvista.com>]

Hello.

Shane McDonald wrote:

> 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>
> ---
> v2: Replaced an ugly magic number with a constant for the reserved
> bits of the FPU CSR.
>
>  arch/mips/include/asm/mipsregs.h |    6 ++++++
>  arch/mips/math-emu/cp1emu.c      |    9 +++++----
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
> index 49382d5..1b17a21 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
> diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c
> index 8f2f8e9..ebecec6 100644
> --- a/arch/mips/math-emu/cp1emu.c
> +++ b/arch/mips/math-emu/cp1emu.c
> @@ -384,10 +384,11 @@ 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 */
>   

   According to CodingStyle, the preferred style of multi-line comments is:

/*
 * bla
 * bla
 */

WBR, Sergei