diff mbox series

[v4,6/7] tests/plugin/mem: add option to print memory accesses

Message ID 20240702184448.551705-7-pierrick.bouvier@linaro.org (mailing list archive)
State New
Headers show
Series plugins: access values during a memory read/write | expand

Commit Message

Pierrick Bouvier July 2, 2024, 6:44 p.m. UTC 
By using "print-accesses=true" option, mem plugin will now print every
value accessed, with associated size, type (store vs load), symbol,
instruction address and phys/virt address accessed.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 tests/plugin/mem.c | 69 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

Comments

Edgar E. Iglesias" via July 3, 2024, 1:56 a.m. UTC | #1 
Tested-by: Xingtao Yao <yaoxt.fnst@fujitsu.com>

one small suggestion:
Keeping the addresses or values of fixed size in output message can improve the readability of logs.
like:
> +    case QEMU_PLUGIN_MEM_VALUE_U8:
> +        g_string_append_printf(out, "0x%"PRIx8, value.data.u8);
> +        break;
case QEMU_PLUGIN_MEM_VALUE_U8:
    g_string_append_printf(out, "0x02%"PRIx8, value.data.u8);
    break;


> -----Original Message-----
> From: qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org
> <qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org> On Behalf Of
> Pierrick Bouvier
> Sent: Wednesday, July 3, 2024 2:45 AM
> To: qemu-devel@nongnu.org
> Cc: Alex Bennée <alex.bennee@linaro.org>; Mahmoud Mandour
> <ma.mandourr@gmail.com>; Pierrick Bouvier <pierrick.bouvier@linaro.org>;
> Alexandre Iooss <erdnaxe@crans.org>; Philippe Mathieu-Daudé
> <philmd@linaro.org>; Paolo Bonzini <pbonzini@redhat.com>; Richard Henderson
> <richard.henderson@linaro.org>; Eduardo Habkost <eduardo@habkost.net>
> Subject: [PATCH v4 6/7] tests/plugin/mem: add option to print memory accesses
> 
> By using "print-accesses=true" option, mem plugin will now print every
> value accessed, with associated size, type (store vs load), symbol,
> instruction address and phys/virt address accessed.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  tests/plugin/mem.c | 69
> +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
> index b650dddcce1..aecbad8e68d 100644
> --- a/tests/plugin/mem.c
> +++ b/tests/plugin/mem.c
> @@ -21,10 +21,15 @@ typedef struct {
>      uint64_t io_count;
>  } CPUCount;
> 
> +typedef struct {
> +    uint64_t vaddr;
> +    const char *sym;
> +} InsnInfo;
> +
>  static struct qemu_plugin_scoreboard *counts;
>  static qemu_plugin_u64 mem_count;
>  static qemu_plugin_u64 io_count;
> -static bool do_inline, do_callback;
> +static bool do_inline, do_callback, do_print_accesses;
>  static bool do_haddr;
>  static enum qemu_plugin_mem_rw rw = QEMU_PLUGIN_MEM_RW;
> 
> @@ -60,6 +65,44 @@ static void vcpu_mem(unsigned int cpu_index,
> qemu_plugin_meminfo_t meminfo,
>      }
>  }
> 
> +static void print_access(unsigned int cpu_index, qemu_plugin_meminfo_t
> meminfo,
> +                         uint64_t vaddr, void *udata)
> +{
> +    InsnInfo *insn_info = udata;
> +    unsigned size = 8 << qemu_plugin_mem_size_shift(meminfo);
> +    const char *type = qemu_plugin_mem_is_store(meminfo) ? "store" : "load";
> +    qemu_plugin_mem_value value = qemu_plugin_mem_get_value(meminfo);
> +    uint64_t hwaddr =
> +        qemu_plugin_hwaddr_phys_addr(qemu_plugin_get_hwaddr(meminfo,
> vaddr));
> +    g_autoptr(GString) out = g_string_new("");
> +    g_string_printf(out,
> +                    "0x%"PRIx64",%s,0x%"PRIx64",0x%"PRIx64",%d,%s,",
> +                    insn_info->vaddr, insn_info->sym,
> +                    vaddr, hwaddr, size, type);
> +    switch (value.type) {
> +    case QEMU_PLUGIN_MEM_VALUE_U8:
> +        g_string_append_printf(out, "0x%"PRIx8, value.data.u8);
> +        break;
> +    case QEMU_PLUGIN_MEM_VALUE_U16:
> +        g_string_append_printf(out, "0x%"PRIx16, value.data.u16);
> +        break;
> +    case QEMU_PLUGIN_MEM_VALUE_U32:
> +        g_string_append_printf(out, "0x%"PRIx32, value.data.u32);
> +        break;
> +    case QEMU_PLUGIN_MEM_VALUE_U64:
> +        g_string_append_printf(out, "0x%"PRIx64, value.data.u64);
> +        break;
> +    case QEMU_PLUGIN_MEM_VALUE_U128:
> +        g_string_append_printf(out, "0x%.0"PRIx64"%"PRIx64,
> +                               value.data.u128.high, value.data.u128.low);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    g_string_append_printf(out, "\n");
> +    qemu_plugin_outs(out->str);
> +}
> +
>  static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>  {
>      size_t n = qemu_plugin_tb_n_insns(tb);
> @@ -79,6 +122,16 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
> qemu_plugin_tb *tb)
>                                               QEMU_PLUGIN_CB_NO_REGS,
>                                               rw, NULL);
>          }
> +        if (do_print_accesses) {
> +            /* we leak this pointer, to avoid locking to keep track of it */
> +            InsnInfo *insn_info = g_malloc(sizeof(InsnInfo));
> +            const char *sym = qemu_plugin_insn_symbol(insn);
> +            insn_info->sym = sym ? sym : "";
> +            insn_info->vaddr = qemu_plugin_insn_vaddr(insn);
> +            qemu_plugin_register_vcpu_mem_cb(insn, print_access,
> +                                             QEMU_PLUGIN_CB_NO_REGS,
> +                                             rw, (void *) insn_info);
> +        }
>      }
>  }
> 
> @@ -117,6 +170,12 @@ QEMU_PLUGIN_EXPORT int
> qemu_plugin_install(qemu_plugin_id_t id,
>                  fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
>                  return -1;
>              }
> +        } else if (g_strcmp0(tokens[0], "print-accesses") == 0) {
> +            if (!qemu_plugin_bool_parse(tokens[0], tokens[1],
> +                                        &do_print_accesses)) {
> +                fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
> +                return -1;
> +            }
>          } else {
>              fprintf(stderr, "option parsing failed: %s\n", opt);
>              return -1;
> @@ -129,6 +188,14 @@ QEMU_PLUGIN_EXPORT int
> qemu_plugin_install(qemu_plugin_id_t id,
>          return -1;
>      }
> 
> +    if (do_print_accesses) {
> +        g_autoptr(GString) out = g_string_new("");
> +        g_string_printf(out,
> +                "insn_vaddr,insn_symbol,mem_vaddr,mem_hwaddr,"
> +                "access_size,access_type,mem_value\n");
> +        qemu_plugin_outs(out->str);
> +    }
> +
>      counts = qemu_plugin_scoreboard_new(sizeof(CPUCount));
>      mem_count = qemu_plugin_scoreboard_u64_in_struct(
>          counts, CPUCount, mem_count);
> --
> 2.39.2
>
Richard Henderson July 4, 2024, 4:30 p.m. UTC | #2 
On 7/2/24 11:44, Pierrick Bouvier wrote:
> +    case QEMU_PLUGIN_MEM_VALUE_U128:
> +        g_string_append_printf(out, "0x%.0"PRIx64"%"PRIx64,
> +                               value.data.u128.high, value.data.u128.low);

PRIx64 does not pad.
You need %016"PRIx64 for the low value.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
diff mbox series

Patch

diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
index b650dddcce1..aecbad8e68d 100644
--- a/tests/plugin/mem.c
+++ b/tests/plugin/mem.c
@@ -21,10 +21,15 @@  typedef struct {
     uint64_t io_count;
 } CPUCount;
 
+typedef struct {
+    uint64_t vaddr;
+    const char *sym;
+} InsnInfo;
+
 static struct qemu_plugin_scoreboard *counts;
 static qemu_plugin_u64 mem_count;
 static qemu_plugin_u64 io_count;
-static bool do_inline, do_callback;
+static bool do_inline, do_callback, do_print_accesses;
 static bool do_haddr;
 static enum qemu_plugin_mem_rw rw = QEMU_PLUGIN_MEM_RW;
 
@@ -60,6 +65,44 @@  static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
     }
 }
 
+static void print_access(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
+                         uint64_t vaddr, void *udata)
+{
+    InsnInfo *insn_info = udata;
+    unsigned size = 8 << qemu_plugin_mem_size_shift(meminfo);
+    const char *type = qemu_plugin_mem_is_store(meminfo) ? "store" : "load";
+    qemu_plugin_mem_value value = qemu_plugin_mem_get_value(meminfo);
+    uint64_t hwaddr =
+        qemu_plugin_hwaddr_phys_addr(qemu_plugin_get_hwaddr(meminfo, vaddr));
+    g_autoptr(GString) out = g_string_new("");
+    g_string_printf(out,
+                    "0x%"PRIx64",%s,0x%"PRIx64",0x%"PRIx64",%d,%s,",
+                    insn_info->vaddr, insn_info->sym,
+                    vaddr, hwaddr, size, type);
+    switch (value.type) {
+    case QEMU_PLUGIN_MEM_VALUE_U8:
+        g_string_append_printf(out, "0x%"PRIx8, value.data.u8);
+        break;
+    case QEMU_PLUGIN_MEM_VALUE_U16:
+        g_string_append_printf(out, "0x%"PRIx16, value.data.u16);
+        break;
+    case QEMU_PLUGIN_MEM_VALUE_U32:
+        g_string_append_printf(out, "0x%"PRIx32, value.data.u32);
+        break;
+    case QEMU_PLUGIN_MEM_VALUE_U64:
+        g_string_append_printf(out, "0x%"PRIx64, value.data.u64);
+        break;
+    case QEMU_PLUGIN_MEM_VALUE_U128:
+        g_string_append_printf(out, "0x%.0"PRIx64"%"PRIx64,
+                               value.data.u128.high, value.data.u128.low);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    g_string_append_printf(out, "\n");
+    qemu_plugin_outs(out->str);
+}
+
 static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
 {
     size_t n = qemu_plugin_tb_n_insns(tb);
@@ -79,6 +122,16 @@  static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
                                              QEMU_PLUGIN_CB_NO_REGS,
                                              rw, NULL);
         }
+        if (do_print_accesses) {
+            /* we leak this pointer, to avoid locking to keep track of it */
+            InsnInfo *insn_info = g_malloc(sizeof(InsnInfo));
+            const char *sym = qemu_plugin_insn_symbol(insn);
+            insn_info->sym = sym ? sym : "";
+            insn_info->vaddr = qemu_plugin_insn_vaddr(insn);
+            qemu_plugin_register_vcpu_mem_cb(insn, print_access,
+                                             QEMU_PLUGIN_CB_NO_REGS,
+                                             rw, (void *) insn_info);
+        }
     }
 }
 
@@ -117,6 +170,12 @@  QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
                 fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
                 return -1;
             }
+        } else if (g_strcmp0(tokens[0], "print-accesses") == 0) {
+            if (!qemu_plugin_bool_parse(tokens[0], tokens[1],
+                                        &do_print_accesses)) {
+                fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+                return -1;
+            }
         } else {
             fprintf(stderr, "option parsing failed: %s\n", opt);
             return -1;
@@ -129,6 +188,14 @@  QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
         return -1;
     }
 
+    if (do_print_accesses) {
+        g_autoptr(GString) out = g_string_new("");
+        g_string_printf(out,
+                "insn_vaddr,insn_symbol,mem_vaddr,mem_hwaddr,"
+                "access_size,access_type,mem_value\n");
+        qemu_plugin_outs(out->str);
+    }
+
     counts = qemu_plugin_scoreboard_new(sizeof(CPUCount));
     mem_count = qemu_plugin_scoreboard_u64_in_struct(
         counts, CPUCount, mem_count);