[PATCH 0/2] plugins: add tb convenience functions

[PATCH 0/2] plugins: add tb convenience functions

[Luke Craig]

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.

Luke Craig (2):
  plugin: extend API with qemu_plugin_tb_get_insn_by_vaddr
  plugin: extend API with qemu_plugin_tb_size

 include/qemu/qemu-plugin.h | 21 +++++++++++++++++++++
 plugins/api.c              | 18 ++++++++++++++++++
 2 files changed, 39 insertions(+)

-- 
2.34.1

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

[Luke Craig]

---
 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;
+}
+
 /*
  * Instruction information
  *
-- 
2.34.1

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

[Luke Craig]

---
 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;
+}
+
 uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb)
 {
     const DisasContextBase *db = tcg_ctx->plugin_db;
-- 
2.34.1

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

[Alex Bennée]

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

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

[Alex Bennée]

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

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

[Pierrick Bouvier]

Hi Luke,

On 1/27/25 12:17, Luke Craig wrote:
> ---
>   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;
> +}
> +
>   uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb)
>   {
>       const DisasContextBase *db = tcg_ctx->plugin_db;

by tb size, do you mean the size, in bytes, of all (original) 
instructions of the tb?

If yes, you can get it by fetching first and last instruction, and 
compute the difference between last->vaddr + last->len -  first->vaddr.

Regards,
Pierrick