target/hexagon: Detect register write conflicts

A conflict exists when any GPR is written by multiple instructions and
at least one write is unconditional.  This catches (1) two unconditional
writes to the same GPR and (2) an unconditional write combined with a
predicated write.

Add HEX_CAUSE_REG_WRITE_CONFLICT and map it to SIGILL.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2696
Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>
Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>

linux-user/hexagon/cpu_loop.c

@@ -65,6 +65,7 @@ void cpu_loop(CPUHexagonState *env)
                             env->gpr[HEX_REG_R31]);
             break;
         case HEX_CAUSE_INVALID_PACKET:
+        case HEX_CAUSE_REG_WRITE_CONFLICT:
             force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC,
                             env->gpr[HEX_REG_PC]);
             break;

target/hexagon/cpu_bits.h

@@ -34,6 +34,7 @@ enum hex_cause {
     HEX_CAUSE_FETCH_NO_UPAGE =  0x012,
     HEX_CAUSE_INVALID_PACKET =  0x015,
     HEX_CAUSE_INVALID_OPCODE =  0x015,
+    HEX_CAUSE_REG_WRITE_CONFLICT = 0x01d,
     HEX_CAUSE_PC_NOT_ALIGNED =  0x01e,
     HEX_CAUSE_PRIV_NO_UREAD  =  0x024,
     HEX_CAUSE_PRIV_NO_UWRITE =  0x025,

target/hexagon/decode.c

@@ -655,6 +655,55 @@ decode_set_slot_number(Packet *pkt)
     return has_valid_slot_assignment(pkt);
 }
 
+/*
+ * Check for GPR write conflicts in the packet.
+ * A conflict exists when a register is written by more than one instruction
+ * and at least one of those writes is unconditional.
+ *
+ * TODO: handle the more general case of any
+ * packet w/multiple-register-write operands.
+ */
+static bool pkt_has_write_conflict(Packet *pkt)
+{
+    DECLARE_BITMAP(all_dest_gprs, 32) = { 0 };
+    DECLARE_BITMAP(wreg_mult_gprs, 32) = { 0 };
+    DECLARE_BITMAP(uncond_wreg_gprs, 32) = { 0 };
+    DECLARE_BITMAP(conflict, 32);
+
+    for (int i = 0; i < pkt->num_insns; i++) {
+        Insn *insn = &pkt->insn[i];
+        int dest = insn->dest_idx;
+
+        if (dest < 0 || !insn->dest_is_gpr) {
+            continue;
+        }
+
+        int rnum = insn->regno[dest];
+        bool is_uncond = !GET_ATTRIB(insn->opcode, A_CONDEXEC);
+
+        if (test_bit(rnum, all_dest_gprs)) {
+            set_bit(rnum, wreg_mult_gprs);
+        }
+        set_bit(rnum, all_dest_gprs);
+        if (is_uncond) {
+            set_bit(rnum, uncond_wreg_gprs);
+        }
+
+        if (insn->dest_is_pair) {
+            if (test_bit(rnum + 1, all_dest_gprs)) {
+                set_bit(rnum + 1, wreg_mult_gprs);
+            }
+            set_bit(rnum + 1, all_dest_gprs);
+            if (is_uncond) {
+                set_bit(rnum + 1, uncond_wreg_gprs);
+            }
+        }
+    }
+
+    bitmap_and(conflict, wreg_mult_gprs, uncond_wreg_gprs, 32);
+    return !bitmap_empty(conflict, 32);
+}
+
 /*
  * decode_packet
  * Decodes packet with given words
@@ -674,6 +723,10 @@ int decode_packet(DisasContext *ctx, int max_words, const uint32_t *words,
 
     /* Initialize */
     memset(pkt, 0, sizeof(*pkt));
+    for (i = 0; i < INSTRUCTIONS_MAX; i++) {
+        pkt->insn[i].dest_idx = -1;
+        pkt->insn[i].new_read_idx = -1;
+    }
     /* Try to build packet */
     while (!end_of_packet && (words_read < max_words)) {
         Insn *insn = &pkt->insn[num_insns];
@@ -737,6 +790,7 @@ int decode_packet(DisasContext *ctx, int max_words, const uint32_t *words,
             /* Invalid packet */
             return 0;
         }
+        pkt->pkt_has_write_conflict = pkt_has_write_conflict(pkt);
     }
     decode_fill_newvalue_regno(pkt);
 

target/hexagon/gen_trans_funcs.py

@@ -91,6 +91,8 @@ def gen_trans_funcs(f):
         new_read_idx = -1
         dest_idx = -1
         dest_idx_reg_id = None
+        dest_is_pair = "false"
+        dest_is_gpr = "false"
         has_pred_dest = "false"
         for regno, (reg_type, reg_id, *_) in enumerate(regs):
             reg = hex_common.get_register(tag, reg_type, reg_id)
@@ -104,6 +106,12 @@ def gen_trans_funcs(f):
                 if dest_idx_reg_id is None or reg_id < dest_idx_reg_id:
                     dest_idx = regno
                     dest_idx_reg_id = reg_id
+                    dest_is_pair = ("true"
+                                    if isinstance(reg, hex_common.Pair)
+                                    else "false")
+                    dest_is_gpr = ("true"
+                                   if reg_type == "R"
+                                   else "false")
             if reg_type == "P" and reg.is_written() and not reg.is_read():
                 has_pred_dest = "true"
 
@@ -129,6 +137,8 @@ def gen_trans_funcs(f):
         f.write(code_fmt(f"""\
             insn->new_read_idx = {new_read_idx};
             insn->dest_idx = {dest_idx};
+            insn->dest_is_pair = {dest_is_pair};
+            insn->dest_is_gpr = {dest_is_gpr};
             insn->has_pred_dest = {has_pred_dest};
         """))
         f.write(textwrap.dedent(f"""\

target/hexagon/insn.h

@@ -41,6 +41,8 @@ struct Instruction {
     uint32_t new_value_producer_slot:4;
     int32_t new_read_idx;
     int32_t dest_idx;
+    bool dest_is_pair;
+    bool dest_is_gpr;
     bool has_pred_dest;
 
     bool part1;              /*
@@ -72,6 +74,8 @@ struct Packet {
     bool pkt_has_hvx;
     Insn *vhist_insn;
 
+    bool pkt_has_write_conflict;
+
     Insn insn[INSTRUCTIONS_MAX];
 };
 

target/hexagon/translate.c

@@ -943,7 +943,7 @@ static void gen_commit_packet(DisasContext *ctx)
 static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx)
 {
     uint32_t words[PACKET_WORDS_MAX];
-    int nwords;
+    int nwords, words_read;
     Packet pkt;
     int i;
 
@@ -954,8 +954,14 @@ static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx)
     }
 
     ctx->pkt = &pkt;
-    if (decode_packet(ctx, nwords, words, &pkt, false) > 0) {
+    words_read = decode_packet(ctx, nwords, words, &pkt, false);
+    if (words_read > 0) {
         pkt.pc = ctx->base.pc_next;
+        if (pkt.pkt_has_write_conflict) {
+            gen_exception_decode_fail(ctx, words_read,
+                                      HEX_CAUSE_REG_WRITE_CONFLICT);
+            return;
+        }
         gen_start_packet(ctx);
         for (i = 0; i < pkt.num_insns; i++) {
             ctx->insn = &pkt.insn[i];

tests/tcg/hexagon/Makefile.target

@@ -52,6 +52,7 @@ HEX_TESTS += hvx_misc
 HEX_TESTS += hvx_histogram
 HEX_TESTS += invalid-slots
 HEX_TESTS += invalid-encoding
+HEX_TESTS += multiple-writes
 HEX_TESTS += unaligned_pc
 
 HEX_TESTS += test_abs

tests/tcg/hexagon/multiple-writes.c

@@ -0,0 +1,169 @@
+/*
+ * Test detection of multiple writes to the same register.
+ *
+ * Ported from the system test (tests/tcg/hexagon/system/multiple_writes.c).
+ * In linux-user mode, duplicate GPR writes are detected at translate time
+ * and raise SIGILL when at least one conflicting write is unconditional.
+ * Purely predicated duplicate writes (e.g., complementary if/if-not) are
+ * legal and are not flagged statically.
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+static void *resume_pc;
+
+static void handle_sigill(int sig, siginfo_t *info, void *puc)
+{
+    ucontext_t *uc = (ucontext_t *)puc;
+
+    if (sig != SIGILL) {
+        _exit(EXIT_FAILURE);
+    }
+
+    uc->uc_mcontext.r0 = SIGILL;
+    uc->uc_mcontext.pc = (unsigned long)resume_pc;
+}
+
+/*
+ * Unconditional pair write overlapping a single write:
+ *   { r1:0 = add(r3:2, r3:2);  r1 = add(r0, r1) }
+ * R1 is written by both instructions.  This is invalid and must raise SIGILL.
+ */
+static int test_static_pair_overlap(void)
+{
+    int sig;
+
+    asm volatile(
+        "r0 = #0\n"
+        "r1 = ##1f\n"
+        "memw(%1) = r1\n"
+        ".word 0xd30242e0\n"  /* r1:0 = add(r3:2, r3:2), parse=01 */
+        ".word 0xf300c101\n"  /* r1 = add(r0, r1), parse=11 (end) */
+        "1:\n"
+        "%0 = r0\n"
+        : "=r"(sig)
+        : "r"(&resume_pc)
+        : "r0", "r1", "memory");
+
+    return sig;
+}
+
+/*
+ * Two predicated writes under complementary predicates:
+ *   { if (p0) r0 = r2;  if (!p0) r0 = r3 }
+ * This is architecturally valid: only one write executes at runtime.
+ * Must NOT raise SIGILL; the result should reflect the executed branch.
+ */
+static int test_legal_predicated(void)
+{
+    int result;
+
+    asm volatile(
+        "r0 = #0\n"
+        "r1 = ##1f\n"
+        "memw(%1) = r1\n"
+        "r2 = #7\n"
+        "r3 = #13\n"
+        "p0 = cmp.eq(r2, r2)\n"
+        "{\n"
+        "    if (p0) r0 = r2\n"
+        "    if (!p0) r0 = r3\n"
+        "}\n"
+        "1:\n"
+        "%0 = r0\n"
+        : "=r"(result)
+        : "r"(&resume_pc)
+        : "r0", "r1", "r2", "r3", "p0", "memory");
+
+    return result;
+}
+
+/*
+ * Mixed: unconditional + predicated writes to the same register:
+ *   { if (p0) r1 = add(r0, #0);  if (!p0) r1 = add(r0, #0);
+ *     r1 = add(r0, #0) }
+ * The unconditional write always conflicts with the predicated writes.
+ * Must raise SIGILL.
+ */
+static int test_mixed_writes(void)
+{
+    int sig;
+
+    asm volatile(
+        "r0 = #0\n"
+        "r1 = ##1f\n"
+        "memw(%1) = r1\n"
+        "p0 = cmp.eq(r0, r0)\n"
+        ".word 0x7e204021\n"  /* if (p0) r1 = add(r0, #0), parse=01 */
+        ".word 0x7ea04021\n"  /* if (!p0) r1 = add(r0, #0), parse=01 */
+        ".word 0x7800c021\n"  /* r1 = add(r0, #0), parse=11 (end) */
+        "1:\n"
+        "%0 = r0\n"
+        : "=r"(sig)
+        : "r"(&resume_pc)
+        : "r0", "r1", "p0", "memory");
+
+    return sig;
+}
+
+/*
+ * Zero encoding (issue #2696):
+ * The encoding 0x00000000 decodes as a duplex with parse bits
+ * [15:14] = 0b00:
+ *   slot1: SL1_loadri_io R0 = memw(R0+#0x0)
+ *   slot0: SL1_loadri_io R0 = memw(R0+#0x0)
+ *
+ * Both sub-instructions write R0 unconditionally, which is an invalid
+ * packet.  This tests what happens when we jump to zeroed memory.
+ * Must raise SIGILL.
+ */
+static int test_zero(void)
+{
+    int sig;
+
+    asm volatile(
+        "r0 = #0\n"
+        "r1 = ##1f\n"
+        "memw(%1) = r1\n"
+        ".word 0x00000000\n"
+        "1:\n"
+        "%0 = r0\n"
+        : "=r"(sig)
+        : "r"(&resume_pc)
+        : "r0", "r1", "memory");
+
+    return sig;
+}
+
+int main()
+{
+    struct sigaction act;
+
+    memset(&act, 0, sizeof(act));
+    act.sa_sigaction = handle_sigill;
+    act.sa_flags = SA_SIGINFO;
+    assert(sigaction(SIGILL, &act, NULL) == 0);
+
+    /* Legal: complementary predicated writes must not raise SIGILL */
+    assert(test_legal_predicated() == 7);
+
+    /* Illegal: unconditional pair + single overlap must raise SIGILL */
+    assert(test_static_pair_overlap() == SIGILL);
+
+    /* Illegal: unconditional + predicated writes to same reg must SIGILL */
+    assert(test_mixed_writes() == SIGILL);
+
+    /* Illegal: zero encoding = duplex with duplicate dest R0 */
+    assert(test_zero() == SIGILL);
+
+    puts("PASS");
+    return EXIT_SUCCESS;
+}