Re: [PATCH] Add support of callbacks after instructions to plugin api

[Alex Bennée]

Mikhail Tyutin <m.tyutin@yadro.com> writes:

> Initially, we can only call the callback BEFORE instructions. This
> commit adds the ability to insert the callback AFTER instructions.

What is the use case for this? Because:

<snip>
>  
> +
> +        /* Stop translation if translate_insn so indicated.  */
> +        if (db->is_jmp != DISAS_NEXT) {
> +            break;
> +        }
> +
>          /*
>           * We can't instrument after instructions that change control
>           * flow although this only really affects post-load operations.
> @@ -193,11 +199,6 @@ void translator_loop(CPUState *cpu, TranslationBlock 
> *tb, int *max_insns,
>              plugin_gen_insn_end();
>          }
>  
> -        /* Stop translation if translate_insn so indicated.  */
> -        if (db->is_jmp != DISAS_NEXT) {
> -            break;
> -        }
> -
>          /* Stop translation if the output buffer is full,
>             or we have executed all of the allowed instructions.  */
>          if (tcg_op_buf_full() || db->num_insns >= db->max_insns) {
> @@ -211,6 +212,13 @@ void translator_loop(CPUState *cpu, TranslationBlock 
> *tb, int *max_insns,
>      gen_tb_end(tb, cflags, icount_start_insn, db->num_insns);
>  
>      if (plugin_enabled) {
> +        /*
> +         * Last chance to call plugin_gen_insn_end() if is skipped in 
> translation
> +         * loop above.
> +         */
> +        if (db->is_jmp != DISAS_NEXT && tcg_ctx->exitreq_label == NULL) {
> +            plugin_gen_insn_end();
> +        }
>          plugin_gen_tb_end(cpu);
>      }
>  
<snip>
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -2819,6 +2819,22 @@ void tcg_gen_exit_tb(const TranslationBlock *tb, 
> unsigned idx)
>          tcg_debug_assert(idx == TB_EXIT_REQUESTED);
>      }
>  
> +#ifdef CONFIG_PLUGIN
> +    /*
> +     * Some of instruction generators insert exit_tb explicitelly to
> +     * trigger early exit from translation block. On the other hand
> +     * translation loop (translator_loop()) inserts plugin callbacks
> +     * after instruction is generated, but it appears as dead code
> +     * because of the explicit exit_tb insert.
> +     *
> +     * Calling plugin_gen_insn_end() here before the exit allows
> +     * plugins to receive control before translation block exits.
> +     */
> +    if (tcg_ctx->plugin_insn) {
> +        plugin_gen_insn_end();
> +    }
> +#endif
> +

This isn't enough as we can exit the run loop in helpers. This is why
the execlog plugin jumps the hoops it does to complete handling of
execution on the next instruction.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro