[Qemu-devel] [PATCH] target/hppa: Avoid squishing DISAS_IAQ_N_STALE_EXIT

Richard Henderson posted 1 patch 6 years, 8 months ago
Test asan failed
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190312224923.25709-1-richard.henderson@linaro.org
Maintainers: Richard Henderson <rth@twiddle.net>
target/hppa/translate.c | 42 +++++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 14 deletions(-)
[Qemu-devel] [PATCH] target/hppa: Avoid squishing DISAS_IAQ_N_STALE_EXIT
Posted by Richard Henderson 6 years, 8 months ago
Within a delay slot, we were squishing both DISAS_IAQ_N_STALE and
DISAS_IAQ_N_STALE_EXIT to DISAS_IAQ_N_UPDATED.  This lost the
required exit to the main loop, and could result in interrupts
never being delivered.

Reported-by: Sven Schnelle <svens@stackframe.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Sven, this should be a better fix for your 11/11.


r~

---
 target/hppa/translate.c | 42 +++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)


diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 35c504087f..43b74367ea 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -347,6 +347,7 @@ static int expand_shl11(int val)
 /* Similarly, but we want to return to the main loop immediately
    to recognize unmasked interrupts.  */
 #define DISAS_IAQ_N_STALE_EXIT      DISAS_TARGET_2
+#define DISAS_EXIT                  DISAS_TARGET_3
 
 /* global register indexes */
 static TCGv_reg cpu_gr[32];
@@ -4218,19 +4219,31 @@ static void hppa_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     ctx->iaoq_b = ctx->iaoq_n;
     ctx->base.pc_next += 4;
 
-    if (ret == DISAS_NORETURN || ret == DISAS_IAQ_N_UPDATED) {
-        return;
-    }
-    if (ctx->iaoq_f == -1) {
-        tcg_gen_mov_reg(cpu_iaoq_f, cpu_iaoq_b);
-        copy_iaoq_entry(cpu_iaoq_b, ctx->iaoq_n, ctx->iaoq_n_var);
+    switch (ret) {
+    case DISAS_NORETURN:
+    case DISAS_IAQ_N_UPDATED:
+        break;
+
+    case DISAS_NEXT:
+    case DISAS_IAQ_N_STALE:
+    case DISAS_IAQ_N_STALE_EXIT:
+        if (ctx->iaoq_f == -1) {
+            tcg_gen_mov_reg(cpu_iaoq_f, cpu_iaoq_b);
+            copy_iaoq_entry(cpu_iaoq_b, ctx->iaoq_n, ctx->iaoq_n_var);
 #ifndef CONFIG_USER_ONLY
-        tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
+            tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
 #endif
-        nullify_save(ctx);
-        ctx->base.is_jmp = DISAS_IAQ_N_UPDATED;
-    } else if (ctx->iaoq_b == -1) {
-        tcg_gen_mov_reg(cpu_iaoq_b, ctx->iaoq_n_var);
+            nullify_save(ctx);
+            ctx->base.is_jmp = (ret == DISAS_IAQ_N_STALE_EXIT
+                                ? DISAS_EXIT
+                                : DISAS_IAQ_N_UPDATED);
+        } else if (ctx->iaoq_b == -1) {
+            tcg_gen_mov_reg(cpu_iaoq_b, ctx->iaoq_n_var);
+        }
+        break;
+
+    default:
+        g_assert_not_reached();
     }
 }
 
@@ -4252,11 +4265,12 @@ static void hppa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
     case DISAS_IAQ_N_UPDATED:
         if (ctx->base.singlestep_enabled) {
             gen_excp_1(EXCP_DEBUG);
-        } else if (is_jmp == DISAS_IAQ_N_STALE_EXIT) {
-            tcg_gen_exit_tb(NULL, 0);
-        } else {
+        } else if (is_jmp != DISAS_IAQ_N_STALE_EXIT) {
             tcg_gen_lookup_and_goto_ptr();
         }
+        /* FALLTHRU */
+    case DISAS_EXIT:
+        tcg_gen_exit_tb(NULL, 0);
         break;
     default:
         g_assert_not_reached();
-- 
2.17.2


Re: [Qemu-devel] [PATCH] target/hppa: Avoid squishing DISAS_IAQ_N_STALE_EXIT
Posted by Sven Schnelle 6 years, 8 months ago
On Tue, Mar 12, 2019 at 03:49:23PM -0700, Richard Henderson wrote:
> Within a delay slot, we were squishing both DISAS_IAQ_N_STALE and
> DISAS_IAQ_N_STALE_EXIT to DISAS_IAQ_N_UPDATED.  This lost the
> required exit to the main loop, and could result in interrupts
> never being delivered.
> 
> Reported-by: Sven Schnelle <svens@stackframe.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Sven, this should be a better fix for your 11/11.

Indeed, much better.

Tested-by: Sven Schnelle <svens@stackframe.org>

Thanks
Sven