Re: [PATCH 2/2] plugin: extend API with qemu_plugin_tb_size

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

Luke Craig <lacraig3@gmail.com> writes:

> ---
>  include/qemu/qemu-plugin.h | 10 ++++++++++
>  plugins/api.c              |  5 +++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index a1c478c54f..1fa656da82 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -476,6 +476,16 @@ void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>  QEMU_PLUGIN_API
>  size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
>  
> +/**
> + * qemu_plugin_tb_size() - query helper for size of TB
> + * @tb: opaque handle to TB passed to callback
> + * 
> + * Returns: size of block in bytes
> + */
> +
> +QEMU_PLUGIN_API
> +size_t qemu_plugin_tb_size(const struct qemu_plugin_tb *tb);
> +
>  /**
>   * qemu_plugin_tb_vaddr() - query helper for vaddr of TB start
>   * @tb: opaque handle to TB passed to callback
> diff --git a/plugins/api.c b/plugins/api.c
> index 7ff5e1c1bd..177f0ac9b6 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -241,6 +241,11 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb)
>      return tb->n;
>  }
>  
> +size_t qemu_plugin_tb_size(const struct qemu_plugin_tb *tb){
> +    const DisasContextBase *db = tcg_ctx->plugin_db;
> +    return db->size;
> +}
> +

FAILED: libqemu-aarch64-linux-user.a.p/plugins_api.c.o 
cc -m64 -Ilibqemu-aarch64-linux-user.a.p -I. -I../.. -Itarget/arm -I../../target/arm -I../../common-user/host/x86_64 -I../../linux-user/include/host/x86_64 -I../../linux-user/include -Ilinux-user -I../../linux-user -Ilinux-user/aarch64 -I../../linux-user/aarch64 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wempty-body -Wendif-labels -Wexpansion-to-defined -Wformat-security -Wformat-y2k -Wignored-qualifiers -Wimplicit-fallthrough=2 -Winit-self -Wmissing-format-attribute -Wmissing-prototypes -Wnested-externs -Wold-style-declaration -Wold-style-definition -Wredundant-decls -Wshadow=local -Wstrict-prototypes -Wtype-limits -Wundef -Wvla -Wwrite-strings -Wno-missing-include-dirs -Wno-psabi -Wno-shift-negative-value -isystem /home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers -iquote . -iquote /home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/include -iquote /home/alex/lsrc/qemu.git/host/include/x86_64 -iquote /home/alex/lsrc/qemu.git/host/include/generic -iquote /home/alex/lsrc/qemu.git/tcg/i386 -pthread -mcx16 -msse2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -ftrivial-auto-var-init=zero -fzero-call-used-regs=used-gpr -fPIE -isystem../../linux-headers -isystemlinux-headers -DCOMPILING_PER_TARGET '-DCONFIG_TARGET="aarch64-linux-user-config-target.h"' -MD -MQ libqemu-aarch64-linux-user.a.p/plugins_api.c.o -MF libqemu-aarch64-linux-user.a.p/plugins_api.c.o.d -o libqemu-aarch64-linux-user.a.p/plugins_api.c.o -c ../../plugins/api.c
../../plugins/api.c: In function ‘qemu_plugin_tb_size’:
../../plugins/api.c:246:14: error: ‘DisasContextBase’ has no member named ‘size’
  246 |     return db->size;
      |              ^~

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?

>  uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb)
>  {
>      const DisasContextBase *db = tcg_ctx->plugin_db;

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro