Re: [Qemu-arm] [PATCH v4 5/6] target/arm: use gen_goto_tb for ISB handling

[Peter Maydell]

On 13 July 2017 at 15:19, Alex Bennée <address@hidden> wrote:
> While an ISB will ensure any raised IRQs happen on the next
> instruction it doesn't cause any to get raised by itself. We can
> therefor use a simple tb exit for ISB instructions and rely on the
> exit_request check at the top of each TB to deal with exiting if
> needed.
>
> Signed-off-by: Alex Bennée <address@hidden>
> Reviewed-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate-a64.c | 2 +-
>  target/arm/translate.c     | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 66139b6046..2ac565eb10 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -1393,7 +1393,7 @@ static void handle_sync(DisasContext *s, uint32_t insn,
>           * a self-modified code correctly and also to take
>           * any pending interrupts immediately.
>           */
> -        s->is_jmp = DISAS_UPDATE;
> +        gen_goto_tb(s, 0, s->pc);
>          return;
>      default:
>          unallocated_encoding(s);
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 493a7b424a..d8892d9ba5 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8168,7 +8168,7 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>                   * self-modifying code correctly and also to take
>                   * any pending interrupts immediately.
>                   */
> -                gen_lookup_tb(s);
> +                gen_goto_tb(s, 0, s->pc & ~1);
>                  return;
>              default:
>                  goto illegal_op;
> @@ -10561,7 +10561,7 @@ static int disas_thumb2_insn(CPUARMState *env, 
> DisasContext *s, uint16_t insn_hw
>                               * and also to take any pending interrupts
>                               * immediately.
>                               */
> -                            gen_lookup_tb(s);
> +                            gen_goto_tb(s, 0, s->pc & ~1);
>                              break;
>                          default:
>                              goto illegal_op;

Why do we need to clear the low bit of s->pc for ISB?
s->pc is the actual PC, not the "PC and low bit indicates
Thumb mode" form that jump addresses have.

thanks
-- PMM