| Message ID | 20170713141928.25419-6-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: fixes for eret, isb and DISAS_UPDATE handling | expand |
On 13 July 2017 at 15:19, Alex Bennée <alex.bennee@linaro.org> 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 <alex.bennee@linaro.org> > Reviewed-by: Richard Henderson <rth@twiddle.net> > --- > 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
Peter Maydell <peter.maydell@linaro.org> writes: > On 13 July 2017 at 15:19, Alex Bennée <alex.bennee@linaro.org> 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 <alex.bennee@linaro.org> >> Reviewed-by: Richard Henderson <rth@twiddle.net> >> --- >> 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
On 14 July 2017 at 15:12, Alex Bennée <alex.bennee@linaro.org> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: >> 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. Hmm, I think that it's unnecessary, but since we were doing this before I guess better to make cleaning it up be a separate patch. thanks -- PMM
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;