qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH] plugins/api: expose symbol lookup to plugins


From: Mahmoud Mandour
Subject: Re: [RFC PATCH] plugins/api: expose symbol lookup to plugins
Date: Wed, 2 Jun 2021 08:19:15 +0200



On Tue, Jun 1, 2021 at 4:58 PM Alex Bennée <alex.bennee@linaro.org> wrote:
This is a quality of life helper for plugins so they don't need to
re-implement symbol lookup when dumping an address. The strings are
constant so don't need to be duplicated. One minor tweak is to return
NULL instead of a zero length string to show lookup failed.
Thank you for implementing this, I found it a really easy addition since you
already told me how this is done in the kick-off meeting and I implemented it
but I'm glad you already posted it :D

Based-on: 20210530063712.6832-4-ma.mandourr@gmail.com
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/qemu/qemu-plugin.h |  9 +++++++++
 contrib/plugins/cache.c    | 10 ++++++++--
 plugins/api.c              |  6 ++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 97cdfd7761..dc3496f36c 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -525,6 +525,15 @@ qemu_plugin_register_vcpu_syscall_ret_cb(qemu_plugin_id_t id,

 char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn);

+/**
+ * qemu_plugin_insn_symbol() - best effort symbol lookup
+ * @insn: instruction reference
+ *
+ * Return a static string referring to the symbol. This is dependent
+ * on the binary QEMU is running having provided a symbol table.
+ */
+const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn);
+
 /**
  * qemu_plugin_vcpu_for_each() - iterate over the existing vCPU
  * @id: plugin ID
diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
index 1e323494bf..afaa3d9db5 100644
--- a/contrib/plugins/cache.c
+++ b/contrib/plugins/cache.c
@@ -46,6 +46,7 @@ enum AccessResult {

 struct InsnData {
     char *disas_str;
+    const char *symbol;
     uint64_t addr;
     uint64_t misses;
 };
@@ -377,10 +378,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
         struct InsnData *idata = g_new(struct InsnData, 1);

         ddata->disas_str = qemu_plugin_insn_disas(insn);
+        ddata->symbol = qemu_plugin_insn_symbol(insn);
         ddata->misses = 0;
         ddata->addr = effective_addr;

         idata->disas_str = g_strdup(ddata->disas_str);
+        idata->symbol = qemu_plugin_insn_symbol(insn);
         idata->misses = 0;
         idata->addr = effective_addr;

@@ -397,8 +400,11 @@ static void print_entry(gpointer data)
 {
     struct InsnData *insn = (struct InsnData *) data;
     g_autoptr(GString) xx = g_string_new("");
-    g_string_append_printf(xx, "0x%" PRIx64 ": %s - misses: %lu\n",
-            insn->addr, insn->disas_str, insn->misses);
+    g_string_append_printf(xx, "0x%" PRIx64, insn->addr);
+    if (insn->symbol) {
+        g_string_append_printf(xx, " (%s)", insn->symbol);
+    }
+    g_string_append_printf(xx, ", %lu, %s\n", insn->misses, insn->disas_str);
     qemu_plugin_outs(xx->str);
 }

diff --git a/plugins/api.c b/plugins/api.c
index 817c9b6b69..332e2c60e2 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -233,6 +233,12 @@ char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn)
     return plugin_disas(cpu, insn->vaddr, insn->data->len);
 } 
+const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn)
+{
+    const char *sym = lookup_symbol(insn->vaddr);
+    return sym[0] != 0 ? sym : NULL;
+}
+
 /*
  * The memory queries allow the plugin to query information about a
  * memory access.
--
2.20.1


How can I address such an addition? Should I add it to my next RFC series
under your name using your Signed-off-by line? Also, I think that somethings
in my series that you're basing your patch on will be changed, such as having
two duplicated entries of InsnData and the stupid name "xx" of the report string
How can I base your patch after my edits?

Thanks,
Mahmoud

reply via email to

[Prev in Thread] Current Thread [Next in Thread]