Matt Borgerson <cont...@mborgerson.com> writes:
> Translation logic may partially decode an instruction, then abort and > remove the instruction from the TB. This can happen for example when an > instruction spans two pages. In this case, plugins may get an incorrect > result when calling qemu_plugin_tb_n_insns to query for the number of > instructions in the TB. This patch updates plugin_gen_tb_end to set the > final instruction count. For some reason this fails to apply cleanly: git am ./v2_20230714_contact_plugins_set_final_instruction_count_in_plugin_gen_tb_end.mbx Applying: plugins: Set final instruction count in plugin_gen_tb_end error: corrupt patch at line 31 Patch failed at 0001 plugins: Set final instruction count in plugin_gen_tb_end > > Signed-off-by: Matt Borgerson <cont...@mborgerson.com> > --- > accel/tcg/plugin-gen.c | 5 ++++- > accel/tcg/translator.c | 2 +- > include/exec/plugin-gen.h | 4 ++-- > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c > index 5c13615112..f18ecd6902 100644 > --- a/accel/tcg/plugin-gen.c > +++ b/accel/tcg/plugin-gen.c > @@ -866,10 +866,13 @@ void plugin_gen_insn_end(void) > * do any clean-up here and make sure things are reset in > * plugin_gen_tb_start. > */ > -void plugin_gen_tb_end(CPUState *cpu) > +void plugin_gen_tb_end(CPUState *cpu, size_t num_insns) > { > struct qemu_plugin_tb *ptb = tcg_ctx->plugin_tb; > I think it might be worth a: g_assert(num_insns <= ptb->n); here to prevent misuse of the API. > + /* translator may have removed instructions, update final count */ > + ptb->n = num_insns; > + > /* collect instrumentation requests */ > qemu_plugin_tb_trans_cb(cpu, ptb); > > diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c > index 0fd9efceba..141f514886 100644 > --- a/accel/tcg/translator.c > +++ b/accel/tcg/translator.c > @@ -215,7 +215,7 @@ void translator_loop(CPUState *cpu, > TranslationBlock *tb, int *max_insns, > gen_tb_end(tb, cflags, icount_start_insn, db->num_insns); > > if (plugin_enabled) { > - plugin_gen_tb_end(cpu); > + plugin_gen_tb_end(cpu, db->num_insns); > } > > /* The disas_log hook may use these values rather than recompute. */ > diff --git a/include/exec/plugin-gen.h b/include/exec/plugin-gen.h > index 52828781bc..c4552b5061 100644 > --- a/include/exec/plugin-gen.h > +++ b/include/exec/plugin-gen.h > @@ -20,7 +20,7 @@ struct DisasContextBase; > > bool plugin_gen_tb_start(CPUState *cpu, const struct DisasContextBase *db, > bool supress); > -void plugin_gen_tb_end(CPUState *cpu); > +void plugin_gen_tb_end(CPUState *cpu, size_t num_insns); > void plugin_gen_insn_start(CPUState *cpu, const struct DisasContextBase *db); > void plugin_gen_insn_end(void); > > @@ -42,7 +42,7 @@ void plugin_gen_insn_start(CPUState *cpu, const > struct DisasContextBase *db) > static inline void plugin_gen_insn_end(void) > { } > > -static inline void plugin_gen_tb_end(CPUState *cpu) > +static inline void plugin_gen_tb_end(CPUState *cpu, size_t num_insns) > { } > > static inline void plugin_gen_disable_mem_helpers(void) -- Alex Bennée Virtualisation Tech Lead @ Linaro