[PATCH] target/s390x: Exit tb after executing ex_value

[PATCH] target/s390x: Exit tb after executing ex_value

[Richard Henderson]

When EXECUTE sets ex_value to interrupt the constructed instruction,
we implicitly disable interrupts so that the value is not corrupted.
Exit to the main loop after execution, so that we re-evaluate any
pending interrupts.

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

Hi Sven.  Will you test this vs your testcase?  Thanks,

r~

---
 target/s390x/tcg/translate.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index fd2433d625..e52c2a4a6f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6620,11 +6620,18 @@ static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 
     dc->base.is_jmp = translate_one(env, dc);
     if (dc->base.is_jmp == DISAS_NEXT) {
-        uint64_t page_start;
-
-        page_start = dc->base.pc_first & TARGET_PAGE_MASK;
-        if (dc->base.pc_next - page_start >= TARGET_PAGE_SIZE || dc->ex_value) {
-            dc->base.is_jmp = DISAS_TOO_MANY;
+        if (unlikely(dc->ex_value)) {
+            /*
+             * Because ex_value was set, s390_cpu_exec_interrupt may
+             * have skipped an interrupt.  Exit to the main loop to
+             * re-evaluate interrupts, as we do for LCTL.
+             */
+            dc->base.is_jmp = DISAS_PC_STALE_NOCHAIN;
+        } else {
+            uint64_t page_start = dc->base.pc_first & TARGET_PAGE_MASK;
+            if (dc->base.pc_next - page_start >= TARGET_PAGE_SIZE) {
+                dc->base.is_jmp = DISAS_TOO_MANY;
+            }
         }
     }
 }
-- 
2.34.1

Re: [PATCH] target/s390x: Exit tb after executing ex_value

[Sven Schnelle]

Hi Richard,

Richard Henderson <richard.henderson@linaro.org> writes:

> When EXECUTE sets ex_value to interrupt the constructed instruction,
> we implicitly disable interrupts so that the value is not corrupted.
> Exit to the main loop after execution, so that we re-evaluate any
> pending interrupts.
>
> Reported-by: Sven Schnelle <svens@linux.ibm.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Hi Sven.  Will you test this vs your testcase?  Thanks,

Of course, i'm happy if someone fixes this so i don't have to. :-)

Unfortunately it doesn't fix the issue:

exec_tb_exit tb:(nil) flags=0x0
exec_tb tb:0x3ff35c66f00 pc=0x400
exec_tb tb:0x3ff3410f300 pc=0x1edf7f8
tcg_handle_interrupt: 2
exec_tb_exit tb:0x3ff340d2d00 flags=0x3
ignoring irq during EX
ignoring irq during EX
exec_tb tb:0x3ff340d2d00 pc=0x1edf810

writing dc->base.is_jmp to the qemu log shows:

s390x_tr_translate_insn: is_jmp: 3
s390x_tr_translate_insn: is_jmp: 3
s390x_tr_translate_insn: is_jmp: 3
s390x_tr_translate_insn: is_jmp: 3
s390x_tr_translate_insn: is_jmp: 3
s390x_tr_translate_insn: is_jmp: 3
[..]

So is_jump is always 3, which is DISAS_TARGET_0. I think the
if (dc->base.is_jmp == DISAS_NEXT) condition therefore never matches.