[PATCH 0/4] linux-user/arm: fpa11 fix and cleanup

[PATCH 0/4] linux-user/arm: fpa11 fix and cleanup

[Richard Henderson]

The bug fix is patch 2, the rest is a bit of tidy-up.


r~

Richard Henderson (4):
  linux-user/arm: Split out emulate_arm_fpa11
  linux-user/arm: Do not emulate fpa11 in thumb mode
  linux-user/arm: Do not fill in si_code for fpa11 exceptions
  linux-user/arm: Simplify accumulating and raising fpa11 exceptions

 linux-user/arm/cpu_loop.c | 125 ++++++++++++++++++++------------------
 1 file changed, 66 insertions(+), 59 deletions(-)

-- 
2.25.1

[PATCH 1/4] linux-user/arm: Split out emulate_arm_fpa11

[Richard Henderson]

Pull out the fpa11 emulation to a helper function.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/arm/cpu_loop.c | 153 +++++++++++++++++++++++---------------
 1 file changed, 94 insertions(+), 59 deletions(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 989d03cd89..106909c7d8 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -224,6 +224,92 @@ static bool insn_is_linux_bkpt(uint32_t opcode, bool is_thumb)
     }
 }
 
+static bool emulate_arm_fpa11(CPUARMState *env, uint32_t opcode)
+{
+    TaskState *ts = env_cpu(env)->opaque;
+    int rc = EmulateAll(opcode, &ts->fpa, env);
+
+    if (rc == 0) {
+        /* Illegal instruction */
+        return false;
+    }
+    if (rc > 0) {
+        /* Everything ok. */
+        env->regs[15] += 4;
+        return true;
+    }
+
+    /* FP exception */
+    int arm_fpe = 0;
+
+    /* Translate softfloat flags to FPSR flags */
+    if (-rc & float_flag_invalid) {
+        arm_fpe |= BIT_IOC;
+    }
+    if (-rc & float_flag_divbyzero) {
+        arm_fpe |= BIT_DZC;
+    }
+    if (-rc & float_flag_overflow) {
+        arm_fpe |= BIT_OFC;
+    }
+    if (-rc & float_flag_underflow) {
+        arm_fpe |= BIT_UFC;
+    }
+    if (-rc & float_flag_inexact) {
+        arm_fpe |= BIT_IXC;
+    }
+
+    /* Exception enabled? */
+    FPSR fpsr = ts->fpa.fpsr;
+    if (fpsr & (arm_fpe << 16)) {
+        target_siginfo_t info;
+
+        info.si_signo = TARGET_SIGFPE;
+        info.si_errno = 0;
+
+        /* ordered by priority, least first */
+        if (arm_fpe & BIT_IXC) {
+            info.si_code = TARGET_FPE_FLTRES;
+        }
+        if (arm_fpe & BIT_UFC) {
+            info.si_code = TARGET_FPE_FLTUND;
+        }
+        if (arm_fpe & BIT_OFC) {
+            info.si_code = TARGET_FPE_FLTOVF;
+        }
+        if (arm_fpe & BIT_DZC) {
+            info.si_code = TARGET_FPE_FLTDIV;
+        }
+        if (arm_fpe & BIT_IOC) {
+            info.si_code = TARGET_FPE_FLTINV;
+        }
+
+        info._sifields._sigfault._addr = env->regs[15];
+        queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+    } else {
+        env->regs[15] += 4;
+    }
+
+    /* Accumulate unenabled exceptions */
+    if ((!(fpsr & BIT_IXE)) && (arm_fpe & BIT_IXC)) {
+        fpsr |= BIT_IXC;
+    }
+    if ((!(fpsr & BIT_UFE)) && (arm_fpe & BIT_UFC)) {
+        fpsr |= BIT_UFC;
+    }
+    if ((!(fpsr & BIT_OFE)) && (arm_fpe & BIT_OFC)) {
+        fpsr |= BIT_OFC;
+    }
+    if ((!(fpsr & BIT_DZE)) && (arm_fpe & BIT_DZC)) {
+        fpsr |= BIT_DZC;
+    }
+    if ((!(fpsr & BIT_IOE)) && (arm_fpe & BIT_IOC)) {
+        fpsr |= BIT_IOC;
+    }
+    ts->fpa.fpsr = fpsr;
+    return true;
+}
+
 void cpu_loop(CPUARMState *env)
 {
     CPUState *cs = env_cpu(env);
@@ -244,9 +330,7 @@ void cpu_loop(CPUARMState *env)
         case EXCP_NOCP:
         case EXCP_INVSTATE:
             {
-                TaskState *ts = cs->opaque;
                 uint32_t opcode;
-                int rc;
 
                 /* we handle the FPU emulation here, as Linux */
                 /* we get the opcode */
@@ -263,64 +347,15 @@ void cpu_loop(CPUARMState *env)
                     goto excp_debug;
                 }
 
-                rc = EmulateAll(opcode, &ts->fpa, env);
-                if (rc == 0) { /* illegal instruction */
-                    info.si_signo = TARGET_SIGILL;
-                    info.si_errno = 0;
-                    info.si_code = TARGET_ILL_ILLOPN;
-                    info._sifields._sigfault._addr = env->regs[15];
-                    queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
-                } else if (rc < 0) { /* FP exception */
-                    int arm_fpe=0;
-
-                    /* translate softfloat flags to FPSR flags */
-                    if (-rc & float_flag_invalid)
-                      arm_fpe |= BIT_IOC;
-                    if (-rc & float_flag_divbyzero)
-                      arm_fpe |= BIT_DZC;
-                    if (-rc & float_flag_overflow)
-                      arm_fpe |= BIT_OFC;
-                    if (-rc & float_flag_underflow)
-                      arm_fpe |= BIT_UFC;
-                    if (-rc & float_flag_inexact)
-                      arm_fpe |= BIT_IXC;
-
-                    FPSR fpsr = ts->fpa.fpsr;
-                    //printf("fpsr 0x%x, arm_fpe 0x%x\n",fpsr,arm_fpe);
-
-                    if (fpsr & (arm_fpe << 16)) { /* exception enabled? */
-                      info.si_signo = TARGET_SIGFPE;
-                      info.si_errno = 0;
-
-                      /* ordered by priority, least first */
-                      if (arm_fpe & BIT_IXC) info.si_code = TARGET_FPE_FLTRES;
-                      if (arm_fpe & BIT_UFC) info.si_code = TARGET_FPE_FLTUND;
-                      if (arm_fpe & BIT_OFC) info.si_code = TARGET_FPE_FLTOVF;
-                      if (arm_fpe & BIT_DZC) info.si_code = TARGET_FPE_FLTDIV;
-                      if (arm_fpe & BIT_IOC) info.si_code = TARGET_FPE_FLTINV;
-
-                      info._sifields._sigfault._addr = env->regs[15];
-                      queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
-                    } else {
-                      env->regs[15] += 4;
-                    }
-
-                    /* accumulate unenabled exceptions */
-                    if ((!(fpsr & BIT_IXE)) && (arm_fpe & BIT_IXC))
-                      fpsr |= BIT_IXC;
-                    if ((!(fpsr & BIT_UFE)) && (arm_fpe & BIT_UFC))
-                      fpsr |= BIT_UFC;
-                    if ((!(fpsr & BIT_OFE)) && (arm_fpe & BIT_OFC))
-                      fpsr |= BIT_OFC;
-                    if ((!(fpsr & BIT_DZE)) && (arm_fpe & BIT_DZC))
-                      fpsr |= BIT_DZC;
-                    if ((!(fpsr & BIT_IOE)) && (arm_fpe & BIT_IOC))
-                      fpsr |= BIT_IOC;
-                    ts->fpa.fpsr=fpsr;
-                } else { /* everything OK */
-                    /* increment PC */
-                    env->regs[15] += 4;
+                if (emulate_arm_fpa11(env, opcode)) {
+                    break;
                 }
+
+                info.si_signo = TARGET_SIGILL;
+                info.si_errno = 0;
+                info.si_code = TARGET_ILL_ILLOPN;
+                info._sifields._sigfault._addr = env->regs[15];
+                queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
             }
             break;
         case EXCP_SWI:
-- 
2.25.1

[PATCH 2/4] linux-user/arm: Do not emulate fpa11 in thumb mode

[Richard Henderson]

These antiquated instructions are arm-mode only.

Buglink: https://bugs.launchpad.net/bugs/1925512
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/arm/cpu_loop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 106909c7d8..e2a1496b9f 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -347,7 +347,7 @@ void cpu_loop(CPUARMState *env)
                     goto excp_debug;
                 }
 
-                if (emulate_arm_fpa11(env, opcode)) {
+                if (!env->thumb && emulate_arm_fpa11(env, opcode)) {
                     break;
                 }
 
-- 
2.25.1

[PATCH 3/4] linux-user/arm: Do not fill in si_code for fpa11 exceptions

[Richard Henderson]

There is no such decoding in linux/arch/arm/nwfpe/fpmodule.c.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/arm/cpu_loop.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index e2a1496b9f..5f61d25717 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -262,29 +262,15 @@ static bool emulate_arm_fpa11(CPUARMState *env, uint32_t opcode)
     /* Exception enabled? */
     FPSR fpsr = ts->fpa.fpsr;
     if (fpsr & (arm_fpe << 16)) {
-        target_siginfo_t info;
+        target_siginfo_t info = { };
 
+        /*
+         * The kernel's nwfpe emulator does not pass a real si_code.
+         * It merely uses send_sig(SIGFPE, current, 1).
+         */
         info.si_signo = TARGET_SIGFPE;
-        info.si_errno = 0;
+        info.si_code = TARGET_SI_KERNEL;
 
-        /* ordered by priority, least first */
-        if (arm_fpe & BIT_IXC) {
-            info.si_code = TARGET_FPE_FLTRES;
-        }
-        if (arm_fpe & BIT_UFC) {
-            info.si_code = TARGET_FPE_FLTUND;
-        }
-        if (arm_fpe & BIT_OFC) {
-            info.si_code = TARGET_FPE_FLTOVF;
-        }
-        if (arm_fpe & BIT_DZC) {
-            info.si_code = TARGET_FPE_FLTDIV;
-        }
-        if (arm_fpe & BIT_IOC) {
-            info.si_code = TARGET_FPE_FLTINV;
-        }
-
-        info._sifields._sigfault._addr = env->regs[15];
         queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
     } else {
         env->regs[15] += 4;
-- 
2.25.1

[PATCH 4/4] linux-user/arm: Simplify accumulating and raising fpa11 exceptions

[Richard Henderson]

Use bit masking instead of an if tree.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/arm/cpu_loop.c | 50 ++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 5f61d25717..69632d15be 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -228,6 +228,7 @@ static bool emulate_arm_fpa11(CPUARMState *env, uint32_t opcode)
 {
     TaskState *ts = env_cpu(env)->opaque;
     int rc = EmulateAll(opcode, &ts->fpa, env);
+    int raise, enabled;
 
     if (rc == 0) {
         /* Illegal instruction */
@@ -240,28 +241,31 @@ static bool emulate_arm_fpa11(CPUARMState *env, uint32_t opcode)
     }
 
     /* FP exception */
-    int arm_fpe = 0;
+    rc = -rc;
+    raise = 0;
 
     /* Translate softfloat flags to FPSR flags */
-    if (-rc & float_flag_invalid) {
-        arm_fpe |= BIT_IOC;
+    if (rc & float_flag_invalid) {
+        raise |= BIT_IOC;
     }
-    if (-rc & float_flag_divbyzero) {
-        arm_fpe |= BIT_DZC;
+    if (rc & float_flag_divbyzero) {
+        raise |= BIT_DZC;
     }
-    if (-rc & float_flag_overflow) {
-        arm_fpe |= BIT_OFC;
+    if (rc & float_flag_overflow) {
+        raise |= BIT_OFC;
     }
-    if (-rc & float_flag_underflow) {
-        arm_fpe |= BIT_UFC;
+    if (rc & float_flag_underflow) {
+        raise |= BIT_UFC;
     }
-    if (-rc & float_flag_inexact) {
-        arm_fpe |= BIT_IXC;
+    if (rc & float_flag_inexact) {
+        raise |= BIT_IXC;
     }
 
-    /* Exception enabled? */
-    FPSR fpsr = ts->fpa.fpsr;
-    if (fpsr & (arm_fpe << 16)) {
+    /* Accumulate unenabled exceptions */
+    enabled = ts->fpa.fpsr >> 16;
+    ts->fpa.fpsr |= raise & ~enabled;
+
+    if (raise & enabled) {
         target_siginfo_t info = { };
 
         /*
@@ -275,24 +279,6 @@ static bool emulate_arm_fpa11(CPUARMState *env, uint32_t opcode)
     } else {
         env->regs[15] += 4;
     }
-
-    /* Accumulate unenabled exceptions */
-    if ((!(fpsr & BIT_IXE)) && (arm_fpe & BIT_IXC)) {
-        fpsr |= BIT_IXC;
-    }
-    if ((!(fpsr & BIT_UFE)) && (arm_fpe & BIT_UFC)) {
-        fpsr |= BIT_UFC;
-    }
-    if ((!(fpsr & BIT_OFE)) && (arm_fpe & BIT_OFC)) {
-        fpsr |= BIT_OFC;
-    }
-    if ((!(fpsr & BIT_DZE)) && (arm_fpe & BIT_DZC)) {
-        fpsr |= BIT_DZC;
-    }
-    if ((!(fpsr & BIT_IOE)) && (arm_fpe & BIT_IOC)) {
-        fpsr |= BIT_IOC;
-    }
-    ts->fpa.fpsr = fpsr;
     return true;
 }
 
-- 
2.25.1

Re: [PATCH 2/4] linux-user/arm: Do not emulate fpa11 in thumb mode

[Peter Maydell]

On Fri, 23 Apr 2021 at 17:58, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These antiquated instructions are arm-mode only.
>
> Buglink: https://bugs.launchpad.net/bugs/1925512
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/arm/cpu_loop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index 106909c7d8..e2a1496b9f 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -347,7 +347,7 @@ void cpu_loop(CPUARMState *env)
>                      goto excp_debug;
>                  }
>
> -                if (emulate_arm_fpa11(env, opcode)) {
> +                if (!env->thumb && emulate_arm_fpa11(env, opcode)) {
>                      break;
>                  }

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 3/4] linux-user/arm: Do not fill in si_code for fpa11 exceptions

[Peter Maydell]

On Fri, 23 Apr 2021 at 17:55, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> There is no such decoding in linux/arch/arm/nwfpe/fpmodule.c.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/arm/cpu_loop.c | 26 ++++++--------------------
>  1 file changed, 6 insertions(+), 20 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I wonder why we put that code in to start with...

thanks
-- PMM

Re: [PATCH 1/4] linux-user/arm: Split out emulate_arm_fpa11

[Peter Maydell]

On Fri, 23 Apr 2021 at 17:58, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Pull out the fpa11 emulation to a helper function.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/arm/cpu_loop.c | 153 +++++++++++++++++++++++---------------
>  1 file changed, 94 insertions(+), 59 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 4/4] linux-user/arm: Simplify accumulating and raising fpa11 exceptions

[Peter Maydell]

On Fri, 23 Apr 2021 at 17:56, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Use bit masking instead of an if tree.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/arm/cpu_loop.c | 50 ++++++++++++++-------------------------
>  1 file changed, 18 insertions(+), 32 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 0/4] linux-user/arm: fpa11 fix and cleanup

[Richard Henderson]

On 4/23/21 11:54 AM, Richard Henderson wrote:
> The bug fix is patch 2, the rest is a bit of tidy-up.
> 
> 
> r~
> 
> Richard Henderson (4):
>    linux-user/arm: Split out emulate_arm_fpa11
>    linux-user/arm: Do not emulate fpa11 in thumb mode
>    linux-user/arm: Do not fill in si_code for fpa11 exceptions
>    linux-user/arm: Simplify accumulating and raising fpa11 exceptions
> 
>   linux-user/arm/cpu_loop.c | 125 ++++++++++++++++++++------------------
>   1 file changed, 66 insertions(+), 59 deletions(-)
> 

Laurent, this is all reviewed.  Do you want to take this through linux-user 
queue, or have Peter take it through his arm queue?


r~

Re: [PATCH 0/4] linux-user/arm: fpa11 fix and cleanup

[Laurent Vivier]

Le 14/05/2021 à 17:54, Richard Henderson a écrit :
> On 4/23/21 11:54 AM, Richard Henderson wrote:
>> The bug fix is patch 2, the rest is a bit of tidy-up.
>>
>>
>> r~
>>
>> Richard Henderson (4):
>>    linux-user/arm: Split out emulate_arm_fpa11
>>    linux-user/arm: Do not emulate fpa11 in thumb mode
>>    linux-user/arm: Do not fill in si_code for fpa11 exceptions
>>    linux-user/arm: Simplify accumulating and raising fpa11 exceptions
>>
>>   linux-user/arm/cpu_loop.c | 125 ++++++++++++++++++++------------------
>>   1 file changed, 66 insertions(+), 59 deletions(-)
>>
> 
> Laurent, this is all reviewed.  Do you want to take this through linux-user queue, or have Peter
> take it through his arm queue?
> 

I'm going to try to do a linux-user PR this WE, but if Peter want to pick up the series before there
is no problem.

thanks,
Laurent

Re: [PATCH 0/4] linux-user/arm: fpa11 fix and cleanup

[Laurent Vivier]

Le 23/04/2021 à 18:54, Richard Henderson a écrit :
> The bug fix is patch 2, the rest is a bit of tidy-up.
> 
> 
> r~
> 
> Richard Henderson (4):
>   linux-user/arm: Split out emulate_arm_fpa11
>   linux-user/arm: Do not emulate fpa11 in thumb mode
>   linux-user/arm: Do not fill in si_code for fpa11 exceptions
>   linux-user/arm: Simplify accumulating and raising fpa11 exceptions
> 
>  linux-user/arm/cpu_loop.c | 125 ++++++++++++++++++++------------------
>  1 file changed, 66 insertions(+), 59 deletions(-)
> 

Applied to my linux-user-for-6.1 branch.

Thanks,
Laurent