accel/tcg/translate-all.c | 1 + target/hexagon/cpu.h | 1 + target/hexagon/genptr.c | 7 ++++--- target/hexagon/translate.c | 17 ++++++++++++++--- target/hexagon/translate.h | 2 ++ 5 files changed, 22 insertions(+), 6 deletions(-)
Hi Taylor,
I ran into an issue when a packet, not executed out of ram (get_page_addr_code_hostp returns -1, see translate-all.c) contains a fault.
This packet is an example:
{
p0 = cmp.eq(r6,#0x6)
if (p0.new) jump:t pass
memw(##0xf2000000) = r6
}
The above packet should always jump to "pass" since r6 is set to #0x6, but if the store faults, the jump is discarded. This happens because do_raise_exception's call to cpu_loop_exit_restore is not able to find a TB to restore the PC to. When an instruction is not associated with a physical RAM page translate-all will create a "one-shot" TB so when cpu_restore_state looks for the TB by calling tcg_tb_loopup none is found. That keeps the PC from being restored.
The change attached restores some of the code from commit 613653e500c0d482784f09aaa71f1297565b6815 / Hexagon (target/hexagon) Remove next_PC from runtime state.
There are two attachments, the qemu update also includes an update to translate-all.c that forces this problem to occur. The second is the testcase which is built using vanilla llvm toolchain configured for hexagon.
Thanks,
From 63e6ae2956334330110949ddba61554eef427683 Mon Sep 17 00:00:00 2001
From: Sid Manning <sidneym@quicinc.com>
Date: Tue, 16 Jan 2024 12:32:45 -0800
Subject: [PATCH] Incorrect PC update for many miss packets.
The following error exposed a hole in how our translation
code was managing pc/next_pc.
A packet with a load/store and a conditional change of flow
can end up at the wrong PC.
Signed-off-by: Sid Manning <sidneym@quicinc.com>
---
accel/tcg/translate-all.c | 1 +
target/hexagon/cpu.h | 1 +
target/hexagon/genptr.c | 7 ++++---
target/hexagon/translate.c | 17 ++++++++++++++---
target/hexagon/translate.h | 2 ++
5 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 79a88f5fb7..04e4d1d0ca 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -301,6 +301,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
qemu_thread_jit_write();
phys_pc = get_page_addr_code_hostp(env, pc, &host_pc);
+ phys_pc = -1; // Force one-shot TB to demonstrate PC issue
if (phys_pc == -1) {
/* Generate a one-shot TB with 1 insn in it */
diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
index 3135086087..40f499c511 100644
--- a/target/hexagon/cpu.h
+++ b/target/hexagon/cpu.h
@@ -427,6 +427,7 @@ typedef struct CPUArchState {
bool ss_pending;
PMUState pmu;
#endif
+ target_ulong hex_next_PC;
} CPUHexagonState;
#define mmvecx_t CPUHexagonState
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index efaa9757ac..371426416a 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -708,14 +708,15 @@ static void gen_write_new_pc_addr(DisasContext *ctx, TCGv addr,
tcg_gen_brcondi_tl(cond, pred, 0, pred_false);
}
+ TCGv PC_wr = ctx->need_next_pc ? hex_next_PC : hex_gpr[HEX_REG_PC];
if (ctx->pkt->pkt_has_multi_cof) {
/* If there are multiple branches in a packet, ignore the second one */
- tcg_gen_movcond_tl(TCG_COND_NE, hex_gpr[HEX_REG_PC],
+ tcg_gen_movcond_tl(TCG_COND_NE, PC_wr,
ctx->branch_taken, tcg_constant_tl(0),
- hex_gpr[HEX_REG_PC], addr);
+ PC_wr, addr);
tcg_gen_movi_tl(ctx->branch_taken, 1);
} else {
- tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], addr);
+ tcg_gen_mov_tl(PC_wr, addr);
}
if (cond != TCG_COND_ALWAYS) {
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 8585c7b616..3d28a5fc27 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -52,6 +52,7 @@ static const AnalyzeInsn opcode_analyze[XX_LAST_OPCODE] = {
TCGv hex_gpr[TOTAL_PER_THREAD_REGS];
TCGv hex_pred[NUM_PREGS];
TCGv hex_slot_cancelled;
+TCGv hex_next_PC;
TCGv hex_new_value_usr;
TCGv hex_reg_written[TOTAL_PER_THREAD_REGS];
TCGv hex_store_addr[STORES_MAX];
@@ -84,6 +85,8 @@ TCGv_i64 hex_cycle_count;
TCGv hex_vstore_addr[VSTORES_MAX];
TCGv hex_vstore_size[VSTORES_MAX];
TCGv hex_vstore_pending[VSTORES_MAX];
+static bool need_next_PC(DisasContext *ctx);
+
static const char * const hexagon_prednames[] = {
"p0", "p1", "p2", "p3"
@@ -238,6 +241,9 @@ static void gen_end_tb(DisasContext *ctx)
gen_exec_counters(ctx);
+ if (need_next_PC(ctx)) {
+ tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], hex_next_PC);
+ }
if (ctx->branch_cond != TCG_COND_NEVER) {
if (ctx->branch_cond != TCG_COND_ALWAYS) {
TCGLabel *skip = gen_new_label();
@@ -960,8 +966,9 @@ static void gen_start_packet(CPUHexagonState *env, DisasContext *ctx)
tcg_gen_movi_tl(ctx->branch_taken, 0);
}
ctx->pkt_ends_tb = pkt_ends_tb(pkt);
- if (need_next_PC(ctx)) {
- tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], next_PC);
+ ctx->need_next_pc = need_next_PC(ctx);
+ if (ctx->need_next_pc) {
+ tcg_gen_movi_tl(hex_next_PC, next_PC);
}
if (HEX_DEBUG) {
ctx->pred_written = tcg_temp_new();
@@ -1676,6 +1683,7 @@ static void hexagon_tr_init_disas_context(DisasContextBase *dcbase,
ctx->pmu_num_packets = 0;
ctx->pmu_hvx_packets = 0;
#endif
+ ctx->need_next_pc = false;
ctx->mem_idx = FIELD_EX32(hex_flags, TB_FLAGS, MMU_INDEX);
@@ -1902,8 +1910,11 @@ void hexagon_translate_init(void)
offsetof(CPUHexagonState, threadId), "threadId");
ss_pending = tcg_global_mem_new(tcg_env,
offsetof(CPUHexagonState, ss_pending), "ss_pending");
-
#endif
+
+ hex_next_PC = tcg_global_mem_new(tcg_env,
+ offsetof(CPUHexagonState, hex_next_PC), "next_PC");
+
for (i = 0; i < STORES_MAX; i++) {
snprintf(store_addr_names[i], NAME_LEN, "store_addr_%d", i);
hex_store_addr[i] = tcg_global_mem_new(tcg_env,
diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
index 0e13b7c831..26beb5285c 100644
--- a/target/hexagon/translate.h
+++ b/target/hexagon/translate.h
@@ -112,6 +112,7 @@ typedef struct DisasContext {
bool ss_pending;
bool short_circuit;
bool has_hvx_helper;
+ bool need_next_pc;
TCGv new_value[TOTAL_PER_THREAD_REGS];
TCGv new_pred_value[NUM_PREGS];
TCGv pred_written;
@@ -302,6 +303,7 @@ extern TCGv hex_vstore_pending[VSTORES_MAX];
extern TCGv hex_slot;
extern TCGv hex_imprecise_exception;
#endif
+extern TCGv hex_next_PC;
void gen_exception(int excp, target_ulong PC);
void gen_exception_end_tb(DisasContext *ctx, int excp);
--
2.17.1
© 2016 - 2024 Red Hat, Inc.