| Message ID | 20240406223248.502699-6-richard.henderson@linaro.org |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel/tcg: Fix can_do_io vs 2nd page mmio | expand |
On Mon, Apr 8, 2024 at 9:15 AM <no-reply@patchew.org> wrote: > When aborting translation of the current insn, restore the > previous value of insn_start. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Acked-by: Paolo Bonzini <pbonzini@redhat.com> The TODO comment in disas_insn() in fact can be expanded a bit. When/if the table-based decoder takes over completely, decoding can be moved completely to insn_start, and insn_start could return a bool to undo the beginning of the translation. Paolo > --- > target/i386/tcg/translate.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c > index 07f642dc9e..76a42c679c 100644 > --- a/target/i386/tcg/translate.c > +++ b/target/i386/tcg/translate.c > @@ -139,6 +139,7 @@ typedef struct DisasContext { > TCGv_i64 tmp1_i64; > > sigjmp_buf jmpbuf; > + TCGOp *prev_insn_start; > TCGOp *prev_insn_end; > } DisasContext; > > @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > /* END TODO */ > s->base.num_insns--; > tcg_remove_ops_after(s->prev_insn_end); > + s->base.insn_start = s->prev_insn_start; > s->base.is_jmp = DISAS_TOO_MANY; > return false; > default: > @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) > DisasContext *dc = container_of(dcbase, DisasContext, base); > target_ulong pc_arg = dc->base.pc_next; > > + dc->prev_insn_start = dc->base.insn_start; > dc->prev_insn_end = tcg_last_op(); > if (tb_cflags(dcbase->tb) & CF_PCREL) { > pc_arg &= ~TARGET_PAGE_MASK; > -- > 2.34.1 >
On 7/4/24 00:32, Richard Henderson wrote: > When aborting translation of the current insn, restore the > previous value of insn_start. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/i386/tcg/translate.c | 3 +++ > 1 file changed, 3 insertions(+) > @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > /* END TODO */ > s->base.num_insns--; > tcg_remove_ops_after(s->prev_insn_end); > + s->base.insn_start = s->prev_insn_start; > s->base.is_jmp = DISAS_TOO_MANY; > return false; > default: > @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) > DisasContext *dc = container_of(dcbase, DisasContext, base); > target_ulong pc_arg = dc->base.pc_next; > > + dc->prev_insn_start = dc->base.insn_start; > dc->prev_insn_end = tcg_last_op(); > if (tb_cflags(dcbase->tb) & CF_PCREL) { > pc_arg &= ~TARGET_PAGE_MASK; Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 9/4/24 17:23, Philippe Mathieu-Daudé wrote: > On 7/4/24 00:32, Richard Henderson wrote: >> When aborting translation of the current insn, restore the >> previous value of insn_start. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> target/i386/tcg/translate.c | 3 +++ >> 1 file changed, 3 insertions(+) > > >> @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState >> *cpu) >> /* END TODO */ >> s->base.num_insns--; >> tcg_remove_ops_after(s->prev_insn_end); >> + s->base.insn_start = s->prev_insn_start; >> s->base.is_jmp = DISAS_TOO_MANY; >> return false; >> default: >> @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase >> *dcbase, CPUState *cpu) >> DisasContext *dc = container_of(dcbase, DisasContext, base); >> target_ulong pc_arg = dc->base.pc_next; >> + dc->prev_insn_start = dc->base.insn_start; >> dc->prev_insn_end = tcg_last_op(); >> if (tb_cflags(dcbase->tb) & CF_PCREL) { >> pc_arg &= ~TARGET_PAGE_MASK; > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> And: Tested-by: Jørgen Hansen <Jorgen.Hansen@wdc.com> (also to patches 1 & 2) :)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 07f642dc9e..76a42c679c 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -139,6 +139,7 @@ typedef struct DisasContext { TCGv_i64 tmp1_i64; sigjmp_buf jmpbuf; + TCGOp *prev_insn_start; TCGOp *prev_insn_end; } DisasContext; @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) /* END TODO */ s->base.num_insns--; tcg_remove_ops_after(s->prev_insn_end); + s->base.insn_start = s->prev_insn_start; s->base.is_jmp = DISAS_TOO_MANY; return false; default: @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) DisasContext *dc = container_of(dcbase, DisasContext, base); target_ulong pc_arg = dc->base.pc_next; + dc->prev_insn_start = dc->base.insn_start; dc->prev_insn_end = tcg_last_op(); if (tb_cflags(dcbase->tb) & CF_PCREL) { pc_arg &= ~TARGET_PAGE_MASK;
When aborting translation of the current insn, restore the previous value of insn_start. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/i386/tcg/translate.c | 3 +++ 1 file changed, 3 insertions(+)