Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
r~
On Thu, 30 Mar 2023, 03:19 Peter Maydell, <peter.mayd...@linaro.org> wrote:
> In commit 049edada we added some code to handle HSTR_EL2 traps, which
> we did as an inline "conditionally branch over a
> gen_exception_insn()". Unfortunately this fails to take account of
> the fact that gen_exception_insn() will set s->base.is_jmp to
> DISAS_NORETURN. That means that at the end of the TB we won't
> generate the necessary code to handle the "branched over the trap and
> continued normal execution" codepath. The result is that the TCG
> main loop thinks that we stopped execution of the TB due to a
> situation that only happens when icount is enabled, and hits an
> assertion. Explicitly set is_jmp back to DISAS_NEXT so we generate
> the correct code for when execution continues past this insn.
>
> Note that this only happens for cpreg reads; writes will call
> gen_lookup_tb() which generates a valid end-of-TB.
>
> Fixes: 049edada ("target/arm: Make HSTR_EL2 traps take priority over
> UNDEF-at-EL1")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1551
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> v1->v2: just unconditionally set is_jmp to DISAS_NEXT.
> ---
> target/arm/tcg/translate.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
> index 2cb9368b1ba..3c8401e9086 100644
> --- a/target/arm/tcg/translate.c
> +++ b/target/arm/tcg/translate.c
> @@ -4623,6 +4623,12 @@ static void do_coproc_insn(DisasContext *s, int
> cpnum, int is64,
> tcg_gen_brcondi_i32(TCG_COND_EQ, t, 0, over.label);
>
> gen_exception_insn(s, 0, EXCP_UDEF, syndrome);
> + /*
> + * gen_exception_insn() will set is_jmp to DISAS_NORETURN,
> + * but since we're conditionally branching over it, we want
> + * to assume continue-to-next-instruction.
> + */
> + s->base.is_jmp = DISAS_NEXT;
> set_disas_label(s, over);
> }
> }
> --
> 2.34.1
>
>
