Hexagon (target/hexagon) fix bug in mem_noshuf load exception

The semantics of a mem_noshuf packet are that the store effectively
happens before the load.  However, in cases where the load raises an
exception, we cannot simply execute the store first.

This change adds a probe to check that the load will not raise an
exception before executing the store.

If the load is predicated, this requires special handling.  We check
the condition before performing the probe.  Since, we need the EA to
perform the check, we move the GET_EA portion inside CHECK_NOSHUF_PRED.

Test case added in tests/tcg/hexagon/mem_noshuf_exception.c

Suggested-by: Alessandro Di Federico <ale@rev.ng>
Suggested-by: Anton Johansson <anjo@rev.ng>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220707210546.15985-3-tsimpson@quicinc.com>

5 files changed, 59 insertions, 21 deletions

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index b0b6b3644e..50634ac459 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -339,13 +339,13 @@
     do { \
         TCGv LSB = tcg_temp_local_new(); \
         TCGLabel *label = gen_new_label(); \
-        GET_EA; \
+        tcg_gen_movi_tl(EA, 0); \
         PRED;  \
+        CHECK_NOSHUF_PRED(GET_EA, SIZE, LSB); \
         PRED_LOAD_CANCEL(LSB, EA); \
         tcg_gen_movi_tl(RdV, 0); \
-        CHECK_NOSHUF; \
         tcg_gen_brcondi_tl(TCG_COND_EQ, LSB, 0, label); \
-            fLOAD(1, SIZE, SIGN, EA, RdV); \
+        fLOAD(1, SIZE, SIGN, EA, RdV); \
         gen_set_label(label); \
         tcg_temp_free(LSB); \
     } while (0)
@@ -399,13 +399,13 @@
     do { \
         TCGv LSB = tcg_temp_local_new(); \
         TCGLabel *label = gen_new_label(); \
-        GET_EA; \
+        tcg_gen_movi_tl(EA, 0); \
         PRED;  \
+        CHECK_NOSHUF_PRED(GET_EA, 8, LSB); \
         PRED_LOAD_CANCEL(LSB, EA); \
         tcg_gen_movi_i64(RddV, 0); \
-        CHECK_NOSHUF; \
         tcg_gen_brcondi_tl(TCG_COND_EQ, LSB, 0, label); \
-            fLOAD(1, 8, u, EA, RddV); \
+        fLOAD(1, 8, u, EA, RddV); \
         gen_set_label(label); \
         tcg_temp_free(LSB); \
     } while (0)
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index cd6af4bceb..8a334ba07b 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -638,5 +638,12 @@ static void vec_to_qvec(size_t size, intptr_t dstoff, intptr_t srcoff)
     tcg_temp_free_i64(mask);
 }
 
+static void probe_noshuf_load(TCGv va, int s, int mi)
+{
+    TCGv size = tcg_constant_tl(s);
+    TCGv mem_idx = tcg_constant_tl(mi);
+    gen_helper_probe_noshuf_load(cpu_env, va, size, mem_idx);
+}
+
 #include "tcg_funcs_generated.c.inc"
 #include "tcg_func_table_generated.c.inc"
diff --git a/target/hexagon/helper.h b/target/hexagon/helper.h
index c89aa4ed4d..368f0b5708 100644
--- a/target/hexagon/helper.h
+++ b/target/hexagon/helper.h
@@ -104,6 +104,7 @@ DEF_HELPER_1(vwhist128q, void, env)
 DEF_HELPER_2(vwhist128m, void, env, s32)
 DEF_HELPER_2(vwhist128qm, void, env, s32)
 
+DEF_HELPER_4(probe_noshuf_load, void, env, i32, int, int)
 DEF_HELPER_2(probe_pkt_scalar_store_s0, void, env, int)
 DEF_HELPER_2(probe_hvx_stores, void, env, int)
 DEF_HELPER_3(probe_pkt_scalar_hvx_stores, void, env, int, int)
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index a78e84faa4..92eb8bbf05 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -87,49 +87,66 @@
  *
  *
  * For qemu, we look for a load in slot 0 when there is  a store in slot 1
- * in the same packet.  When we see this, we call a helper that merges the
- * bytes from the store buffer with the value loaded from memory.
+ * in the same packet.  When we see this, we call a helper that probes the
+ * load to make sure it doesn't fault.  Then, we process the store ahead of
+ * the actual load.
+
  */
-#define CHECK_NOSHUF \
+#define CHECK_NOSHUF(VA, SIZE) \
     do { \
         if (insn->slot == 0 && pkt->pkt_has_store_s1) { \
+            probe_noshuf_load(VA, SIZE, ctx->mem_idx); \
+            process_store(ctx, pkt, 1); \
+        } \
+    } while (0)
+
+#define CHECK_NOSHUF_PRED(GET_EA, SIZE, PRED) \
+    do { \
+        TCGLabel *label = gen_new_label(); \
+        tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, label); \
+        GET_EA; \
+        if (insn->slot == 0 && pkt->pkt_has_store_s1) { \
+            probe_noshuf_load(EA, SIZE, ctx->mem_idx); \
+        } \
+        gen_set_label(label); \
+        if (insn->slot == 0 && pkt->pkt_has_store_s1) { \
             process_store(ctx, pkt, 1); \
         } \
     } while (0)
 
 #define MEM_LOAD1s(DST, VA) \
     do { \
-        CHECK_NOSHUF; \
+        CHECK_NOSHUF(VA, 1); \
         tcg_gen_qemu_ld8s(DST, VA, ctx->mem_idx); \
     } while (0)
 #define MEM_LOAD1u(DST, VA) \
     do { \
-        CHECK_NOSHUF; \
+        CHECK_NOSHUF(VA, 1); \
         tcg_gen_qemu_ld8u(DST, VA, ctx->mem_idx); \
     } while (0)
 #define MEM_LOAD2s(DST, VA) \
     do { \
-        CHECK_NOSHUF; \
+        CHECK_NOSHUF(VA, 2); \
         tcg_gen_qemu_ld16s(DST, VA, ctx->mem_idx); \
     } while (0)
 #define MEM_LOAD2u(DST, VA) \
     do { \
-        CHECK_NOSHUF; \
+        CHECK_NOSHUF(VA, 2); \
         tcg_gen_qemu_ld16u(DST, VA, ctx->mem_idx); \
     } while (0)
 #define MEM_LOAD4s(DST, VA) \
     do { \
-        CHECK_NOSHUF; \
+        CHECK_NOSHUF(VA, 4); \
         tcg_gen_qemu_ld32s(DST, VA, ctx->mem_idx); \
     } while (0)
 #define MEM_LOAD4u(DST, VA) \
     do { \
-        CHECK_NOSHUF; \
+        CHECK_NOSHUF(VA, 4); \
         tcg_gen_qemu_ld32s(DST, VA, ctx->mem_idx); \
     } while (0)
 #define MEM_LOAD8u(DST, VA) \
     do { \
-        CHECK_NOSHUF; \
+        CHECK_NOSHUF(VA, 8); \
         tcg_gen_qemu_ld64(DST, VA, ctx->mem_idx); \
     } while (0)
 
diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index a5ed819c04..085afc3274 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -442,6 +442,17 @@ static void probe_store(CPUHexagonState *env, int slot, int mmu_idx)
     }
 }
 
+/*
+ * Called from a mem_noshuf packet to make sure the load doesn't
+ * raise an exception
+ */
+void HELPER(probe_noshuf_load)(CPUHexagonState *env, target_ulong va,
+                               int size, int mmu_idx)
+{
+    uintptr_t retaddr = GETPC();
+    probe_read(env, va, size, mmu_idx, retaddr);
+}
+
 /* Called during packet commit when there are two scalar stores */
 void HELPER(probe_pkt_scalar_store_s0)(CPUHexagonState *env, int mmu_idx)
 {
@@ -514,10 +525,12 @@ void HELPER(probe_pkt_scalar_hvx_stores)(CPUHexagonState *env, int mask,
  * If the load is in slot 0 and there is a store in slot1 (that
  * wasn't cancelled), we have to do the store first.
  */
-static void check_noshuf(CPUHexagonState *env, uint32_t slot)
+static void check_noshuf(CPUHexagonState *env, uint32_t slot,
+                         target_ulong vaddr, int size)
 {
     if (slot == 0 && env->pkt_has_store_s1 &&
         ((env->slot_cancelled & (1 << 1)) == 0)) {
+        HELPER(probe_noshuf_load)(env, vaddr, size, MMU_USER_IDX);
         HELPER(commit_store)(env, 1);
     }
 }
@@ -526,7 +539,7 @@ static uint8_t mem_load1(CPUHexagonState *env, uint32_t slot,
                          target_ulong vaddr)
 {
     uintptr_t ra = GETPC();
-    check_noshuf(env, slot);
+    check_noshuf(env, slot, vaddr, 1);
     return cpu_ldub_data_ra(env, vaddr, ra);
 }
 
@@ -534,7 +547,7 @@ static uint16_t mem_load2(CPUHexagonState *env, uint32_t slot,
                           target_ulong vaddr)
 {
     uintptr_t ra = GETPC();
-    check_noshuf(env, slot);
+    check_noshuf(env, slot, vaddr, 2);
     return cpu_lduw_data_ra(env, vaddr, ra);
 }
 
@@ -542,7 +555,7 @@ static uint32_t mem_load4(CPUHexagonState *env, uint32_t slot,
                           target_ulong vaddr)
 {
     uintptr_t ra = GETPC();
-    check_noshuf(env, slot);
+    check_noshuf(env, slot, vaddr, 4);
     return cpu_ldl_data_ra(env, vaddr, ra);
 }
 
@@ -550,7 +563,7 @@ static uint64_t mem_load8(CPUHexagonState *env, uint32_t slot,
                           target_ulong vaddr)
 {
     uintptr_t ra = GETPC();
-    check_noshuf(env, slot);
+    check_noshuf(env, slot, vaddr, 8);
     return cpu_ldq_data_ra(env, vaddr, ra);
 }