Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 plugin-examples/bbcount_avgsize_racy.c | 50 ++++++++++++++++++++++
 plugin-examples/mem_count_racy_both.c  | 58 ++++++++++++++++++++++++++
 plugin-examples/Makefile               | 31 ++++++++++++++
 3 files changed, 139 insertions(+)
 create mode 100644 plugin-examples/bbcount_avgsize_racy.c
 create mode 100644 plugin-examples/mem_count_racy_both.c
 create mode 100644 plugin-examples/Makefile

diff --git a/plugin-examples/bbcount_avgsize_racy.c b/plugin-examples/bbcount_avgsize_racy.c
new file mode 100644
index 0000000000..ccdf96c1fa
--- /dev/null
+++ b/plugin-examples/bbcount_avgsize_racy.c
@@ -0,0 +1,50 @@
+#include <inttypes.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#include <qemu-plugin.h>
+
+static uint64_t bb_count;
+static uint64_t insn_count;
+const char *filename;
+static int stdout_fd;
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+    dprintf(stdout_fd, "insns: %" PRIu64", bb: %" PRIu64 ", "
+            "avg insns/bb: %.2f\n",
+            insn_count, bb_count, (double)insn_count / bb_count);
+}
+
+static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
+{
+    unsigned long n_insns = (unsigned long)udata;
+
+    insn_count += n_insns;
+    bb_count++;
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, unsigned int cpu_index,
+                          struct qemu_plugin_tb *tb)
+{
+    unsigned long n_insns = qemu_plugin_tb_n_insns(tb);
+
+    qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
+                                         QEMU_PLUGIN_CB_NO_REGS,
+                                         (void *)n_insns);
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, int argc,
+                                           char **argv)
+{
+    /* plugin_exit might write to stdout after stdout has been closed */
+    stdout_fd = dup(STDOUT_FILENO);
+    assert(stdout_fd);
+
+    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+    return 0;
+}
diff --git a/plugin-examples/mem_count_racy_both.c b/plugin-examples/mem_count_racy_both.c
new file mode 100644
index 0000000000..a47f2025bf
--- /dev/null
+++ b/plugin-examples/mem_count_racy_both.c
@@ -0,0 +1,58 @@
+#include <inttypes.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#include <qemu-plugin.h>
+
+static uint64_t mem_count;
+static int stdout_fd;
+static bool do_inline;
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+    dprintf(stdout_fd, "accesses: %" PRIu64 "\n", mem_count);
+}
+
+static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
+                     uint64_t vaddr, void *udata)
+{
+    mem_count++;
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, unsigned int cpu_index,
+                          struct qemu_plugin_tb *tb)
+{
+    size_t n = qemu_plugin_tb_n_insns(tb);
+    size_t i;
+
+    for (i = 0; i < n; i++) {
+        struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
+
+        if (do_inline) {
+            qemu_plugin_register_vcpu_mem_inline(insn,
+                                                 QEMU_PLUGIN_INLINE_ADD_U64,
+                                                 &mem_count, 1);
+        } else {
+            qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem,
+                                             QEMU_PLUGIN_CB_NO_REGS, NULL);
+        }
+    }
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, int argc,
+                                           char **argv)
+{
+    if (argc && strcmp(argv[0], "inline") == 0) {
+        do_inline = true;
+    }
+    /* plugin_exit might write to stdout after stdout has been closed */
+    stdout_fd = dup(STDOUT_FILENO);
+    assert(stdout_fd);
+
+    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+    return 0;
+}
diff --git a/plugin-examples/Makefile b/plugin-examples/Makefile
new file mode 100644
index 0000000000..71bbcda7a8
--- /dev/null
+++ b/plugin-examples/Makefile
@@ -0,0 +1,31 @@
+NAMES :=
+NAMES += bbcount_avgsize_racy
+NAMES += mem_count_racy_both
+
+SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
+
+# QEMU installed path, set by --prefix during configure
+QEMU_PATH ?= /data/src/qemu-inst/plugin-test
+
+CC := gcc
+CFLAGS :=
+CFLAGS += -O2 -Werror -Wall
+CFLAGS += -Wundef -Wwrite-strings -Wmissing-prototypes
+CFLAGS += -Wstrict-prototypes -Wredundant-decls
+CFLAGS += -fno-strict-aliasing -fno-common -fwrapv
+CFLAGS += -I$(QEMU_PATH)/include
+LDLIBS := -lc
+
+all: $(SONAMES)
+
+%.o: %.c
+	$(CC) $(CFLAGS) -fPIC -c $< -o $@
+
+lib%.so: %.o
+	$(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)
+
+clean:
+	$(RM) -f *.o *.so
+	$(RM) -Rf .libs
+
+.PHONY: all clean
-- 
2.17.1

> From: Emilio G. Cota [mailto:cota@braap.org]
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  plugin-examples/bbcount_avgsize_racy.c | 50 ++++++++++++++++++++++
>  plugin-examples/mem_count_racy_both.c  | 58 ++++++++++++++++++++++++++
>  plugin-examples/Makefile               | 31 ++++++++++++++
>  3 files changed, 139 insertions(+)
>  create mode 100644 plugin-examples/bbcount_avgsize_racy.c
>  create mode 100644 plugin-examples/mem_count_racy_both.c
>  create mode 100644 plugin-examples/Makefile
> 

<snip>

> diff --git a/plugin-examples/mem_count_racy_both.c b/plugin-examples/mem_count_racy_both.c
> new file mode 100644
> index 0000000000..a47f2025bf
> --- /dev/null
> +++ b/plugin-examples/mem_count_racy_both.c
> @@ -0,0 +1,58 @@
> +#include <inttypes.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +#include <qemu-plugin.h>
> +
> +static uint64_t mem_count;
> +static int stdout_fd;
> +static bool do_inline;
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +    dprintf(stdout_fd, "accesses: %" PRIu64 "\n", mem_count);
> +}
> +
> +static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
> +                     uint64_t vaddr, void *udata)
> +{
> +    mem_count++;
> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, unsigned int cpu_index,
> +                          struct qemu_plugin_tb *tb)
> +{
> +    size_t n = qemu_plugin_tb_n_insns(tb);
> +    size_t i;
> +
> +    for (i = 0; i < n; i++) {
> +        struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> +
> +        if (do_inline) {
> +            qemu_plugin_register_vcpu_mem_inline(insn,
> +                                                 QEMU_PLUGIN_INLINE_ADD_U64,
> +                                                 &mem_count, 1);
> +        } else {
> +            qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem,
> +                                             QEMU_PLUGIN_CB_NO_REGS, NULL);
> +        }
> +    }
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, int argc,
> +                                           char **argv)
> +{
> +    if (argc && strcmp(argv[0], "inline") == 0) {
> +        do_inline = true;
> +    }
> +    /* plugin_exit might write to stdout after stdout has been closed */
> +    stdout_fd = dup(STDOUT_FILENO);
> +    assert(stdout_fd);
> +
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +    return 0;
> +}

Thanks for the series.
Can you provide more plugin examples for better understanding of double-translate idea?
E.g., plugins that hook specific instructions or addresses.

Pavel Dovgalyuk

On Mon, Oct 29, 2018 at 13:59:03 +0300, Pavel Dovgalyuk wrote:
> > From: Emilio G. Cota [mailto:cota@braap.org]
> > Signed-off-by: Emilio G. Cota <cota@braap.org>
(snip)
> Thanks for the series.
> Can you provide more plugin examples for better understanding of double-translate idea?
> E.g., plugins that hook specific instructions or addresses.

Here's a plugin that at TB translation time, disassembles the
instructions provided via capstone. It doesn't do anything
with that info (vcpu_tb_exec is empty), but say a simulator
would pass via the *udata pointer some descriptor (allocated
at translation time) based on the instructions in the TB.

Note that the disassembly happens because we already have
a fully formed TB thanks to the 2-pass translation. Without it,
we'd have to (1) perform additional loads in the guest to read
instructions given the PC, and (2) guess when the TB would end
(recall that when to finish a TB is a decision internal to QEMU).

Thanks,

		Emilio

PS. Compile with -lcapstone
---
#include <inttypes.h>
#include <assert.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <stdio.h>

#include <capstone/capstone.h>
#include <qemu-plugin.h>

struct tb {
	size_t n_insns;
};

static csh cap_handle;
static cs_insn *cap_insn;

static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
		     uint64_t vaddr, void *udata)
{ }

static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
{ }

static void vcpu_tb_trans(qemu_plugin_id_t id, unsigned int cpu_index, struct qemu_plugin_tb *tb)
{
	struct tb *desc;
	size_t n = qemu_plugin_tb_n_insns(tb);
	size_t i;

	for (i = 0; i < n; i++) {
		struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
		size_t size = qemu_plugin_insn_size(insn);
		const uint8_t *code = qemu_plugin_insn_data(insn);
		uint64_t offset = 0;
		bool success;

		success = cs_disasm_iter(cap_handle, &code, &size, &offset, cap_insn);
		assert(success);
		qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem,
						 QEMU_PLUGIN_CB_NO_REGS, NULL);
	}
	desc = malloc(sizeof(*desc));
	assert(desc);
	desc->n_insns = n;

	qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
					     QEMU_PLUGIN_CB_NO_REGS, desc);
}

QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, int argc, char **argv)
{
	if (cs_open(CS_ARCH_X86, CS_MODE_64, &cap_handle) != CS_ERR_OK) {
		return -1;
	}
	cap_insn = cs_malloc(cap_handle);
	if (cap_insn == NULL) {
		return -1;
	}
	qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
	return 0;
}

Emilio G. Cota <cota@braap.org> writes:

> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  plugin-examples/bbcount_avgsize_racy.c | 50 ++++++++++++++++++++++
>  plugin-examples/mem_count_racy_both.c  | 58 ++++++++++++++++++++++++++
>  plugin-examples/Makefile               | 31 ++++++++++++++

So I think we need to be putting these somewhere else and also building
the examples by default. As plugins only make sense with tcg guests
maybe a layout like:

  tcg/plugins/plugin.c
  tcg/plugins/examples/

>  3 files changed, 139 insertions(+)
>  create mode 100644 plugin-examples/bbcount_avgsize_racy.c
>  create mode 100644 plugin-examples/mem_count_racy_both.c
>  create mode 100644 plugin-examples/Makefile
>
> diff --git a/plugin-examples/bbcount_avgsize_racy.c b/plugin-examples/bbcount_avgsize_racy.c
> new file mode 100644
> index 0000000000..ccdf96c1fa
> --- /dev/null
> +++ b/plugin-examples/bbcount_avgsize_racy.c
> @@ -0,0 +1,50 @@
> +#include <inttypes.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +#include <qemu-plugin.h>

-#include <qemu-plugin.h>
+#include <plugin-api.h>

> +
> +static uint64_t bb_count;
> +static uint64_t insn_count;
> +const char *filename;
> +static int stdout_fd;
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +    dprintf(stdout_fd, "insns: %" PRIu64", bb: %" PRIu64 ", "
> +            "avg insns/bb: %.2f\n",
> +            insn_count, bb_count, (double)insn_count / bb_count);
> +}
> +
> +static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
> +{
> +    unsigned long n_insns = (unsigned long)udata;
> +
> +    insn_count += n_insns;
> +    bb_count++;
> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, unsigned int cpu_index,
> +                          struct qemu_plugin_tb *tb)
> +{
> +    unsigned long n_insns = qemu_plugin_tb_n_insns(tb);
> +
> +    qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
> +                                         QEMU_PLUGIN_CB_NO_REGS,
> +                                         (void *)n_insns);
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, int argc,
> +                                           char **argv)
> +{
> +    /* plugin_exit might write to stdout after stdout has been closed */
> +    stdout_fd = dup(STDOUT_FILENO);
> +    assert(stdout_fd);
> +
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +    return 0;
> +}
> diff --git a/plugin-examples/mem_count_racy_both.c b/plugin-examples/mem_count_racy_both.c
> new file mode 100644
> index 0000000000..a47f2025bf
> --- /dev/null
> +++ b/plugin-examples/mem_count_racy_both.c
> @@ -0,0 +1,58 @@
> +#include <inttypes.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +#include <qemu-plugin.h>

-#include <qemu-plugin.h>
+#include <plugin-api.h>

> +
> +static uint64_t mem_count;
> +static int stdout_fd;
> +static bool do_inline;
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +    dprintf(stdout_fd, "accesses: %" PRIu64 "\n", mem_count);
> +}
> +
> +static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
> +                     uint64_t vaddr, void *udata)
> +{
> +    mem_count++;
> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, unsigned int cpu_index,
> +                          struct qemu_plugin_tb *tb)
> +{
> +    size_t n = qemu_plugin_tb_n_insns(tb);
> +    size_t i;
> +
> +    for (i = 0; i < n; i++) {
> +        struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> +
> +        if (do_inline) {
> +            qemu_plugin_register_vcpu_mem_inline(insn,
> +                                                 QEMU_PLUGIN_INLINE_ADD_U64,
> +                                                 &mem_count, 1);
> +        } else {
> +            qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem,
> +                                             QEMU_PLUGIN_CB_NO_REGS, NULL);
> +        }
> +    }
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, int argc,
> +                                           char **argv)
> +{
> +    if (argc && strcmp(argv[0], "inline") == 0) {
> +        do_inline = true;
> +    }
> +    /* plugin_exit might write to stdout after stdout has been closed */
> +    stdout_fd = dup(STDOUT_FILENO);
> +    assert(stdout_fd);
> +
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +    return 0;
> +}
> diff --git a/plugin-examples/Makefile b/plugin-examples/Makefile
> new file mode 100644
> index 0000000000..71bbcda7a8
> --- /dev/null
> +++ b/plugin-examples/Makefile
> @@ -0,0 +1,31 @@
> +NAMES :=
> +NAMES += bbcount_avgsize_racy
> +NAMES += mem_count_racy_both
> +
> +SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
> +
> +# QEMU installed path, set by --prefix during configure
> +QEMU_PATH ?= /data/src/qemu-inst/plugin-test

I guess we should use SRC_PATH (or maybe QEMU_SRC_PATH) if we want the
Makefile to be both invoked as a sub-make from a normal QEMU build and
to be used as an example for out-of-tree builds.

> +
> +CC := gcc
> +CFLAGS :=
> +CFLAGS += -O2 -Werror -Wall
> +CFLAGS += -Wundef -Wwrite-strings -Wmissing-prototypes
> +CFLAGS += -Wstrict-prototypes -Wredundant-decls
> +CFLAGS += -fno-strict-aliasing -fno-common -fwrapv
> +CFLAGS += -I$(QEMU_PATH)/include

-CFLAGS += -I$(QEMU_PATH)/include
+CFLAGS += -I$(QEMU_PATH)/include/qemu

--
Alex Bennée

On Wed, Nov 28, 2018 at 11:28:11 +0000, Alex Bennée wrote:
> 
> Emilio G. Cota <cota@braap.org> writes:
> 
> > Signed-off-by: Emilio G. Cota <cota@braap.org>
> > ---
> >  plugin-examples/bbcount_avgsize_racy.c | 50 ++++++++++++++++++++++
> >  plugin-examples/mem_count_racy_both.c  | 58 ++++++++++++++++++++++++++
> >  plugin-examples/Makefile               | 31 ++++++++++++++
> 
> So I think we need to be putting these somewhere else and also building
> the examples by default. As plugins only make sense with tcg guests

Most of plugin functionality is related to TCG, but some of
it is not. For instance, one could use a plugin to just control
the guest clock, and nothing else. This would work with KVM.

This is why I think $SRC_PATH/plugin.c makes sense.

Wrt the examples, I just included them here for reviewing purposes.
If we end up adding them, I think tests/plugin[s] would be an appropriate
place for them.

> > +# QEMU installed path, set by --prefix during configure
> > +QEMU_PATH ?= /data/src/qemu-inst/plugin-test
> 
> I guess we should use SRC_PATH (or maybe QEMU_SRC_PATH) if we want the
> Makefile to be both invoked as a sub-make from a normal QEMU build and
> to be used as an example for out-of-tree builds.

(As I said, this was a quick hack just to get the RFC out :P)

Thanks,

		Emilio

On Thu, Oct 25, 2018 at 01:20:57PM -0400, Emilio G. Cota wrote:
> +
> +lib%.so: %.o
> +	$(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)

The rule should be a bit different for macOS:
%.bundle: %.o
       $(CC) -bundle -Wl,-bundle_loader,PATH_TO_QEMU_EXE -o $@ $^ $(LDLIBS)

"-bundle" flag is needed because macOS has two kinds of Mach-O
dynamically loaded executables:
 - dylib (MH_DYLIB) - it's like an ELF shared object used for dynamic
   linking. Can be loaded, preloaded for sake of function interposing
   but cannot be explicitly unloaded.
 - bundle (MH_BUNDLE) - similar to an ELF shared object used in plugin
   dlopen/dlclose scenario.

We can pick any (enabled in configure) qemu-system executable as
bundle_loader. We specify it to check if all symbols in a bundle,
including the ones coming from the executable are defined. FWIW, here's
the reference for bundle_loader flag:
  -bundle_loader executable
      This specifies the executable that will be loading the bundle
      output file being linked. Undefined symbols from the bundle are
      checked against the specified executable like it was one of the
      dynamic libraries the bundle was linked with


The ".bundle" extension is not required but IMO it feels more native to
the system than ".so".

I've checked both plugins can be loaded and print a line when QEMU exits.

Best regards,
Roman

On Thu, Nov 29, 2018 at 23:45:18 +0300, Roman Bolshakov wrote:
> On Thu, Oct 25, 2018 at 01:20:57PM -0400, Emilio G. Cota wrote:
> > +
> > +lib%.so: %.o
> > +	$(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)
> 
> The rule should be a bit different for macOS:
> %.bundle: %.o
>        $(CC) -bundle -Wl,-bundle_loader,PATH_TO_QEMU_EXE -o $@ $^ $(LDLIBS)
> 
> "-bundle" flag is needed because macOS has two kinds of Mach-O
> dynamically loaded executables:
>  - dylib (MH_DYLIB) - it's like an ELF shared object used for dynamic
>    linking. Can be loaded, preloaded for sake of function interposing
>    but cannot be explicitly unloaded.
>  - bundle (MH_BUNDLE) - similar to an ELF shared object used in plugin
>    dlopen/dlclose scenario.
> 
> We can pick any (enabled in configure) qemu-system executable as
> bundle_loader. We specify it to check if all symbols in a bundle,
> including the ones coming from the executable are defined. FWIW, here's
> the reference for bundle_loader flag:
>   -bundle_loader executable
>       This specifies the executable that will be loading the bundle
>       output file being linked. Undefined symbols from the bundle are
>       checked against the specified executable like it was one of the
>       dynamic libraries the bundle was linked with
> 
> 
> The ".bundle" extension is not required but IMO it feels more native to
> the system than ".so".

The goal of the examples is to be target-independent, so I'm not
convinced that we want to bury $PATH_TO_QEMU_EXE in the build recipe
(or get configure involved in this).

Since you say the "bundle" business isn't a requirement, I'll leave just
the .so rule in v2.

Thanks,

		Emilio