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
