Re: [PATCH 1/2] MIPS: math-emu: Update debugfs FP exception stats for certain instructions

[James Hogan]

On Fri, Oct 06, 2017 at 07:29:00PM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <aleksandar.markovic@xxxxxxxxxx>
> 
> Fix omission of updating of debugfs FP exception stats for
> instructions <CLASS|MADDF|MSUBF|MAX|MIN|MAXA|MINA>.<D|S>.
> 
> CLASS.<D|S> can generate Unimplemented Operation FP exception.
> <MADDF|MSUBF|MAX|MIN|MAXA|MINA>>.<D|S> can generate Inexact,

nit: s/>>/>/

> Unimplemented Operation, Invalid Operation, Overflow, and
> Underflow FP exceptions. In such cases, and prior to this

Well, according to the manual I'm looking at MAX|MIN|MAXA|MINA can't
generate inexact, overflow, or underflow FP exceptions, and in practice
the only FPE generated by emulating these instructions is invalid
operation for MADDF/MSUBF. So presumably that's the only case we really
care about.

I.e. the MADDF/MSUBF invalid operation should be counted, but crucially
the setting of rcsr bits allows it to generate a SIGFPE which from a
glance it doesn't appear to do until this patch. The other changes are
redundant and harmless.

Does that sound correct? (appologies if I'm missing something, I'm just
reading the code, and reading FPU emulation code late at night is
probably asking for trouble).

Given the potential fixing of SIGFPE in that case should this be tagged
for stable?

Thanks
James


> patch, debugfs FP exception stats were not updated, and
> therefore contained overall wrong values.
> 
> This brings the emulation of mentioned instructions consistent
> with the previously implemented emulation of other related
> FPU instructions.
> 
> There is still some room for refactoring and improving the
> code segment under label "copcsr", but this is beyond the
> scope of this patch.
> 
> Fixes: 38db37ba069f ("MIPS: math-emu: Add support for the MIPS R6 CLASS FPU instruction")
> Fixes: e24c3bec3e8e ("MIPS: math-emu: Add support for the MIPS R6 MADDF FPU instruction")
> Fixes: 83d43305a1df ("MIPS: math-emu: Add support for the MIPS R6 MSUBF FPU instruction")
> Fixes: a79f5f9ba508 ("MIPS: math-emu: Add support for the MIPS R6 MAX{, A} FPU instruction")
> Fixes: 4e9561b20e2f ("MIPS: math-emu: Add support for the MIPS R6 MIN{, A} FPU instruction")
> 
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@xxxxxxxxxx>
> ---
>  arch/mips/math-emu/cp1emu.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c
> index 192542d..d2fcb30 100644
> --- a/arch/mips/math-emu/cp1emu.c
> +++ b/arch/mips/math-emu/cp1emu.c
> @@ -1795,7 +1795,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			SPFROMREG(fs, MIPSInst_FS(ir));
>  			SPFROMREG(fd, MIPSInst_FD(ir));
>  			rv.s = ieee754sp_maddf(fd, fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmsubf_op: {
> @@ -1809,7 +1809,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			SPFROMREG(fs, MIPSInst_FS(ir));
>  			SPFROMREG(fd, MIPSInst_FD(ir));
>  			rv.s = ieee754sp_msubf(fd, fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case frint_op: {
> @@ -1834,7 +1834,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			SPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.w = ieee754sp_2008class(fs);
>  			rfmt = w_fmt;
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmin_op: {
> @@ -1847,7 +1847,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			SPFROMREG(ft, MIPSInst_FT(ir));
>  			SPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.s = ieee754sp_fmin(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmina_op: {
> @@ -1860,7 +1860,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			SPFROMREG(ft, MIPSInst_FT(ir));
>  			SPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.s = ieee754sp_fmina(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmax_op: {
> @@ -1873,7 +1873,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			SPFROMREG(ft, MIPSInst_FT(ir));
>  			SPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.s = ieee754sp_fmax(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmaxa_op: {
> @@ -1886,7 +1886,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			SPFROMREG(ft, MIPSInst_FT(ir));
>  			SPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.s = ieee754sp_fmaxa(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fabs_op:
> @@ -2165,7 +2165,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			DPFROMREG(fs, MIPSInst_FS(ir));
>  			DPFROMREG(fd, MIPSInst_FD(ir));
>  			rv.d = ieee754dp_maddf(fd, fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmsubf_op: {
> @@ -2179,7 +2179,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			DPFROMREG(fs, MIPSInst_FS(ir));
>  			DPFROMREG(fd, MIPSInst_FD(ir));
>  			rv.d = ieee754dp_msubf(fd, fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case frint_op: {
> @@ -2204,7 +2204,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			DPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.l = ieee754dp_2008class(fs);
>  			rfmt = l_fmt;
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmin_op: {
> @@ -2217,7 +2217,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			DPFROMREG(ft, MIPSInst_FT(ir));
>  			DPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.d = ieee754dp_fmin(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmina_op: {
> @@ -2230,7 +2230,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			DPFROMREG(ft, MIPSInst_FT(ir));
>  			DPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.d = ieee754dp_fmina(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmax_op: {
> @@ -2243,7 +2243,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			DPFROMREG(ft, MIPSInst_FT(ir));
>  			DPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.d = ieee754dp_fmax(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fmaxa_op: {
> @@ -2256,7 +2256,7 @@ static int fpu_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  			DPFROMREG(ft, MIPSInst_FT(ir));
>  			DPFROMREG(fs, MIPSInst_FS(ir));
>  			rv.d = ieee754dp_fmaxa(fs, ft);
> -			break;
> +			goto copcsr;
>  		}
>  
>  		case fabs_op:
> -- 
> 2.7.4
>

Attachment: signature.asc
Description: Digital signature