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

[Paul Burton <paul.burton@imgtec.com>]

[-- 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 --]