Re: [Qemu-devel] [PATCH] target/aarch64: exit to main loop after 'msr daifclr'

["Emilio G. Cota" <cota@braap.org>]

On Wed, Jun 14, 2017 at 18:20:29 -0700, Richard Henderson wrote:
> On 06/14/2017 01:33 PM, Emilio G. Cota wrote:
> >On Wed, Jun 14, 2017 at 12:48:21 -0700, Richard Henderson wrote:
> >>Exit to cpu loop so we reevaluate cpu_arm_hw_interrupts.
> >>
> >>Cc: qemu-arm@nongnu.org
> >>Cc: Peter Maydell <peter.maydell@linaro.org>
> >>Signed-off-by: Richard Henderson <rth@twiddle.net>
> >>---
> >>  target/arm/translate-a64.c | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> >>index 860e279..e55547d 100644
> >>--- a/target/arm/translate-a64.c
> >>+++ b/target/arm/translate-a64.c
> >>@@ -1422,7 +1422,9 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
> >>          gen_helper_msr_i_pstate(cpu_env, tcg_op, tcg_imm);
> >>          tcg_temp_free_i32(tcg_imm);
> >>          tcg_temp_free_i32(tcg_op);
> >>-        s->is_jmp = DISAS_UPDATE;
> >>+        /* For DAIFClear, exit the cpu loop to re-evaluate pending IRQs.  */
> >>+        gen_a64_set_pc_im(s->pc);
> >
> >For op != 0x1f we end up setting the pc twice (first here, then in
> >the switch statement). It's still correct though.
> 
> No, that's why I switched to DISAS_JUMP.
> 
(snip)
> >+        case DISAS_EXIT:
> >+            gen_a64_set_pc_im(dc->pc);
> >+            tcg_gen_exit_tb(0);
> >+            break;
> 
> This gives translate-a64.c and translate.c different semantics for
> DISAS_EXIT. I considered that to be a bad thing.

Agreed with the above two. Sorry I missed this in my first read
of the patch, it seems that my writing of my version of this patch
impaired my ability to review another version :-)

Thanks for the clarifications!

Reviewed-by: Emilio G. Cota <cota@braap.org>
Tested-by: Emilio G. Cota <cota@braap.org>

		E.