Re: [PATCH v3 24/26] tests/plugins: add instruction matching to libinsn.so

[Philippe Mathieu-Daudé]

On 4/2/22 21:43, Alex Bennée wrote:
> This adds simple instruction matching to the libinsn.so plugin which
> is useful for examining the execution distance between instructions.
> For example to track how often we flush in ARM due to TLB updates:
>
>    -plugin ./tests/plugin/libinsn.so,match=tlbi
>
> which leads to output like this:
>
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5702 hits, 31825 match hits, Δ+8112 
> since last match, 68859 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5703 hits, 56593 match hits, 
> Δ+17712125 since last match, 33455 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5704 hits, 56594 match hits, Δ+12689 
> since last match, 33454 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5705 hits, 56595 match hits, Δ+12585 
> since last match, 33454 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5706 hits, 56596 match hits, Δ+10491 
> since last match, 33454 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5707 hits, 56597 match hits, Δ+4721 
> since last match, 33453 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5708 hits, 56598 match hits, Δ+10733 
> since last match, 33453 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5709 hits, 56599 match hits, Δ+61959 
> since last match, 33453 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5710 hits, 56600 match hits, Δ+55235 
> since last match, 33454 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5711 hits, 56601 match hits, Δ+54373 
> since last match, 33454 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5712 hits, 56602 match hits, Δ+2705 
> since last match, 33453 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5713 hits, 56603 match hits, Δ+17262 
> since last match, 33453 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5714 hits, 56604 match hits, Δ+17206 
> since last match, 33453 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5715 hits, 56605 match hits, Δ+28940 
> since last match, 33453 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5716 hits, 56606 match hits, Δ+7370 
> since last match, 33452 avg insns/match
>    0xffffffc01019a918, 'tlbi vale1is, x1', 5717 hits, 56607 match hits, Δ+7066 
> since last match, 33452 avg insns/match
>
> showing we do some sort of TLBI invalidation every 33 thousand
> instructions.
>
> Cc: Vasilev Oleg <vasilev.oleg@huawei.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Emilio Cota <cota@braap.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20220124201608.604599-21-alex.bennee@linaro.org>
>
> ---
> v2
>    - quote disassembly
>    - try and improve formatting
> ---
>   tests/plugin/insn.c | 89 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 88 insertions(+), 1 deletion(-)

>   static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>   {
>       size_t n = qemu_plugin_tb_n_insns(tb);
> @@ -71,6 +118,29 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
> qemu_plugin_tb *tb)
>               unsigned long *cnt = &g_array_index(sizes, unsigned long, sz);
>               (*cnt)++;
>           }
> +
> +        /*
> +         * If we are tracking certain instructions we will need more
> +         * information about the instruction which we also need to
> +         * save if there is a hit.
> +         */
> +        if (matches) {
> +            char *insn_disas = qemu_plugin_insn_disas(insn);
> +            int j;
> +            for (j = 0; j < matches->len; j++) {
> +                Match *m = &g_array_index(matches, Match, j);
> +                if (g_str_has_prefix(insn_disas, m->match_string)) {
> +                    Instruction *rec = g_new0(Instruction, 1);

Maybe release matches and its Instruction entries in atexit?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +                    rec->disas = g_strdup(insn_disas);
> +                    rec->vaddr = qemu_plugin_insn_vaddr(insn);
> +                    rec->match = m;
> +                    qemu_plugin_register_vcpu_insn_exec_cb(
> +                        insn, vcpu_insn_matched_exec_before,
> +                        QEMU_PLUGIN_CB_NO_REGS, rec);
> +                }
> +            }
> +            g_free(insn_disas);
> +        }
>       }
>   }