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@xxxxxxxxxx> 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:
signature.asc
Description: Digital signature
