All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] target/microblaze: Clean up MMU translation failed path
@ 2021-06-03  9:03 Philippe Mathieu-Daudé
  2021-06-03  9:03 ` [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-03  9:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Richard Henderson, Alistair Francis,
	Philippe Mathieu-Daudé

Hi Edgar,

While reviewing/auditing how each arch handles failed MMU
transactions, I noticed some confusing code regarding the
On-chip Peripheral Bus (OPB) interface which is currently
not implemented. I took some notes and re-ordered the code
a bit, resulting in this series.

Q: Should we exit gracefully in mb_cpu_realizefn() if the
user requests features that are not implemented?

Thanks,

Phil.

Philippe Mathieu-Daudé (6):
  target/microblaze: Use the IEC binary prefix definitions
  target/microblaze: Extract FPU helpers to fpu_helper.c
  target/microblaze: Assert transaction failures have exception enabled
  target/microblaze: Fix Exception Status Register 'Cause' definitions
  target/microblaze: Replace magic values by proper definitions
  target/microblaze: Set OPB bits in tlb_fill, not in transaction_failed

 target/microblaze/cpu.h        |   8 +-
 target/microblaze/fpu_helper.c | 308 +++++++++++++++++++++++++++++++++
 target/microblaze/helper.c     |  35 +++-
 target/microblaze/mmu.c        |   5 +-
 target/microblaze/op_helper.c  | 304 +-------------------------------
 target/microblaze/meson.build  |   1 +
 6 files changed, 349 insertions(+), 312 deletions(-)
 create mode 100644 target/microblaze/fpu_helper.c

-- 
2.26.3



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions
  2021-06-03  9:03 [PATCH 0/6] target/microblaze: Clean up MMU translation failed path Philippe Mathieu-Daudé
@ 2021-06-03  9:03 ` Philippe Mathieu-Daudé
  2021-06-03 16:29   ` Edgar E. Iglesias
                     ` (2 more replies)
  2021-06-03  9:03 ` [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-03  9:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Richard Henderson, Alistair Francis,
	Philippe Mathieu-Daudé

IEC binary prefixes ease code review: the unit is explicit.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/microblaze/mmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index cc40f275eaf..1481e2769f1 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -19,14 +19,15 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 
 static unsigned int tlb_decode_size(unsigned int f)
 {
     static const unsigned int sizes[] = {
-        1 * 1024, 4 * 1024, 16 * 1024, 64 * 1024, 256 * 1024,
-        1 * 1024 * 1024, 4 * 1024 * 1024, 16 * 1024 * 1024
+        1 * KiB, 4 * KiB, 16 * KiB, 64 * KiB, 256 * KiB,
+        1 * MiB, 4 * MiB, 16 * MiB
     };
     assert(f < ARRAY_SIZE(sizes));
     return sizes[f];
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c
  2021-06-03  9:03 [PATCH 0/6] target/microblaze: Clean up MMU translation failed path Philippe Mathieu-Daudé
  2021-06-03  9:03 ` [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
@ 2021-06-03  9:03 ` Philippe Mathieu-Daudé
  2021-06-03 16:29   ` Edgar E. Iglesias
  2021-06-03 23:35   ` Alistair Francis
  2021-06-03  9:03 ` [PATCH 3/6] target/microblaze: Assert transaction failures have exception enabled Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-03  9:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Richard Henderson, Alistair Francis,
	Philippe Mathieu-Daudé

Extract FPU helpers to their own file: fpu_helper.c,
so it is easier to focus on the generic helpers in
op_helper.c.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/microblaze/fpu_helper.c | 308 +++++++++++++++++++++++++++++++++
 target/microblaze/op_helper.c  | 287 +-----------------------------
 target/microblaze/meson.build  |   1 +
 3 files changed, 310 insertions(+), 286 deletions(-)
 create mode 100644 target/microblaze/fpu_helper.c

diff --git a/target/microblaze/fpu_helper.c b/target/microblaze/fpu_helper.c
new file mode 100644
index 00000000000..ce729947079
--- /dev/null
+++ b/target/microblaze/fpu_helper.c
@@ -0,0 +1,308 @@
+/*
+ *  Microblaze FPU helper routines.
+ *
+ *  Copyright (c) 2009 Edgar E. Iglesias <edgar.iglesias@gmail.com>.
+ *  Copyright (c) 2009-2012 PetaLogix Qld Pty Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/helper-proto.h"
+#include "exec/exec-all.h"
+#include "fpu/softfloat.h"
+
+static bool check_divz(CPUMBState *env, uint32_t a, uint32_t b, uintptr_t ra)
+{
+    if (unlikely(b == 0)) {
+        env->msr |= MSR_DZ;
+
+        if ((env->msr & MSR_EE) &&
+            env_archcpu(env)->cfg.div_zero_exception) {
+            CPUState *cs = env_cpu(env);
+
+            env->esr = ESR_EC_DIVZERO;
+            cs->exception_index = EXCP_HW_EXCP;
+            cpu_loop_exit_restore(cs, ra);
+        }
+        return false;
+    }
+    return true;
+}
+
+uint32_t helper_divs(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    if (!check_divz(env, a, b, GETPC())) {
+        return 0;
+    }
+    return (int32_t)a / (int32_t)b;
+}
+
+uint32_t helper_divu(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    if (!check_divz(env, a, b, GETPC())) {
+        return 0;
+    }
+    return a / b;
+}
+
+/* raise FPU exception.  */
+static void raise_fpu_exception(CPUMBState *env, uintptr_t ra)
+{
+    CPUState *cs = env_cpu(env);
+
+    env->esr = ESR_EC_FPU;
+    cs->exception_index = EXCP_HW_EXCP;
+    cpu_loop_exit_restore(cs, ra);
+}
+
+static void update_fpu_flags(CPUMBState *env, int flags, uintptr_t ra)
+{
+    int raise = 0;
+
+    if (flags & float_flag_invalid) {
+        env->fsr |= FSR_IO;
+        raise = 1;
+    }
+    if (flags & float_flag_divbyzero) {
+        env->fsr |= FSR_DZ;
+        raise = 1;
+    }
+    if (flags & float_flag_overflow) {
+        env->fsr |= FSR_OF;
+        raise = 1;
+    }
+    if (flags & float_flag_underflow) {
+        env->fsr |= FSR_UF;
+        raise = 1;
+    }
+    if (raise
+        && (env_archcpu(env)->cfg.pvr_regs[2] & PVR2_FPU_EXC_MASK)
+        && (env->msr & MSR_EE)) {
+        raise_fpu_exception(env, ra);
+    }
+}
+
+uint32_t helper_fadd(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fd, fa, fb;
+    int flags;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    fb.l = b;
+    fd.f = float32_add(fa.f, fb.f, &env->fp_status);
+
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags, GETPC());
+    return fd.l;
+}
+
+uint32_t helper_frsub(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fd, fa, fb;
+    int flags;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    fb.l = b;
+    fd.f = float32_sub(fb.f, fa.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags, GETPC());
+    return fd.l;
+}
+
+uint32_t helper_fmul(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fd, fa, fb;
+    int flags;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    fb.l = b;
+    fd.f = float32_mul(fa.f, fb.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags, GETPC());
+
+    return fd.l;
+}
+
+uint32_t helper_fdiv(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fd, fa, fb;
+    int flags;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    fb.l = b;
+    fd.f = float32_div(fb.f, fa.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags, GETPC());
+
+    return fd.l;
+}
+
+uint32_t helper_fcmp_un(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fa, fb;
+    uint32_t r = 0;
+
+    fa.l = a;
+    fb.l = b;
+
+    if (float32_is_signaling_nan(fa.f, &env->fp_status) ||
+        float32_is_signaling_nan(fb.f, &env->fp_status)) {
+        update_fpu_flags(env, float_flag_invalid, GETPC());
+        r = 1;
+    }
+
+    if (float32_is_quiet_nan(fa.f, &env->fp_status) ||
+        float32_is_quiet_nan(fb.f, &env->fp_status)) {
+        r = 1;
+    }
+
+    return r;
+}
+
+uint32_t helper_fcmp_lt(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fa, fb;
+    int r;
+    int flags;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    fb.l = b;
+    r = float32_lt(fb.f, fa.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
+
+    return r;
+}
+
+uint32_t helper_fcmp_eq(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fa, fb;
+    int flags;
+    int r;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    fb.l = b;
+    r = float32_eq_quiet(fa.f, fb.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
+
+    return r;
+}
+
+uint32_t helper_fcmp_le(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fa, fb;
+    int flags;
+    int r;
+
+    fa.l = a;
+    fb.l = b;
+    set_float_exception_flags(0, &env->fp_status);
+    r = float32_le(fa.f, fb.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
+
+
+    return r;
+}
+
+uint32_t helper_fcmp_gt(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fa, fb;
+    int flags, r;
+
+    fa.l = a;
+    fb.l = b;
+    set_float_exception_flags(0, &env->fp_status);
+    r = float32_lt(fa.f, fb.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
+    return r;
+}
+
+uint32_t helper_fcmp_ne(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fa, fb;
+    int flags, r;
+
+    fa.l = a;
+    fb.l = b;
+    set_float_exception_flags(0, &env->fp_status);
+    r = !float32_eq_quiet(fa.f, fb.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
+
+    return r;
+}
+
+uint32_t helper_fcmp_ge(CPUMBState *env, uint32_t a, uint32_t b)
+{
+    CPU_FloatU fa, fb;
+    int flags, r;
+
+    fa.l = a;
+    fb.l = b;
+    set_float_exception_flags(0, &env->fp_status);
+    r = !float32_lt(fa.f, fb.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
+
+    return r;
+}
+
+uint32_t helper_flt(CPUMBState *env, uint32_t a)
+{
+    CPU_FloatU fd, fa;
+
+    fa.l = a;
+    fd.f = int32_to_float32(fa.l, &env->fp_status);
+    return fd.l;
+}
+
+uint32_t helper_fint(CPUMBState *env, uint32_t a)
+{
+    CPU_FloatU fa;
+    uint32_t r;
+    int flags;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    r = float32_to_int32(fa.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags, GETPC());
+
+    return r;
+}
+
+uint32_t helper_fsqrt(CPUMBState *env, uint32_t a)
+{
+    CPU_FloatU fd, fa;
+    int flags;
+
+    set_float_exception_flags(0, &env->fp_status);
+    fa.l = a;
+    fd.l = float32_sqrt(fa.f, &env->fp_status);
+    flags = get_float_exception_flags(&env->fp_status);
+    update_fpu_flags(env, flags, GETPC());
+
+    return fd.l;
+}
diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 58d633584d3..8d20522ee88 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -21,10 +21,8 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
-#include "qemu/host-utils.h"
+#include "qemu/log.h"
 #include "exec/exec-all.h"
-#include "exec/cpu_ldst.h"
-#include "fpu/softfloat.h"
 
 void helper_put(uint32_t id, uint32_t ctrl, uint32_t data)
 {
@@ -69,289 +67,6 @@ void helper_raise_exception(CPUMBState *env, uint32_t index)
     cpu_loop_exit(cs);
 }
 
-static bool check_divz(CPUMBState *env, uint32_t a, uint32_t b, uintptr_t ra)
-{
-    if (unlikely(b == 0)) {
-        env->msr |= MSR_DZ;
-
-        if ((env->msr & MSR_EE) &&
-            env_archcpu(env)->cfg.div_zero_exception) {
-            CPUState *cs = env_cpu(env);
-
-            env->esr = ESR_EC_DIVZERO;
-            cs->exception_index = EXCP_HW_EXCP;
-            cpu_loop_exit_restore(cs, ra);
-        }
-        return false;
-    }
-    return true;
-}
-
-uint32_t helper_divs(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    if (!check_divz(env, a, b, GETPC())) {
-        return 0;
-    }
-    return (int32_t)a / (int32_t)b;
-}
-
-uint32_t helper_divu(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    if (!check_divz(env, a, b, GETPC())) {
-        return 0;
-    }
-    return a / b;
-}
-
-/* raise FPU exception.  */
-static void raise_fpu_exception(CPUMBState *env, uintptr_t ra)
-{
-    CPUState *cs = env_cpu(env);
-
-    env->esr = ESR_EC_FPU;
-    cs->exception_index = EXCP_HW_EXCP;
-    cpu_loop_exit_restore(cs, ra);
-}
-
-static void update_fpu_flags(CPUMBState *env, int flags, uintptr_t ra)
-{
-    int raise = 0;
-
-    if (flags & float_flag_invalid) {
-        env->fsr |= FSR_IO;
-        raise = 1;
-    }
-    if (flags & float_flag_divbyzero) {
-        env->fsr |= FSR_DZ;
-        raise = 1;
-    }
-    if (flags & float_flag_overflow) {
-        env->fsr |= FSR_OF;
-        raise = 1;
-    }
-    if (flags & float_flag_underflow) {
-        env->fsr |= FSR_UF;
-        raise = 1;
-    }
-    if (raise
-        && (env_archcpu(env)->cfg.pvr_regs[2] & PVR2_FPU_EXC_MASK)
-        && (env->msr & MSR_EE)) {
-        raise_fpu_exception(env, ra);
-    }
-}
-
-uint32_t helper_fadd(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fd, fa, fb;
-    int flags;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    fb.l = b;
-    fd.f = float32_add(fa.f, fb.f, &env->fp_status);
-
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags, GETPC());
-    return fd.l;
-}
-
-uint32_t helper_frsub(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fd, fa, fb;
-    int flags;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    fb.l = b;
-    fd.f = float32_sub(fb.f, fa.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags, GETPC());
-    return fd.l;
-}
-
-uint32_t helper_fmul(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fd, fa, fb;
-    int flags;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    fb.l = b;
-    fd.f = float32_mul(fa.f, fb.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags, GETPC());
-
-    return fd.l;
-}
-
-uint32_t helper_fdiv(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fd, fa, fb;
-    int flags;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    fb.l = b;
-    fd.f = float32_div(fb.f, fa.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags, GETPC());
-
-    return fd.l;
-}
-
-uint32_t helper_fcmp_un(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fa, fb;
-    uint32_t r = 0;
-
-    fa.l = a;
-    fb.l = b;
-
-    if (float32_is_signaling_nan(fa.f, &env->fp_status) ||
-        float32_is_signaling_nan(fb.f, &env->fp_status)) {
-        update_fpu_flags(env, float_flag_invalid, GETPC());
-        r = 1;
-    }
-
-    if (float32_is_quiet_nan(fa.f, &env->fp_status) ||
-        float32_is_quiet_nan(fb.f, &env->fp_status)) {
-        r = 1;
-    }
-
-    return r;
-}
-
-uint32_t helper_fcmp_lt(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fa, fb;
-    int r;
-    int flags;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    fb.l = b;
-    r = float32_lt(fb.f, fa.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
-
-    return r;
-}
-
-uint32_t helper_fcmp_eq(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fa, fb;
-    int flags;
-    int r;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    fb.l = b;
-    r = float32_eq_quiet(fa.f, fb.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
-
-    return r;
-}
-
-uint32_t helper_fcmp_le(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fa, fb;
-    int flags;
-    int r;
-
-    fa.l = a;
-    fb.l = b;
-    set_float_exception_flags(0, &env->fp_status);
-    r = float32_le(fa.f, fb.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
-
-
-    return r;
-}
-
-uint32_t helper_fcmp_gt(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fa, fb;
-    int flags, r;
-
-    fa.l = a;
-    fb.l = b;
-    set_float_exception_flags(0, &env->fp_status);
-    r = float32_lt(fa.f, fb.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
-    return r;
-}
-
-uint32_t helper_fcmp_ne(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fa, fb;
-    int flags, r;
-
-    fa.l = a;
-    fb.l = b;
-    set_float_exception_flags(0, &env->fp_status);
-    r = !float32_eq_quiet(fa.f, fb.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
-
-    return r;
-}
-
-uint32_t helper_fcmp_ge(CPUMBState *env, uint32_t a, uint32_t b)
-{
-    CPU_FloatU fa, fb;
-    int flags, r;
-
-    fa.l = a;
-    fb.l = b;
-    set_float_exception_flags(0, &env->fp_status);
-    r = !float32_lt(fa.f, fb.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
-
-    return r;
-}
-
-uint32_t helper_flt(CPUMBState *env, uint32_t a)
-{
-    CPU_FloatU fd, fa;
-
-    fa.l = a;
-    fd.f = int32_to_float32(fa.l, &env->fp_status);
-    return fd.l;
-}
-
-uint32_t helper_fint(CPUMBState *env, uint32_t a)
-{
-    CPU_FloatU fa;
-    uint32_t r;
-    int flags;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    r = float32_to_int32(fa.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags, GETPC());
-
-    return r;
-}
-
-uint32_t helper_fsqrt(CPUMBState *env, uint32_t a)
-{
-    CPU_FloatU fd, fa;
-    int flags;
-
-    set_float_exception_flags(0, &env->fp_status);
-    fa.l = a;
-    fd.l = float32_sqrt(fa.f, &env->fp_status);
-    flags = get_float_exception_flags(&env->fp_status);
-    update_fpu_flags(env, flags, GETPC());
-
-    return fd.l;
-}
-
 uint32_t helper_pcmpbf(uint32_t a, uint32_t b)
 {
     unsigned int i;
diff --git a/target/microblaze/meson.build b/target/microblaze/meson.build
index 05ee0ec1635..0a5e46027af 100644
--- a/target/microblaze/meson.build
+++ b/target/microblaze/meson.build
@@ -4,6 +4,7 @@
 microblaze_ss.add(gen)
 microblaze_ss.add(files(
   'cpu.c',
+  'fpu_helper.c',
   'gdbstub.c',
   'helper.c',
   'op_helper.c',
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/6] target/microblaze: Assert transaction failures have exception enabled
  2021-06-03  9:03 [PATCH 0/6] target/microblaze: Clean up MMU translation failed path Philippe Mathieu-Daudé
  2021-06-03  9:03 ` [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
  2021-06-03  9:03 ` [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c Philippe Mathieu-Daudé
@ 2021-06-03  9:03 ` Philippe Mathieu-Daudé
  2021-06-03 16:30   ` Edgar E. Iglesias
  2021-06-03  9:03 ` [PATCH 4/6] target/microblaze: Fix Exception Status Register 'Cause' definitions Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-03  9:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Richard Henderson, Alistair Francis,
	Philippe Mathieu-Daudé

If exceptions are disabled, we must not get a transaction failure.
Assert they are enabled passed that point.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/microblaze/op_helper.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 8d20522ee88..1048e656e27 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -122,9 +122,7 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
                   access_type == MMU_INST_FETCH ? "INST_FETCH" :
                   (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
 
-    if (!(env->msr & MSR_EE)) {
-        return;
-    }
+    assert(env->msr & MSR_EE);
 
     if (access_type == MMU_INST_FETCH) {
         if (!cpu->cfg.iopb_bus_exception) {
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/6] target/microblaze: Fix Exception Status Register 'Cause' definitions
  2021-06-03  9:03 [PATCH 0/6] target/microblaze: Clean up MMU translation failed path Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-06-03  9:03 ` [PATCH 3/6] target/microblaze: Assert transaction failures have exception enabled Philippe Mathieu-Daudé
@ 2021-06-03  9:03 ` Philippe Mathieu-Daudé
  2021-06-03 16:35   ` Edgar E. Iglesias
  2021-06-03  9:03 ` [PATCH 5/6] target/microblaze: Replace magic values by proper definitions Philippe Mathieu-Daudé
  2021-06-03  9:03 ` [PATCH 6/6] target/microblaze: Set OPB bits in tlb_fill, not in transaction_failed Philippe Mathieu-Daudé
  5 siblings, 1 reply; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-03  9:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Richard Henderson, Alistair Francis,
	Philippe Mathieu-Daudé

See 'MicroBlaze Processor Reference Guide' UG081 (v9.0),
Table 1-11: "Exception Status Register (ESR)".

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/microblaze/cpu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index e4bba8a7551..42b9ad8d313 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -95,10 +95,10 @@ typedef struct CPUMBState CPUMBState;
 #define          ESR_EC_FPU             6
 #define          ESR_EC_PRIVINSN        7
 #define          ESR_EC_STACKPROT       7  /* Same as PRIVINSN.  */
-#define          ESR_EC_DATA_STORAGE    8
-#define          ESR_EC_INSN_STORAGE    9
-#define          ESR_EC_DATA_TLB        10
-#define          ESR_EC_INSN_TLB        11
+#define          ESR_EC_DATA_STORAGE    16
+#define          ESR_EC_INSN_STORAGE    17
+#define          ESR_EC_DATA_TLB        18
+#define          ESR_EC_INSN_TLB        19
 #define          ESR_EC_MASK            31
 
 /* Floating Point Status Register (FSR) Bits */
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/6] target/microblaze: Replace magic values by proper definitions
  2021-06-03  9:03 [PATCH 0/6] target/microblaze: Clean up MMU translation failed path Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-06-03  9:03 ` [PATCH 4/6] target/microblaze: Fix Exception Status Register 'Cause' definitions Philippe Mathieu-Daudé
@ 2021-06-03  9:03 ` Philippe Mathieu-Daudé
  2021-06-03 16:36   ` Edgar E. Iglesias
  2021-06-03 16:37   ` Richard Henderson
  2021-06-03  9:03 ` [PATCH 6/6] target/microblaze: Set OPB bits in tlb_fill, not in transaction_failed Philippe Mathieu-Daudé
  5 siblings, 2 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-03  9:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Richard Henderson, Alistair Francis,
	Philippe Mathieu-Daudé

Use the Exception Status Register definitions from "cpu.h".

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/microblaze/helper.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index 20dbd673136..d537f300ca6 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -99,14 +99,22 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                   mmu_idx, address);
 
     env->ear = address;
+
+    env->esr = (access_type == MMU_DATA_STORE) ? ESR_S : 0;
     switch (lu.err) {
     case ERR_PROT:
-        env->esr = access_type == MMU_INST_FETCH ? 17 : 16;
-        env->esr |= (access_type == MMU_DATA_STORE) << 10;
+        if (access_type == MMU_INST_FETCH) {
+            env->esr |= ESR_EC_INSN_STORAGE;
+        } else {
+           env->esr |= ESR_EC_DATA_STORAGE;
+        }
         break;
     case ERR_MISS:
-        env->esr = access_type == MMU_INST_FETCH ? 19 : 18;
-        env->esr |= (access_type == MMU_DATA_STORE) << 10;
+        if (access_type == MMU_INST_FETCH) {
+            env->esr |= ESR_EC_INSN_TLB;
+        } else {
+           env->esr |= ESR_EC_DATA_TLB;
+        }
         break;
     default:
         abort();
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/6] target/microblaze: Set OPB bits in tlb_fill, not in transaction_failed
  2021-06-03  9:03 [PATCH 0/6] target/microblaze: Clean up MMU translation failed path Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2021-06-03  9:03 ` [PATCH 5/6] target/microblaze: Replace magic values by proper definitions Philippe Mathieu-Daudé
@ 2021-06-03  9:03 ` Philippe Mathieu-Daudé
  2021-06-03 16:47   ` Edgar E. Iglesias
  5 siblings, 1 reply; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-03  9:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Richard Henderson, Alistair Francis,
	Philippe Mathieu-Daudé

Per the 'MicroBlaze Processor Reference Guide' UG081 (v9.0),
"Hardware Exceptions" chapter:

  Exception Causes:

  * Instruction Bus Exception

  The instruction On-chip Peripheral Bus exception is caused by an
  active error signal from the slave (IOPB_errAck) or timeout signal
  from the arbiter (IOPB_timeout).

  * Data Bus Exception

  The data On-chip Peripheral Bus exception is caused by an active
  error signal from the slave (DOPB_errAck) or timeout signal from
  the arbiter (DOPB_timeout).

the table 1-24 (Processor Version Register 2):

  * IOPBEXC:  Generate exception for IOPB error

  * DOPBEXC: Generate exception for DOPB error

and the table 2-12 (MPD Parameters):

  * C_IOPB_BUS_EXCEPTION

  Enable exception handling for IOPB bus error

  * C_DOPB_BUS_EXCEPTION

  Enable exception handling for DOPB bus error

So if PVR2.[ID]OPBEXC feature is disabled, no exception will be
generated. Thus we can not get to the transaction_failed() handler.
The ESR bits have to be set in tlb_fill().

However we never implemented the MMU check whether the address belong
to the On-chip Peripheral Bus interface, so simply add a stub for it,
warning the feature is not implemented.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/microblaze/helper.c    | 19 +++++++++++++++++++
 target/microblaze/op_helper.c | 13 -------------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index d537f300ca6..60e62bc0710 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -56,6 +56,18 @@ static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
     }
 }
 
+/* On-chip Peripheral Bus (OPB) interface */
+static bool mb_cpu_address_is_opb(MicroBlazeCPU *cpu,
+                                  vaddr address, unsigned size)
+{
+    if (cpu->cfg.iopb_bus_exception || cpu->cfg.dopb_bus_exception) {
+        /* TODO */
+        warn_report_once("On-chip Peripheral Bus (OPB) interface "
+                         "feature not implemented.");
+    }
+    return false;
+}
+
 bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                      MMUAccessType access_type, int mmu_idx,
                      bool probe, uintptr_t retaddr)
@@ -119,6 +131,13 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     default:
         abort();
     }
+    if (mb_cpu_address_is_opb(cpu, address, size)) {
+        if (access_type == MMU_INST_FETCH) {
+            env->esr = ESR_EC_INSN_BUS;
+        } else {
+           env->esr = ESR_EC_DATA_BUS;
+        }
+    }
 
     if (cs->exception_index == EXCP_MMU) {
         cpu_abort(cs, "recursive faults\n");
diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 1048e656e27..171c4cf99a0 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -123,19 +123,6 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
                   (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
 
     assert(env->msr & MSR_EE);
-
-    if (access_type == MMU_INST_FETCH) {
-        if (!cpu->cfg.iopb_bus_exception) {
-            return;
-        }
-        env->esr = ESR_EC_INSN_BUS;
-    } else {
-        if (!cpu->cfg.dopb_bus_exception) {
-            return;
-        }
-        env->esr = ESR_EC_DATA_BUS;
-    }
-
     env->ear = addr;
     cs->exception_index = EXCP_HW_EXCP;
     cpu_loop_exit_restore(cs, retaddr);
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions
  2021-06-03  9:03 ` [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
@ 2021-06-03 16:29   ` Edgar E. Iglesias
  2021-06-03 16:34   ` Richard Henderson
  2021-06-03 23:34   ` Alistair Francis
  2 siblings, 0 replies; 17+ messages in thread
From: Edgar E. Iglesias @ 2021-06-03 16:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Richard Henderson, Alistair Francis, qemu-devel

On Thu, Jun 03, 2021 at 11:03:05AM +0200, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> ---
>  target/microblaze/mmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
> index cc40f275eaf..1481e2769f1 100644
> --- a/target/microblaze/mmu.c
> +++ b/target/microblaze/mmu.c
> @@ -19,14 +19,15 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "cpu.h"
>  #include "exec/exec-all.h"
>  
>  static unsigned int tlb_decode_size(unsigned int f)
>  {
>      static const unsigned int sizes[] = {
> -        1 * 1024, 4 * 1024, 16 * 1024, 64 * 1024, 256 * 1024,
> -        1 * 1024 * 1024, 4 * 1024 * 1024, 16 * 1024 * 1024
> +        1 * KiB, 4 * KiB, 16 * KiB, 64 * KiB, 256 * KiB,
> +        1 * MiB, 4 * MiB, 16 * MiB
>      };
>      assert(f < ARRAY_SIZE(sizes));
>      return sizes[f];
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c
  2021-06-03  9:03 ` [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c Philippe Mathieu-Daudé
@ 2021-06-03 16:29   ` Edgar E. Iglesias
  2021-06-03 23:35   ` Alistair Francis
  1 sibling, 0 replies; 17+ messages in thread
From: Edgar E. Iglesias @ 2021-06-03 16:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Richard Henderson, Alistair Francis, qemu-devel

On Thu, Jun 03, 2021 at 11:03:06AM +0200, Philippe Mathieu-Daudé wrote:
> Extract FPU helpers to their own file: fpu_helper.c,
> so it is easier to focus on the generic helpers in
> op_helper.c.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target/microblaze/fpu_helper.c | 308 +++++++++++++++++++++++++++++++++
>  target/microblaze/op_helper.c  | 287 +-----------------------------
>  target/microblaze/meson.build  |   1 +
>  3 files changed, 310 insertions(+), 286 deletions(-)
>  create mode 100644 target/microblaze/fpu_helper.c
> 
> diff --git a/target/microblaze/fpu_helper.c b/target/microblaze/fpu_helper.c
> new file mode 100644
> index 00000000000..ce729947079
> --- /dev/null
> +++ b/target/microblaze/fpu_helper.c
> @@ -0,0 +1,308 @@
> +/*
> + *  Microblaze FPU helper routines.
> + *
> + *  Copyright (c) 2009 Edgar E. Iglesias <edgar.iglesias@gmail.com>.
> + *  Copyright (c) 2009-2012 PetaLogix Qld Pty Ltd.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "exec/helper-proto.h"
> +#include "exec/exec-all.h"
> +#include "fpu/softfloat.h"
> +
> +static bool check_divz(CPUMBState *env, uint32_t a, uint32_t b, uintptr_t ra)
> +{
> +    if (unlikely(b == 0)) {
> +        env->msr |= MSR_DZ;
> +
> +        if ((env->msr & MSR_EE) &&
> +            env_archcpu(env)->cfg.div_zero_exception) {
> +            CPUState *cs = env_cpu(env);
> +
> +            env->esr = ESR_EC_DIVZERO;
> +            cs->exception_index = EXCP_HW_EXCP;
> +            cpu_loop_exit_restore(cs, ra);
> +        }
> +        return false;
> +    }
> +    return true;
> +}
> +
> +uint32_t helper_divs(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    if (!check_divz(env, a, b, GETPC())) {
> +        return 0;
> +    }
> +    return (int32_t)a / (int32_t)b;
> +}
> +
> +uint32_t helper_divu(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    if (!check_divz(env, a, b, GETPC())) {
> +        return 0;
> +    }
> +    return a / b;
> +}
> +
> +/* raise FPU exception.  */
> +static void raise_fpu_exception(CPUMBState *env, uintptr_t ra)
> +{
> +    CPUState *cs = env_cpu(env);
> +
> +    env->esr = ESR_EC_FPU;
> +    cs->exception_index = EXCP_HW_EXCP;
> +    cpu_loop_exit_restore(cs, ra);
> +}
> +
> +static void update_fpu_flags(CPUMBState *env, int flags, uintptr_t ra)
> +{
> +    int raise = 0;
> +
> +    if (flags & float_flag_invalid) {
> +        env->fsr |= FSR_IO;
> +        raise = 1;
> +    }
> +    if (flags & float_flag_divbyzero) {
> +        env->fsr |= FSR_DZ;
> +        raise = 1;
> +    }
> +    if (flags & float_flag_overflow) {
> +        env->fsr |= FSR_OF;
> +        raise = 1;
> +    }
> +    if (flags & float_flag_underflow) {
> +        env->fsr |= FSR_UF;
> +        raise = 1;
> +    }
> +    if (raise
> +        && (env_archcpu(env)->cfg.pvr_regs[2] & PVR2_FPU_EXC_MASK)
> +        && (env->msr & MSR_EE)) {
> +        raise_fpu_exception(env, ra);
> +    }
> +}
> +
> +uint32_t helper_fadd(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_add(fa.f, fb.f, &env->fp_status);
> +
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +    return fd.l;
> +}
> +
> +uint32_t helper_frsub(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_sub(fb.f, fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +    return fd.l;
> +}
> +
> +uint32_t helper_fmul(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_mul(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return fd.l;
> +}
> +
> +uint32_t helper_fdiv(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_div(fb.f, fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return fd.l;
> +}
> +
> +uint32_t helper_fcmp_un(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    uint32_t r = 0;
> +
> +    fa.l = a;
> +    fb.l = b;
> +
> +    if (float32_is_signaling_nan(fa.f, &env->fp_status) ||
> +        float32_is_signaling_nan(fb.f, &env->fp_status)) {
> +        update_fpu_flags(env, float_flag_invalid, GETPC());
> +        r = 1;
> +    }
> +
> +    if (float32_is_quiet_nan(fa.f, &env->fp_status) ||
> +        float32_is_quiet_nan(fb.f, &env->fp_status)) {
> +        r = 1;
> +    }
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_lt(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int r;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    r = float32_lt(fb.f, fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_eq(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags;
> +    int r;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    r = float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_le(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags;
> +    int r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = float32_le(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_gt(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags, r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = float32_lt(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_ne(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags, r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = !float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_ge(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags, r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = !float32_lt(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_flt(CPUMBState *env, uint32_t a)
> +{
> +    CPU_FloatU fd, fa;
> +
> +    fa.l = a;
> +    fd.f = int32_to_float32(fa.l, &env->fp_status);
> +    return fd.l;
> +}
> +
> +uint32_t helper_fint(CPUMBState *env, uint32_t a)
> +{
> +    CPU_FloatU fa;
> +    uint32_t r;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    r = float32_to_int32(fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fsqrt(CPUMBState *env, uint32_t a)
> +{
> +    CPU_FloatU fd, fa;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fd.l = float32_sqrt(fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return fd.l;
> +}
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 58d633584d3..8d20522ee88 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -21,10 +21,8 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "exec/helper-proto.h"
> -#include "qemu/host-utils.h"
> +#include "qemu/log.h"
>  #include "exec/exec-all.h"
> -#include "exec/cpu_ldst.h"
> -#include "fpu/softfloat.h"
>  
>  void helper_put(uint32_t id, uint32_t ctrl, uint32_t data)
>  {
> @@ -69,289 +67,6 @@ void helper_raise_exception(CPUMBState *env, uint32_t index)
>      cpu_loop_exit(cs);
>  }
>  
> -static bool check_divz(CPUMBState *env, uint32_t a, uint32_t b, uintptr_t ra)
> -{
> -    if (unlikely(b == 0)) {
> -        env->msr |= MSR_DZ;
> -
> -        if ((env->msr & MSR_EE) &&
> -            env_archcpu(env)->cfg.div_zero_exception) {
> -            CPUState *cs = env_cpu(env);
> -
> -            env->esr = ESR_EC_DIVZERO;
> -            cs->exception_index = EXCP_HW_EXCP;
> -            cpu_loop_exit_restore(cs, ra);
> -        }
> -        return false;
> -    }
> -    return true;
> -}
> -
> -uint32_t helper_divs(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    if (!check_divz(env, a, b, GETPC())) {
> -        return 0;
> -    }
> -    return (int32_t)a / (int32_t)b;
> -}
> -
> -uint32_t helper_divu(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    if (!check_divz(env, a, b, GETPC())) {
> -        return 0;
> -    }
> -    return a / b;
> -}
> -
> -/* raise FPU exception.  */
> -static void raise_fpu_exception(CPUMBState *env, uintptr_t ra)
> -{
> -    CPUState *cs = env_cpu(env);
> -
> -    env->esr = ESR_EC_FPU;
> -    cs->exception_index = EXCP_HW_EXCP;
> -    cpu_loop_exit_restore(cs, ra);
> -}
> -
> -static void update_fpu_flags(CPUMBState *env, int flags, uintptr_t ra)
> -{
> -    int raise = 0;
> -
> -    if (flags & float_flag_invalid) {
> -        env->fsr |= FSR_IO;
> -        raise = 1;
> -    }
> -    if (flags & float_flag_divbyzero) {
> -        env->fsr |= FSR_DZ;
> -        raise = 1;
> -    }
> -    if (flags & float_flag_overflow) {
> -        env->fsr |= FSR_OF;
> -        raise = 1;
> -    }
> -    if (flags & float_flag_underflow) {
> -        env->fsr |= FSR_UF;
> -        raise = 1;
> -    }
> -    if (raise
> -        && (env_archcpu(env)->cfg.pvr_regs[2] & PVR2_FPU_EXC_MASK)
> -        && (env->msr & MSR_EE)) {
> -        raise_fpu_exception(env, ra);
> -    }
> -}
> -
> -uint32_t helper_fadd(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_add(fa.f, fb.f, &env->fp_status);
> -
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -    return fd.l;
> -}
> -
> -uint32_t helper_frsub(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_sub(fb.f, fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -    return fd.l;
> -}
> -
> -uint32_t helper_fmul(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_mul(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return fd.l;
> -}
> -
> -uint32_t helper_fdiv(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_div(fb.f, fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return fd.l;
> -}
> -
> -uint32_t helper_fcmp_un(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    uint32_t r = 0;
> -
> -    fa.l = a;
> -    fb.l = b;
> -
> -    if (float32_is_signaling_nan(fa.f, &env->fp_status) ||
> -        float32_is_signaling_nan(fb.f, &env->fp_status)) {
> -        update_fpu_flags(env, float_flag_invalid, GETPC());
> -        r = 1;
> -    }
> -
> -    if (float32_is_quiet_nan(fa.f, &env->fp_status) ||
> -        float32_is_quiet_nan(fb.f, &env->fp_status)) {
> -        r = 1;
> -    }
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_lt(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int r;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    r = float32_lt(fb.f, fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_eq(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags;
> -    int r;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    r = float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_le(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags;
> -    int r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = float32_le(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_gt(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags, r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = float32_lt(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_ne(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags, r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = !float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_ge(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags, r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = !float32_lt(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_flt(CPUMBState *env, uint32_t a)
> -{
> -    CPU_FloatU fd, fa;
> -
> -    fa.l = a;
> -    fd.f = int32_to_float32(fa.l, &env->fp_status);
> -    return fd.l;
> -}
> -
> -uint32_t helper_fint(CPUMBState *env, uint32_t a)
> -{
> -    CPU_FloatU fa;
> -    uint32_t r;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    r = float32_to_int32(fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fsqrt(CPUMBState *env, uint32_t a)
> -{
> -    CPU_FloatU fd, fa;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fd.l = float32_sqrt(fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return fd.l;
> -}
> -
>  uint32_t helper_pcmpbf(uint32_t a, uint32_t b)
>  {
>      unsigned int i;
> diff --git a/target/microblaze/meson.build b/target/microblaze/meson.build
> index 05ee0ec1635..0a5e46027af 100644
> --- a/target/microblaze/meson.build
> +++ b/target/microblaze/meson.build
> @@ -4,6 +4,7 @@
>  microblaze_ss.add(gen)
>  microblaze_ss.add(files(
>    'cpu.c',
> +  'fpu_helper.c',
>    'gdbstub.c',
>    'helper.c',
>    'op_helper.c',
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] target/microblaze: Assert transaction failures have exception enabled
  2021-06-03  9:03 ` [PATCH 3/6] target/microblaze: Assert transaction failures have exception enabled Philippe Mathieu-Daudé
@ 2021-06-03 16:30   ` Edgar E. Iglesias
  0 siblings, 0 replies; 17+ messages in thread
From: Edgar E. Iglesias @ 2021-06-03 16:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Richard Henderson, Alistair Francis, qemu-devel

On Thu, Jun 03, 2021 at 11:03:07AM +0200, Philippe Mathieu-Daudé wrote:
> If exceptions are disabled, we must not get a transaction failure.
> Assert they are enabled passed that point.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/microblaze/op_helper.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 8d20522ee88..1048e656e27 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -122,9 +122,7 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
>                    access_type == MMU_INST_FETCH ? "INST_FETCH" :
>                    (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
>  
> -    if (!(env->msr & MSR_EE)) {
> -        return;
> -    }
> +    assert(env->msr & MSR_EE);


This doesn't look correct. MSR_EE is a runtime flag...


>  
>      if (access_type == MMU_INST_FETCH) {
>          if (!cpu->cfg.iopb_bus_exception) {
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions
  2021-06-03  9:03 ` [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
  2021-06-03 16:29   ` Edgar E. Iglesias
@ 2021-06-03 16:34   ` Richard Henderson
  2021-06-03 23:34   ` Alistair Francis
  2 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2021-06-03 16:34 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis

On 6/3/21 2:03 AM, Philippe Mathieu-Daudé wrote:
>   static unsigned int tlb_decode_size(unsigned int f)
>   {
>       static const unsigned int sizes[] = {
> -        1 * 1024, 4 * 1024, 16 * 1024, 64 * 1024, 256 * 1024,
> -        1 * 1024 * 1024, 4 * 1024 * 1024, 16 * 1024 * 1024
> +        1 * KiB, 4 * KiB, 16 * KiB, 64 * KiB, 256 * KiB,
> +        1 * MiB, 4 * MiB, 16 * MiB
>       };
>       assert(f < ARRAY_SIZE(sizes));
>       return sizes[f];

I guess this is clearer, but I'll also note that this is 4**f KiB, so could 
just as well be computed by

     assert(f < 8);
     return KiB << (f * 2);

Either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/6] target/microblaze: Fix Exception Status Register 'Cause' definitions
  2021-06-03  9:03 ` [PATCH 4/6] target/microblaze: Fix Exception Status Register 'Cause' definitions Philippe Mathieu-Daudé
@ 2021-06-03 16:35   ` Edgar E. Iglesias
  0 siblings, 0 replies; 17+ messages in thread
From: Edgar E. Iglesias @ 2021-06-03 16:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Richard Henderson, Alistair Francis, qemu-devel

On Thu, Jun 03, 2021 at 11:03:08AM +0200, Philippe Mathieu-Daudé wrote:
> See 'MicroBlaze Processor Reference Guide' UG081 (v9.0),
> Table 1-11: "Exception Status Register (ESR)".
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target/microblaze/cpu.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index e4bba8a7551..42b9ad8d313 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -95,10 +95,10 @@ typedef struct CPUMBState CPUMBState;
>  #define          ESR_EC_FPU             6
>  #define          ESR_EC_PRIVINSN        7
>  #define          ESR_EC_STACKPROT       7  /* Same as PRIVINSN.  */
> -#define          ESR_EC_DATA_STORAGE    8
> -#define          ESR_EC_INSN_STORAGE    9
> -#define          ESR_EC_DATA_TLB        10
> -#define          ESR_EC_INSN_TLB        11
> +#define          ESR_EC_DATA_STORAGE    16
> +#define          ESR_EC_INSN_STORAGE    17
> +#define          ESR_EC_DATA_TLB        18
> +#define          ESR_EC_INSN_TLB        19
>  #define          ESR_EC_MASK            31
>  
>  /* Floating Point Status Register (FSR) Bits */
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6] target/microblaze: Replace magic values by proper definitions
  2021-06-03  9:03 ` [PATCH 5/6] target/microblaze: Replace magic values by proper definitions Philippe Mathieu-Daudé
@ 2021-06-03 16:36   ` Edgar E. Iglesias
  2021-06-03 16:37   ` Richard Henderson
  1 sibling, 0 replies; 17+ messages in thread
From: Edgar E. Iglesias @ 2021-06-03 16:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Richard Henderson, Alistair Francis, qemu-devel

On Thu, Jun 03, 2021 at 11:03:09AM +0200, Philippe Mathieu-Daudé wrote:
> Use the Exception Status Register definitions from "cpu.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target/microblaze/helper.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index 20dbd673136..d537f300ca6 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -99,14 +99,22 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>                    mmu_idx, address);
>  
>      env->ear = address;
> +
> +    env->esr = (access_type == MMU_DATA_STORE) ? ESR_S : 0;
>      switch (lu.err) {
>      case ERR_PROT:
> -        env->esr = access_type == MMU_INST_FETCH ? 17 : 16;
> -        env->esr |= (access_type == MMU_DATA_STORE) << 10;
> +        if (access_type == MMU_INST_FETCH) {
> +            env->esr |= ESR_EC_INSN_STORAGE;
> +        } else {
> +           env->esr |= ESR_EC_DATA_STORAGE;
> +        }
>          break;
>      case ERR_MISS:
> -        env->esr = access_type == MMU_INST_FETCH ? 19 : 18;
> -        env->esr |= (access_type == MMU_DATA_STORE) << 10;
> +        if (access_type == MMU_INST_FETCH) {
> +            env->esr |= ESR_EC_INSN_TLB;
> +        } else {
> +           env->esr |= ESR_EC_DATA_TLB;
> +        }
>          break;
>      default:
>          abort();
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6] target/microblaze: Replace magic values by proper definitions
  2021-06-03  9:03 ` [PATCH 5/6] target/microblaze: Replace magic values by proper definitions Philippe Mathieu-Daudé
  2021-06-03 16:36   ` Edgar E. Iglesias
@ 2021-06-03 16:37   ` Richard Henderson
  1 sibling, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2021-06-03 16:37 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis

On 6/3/21 2:03 AM, Philippe Mathieu-Daudé wrote:
> +        if (access_type == MMU_INST_FETCH) {
> +            env->esr |= ESR_EC_INSN_STORAGE;
> +        } else {
> +           env->esr |= ESR_EC_DATA_STORAGE;
> +        }

indentation is off.

r~


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/6] target/microblaze: Set OPB bits in tlb_fill, not in transaction_failed
  2021-06-03  9:03 ` [PATCH 6/6] target/microblaze: Set OPB bits in tlb_fill, not in transaction_failed Philippe Mathieu-Daudé
@ 2021-06-03 16:47   ` Edgar E. Iglesias
  0 siblings, 0 replies; 17+ messages in thread
From: Edgar E. Iglesias @ 2021-06-03 16:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Richard Henderson, Alistair Francis, qemu-devel

On Thu, Jun 03, 2021 at 11:03:10AM +0200, Philippe Mathieu-Daudé wrote:
> Per the 'MicroBlaze Processor Reference Guide' UG081 (v9.0),
> "Hardware Exceptions" chapter:
> 
>   Exception Causes:
> 
>   * Instruction Bus Exception
> 
>   The instruction On-chip Peripheral Bus exception is caused by an
>   active error signal from the slave (IOPB_errAck) or timeout signal
>   from the arbiter (IOPB_timeout).
> 
>   * Data Bus Exception
> 
>   The data On-chip Peripheral Bus exception is caused by an active
>   error signal from the slave (DOPB_errAck) or timeout signal from
>   the arbiter (DOPB_timeout).
> 
> the table 1-24 (Processor Version Register 2):
> 
>   * IOPBEXC:  Generate exception for IOPB error
> 
>   * DOPBEXC: Generate exception for DOPB error
> 
> and the table 2-12 (MPD Parameters):
> 
>   * C_IOPB_BUS_EXCEPTION
> 
>   Enable exception handling for IOPB bus error
> 
>   * C_DOPB_BUS_EXCEPTION
> 
>   Enable exception handling for DOPB bus error
> 
> So if PVR2.[ID]OPBEXC feature is disabled, no exception will be
> generated. Thus we can not get to the transaction_failed() handler.
> The ESR bits have to be set in tlb_fill().
> 
> However we never implemented the MMU check whether the address belong
> to the On-chip Peripheral Bus interface, so simply add a stub for it,
> warning the feature is not implemented.


This doesn't look correct either...

The OPB interface that you're refering to is implemented. It's the
insn and data memory ports of the cpu. Most MB designs today use
AXI though, but the name originated from the old DT bindings.

The transaction_failed() you're editing is not related to the MMU.
It's related to bus errors (e.g device slave errors).
These are not "avoided" by the CPU, if SW issues a transactions that
results in one, the MSR_EE flag and the properties invovled here
determine if the core will generate an trap for SW to handle the
bus error or if the error will be ignored.

Cheers,
Edgar



> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/microblaze/helper.c    | 19 +++++++++++++++++++
>  target/microblaze/op_helper.c | 13 -------------
>  2 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index d537f300ca6..60e62bc0710 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -56,6 +56,18 @@ static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
>      }
>  }
>  
> +/* On-chip Peripheral Bus (OPB) interface */
> +static bool mb_cpu_address_is_opb(MicroBlazeCPU *cpu,
> +                                  vaddr address, unsigned size)
> +{
> +    if (cpu->cfg.iopb_bus_exception || cpu->cfg.dopb_bus_exception) {
> +        /* TODO */
> +        warn_report_once("On-chip Peripheral Bus (OPB) interface "
> +                         "feature not implemented.");
> +    }
> +    return false;
> +}
> +
>  bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>                       MMUAccessType access_type, int mmu_idx,
>                       bool probe, uintptr_t retaddr)
> @@ -119,6 +131,13 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      default:
>          abort();
>      }
> +    if (mb_cpu_address_is_opb(cpu, address, size)) {
> +        if (access_type == MMU_INST_FETCH) {
> +            env->esr = ESR_EC_INSN_BUS;
> +        } else {
> +           env->esr = ESR_EC_DATA_BUS;
> +        }
> +    }
>  
>      if (cs->exception_index == EXCP_MMU) {
>          cpu_abort(cs, "recursive faults\n");
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 1048e656e27..171c4cf99a0 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -123,19 +123,6 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
>                    (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
>  
>      assert(env->msr & MSR_EE);
> -
> -    if (access_type == MMU_INST_FETCH) {
> -        if (!cpu->cfg.iopb_bus_exception) {
> -            return;
> -        }
> -        env->esr = ESR_EC_INSN_BUS;
> -    } else {
> -        if (!cpu->cfg.dopb_bus_exception) {
> -            return;
> -        }
> -        env->esr = ESR_EC_DATA_BUS;
> -    }
> -
>      env->ear = addr;
>      cs->exception_index = EXCP_HW_EXCP;
>      cpu_loop_exit_restore(cs, retaddr);
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions
  2021-06-03  9:03 ` [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
  2021-06-03 16:29   ` Edgar E. Iglesias
  2021-06-03 16:34   ` Richard Henderson
@ 2021-06-03 23:34   ` Alistair Francis
  2 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2021-06-03 23:34 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Edgar E. Iglesias, Alistair Francis, Richard Henderson,
	qemu-devel@nongnu.org Developers

On Thu, Jun 3, 2021 at 7:03 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/microblaze/mmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
> index cc40f275eaf..1481e2769f1 100644
> --- a/target/microblaze/mmu.c
> +++ b/target/microblaze/mmu.c
> @@ -19,14 +19,15 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "cpu.h"
>  #include "exec/exec-all.h"
>
>  static unsigned int tlb_decode_size(unsigned int f)
>  {
>      static const unsigned int sizes[] = {
> -        1 * 1024, 4 * 1024, 16 * 1024, 64 * 1024, 256 * 1024,
> -        1 * 1024 * 1024, 4 * 1024 * 1024, 16 * 1024 * 1024
> +        1 * KiB, 4 * KiB, 16 * KiB, 64 * KiB, 256 * KiB,
> +        1 * MiB, 4 * MiB, 16 * MiB
>      };
>      assert(f < ARRAY_SIZE(sizes));
>      return sizes[f];
> --
> 2.26.3
>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c
  2021-06-03  9:03 ` [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c Philippe Mathieu-Daudé
  2021-06-03 16:29   ` Edgar E. Iglesias
@ 2021-06-03 23:35   ` Alistair Francis
  1 sibling, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2021-06-03 23:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Edgar E. Iglesias, Alistair Francis, Richard Henderson,
	qemu-devel@nongnu.org Developers

On Thu, Jun 3, 2021 at 7:05 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Extract FPU helpers to their own file: fpu_helper.c,
> so it is easier to focus on the generic helpers in
> op_helper.c.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/microblaze/fpu_helper.c | 308 +++++++++++++++++++++++++++++++++
>  target/microblaze/op_helper.c  | 287 +-----------------------------
>  target/microblaze/meson.build  |   1 +
>  3 files changed, 310 insertions(+), 286 deletions(-)
>  create mode 100644 target/microblaze/fpu_helper.c
>
> diff --git a/target/microblaze/fpu_helper.c b/target/microblaze/fpu_helper.c
> new file mode 100644
> index 00000000000..ce729947079
> --- /dev/null
> +++ b/target/microblaze/fpu_helper.c
> @@ -0,0 +1,308 @@
> +/*
> + *  Microblaze FPU helper routines.
> + *
> + *  Copyright (c) 2009 Edgar E. Iglesias <edgar.iglesias@gmail.com>.
> + *  Copyright (c) 2009-2012 PetaLogix Qld Pty Ltd.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "exec/helper-proto.h"
> +#include "exec/exec-all.h"
> +#include "fpu/softfloat.h"
> +
> +static bool check_divz(CPUMBState *env, uint32_t a, uint32_t b, uintptr_t ra)
> +{
> +    if (unlikely(b == 0)) {
> +        env->msr |= MSR_DZ;
> +
> +        if ((env->msr & MSR_EE) &&
> +            env_archcpu(env)->cfg.div_zero_exception) {
> +            CPUState *cs = env_cpu(env);
> +
> +            env->esr = ESR_EC_DIVZERO;
> +            cs->exception_index = EXCP_HW_EXCP;
> +            cpu_loop_exit_restore(cs, ra);
> +        }
> +        return false;
> +    }
> +    return true;
> +}
> +
> +uint32_t helper_divs(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    if (!check_divz(env, a, b, GETPC())) {
> +        return 0;
> +    }
> +    return (int32_t)a / (int32_t)b;
> +}
> +
> +uint32_t helper_divu(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    if (!check_divz(env, a, b, GETPC())) {
> +        return 0;
> +    }
> +    return a / b;
> +}
> +
> +/* raise FPU exception.  */
> +static void raise_fpu_exception(CPUMBState *env, uintptr_t ra)
> +{
> +    CPUState *cs = env_cpu(env);
> +
> +    env->esr = ESR_EC_FPU;
> +    cs->exception_index = EXCP_HW_EXCP;
> +    cpu_loop_exit_restore(cs, ra);
> +}
> +
> +static void update_fpu_flags(CPUMBState *env, int flags, uintptr_t ra)
> +{
> +    int raise = 0;
> +
> +    if (flags & float_flag_invalid) {
> +        env->fsr |= FSR_IO;
> +        raise = 1;
> +    }
> +    if (flags & float_flag_divbyzero) {
> +        env->fsr |= FSR_DZ;
> +        raise = 1;
> +    }
> +    if (flags & float_flag_overflow) {
> +        env->fsr |= FSR_OF;
> +        raise = 1;
> +    }
> +    if (flags & float_flag_underflow) {
> +        env->fsr |= FSR_UF;
> +        raise = 1;
> +    }
> +    if (raise
> +        && (env_archcpu(env)->cfg.pvr_regs[2] & PVR2_FPU_EXC_MASK)
> +        && (env->msr & MSR_EE)) {
> +        raise_fpu_exception(env, ra);
> +    }
> +}
> +
> +uint32_t helper_fadd(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_add(fa.f, fb.f, &env->fp_status);
> +
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +    return fd.l;
> +}
> +
> +uint32_t helper_frsub(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_sub(fb.f, fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +    return fd.l;
> +}
> +
> +uint32_t helper_fmul(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_mul(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return fd.l;
> +}
> +
> +uint32_t helper_fdiv(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fd, fa, fb;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    fd.f = float32_div(fb.f, fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return fd.l;
> +}
> +
> +uint32_t helper_fcmp_un(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    uint32_t r = 0;
> +
> +    fa.l = a;
> +    fb.l = b;
> +
> +    if (float32_is_signaling_nan(fa.f, &env->fp_status) ||
> +        float32_is_signaling_nan(fb.f, &env->fp_status)) {
> +        update_fpu_flags(env, float_flag_invalid, GETPC());
> +        r = 1;
> +    }
> +
> +    if (float32_is_quiet_nan(fa.f, &env->fp_status) ||
> +        float32_is_quiet_nan(fb.f, &env->fp_status)) {
> +        r = 1;
> +    }
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_lt(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int r;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    r = float32_lt(fb.f, fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_eq(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags;
> +    int r;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fb.l = b;
> +    r = float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_le(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags;
> +    int r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = float32_le(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_gt(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags, r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = float32_lt(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_ne(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags, r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = !float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fcmp_ge(CPUMBState *env, uint32_t a, uint32_t b)
> +{
> +    CPU_FloatU fa, fb;
> +    int flags, r;
> +
> +    fa.l = a;
> +    fb.l = b;
> +    set_float_exception_flags(0, &env->fp_status);
> +    r = !float32_lt(fa.f, fb.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_flt(CPUMBState *env, uint32_t a)
> +{
> +    CPU_FloatU fd, fa;
> +
> +    fa.l = a;
> +    fd.f = int32_to_float32(fa.l, &env->fp_status);
> +    return fd.l;
> +}
> +
> +uint32_t helper_fint(CPUMBState *env, uint32_t a)
> +{
> +    CPU_FloatU fa;
> +    uint32_t r;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    r = float32_to_int32(fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return r;
> +}
> +
> +uint32_t helper_fsqrt(CPUMBState *env, uint32_t a)
> +{
> +    CPU_FloatU fd, fa;
> +    int flags;
> +
> +    set_float_exception_flags(0, &env->fp_status);
> +    fa.l = a;
> +    fd.l = float32_sqrt(fa.f, &env->fp_status);
> +    flags = get_float_exception_flags(&env->fp_status);
> +    update_fpu_flags(env, flags, GETPC());
> +
> +    return fd.l;
> +}
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 58d633584d3..8d20522ee88 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -21,10 +21,8 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "exec/helper-proto.h"
> -#include "qemu/host-utils.h"
> +#include "qemu/log.h"
>  #include "exec/exec-all.h"
> -#include "exec/cpu_ldst.h"
> -#include "fpu/softfloat.h"
>
>  void helper_put(uint32_t id, uint32_t ctrl, uint32_t data)
>  {
> @@ -69,289 +67,6 @@ void helper_raise_exception(CPUMBState *env, uint32_t index)
>      cpu_loop_exit(cs);
>  }
>
> -static bool check_divz(CPUMBState *env, uint32_t a, uint32_t b, uintptr_t ra)
> -{
> -    if (unlikely(b == 0)) {
> -        env->msr |= MSR_DZ;
> -
> -        if ((env->msr & MSR_EE) &&
> -            env_archcpu(env)->cfg.div_zero_exception) {
> -            CPUState *cs = env_cpu(env);
> -
> -            env->esr = ESR_EC_DIVZERO;
> -            cs->exception_index = EXCP_HW_EXCP;
> -            cpu_loop_exit_restore(cs, ra);
> -        }
> -        return false;
> -    }
> -    return true;
> -}
> -
> -uint32_t helper_divs(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    if (!check_divz(env, a, b, GETPC())) {
> -        return 0;
> -    }
> -    return (int32_t)a / (int32_t)b;
> -}
> -
> -uint32_t helper_divu(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    if (!check_divz(env, a, b, GETPC())) {
> -        return 0;
> -    }
> -    return a / b;
> -}
> -
> -/* raise FPU exception.  */
> -static void raise_fpu_exception(CPUMBState *env, uintptr_t ra)
> -{
> -    CPUState *cs = env_cpu(env);
> -
> -    env->esr = ESR_EC_FPU;
> -    cs->exception_index = EXCP_HW_EXCP;
> -    cpu_loop_exit_restore(cs, ra);
> -}
> -
> -static void update_fpu_flags(CPUMBState *env, int flags, uintptr_t ra)
> -{
> -    int raise = 0;
> -
> -    if (flags & float_flag_invalid) {
> -        env->fsr |= FSR_IO;
> -        raise = 1;
> -    }
> -    if (flags & float_flag_divbyzero) {
> -        env->fsr |= FSR_DZ;
> -        raise = 1;
> -    }
> -    if (flags & float_flag_overflow) {
> -        env->fsr |= FSR_OF;
> -        raise = 1;
> -    }
> -    if (flags & float_flag_underflow) {
> -        env->fsr |= FSR_UF;
> -        raise = 1;
> -    }
> -    if (raise
> -        && (env_archcpu(env)->cfg.pvr_regs[2] & PVR2_FPU_EXC_MASK)
> -        && (env->msr & MSR_EE)) {
> -        raise_fpu_exception(env, ra);
> -    }
> -}
> -
> -uint32_t helper_fadd(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_add(fa.f, fb.f, &env->fp_status);
> -
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -    return fd.l;
> -}
> -
> -uint32_t helper_frsub(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_sub(fb.f, fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -    return fd.l;
> -}
> -
> -uint32_t helper_fmul(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_mul(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return fd.l;
> -}
> -
> -uint32_t helper_fdiv(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fd, fa, fb;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    fd.f = float32_div(fb.f, fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return fd.l;
> -}
> -
> -uint32_t helper_fcmp_un(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    uint32_t r = 0;
> -
> -    fa.l = a;
> -    fb.l = b;
> -
> -    if (float32_is_signaling_nan(fa.f, &env->fp_status) ||
> -        float32_is_signaling_nan(fb.f, &env->fp_status)) {
> -        update_fpu_flags(env, float_flag_invalid, GETPC());
> -        r = 1;
> -    }
> -
> -    if (float32_is_quiet_nan(fa.f, &env->fp_status) ||
> -        float32_is_quiet_nan(fb.f, &env->fp_status)) {
> -        r = 1;
> -    }
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_lt(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int r;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    r = float32_lt(fb.f, fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_eq(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags;
> -    int r;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fb.l = b;
> -    r = float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_le(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags;
> -    int r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = float32_le(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_gt(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags, r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = float32_lt(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_ne(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags, r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = !float32_eq_quiet(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fcmp_ge(CPUMBState *env, uint32_t a, uint32_t b)
> -{
> -    CPU_FloatU fa, fb;
> -    int flags, r;
> -
> -    fa.l = a;
> -    fb.l = b;
> -    set_float_exception_flags(0, &env->fp_status);
> -    r = !float32_lt(fa.f, fb.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags & float_flag_invalid, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_flt(CPUMBState *env, uint32_t a)
> -{
> -    CPU_FloatU fd, fa;
> -
> -    fa.l = a;
> -    fd.f = int32_to_float32(fa.l, &env->fp_status);
> -    return fd.l;
> -}
> -
> -uint32_t helper_fint(CPUMBState *env, uint32_t a)
> -{
> -    CPU_FloatU fa;
> -    uint32_t r;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    r = float32_to_int32(fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return r;
> -}
> -
> -uint32_t helper_fsqrt(CPUMBState *env, uint32_t a)
> -{
> -    CPU_FloatU fd, fa;
> -    int flags;
> -
> -    set_float_exception_flags(0, &env->fp_status);
> -    fa.l = a;
> -    fd.l = float32_sqrt(fa.f, &env->fp_status);
> -    flags = get_float_exception_flags(&env->fp_status);
> -    update_fpu_flags(env, flags, GETPC());
> -
> -    return fd.l;
> -}
> -
>  uint32_t helper_pcmpbf(uint32_t a, uint32_t b)
>  {
>      unsigned int i;
> diff --git a/target/microblaze/meson.build b/target/microblaze/meson.build
> index 05ee0ec1635..0a5e46027af 100644
> --- a/target/microblaze/meson.build
> +++ b/target/microblaze/meson.build
> @@ -4,6 +4,7 @@
>  microblaze_ss.add(gen)
>  microblaze_ss.add(files(
>    'cpu.c',
> +  'fpu_helper.c',
>    'gdbstub.c',
>    'helper.c',
>    'op_helper.c',
> --
> 2.26.3
>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-06-03 23:36 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03  9:03 [PATCH 0/6] target/microblaze: Clean up MMU translation failed path Philippe Mathieu-Daudé
2021-06-03  9:03 ` [PATCH 1/6] target/microblaze: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
2021-06-03 16:29   ` Edgar E. Iglesias
2021-06-03 16:34   ` Richard Henderson
2021-06-03 23:34   ` Alistair Francis
2021-06-03  9:03 ` [PATCH 2/6] target/microblaze: Extract FPU helpers to fpu_helper.c Philippe Mathieu-Daudé
2021-06-03 16:29   ` Edgar E. Iglesias
2021-06-03 23:35   ` Alistair Francis
2021-06-03  9:03 ` [PATCH 3/6] target/microblaze: Assert transaction failures have exception enabled Philippe Mathieu-Daudé
2021-06-03 16:30   ` Edgar E. Iglesias
2021-06-03  9:03 ` [PATCH 4/6] target/microblaze: Fix Exception Status Register 'Cause' definitions Philippe Mathieu-Daudé
2021-06-03 16:35   ` Edgar E. Iglesias
2021-06-03  9:03 ` [PATCH 5/6] target/microblaze: Replace magic values by proper definitions Philippe Mathieu-Daudé
2021-06-03 16:36   ` Edgar E. Iglesias
2021-06-03 16:37   ` Richard Henderson
2021-06-03  9:03 ` [PATCH 6/6] target/microblaze: Set OPB bits in tlb_fill, not in transaction_failed Philippe Mathieu-Daudé
2021-06-03 16:47   ` Edgar E. Iglesias

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.