On Wed, Apr 26, 2017 at 09:54:07 +0200, Richard Henderson wrote:
> On 04/26/2017 08:23 AM, Emilio G. Cota wrote:
> >+static bool gen_jr;...
> > case DISAS_JUMP:
> >+ if (gen_jr) {
>
> Why the variable? Why not just try the goto_ptr for any DISAS_JUMP?
We have code that assumes DISAS_JUMP implies "go to exec loop", e.g.:
case 6: /* isb */
/* We need to break the TB after this insn to execute
* self-modifying code correctly and also to take
* any pending interrupts immediately.
*/
gen_lookup_tb(s);
where gen_lookup_tb does:
/* Force a TB lookup after an instruction that changes the CPU state. */
static inline void gen_lookup_tb(DisasContext *s)
{
tcg_gen_movi_i32(cpu_R[15], s->pc & ~1);
s->is_jmp = DISAS_JUMP;
}
Also, the gen_exception_* functions set DISAS_JUMP. I suspect we want to go
to the exec loop with those as well.
Testing shows that I'm onto something; if I remove the variable,
and note that I make sure DISAS_UPDATE is not falling through, I get
easily reproducible (~1 out of 5) freezes and other instability
(e.g. RCU lockup warnings) when booting + shutting down debian jessie
in the guest.
In v4 I've added a comment about this.
Thanks,
E.
