Hi Akihiko, and thanks for contributing this new plugin.
Recently, plugins documentation has been modified, and list of plugins
and their doc is now in "docs/about/emulation.rst". You may want to
rebase on top of master.
Globally, I'm ok with this plugin and the implementation. Just a few
fixes are needed for concurrent accesses.
On 8/12/24 23:46, Akihiko Odaki wrote:
SimPoint is a widely used tool to find the ideal microarchitecture
simulation points so Valgrind[2] and Pin[3] support generating basic
block vectors for use with them. Let's add a corresponding plugin to
QEMU too.
Note that this plugin has a different goal with tests/plugin/bb.c.
This plugin creates a vector for each constant interval instead of
counting the execution of basic blocks for the entire run and able to
describe the change of execution behavior. Its output is also
syntactically simple and better suited for parsing, while the output of
tests/plugin/bb.c is more human-readable.
I think it can be confusing to have two plugins named bb. How about
simpoint, or bbv?
[1] https://cseweb.ucsd.edu/~calder/simpoint/
[2] https://valgrind.org/docs/manual/bbv-manual.html
[3]
https://www.intel.com/content/www/us/en/developer/articles/tool/pin-a-dynamic-binary-instrumentation-tool.html
Signed-off-by: Yotaro Nada <yotaro.n...@gmail.com>
Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
---
docs/devel/tcg-plugins.rst | 20 ++++++
contrib/plugins/bb.c | 153 +++++++++++++++++++++++++++++++++++++++++++++
contrib/plugins/Makefile | 1 +
3 files changed, 174 insertions(+)
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 9cc09d8c3da1..2859eecc13b9 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -332,6 +332,26 @@ run::
160 1 0
135 1 0
+- contrib/plugins/bb.c
+
+The bb plugin allows you to generates basic block vectors for use with the
+`SimPoint <https://cseweb.ucsd.edu/~calder/simpoint/>`__ analysis tool.
+
+It has two options, ``interval`` and ``outfile``. ``interval`` specifies the
+interval to generate a basic block vector by the number of instructions. It is
+optional, and its default value is 100000000. ``outfile`` is the path to
+output files, and it will be suffixed with ``.N.bb`` where ``N`` is a vCPU
+index.
+
+Example::
+
+ $ qemu-aarch64 \
+ -plugin contrib/plugins/libb.so,interval=100,outfile=sha1 \
+ tests/tcg/aarch64-linux-user/sha1
+ SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
+ $ du sha1.0.bb
+ 23128 sha1.0.bb
+
- contrib/plugins/hotblocks.c
The hotblocks plugin allows you to examine the where hot paths of
diff --git a/contrib/plugins/bb.c b/contrib/plugins/bb.c
new file mode 100644
index 000000000000..4f1266d07ff5
--- /dev/null
+++ b/contrib/plugins/bb.c
@@ -0,0 +1,153 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
A brief summary and the link to simpoint page can be added as a comment
here.
+#include <stdio.h>
+#include <glib.h>
+
+#include <qemu-plugin.h>
+
+typedef struct Bb {
+ struct qemu_plugin_scoreboard *count;
+ unsigned int index;
+} Bb;
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+static GHashTable *bbs;
+static GPtrArray *files;
+static char *filename;
+static struct qemu_plugin_scoreboard *count;
+static uint64_t interval = 100000000;
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+ g_hash_table_unref(bbs);
+ g_ptr_array_unref(files);
+ g_free(filename);
+ qemu_plugin_scoreboard_free(count);
+}
+
+static void free_bb(void *data)
+{
+ qemu_plugin_scoreboard_free(((Bb *)data)->count);
+ g_free(data);
+}
+
+static void free_file(void *data)
+{
+ fclose(data);
+}
+ > +static directly count_u64(void)
+{
+ return qemu_plugin_scoreboard_u64(count);
+}
+
+static qemu_plugin_u64 bb_count_u64(Bb *bb)
+{
+ return qemu_plugin_scoreboard_u64(bb->count);
+}
+
+static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+ g_autofree gchar *vcpu_filename = NULL;
+
+ if (vcpu_index >= files->len) {
+ g_ptr_array_set_size(files, vcpu_index + 1);
+ } else if (g_ptr_array_index(files, vcpu_index)) {
+ return;
+ }
+
You need a lock for files array for expansion/access.
+ vcpu_filename = g_strdup_printf("%s.%u.bb", filename, vcpu_index);
+ g_ptr_array_index(files, vcpu_index) = fopen(vcpu_filename, "w");
+}
+
+static void vcpu_tb_exec(unsigned int vcpu_index, void *udata)
vcpu_tb_exec is a confusing name, as it is called when interval of insn
is executed. vcpu_interval_exec would be more clear.
+{
+ FILE *file = g_ptr_array_index(files, vcpu_index);
Need lock when accessing this array.
+ uint64_t count = qemu_plugin_u64_get(count_u64(), vcpu_index) - interval;
+ GHashTableIter iter;
+ void *value;
+
+ if (!file) {
+ return;
+ }
+
+ qemu_plugin_u64_set(count_u64(), vcpu_index, count);
+ > + fputc('T', file);
Maybe you can add a comment with a link to
https://valgrind.org/docs/manual/bbv-manual.html at this point. It's
already mentioned in the commit message, but IMHO, it's convenient to
have it here, so we know which data is written exactly.
+
+ g_hash_table_iter_init(&iter, bbs);
+
The access to this hashtable should be under a dedicated lock.
+ while (g_hash_table_iter_next(&iter, NULL, &value)) {
+ Bb *bb = value;
+ uint64_t bb_count = qemu_plugin_u64_get(bb_count_u64(bb), vcpu_index);
+
+ if (!bb_count) {
+ continue;
+ }
+
+ fprintf(file, ":%u:%" PRIu64 " ", bb->index, bb_count);
+ qemu_plugin_u64_set(bb_count_u64(bb), vcpu_index, 0);
+ }
+
+ fputc('\n', file);
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+ uint64_t n_insns = qemu_plugin_tb_n_insns(tb);
+ uint64_t vaddr = qemu_plugin_tb_vaddr(tb);
Need lock for hashtable access.
+ Bb *bb = g_hash_table_lookup(bbs, &vaddr);
+
+ if (!bb) {
+ uint64_t *key = g_new(uint64_t, 1);
+
+ *key = vaddr;
+ bb = g_new(Bb, 1);
+ g_hash_table_insert(bbs, key, bb);
+ bb->count = qemu_plugin_scoreboard_new(sizeof(uint64_t));
+ bb->index = g_hash_table_size(bbs);
+ }
+
+ qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+ tb, QEMU_PLUGIN_INLINE_ADD_U64, count_u64(), n_insns);
+
+ qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+ tb, QEMU_PLUGIN_INLINE_ADD_U64, bb_count_u64(bb), n_insns);
+
+ qemu_plugin_register_vcpu_tb_exec_cond_cb(
+ tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS,
+ QEMU_PLUGIN_COND_GE, count_u64(), interval, NULL);
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+ const qemu_info_t *info,
+ int argc, char **argv)
+{
+ for (int i = 0; i < argc; i++) {
+ char *opt = argv[i];
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
+ if (g_strcmp0(tokens[0], "interval") == 0) {
+ interval = g_ascii_strtoull(tokens[1], NULL, 10);
+ } else if (g_strcmp0(tokens[0], "outfile") == 0) {
+ filename = tokens[1];
+ tokens[1] = NULL;
+ } else {
+ fprintf(stderr, "option parsing failed: %s\n", opt);
+ return -1;
+ }
+ }
+
+ if (!filename) {
+ fputs("outfile unspecified\n", stderr);
+ return -1;
+ }
+
+ bbs = g_hash_table_new_full(g_int64_hash, g_int64_equal, g_free, free_bb);
+ files = g_ptr_array_new_with_free_func(free_file);
+ count = qemu_plugin_scoreboard_new(sizeof(uint64_t));
+ qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+ qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
+ qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+
+ return 0;
+}
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index 0b64d2c1e3a9..14bc88bb4f86 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -13,6 +13,7 @@ TOP_SRC_PATH = $(SRC_PATH)/../..
VPATH += $(SRC_PATH)
NAMES :=
+NAMES += bb
NAMES += execlog
NAMES += hotblocks
NAMES += hotpages
---
base-commit: 74abb45dac6979e7ff76172b7f0a24e869405184
change-id: 20240618-bb-93387ddf765b
Best regards,
Regards,
Pierrick
