[Qemu-devel] [PATCH v2 4/5] target/arm: use DISAS_JUMP for ISB handling

Alex Bennée posted 5 patches 8 years, 3 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 4/5] target/arm: use DISAS_JUMP for ISB handling
Posted by Alex Bennée 8 years, 3 months ago
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 DISAS_JUMP 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 <alex.bennee@linaro.org>
---
 target/arm/translate-a64.c |  3 ++-
 target/arm/translate.c     | 13 +++++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 66139b6046..ad46d84efb 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1393,7 +1393,8 @@ 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_a64_set_pc_im(s->pc);
+        s->is_jmp = DISAS_JUMP;
         return;
     default:
         unallocated_encoding(s);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index ccc4768b2e..94aa4bbb4d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1201,6 +1201,15 @@ static inline void gen_lookup_tb(DisasContext *s)
     s->is_jmp = DISAS_EXIT;
 }
 
+/* End the current block and force a TB lookup. We may chain to the
+ * next TB but exit_req will be immediately checked so we will exit to
+ * the main loop if we need to */
+static inline void gen_jump_tb(DisasContext *s)
+{
+    tcg_gen_movi_i32(cpu_R[15], s->pc & ~1);
+    s->is_jmp = DISAS_JUMP;
+}
+
 static inline void gen_hlt(DisasContext *s, int imm)
 {
     /* HLT. This has two purposes.
@@ -8165,7 +8174,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_jump_tb(s);
                 return;
             default:
                 goto illegal_op;
@@ -10558,7 +10567,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_jump_tb(s);
                             break;
                         default:
                             goto illegal_op;
-- 
2.13.0


Re: [Qemu-devel] [PATCH v2 4/5] target/arm: use DISAS_JUMP for ISB handling
Posted by Richard Henderson 8 years, 3 months ago
On 07/10/2017 09:21 AM, Alex Bennée wrote:
> -        s->is_jmp = DISAS_UPDATE;
> +        gen_a64_set_pc_im(s->pc);
> +        s->is_jmp = DISAS_JUMP;

Better would be gen_goto_tb.  The destination is known, so there's no need to 
go through lookup_and_goto_ptr.  You still get the icount_decr check at the 
start of the linked TB, which is what you're looking for.

Interesting that a64's gen_goto_tb sets is_jmp, but a32 does not...


r~

Re: [Qemu-devel] [PATCH v2 4/5] target/arm: use DISAS_JUMP for ISB handling
Posted by Alex Bennée 8 years, 3 months ago
Richard Henderson <rth@twiddle.net> writes:

> On 07/10/2017 09:21 AM, Alex Bennée wrote:
>> -        s->is_jmp = DISAS_UPDATE;
>> +        gen_a64_set_pc_im(s->pc);
>> +        s->is_jmp = DISAS_JUMP;
>
> Better would be gen_goto_tb.  The destination is known, so there's no
> need to go through lookup_and_goto_ptr.  You still get the icount_decr
> check at the start of the linked TB, which is what you're looking for.
>
> Interesting that a64's gen_goto_tb sets is_jmp, but a32 does not...

Hmm the only caller that is not already in the exit path sets it. Maybe
I should push the s->is_jmp to the a32 gen_goto_tb? I can then do the
same in both.

--
Alex Bennée

Re: [Qemu-devel] [PATCH v2 4/5] target/arm: use DISAS_JUMP for ISB handling
Posted by Richard Henderson 8 years, 3 months ago
On 07/10/2017 10:27 PM, Alex Bennée wrote:
> 
> Richard Henderson <rth@twiddle.net> writes:
> 
>> On 07/10/2017 09:21 AM, Alex Bennée wrote:
>>> -        s->is_jmp = DISAS_UPDATE;
>>> +        gen_a64_set_pc_im(s->pc);
>>> +        s->is_jmp = DISAS_JUMP;
>>
>> Better would be gen_goto_tb.  The destination is known, so there's no
>> need to go through lookup_and_goto_ptr.  You still get the icount_decr
>> check at the start of the linked TB, which is what you're looking for.
>>
>> Interesting that a64's gen_goto_tb sets is_jmp, but a32 does not...
> 
> Hmm the only caller that is not already in the exit path sets it. Maybe
> I should push the s->is_jmp to the a32 gen_goto_tb? I can then do the
> same in both.

Sounds reasonable.


r~