Re: [PATCH v2 0/3] plugins: add tb convenience functions

[Pierrick Bouvier]

Hi Luke,

On 1/31/25 09:57, Luke Craig wrote:
> This PR extends the plugin API with two functions which allow convenient access 
> around tbs.
>
> The first, qemu_plugin_tb_size, provides a mechanism for determining the total 
> size of a translation block.
>
> The second, qemu_plugin_tb_get_insn_by_vaddr, allows users to get a reference 
> to an instruction by its virtual address rather than just its index.
>
> In response to feedback from Pierrick I have updated the implementation of 
> qemu_plugin_tb_size.
>
> Additionally, I have added these functions to the insn.c test plugin in 
> response to Alex's feedback.
>
> Lastly, I'll provide a reply to Alex's feeback (repeated below):
>
> > But the general comment is this is an example of tying the plugin API
> > too deeply with the internals of the translator. Why does a plugin need
> > to know what is an implementation detail?
>
> Finding the line between implementation detail and relevant to plugins is 
> challenging, but I submitted this change because I found myself implementing 
> these functions in plugins. If you'd like for me to enumerate examples where 
> knowing the tb_size is relevant to analysis I'd be happy to submit some.
>
> Luke Craig (3):
>    plugin: extend API with qemu_plugin_tb_get_insn_by_vaddr
>    plugin: extend API with qemu_plugin_tb_size
>    plugins: extend insn test for new convenience functions

For all the series,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

>   include/qemu/qemu-plugin.h | 21 +++++++++++++++++++++
>   plugins/api.c              | 20 ++++++++++++++++++++
>   tests/tcg/plugins/insn.c   | 10 ++++++++++
>   3 files changed, 51 insertions(+)

To accept this series, commits should be signed off [1].
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#patch-emails-must-include-a-signed-off-by-line

Thanks,
Pierrick