[RFC PATCH v4 1/2] MIPS: move mmips branch support code out of the fpu emu code

[RFC PATCH v4 1/2] MIPS: move mmips branch support code out of the fpu emu code

[Manuel Lauss]

moves mm_isBranchInstr() from math-emu code to branch code.  Required
for a follow-on patch to disable the math-emu code completely.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
v4: first release

 arch/mips/include/asm/branch.h       |   2 +
 arch/mips/include/asm/fpu_emulator.h |   2 -
 arch/mips/kernel/branch.c            | 200 +++++++++++++++++++++++++++++++++++
 arch/mips/math-emu/cp1emu.c          | 198 ----------------------------------
 4 files changed, 202 insertions(+), 200 deletions(-)

diff --git a/arch/mips/include/asm/branch.h b/arch/mips/include/asm/branch.h
index e28a3e0..1c6b5c2 100644
--- a/arch/mips/include/asm/branch.h
+++ b/arch/mips/include/asm/branch.h
@@ -17,6 +17,8 @@ extern int __compute_return_epc_for_insn(struct pt_regs *regs,
 					 union mips_instruction insn);
 extern int __microMIPS_compute_return_epc(struct pt_regs *regs);
 extern int __MIPS16e_compute_return_epc(struct pt_regs *regs);
+int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
+		     unsigned long *contpc);
 
 
 static inline int delay_slot(struct pt_regs *regs)
diff --git a/arch/mips/include/asm/fpu_emulator.h b/arch/mips/include/asm/fpu_emulator.h
index 2abb587..283e6f3 100644
--- a/arch/mips/include/asm/fpu_emulator.h
+++ b/arch/mips/include/asm/fpu_emulator.h
@@ -58,8 +58,6 @@ extern int fpu_emulator_cop1Handler(struct pt_regs *xcp,
 				    struct mips_fpu_struct *ctx, int has_fpu,
 				    void *__user *fault_addr);
 int process_fpemu_return(int sig, void __user *fault_addr);
-int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
-		     unsigned long *contpc);
 
 /*
  * Instruction inserted following the badinst to further tag the sequence
diff --git a/arch/mips/kernel/branch.c b/arch/mips/kernel/branch.c
index 4d78bf4..47eadf0 100644
--- a/arch/mips/kernel/branch.c
+++ b/arch/mips/kernel/branch.c
@@ -19,6 +19,13 @@
 #include <asm/ptrace.h>
 #include <asm/uaccess.h>
 
+/* microMIPS bitfields */
+#define MM_POOL32A_MINOR_MASK	0x3f
+#define MM_POOL32A_MINOR_SHIFT	0x6
+
+/* (microMIPS) Convert 16-bit register encoding to 32-bit register encoding. */
+static const unsigned int reg16to32map[8] = {16, 17, 2, 3, 4, 5, 6, 7};
+
 /*
  * Calculate and return exception PC in case of branch delay slot
  * for microMIPS and MIPS16e. It does not clear the ISA mode bit.
@@ -48,6 +55,199 @@ int __isa_exception_epc(struct pt_regs *regs)
 	return epc;
 }
 
+int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
+		     unsigned long *contpc)
+{
+	union mips_instruction insn = (union mips_instruction)dec_insn.insn;
+	int bc_false = 0;
+	unsigned int fcr31;
+	unsigned int bit;
+
+	if (!cpu_has_mmips)
+		return 0;
+
+	switch (insn.mm_i_format.opcode) {
+	case mm_pool32a_op:
+		if ((insn.mm_i_format.simmediate & MM_POOL32A_MINOR_MASK) ==
+		    mm_pool32axf_op) {
+			switch (insn.mm_i_format.simmediate >>
+				MM_POOL32A_MINOR_SHIFT) {
+			case mm_jalr_op:
+			case mm_jalrhb_op:
+			case mm_jalrs_op:
+			case mm_jalrshb_op:
+				if (insn.mm_i_format.rt != 0)	/* Not mm_jr */
+					regs->regs[insn.mm_i_format.rt] =
+						regs->cp0_epc +
+						dec_insn.pc_inc +
+						dec_insn.next_pc_inc;
+				*contpc = regs->regs[insn.mm_i_format.rs];
+				return 1;
+			}
+		}
+		break;
+	case mm_pool32i_op:
+		switch (insn.mm_i_format.rt) {
+		case mm_bltzals_op:
+		case mm_bltzal_op:
+			regs->regs[31] = regs->cp0_epc +
+				dec_insn.pc_inc +
+				dec_insn.next_pc_inc;
+			/* Fall through */
+		case mm_bltz_op:
+			if ((long)regs->regs[insn.mm_i_format.rs] < 0)
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					(insn.mm_i_format.simmediate << 1);
+			else
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					dec_insn.next_pc_inc;
+			return 1;
+		case mm_bgezals_op:
+		case mm_bgezal_op:
+			regs->regs[31] = regs->cp0_epc +
+					dec_insn.pc_inc +
+					dec_insn.next_pc_inc;
+			/* Fall through */
+		case mm_bgez_op:
+			if ((long)regs->regs[insn.mm_i_format.rs] >= 0)
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					(insn.mm_i_format.simmediate << 1);
+			else
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					dec_insn.next_pc_inc;
+			return 1;
+		case mm_blez_op:
+			if ((long)regs->regs[insn.mm_i_format.rs] <= 0)
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					(insn.mm_i_format.simmediate << 1);
+			else
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					dec_insn.next_pc_inc;
+			return 1;
+		case mm_bgtz_op:
+			if ((long)regs->regs[insn.mm_i_format.rs] <= 0)
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					(insn.mm_i_format.simmediate << 1);
+			else
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					dec_insn.next_pc_inc;
+			return 1;
+		case mm_bc2f_op:
+		case mm_bc1f_op:
+			bc_false = 1;
+			/* Fall through */
+		case mm_bc2t_op:
+		case mm_bc1t_op:
+			preempt_disable();
+			if (is_fpu_owner())
+				asm volatile("cfc1\t%0,$31" : "=r" (fcr31));
+			else
+				fcr31 = current->thread.fpu.fcr31;
+			preempt_enable();
+
+			if (bc_false)
+				fcr31 = ~fcr31;
+
+			bit = (insn.mm_i_format.rs >> 2);
+			bit += (bit != 0);
+			bit += 23;
+			if (fcr31 & (1 << bit))
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc +
+					(insn.mm_i_format.simmediate << 1);
+			else
+				*contpc = regs->cp0_epc +
+					dec_insn.pc_inc + dec_insn.next_pc_inc;
+			return 1;
+		}
+		break;
+	case mm_pool16c_op:
+		switch (insn.mm_i_format.rt) {
+		case mm_jalr16_op:
+		case mm_jalrs16_op:
+			regs->regs[31] = regs->cp0_epc +
+				dec_insn.pc_inc + dec_insn.next_pc_inc;
+			/* Fall through */
+		case mm_jr16_op:
+			*contpc = regs->regs[insn.mm_i_format.rs];
+			return 1;
+		}
+		break;
+	case mm_beqz16_op:
+		if ((long)regs->regs[reg16to32map[insn.mm_b1_format.rs]] == 0)
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc +
+				(insn.mm_b1_format.simmediate << 1);
+		else
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc + dec_insn.next_pc_inc;
+		return 1;
+	case mm_bnez16_op:
+		if ((long)regs->regs[reg16to32map[insn.mm_b1_format.rs]] != 0)
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc +
+				(insn.mm_b1_format.simmediate << 1);
+		else
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc + dec_insn.next_pc_inc;
+		return 1;
+	case mm_b16_op:
+		*contpc = regs->cp0_epc + dec_insn.pc_inc +
+			 (insn.mm_b0_format.simmediate << 1);
+		return 1;
+	case mm_beq32_op:
+		if (regs->regs[insn.mm_i_format.rs] ==
+		    regs->regs[insn.mm_i_format.rt])
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc +
+				(insn.mm_i_format.simmediate << 1);
+		else
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc +
+				dec_insn.next_pc_inc;
+		return 1;
+	case mm_bne32_op:
+		if (regs->regs[insn.mm_i_format.rs] !=
+		    regs->regs[insn.mm_i_format.rt])
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc +
+				(insn.mm_i_format.simmediate << 1);
+		else
+			*contpc = regs->cp0_epc +
+				dec_insn.pc_inc + dec_insn.next_pc_inc;
+		return 1;
+	case mm_jalx32_op:
+		regs->regs[31] = regs->cp0_epc +
+			dec_insn.pc_inc + dec_insn.next_pc_inc;
+		*contpc = regs->cp0_epc + dec_insn.pc_inc;
+		*contpc >>= 28;
+		*contpc <<= 28;
+		*contpc |= (insn.j_format.target << 2);
+		return 1;
+	case mm_jals32_op:
+	case mm_jal32_op:
+		regs->regs[31] = regs->cp0_epc +
+			dec_insn.pc_inc + dec_insn.next_pc_inc;
+		/* Fall through */
+	case mm_j32_op:
+		*contpc = regs->cp0_epc + dec_insn.pc_inc;
+		*contpc >>= 27;
+		*contpc <<= 27;
+		*contpc |= (insn.j_format.target << 1);
+		set_isa16_mode(*contpc);
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * Compute return address and emulate branch in microMIPS mode after an
  * exception only. It does not handle compact branches/jumps and cannot
diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c
index 7b3c9ac..8f36146 100644
--- a/arch/mips/math-emu/cp1emu.c
+++ b/arch/mips/math-emu/cp1emu.c
@@ -83,8 +83,6 @@ DEFINE_PER_CPU(struct mips_fpu_emulator_stats, fpuemustats);
 #define modeindex(v) ((v) & FPU_CSR_RM)
 
 /* microMIPS bitfields */
-#define MM_POOL32A_MINOR_MASK	0x3f
-#define MM_POOL32A_MINOR_SHIFT	0x6
 #define MM_MIPS32_COND_FC	0x30
 
 /* Convert Mips rounding mode (0..3) to IEEE library modes. */
@@ -116,9 +114,6 @@ static const unsigned int fpucondbit[8] = {
 };
 #endif
 
-/* (microMIPS) Convert 16-bit register encoding to 32-bit register encoding. */
-static const unsigned int reg16to32map[8] = {16, 17, 2, 3, 4, 5, 6, 7};
-
 /* (microMIPS) Convert certain microMIPS instructions to MIPS32 format. */
 static const int sd_format[] = {16, 17, 0, 0, 0, 0, 0, 0};
 static const int sdps_format[] = {16, 17, 22, 0, 0, 0, 0, 0};
@@ -466,199 +461,6 @@ static int microMIPS32_to_MIPS32(union mips_instruction *insn_ptr)
 	return 0;
 }
 
-int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
-		     unsigned long *contpc)
-{
-	union mips_instruction insn = (union mips_instruction)dec_insn.insn;
-	int bc_false = 0;
-	unsigned int fcr31;
-	unsigned int bit;
-
-	if (!cpu_has_mmips)
-		return 0;
-
-	switch (insn.mm_i_format.opcode) {
-	case mm_pool32a_op:
-		if ((insn.mm_i_format.simmediate & MM_POOL32A_MINOR_MASK) ==
-		    mm_pool32axf_op) {
-			switch (insn.mm_i_format.simmediate >>
-				MM_POOL32A_MINOR_SHIFT) {
-			case mm_jalr_op:
-			case mm_jalrhb_op:
-			case mm_jalrs_op:
-			case mm_jalrshb_op:
-				if (insn.mm_i_format.rt != 0)	/* Not mm_jr */
-					regs->regs[insn.mm_i_format.rt] =
-						regs->cp0_epc +
-						dec_insn.pc_inc +
-						dec_insn.next_pc_inc;
-				*contpc = regs->regs[insn.mm_i_format.rs];
-				return 1;
-			}
-		}
-		break;
-	case mm_pool32i_op:
-		switch (insn.mm_i_format.rt) {
-		case mm_bltzals_op:
-		case mm_bltzal_op:
-			regs->regs[31] = regs->cp0_epc +
-				dec_insn.pc_inc +
-				dec_insn.next_pc_inc;
-			/* Fall through */
-		case mm_bltz_op:
-			if ((long)regs->regs[insn.mm_i_format.rs] < 0)
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					(insn.mm_i_format.simmediate << 1);
-			else
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					dec_insn.next_pc_inc;
-			return 1;
-		case mm_bgezals_op:
-		case mm_bgezal_op:
-			regs->regs[31] = regs->cp0_epc +
-					dec_insn.pc_inc +
-					dec_insn.next_pc_inc;
-			/* Fall through */
-		case mm_bgez_op:
-			if ((long)regs->regs[insn.mm_i_format.rs] >= 0)
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					(insn.mm_i_format.simmediate << 1);
-			else
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					dec_insn.next_pc_inc;
-			return 1;
-		case mm_blez_op:
-			if ((long)regs->regs[insn.mm_i_format.rs] <= 0)
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					(insn.mm_i_format.simmediate << 1);
-			else
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					dec_insn.next_pc_inc;
-			return 1;
-		case mm_bgtz_op:
-			if ((long)regs->regs[insn.mm_i_format.rs] <= 0)
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					(insn.mm_i_format.simmediate << 1);
-			else
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					dec_insn.next_pc_inc;
-			return 1;
-		case mm_bc2f_op:
-		case mm_bc1f_op:
-			bc_false = 1;
-			/* Fall through */
-		case mm_bc2t_op:
-		case mm_bc1t_op:
-			preempt_disable();
-			if (is_fpu_owner())
-				asm volatile("cfc1\t%0,$31" : "=r" (fcr31));
-			else
-				fcr31 = current->thread.fpu.fcr31;
-			preempt_enable();
-
-			if (bc_false)
-				fcr31 = ~fcr31;
-
-			bit = (insn.mm_i_format.rs >> 2);
-			bit += (bit != 0);
-			bit += 23;
-			if (fcr31 & (1 << bit))
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc +
-					(insn.mm_i_format.simmediate << 1);
-			else
-				*contpc = regs->cp0_epc +
-					dec_insn.pc_inc + dec_insn.next_pc_inc;
-			return 1;
-		}
-		break;
-	case mm_pool16c_op:
-		switch (insn.mm_i_format.rt) {
-		case mm_jalr16_op:
-		case mm_jalrs16_op:
-			regs->regs[31] = regs->cp0_epc +
-				dec_insn.pc_inc + dec_insn.next_pc_inc;
-			/* Fall through */
-		case mm_jr16_op:
-			*contpc = regs->regs[insn.mm_i_format.rs];
-			return 1;
-		}
-		break;
-	case mm_beqz16_op:
-		if ((long)regs->regs[reg16to32map[insn.mm_b1_format.rs]] == 0)
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc +
-				(insn.mm_b1_format.simmediate << 1);
-		else
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc + dec_insn.next_pc_inc;
-		return 1;
-	case mm_bnez16_op:
-		if ((long)regs->regs[reg16to32map[insn.mm_b1_format.rs]] != 0)
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc +
-				(insn.mm_b1_format.simmediate << 1);
-		else
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc + dec_insn.next_pc_inc;
-		return 1;
-	case mm_b16_op:
-		*contpc = regs->cp0_epc + dec_insn.pc_inc +
-			 (insn.mm_b0_format.simmediate << 1);
-		return 1;
-	case mm_beq32_op:
-		if (regs->regs[insn.mm_i_format.rs] ==
-		    regs->regs[insn.mm_i_format.rt])
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc +
-				(insn.mm_i_format.simmediate << 1);
-		else
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc +
-				dec_insn.next_pc_inc;
-		return 1;
-	case mm_bne32_op:
-		if (regs->regs[insn.mm_i_format.rs] !=
-		    regs->regs[insn.mm_i_format.rt])
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc +
-				(insn.mm_i_format.simmediate << 1);
-		else
-			*contpc = regs->cp0_epc +
-				dec_insn.pc_inc + dec_insn.next_pc_inc;
-		return 1;
-	case mm_jalx32_op:
-		regs->regs[31] = regs->cp0_epc +
-			dec_insn.pc_inc + dec_insn.next_pc_inc;
-		*contpc = regs->cp0_epc + dec_insn.pc_inc;
-		*contpc >>= 28;
-		*contpc <<= 28;
-		*contpc |= (insn.j_format.target << 2);
-		return 1;
-	case mm_jals32_op:
-	case mm_jal32_op:
-		regs->regs[31] = regs->cp0_epc +
-			dec_insn.pc_inc + dec_insn.next_pc_inc;
-		/* Fall through */
-	case mm_j32_op:
-		*contpc = regs->cp0_epc + dec_insn.pc_inc;
-		*contpc >>= 27;
-		*contpc <<= 27;
-		*contpc |= (insn.j_format.target << 1);
-		set_isa16_mode(*contpc);
-		return 1;
-	}
-	return 0;
-}
-
 /*
  * Redundant with logic already in kernel/branch.c,
  * embedded in compute_return_epc.  At some point,
-- 
1.9.1

[RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Manuel Lauss]

This small patch makes the MIPS FPU emulator optional. The kernel
kills float-users on systems without a hardware FPU by sending a SIGILL.

Disabling the emulator shrinks vmlinux by about 54kBytes (32bit,
optimizing for size).

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
v4: rediffed because of patch 1/2, should now work with micromips as well
v3: updated patch description with size savings.
v2: incorporated changes suggested by Jonas Gorski
    force the fpu emulator on for micromips: relocating the parts
    of the mmips code in the emulator to other areas would be a
    much larger change; I went the cheap route instead with this.

 arch/mips/Kbuild                     |  2 +-
 arch/mips/Kconfig                    | 14 ++++++++++++++
 arch/mips/include/asm/fpu.h          |  5 +++--
 arch/mips/include/asm/fpu_emulator.h | 15 +++++++++++++++
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/arch/mips/Kbuild b/arch/mips/Kbuild
index d2cfe45..426c264 100644
--- a/arch/mips/Kbuild
+++ b/arch/mips/Kbuild
@@ -16,7 +16,7 @@ obj- := $(platform-)
 
 obj-y += kernel/
 obj-y += mm/
-obj-y += math-emu/
+obj-$(CONFIG_MIPS_FPU_EMULATOR) += math-emu/
 
 ifdef CONFIG_KVM
 obj-y += kvm/
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 9b53358..3924396 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2482,6 +2482,20 @@ config MIPS_O32_FP64_SUPPORT
 
 	  If unsure, say N.
 
+config MIPS_FPU_EMULATOR
+	bool "MIPS FPU Emulator"
+	default y
+	help
+	  This option lets you disable the built-in MIPS FPU (Coprocessor 1)
+	  emulator, which handles floating-point instructions on processors
+	  without a hardware FPU.  It is generally a good idea to keep the
+	  emulator built-in, unless you are perfectly sure you have a
+	  complete soft-float environment.  With the emulator disabled, all
+	  users of float operations will be killed with an illegal instr-
+	  uction exception.
+
+	  Say Y, please.
+
 config USE_OF
 	bool
 	select OF
diff --git a/arch/mips/include/asm/fpu.h b/arch/mips/include/asm/fpu.h
index 4d86b72..c5203bb 100644
--- a/arch/mips/include/asm/fpu.h
+++ b/arch/mips/include/asm/fpu.h
@@ -160,9 +160,10 @@ static inline int init_fpu(void)
 		ret = __own_fpu();
 		if (!ret)
 			_init_fpu();
-	} else {
+	} else if (IS_ENABLED(CONFIG_MIPS_FPU_EMULATOR))
 		fpu_emulator_init_fpu();
-	}
+	else
+		ret = SIGILL;
 
 	preempt_enable();
 	return ret;
diff --git a/arch/mips/include/asm/fpu_emulator.h b/arch/mips/include/asm/fpu_emulator.h
index 283e6f3..df0ca0c 100644
--- a/arch/mips/include/asm/fpu_emulator.h
+++ b/arch/mips/include/asm/fpu_emulator.h
@@ -27,6 +27,7 @@
 #include <asm/inst.h>
 #include <asm/local.h>
 
+#ifdef CONFIG_MIPS_FPU_EMULATOR
 #ifdef CONFIG_DEBUG_FS
 
 struct mips_fpu_emulator_stats {
@@ -57,6 +58,20 @@ extern int do_dsemulret(struct pt_regs *xcp);
 extern int fpu_emulator_cop1Handler(struct pt_regs *xcp,
 				    struct mips_fpu_struct *ctx, int has_fpu,
 				    void *__user *fault_addr);
+#else	/* no CONFIG_MIPS_FPU_EMULATOR */
+static inline int do_dsemulret(struct pt_regs *xcp)
+{
+	return 0;	/* 0 means error, should never get here anyway */
+}
+
+static inline int fpu_emulator_cop1Handler(struct pt_regs *xcp,
+				struct mips_fpu_struct *ctx, int has_fpu,
+				void *__user *fault_addr)
+{
+	return SIGILL;	/* we don't speak MIPS FPU */
+}
+#endif	/* CONFIG_MIPS_FPU_EMULATOR */
+
 int process_fpemu_return(int sig, void __user *fault_addr);
 
 /*
-- 
1.9.1

Re: [RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 4605 bytes --]

On Mon, Apr 07, 2014 at 12:57:04PM +0200, Manuel Lauss wrote:
> This small patch makes the MIPS FPU emulator optional. The kernel
> kills float-users on systems without a hardware FPU by sending a SIGILL.

One issue with this is that if someone runs a kernel with the FPU
emulator disabled on hardware that has an FPU, they're likely to hit
seemingly odd behaviour where FP works just fine until they hit a
condition the hardware doesn't support. To make it clear that using FP
without the emulator is a bad idea, perhaps it would be safer to disable
FP entirely rather than only the emulator? Then userland can die the
first time it uses FP instead of when it happens to operate on a
denormal.

Unless there are FPUs which never generate an unimplemented operation
exception, in which case perhaps more Kconfig is needed to identify such
systems & allow the emulator to be disabled for those only.

Thanks,
    Paul

> 
> Disabling the emulator shrinks vmlinux by about 54kBytes (32bit,
> optimizing for size).
> 
> Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
> ---
> v4: rediffed because of patch 1/2, should now work with micromips as well
> v3: updated patch description with size savings.
> v2: incorporated changes suggested by Jonas Gorski
>     force the fpu emulator on for micromips: relocating the parts
>     of the mmips code in the emulator to other areas would be a
>     much larger change; I went the cheap route instead with this.
> 
>  arch/mips/Kbuild                     |  2 +-
>  arch/mips/Kconfig                    | 14 ++++++++++++++
>  arch/mips/include/asm/fpu.h          |  5 +++--
>  arch/mips/include/asm/fpu_emulator.h | 15 +++++++++++++++
>  4 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/Kbuild b/arch/mips/Kbuild
> index d2cfe45..426c264 100644
> --- a/arch/mips/Kbuild
> +++ b/arch/mips/Kbuild
> @@ -16,7 +16,7 @@ obj- := $(platform-)
>  
>  obj-y += kernel/
>  obj-y += mm/
> -obj-y += math-emu/
> +obj-$(CONFIG_MIPS_FPU_EMULATOR) += math-emu/
>  
>  ifdef CONFIG_KVM
>  obj-y += kvm/
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 9b53358..3924396 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2482,6 +2482,20 @@ config MIPS_O32_FP64_SUPPORT
>  
>  	  If unsure, say N.
>  
> +config MIPS_FPU_EMULATOR
> +	bool "MIPS FPU Emulator"
> +	default y
> +	help
> +	  This option lets you disable the built-in MIPS FPU (Coprocessor 1)
> +	  emulator, which handles floating-point instructions on processors
> +	  without a hardware FPU.  It is generally a good idea to keep the
> +	  emulator built-in, unless you are perfectly sure you have a
> +	  complete soft-float environment.  With the emulator disabled, all
> +	  users of float operations will be killed with an illegal instr-
> +	  uction exception.
> +
> +	  Say Y, please.
> +
>  config USE_OF
>  	bool
>  	select OF
> diff --git a/arch/mips/include/asm/fpu.h b/arch/mips/include/asm/fpu.h
> index 4d86b72..c5203bb 100644
> --- a/arch/mips/include/asm/fpu.h
> +++ b/arch/mips/include/asm/fpu.h
> @@ -160,9 +160,10 @@ static inline int init_fpu(void)
>  		ret = __own_fpu();
>  		if (!ret)
>  			_init_fpu();
> -	} else {
> +	} else if (IS_ENABLED(CONFIG_MIPS_FPU_EMULATOR))
>  		fpu_emulator_init_fpu();
> -	}
> +	else
> +		ret = SIGILL;
>  
>  	preempt_enable();
>  	return ret;
> diff --git a/arch/mips/include/asm/fpu_emulator.h b/arch/mips/include/asm/fpu_emulator.h
> index 283e6f3..df0ca0c 100644
> --- a/arch/mips/include/asm/fpu_emulator.h
> +++ b/arch/mips/include/asm/fpu_emulator.h
> @@ -27,6 +27,7 @@
>  #include <asm/inst.h>
>  #include <asm/local.h>
>  
> +#ifdef CONFIG_MIPS_FPU_EMULATOR
>  #ifdef CONFIG_DEBUG_FS
>  
>  struct mips_fpu_emulator_stats {
> @@ -57,6 +58,20 @@ extern int do_dsemulret(struct pt_regs *xcp);
>  extern int fpu_emulator_cop1Handler(struct pt_regs *xcp,
>  				    struct mips_fpu_struct *ctx, int has_fpu,
>  				    void *__user *fault_addr);
> +#else	/* no CONFIG_MIPS_FPU_EMULATOR */
> +static inline int do_dsemulret(struct pt_regs *xcp)
> +{
> +	return 0;	/* 0 means error, should never get here anyway */
> +}
> +
> +static inline int fpu_emulator_cop1Handler(struct pt_regs *xcp,
> +				struct mips_fpu_struct *ctx, int has_fpu,
> +				void *__user *fault_addr)
> +{
> +	return SIGILL;	/* we don't speak MIPS FPU */
> +}
> +#endif	/* CONFIG_MIPS_FPU_EMULATOR */
> +
>  int process_fpemu_return(int sig, void __user *fault_addr);
>  
>  /*
> -- 
> 1.9.1
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Manuel Lauss]

On Mon, Apr 7, 2014 at 3:53 PM, Paul Burton <paul.burton@imgtec.com> wrote:
> On Mon, Apr 07, 2014 at 12:57:04PM +0200, Manuel Lauss wrote:
>> This small patch makes the MIPS FPU emulator optional. The kernel
>> kills float-users on systems without a hardware FPU by sending a SIGILL.
>
> One issue with this is that if someone runs a kernel with the FPU
> emulator disabled on hardware that has an FPU, they're likely to hit
> seemingly odd behaviour where FP works just fine until they hit a
> condition the hardware doesn't support. To make it clear that using FP
> without the emulator is a bad idea, perhaps it would be safer to disable
> FP entirely rather than only the emulator? Then userland can die the
> first time it uses FP instead of when it happens to operate on a
> denormal.

Very good point, I understand.
How about this addon-patch?  I don't want to sprinkle the whole codebase
with #ifdef MIPS_FPU_SUPPORT lines.
Untested, since I don't have any hardware with FPU.


> Unless there are FPUs which never generate an unimplemented operation
> exception, in which case perhaps more Kconfig is needed to identify such
> systems & allow the emulator to be disabled for those only.

I'd rather keep the simple patch I sent, and maybe hide the option behind
CONFIG_EXPERT so only people who want to save space and know what
they're doing can disable it (e.g. OpenWRT).


diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 3924396..52de5b8 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2482,19 +2482,16 @@ config MIPS_O32_FP64_SUPPORT

          If unsure, say N.

-config MIPS_FPU_EMULATOR
-       bool "MIPS FPU Emulator"
+config MIPS_FPU_SUPPORT
+       bool "MIPS FPU Support"
        default y
        help
-         This option lets you disable the built-in MIPS FPU (Coprocessor 1)
-         emulator, which handles floating-point instructions on processors
-         without a hardware FPU.  It is generally a good idea to keep the
-         emulator built-in, unless you are perfectly sure you have a
-         complete soft-float environment.  With the emulator disabled, all
-         users of float operations will be killed with an illegal instr-
-         uction exception.
+         Enable support for floating point math, be it hardware FPU or the
+         kernels' FPU emulator.  With this option disabled, any user of
+         float math will be killed by illegal instruction exception,
+         regardless of the availability of hardware floating point support.

-         Say Y, please.
+         Say Y, unless you have a pure soft-float userspace.

 config USE_OF
        bool
diff --git a/arch/mips/include/asm/fpu.h b/arch/mips/include/asm/fpu.h
index c5203bb..ed68719 100644
--- a/arch/mips/include/asm/fpu.h
+++ b/arch/mips/include/asm/fpu.h
@@ -156,13 +156,14 @@ static inline int init_fpu(void)
        int ret = 0;

        preempt_disable();
-       if (cpu_has_fpu) {
-               ret = __own_fpu();
-               if (!ret)
-                       _init_fpu();
-       } else if (IS_ENABLED(CONFIG_MIPS_FPU_EMULATOR))
-               fpu_emulator_init_fpu();
-       else
+       if (IS_ENABLED(CONFIG_MIPS_FPU_SUPPORT)) {
+               if (cpu_has_fpu) {
+                       ret = __own_fpu();
+                       if (!ret)
+                               _init_fpu();
+               } else
+                       fpu_emulator_init_fpu();
+       } else
                ret = SIGILL;

        preempt_enable();


Thanks,
       Mano

Re: [RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 4011 bytes --]

On Mon, Apr 07, 2014 at 04:48:58PM +0200, Manuel Lauss wrote:
> On Mon, Apr 7, 2014 at 3:53 PM, Paul Burton <paul.burton@imgtec.com> wrote:
> > On Mon, Apr 07, 2014 at 12:57:04PM +0200, Manuel Lauss wrote:
> >> This small patch makes the MIPS FPU emulator optional. The kernel
> >> kills float-users on systems without a hardware FPU by sending a SIGILL.
> >
> > One issue with this is that if someone runs a kernel with the FPU
> > emulator disabled on hardware that has an FPU, they're likely to hit
> > seemingly odd behaviour where FP works just fine until they hit a
> > condition the hardware doesn't support. To make it clear that using FP
> > without the emulator is a bad idea, perhaps it would be safer to disable
> > FP entirely rather than only the emulator? Then userland can die the
> > first time it uses FP instead of when it happens to operate on a
> > denormal.
> 
> Very good point, I understand.
> How about this addon-patch?  I don't want to sprinkle the whole codebase
> with #ifdef MIPS_FPU_SUPPORT lines.
> Untested, since I don't have any hardware with FPU.
> 

Failing init_fpu seems reasonable to me, though you could probably just
return before the preempt_disable & avoid that bit of overhead.

Thanks,
    Paul

> 
> > Unless there are FPUs which never generate an unimplemented operation
> > exception, in which case perhaps more Kconfig is needed to identify such
> > systems & allow the emulator to be disabled for those only.
> 
> I'd rather keep the simple patch I sent, and maybe hide the option behind
> CONFIG_EXPERT so only people who want to save space and know what
> they're doing can disable it (e.g. OpenWRT).
> 
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 3924396..52de5b8 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2482,19 +2482,16 @@ config MIPS_O32_FP64_SUPPORT
> 
>           If unsure, say N.
> 
> -config MIPS_FPU_EMULATOR
> -       bool "MIPS FPU Emulator"
> +config MIPS_FPU_SUPPORT
> +       bool "MIPS FPU Support"
>         default y
>         help
> -         This option lets you disable the built-in MIPS FPU (Coprocessor 1)
> -         emulator, which handles floating-point instructions on processors
> -         without a hardware FPU.  It is generally a good idea to keep the
> -         emulator built-in, unless you are perfectly sure you have a
> -         complete soft-float environment.  With the emulator disabled, all
> -         users of float operations will be killed with an illegal instr-
> -         uction exception.
> +         Enable support for floating point math, be it hardware FPU or the
> +         kernels' FPU emulator.  With this option disabled, any user of
> +         float math will be killed by illegal instruction exception,
> +         regardless of the availability of hardware floating point support.
> 
> -         Say Y, please.
> +         Say Y, unless you have a pure soft-float userspace.
> 
>  config USE_OF
>         bool
> diff --git a/arch/mips/include/asm/fpu.h b/arch/mips/include/asm/fpu.h
> index c5203bb..ed68719 100644
> --- a/arch/mips/include/asm/fpu.h
> +++ b/arch/mips/include/asm/fpu.h
> @@ -156,13 +156,14 @@ static inline int init_fpu(void)
>         int ret = 0;
> 
>         preempt_disable();
> -       if (cpu_has_fpu) {
> -               ret = __own_fpu();
> -               if (!ret)
> -                       _init_fpu();
> -       } else if (IS_ENABLED(CONFIG_MIPS_FPU_EMULATOR))
> -               fpu_emulator_init_fpu();
> -       else
> +       if (IS_ENABLED(CONFIG_MIPS_FPU_SUPPORT)) {
> +               if (cpu_has_fpu) {
> +                       ret = __own_fpu();
> +                       if (!ret)
> +                               _init_fpu();
> +               } else
> +                       fpu_emulator_init_fpu();
> +       } else
>                 ret = SIGILL;
> 
>         preempt_enable();
> 
> 
> Thanks,
>        Mano

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Ralf Baechle]

On Mon, Apr 07, 2014 at 02:53:15PM +0100, Paul Burton wrote:

> On Mon, Apr 07, 2014 at 12:57:04PM +0200, Manuel Lauss wrote:
> > This small patch makes the MIPS FPU emulator optional. The kernel
> > kills float-users on systems without a hardware FPU by sending a SIGILL.
> 
> One issue with this is that if someone runs a kernel with the FPU
> emulator disabled on hardware that has an FPU, they're likely to hit
> seemingly odd behaviour where FP works just fine until they hit a
> condition the hardware doesn't support. To make it clear that using FP
> without the emulator is a bad idea, perhaps it would be safer to disable
> FP entirely rather than only the emulator? Then userland can die the
> first time it uses FP instead of when it happens to operate on a
> denormal.
> 
> Unless there are FPUs which never generate an unimplemented operation
> exception, in which case perhaps more Kconfig is needed to identify such
> systems & allow the emulator to be disabled for those only.

The original reason for me to remove the FPU emulator option was that I
was getting flooded by bogus bug reports because users thought they could
remove the FPU emulator with a hardware FPU present.

Another pain point is that soft-FPU establishes another ABI variant so
I'd not mind to see soft-FPU go.

Some of the tradeoffs involved were a bit bogus at times.  Soft-fp
application code can be much bigger than hard-fp - to the point where the
50k or so for the kernel FP software are a good investment.

But I don't mind making the FPU emulator selectable again - but this
time with nasty kernel messages and killing of processes that happen to
dare to execute a FPU instruction.

  Ralf

Re: [RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Florian Fainelli]

2014-04-07 9:28 GMT-07:00 Ralf Baechle <ralf@linux-mips.org>:
> On Mon, Apr 07, 2014 at 02:53:15PM +0100, Paul Burton wrote:
>
>> On Mon, Apr 07, 2014 at 12:57:04PM +0200, Manuel Lauss wrote:
>> > This small patch makes the MIPS FPU emulator optional. The kernel
>> > kills float-users on systems without a hardware FPU by sending a SIGILL.
>>
>> One issue with this is that if someone runs a kernel with the FPU
>> emulator disabled on hardware that has an FPU, they're likely to hit
>> seemingly odd behaviour where FP works just fine until they hit a
>> condition the hardware doesn't support. To make it clear that using FP
>> without the emulator is a bad idea, perhaps it would be safer to disable
>> FP entirely rather than only the emulator? Then userland can die the
>> first time it uses FP instead of when it happens to operate on a
>> denormal.
>>
>> Unless there are FPUs which never generate an unimplemented operation
>> exception, in which case perhaps more Kconfig is needed to identify such
>> systems & allow the emulator to be disabled for those only.
>
> The original reason for me to remove the FPU emulator option was that I
> was getting flooded by bogus bug reports because users thought they could
> remove the FPU emulator with a hardware FPU present.

We have had the same problems in OpenWrt, but we now output a big fat
warning to say "hey your kernel does not have FPU".

>
> Another pain point is that soft-FPU establishes another ABI variant so
> I'd not mind to see soft-FPU go.
>
> Some of the tradeoffs involved were a bit bogus at times.  Soft-fp
> application code can be much bigger than hard-fp - to the point where the
> 50k or so for the kernel FP software are a good investment.

There are some braindead bootloaders out there that will only allow
for a small kernel partition, while the rootfs partition can virtually
span the remainder of the available flash space... and all sorts of
crazy designs like these.

>
> But I don't mind making the FPU emulator selectable again - but this
> time with nasty kernel messages and killing of processes that happen to
> dare to execute a FPU instruction.

I think it is good to offer this as an option (hint: just like what
ARM does) along with the necessary warnings/messages to make sure that
bug filters can be easily filtered out.
-- 
Florian

Re: [RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Ralf Baechle]

On Mon, Apr 07, 2014 at 09:37:42AM -0700, Florian Fainelli wrote:

> There are some braindead bootloaders out there that will only allow
> for a small kernel partition, while the rootfs partition can virtually
> span the remainder of the available flash space... and all sorts of
> crazy designs like these.

In a case like that 50k are going to buy you a release or two of Moore's
law applied to kernel bloat.

How making the FPU emu a module?

  Ralf

Re: [RFC PATCH v4 2/2] MIPS: make FPU emulator optional

[Florian Fainelli]

2014-04-07 11:02 GMT-07:00 Ralf Baechle <ralf@linux-mips.org>:
> On Mon, Apr 07, 2014 at 09:37:42AM -0700, Florian Fainelli wrote:
>
>> There are some braindead bootloaders out there that will only allow
>> for a small kernel partition, while the rootfs partition can virtually
>> span the remainder of the available flash space... and all sorts of
>> crazy designs like these.
>
> In a case like that 50k are going to buy you a release or two of Moore's
> law applied to kernel bloat.
>
> How making the FPU emu a module?

busybox's init uses the FPU for some operations (can't remember the
details), which sort of creates a chicken and egg problem here.
-- 
Florian