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

[Alex Bennée]

Peter Maydell <address@hidden> writes:

> 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.

It's what gen_lookup_tb does to it's PC before the calculated jump. If
it can never happen I can get rid of it.

--
Alex Bennée