[PATCH] contrib/plugins: add mask plugin

Oleg Vasilev posted 1 patch 2 years, 2 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220124111602.671654-1-me@svin.in
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>
contrib/plugins/Makefile |   1 +
contrib/plugins/mask.c   | 144 +++++++++++++++++++++++++++++++++++++++
2 files changed, 145 insertions(+)
create mode 100644 contrib/plugins/mask.c
[PATCH] contrib/plugins: add mask plugin
Posted by Oleg Vasilev 2 years, 2 months ago
From: Oleg Vasilev <vasilev.oleg@huawei.com>

Plugin can be used to track statistics based on virtual address mask matching.
Useful for tracking kernel vs user translation blocks.

Signed-off-by: Oleg Vasilev <vasilev.oleg@huawei.com>
Signed-off-by: Oleg Vasilev <me@svin.in>
---
 contrib/plugins/Makefile |   1 +
 contrib/plugins/mask.c   | 144 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 145 insertions(+)
 create mode 100644 contrib/plugins/mask.c

diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index 54ac5ccd9f..7e9cb51c9d 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -20,6 +20,7 @@ NAMES += howvec
 NAMES += lockstep
 NAMES += hwprofile
 NAMES += cache
+NAMES += mask
 
 SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
 
diff --git a/contrib/plugins/mask.c b/contrib/plugins/mask.c
new file mode 100644
index 0000000000..c6d2bd2386
--- /dev/null
+++ b/contrib/plugins/mask.c
@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) 2022, Oleg Vasilev <vasilev.oleg@huawei.com>
+ *
+ * Track statistics based on virtual address mask matching.
+ * Useful for tracking kernel vs user translation blocks.
+ *
+ * License: GNU GPL, version 2 or later.
+ *   See the COPYING file in the top-level directory.
+ */
+
+#include <assert.h>
+#include <compiler.h>
+#include <glib.h>
+#include <inttypes.h>
+#include <qemu-plugin.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <atomic.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+typedef struct {
+    GMutex lock;
+    const char *hint;
+    uint64_t mask;
+    uint64_t bits;
+    uint64_t tb_exec;
+    uint64_t tb_trans;
+    uint64_t isins;
+} MaskCounter;
+
+static GPtrArray *counters;
+
+static uint64_t report_every = 1 << 28;
+static uint64_t tb_exec_every = 1 << 10;
+static uint64_t total_tb_exec;
+
+static void gen_one_report(MaskCounter *counter, GString *report)
+{
+    g_mutex_lock(&counter->lock);
+    uint64_t tb_exec = counter->tb_exec * tb_exec_every;
+
+    double hit_rate = (double)counter->tb_trans / tb_exec;
+    hit_rate = 1 - hit_rate;
+
+    double mask_freq = (double) counter->tb_exec * tb_exec_every / report_every;
+
+    g_string_append_printf(report,
+                           "hint: %s, mask: 0x%016lx, bits: 0x%016lx, hit_rate: %f, "
+                           "mask_freq: %f, tb_exec: %ld, tb_trans: %ld\n",
+                           counter->hint, counter->mask, counter->bits, hit_rate,
+                           mask_freq, tb_exec, counter->tb_trans);
+
+    counter->tb_exec = 0;
+    counter->tb_trans = 0;
+    counter->isins = 0;
+
+    g_mutex_unlock(&counter->lock);
+}
+
+static void report_all(void)
+{
+    g_autoptr(GString) report = g_string_new("");
+    g_ptr_array_foreach(counters, (GFunc)gen_one_report, report);
+    qemu_plugin_outs(report->str);
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+    report_all();
+}
+
+static bool match(MaskCounter *counter, struct qemu_plugin_tb *tb)
+{
+    return (counter->mask & qemu_plugin_tb_vaddr(tb)) == counter->bits;
+}
+
+static void tb_exec(MaskCounter *counter, struct qemu_plugin_tb *tb)
+{
+    if (!match(counter, tb)) {
+        return;
+    }
+    g_mutex_lock(&counter->lock);
+    counter->tb_exec++;
+    g_mutex_unlock(&counter->lock);
+}
+
+static void vcpu_tb_exec(unsigned int cpu_index, void *tb)
+{
+    uint64_t cur_tb_exec = qatomic_fetch_inc(&total_tb_exec);
+    if ((cur_tb_exec & (tb_exec_every - 1)) == 0) {
+        g_ptr_array_foreach(counters, (GFunc)tb_exec, tb);
+    }
+
+    if ((cur_tb_exec & (report_every - 1)) == 0) {
+        report_all();
+    }
+}
+
+static void tb_trans(MaskCounter *counter, struct qemu_plugin_tb *tb)
+{
+    if (!match(counter, tb)) {
+        return;
+    }
+    g_mutex_lock(&counter->lock);
+    counter->tb_trans++;
+    g_mutex_unlock(&counter->lock);
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+    qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
+                                         QEMU_PLUGIN_CB_NO_REGS, tb);
+    g_ptr_array_foreach(counters, (GFunc)tb_trans, tb);
+}
+
+static void add_counter(const char *hint, uint64_t mask, uint64_t bits)
+{
+    MaskCounter *counter = g_new0(MaskCounter, 1);
+    counter->hint = hint;
+    counter->mask = mask;
+    counter->bits = bits;
+    g_mutex_init(&counter->lock);
+    g_ptr_array_add(counters, counter);
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+                                           const qemu_info_t *info, int argc,
+                                           char **argv)
+{
+    counters = g_ptr_array_new();
+
+    // Update for a different mask
+    add_counter("all", 0, 0);
+    add_counter("kernel", 0x1ll << 63, 0x1ll << 63);
+    add_counter("user", 0x1ll << 63, 0);
+
+    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+    return 0;
+}
-- 
2.33.1


Re: [PATCH] contrib/plugins: add mask plugin
Posted by Alex Bennée 2 years, 2 months ago
Oleg Vasilev <me@svin.in> writes:

> From: Oleg Vasilev <vasilev.oleg@huawei.com>
>
> Plugin can be used to track statistics based on virtual address mask matching.
> Useful for tracking kernel vs user translation blocks.

Could we have a bit more detail please, maybe some words for
devel/tcg/plugins.rst. Running:

./qemu-system-x86_64 -monitor none -display none -chardev file,path=test.out,id=output -device isa-debugcon,chardev=output -device isa-debug-exit,iobase=0xf4,iosize=0x4 -
kernel ./tests/tcg/x86_64-softmmu/memory -plugin contrib/plugins/libmask.so -d plugin
hint: all, mask: 0x0000000000000000, bits: 0x0000000000000000, hit_rate: 0.999023, mask_freq: 0.000004, tb_exec: 1024, tb_trans: 1
hint: kernel, mask: 0x8000000000000000, bits: 0x8000000000000000, hit_rate: -nan, mask_freq: 0.000000, tb_exec: 0, tb_trans: 0
hint: user, mask: 0x8000000000000000, bits: 0x0000000000000000, hit_rate: 0.999023, mask_freq: 0.000004, tb_exec: 1024, tb_trans: 1
hint: all, mask: 0x0000000000000000, bits: 0x0000000000000000, hit_rate: 0.999849, mask_freq: 0.078281, tb_exec: 21013504, tb_trans: 3169
hint: kernel, mask: 0x8000000000000000, bits: 0x8000000000000000, hit_rate: -nan, mask_freq: 0.000000, tb_exec: 0, tb_trans: 0
hint: user, mask: 0x8000000000000000, bits: 0x0000000000000000, hit_rate: 0.999849, mask_freq: 0.078281, tb_exec: 21013504, tb_trans: 3169

ends up being a bit incomprehensible.

>
> Signed-off-by: Oleg Vasilev <vasilev.oleg@huawei.com>
> Signed-off-by: Oleg Vasilev <me@svin.in>
> ---
>  contrib/plugins/Makefile |   1 +
>  contrib/plugins/mask.c   | 144 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 145 insertions(+)
>  create mode 100644 contrib/plugins/mask.c
>
> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> index 54ac5ccd9f..7e9cb51c9d 100644
> --- a/contrib/plugins/Makefile
> +++ b/contrib/plugins/Makefile
> @@ -20,6 +20,7 @@ NAMES += howvec
>  NAMES += lockstep
>  NAMES += hwprofile
>  NAMES += cache
> +NAMES += mask
>  
>  SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>  
> diff --git a/contrib/plugins/mask.c b/contrib/plugins/mask.c
> new file mode 100644
> index 0000000000..c6d2bd2386
> --- /dev/null
> +++ b/contrib/plugins/mask.c
> @@ -0,0 +1,144 @@
> +/*
> + * Copyright (C) 2022, Oleg Vasilev <vasilev.oleg@huawei.com>
> + *
> + * Track statistics based on virtual address mask matching.
> + * Useful for tracking kernel vs user translation blocks.
> + *
> + * License: GNU GPL, version 2 or later.
> + *   See the COPYING file in the top-level directory.
> + */
> +
> +#include <assert.h>
> +#include <compiler.h>
> +#include <glib.h>
> +#include <inttypes.h>
> +#include <qemu-plugin.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <atomic.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +typedef struct {
> +    GMutex lock;
> +    const char *hint;
> +    uint64_t mask;
> +    uint64_t bits;
> +    uint64_t tb_exec;
> +    uint64_t tb_trans;
> +    uint64_t isins;
> +} MaskCounter;
> +
> +static GPtrArray *counters;
> +
> +static uint64_t report_every = 1 << 28;
> +static uint64_t tb_exec_every = 1 << 10;
> +static uint64_t total_tb_exec;
> +
> +static void gen_one_report(MaskCounter *counter, GString *report)
> +{
> +    g_mutex_lock(&counter->lock);
> +    uint64_t tb_exec = counter->tb_exec * tb_exec_every;
> +
> +    double hit_rate = (double)counter->tb_trans / tb_exec;
> +    hit_rate = 1 - hit_rate;
> +
> +    double mask_freq = (double) counter->tb_exec * tb_exec_every / report_every;
> +
> +    g_string_append_printf(report,
> +                           "hint: %s, mask: 0x%016lx, bits: 0x%016lx, hit_rate: %f, "
> +                           "mask_freq: %f, tb_exec: %ld, tb_trans: %ld\n",
> +                           counter->hint, counter->mask, counter->bits, hit_rate,
> +                           mask_freq, tb_exec, counter->tb_trans);

Could aiming for plainer CSV format make this information more
digestible by tooling?

> +
> +    counter->tb_exec = 0;
> +    counter->tb_trans = 0;
> +    counter->isins = 0;

Would it be worth tracking total lifetime vs period counts?

> +
> +    g_mutex_unlock(&counter->lock);
> +}
> +
> +static void report_all(void)
> +{
> +    g_autoptr(GString) report = g_string_new("");
> +    g_ptr_array_foreach(counters, (GFunc)gen_one_report, report);
> +    qemu_plugin_outs(report->str);
> +}
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +    report_all();
> +}
> +
> +static bool match(MaskCounter *counter, struct qemu_plugin_tb *tb)
> +{
> +    return (counter->mask & qemu_plugin_tb_vaddr(tb)) == counter->bits;
> +}
> +
> +static void tb_exec(MaskCounter *counter, struct qemu_plugin_tb *tb)
> +{
> +    if (!match(counter, tb)) {
> +        return;
> +    }
> +    g_mutex_lock(&counter->lock);
> +    counter->tb_exec++;
> +    g_mutex_unlock(&counter->lock);
> +}
> +
> +static void vcpu_tb_exec(unsigned int cpu_index, void *tb)
> +{
> +    uint64_t cur_tb_exec = qatomic_fetch_inc(&total_tb_exec);
> +    if ((cur_tb_exec & (tb_exec_every - 1)) == 0) {
> +        g_ptr_array_foreach(counters, (GFunc)tb_exec, tb);
> +    }
> +
> +    if ((cur_tb_exec & (report_every - 1)) == 0) {
> +        report_all();
> +    }
> +}
> +
> +static void tb_trans(MaskCounter *counter, struct qemu_plugin_tb *tb)
> +{
> +    if (!match(counter, tb)) {
> +        return;
> +    }
> +    g_mutex_lock(&counter->lock);
> +    counter->tb_trans++;
> +    g_mutex_unlock(&counter->lock);
> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> +    qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
> +                                         QEMU_PLUGIN_CB_NO_REGS, tb);

You can't pass qemu_plugin_tb to the exec handler like this as it is
only valid during the lifetime of translation. You need to extract the
information you need (qemu_plugin_tb_vaddr(tb)) and pass that.

The restriction is the same as we state for qemu_plugin_tb_get_insn():

 The returned handle can be used in follow up helper queries as well
 as when instrumenting an instruction. It is only valid for the
 lifetime of the callback.

but it isn't attached to a API docs, we do mention it in the general
principles:

  Lifetime of the query handle
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  Each callback provides an opaque anonymous information handle which
  can usually be further queried to find out information about a
  translation, instruction or operation. The handles themselves are only
  valid during the lifetime of the callback so it is important that any
  information that is needed is extracted during the callback and saved
  by the plugin.


> +    g_ptr_array_foreach(counters, (GFunc)tb_trans, tb);
> +}
> +
> +static void add_counter(const char *hint, uint64_t mask, uint64_t bits)
> +{
> +    MaskCounter *counter = g_new0(MaskCounter, 1);
> +    counter->hint = hint;
> +    counter->mask = mask;
> +    counter->bits = bits;
> +    g_mutex_init(&counter->lock);
> +    g_ptr_array_add(counters, counter);
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> +                                           const qemu_info_t *info, int argc,
> +                                           char **argv)
> +{
> +    counters = g_ptr_array_new();
> +
> +    // Update for a different mask
> +    add_counter("all", 0, 0);
> +    add_counter("kernel", 0x1ll << 63, 0x1ll << 63);
> +    add_counter("user", 0x1ll << 63, 0);

These seem very fixed - do they apply for all kernels/architectures?
Perhaps we could have sensible defaults based on info->target_name with
an option to hand set zones via CLI options?

Also what should happen in the case of !info->system_emulation? There is
probably a case for something similar tracking execution in various .so
libs but that would need a little additional information from the
infrastructure to track.

> +
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +    return 0;
> +}


-- 
Alex Bennée