diff mbox series

[5/6] Determine the desired FPU mode

Message ID 1540563667-23300-6-git-send-email-stefan.markovic@rt-rk.com
State New
Headers show
Series target/mips: Add support for prctl() PR_GET_FP_MODE and PR_SET_FP_MODE | expand

Commit Message

Stefan Markovic Oct. 26, 2018, 2:21 p.m. UTC 
From: Stefan Markovic <smarkovic@wavecomp.com>

Floating-point mode is calculated from MIPS.abiflags FP ABI value
(based on kernel implementation). Illegal combinations are rejected.

Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
---
 linux-user/mips/cpu_loop.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

Comments

Aleksandar Markovic Oct. 26, 2018, 4:06 p.m. UTC | #1 
> Subject: [PATCH 5/6] Determine the desired FPU mode
> 
> From: Stefan Markovic <smarkovic@wavecomp.com>
> 
> Floating-point mode is calculated from MIPS.abiflags FP ABI value
> (based on kernel implementation). Illegal combinations are rejected.
> 
> Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> ---

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Peter Maydell Oct. 26, 2018, 6:12 p.m. UTC | #2 
On 26 October 2018 at 15:21, Stefan Markovic <stefan.markovic@rt-rk.com> wrote:
> From: Stefan Markovic <smarkovic@wavecomp.com>
>
> Floating-point mode is calculated from MIPS.abiflags FP ABI value
> (based on kernel implementation). Illegal combinations are rejected.
>
> Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> ---
>  linux-user/mips/cpu_loop.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)

> +     if ((info->fp_abi > MAX_FP_ABI && info->fp_abi != MIPS_ABI_FP_UNKNOWN)
> +        || (info->interp_fp_abi > MAX_FP_ABI &&
> +            info->interp_fp_abi != MIPS_ABI_FP_UNKNOWN)) {
> +        fprintf(stderr, "qemu: Program and interpreter have "
> +                        "unexpected FPU modes\n");
> +        exit(137);

Why are we exit()ing with a funny exit status code here?

If this is a "can't happen" case, then we should assert(). If
it is a "can happen if fed an odd binary" case, then we should just
exit(1) as we do already in this function for an unsupported NaN mode.

> +    }
> +
> +    prog_req = (info->fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> +                                            : fpu_reqs[info->fp_abi];
> +    interp_req = (info->interp_fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> +                                            : fpu_reqs[info->interp_fp_abi];
> +
> +    prog_req.single &= interp_req.single;
> +    prog_req.soft &= interp_req.soft;
> +    prog_req.fr1 &= interp_req.fr1;
> +    prog_req.frdefault &= interp_req.frdefault;
> +    prog_req.fre &= interp_req.fre;
> +
> +    bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 ||
> +                              env->insn_flags & ISA_MIPS64R2 ||
> +                              env->insn_flags & ISA_MIPS32R6 ||
> +                              env->insn_flags & ISA_MIPS64R6;
> +
> +    if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) {
> +        env->CP0_Config5 |= (1 << CP0C5_FRE);
> +        if (env->active_fpu.fcr0 & (1 << FCR0_FREP)) {
> +            env->hflags |= MIPS_HFLAG_FRE;
> +        }
> +    } else if ((prog_req.fr1 && prog_req.frdefault) ||
> +         (prog_req.single && !prog_req.frdefault)) {
> +        if ((env->active_fpu.fcr0 & (1 << FCR0_F64)
> +            && cpu_has_mips_r2_r6) || prog_req.fr1) {
> +            env->CP0_Status |= (1 << CP0St_FR);
> +            env->hflags |= MIPS_HFLAG_F64;
> +        }
> +    } else  if (!prog_req.fre && !prog_req.frdefault &&
> +          !prog_req.fr1 && !prog_req.single && !prog_req.soft) {
> +        exit(137);
> +    }

Ditto here (and we haven't printed any error message here...)

thanks
-- PMM
Aleksandar Markovic Oct. 26, 2018, 7:10 p.m. UTC | #3 
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: Re: [Qemu-devel] [PATCH 5/6] Determine the desired FPU mode
> 
> On 26 October 2018 at 15:21, Stefan Markovic <stefan.markovic@rt-rk.com> wrote:
> > From: Stefan Markovic <smarkovic@wavecomp.com>
> >
> > Floating-point mode is calculated from MIPS.abiflags FP ABI value
> > (based on kernel implementation). Illegal combinations are rejected.
> >
> > Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> > ---
> >  linux-user/mips/cpu_loop.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 75 insertions(+)
> 
> > +     if ((info->fp_abi > MAX_FP_ABI && info->fp_abi != MIPS_ABI_FP_UNKNOWN)
> > +        || (info->interp_fp_abi > MAX_FP_ABI &&
> > +            info->interp_fp_abi != MIPS_ABI_FP_UNKNOWN)) {
> > +        fprintf(stderr, "qemu: Program and interpreter have "
> > +                        "unexpected FPU modes\n");
> > +        exit(137);
> 
> Why are we exit()ing with a funny exit status code here?
> 
> If this is a "can't happen" case, then we should assert(). If
> it is a "can happen if fed an odd binary" case, then we should just
> exit(1) as we do already in this function for an unsupported NaN mode.
> 

Thanks for the review.

This is a "can happen if fed an odd binary" case. Or, in other words, and more precisely, an executable compiled with one FP option attempts to load a library compiled with another, incompatible, FP option.

Kernel counterpart lines are:

https://elixir.bootlin.com/linux/v4.19/source/arch/mips/kernel/elf.c#L211
https://elixir.bootlin.com/linux/v4.19/source/arch/mips/kernel/elf.c#L263

I think the error code is important for MIPS loader to work as designed in such cases. Stefan should be best positioned to explain and analyze the cases, since he worked on verifying and fixing involved scenarios, not only from QEMU perspective. However, he will be back most likely only on Monday.

Thanks again,
Aleksandar

> > +    }
> > +
> > +    prog_req = (info->fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> > +                                            : fpu_reqs[info->fp_abi];
> > +    interp_req = (info->interp_fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> > +                                            : fpu_reqs[info->interp_fp_abi];
> > +
> > +    prog_req.single &= interp_req.single;
> > +    prog_req.soft &= interp_req.soft;
> > +    prog_req.fr1 &= interp_req.fr1;
> > +    prog_req.frdefault &= interp_req.frdefault;
> > +    prog_req.fre &= interp_req.fre;
> > +
> > +    bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 ||
> > +                              env->insn_flags & ISA_MIPS64R2 ||
> > +                              env->insn_flags & ISA_MIPS32R6 ||
> > +                              env->insn_flags & ISA_MIPS64R6;
> > +
> > +    if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) {
> > +        env->CP0_Config5 |= (1 << CP0C5_FRE);
> > +        if (env->active_fpu.fcr0 & (1 << FCR0_FREP)) {
> > +            env->hflags |= MIPS_HFLAG_FRE;
> > +        }
> > +    } else if ((prog_req.fr1 && prog_req.frdefault) ||
> > +         (prog_req.single && !prog_req.frdefault)) {
> > +        if ((env->active_fpu.fcr0 & (1 << FCR0_F64)
> > +            && cpu_has_mips_r2_r6) || prog_req.fr1) {
> > +            env->CP0_Status |= (1 << CP0St_FR);
> > +            env->hflags |= MIPS_HFLAG_F64;
> > +        }
> > +    } else  if (!prog_req.fre && !prog_req.frdefault &&
> > +          !prog_req.fr1 && !prog_req.single && !prog_req.soft) {
> > +        exit(137);
> > +    }
> 
> Ditto here (and we haven't printed any error message here...)
> 
> thanks
> -- PMM
>
Stefan Markovic Oct. 29, 2018, 1:15 p.m. UTC | #4 
exit() error codes are taken and left over from related kernel code. Will be set to 1 in next series version. Also, appropriate error messages printing will be added.

Regards,
Stefan

-------- Original Message --------
Subject: Re: [Qemu-devel] [PATCH 5/6] Determine the desired FPU mode
Date: Friday, October 26, 2018 20:12 CEST
From: Peter Maydell <peter.maydell@linaro.org>
To: Stefan Markovic <stefan.markovic@rt-rk.com>
CC: QEMU Developers <qemu-devel@nongnu.org>, Petar Jovanovic <pjovanovic@wavecomp.com>, Riku Voipio <riku.voipio@iki.fi>, Aleksandar Markovic <amarkovic@wavecomp.com>, Aurelien Jarno <aurelien@aurel32.net>, Laurent Vivier <laurent@vivier.eu>
References: <1540563667-23300-1-git-send-email-stefan.markovic@rt-rk.com> <1540563667-23300-6-git-send-email-stefan.markovic@rt-rk.com>


 On 26 October 2018 at 15:21, Stefan Markovic <stefan.markovic@rt-rk.com> wrote:
> From: Stefan Markovic <smarkovic@wavecomp.com>
>
> Floating-point mode is calculated from MIPS.abiflags FP ABI value
> (based on kernel implementation). Illegal combinations are rejected.
>
> Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> ---
> linux-user/mips/cpu_loop.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)

> + if ((info->fp_abi > MAX_FP_ABI && info->fp_abi != MIPS_ABI_FP_UNKNOWN)
> + || (info->interp_fp_abi > MAX_FP_ABI &&
> + info->interp_fp_abi != MIPS_ABI_FP_UNKNOWN)) {
> + fprintf(stderr, "qemu: Program and interpreter have "
> + "unexpected FPU modes\n");
> + exit(137);

Why are we exit()ing with a funny exit status code here?

If this is a "can't happen" case, then we should assert(). If
it is a "can happen if fed an odd binary" case, then we should just
exit(1) as we do already in this function for an unsupported NaN mode.

> + }
> +
> + prog_req = (info->fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> + : fpu_reqs[info->fp_abi];
> + interp_req = (info->interp_fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
> + : fpu_reqs[info->interp_fp_abi];
> +
> + prog_req.single &= interp_req.single;
> + prog_req.soft &= interp_req.soft;
> + prog_req.fr1 &= interp_req.fr1;
> + prog_req.frdefault &= interp_req.frdefault;
> + prog_req.fre &= interp_req.fre;
> +
> + bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 ||
> + env->insn_flags & ISA_MIPS64R2 ||
> + env->insn_flags & ISA_MIPS32R6 ||
> + env->insn_flags & ISA_MIPS64R6;
> +
> + if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) {
> + env->CP0_Config5 |= (1 << CP0C5_FRE);
> + if (env->active_fpu.fcr0 & (1 << FCR0_FREP)) {
> + env->hflags |= MIPS_HFLAG_FRE;
> + }
> + } else if ((prog_req.fr1 && prog_req.frdefault) ||
> + (prog_req.single && !prog_req.frdefault)) {
> + if ((env->active_fpu.fcr0 & (1 << FCR0_F64)
> + && cpu_has_mips_r2_r6) || prog_req.fr1) {
> + env->CP0_Status |= (1 << CP0St_FR);
> + env->hflags |= MIPS_HFLAG_F64;
> + }
> + } else if (!prog_req.fre && !prog_req.frdefault &&
> + !prog_req.fr1 && !prog_req.single && !prog_req.soft) {
> + exit(137);
> + }

Ditto here (and we haven't printed any error message here...)

thanks
-- PMM
diff mbox series

Patch

diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index c9c20cf..fd96e46 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -740,6 +740,34 @@  void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
     struct image_info *info = ts->info;
     int i;
 
+    struct mode_req {
+        bool single;
+        bool soft;
+        bool fr1;
+        bool frdefault;
+        bool fre;
+    };
+
+    static const struct mode_req fpu_reqs[] = {
+        [MIPS_ABI_FP_ANY]    = { true,  true,  true,  true,  true  },
+        [MIPS_ABI_FP_DOUBLE] = { false, false, false, true,  true  },
+        [MIPS_ABI_FP_SINGLE] = { true,  false, false, false, false },
+        [MIPS_ABI_FP_SOFT]   = { false, true,  false, false, false },
+        [MIPS_ABI_FP_OLD_64] = { false, false, false, false, false },
+        [MIPS_ABI_FP_XX]     = { false, false, true,  true,  true  },
+        [MIPS_ABI_FP_64]     = { false, false, true,  false, false },
+        [MIPS_ABI_FP_64A]    = { false, false, true,  false, true  }
+    };
+
+    /*
+     * Mode requirements when .MIPS.abiflags is not present in the ELF.
+     * Not present means that everything is acceptable except FR1.
+     */
+    static struct mode_req none_req = { true, true, false, true, true };
+
+    struct mode_req prog_req;
+    struct mode_req interp_req;
+
     for(i = 0; i < 32; i++) {
         env->active_tc.gpr[i] = regs->regs[i];
     }
@@ -747,6 +775,53 @@  void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
     if (regs->cp0_epc & 1) {
         env->hflags |= MIPS_HFLAG_M16;
     }
+
+#ifdef TARGET_ABI_MIPSO32
+# define MAX_FP_ABI MIPS_ABI_FP_64A
+#else
+# define MAX_FP_ABI MIPS_ABI_FP_SOFT
+#endif
+     if ((info->fp_abi > MAX_FP_ABI && info->fp_abi != MIPS_ABI_FP_UNKNOWN)
+        || (info->interp_fp_abi > MAX_FP_ABI &&
+            info->interp_fp_abi != MIPS_ABI_FP_UNKNOWN)) {
+        fprintf(stderr, "qemu: Program and interpreter have "
+                        "unexpected FPU modes\n");
+        exit(137);
+    }
+
+    prog_req = (info->fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
+                                            : fpu_reqs[info->fp_abi];
+    interp_req = (info->interp_fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req
+                                            : fpu_reqs[info->interp_fp_abi];
+
+    prog_req.single &= interp_req.single;
+    prog_req.soft &= interp_req.soft;
+    prog_req.fr1 &= interp_req.fr1;
+    prog_req.frdefault &= interp_req.frdefault;
+    prog_req.fre &= interp_req.fre;
+
+    bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 ||
+                              env->insn_flags & ISA_MIPS64R2 ||
+                              env->insn_flags & ISA_MIPS32R6 ||
+                              env->insn_flags & ISA_MIPS64R6;
+
+    if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) {
+        env->CP0_Config5 |= (1 << CP0C5_FRE);
+        if (env->active_fpu.fcr0 & (1 << FCR0_FREP)) {
+            env->hflags |= MIPS_HFLAG_FRE;
+        }
+    } else if ((prog_req.fr1 && prog_req.frdefault) ||
+         (prog_req.single && !prog_req.frdefault)) {
+        if ((env->active_fpu.fcr0 & (1 << FCR0_F64)
+            && cpu_has_mips_r2_r6) || prog_req.fr1) {
+            env->CP0_Status |= (1 << CP0St_FR);
+            env->hflags |= MIPS_HFLAG_F64;
+        }
+    } else  if (!prog_req.fre && !prog_req.frdefault &&
+          !prog_req.fr1 && !prog_req.single && !prog_req.soft) {
+        exit(137);
+    }
+
     if (env->insn_flags & ISA_NANOMIPS32) {
         return;
     }