| Message ID | 20220630031635.271353-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/s390x: Exit tb after executing ex_value | expand |
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.
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; + } } } }
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(-)