Re: [PATCH] mips/math-emu: fix emulation of the prefx instruction

[Thomas Bogendoerfer <tsbogend@alpha.franken.de>]

On Sun, Jan 05, 2025 at 10:18:06PM +0100, Mateusz Jończyk wrote:
> Currently, installation of Debian 12.8 for mipsel fails on machines
> without an FPU [1]. This is caused by the fact that zstd (which is used
> for initramfs compression) executes the prefx instruction, which is not
> emulated properly by the kernel.
> 
> The prefx (Prefetch Indexed) instruction fetches data from memory into
> the cache without any side effects. Though functionally unrelated, it
> requires an FPU [2].
> 
> Bytecode format of this instruction ends on "001111" binary:
> 
> 	(prefx instruction format) & 0x0000003f = 0x0000000f
> 
> The code in fpux_emu() runs like so:
> 
> 	#define MIPSInst(x) x
> 	#define MIPSInst_FMA_FFMT(x) (MIPSInst(x) & 0x00000007)
> 	#define MIPSInst_FUNC(x) (MIPSInst(x) & 0x0000003f)
> 	enum cop1x_func { ..., pfetch_op = 0x0f, ... };
> 
> 	...
> 
> 	switch (MIPSInst_FMA_FFMT(ir)) {
> 	...
> 
> 	case 0x3:
> 		if (MIPSInst_FUNC(ir) != pfetch_op)
> 			return SIGILL;
> 
> 		/* ignore prefx operation */
> 		break;
> 
> 	default:
> 		return SIGILL;
> 	}
> 
> That snippet above contains a logic error and the
> 	if (MIPSInst_FUNC(ir) != pfetch_op)
> comparison always fires.
> 
> When MIPSInst_FUNC(ir) is equal to pfetch_op, ir must end on 001111
> binary. In this case, MIPSInst_FMA_FFMT(ir) must be equal to 0x7, which
> does not match that case label.
> 
> This causes emulation failure for the prefx instruction. Fix it.
> 
> This has been broken by
> commit 919af8b96c89 ("MIPS: Make definitions of MIPSInst_FMA_{FUNC,FMTM} consistent with MIPS64 manual")
> which modified the MIPSInst_FMA_FFMT macro without updating the users.
> 
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: stable@vger.kernel.org # after 3 weeks
> Cc: Dengcheng Zhu <dzhu@wavecomp.com>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Ming Wang <wangming01@loongson.cn>
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Fixes: 919af8b96c89 ("MIPS: Make definitions of MIPSInst_FMA_{FUNC,FMTM} consistent with MIPS64 manual")
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1091858
> [2] MIPS Architecture For Programmers Volume II-A: The MIPS32 Instruction Set
> 
> ---
> 
> Tested in QEMU for mipsel and mips64el.
> ---
>  arch/mips/math-emu/cp1emu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c
> index 265bc57819df..c89e70df43d8 100644
> --- a/arch/mips/math-emu/cp1emu.c
> +++ b/arch/mips/math-emu/cp1emu.c
> @@ -1660,7 +1660,7 @@ static int fpux_emu(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
>  		break;
>  	}
>  
> -	case 0x3:
> +	case 0x7:
>  		if (MIPSInst_FUNC(ir) != pfetch_op)
>  			return SIGILL;
>  
> -- 
> 2.25.1

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]