[PATCH 0/4] plugins/execlog: add data address match and address range support

[PATCH 0/4] plugins/execlog: add data address match and address range support

[Sven Schnelle]

Hi List,

this patchset adds a new -dfilter option and address range matching. With this
execlog can match only a certain range of address for both instruction and
data adresses.

Example usage:

qemu-system-xxx <other options> -d plugin -plugin libexeclog.so,afilter=0x1000-0x2000,dfilter=0x388

This would only log instruction in the address range 0x1000 to 0x2000
and accessing data at address 0x388.

Sven Schnelle (4):
  plugins/execlog: add struct execlog_ctx
  plugins/execlog: pass matches array to parse_vaddr_match
  plugins/execlog: add data address match
  plugins/execlog: add address range matching

 contrib/plugins/execlog.c | 102 ++++++++++++++++++++++++++++++--------
 1 file changed, 82 insertions(+), 20 deletions(-)

-- 
2.43.2

[PATCH 1/4] plugins/execlog: add struct execlog_ctx

[Sven Schnelle]

Add a context structure for future enhancements. No functional
change intended.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 contrib/plugins/execlog.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 82dc2f584e..90da1911b2 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -24,6 +24,10 @@ static GRWLock expand_array_lock;
 static GPtrArray *imatches;
 static GArray *amatches;
 
+struct execlog_ctx {
+    GString *s;
+};
+
 /*
  * Expand last_exec array.
  *
@@ -34,8 +38,9 @@ static void expand_last_exec(int cpu_index)
 {
     g_rw_lock_writer_lock(&expand_array_lock);
     while (cpu_index >= last_exec->len) {
-        GString *s = g_string_new(NULL);
-        g_ptr_array_add(last_exec, s);
+        struct execlog_ctx *ctx = g_new(struct execlog_ctx, 1);
+        ctx->s = g_string_new(NULL);
+        g_ptr_array_add(last_exec, ctx);
     }
     g_rw_lock_writer_unlock(&expand_array_lock);
 }
@@ -46,14 +51,13 @@ static void expand_last_exec(int cpu_index)
 static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
                      uint64_t vaddr, void *udata)
 {
-    GString *s;
-
     /* Find vCPU in array */
     g_rw_lock_reader_lock(&expand_array_lock);
     g_assert(cpu_index < last_exec->len);
-    s = g_ptr_array_index(last_exec, cpu_index);
+    struct execlog_ctx *ctx = g_ptr_array_index(last_exec, cpu_index);
     g_rw_lock_reader_unlock(&expand_array_lock);
 
+    GString *s = ctx->s;
     /* Indicate type of memory access */
     if (qemu_plugin_mem_is_store(info)) {
         g_string_append(s, ", store");
@@ -77,8 +81,6 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
  */
 static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
 {
-    GString *s;
-
     /* Find or create vCPU in array */
     g_rw_lock_reader_lock(&expand_array_lock);
     if (cpu_index >= last_exec->len) {
@@ -86,8 +88,9 @@ static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
         expand_last_exec(cpu_index);
         g_rw_lock_reader_lock(&expand_array_lock);
     }
-    s = g_ptr_array_index(last_exec, cpu_index);
+    struct execlog_ctx *ctx = g_ptr_array_index(last_exec, cpu_index);
     g_rw_lock_reader_unlock(&expand_array_lock);
+    GString *s = ctx->s;
 
     /* Print previous instruction in cache */
     if (s->len) {
@@ -183,9 +186,10 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
 static void plugin_exit(qemu_plugin_id_t id, void *p)
 {
     guint i;
-    GString *s;
+
     for (i = 0; i < last_exec->len; i++) {
-        s = g_ptr_array_index(last_exec, i);
+        struct execlog_ctx *ctx = g_ptr_array_index(last_exec, i);
+        GString *s = ctx->s;
         if (s->str) {
             qemu_plugin_outs(s->str);
             qemu_plugin_outs("\n");
-- 
2.43.2

[PATCH 2/4] plugins/execlog: pass matches array to parse_vaddr_match

[Sven Schnelle]

Pass the matches array to parse_vaddr_match(), so future address
matches can reuse that function.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 contrib/plugins/execlog.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 90da1911b2..b4b5ba113c 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -206,14 +206,14 @@ static void parse_insn_match(char *match)
     g_ptr_array_add(imatches, match);
 }
 
-static void parse_vaddr_match(char *match)
+static void parse_vaddr_match(GArray **matches, char *match)
 {
     uint64_t v = g_ascii_strtoull(match, NULL, 16);
 
-    if (!amatches) {
-        amatches = g_array_new(false, true, sizeof(uint64_t));
+    if (!matches) {
+        *matches = g_array_new(false, true, sizeof(uint64_t));
     }
-    g_array_append_val(amatches, v);
+    g_array_append_val(*matches, v);
 }
 
 /**
@@ -239,7 +239,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
         if (g_strcmp0(tokens[0], "ifilter") == 0) {
             parse_insn_match(tokens[1]);
         } else if (g_strcmp0(tokens[0], "afilter") == 0) {
-            parse_vaddr_match(tokens[1]);
+            parse_vaddr_match(&amatches, tokens[1]);
         } else {
             fprintf(stderr, "option parsing failed: %s\n", opt);
             return -1;
-- 
2.43.2

[PATCH 3/4] plugins/execlog: add data address match

[Sven Schnelle]

Add a match similar to the afilter address match, but for data
addresses. When an address is specified with '-dfilter=0x12345'
only load/stores to/from address 0x12345 are printed. All other
instructions are hidden.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 contrib/plugins/execlog.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index b4b5ba113c..33fef9bfc6 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -23,9 +23,11 @@ static GRWLock expand_array_lock;
 
 static GPtrArray *imatches;
 static GArray *amatches;
+static GArray *dmatches;
 
 struct execlog_ctx {
     GString *s;
+    bool log;
 };
 
 /*
@@ -45,6 +47,17 @@ static void expand_last_exec(int cpu_index)
     g_rw_lock_writer_unlock(&expand_array_lock);
 }
 
+static bool match_vaddr(struct execlog_ctx *ctx, uint64_t vaddr)
+{
+    for (int i = 0; i < dmatches->len; i++) {
+        uint64_t v = g_array_index(dmatches, uint64_t, i);
+        if (v == vaddr) {
+            ctx->log = true;
+            return true;
+        }
+    }
+    return false;
+}
 /**
  * Add memory read or write information to current instruction log
  */
@@ -57,6 +70,9 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
     struct execlog_ctx *ctx = g_ptr_array_index(last_exec, cpu_index);
     g_rw_lock_reader_unlock(&expand_array_lock);
 
+    if (dmatches && !match_vaddr(ctx, vaddr)) {
+        return;
+    }
     GString *s = ctx->s;
     /* Indicate type of memory access */
     if (qemu_plugin_mem_is_store(info)) {
@@ -93,7 +109,7 @@ static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
     GString *s = ctx->s;
 
     /* Print previous instruction in cache */
-    if (s->len) {
+    if (ctx->log && s->len) {
         qemu_plugin_outs(s->str);
         qemu_plugin_outs("\n");
     }
@@ -102,6 +118,7 @@ static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
     /* vcpu_mem will add memory access information to last_exec */
     g_string_printf(s, "%u, ", cpu_index);
     g_string_append(s, (char *)udata);
+    ctx->log = dmatches ? false : true;
 }
 
 /**
@@ -190,7 +207,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     for (i = 0; i < last_exec->len; i++) {
         struct execlog_ctx *ctx = g_ptr_array_index(last_exec, i);
         GString *s = ctx->s;
-        if (s->str) {
+        if (ctx->log && s->str) {
             qemu_plugin_outs(s->str);
             qemu_plugin_outs("\n");
         }
@@ -240,6 +257,8 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
             parse_insn_match(tokens[1]);
         } else if (g_strcmp0(tokens[0], "afilter") == 0) {
             parse_vaddr_match(&amatches, tokens[1]);
+        } else if (g_strcmp0(tokens[0], "dfilter") == 0) {
+            parse_vaddr_match(&dmatches, tokens[1]);
         } else {
             fprintf(stderr, "option parsing failed: %s\n", opt);
             return -1;
-- 
2.43.2

[PATCH 4/4] plugins/execlog: add address range matching

[Sven Schnelle]

Allow to match memory ranges with the address matches. This
allows to give a range of adresses like '-dfilter=0-0x400'
which would only log memory accesses between 0 and 400.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 contrib/plugins/execlog.c | 65 +++++++++++++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 13 deletions(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 33fef9bfc6..a505f98be8 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -30,6 +30,11 @@ struct execlog_ctx {
     bool log;
 };
 
+struct address_match {
+    uint64_t low;
+    uint64_t high;
+};
+
 /*
  * Expand last_exec array.
  *
@@ -47,17 +52,18 @@ static void expand_last_exec(int cpu_index)
     g_rw_lock_writer_unlock(&expand_array_lock);
 }
 
-static bool match_vaddr(struct execlog_ctx *ctx, uint64_t vaddr)
+static bool match_address_range(GArray *match, uint64_t vaddr)
 {
-    for (int i = 0; i < dmatches->len; i++) {
-        uint64_t v = g_array_index(dmatches, uint64_t, i);
-        if (v == vaddr) {
-            ctx->log = true;
+    for (int i = 0; i < match->len; i++) {
+        struct address_match *m =
+            g_array_index(match, struct address_match *, i);
+        if (vaddr >= m->low && vaddr <= m->high) {
             return true;
         }
     }
     return false;
 }
+
 /**
  * Add memory read or write information to current instruction log
  */
@@ -70,9 +76,10 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
     struct execlog_ctx *ctx = g_ptr_array_index(last_exec, cpu_index);
     g_rw_lock_reader_unlock(&expand_array_lock);
 
-    if (dmatches && !match_vaddr(ctx, vaddr)) {
+    if (dmatches && !match_address_range(dmatches, vaddr)) {
         return;
     }
+    ctx->log = true;
     GString *s = ctx->s;
     /* Indicate type of memory access */
     if (qemu_plugin_mem_is_store(info)) {
@@ -166,8 +173,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
         if (skip && amatches) {
             int j;
             for (j = 0; j < amatches->len && skip; j++) {
-                uint64_t v = g_array_index(amatches, uint64_t, j);
-                if (v == insn_vaddr) {
+                if (match_address_range(amatches, insn_vaddr)) {
                     skip = false;
                 }
             }
@@ -197,6 +203,16 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
     }
 }
 
+static void free_matches(GArray *matches)
+{
+    if (!matches) {
+        return;
+    }
+
+    for (int i = 0; i < matches->len; i++) {
+        g_free(g_array_index(matches, struct address_match *, i));
+    }
+}
 /**
  * On plugin exit, print last instruction in cache
  */
@@ -212,6 +228,9 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
             qemu_plugin_outs("\n");
         }
     }
+
+    free_matches(amatches);
+    free_matches(dmatches);
 }
 
 /* Add a match to the array of matches */
@@ -223,14 +242,34 @@ static void parse_insn_match(char *match)
     g_ptr_array_add(imatches, match);
 }
 
-static void parse_vaddr_match(GArray **matches, char *match)
+static void parse_vaddr_match(GArray **matches, char *token)
 {
-    uint64_t v = g_ascii_strtoull(match, NULL, 16);
+    uint64_t low, high;
+    gchar *endp;
 
-    if (!matches) {
-        *matches = g_array_new(false, true, sizeof(uint64_t));
+    low = g_ascii_strtoull(token, &endp, 16);
+    if (endp == token) {
+        fprintf(stderr, "Invalid address(range) specified: %s\n", token);
+        return;
+    }
+
+    if (*endp != '-') {
+        high = low;
+    } else {
+        high = g_ascii_strtoull(endp + 1, &endp, 16);
+        if (endp == token) {
+            fprintf(stderr, "Invalid address(range) specified: %s\n", token);
+            return;
+        }
+    }
+
+    if (!*matches) {
+        *matches = g_array_new(false, true, sizeof(struct address_match));
     }
-    g_array_append_val(*matches, v);
+    struct address_match *match = g_new(struct address_match, 1);
+    match->low = low;
+    match->high = high;
+    g_array_append_val(*matches, match);
 }
 
 /**
-- 
2.43.2

Re: [PATCH 0/4] plugins/execlog: add data address match and address range support

[Pierrick Bouvier]

Hi Sven, thanks for your series.

Yesterday, series for new API to access registers from plugins was 
merged. As part of it, execlog plugin was extended to support this [1].
This conflict with the changes presented here.

Could you please rebase this series on top of master?

Thanks,
Pierrick

[1] 
https://gitlab.com/qemu-project/qemu/-/commit/af6e4e0a22c18a7cc97650caec56ed99c9899dd7

On 2/29/24 12:02 AM, Sven Schnelle wrote:
> Hi List,
> 
> this patchset adds a new -dfilter option and address range matching. With this
> execlog can match only a certain range of address for both instruction and
> data adresses.
> 
> Example usage:
> 
> qemu-system-xxx <other options> -d plugin -plugin libexeclog.so,afilter=0x1000-0x2000,dfilter=0x388
> 
> This would only log instruction in the address range 0x1000 to 0x2000
> and accessing data at address 0x388.
> 
> Sven Schnelle (4):
>    plugins/execlog: add struct execlog_ctx
>    plugins/execlog: pass matches array to parse_vaddr_match
>    plugins/execlog: add data address match
>    plugins/execlog: add address range matching
> 
>   contrib/plugins/execlog.c | 102 ++++++++++++++++++++++++++++++--------
>   1 file changed, 82 insertions(+), 20 deletions(-)
> 

Re: [PATCH 0/4] plugins/execlog: add data address match and address range support

[Sven Schnelle]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> Hi Sven, thanks for your series.
>
> Yesterday, series for new API to access registers from plugins was
> merged. As part of it, execlog plugin was extended to support this
> [1].
> This conflict with the changes presented here.
>
> Could you please rebase this series on top of master?

I sent out a rebased version. Should have done a git pull before...

Thanks,
Sven