Re: [PATCH 1/2] plugin: extend API with qemu_plugin_tb_get_insn_by_vaddr

["Alex Bennée" <alex.bennee@linaro.org>]

Luke Craig <lacraig3@gmail.com> writes:

> ---
>  include/qemu/qemu-plugin.h | 11 +++++++++++
>  plugins/api.c              | 13 +++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 3a850aa216..a1c478c54f 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -500,6 +500,17 @@ QEMU_PLUGIN_API
>  struct qemu_plugin_insn *
>  qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
>  
> +/**
> + * qemu_plugin_tb_get_insn_by_vaddr() - lookup handle for instruction by vaddr
> + * @tb: opaque handle to TB passed to callback
> + * @vaddr: virtual address of instruction
> + *
> + * Returns: opaque handle to instruction
> + */
> +QEMU_PLUGIN_API
> +struct qemu_plugin_insn *
> +qemu_plugin_tb_get_insn_by_vaddr(const struct qemu_plugin_tb *tb, uint64_t vaddr);
> +
>  /**
>   * qemu_plugin_insn_data() - copy instruction data
>   * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
> diff --git a/plugins/api.c b/plugins/api.c
> index 4110cfaa23..7ff5e1c1bd 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -258,6 +258,19 @@ qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx)
>      return insn;
>  }
>  
> +struct qemu_plugin_insn *
> +qemu_plugin_tb_get_insn_by_vaddr(const struct qemu_plugin_tb *tb, uint64_t vaddr)
> +{
> +    struct qemu_plugin_insn *insn;
> +    for (size_t i = 0; i < tb->n; i++){
> +        insn = g_ptr_array_index(tb->insns, idx);
> +        if (insn != NULL && insn->vaddr == vaddr){
> +            return insn;
> +        }
> +    }
> +    return NULL;
> +}
> +

I don't have any fundamental objection to this. I would prefer it if the
new helper was used by either one of the test or contrib plugins to
ensure we don't bitrot it.

>  /*
>   * Instruction information
>   *

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro