[Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Provides the ability for the guest to communicate with user-provided code inside
QEMU itself, using a lightweight mechanism.

See first commit for a full description.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---

Changes in v2:

* Rebase on 1c8a881daaca6fe0646a425b0970fb3ad25f6732 from master.
* Multiple documentation fixes and clarifications.
* Fixed a couple of bugs when freeing memory.
* Actually call 'backdoor_fini' in user mode when exiting.
* Try to auto-detect the backdoor device in the example guest library when
  running a full-system linux.
* Rewrite code to read and write into the control channel in softmmu mode,
  making it much clearer and leaving endianness handling up to the user-provided
  backdoor callbacks.


Lluís Vilanova (5):
      backdoor: Add documentation
      backdoor: Add build infrastructure
      backdoor: [*-user] Add QEMU-side proxy to "libbackdoor.a"
      backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"
      backdoor: Add guest-side library


 Makefile                       |    4 +
 Makefile.objs                  |   21 ++++
 Makefile.target                |    4 +
 backdoor/guest/Makefile        |   18 ++++
 backdoor/guest/common.c        |  205 ++++++++++++++++++++++++++++++++++++++++
 backdoor/guest/qemu-backdoor.h |   59 ++++++++++++
 backdoor/qemu/qemu-backdoor.h  |   31 ++++++
 backdoor/qemu/softmmu.c        |  186 ++++++++++++++++++++++++++++++++++++
 backdoor/qemu/user.c           |  202 +++++++++++++++++++++++++++++++++++++++
 backdoor/qemu/user.h           |   30 ++++++
 bsd-user/main.c                |   26 +++++
 bsd-user/mmap.c                |    7 +
 bsd-user/syscall.c             |   13 +++
 configure                      |   35 +++++++
 darwin-user/main.c             |   26 +++++
 darwin-user/mmap.c             |    7 +
 docs/backdoor.txt              |  167 +++++++++++++++++++++++++++++++++
 hw/pci.h                       |    1 
 linux-user/main.c              |   32 ++++++
 linux-user/mmap.c              |    7 +
 linux-user/syscall.c           |   10 ++
 21 files changed, 1089 insertions(+), 2 deletions(-)
 create mode 100644 backdoor/guest/Makefile
 create mode 100644 backdoor/guest/common.c
 create mode 100644 backdoor/guest/qemu-backdoor.h
 create mode 100644 backdoor/qemu/qemu-backdoor.h
 create mode 100644 backdoor/qemu/softmmu.c
 create mode 100644 backdoor/qemu/user.c
 create mode 100644 backdoor/qemu/user.h
 create mode 100644 docs/backdoor.txt

[Qemu-devel] [PATCH v2 1/5] backdoor: Add documentation

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 docs/backdoor.txt |  167 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 167 insertions(+), 0 deletions(-)
 create mode 100644 docs/backdoor.txt

diff --git a/docs/backdoor.txt b/docs/backdoor.txt
new file mode 100644
index 0000000..1c6502a
--- /dev/null
+++ b/docs/backdoor.txt
@@ -0,0 +1,167 @@
+= Backdoor communication channel =
+
+== Introduction ==
+
+This document describes how the guest can use the backdoor communication channel
+to interact with user-provided code inside QEMU.
+
+The backdoor provides a lightweight and guest-initiated communication channel
+between code running inside the guest system and code in QEMU, including both
+QEMU in 'softmmu' and 'user' modes.
+
+The semantics of the backdoor channel are up to the user, who must provide the
+implementation of the QEMU-side callbacks used when the backdoor channel is
+invoked.
+
+On the guest side, code can be linked against a simple library provided in QEMU
+to interface with the backdoor channel.
+
+The features of this mechanism are:
+
+* Minimal setup for the guest.
+* Independent of guest architecture.
+* Works with 'softmmu' and 'user' mode.
+* Negligible guest overhead; guest invocations of the backdoor channel does not
+  go through any OS abstraction, except during the setup of the communication
+  channel.
+* Negligible host overhead; invocations of the backdoor channel are interpreted
+  by QEMU, while a side-channel can be used as regular memory to communicate
+  bulk data without any extra overhead.
+* The user-provided backdoor callbacks can perform arbitrary actions on the
+  guest system (e.g., read or write memory, change register values, etc.).
+
+
+== QEMU-side code ==
+
+1. Create the "Makefile" to build the user-provided backdoor channel library:
+
+    mkdir /tmp/my-backdoor-qemu
+    cat > /tmp/my-backdoor-qemu/Makefile <<EOF
+    include $(BUILD_DIR)/config-host.mak
+    include $(BUILD_DIR)/$(TARGET_DIR)../config-target.mak
+    include $(SRC_PATH)/rules.mak
+    
+    vpath %.c /tmp/my-backdoor-qemu
+    
+    
+    libbackdoor.a: backdoor.o
+    
+    
+    # Include automatically generated dependency files
+    -include $(wildcard *.d)
+    EOF
+
+2. Implement the callbacks declared in "backdoor/qemu/qemu-backdoor.h":
+
+    cat > /tmp/my-backdoor-qemu/backdoor.c <<EOF
+    #include "backdoor/qemu/qemu-backdoor.h"
+    
+    #include "cpu.h"
+    
+    #include <stdio.h>
+    
+    
+    void qemu_backdoor_init(uint64_t data_size)
+    {
+        printf("+ %"PRId64"\n", data_size);
+    }
+    
+    void qemu_backdoor(uint64_t cmd, void *data)
+    {
+        /* Perform any endianess-wise loads to interpret the data */
+        uint64_t c = tswap64(cmd);
+        uint64_t d = tswap64(*(uint64_t*)data);
+        printf("-> %"PRIx64" :: %"PRIx64"\n", c, d);
+    }
+    EOF
+
+3. Build QEMU with the backdoor feature:
+
+    /path/to/qemu/configure --with-backdoor=/tmp/my-backdoor-qemu
+
+
+== Guest-side code ==
+
+1. Compile the corresponding guest-side interface library:
+
+    make -C /path/to/qemu-build/x86_64-linux-user/backdoor/guest
+
+2. Create a guest application to interact with the backdoor channel through the
+   interface declared in "backdoor/guest/qemu-backdoor.h":
+
+    cat > /tmp/my-backdoor-guest.c <<EOF
+    #include <stdio.h>
+    #include <errno.h>
+    #include <stdlib.h>
+    #include <string.h>
+    #include <qemu-backdoor.h>
+    
+    
+    int main()
+    {
+        /* This base path is only applicable to 'user' mode */
+        if (qemu_backdoor_init("/tmp/backdoor") != 0) {
+            fprintf(stderr, "error: qemu_backdoor_init: %s\n", strerror(errno));
+            abort();
+        }
+    
+        uint64_t vcmd  = 0xcafe;
+        uint64_t vdata = 0xbabe;
+    
+        printf("size: %"PRId64"\n", qemu_backdoor_data_size());
+        printf("sending cmd: 0x%"PRIx64" data: 0x%"PRIx64"\n", vcmd, vdata);
+    
+        /* Get a pointer to the beginning of the data channel */
+        uint64_t * data = qemu_backdoor_data();
+     
+        /* Write anything into the channel */
+        *data = vdata;
+        /* Invoke the channel */
+        qemu_backdoor(vcmd);
+    }
+    EOF
+
+3. Link the guest application against "libqemu-backdoor-guest.a":
+
+    gcc -o /tmp/my-backdoor-guest /tmp/my-backdoor-guest.c /path/to/qemu-build/x86_64-linux-user/backdoor/guest/libqemu-backdoor-guest.a -I/path/to/qemu/backdoor/guest
+
+
+== Running QEMU ==
+
+If you want to use QEMU's 'softmmu' mode:
+
+    /path/to/qemu-build/x86_64-softmmu/qemu-system-x86_64 -device backdoor
+    sudo /tmp/my-backdoor-guest # inside the VM
+
+If you want to use QEMU's 'user' mode:
+
+    /path/to/qemu-build/x86_64-linux-user/qemu-x86_64 -backdoor /tmp/backdoor /tmp/my-backdoor-guest
+
+
+== Implementation details ==
+
+The backdoor channel is composed of two channels that are handled as
+memory-mapped files. The data channel is used to contain arbitrary data to
+communicate back and forth between the guest and QEMU. The control channel is
+used by the guest to read the size of the data channel and to write into it to
+signal that the data channel is ready to be used.
+
+When using the 'softmmu' mode, the backdoor communication channels are provided
+as a virtual device used through MMIO. The data channel acts as regular memory
+and the control channel intercepts all accesses to it to proxy them to the
+user-provided backdoor library.
+
+When using the 'user' mode, the backdoor communication channels are provided as
+regular files in the host system that the guest must 'mmap' into its address
+space. The data channel acts as regular memory and the 'mmap' of the control
+channel is intercepted in QEMU to establish if it's an 'mmap' for the control
+channel file. If that's the case, the memory that QEMU allocates for the guest
+is 'mprotect'ed to intercept all accesses to it performed by the guest and proxy
+them to the user-provided backdoor library.
+
+Note that guest accesses to the device are automatically serialized by QEMU into
+a single thread that handles the host-side backdoor callbacks. This means that
+its operation is thread-safe (as long as the command is written with a single
+guest instruction), and the 64 bits used to signal a backdoor command can be used
+to index into different portions of the data channel so that multiple guest
+processes/threads can use the backdoor channel concurrently.

[Qemu-devel] [PATCH v2 2/5] backdoor: Add build infrastructure

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile                      |    4 +++-
 Makefile.objs                 |   18 ++++++++++++++++++
 Makefile.target               |    4 +++-
 backdoor/qemu/qemu-backdoor.h |   31 +++++++++++++++++++++++++++++++
 configure                     |   32 ++++++++++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 2 deletions(-)
 create mode 100644 backdoor/qemu/qemu-backdoor.h

diff --git a/Makefile b/Makefile
index 301c75e..5f89058 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,9 @@ else
 DOCS=
 endif
 
-SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR)
+SUBDIR_MAKEFLAGS_=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR)
+# make sure the variable is available to makes in sub-directories
+SUBDIR_MAKEFLAGS=$(SUBDIR_MAKEFLAGS_) SUBDIR_MAKEFLAGS="$(SUBDIR_MAKEFLAGS_)"
 SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS))
 SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %/config-devices.mak.d, $(TARGET_DIRS))
 
diff --git a/Makefile.objs b/Makefile.objs
index d7a6539..df943e9 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -398,6 +398,24 @@ trace-obj-y += $(addprefix trace/, $(trace-nested-y))
 $(trace-obj-y): $(GENERATED_HEADERS)
 
 ######################################################################
+# backdoor
+
+backdoor-obj-y += $(addprefix backdoor/qemu/, $(backdoor-nested-y))
+
+ifdef CONFIG_BACKDOOR
+LIBBACKDOOR = libbackdoor/libbackdoor.a
+
+.PHONY: force
+force:
+
+$(LIBBACKDOOR): $(dir $(LIBBACKDOOR))/Makefile force
+	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@)	\
+		QEMU_CFLAGS="$(QEMU_CFLAGS) -I$(BUILD_DIR)"		\
+		TARGET_DIR="$(TARGET_DIR)$(dir $@)/" VPATH="$(VPATH)"	\
+		SRC_PATH="$(SRC_PATH)" V="$(V)" "$(notdir $@)")
+endif
+
+######################################################################
 # smartcard
 
 libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o card_7816.o
diff --git a/Makefile.target b/Makefile.target
index e244040..9533469 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -90,6 +90,8 @@ libobj-$(CONFIG_TCI_DIS) += tci-dis.o
 
 tci-dis.o: QEMU_CFLAGS += -I$(SRC_PATH)/tcg -I$(SRC_PATH)/tcg/tci
 
+libobj-$(CONFIG_BACKDOOR) += $(backdoor-obj-y)
+
 $(libobj-y): $(GENERATED_HEADERS)
 
 # libqemu
@@ -413,7 +415,7 @@ endif # CONFIG_LINUX_USER
 
 obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o
 
-$(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y)
+$(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) $(LIBBACKDOOR)
 	$(call LINK,$^)
 
 
diff --git a/backdoor/qemu/qemu-backdoor.h b/backdoor/qemu/qemu-backdoor.h
new file mode 100644
index 0000000..05e960d
--- /dev/null
+++ b/backdoor/qemu/qemu-backdoor.h
@@ -0,0 +1,31 @@
+/*
+ * QEMU-side user-provided callbacks for the backdoor communication channel.
+ *
+ * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <stdint.h>
+#include <sys/types.h>
+
+
+/**
+ * Initialize the backdoor channel.
+ *
+ * @param data_size Size of the data channel.
+ */
+void qemu_backdoor_init(uint64_t data_size);
+
+/**
+ * Handle an invocation of the backdoor channel.
+ *
+ * The command argument can be used to achieve concurrency among different
+ * clients (e.g., threads in the guest system) of the backdoor channel (e.g.,
+ * different clients write to different parts of the data channel).
+ *
+ * @param cmd  Command to invoke.
+ * @param data Pointer to the beginning of the data channel.
+ */
+void qemu_backdoor(uint64_t cmd, void *data);
diff --git a/configure b/configure
index ac4840d..f362ef6 100755
--- a/configure
+++ b/configure
@@ -176,6 +176,7 @@ pie=""
 zero_malloc=""
 trace_backend="nop"
 trace_file="trace"
+backdoor=""
 spice=""
 rbd=""
 smartcard=""
@@ -549,6 +550,8 @@ for opt do
   ;;
   --with-trace-file=*) trace_file="$optarg"
   ;;
+  --with-backdoor=*) backdoor="$optarg"
+  ;;
   --enable-gprof) gprof="yes"
   ;;
   --static)
@@ -1062,6 +1065,7 @@ echo "  --enable-trace-backend=B Set trace backend"
 echo "                           Available backends:" $("$source_path"/scripts/tracetool --list-backends)
 echo "  --with-trace-file=NAME   Full PATH,NAME of file to store traces"
 echo "                           Default:trace-<pid>"
+echo "  --with-backdoor=PATH     Directory to build user-provided backdoor library"
 echo "  --disable-spice          disable spice"
 echo "  --enable-spice           enable spice"
 echo "  --enable-rbd             enable building the rados block device (rbd)"
@@ -2619,6 +2623,20 @@ if test "$trace_backend" = "dtrace"; then
 fi
 
 ##########################################
+# check for a valid directory for backdoor
+if test -n "$backdoor"; then
+    if test ! -f "$backdoor/Makefile"; then
+        echo
+        echo "Error: cannot make into '$backdoor'"
+        echo "Please choose a directory where I can run 'make'"
+        echo
+        exit 1
+    fi
+    backdoor=`readlink -f "$backdoor"`
+fi
+
+
+##########################################
 # __sync_fetch_and_and requires at least -march=i486. Many toolchains
 # use i686 as default anyway, but for those that don't, an explicit
 # specification is necessary
@@ -2838,6 +2856,9 @@ echo "uuid support      $uuid"
 echo "vhost-net support $vhost_net"
 echo "Trace backend     $trace_backend"
 echo "Trace output file $trace_file-<pid>"
+if test -n "$backdoor"; then
+    echo "Backdoor lib dir  $backdoor"
+fi
 echo "spice support     $spice"
 echo "rbd support       $rbd"
 echo "xfsctl support    $xfs"
@@ -3231,6 +3252,13 @@ if test "$trace_default" = "yes"; then
   echo "CONFIG_TRACE_DEFAULT=y" >> $config_host_mak
 fi
 
+# backdoor channel
+if test -n "$backdoor"; then
+  echo "CONFIG_BACKDOOR=y" >> $config_host_mak
+  echo "BACKDOOR_PATH=\"$backdoor\"" >> $config_host_mak
+  QEMU_CFLAGS="-I\"$backdoor\" $QEMU_CFLAGS"
+fi
+
 echo "TOOLS=$tools" >> $config_host_mak
 echo "CHECKS=$checks" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
@@ -3342,6 +3370,10 @@ if test "$target" = "arm-linux-user" -o "$target" = "armeb-linux-user" -o "$targ
   mkdir -p $target_dir/nwfpe
 fi
 symlink $source_path/Makefile.target $target_dir/Makefile
+if test -n "$backdoor"; then
+    mkdir -p $target_dir/libbackdoor
+    symlink $backdoor/Makefile $target_dir/libbackdoor/Makefile
+fi
 
 
 echo "# Automatically generated by configure - do not modify" > $config_target_mak

[Qemu-devel] [PATCH v2 3/5] backdoor: [*-user] Add QEMU-side proxy to "libbackdoor.a"

[Lluís Vilanova]

QEMU detects when the guest uses 'mmap' on the control channel file, and then
uses 'mprotect' to detect accesses to it, which are redirected to the
user-provided "libbackdoor.a".

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile.objs        |    2 
 backdoor/qemu/user.c |  202 ++++++++++++++++++++++++++++++++++++++++++++++++++
 backdoor/qemu/user.h |   30 +++++++
 bsd-user/main.c      |   26 ++++++
 bsd-user/mmap.c      |    7 ++
 bsd-user/syscall.c   |   13 +++
 configure            |    1 
 darwin-user/main.c   |   26 ++++++
 darwin-user/mmap.c   |    7 ++
 linux-user/main.c    |   32 ++++++++
 linux-user/mmap.c    |    7 ++
 linux-user/syscall.c |   10 ++
 12 files changed, 363 insertions(+), 0 deletions(-)
 create mode 100644 backdoor/qemu/user.c
 create mode 100644 backdoor/qemu/user.h

diff --git a/Makefile.objs b/Makefile.objs
index df943e9..9784441 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -400,6 +400,8 @@ $(trace-obj-y): $(GENERATED_HEADERS)
 ######################################################################
 # backdoor
 
+backdoor-nested-$(CONFIG_USER_ONLY) += user.o
+
 backdoor-obj-y += $(addprefix backdoor/qemu/, $(backdoor-nested-y))
 
 ifdef CONFIG_BACKDOOR
diff --git a/backdoor/qemu/user.c b/backdoor/qemu/user.c
new file mode 100644
index 0000000..6e178cc
--- /dev/null
+++ b/backdoor/qemu/user.c
@@ -0,0 +1,202 @@
+/*
+ * QEMU-side management of backdoor channels in user-level emulation.
+ *
+ * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "backdoor/qemu/user.h"
+
+#include <string.h>
+#include <sys/mman.h>
+
+#include "qemu-common.h"
+#include "backdoor/qemu/qemu-backdoor.h"
+
+
+static char *data_path = NULL;
+static char *control_path = NULL;
+static int data_fd = -1;
+static int control_fd = -1;
+
+static void *data = NULL;
+static void *qemu_control_0 = NULL;
+static void *qemu_control_1 = NULL;
+
+static struct stat control_fd_stat;
+
+struct sigaction segv_next;
+static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt);
+
+
+static void init_channel(const char *base, const char *suffix, size_t size,
+                         char ** path, int *fd, void **addr)
+{
+    *path = g_malloc(strlen(base) + strlen(suffix) + 1);
+    sprintf(*path, "%s%s", base, suffix);
+
+    *fd = open(*path, O_CREAT | O_EXCL | O_RDWR, S_IRUSR | S_IWUSR);
+    if (*fd == -1) {
+        fprintf(stderr, "error: open(%s): %s\n", *path, strerror(errno));
+        abort();
+    }
+
+    off_t lres = lseek(*fd, size - 1, SEEK_SET);
+    if (lres == (off_t)-1) {
+        fprintf(stderr, "error: lseek(%s): %s\n", *path, strerror(errno));
+        abort();
+    }
+
+    char tmp;
+    ssize_t wres = write(*fd, &tmp, 1);
+    if (wres == -1) {
+        fprintf(stderr, "error: write(%s): %s\n", *path, strerror(errno));
+        abort();
+    }
+
+    if (addr) {
+        *addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, *fd, 0);
+        if (*addr == MAP_FAILED) {
+            fprintf(stderr, "error: mmap(%s): %s\n", *path, strerror(errno));
+            abort();
+        }
+    }
+}
+
+void backdoor_init(const char *base, uint64_t data_size)
+{
+    if (base == NULL) {
+        return;
+    }
+
+    init_channel(base, "-data", data_size, &data_path, &data_fd, &data);
+    void *control;
+    init_channel(base, "-control", getpagesize() * 2, &control_path, &control_fd, &control);
+
+    /* store channel size in first 64bits (target endianess); command goes into
+     * 2nd slot
+     */
+    *(uint64_t*)control = tswap64(data_size);
+
+    if (fstat(control_fd, &control_fd_stat) == -1) {
+        fprintf(stderr, "error: fstat(backdoor_control): %s\n", strerror(errno));
+        abort();
+    }
+
+    struct sigaction segv;
+    memset(&segv, 0, sizeof(segv));
+    segv.sa_sigaction = segv_handler;
+    segv.sa_flags = SA_SIGINFO | SA_RESTART;
+    sigemptyset(&segv.sa_mask);
+
+    if (sigaction(SIGSEGV, &segv, &segv_next) != 0) {
+        fprintf(stderr, "error: sigaction(SIGSEGV): %s\n", strerror(errno));
+        abort();
+    }
+
+    qemu_backdoor_init(data_size);
+}
+
+
+static void fini_channel(int *fd, char **path)
+{
+    if (*fd != -1) {
+        if (close(*fd) == -1) {
+            fprintf(stderr, "error: close: %s\n", strerror(errno));
+            abort();
+        }
+        if (unlink(*path) == -1) {
+            fprintf(stderr, "error: unlink(%s): %s\n", *path, strerror(errno));
+            abort();
+        }
+        *fd = -1;
+    }
+    if (*path != NULL) {
+        g_free(path);
+        *path =  NULL;
+    }
+}
+
+void backdoor_fini(void)
+{
+    static bool atexit_in = false;
+    if (atexit_in) {
+        return;
+    }
+    atexit_in = true;
+
+    if (sigaction(SIGSEGV, &segv_next, NULL) != 0) {
+        fprintf(stderr, "error: sigaction(SIGSEGV): %s\n", strerror(errno));
+        abort();
+    }
+    fini_channel(&data_fd, &data_path);
+    fini_channel(&control_fd, &control_path);
+}
+
+
+void backdoor_guest_mmap(int fd, void *qemu_addr)
+{
+    struct stat s;
+    if (fstat(fd, &s) != 0) {
+        return;
+    }
+
+    if (s.st_dev != control_fd_stat.st_dev ||
+        s.st_ino != control_fd_stat.st_ino) {
+        return;
+    }
+
+    /* it's an mmap of the control channel; split it in two and mprotect it to
+     * detect writes (cmd is written once on each part)
+     */
+    qemu_control_0 = qemu_addr;
+    qemu_control_1 = qemu_control_0 + getpagesize();
+    if (mprotect(qemu_control_0, getpagesize(), PROT_READ) == -1) {
+        fprintf(stderr, "error: mprotect(backdoor_control): %s\n",
+                strerror(errno));
+        abort();
+    }
+}
+
+static void swap_control(void *from, void *to)
+{
+    if (mprotect(from, getpagesize(), PROT_READ | PROT_WRITE) == -1) {
+        fprintf(stderr, "error: mprotect(from): %s\n",
+                strerror(errno));
+        abort();
+    }
+    if (mprotect(to, getpagesize(), PROT_READ) == -1) {
+        fprintf(stderr, "error: mprotect(to): %s\n",
+                strerror(errno));
+        abort();
+    }
+}
+
+static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
+{
+    if (qemu_control_0 <= siginfo->si_addr &&
+        siginfo->si_addr < qemu_control_1) {
+
+        /* 1st fault (guest will write cmd) */
+        assert(((target_ulong)siginfo->si_addr % getpagesize()) == sizeof(uint64_t));
+        swap_control(qemu_control_0, qemu_control_1);
+
+    } else if (qemu_control_1 <= siginfo->si_addr &&
+               siginfo->si_addr < qemu_control_1 + getpagesize()) {
+
+        /* 2nd fault (invoke) */
+        assert(((target_ulong)siginfo->si_addr % getpagesize()) == sizeof(uint64_t));
+        qemu_backdoor(((uint64_t*)qemu_control_0)[1], data);
+        swap_control(qemu_control_1, qemu_control_0);
+
+    } else {
+        /* proxy to next handler */
+        if (segv_next.sa_sigaction != NULL) {
+            segv_next.sa_sigaction(signum, siginfo, sigctxt);
+        } else if (segv_next.sa_handler != NULL) {
+            segv_next.sa_handler(signum);
+        }
+    }
+}
diff --git a/backdoor/qemu/user.h b/backdoor/qemu/user.h
new file mode 100644
index 0000000..04f9aea
--- /dev/null
+++ b/backdoor/qemu/user.h
@@ -0,0 +1,30 @@
+/*
+ * QEMU-side management of backdoor channels in user-level emulation.
+ *
+ * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <stdint.h>
+#include <sys/types.h>
+
+
+/**
+ * Initialize the backing files for the backdoor channel.
+ *
+ * @param base Base path for the backdoor channel files.
+ * @param data_size Length in bytes for the data channel.
+ */
+void backdoor_init(const char *base, uint64_t data_size);
+
+/**
+ * Check if this mmap is for the control channel and act accordingly.
+ */
+void backdoor_guest_mmap(int fd, void *qemu_addr);
+
+/**
+ * Remove the backing files for the backdoor channel.
+ */
+void backdoor_fini(void);
diff --git a/bsd-user/main.c b/bsd-user/main.c
index cc7d4a3..0c6c42c 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -34,6 +34,10 @@
 #include "qemu-timer.h"
 #include "envlist.h"
 
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
+
 #define DEBUG_LOGFILE "/tmp/qemu.log"
 
 int singlestep;
@@ -688,6 +692,11 @@ static void usage(void)
            "-B address        set guest_base address to address\n"
 #endif
            "-bsd type         select emulated BSD type FreeBSD/NetBSD/OpenBSD (default)\n"
+#if defined(CONFIG_BACKDOOR)
+           "-backdoor path    base path to backdoor channel\n"
+           "-backdoor-pages value\n"
+           "                  number of pages for the backdoor data channel (default: 1)\n"
+#endif
            "\n"
            "Debug options:\n"
            "-d options   activate log (default logfile=%s)\n"
@@ -744,6 +753,10 @@ int main(int argc, char **argv)
     char **target_environ, **wrk;
     envlist_t *envlist = NULL;
     bsd_type = target_openbsd;
+#if defined(CONFIG_BACKDOOR)
+    char *backdoor_base = NULL;
+    uint64_t backdoor_pages = 1;
+#endif
 
     if (argc <= 1)
         usage();
@@ -851,6 +864,12 @@ int main(int argc, char **argv)
             singlestep = 1;
         } else if (!strcmp(r, "strace")) {
             do_strace = 1;
+#if defined(CONFIG_BACKDOOR)
+        } else if (!strcmp(r, "backdoor")) {
+            backdoor_base = argv[optind++];
+        } else if (!strcmp(r, "backdoor-pages")) {
+            backdoor_pages = atoi(argv[optind++]);
+#endif
         } else
         {
             usage();
@@ -988,6 +1007,13 @@ int main(int argc, char **argv)
     target_set_brk(info->brk);
     syscall_init();
     signal_init();
+#if defined(CONFIG_BACKDOOR)
+    if (atexit(backdoor_fini) != 0) {
+        fprintf(stderr, "error: atexit: %s\n", strerror(errno));
+        abort();
+    }
+    backdoor_init(backdoor_base, backdoor_pages * getpagesize());
+#endif
 
 #if defined(CONFIG_USE_GUEST_BASE)
     /* Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 5d6cffc..afa57df 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -28,6 +28,10 @@
 #include "qemu-common.h"
 #include "bsd-mman.h"
 
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
+
 //#define DEBUG_MMAP
 
 #if defined(CONFIG_USE_NPTL)
@@ -473,6 +477,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         }
     }
  the_end1:
+#if defined(CONFIG_BACKDOOR)
+    backdoor_guest_mmap(fd, (void *)g2h(start));
+#endif
     page_set_flags(start, start + len, prot | PAGE_VALID);
  the_end:
 #ifdef DEBUG_MMAP
diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
index 18b43f1..fa621ea 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -36,6 +36,10 @@
 #include "qemu.h"
 #include "qemu-common.h"
 
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
+
 //#define DEBUG
 
 static abi_ulong target_brk;
@@ -334,6 +338,9 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+#if defined(CONFIG_BACKDOOR)
+        backdoor_fini();
+#endif
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
         ret = 0; /* avoid warning */
@@ -429,6 +436,9 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+#if defined(CONFIG_BACKDOOR)
+        backdoor_fini();
+#endif
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
         ret = 0; /* avoid warning */
@@ -501,6 +511,9 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+#if defined(CONFIG_BACKDOOR)
+        backdoor_fini();
+#endif
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
         ret = 0; /* avoid warning */
diff --git a/configure b/configure
index f362ef6..8af8313 100755
--- a/configure
+++ b/configure
@@ -3373,6 +3373,7 @@ symlink $source_path/Makefile.target $target_dir/Makefile
 if test -n "$backdoor"; then
     mkdir -p $target_dir/libbackdoor
     symlink $backdoor/Makefile $target_dir/libbackdoor/Makefile
+    mkdir -p $target_dir/backdoor/qemu
 fi
 
 
diff --git a/darwin-user/main.c b/darwin-user/main.c
index c0f14f8..aa57140 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -28,6 +28,9 @@
 #include <sys/mman.h>
 
 #include "qemu.h"
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
 
 #define DEBUG_LOGFILE "/tmp/qemu.log"
 
@@ -713,6 +716,12 @@ static void usage(void)
            "-h           print this help\n"
            "-L path      set the %s library path (default='%s')\n"
            "-s size      set the stack size in bytes (default=%ld)\n"
+#if defined(CONFIG_BACKDOOR)
+           "-backdoor path\n"
+           "             base path to backdoor channel\n"
+           "-backdoor-pages value\n"
+           "             number of pages for the backdoor data channel (default: 1)\n"
+#endif
            "\n"
            "debug options:\n"
            "-d options   activate log (logfile='%s')\n"
@@ -745,6 +754,10 @@ int main(int argc, char **argv)
     short use_gdbstub = 0;
     const char *r;
     const char *cpu_model;
+#if defined(CONFIG_BACKDOOR)
+    char *backdoor_base = NULL;
+    uint64_t backdoor_pages = 1;
+#endif
 
     if (argc <= 1)
         usage();
@@ -802,6 +815,12 @@ int main(int argc, char **argv)
             }
         } else if (!strcmp(r, "singlestep")) {
             singlestep = 1;
+#if defined(CONFIG_BACKDOOR)
+        } else if (!strcmp(r, "backdoor")) {
+            backdoor_base = argv[optind++];
+        } else if (!strcmp(r, "backdoor-pages")) {
+            backdoor_pages = atoi(argv[optind++]);
+#endif
         } else
         {
             usage();
@@ -868,6 +887,13 @@ int main(int argc, char **argv)
 
     syscall_init();
     signal_init();
+#if defined(CONFIG_BACKDOOR)
+    if (atexit(backdoor_fini) != 0) {
+        fprintf(stderr, "error: atexit: %s\n", strerror(errno));
+        abort();
+    }
+    backdoor_init(backdoor_base, backdoor_pages * getpagesize());
+#endif
     global_env = env;
 
     /* build Task State */
diff --git a/darwin-user/mmap.c b/darwin-user/mmap.c
index d840b28..047c5bf 100644
--- a/darwin-user/mmap.c
+++ b/darwin-user/mmap.c
@@ -26,6 +26,10 @@
 
 #include "qemu.h"
 
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
+
 //#define DEBUG_MMAP
 
 /* NOTE: all the constants are the HOST ones */
@@ -308,6 +312,9 @@ long target_mmap(unsigned long start, unsigned long len, int prot,
             return ret;
     }
  the_end1:
+#if defined(CONFIG_BACKDOOR)
+    backdoor_guest_mmap(fd, (void *)g2h(start));
+#endif
     page_set_flags(start, start + len, prot | PAGE_VALID);
  the_end:
 #ifdef DEBUG_MMAP
diff --git a/linux-user/main.c b/linux-user/main.c
index d1bbc57..0c1db30 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -33,6 +33,9 @@
 #include "tcg.h"
 #include "qemu-timer.h"
 #include "envlist.h"
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
 
 #define DEBUG_LOGFILE "/tmp/qemu.log"
 
@@ -50,6 +53,10 @@ unsigned long guest_base;
 int have_guest_base;
 unsigned long reserved_va;
 #endif
+#if defined(CONFIG_BACKDOOR)
+const char *backdoor_base = NULL;
+uint64_t backdoor_pages = 1;
+#endif
 
 static void usage(void);
 
@@ -3082,6 +3089,18 @@ static void handle_arg_strace(const char *arg)
     do_strace = 1;
 }
 
+#if defined(CONFIG_BACKDOOR)
+static void handle_arg_backdoor(const char *arg)
+{
+    backdoor_base = arg;
+}
+
+static void handle_arg_backdoor_pages(const char *arg)
+{
+    backdoor_pages = atoi(arg);
+}
+#endif
+
 static void handle_arg_version(const char *arg)
 {
     printf("qemu-" TARGET_ARCH " version " QEMU_VERSION QEMU_PKGVERSION
@@ -3123,6 +3142,12 @@ struct qemu_argument arg_table[] = {
     {"R",          "QEMU_RESERVED_VA", true,  handle_arg_reserved_va,
      "size",       "reserve 'size' bytes for guest virtual address space"},
 #endif
+#if defined(CONFIG_BACKDOOR)
+    {"backdoor",   "QEMU_BACKDOOR",    true,  handle_arg_backdoor,
+     "path",       "base path to backdoor channel"},
+    {"backdoor-pages", "QEMU_BACKDOOR_PAGES", true,  handle_arg_backdoor_pages,
+     "num",        "number of pages for the backdoor data channel (default: 1)"},
+#endif
     {"d",          "QEMU_LOG",         true,  handle_arg_log,
      "options",    "activate log"},
     {"p",          "QEMU_PAGESIZE",    true,  handle_arg_pagesize,
@@ -3510,6 +3535,13 @@ int main(int argc, char **argv, char **envp)
     target_set_brk(info->brk);
     syscall_init();
     signal_init();
+#if defined(CONFIG_BACKDOOR)
+    if (atexit(backdoor_fini)) {
+        fprintf(stderr, "error: atexit: %s\n", strerror(errno));
+        abort();
+    }
+    backdoor_init(backdoor_base, backdoor_pages * getpagesize());
+#endif
 
 #if defined(CONFIG_USE_GUEST_BASE)
     /* Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 994c02b..2232363 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -31,6 +31,10 @@
 #include "qemu.h"
 #include "qemu-common.h"
 
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
+
 //#define DEBUG_MMAP
 
 #if defined(CONFIG_USE_NPTL)
@@ -553,6 +557,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         }
     }
  the_end1:
+#if defined(CONFIG_BACKDOOR)
+    backdoor_guest_mmap(fd, (void *)g2h(start));
+#endif
     page_set_flags(start, start + len, prot | PAGE_VALID);
  the_end:
 #ifdef DEBUG_MMAP
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f227097..d2dc11e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -100,6 +100,10 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
 
 #include "qemu.h"
 
+#if defined(CONFIG_BACKDOOR)
+#include "backdoor/qemu/user.h"
+#endif
+
 #if defined(CONFIG_USE_NPTL)
 #define CLONE_NPTL_FLAGS2 (CLONE_SETTLS | \
     CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID)
@@ -4663,6 +4667,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+#if defined(CONFIG_BACKDOOR)
+        backdoor_fini();
+#endif
         _exit(arg1);
         ret = 0; /* avoid warning */
         break;
@@ -6397,6 +6404,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+#if defined(CONFIG_BACKDOOR)
+        backdoor_fini();
+#endif
         ret = get_errno(exit_group(arg1));
         break;
 #endif

[Qemu-devel] [PATCH v2 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"

[Lluís Vilanova]

Uses a virtual device to proxy uses of the backdoor communication channel to the
user-provided code.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile.objs           |    1 
 backdoor/qemu/softmmu.c |  186 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/pci.h                |    1 
 3 files changed, 188 insertions(+), 0 deletions(-)
 create mode 100644 backdoor/qemu/softmmu.c

diff --git a/Makefile.objs b/Makefile.objs
index 9784441..a45ff56 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -401,6 +401,7 @@ $(trace-obj-y): $(GENERATED_HEADERS)
 # backdoor
 
 backdoor-nested-$(CONFIG_USER_ONLY) += user.o
+backdoor-nested-$(CONFIG_SOFTMMU) += softmmu.o
 
 backdoor-obj-y += $(addprefix backdoor/qemu/, $(backdoor-nested-y))
 
diff --git a/backdoor/qemu/softmmu.c b/backdoor/qemu/softmmu.c
new file mode 100644
index 0000000..9cde59f
--- /dev/null
+++ b/backdoor/qemu/softmmu.c
@@ -0,0 +1,186 @@
+/*
+ * QEMU-side management of backdoor channels in softmmu emulation.
+ *
+ * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "hw/pci.h"
+#include "backdoor/qemu/qemu-backdoor.h"
+
+
+#define PAGE_SIZE TARGET_PAGE_SIZE
+
+
+typedef struct BackdoorState
+{
+    PCIDevice dev;
+
+    uint8_t pages;
+    uint64_t size;
+
+    union
+    {
+        uint64_t v;
+        char     a[8];
+    } c_size;
+    union
+    {
+        uint64_t v;
+        uint8_t  a[8];
+    } c_cmd;
+
+    void *data_ptr;
+    MemoryRegion data;
+    MemoryRegion control;
+} BackdoorState;
+
+
+static uint64_t backdoor_control_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
+{
+    BackdoorState *s = opaque;
+
+    /* c_size already has target endianess */
+
+    switch (size) {
+    case 1:
+    {
+        uint8_t *res = (uint8_t*)&s->c_size.a[addr % sizeof(uint64_t)];
+        return *res;
+    }
+    case 2:
+    {
+        uint16_t *res = (uint16_t*)&s->c_size.a[addr % sizeof(uint64_t)];
+        return *res;
+    }
+    case 4:
+    {
+        uint32_t *res = (uint32_t*)&s->c_size.a[addr % sizeof(uint64_t)];
+        return *res;
+    }
+    case 8:
+    {
+        uint64_t *res = (uint64_t*)&s->c_size.a[addr % sizeof(uint64_t)];
+        return *res;
+    }
+    default:
+        fprintf(stderr, "error: backdoor: Unexpected read of size %d\n", size);
+        abort();
+    }
+}
+
+static void backdoor_control_io_write(void *opaque, target_phys_addr_t addr, uint64_t data, unsigned size)
+{
+    BackdoorState *s = opaque;
+
+    /* c_cmd will have target endianess (left up to the user) */
+
+    switch (size) {
+    case 1:
+    {
+        uint8_t *res = (uint8_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
+        *res = (uint8_t)data;
+        break;
+    }
+    case 2:
+    {
+        uint16_t *res = (uint16_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
+        *res = (uint16_t)data;
+        break;
+    }
+    case 4:
+    {
+        uint32_t *res = (uint32_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
+        *res = (uint32_t)data;
+        break;
+    }
+    case 8:
+    {
+        uint64_t *res = (uint64_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
+        *res = (uint64_t)data;
+        break;
+    }
+    default:
+        fprintf(stderr, "error: backdoor: Unexpected write of size %d\n", size);
+        abort();
+    }
+
+    if ((addr + size) % sizeof(s->c_cmd.v) == 0) {
+        qemu_backdoor(s->c_cmd.v, s->data_ptr);
+    }
+}
+
+static const MemoryRegionOps backdoor_control_ops = {
+    .read = backdoor_control_io_read,
+    .write = backdoor_control_io_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+    },
+};
+
+
+static int backdoor_init(PCIDevice *dev)
+{
+    BackdoorState *s = DO_UPCAST(BackdoorState, dev, dev);
+
+    if (s->pages < 1) {
+        fprintf(stderr, "error: backdoor: "
+                "the data channel must have one or more pages\n");
+        return -1;
+    }
+    s->size = s->pages * PAGE_SIZE;
+    s->c_size.v = tswap64(s->size);
+
+    pci_set_word(s->dev.config + PCI_COMMAND,
+                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
+
+    memory_region_init_io(&s->control, &backdoor_control_ops, s,
+                          "backdoor.control", PAGE_SIZE);
+    pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->control);
+
+    memory_region_init_ram(&s->data, &s->dev.qdev,
+                           "backdoor.data", s->c_size.v);
+    pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->data);
+    s->data_ptr = qemu_get_ram_ptr(s->data.ram_addr);
+
+    qemu_backdoor_init(s->c_size.v);
+
+    return 0;
+}
+
+static int backdoor_fini(PCIDevice *dev)
+{
+    BackdoorState *s = DO_UPCAST(BackdoorState, dev, dev);
+
+    memory_region_destroy(&s->data);
+    memory_region_destroy(&s->control);
+
+    return 0;
+}
+
+
+static PCIDeviceInfo backdoor_info = {
+    .qdev.name  = "backdoor",
+    .qdev.desc  = "Backdoor communication channel",
+    .qdev.size  = sizeof(BackdoorState),
+    .init       = backdoor_init,
+    .exit       = backdoor_fini,
+    .vendor_id  = PCI_VENDOR_ID_REDHAT_QUMRANET,
+    .device_id  = PCI_DEVICE_ID_BACKDOOR,
+    .class_id   = PCI_CLASS_MEMORY_RAM,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_UINT8("pages", BackdoorState, pages, 1),
+        DEFINE_PROP_END_OF_LIST(),
+    }
+};
+
+static void backdoor_register_device(void)
+{
+    pci_qdev_register(&backdoor_info);
+}
+
+device_init(backdoor_register_device)
diff --git a/hw/pci.h b/hw/pci.h
index 625e717..e7dc3cb 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -75,6 +75,7 @@
 #define PCI_DEVICE_ID_VIRTIO_BLOCK       0x1001
 #define PCI_DEVICE_ID_VIRTIO_BALLOON     0x1002
 #define PCI_DEVICE_ID_VIRTIO_CONSOLE     0x1003
+#define PCI_DEVICE_ID_BACKDOOR           0x1004
 
 #define FMT_PCIBUS                      PRIx64
 

[Qemu-devel] [PATCH v2 5/5] backdoor: Add guest-side library

[Lluís Vilanova]

Guest applications can link against "libqemu-backdoor-guest.a" to use the
backdoor communication channel.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 backdoor/guest/Makefile        |   18 ++++
 backdoor/guest/common.c        |  205 ++++++++++++++++++++++++++++++++++++++++
 backdoor/guest/qemu-backdoor.h |   59 ++++++++++++
 backdoor/qemu/user.c           |    2 
 configure                      |    2 
 5 files changed, 285 insertions(+), 1 deletions(-)
 create mode 100644 backdoor/guest/Makefile
 create mode 100644 backdoor/guest/common.c
 create mode 100644 backdoor/guest/qemu-backdoor.h

diff --git a/backdoor/guest/Makefile b/backdoor/guest/Makefile
new file mode 100644
index 0000000..200ee3b
--- /dev/null
+++ b/backdoor/guest/Makefile
@@ -0,0 +1,18 @@
+include ../../../config-host.mak
+include ../../config-target.mak
+include $(SRC_PATH)/rules.mak
+
+vpath % $(SRC_PATH)/backdoor/guest
+
+QEMU_CFLAGS += $(GLIB_CFLAGS)
+QEMU_CFLAGS += -I../../../
+QEMU_CFLAGS += -I../../
+
+obj-y = common.o
+
+libqemu-backdoor-guest.a: $(obj-y)
+
+all: libqemu-backdoor-guest.a
+
+clean:
+	rm -f $(obj-y) libqemu-backdoor-guest.a
diff --git a/backdoor/guest/common.c b/backdoor/guest/common.c
new file mode 100644
index 0000000..2238dfa
--- /dev/null
+++ b/backdoor/guest/common.c
@@ -0,0 +1,205 @@
+/*
+ * Guest-side management of backdoor channels.
+ *
+ * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu-backdoor.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <glob.h>
+
+#include "config-target.h"
+#include "hw/pci_ids.h"
+#include "hw/pci.h"
+
+
+static char *data_path = NULL;
+static char *control_path = NULL;
+static int data_fd = -1;
+static int control_fd = -1;
+
+static void *data_addr = NULL;
+static void *control_addr = NULL;
+
+
+static int init_channel_file(const char *base, const char *suffix, size_t size,
+                             char ** path, int *fd, void **addr)
+{
+    *path = malloc(strlen(base) + strlen(suffix) + 1);
+    sprintf(*path, "%s%s", base, suffix);
+
+    *fd = open(*path, O_RDWR);
+    if (*fd == -1) {
+        return -1;
+    }
+
+    *addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, *fd, 0);
+    if (*addr == MAP_FAILED) {
+        return -1;
+    }
+    return 0;
+}
+
+#if !defined(CONFIG_USER_ONLY) && defined(__linux__)
+static int check_device_id (const char *base, const char *name, uint64_t value)
+{
+    char tmp[1024];
+    sprintf(tmp, "%s/%s", base, name);
+
+    int fd = open(tmp, O_RDONLY);
+    if (fd < 0) {
+        return -1;
+    }
+
+    char v[1024];
+    ssize_t s = read(fd, v, sizeof(v));
+    if (s < 0) {
+        close(fd);
+        return -1;
+    }
+    v[s] = '\0';
+
+    char *end;
+    uint64_t vv = strtoull(v, &end, 16);
+    if (*end == '\n' && vv == value) {
+        return 0;
+    }
+    else {
+        return -1;
+    }
+}
+
+static char* find_device(void)
+{
+    static char tmp[1024];
+    char *res = NULL;
+
+    glob_t g;
+    if (glob("/sys/devices/pci*/*", GLOB_NOSORT, NULL, &g) != 0) {
+        return NULL;
+    }
+
+
+    int i;
+    for (i = 0; i < g.gl_pathc; i++) {
+        char *path = g.gl_pathv[i];
+
+        if (check_device_id(path, "vendor", PCI_VENDOR_ID_REDHAT_QUMRANET) < 0) {
+            continue;
+        }
+        if (check_device_id(path, "device", PCI_DEVICE_ID_BACKDOOR) < 0) {
+            continue;
+        }
+
+        sprintf(tmp, "%s", path);
+        res = tmp;
+        break;
+    }
+
+    globfree(&g);
+
+    return res;
+}
+#endif
+
+int qemu_backdoor_init(const char *base)
+{
+#if defined(CONFIG_USER_ONLY)
+    const char *control_suff = "-control";
+    const size_t control_size = getpagesize() * 2;
+    const char *data_suff = "-data";
+#elif defined(__linux__)
+    const char *control_suff = "/resource0";
+    const size_t control_size = getpagesize();
+    const char *data_suff = "/resource1";
+#else
+#error Unsupported OS
+#endif
+
+#if !defined(CONFIG_USER_ONLY)
+    if (base == NULL) {
+        /* try to guess the base path */
+        base = find_device();
+        if (base == NULL) {
+            return -1;
+        }
+    }
+#endif
+
+    int res;
+    res = init_channel_file(base, control_suff, control_size,
+                            &control_path, &control_fd, &control_addr);
+    if (res != 0) {
+        return res;
+    }
+
+    res = init_channel_file(base, data_suff, qemu_backdoor_data_size(),
+                            &data_path, &data_fd, &data_addr);
+    if (res != 0) {
+        return res;
+    }
+    return 0;
+}
+
+
+static int fini_channel(int *fd, char **path)
+{
+    if (*fd != -1) {
+        if (close(*fd) == -1) {
+            return -1;
+        }
+        *fd = -1;
+    }
+    if (*path != NULL) {
+        free(*path);
+        *path =  NULL;
+    }
+    return 0;
+}
+
+int qemu_backdoor_fini(void)
+{
+    if (fini_channel(&data_fd, &data_path) != 0) {
+        return -1;
+    }
+    if (fini_channel(&control_fd, &control_path) != 0) {
+        return -1;
+    }
+    return 0;
+}
+
+
+uint64_t qemu_backdoor_data_size(void)
+{
+    return *(uint64_t*)control_addr;
+}
+
+void *qemu_backdoor_data(void)
+{
+    return data_addr;
+}
+
+void qemu_backdoor (uint64_t cmd)
+{
+    uint64_t *ctrl;
+    ctrl = control_addr;
+    ctrl[1] = cmd;
+#if defined(CONFIG_USER_ONLY)
+    /* QEMU in 'user' mode uses two faulting pages to detect invocations */
+    ctrl = control_addr + getpagesize();
+    ctrl[1] = cmd;
+#endif
+}
diff --git a/backdoor/guest/qemu-backdoor.h b/backdoor/guest/qemu-backdoor.h
new file mode 100644
index 0000000..0e34dc9
--- /dev/null
+++ b/backdoor/guest/qemu-backdoor.h
@@ -0,0 +1,59 @@
+/*
+ * Guest-side management of backdoor channels.
+ *
+ * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <stdint.h>
+#include <sys/types.h>
+
+
+/**
+ * Initialize the backdoor channel.
+ *
+ * The base path to the backdoor channel is dependant on the type of QEMU
+ * target:
+ *
+ * - User (single-application)
+ *   The base path provided when starting QEMU ("-backdoor" commandline option).
+ *
+ * - Softmmu (full-system); OS-dependant
+ *   - Linux: The base path to the backdoor channel virtual device (on a default
+ *     device setup for x86 this is "/sys/devices/pci0000:00/0000:00:04.0"). If
+ *     NULL is provided, the first backdoor device to be found will be used.
+ *
+ * @param base Base path to the backdoor channel.
+ * @return Zero on success.
+ */
+int qemu_backdoor_init(const char *base);
+
+/**
+ * Deinitialize the backdoor channel.
+ *
+ * @return Zero on success.
+ */
+int qemu_backdoor_fini(void);
+
+/**
+ * The amount of data that can be passed through the data channel.
+ */
+uint64_t qemu_backdoor_data_size(void);
+
+/**
+ * Pointer to the start of the data channel.
+ */
+void *qemu_backdoor_data(void);
+
+/**
+ * Invoke the control channel.
+ *
+ * The command in the control channel can be used to, e.g., differentiate
+ * between multiple users of the backdoor channel without resorting to the data
+ * channel for deambiguation.
+ *
+ * @param cmd Integral number passed out-of-band w.r.t the data channel.
+ */
+void qemu_backdoor(uint64_t cmd);
diff --git a/backdoor/qemu/user.c b/backdoor/qemu/user.c
index 6e178cc..54d797f 100644
--- a/backdoor/qemu/user.c
+++ b/backdoor/qemu/user.c
@@ -114,7 +114,7 @@ static void fini_channel(int *fd, char **path)
         *fd = -1;
     }
     if (*path != NULL) {
-        g_free(path);
+        g_free(*path);
         *path =  NULL;
     }
 }
diff --git a/configure b/configure
index 8af8313..67d4c9a 100755
--- a/configure
+++ b/configure
@@ -3374,6 +3374,8 @@ if test -n "$backdoor"; then
     mkdir -p $target_dir/libbackdoor
     symlink $backdoor/Makefile $target_dir/libbackdoor/Makefile
     mkdir -p $target_dir/backdoor/qemu
+    mkdir -p $target_dir/backdoor/guest
+    symlink $source_path/backdoor/guest/Makefile $target_dir/backdoor/guest/Makefile
 fi
 
 

Re: [Qemu-devel] [PATCH v2 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"

[Anthony Liguori]

I really worry about us introducing so many of these one-off paravirtual devices.

I would much prefer that you look at doing this as an extension to the ivshmem 
device as it already has this sort of scope.  You should be able to do this by 
just extending the size of bar 1 and using a well known guest id.

Regards,

Anthony Liguori

On 12/05/2011 04:23 PM, Lluís Vilanova wrote:
> Uses a virtual device to proxy uses of the backdoor communication channel to the
> user-provided code.
>
> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
> ---
>   Makefile.objs           |    1
>   backdoor/qemu/softmmu.c |  186 +++++++++++++++++++++++++++++++++++++++++++++++
>   hw/pci.h                |    1
>   3 files changed, 188 insertions(+), 0 deletions(-)
>   create mode 100644 backdoor/qemu/softmmu.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 9784441..a45ff56 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -401,6 +401,7 @@ $(trace-obj-y): $(GENERATED_HEADERS)
>   # backdoor
>
>   backdoor-nested-$(CONFIG_USER_ONLY) += user.o
> +backdoor-nested-$(CONFIG_SOFTMMU) += softmmu.o
>
>   backdoor-obj-y += $(addprefix backdoor/qemu/, $(backdoor-nested-y))
>
> diff --git a/backdoor/qemu/softmmu.c b/backdoor/qemu/softmmu.c
> new file mode 100644
> index 0000000..9cde59f
> --- /dev/null
> +++ b/backdoor/qemu/softmmu.c
> @@ -0,0 +1,186 @@
> +/*
> + * QEMU-side management of backdoor channels in softmmu emulation.
> + *
> + * Copyright (C) 2011 Lluís Vilanova<vilanova@ac.upc.edu>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "hw/pci.h"
> +#include "backdoor/qemu/qemu-backdoor.h"
> +
> +
> +#define PAGE_SIZE TARGET_PAGE_SIZE
> +
> +
> +typedef struct BackdoorState
> +{
> +    PCIDevice dev;
> +
> +    uint8_t pages;
> +    uint64_t size;
> +
> +    union
> +    {
> +        uint64_t v;
> +        char     a[8];
> +    } c_size;
> +    union
> +    {
> +        uint64_t v;
> +        uint8_t  a[8];
> +    } c_cmd;
> +
> +    void *data_ptr;
> +    MemoryRegion data;
> +    MemoryRegion control;
> +} BackdoorState;
> +
> +
> +static uint64_t backdoor_control_io_read(void *opaque, target_phys_addr_t addr, unsigned size)
> +{
> +    BackdoorState *s = opaque;
> +
> +    /* c_size already has target endianess */
> +
> +    switch (size) {
> +    case 1:
> +    {
> +        uint8_t *res = (uint8_t*)&s->c_size.a[addr % sizeof(uint64_t)];
> +        return *res;
> +    }
> +    case 2:
> +    {
> +        uint16_t *res = (uint16_t*)&s->c_size.a[addr % sizeof(uint64_t)];
> +        return *res;
> +    }
> +    case 4:
> +    {
> +        uint32_t *res = (uint32_t*)&s->c_size.a[addr % sizeof(uint64_t)];
> +        return *res;
> +    }
> +    case 8:
> +    {
> +        uint64_t *res = (uint64_t*)&s->c_size.a[addr % sizeof(uint64_t)];
> +        return *res;
> +    }
> +    default:
> +        fprintf(stderr, "error: backdoor: Unexpected read of size %d\n", size);
> +        abort();
> +    }
> +}
> +
> +static void backdoor_control_io_write(void *opaque, target_phys_addr_t addr, uint64_t data, unsigned size)
> +{
> +    BackdoorState *s = opaque;
> +
> +    /* c_cmd will have target endianess (left up to the user) */
> +
> +    switch (size) {
> +    case 1:
> +    {
> +        uint8_t *res = (uint8_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
> +        *res = (uint8_t)data;
> +        break;
> +    }
> +    case 2:
> +    {
> +        uint16_t *res = (uint16_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
> +        *res = (uint16_t)data;
> +        break;
> +    }
> +    case 4:
> +    {
> +        uint32_t *res = (uint32_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
> +        *res = (uint32_t)data;
> +        break;
> +    }
> +    case 8:
> +    {
> +        uint64_t *res = (uint64_t*)&s->c_cmd.a[addr % sizeof(uint64_t)];
> +        *res = (uint64_t)data;
> +        break;
> +    }
> +    default:
> +        fprintf(stderr, "error: backdoor: Unexpected write of size %d\n", size);
> +        abort();
> +    }
> +
> +    if ((addr + size) % sizeof(s->c_cmd.v) == 0) {
> +        qemu_backdoor(s->c_cmd.v, s->data_ptr);
> +    }
> +}
> +
> +static const MemoryRegionOps backdoor_control_ops = {
> +    .read = backdoor_control_io_read,
> +    .write = backdoor_control_io_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +};
> +
> +
> +static int backdoor_init(PCIDevice *dev)
> +{
> +    BackdoorState *s = DO_UPCAST(BackdoorState, dev, dev);
> +
> +    if (s->pages<  1) {
> +        fprintf(stderr, "error: backdoor: "
> +                "the data channel must have one or more pages\n");
> +        return -1;
> +    }
> +    s->size = s->pages * PAGE_SIZE;
> +    s->c_size.v = tswap64(s->size);
> +
> +    pci_set_word(s->dev.config + PCI_COMMAND,
> +                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
> +
> +    memory_region_init_io(&s->control,&backdoor_control_ops, s,
> +                          "backdoor.control", PAGE_SIZE);
> +    pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,&s->control);
> +
> +    memory_region_init_ram(&s->data,&s->dev.qdev,
> +                           "backdoor.data", s->c_size.v);
> +    pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,&s->data);
> +    s->data_ptr = qemu_get_ram_ptr(s->data.ram_addr);
> +
> +    qemu_backdoor_init(s->c_size.v);
> +
> +    return 0;
> +}
> +
> +static int backdoor_fini(PCIDevice *dev)
> +{
> +    BackdoorState *s = DO_UPCAST(BackdoorState, dev, dev);
> +
> +    memory_region_destroy(&s->data);
> +    memory_region_destroy(&s->control);
> +
> +    return 0;
> +}
> +
> +
> +static PCIDeviceInfo backdoor_info = {
> +    .qdev.name  = "backdoor",
> +    .qdev.desc  = "Backdoor communication channel",
> +    .qdev.size  = sizeof(BackdoorState),
> +    .init       = backdoor_init,
> +    .exit       = backdoor_fini,
> +    .vendor_id  = PCI_VENDOR_ID_REDHAT_QUMRANET,
> +    .device_id  = PCI_DEVICE_ID_BACKDOOR,
> +    .class_id   = PCI_CLASS_MEMORY_RAM,
> +    .qdev.props = (Property[]) {
> +        DEFINE_PROP_UINT8("pages", BackdoorState, pages, 1),
> +        DEFINE_PROP_END_OF_LIST(),
> +    }
> +};
> +
> +static void backdoor_register_device(void)
> +{
> +    pci_qdev_register(&backdoor_info);
> +}
> +
> +device_init(backdoor_register_device)
> diff --git a/hw/pci.h b/hw/pci.h
> index 625e717..e7dc3cb 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -75,6 +75,7 @@
>   #define PCI_DEVICE_ID_VIRTIO_BLOCK       0x1001
>   #define PCI_DEVICE_ID_VIRTIO_BALLOON     0x1002
>   #define PCI_DEVICE_ID_VIRTIO_CONSOLE     0x1003
> +#define PCI_DEVICE_ID_BACKDOOR           0x1004
>
>   #define FMT_PCIBUS                      PRIx64
>
>
>

Re: [Qemu-devel] [PATCH v2 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"

[Lluís Vilanova]

Anthony Liguori writes:

> I really worry about us introducing so many of these one-off paravirtual devices.
> I would much prefer that you look at doing this as an extension to the ivshmem
> device as it already has this sort of scope.  You should be able to do this by
> just extending the size of bar 1 and using a well known guest id.

I did in fact look at ivshmem some time ago, and it's true that both use the
same mechanisms; but each device has a completely different purpose. To me it
just seems that extending the control BAR in ivshmem to call the user-provided
backdoor callbacks is just conflating two completely separate devices into a
single one. Besides, I think that the qemu-side of the backdoor is simple enough
to avoid being a maintenance burden.

Another question would be to join both so that the backdoor can be used to
orchestrate operations between multiple VMs through ivshmem's server, but I
really have no time to look into that and don't even know whether it would then
make sense to join both devices.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"

[Anthony Liguori]

On 12/06/2011 04:30 PM, Lluís Vilanova wrote:
> Anthony Liguori writes:
>
>> I really worry about us introducing so many of these one-off paravirtual devices.
>> I would much prefer that you look at doing this as an extension to the ivshmem
>> device as it already has this sort of scope.  You should be able to do this by
>> just extending the size of bar 1 and using a well known guest id.
>
> I did in fact look at ivshmem some time ago, and it's true that both use the
> same mechanisms; but each device has a completely different purpose. To me it
> just seems that extending the control BAR in ivshmem to call the user-provided
> backdoor callbacks is just conflating two completely separate devices into a
> single one. Besides, I think that the qemu-side of the backdoor is simple enough
> to avoid being a maintenance burden.

They have the same purpose (which are both vague TBH).  The only reason I'm 
sympathetic to this device is that virtio-serial has such insane semantics.

It's bad enough we already have two "backdoor" mechanisms, adding a third seems 
insane to me.

Regards,

Anthony Liguori

>
> Another question would be to join both so that the backdoor can be used to
> orchestrate operations between multiple VMs through ivshmem's server, but I
> really have no time to look into that and don't even know whether it would then
> make sense to join both devices.
>
>
> Thanks,
>    Lluis
>

Re: [Qemu-devel] [PATCH v2 1/5] backdoor: Add documentation

[Peter Maydell]

2011/12/5 Lluís Vilanova <vilanova@ac.upc.edu>:
> +3. Build QEMU with the backdoor feature:
> +
> +    /path/to/qemu/configure --with-backdoor=/tmp/my-backdoor-qemu

If we want to do this wouldn't it make more sense to do it
by having qemu dynamically load a plugin DLL with a specified
ABI rather than having to compile the specific backdoor
handler into qemu proper?

-- PMM

Re: [Qemu-devel] [PATCH v2 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"

[Peter Maydell]

2011/12/6 Anthony Liguori <anthony@codemonkey.ws>:
> It's bad enough we already have two "backdoor" mechanisms, adding a third
> seems insane to me.

Isn't there yet another one out-of-tree in the android emulator?

-- PMM

Re: [Qemu-devel] [PATCH v2 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"

[Lluís Vilanova]

Lluís Vilanova writes:

> Anthony Liguori writes:
>> I really worry about us introducing so many of these one-off paravirtual devices.
>> I would much prefer that you look at doing this as an extension to the ivshmem
>> device as it already has this sort of scope.  You should be able to do this by
>> just extending the size of bar 1 and using a well known guest id.

> I did in fact look at ivshmem some time ago, and it's true that both use the
> same mechanisms; but each device has a completely different purpose. To me it
> just seems that extending the control BAR in ivshmem to call the user-provided
> backdoor callbacks is just conflating two completely separate devices into a
> single one. Besides, I think that the qemu-side of the backdoor is simple enough
> to avoid being a maintenance burden.

> Another question would be to join both so that the backdoor can be used to
> orchestrate operations between multiple VMs through ivshmem's server, but I
> really have no time to look into that and don't even know whether it would then
> make sense to join both devices.

BTW, I think that having the softmmu-side of the backdoor inside ivshmem would
in fact be a simple change. I just want to make sure whether the reason to merge
both is about minimising code maintenance or rather thinking that it would make
more sense to have both as a single pack of functionalities.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 1/5] backdoor: Add documentation

[Anthony Liguori]

On 12/05/2011 04:22 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
> ---
>   docs/backdoor.txt |  167 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 167 insertions(+), 0 deletions(-)
>   create mode 100644 docs/backdoor.txt
>
> diff --git a/docs/backdoor.txt b/docs/backdoor.txt
> new file mode 100644
> index 0000000..1c6502a
> --- /dev/null
> +++ b/docs/backdoor.txt
> @@ -0,0 +1,167 @@
> += Backdoor communication channel =
> +
> +== Introduction ==
> +
> +This document describes how the guest can use the backdoor communication channel
> +to interact with user-provided code inside QEMU.
> +
> +The backdoor provides a lightweight and guest-initiated communication channel
> +between code running inside the guest system and code in QEMU, including both
> +QEMU in 'softmmu' and 'user' modes.
> +
> +The semantics of the backdoor channel are up to the user, who must provide the
> +implementation of the QEMU-side callbacks used when the backdoor channel is
> +invoked.
> +
> +On the guest side, code can be linked against a simple library provided in QEMU
> +to interface with the backdoor channel.
> +
> +The features of this mechanism are:
> +
> +* Minimal setup for the guest.
> +* Independent of guest architecture.
> +* Works with 'softmmu' and 'user' mode.
> +* Negligible guest overhead; guest invocations of the backdoor channel does not
> +  go through any OS abstraction, except during the setup of the communication
> +  channel.
> +* Negligible host overhead; invocations of the backdoor channel are interpreted
> +  by QEMU, while a side-channel can be used as regular memory to communicate
> +  bulk data without any extra overhead.
> +* The user-provided backdoor callbacks can perform arbitrary actions on the
> +  guest system (e.g., read or write memory, change register values, etc.).
> +
> +
> +== QEMU-side code ==
> +
> +1. Create the "Makefile" to build the user-provided backdoor channel library:
> +
> +    mkdir /tmp/my-backdoor-qemu
> +    cat>  /tmp/my-backdoor-qemu/Makefile<<EOF
> +    include $(BUILD_DIR)/config-host.mak
> +    include $(BUILD_DIR)/$(TARGET_DIR)../config-target.mak
> +    include $(SRC_PATH)/rules.mak
> +
> +    vpath %.c /tmp/my-backdoor-qemu
> +
> +
> +    libbackdoor.a: backdoor.o
> +
> +
> +    # Include automatically generated dependency files
> +    -include $(wildcard *.d)
> +    EOF
> +
> +2. Implement the callbacks declared in "backdoor/qemu/qemu-backdoor.h":
> +
> +    cat>  /tmp/my-backdoor-qemu/backdoor.c<<EOF

Oh, I didn't see this.

Major Nack on this.  This is not reasonable at all to do.  We're not sneaking in 
an ad-hoc plugin interface here.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v2 1/5] backdoor: Add documentation

[Anthony Liguori]

On 12/06/2011 04:36 PM, Peter Maydell wrote:
> 2011/12/5 Lluís Vilanova<vilanova@ac.upc.edu>:
>> +3. Build QEMU with the backdoor feature:
>> +
>> +    /path/to/qemu/configure --with-backdoor=/tmp/my-backdoor-qemu
>
> If we want to do this wouldn't it make more sense to do it
> by having qemu dynamically load a plugin DLL with a specified
> ABI rather than having to compile the specific backdoor
> handler into qemu proper?

No way.  If you want to implement a custom device, then send a proper patch. 
We're not adding hooks to let people add random things like this.

Regards,

Anthony Liguori

>
> -- PMM
>

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Anthony Liguori]

On 12/05/2011 04:22 PM, Lluís Vilanova wrote:
> Provides the ability for the guest to communicate with user-provided code inside
> QEMU itself, using a lightweight mechanism.
>
> See first commit for a full description.
>
> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>

This whole series:

Nacked-by: Anthony Liguori <aliguori@us.ibm.com>

If you want to extend QEMU, then send proper patches.  Adding random C files to 
the build is unacceptable.

Regards,

Anthony Liguori

> ---
>
> Changes in v2:
>
> * Rebase on 1c8a881daaca6fe0646a425b0970fb3ad25f6732 from master.
> * Multiple documentation fixes and clarifications.
> * Fixed a couple of bugs when freeing memory.
> * Actually call 'backdoor_fini' in user mode when exiting.
> * Try to auto-detect the backdoor device in the example guest library when
>    running a full-system linux.
> * Rewrite code to read and write into the control channel in softmmu mode,
>    making it much clearer and leaving endianness handling up to the user-provided
>    backdoor callbacks.
>
>
> Lluís Vilanova (5):
>        backdoor: Add documentation
>        backdoor: Add build infrastructure
>        backdoor: [*-user] Add QEMU-side proxy to "libbackdoor.a"
>        backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"
>        backdoor: Add guest-side library
>
>
>   Makefile                       |    4 +
>   Makefile.objs                  |   21 ++++
>   Makefile.target                |    4 +
>   backdoor/guest/Makefile        |   18 ++++
>   backdoor/guest/common.c        |  205 ++++++++++++++++++++++++++++++++++++++++
>   backdoor/guest/qemu-backdoor.h |   59 ++++++++++++
>   backdoor/qemu/qemu-backdoor.h  |   31 ++++++
>   backdoor/qemu/softmmu.c        |  186 ++++++++++++++++++++++++++++++++++++
>   backdoor/qemu/user.c           |  202 +++++++++++++++++++++++++++++++++++++++
>   backdoor/qemu/user.h           |   30 ++++++
>   bsd-user/main.c                |   26 +++++
>   bsd-user/mmap.c                |    7 +
>   bsd-user/syscall.c             |   13 +++
>   configure                      |   35 +++++++
>   darwin-user/main.c             |   26 +++++
>   darwin-user/mmap.c             |    7 +
>   docs/backdoor.txt              |  167 +++++++++++++++++++++++++++++++++
>   hw/pci.h                       |    1
>   linux-user/main.c              |   32 ++++++
>   linux-user/mmap.c              |    7 +
>   linux-user/syscall.c           |   10 ++
>   21 files changed, 1089 insertions(+), 2 deletions(-)
>   create mode 100644 backdoor/guest/Makefile
>   create mode 100644 backdoor/guest/common.c
>   create mode 100644 backdoor/guest/qemu-backdoor.h
>   create mode 100644 backdoor/qemu/qemu-backdoor.h
>   create mode 100644 backdoor/qemu/softmmu.c
>   create mode 100644 backdoor/qemu/user.c
>   create mode 100644 backdoor/qemu/user.h
>   create mode 100644 docs/backdoor.txt
>
>

[Qemu-devel] Insane virtio-serial semantics (was: [PATCH v2 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a")

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 12/06/2011 04:30 PM, Lluís Vilanova wrote:
>> Anthony Liguori writes:
>>
>>> I really worry about us introducing so many of these one-off paravirtual devices.
>>> I would much prefer that you look at doing this as an extension to the ivshmem
>>> device as it already has this sort of scope.  You should be able to do this by
>>> just extending the size of bar 1 and using a well known guest id.
>>
>> I did in fact look at ivshmem some time ago, and it's true that both use the
>> same mechanisms; but each device has a completely different purpose. To me it
>> just seems that extending the control BAR in ivshmem to call the user-provided
>> backdoor callbacks is just conflating two completely separate devices into a
>> single one. Besides, I think that the qemu-side of the backdoor is simple enough
>> to avoid being a maintenance burden.
>
> They have the same purpose (which are both vague TBH).  The only
> reason I'm sympathetic to this device is that virtio-serial has such
> insane semantics.

Could you summarize what's wrong?  Is it fixable?

[...]

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Anthony Liguori writes:

> On 12/05/2011 04:22 PM, Lluís Vilanova wrote:
>> Provides the ability for the guest to communicate with user-provided code inside
>> QEMU itself, using a lightweight mechanism.
>> 
>> See first commit for a full description.
>> 
>> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>

> This whole series:

> Nacked-by: Anthony Liguori <aliguori@us.ibm.com>

> If you want to extend QEMU, then send proper patches.  Adding random C files to
> the build is unacceptable.

What do you mean by proper patches? The whole point of this is to ease the
process for the user to add custom guest-to-qemu interactions, so there is no
"proper implementation" here.

Blue Swirl already mentioned this can be used for testing qemu itself, while my
use case is coupled with the trace instrumentation features (I'll probably send
the first batch today).


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] Insane virtio-serial semantics

[Anthony Liguori]

On 12/07/2011 02:21 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>> On 12/06/2011 04:30 PM, Lluís Vilanova wrote:
>>> Anthony Liguori writes:
>>>
>>>> I really worry about us introducing so many of these one-off paravirtual devices.
>>>> I would much prefer that you look at doing this as an extension to the ivshmem
>>>> device as it already has this sort of scope.  You should be able to do this by
>>>> just extending the size of bar 1 and using a well known guest id.
>>>
>>> I did in fact look at ivshmem some time ago, and it's true that both use the
>>> same mechanisms; but each device has a completely different purpose. To me it
>>> just seems that extending the control BAR in ivshmem to call the user-provided
>>> backdoor callbacks is just conflating two completely separate devices into a
>>> single one. Besides, I think that the qemu-side of the backdoor is simple enough
>>> to avoid being a maintenance burden.
>>
>> They have the same purpose (which are both vague TBH).  The only
>> reason I'm sympathetic to this device is that virtio-serial has such
>> insane semantics.
>
> Could you summarize what's wrong?  Is it fixable?

I don't think so as it's part of the userspace ABI now.

Mike, please help me make sure I get this all right.  A normal file/socket has 
the following guest semantics:

1) When a disconnect occurs, you will receive a return of '0' or -EPIPE 
depending on the platform.  The fd is now unusable and you must close/reopen.

2) You can setup SIGIO/SIGPIPE to fire off whenever a file descriptor becomes 
readable/writable.

virtio serial has the following semantics:

1) When a disconnect occurs, if you read() you will receive an -EPIPE.

2) However, if a reconnect occurs before you issue your read(), the read will 
complete with no indication that a disconnect occurred.

3) This makes it impossible to determine whether a disconnect has occurred which 
makes it very hard to reset your protocol stream.  To deal with this, 
virtio-serial can issue a SIGIO signal upon disconnect.

4) Signals are asynchronous, so a reconnect may have occurred by the time you 
get the SIGIO signal.  It's unclear that you can do anything useful with this.

So besides overloading the meaning of SIGIO, there's really no way to figure out 
in the guest when a reconnect has occurred.  To deal with this in qemu-ga, we 
actually only allow 7-bit data transfers and use the 8th bit as an in-band 
message to tell the guest that a reset has occurred.

Regards,

Anthony Liguori

>
> [...]
>

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Anthony Liguori]

On 12/07/2011 06:21 AM, Lluís Vilanova wrote:
> Anthony Liguori writes:
>
>> On 12/05/2011 04:22 PM, Lluís Vilanova wrote:
>>> Provides the ability for the guest to communicate with user-provided code inside
>>> QEMU itself, using a lightweight mechanism.
>>>
>>> See first commit for a full description.
>>>
>>> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
>
>> This whole series:
>
>> Nacked-by: Anthony Liguori<aliguori@us.ibm.com>
>
>> If you want to extend QEMU, then send proper patches.  Adding random C files to
>> the build is unacceptable.
>
> What do you mean by proper patches?

If you want to add "custom functionality" to QEMU, send a patch to upstream QEMU.

> The whole point of this is to ease the
> process for the user to add custom guest-to-qemu interactions, so there is no
> "proper implementation" here.

That's too vague as far as I'm concerned.  The only reason I can see to have a 
mechanism like this is 1) to avoid pushing stuff upstream and/or 2) to 
circumvent QEMU licensing by shipping a separate file that the user includes 
themselves.

I see no reason to encourage either of these things.  It also creates an 
inevitable problem of internal API stability.  What happens when the tree 
changes and all of these "custom guest-to-qemu interactions" break?

You may claim that it's an unsupported interface, but what will actually happen 
is that users will start depending on these custom features long after the 
initial developer has stopped caring.

> Blue Swirl already mentioned this can be used for testing qemu itself, while my
> use case is coupled with the trace instrumentation features (I'll probably send
> the first batch today).

In terms of supporting this mechanism, all backend code needs to live in 
upstream QEMU.  That's a hard requirement.  No configure flags to pull in random 
C files and no dlopens.

If you resent the series doing that, it'd be more likely to be merged but since 
I don't think your intention is to push backend code into QEMU, I wonder whether 
it's worth really merging this mechanism at all.

If this is just infrastructure that is only used by private forks, I don't think 
it belongs upstream.

Regards,

Anthony Liguori

>
>
> Lluis
>

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Anthony Liguori writes:
[...]
>>> If you want to extend QEMU, then send proper patches.  Adding random C files to
>>> the build is unacceptable.
>> 
>> What do you mean by proper patches?

> If you want to add "custom functionality" to QEMU, send a patch to upstream QEMU.

See below.


>> The whole point of this is to ease the
>> process for the user to add custom guest-to-qemu interactions, so there is no
>> "proper implementation" here.

> That's too vague as far as I'm concerned.  The only reason I can see to have a
> mechanism like this is 1) to avoid pushing stuff upstream and/or 2) to
> circumvent QEMU licensing by shipping a separate file that the user includes
> themselves.

AFAIK, GPL licensing restrictions only apply when the code is shipped in a
binary blob. If this is the case, the library is statically included in qemu, so
it must still comply with the licensing requirements of qemu. Still, take this
with a grain of salt as I'm not a licensing expert.

My motivation is more like 1 (I'll release the analyzer library I'm developing
once it's finished), but for the reasons stated below.


> I see no reason to encourage either of these things.  It also creates an
> inevitable problem of internal API stability.  What happens when the tree
> changes and all of these "custom guest-to-qemu interactions" break?

> You may claim that it's an unsupported interface, but what will actually happen
> is that users will start depending on these custom features long after the
> initial developer has stopped caring.

>> Blue Swirl already mentioned this can be used for testing qemu itself, while my
>> use case is coupled with the trace instrumentation features (I'll probably send
>> the first batch today).

> In terms of supporting this mechanism, all backend code needs to live in
> upstream QEMU.  That's a hard requirement.  No configure flags to pull in random
> C files and no dlopens.

> If you resent the series doing that, it'd be more likely to be merged but since
> I don't think your intention is to push backend code into QEMU, I wonder whether
> it's worth really merging this mechanism at all.

> If this is just infrastructure that is only used by private forks, I don't think
> it belongs upstream.

Well, both backdoor and trace instrumentation are implemented using the same
approach (a static library selected at compile-time). The user sets which events
to instrument in the "trace-events" file. This has the effect that the tracetool
script will not generate the tracing functions for those events, and instead the
user must provide the implementation for these events on a static library
provided at compile time.

The analyses one might want to perform are ad-hoc in nature. For example, some
recent work sent here did coverage analyses through ad-hoc modifications on the
core of qemu, I want to generate detailed traces of the guest code that is
executed (although only when certain conditions in the guest are met). There is
no silver bullet here, but the development of analyzers can be simplified by
letting users perform static instrumentation of the tracing events, which act as
regular tracing events otherwise.

In this environment, the backdoor is just a handy channel to let the guest
communicate with code on that analyzer library, like signalling semanticful
events of interest (e.g., I have a trivial linux kernel module that installs a
tracepoint callback in the process switch routine and passes that information
down to the analyzer library through the backdoor channel).

As these analyzes are ad-hoc in nature, I don't see any way other than providing
this extension functionality.

Does it make sense as a whole? I thought from previous mails that being able to
trace and analyze guest code was something perceived as a useful feature to have
in qemu.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Anthony Liguori]

On 12/07/2011 09:23 AM, Lluís Vilanova wrote:
> Anthony Liguori writes:
> [...]
>>>> If you want to extend QEMU, then send proper patches.  Adding random C files to
>>>> the build is unacceptable.
>>>
>>> What do you mean by proper patches?
>
>> If you want to add "custom functionality" to QEMU, send a patch to upstream QEMU.
>
> See below.
>
>
>>> The whole point of this is to ease the
>>> process for the user to add custom guest-to-qemu interactions, so there is no
>>> "proper implementation" here.
>
>> That's too vague as far as I'm concerned.  The only reason I can see to have a
>> mechanism like this is 1) to avoid pushing stuff upstream and/or 2) to
>> circumvent QEMU licensing by shipping a separate file that the user includes
>> themselves.
>
> AFAIK, GPL licensing restrictions only apply when the code is shipped in a
> binary blob. If this is the case, the library is statically included in qemu, so
> it must still comply with the licensing requirements of qemu. Still, take this
> with a grain of salt as I'm not a licensing expert.
>
> My motivation is more like 1 (I'll release the analyzer library I'm developing
> once it's finished), but for the reasons stated below.
>
>
>> I see no reason to encourage either of these things.  It also creates an
>> inevitable problem of internal API stability.  What happens when the tree
>> changes and all of these "custom guest-to-qemu interactions" break?
>
>> You may claim that it's an unsupported interface, but what will actually happen
>> is that users will start depending on these custom features long after the
>> initial developer has stopped caring.
>
>>> Blue Swirl already mentioned this can be used for testing qemu itself, while my
>>> use case is coupled with the trace instrumentation features (I'll probably send
>>> the first batch today).
>
>> In terms of supporting this mechanism, all backend code needs to live in
>> upstream QEMU.  That's a hard requirement.  No configure flags to pull in random
>> C files and no dlopens.
>
>> If you resent the series doing that, it'd be more likely to be merged but since
>> I don't think your intention is to push backend code into QEMU, I wonder whether
>> it's worth really merging this mechanism at all.
>
>> If this is just infrastructure that is only used by private forks, I don't think
>> it belongs upstream.
>
> Well, both backdoor and trace instrumentation are implemented using the same
> approach (a static library selected at compile-time). The user sets which events
> to instrument in the "trace-events" file. This has the effect that the tracetool
> script will not generate the tracing functions for those events, and instead the
> user must provide the implementation for these events on a static library
> provided at compile time.

I don't think this is the right approach to tracing.  What not just use 
something like SystemTap to implement logic around tracing?

http://blog.vmsplice.net/2011/03/how-to-write-trace-analysis-scripts-for.html

> The analyses one might want to perform are ad-hoc in nature. For example, some
> recent work sent here did coverage analyses through ad-hoc modifications on the
> core of qemu, I want to generate detailed traces of the guest code that is
> executed (although only when certain conditions in the guest are met). There is
> no silver bullet here, but the development of analyzers can be simplified by
> letting users perform static instrumentation of the tracing events, which act as
> regular tracing events otherwise.
>
> In this environment, the backdoor is just a handy channel to let the guest
> communicate with code on that analyzer library, like signalling semanticful
> events of interest (e.g., I have a trivial linux kernel module that installs a
> tracepoint callback in the process switch routine and passes that information
> down to the analyzer library through the backdoor channel).
>
> As these analyzes are ad-hoc in nature, I don't see any way other than providing
> this extension functionality.

But the effect is that the infrastructure is useless without out-of-tree code 
which means it's impossible to test.  Furthermore, when the ad-hoc analyzer 
library breaks because of an API change in QEMU, it puts us in a very difficult 
situation.

>
> Does it make sense as a whole? I thought from previous mails that being able to
> trace and analyze guest code was something perceived as a useful feature to have
> in qemu.

I think there's wide agreement that extending tracing to guests is very useful 
and something we should do.

I don't think there's agreement that we should have a mechanism to pull in 
third-party code to implement the logic for these trace hooks within QEMU.

The right way to do analysis is via something like SystemTap.

Regards,

Anthony Liguori

>
>
> Lluis
>

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Anthony Liguori writes:
>> Well, both backdoor and trace instrumentation are implemented using the same
>> approach (a static library selected at compile-time). The user sets which events
>> to instrument in the "trace-events" file. This has the effect that the tracetool
>> script will not generate the tracing functions for those events, and instead the
>> user must provide the implementation for these events on a static library
>> provided at compile time.

> I don't think this is the right approach to tracing.  What not just use
> something like SystemTap to implement logic around tracing?

> http://blog.vmsplice.net/2011/03/how-to-write-trace-analysis-scripts-for.html

This would only allow me to post-process all the traces, with no possible
interaction with qemy from the analysis library.

Compiling the analysis library into qemu itself lets me control when TCG code to
actually trace an instruction is generated (with guest code, trace events are
instrumented at both code translation and code execution time):

translate:
    gen_helper_trace_mem(...)
      -- trace instrumentation -->  ... if (...) gen_helper_trace_mem_backend(...); ...
      -- otherwise             -->  gen_helper_trace_mem_backend(...);

execute:
    helper_trace_mem(...)
      trace_mem(...)
        -- trace instrumentation --> ... whatever ...
        -- otherwise             --> trace_fetch_backend(...);

One could argue that gen_helper_trace_mem can just check some in-qemu central
structure to see if it must generate the call to helper_trace_mem, but then the
best you can do is an all-or-nothing on a per-event basis.

You must have in mind that we're talking about an extreme tracing bandwidth
here, so the analyzer is willing to filter-out as many events as soon as
possible (e.g., just tracing a specific set of IPs can be optimized at
translation time, and tracing instructions using specific registers can be
optimized by generating extra checks with TCG). This requires some sort of
in-line communication between qemu and the analyzer.

Still, I tried to build this into perfectly separate and independent features:

  * Support for tracing guest code through TCG.
  * Support for different sets of enabled guest tracing events on a per-vCPU
    basis.
  * Support for instrumenting tracing events (including those for guest code).


>> The analyses one might want to perform are ad-hoc in nature. For example, some
>> recent work sent here did coverage analyses through ad-hoc modifications on the
>> core of qemu, I want to generate detailed traces of the guest code that is
>> executed (although only when certain conditions in the guest are met). There is
>> no silver bullet here, but the development of analyzers can be simplified by
>> letting users perform static instrumentation of the tracing events, which act as
>> regular tracing events otherwise.
>> 
>> In this environment, the backdoor is just a handy channel to let the guest
>> communicate with code on that analyzer library, like signalling semanticful
>> events of interest (e.g., I have a trivial linux kernel module that installs a
>> tracepoint callback in the process switch routine and passes that information
>> down to the analyzer library through the backdoor channel).
>> 
>> As these analyzes are ad-hoc in nature, I don't see any way other than providing
>> this extension functionality.

> But the effect is that the infrastructure is useless without out-of-tree code
> which means it's impossible to test.  Furthermore, when the ad-hoc analyzer
> library breaks because of an API change in QEMU, it puts us in a very difficult
> situation.

The infrastructure itself is pretty simple (the least trivial part is code in
tracetool), and can be tested by having a testing library that can be shipped
with qemu, as well as can be used to test qemu itself.

I also think that the point of breaking third-party analyzers is less important
if the access to TCG code generation and the tracing events are enough for most
of them (the TCG API is pretty stable). Some analyzers might want to access
register values or something else, but they would still do that even without
instrumentation through ad-hoc changes in a rapidly aging version of qemu. My
only intent is to ease the effort and the performance costs of developing such
analyzers.


>> Does it make sense as a whole? I thought from previous mails that being able to
>> trace and analyze guest code was something perceived as a useful feature to have
>> in qemu.

> I think there's wide agreement that extending tracing to guests is very useful
> and something we should do.

> I don't think there's agreement that we should have a mechanism to pull in
> third-party code to implement the logic for these trace hooks within QEMU.

> The right way to do analysis is via something like SystemTap.

Well, as I tried to explain above, this comes at the expense of performance, as
the analyzer is not inlined into qemu.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Anthony Liguori]

On 12/07/2011 10:59 AM, Lluís Vilanova wrote:
> Anthony Liguori writes:
>>> Well, both backdoor and trace instrumentation are implemented using the same
>>> approach (a static library selected at compile-time). The user sets which events
>>> to instrument in the "trace-events" file. This has the effect that the tracetool
>>> script will not generate the tracing functions for those events, and instead the
>>> user must provide the implementation for these events on a static library
>>> provided at compile time.
>
>> I don't think this is the right approach to tracing.  What not just use
>> something like SystemTap to implement logic around tracing?
>
>> http://blog.vmsplice.net/2011/03/how-to-write-trace-analysis-scripts-for.html
>
> This would only allow me to post-process all the traces, with no possible
> interaction with qemy from the analysis library.
>
> Compiling the analysis library into qemu itself lets me control when TCG code to
> actually trace an instruction is generated (with guest code, trace events are
> instrumented at both code translation and code execution time):
>
> translate:
>      gen_helper_trace_mem(...)
>        -- trace instrumentation -->   ... if (...) gen_helper_trace_mem_backend(...); ...
>        -- otherwise             -->   gen_helper_trace_mem_backend(...);
>
> execute:
>      helper_trace_mem(...)
>        trace_mem(...)
>          -- trace instrumentation -->  ... whatever ...
>          -- otherwise             -->  trace_fetch_backend(...);
>
> One could argue that gen_helper_trace_mem can just check some in-qemu central
> structure to see if it must generate the call to helper_trace_mem, but then the
> best you can do is an all-or-nothing on a per-event basis.
>
> You must have in mind that we're talking about an extreme tracing bandwidth
> here, so the analyzer is willing to filter-out as many events as soon as
> possible (e.g., just tracing a specific set of IPs can be optimized at
> translation time, and tracing instructions using specific registers can be
> optimized by generating extra checks with TCG). This requires some sort of
> in-line communication between qemu and the analyzer.

Why should this analyzer live outside of QEMU in the first place?  I fail to see 
the rationale for that other than not wanting to do the work of making it 
suitable for upstream consumption.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Anthony Liguori writes:
[...]
> Why should this analyzer live outside of QEMU in the first place?  I fail to see
> the rationale for that other than not wanting to do the work of making it
> suitable for upstream consumption.

For the same reason that SystemTap lets you add user-provided code into the
trace hooks, you never know what the user actually wants. The difference is that
when dealing with traces that require a high bandwidth, different people may be
interested on analyzing different events and under different conditions, and
having a JIT in QEMU comes in handy to optimize the traces. This includes the
generation of extra tracing code at translation time (e.g., I generate checks in
TCG to establish when I want to trace a specific event, and someone else may
just want to increment a counter using TCG code).

Guest trace instrumentation turns QEMU into a highly-performant tool for dynamic
binary instrumentation, with all the benefits that stem from QEMU (support for a
myriad of target architectures, as well as support for both full-system and
user-level applications).

I can understand if someone thinks this is not a desired feature for QEMU [1],
but not including it into upstream will just turn it into a patch-based feature
that will rapidly fail to apply on top of upstream QEMU.

[1] Of course, I will still contribute support for guest tracing, as well as the
    guest events I added.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Peter Maydell]

2011/12/7 Lluís Vilanova <vilanova@ac.upc.edu>:
> Anthony Liguori writes:
> [...]
>> Why should this analyzer live outside of QEMU in the first place?  I fail to see
>> the rationale for that other than not wanting to do the work of making it
>> suitable for upstream consumption.
>
> For the same reason that SystemTap lets you add user-provided code into the
> trace hooks, you never know what the user actually wants. The difference is that
> when dealing with traces that require a high bandwidth, different people may be
> interested on analyzing different events and under different conditions, and
> having a JIT in QEMU comes in handy to optimize the traces. This includes the
> generation of extra tracing code at translation time (e.g., I generate checks in
> TCG to establish when I want to trace a specific event, and someone else may
> just want to increment a counter using TCG code).
>
> Guest trace instrumentation turns QEMU into a highly-performant tool for dynamic
> binary instrumentation, with all the benefits that stem from QEMU (support for a
> myriad of target architectures, as well as support for both full-system and
> user-level applications).

I think this *is* useful. However I also think that it *is* effectively
defining an API for people writing this hook code, and as such we ought
to do it properly if we're going to do it. (ie nail down what we are
providing for hook authors and don't let them grub around in the internals
otherwise).

-- PMM

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Anthony Liguori]

On 12/07/2011 12:51 PM, Peter Maydell wrote:
> 2011/12/7 Lluís Vilanova<vilanova@ac.upc.edu>:
>> Anthony Liguori writes:
>> [...]
>>> Why should this analyzer live outside of QEMU in the first place?  I fail to see
>>> the rationale for that other than not wanting to do the work of making it
>>> suitable for upstream consumption.
>>
>> For the same reason that SystemTap lets you add user-provided code into the
>> trace hooks, you never know what the user actually wants. The difference is that
>> when dealing with traces that require a high bandwidth, different people may be
>> interested on analyzing different events and under different conditions, and
>> having a JIT in QEMU comes in handy to optimize the traces. This includes the
>> generation of extra tracing code at translation time (e.g., I generate checks in
>> TCG to establish when I want to trace a specific event, and someone else may
>> just want to increment a counter using TCG code).
>>
>> Guest trace instrumentation turns QEMU into a highly-performant tool for dynamic
>> binary instrumentation, with all the benefits that stem from QEMU (support for a
>> myriad of target architectures, as well as support for both full-system and
>> user-level applications).
>
> I think this *is* useful. However I also think that it *is* effectively
> defining an API for people writing this hook code, and as such we ought
> to do it properly if we're going to do it. (ie nail down what we are
> providing for hook authors and don't let them grub around in the internals
> otherwise).

I strongly suspect that you could define interfaces in QEMU such that you could 
do most of what you want without needing to link any code against QEMU itself.

This is probably a case where LUA integration might make a lot of sense.

Regards,

Anthony Liguori

>
> -- PMM
>
>

Re: [Qemu-devel] Insane virtio-serial semantics

[Michael Roth]

On 12/07/2011 07:49 AM, Anthony Liguori wrote:
> On 12/07/2011 02:21 AM, Markus Armbruster wrote:
>> Anthony Liguori<anthony@codemonkey.ws> writes:
>>
>>> On 12/06/2011 04:30 PM, Lluís Vilanova wrote:
>>>> Anthony Liguori writes:
>>>>
>>>>> I really worry about us introducing so many of these one-off
>>>>> paravirtual devices.
>>>>> I would much prefer that you look at doing this as an extension to
>>>>> the ivshmem
>>>>> device as it already has this sort of scope. You should be able to
>>>>> do this by
>>>>> just extending the size of bar 1 and using a well known guest id.
>>>>
>>>> I did in fact look at ivshmem some time ago, and it's true that both
>>>> use the
>>>> same mechanisms; but each device has a completely different purpose.
>>>> To me it
>>>> just seems that extending the control BAR in ivshmem to call the
>>>> user-provided
>>>> backdoor callbacks is just conflating two completely separate
>>>> devices into a
>>>> single one. Besides, I think that the qemu-side of the backdoor is
>>>> simple enough
>>>> to avoid being a maintenance burden.
>>>
>>> They have the same purpose (which are both vague TBH). The only
>>> reason I'm sympathetic to this device is that virtio-serial has such
>>> insane semantics.
>>
>> Could you summarize what's wrong? Is it fixable?
>
> I don't think so as it's part of the userspace ABI now.
>
> Mike, please help me make sure I get this all right. A normal
> file/socket has the following guest semantics:
>
> 1) When a disconnect occurs, you will receive a return of '0' or -EPIPE
> depending on the platform. The fd is now unusable and you must
> close/reopen.
>
> 2) You can setup SIGIO/SIGPIPE to fire off whenever a file descriptor
> becomes readable/writable.
>
> virtio serial has the following semantics:
>
> 1) When a disconnect occurs, if you read() you will receive an -EPIPE.
>
> 2) However, if a reconnect occurs before you issue your read(), the read
> will complete with no indication that a disconnect occurred.
>
> 3) This makes it impossible to determine whether a disconnect has
> occurred which makes it very hard to reset your protocol stream. To deal
> with this, virtio-serial can issue a SIGIO signal upon disconnect.
>
> 4) Signals are asynchronous, so a reconnect may have occurred by the
> time you get the SIGIO signal. It's unclear that you can do anything
> useful with this.

That about sums it up. There was a thread about this a while back where 
there was some tentative agreement on a way to fix this by introducing 
QEMU flags that invoke similar semantics to unix sockets:

http://thread.gmane.org/gmane.comp.emulators.qemu/94721/focus=95496

But at this point we'd need to re-visit, since there's a fair number of 
virtio-serial users now. It'd probably need to be something you could 
switch on from the guest via an fcntl() or something.

>
> So besides overloading the meaning of SIGIO, there's really no way to
> figure out in the guest when a reconnect has occurred. To deal with this
> in qemu-ga, we actually only allow 7-bit data transfers and use the 8th
> bit as an in-band message to tell the guest that a reset has occurred.

Yup, it's not perfect though, due to a delayed/spurious response from an 
agent that sent data before it read/handled the reset sequence. We don't 
get that problem with unix sockets since they'd get an -EPIPE and would 
be blocked from sending to a newly opened session.

We try to account for this on the host by following up a reset sequences 
will the guest-sync RPC, which contains a unique ID that the guest echos 
back to us. That way we can throw away stale data on the host until we 
get the intended response. In our case, it's not quite perfect since if 
the agent sent a "{" before getting reset, subsequent transmission of 
the guest-sync response can be lost. We'd need to precede responses to 
guest-sync with a 0xFF as well, so that the host flushes it's rcv 
buffer/parser state...

And, somewhat off-topic, but none of addresses the case where an agent 
hangs on an RPC. This would require some additional handling by the 
agent side where we might have tie some additional action to the 0xFF 
sequence.

Previously this scenario was handled by a hard-coded timeout mechanism 
in the agent, with a seperate thread handling the RPCs, but we've since 
dropped the thread due to potential for memory leaks (with plans to 
re-introduce using a child process).

client-induced resets would be much nicer though, and a reserved byte is 
the best solution we've been able to come up with given the current 
virtio-serial semantics.

>
> Regards,
>
> Anthony Liguori
>
>>
>> [...]
>>
>

Re: [Qemu-devel] Insane virtio-serial semantics

[Anthony Liguori]

On 12/07/2011 01:44 PM, Michael Roth wrote:
> On 12/07/2011 07:49 AM, Anthony Liguori wrote:
>> On 12/07/2011 02:21 AM, Markus Armbruster wrote:
>>> Anthony Liguori<anthony@codemonkey.ws> writes:
>>>
>>>> On 12/06/2011 04:30 PM, Lluís Vilanova wrote:
>>>>> Anthony Liguori writes:
>>>>>
>>>>>> I really worry about us introducing so many of these one-off
>>>>>> paravirtual devices.
>>>>>> I would much prefer that you look at doing this as an extension to
>>>>>> the ivshmem
>>>>>> device as it already has this sort of scope. You should be able to
>>>>>> do this by
>>>>>> just extending the size of bar 1 and using a well known guest id.
>>>>>
>>>>> I did in fact look at ivshmem some time ago, and it's true that both
>>>>> use the
>>>>> same mechanisms; but each device has a completely different purpose.
>>>>> To me it
>>>>> just seems that extending the control BAR in ivshmem to call the
>>>>> user-provided
>>>>> backdoor callbacks is just conflating two completely separate
>>>>> devices into a
>>>>> single one. Besides, I think that the qemu-side of the backdoor is
>>>>> simple enough
>>>>> to avoid being a maintenance burden.
>>>>
>>>> They have the same purpose (which are both vague TBH). The only
>>>> reason I'm sympathetic to this device is that virtio-serial has such
>>>> insane semantics.
>>>
>>> Could you summarize what's wrong? Is it fixable?
>>
>> I don't think so as it's part of the userspace ABI now.
>>
>> Mike, please help me make sure I get this all right. A normal
>> file/socket has the following guest semantics:
>>
>> 1) When a disconnect occurs, you will receive a return of '0' or -EPIPE
>> depending on the platform. The fd is now unusable and you must
>> close/reopen.
>>
>> 2) You can setup SIGIO/SIGPIPE to fire off whenever a file descriptor
>> becomes readable/writable.
>>
>> virtio serial has the following semantics:
>>
>> 1) When a disconnect occurs, if you read() you will receive an -EPIPE.
>>
>> 2) However, if a reconnect occurs before you issue your read(), the read
>> will complete with no indication that a disconnect occurred.
>>
>> 3) This makes it impossible to determine whether a disconnect has
>> occurred which makes it very hard to reset your protocol stream. To deal
>> with this, virtio-serial can issue a SIGIO signal upon disconnect.
>>
>> 4) Signals are asynchronous, so a reconnect may have occurred by the
>> time you get the SIGIO signal. It's unclear that you can do anything
>> useful with this.
>
> That about sums it up. There was a thread about this a while back where there
> was some tentative agreement on a way to fix this by introducing QEMU flags that
> invoke similar semantics to unix sockets:
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/94721/focus=95496
>
> But at this point we'd need to re-visit, since there's a fair number of
> virtio-serial users now. It'd probably need to be something you could switch on
> from the guest via an fcntl() or something.
>
>>
>> So besides overloading the meaning of SIGIO, there's really no way to
>> figure out in the guest when a reconnect has occurred. To deal with this
>> in qemu-ga, we actually only allow 7-bit data transfers and use the 8th
>> bit as an in-band message to tell the guest that a reset has occurred.
>
> Yup, it's not perfect though, due to a delayed/spurious response from an agent
> that sent data before it read/handled the reset sequence. We don't get that
> problem with unix sockets since they'd get an -EPIPE and would be blocked from
> sending to a newly opened session.
>
> We try to account for this on the host by following up a reset sequences will
> the guest-sync RPC, which contains a unique ID that the guest echos back to us.
> That way we can throw away stale data on the host until we get the intended
> response. In our case, it's not quite perfect since if the agent sent a "{"
> before getting reset, subsequent transmission of the guest-sync response can be
> lost. We'd need to precede responses to guest-sync with a 0xFF as well, so that
> the host flushes it's rcv buffer/parser state...
>
> And, somewhat off-topic, but none of addresses the case where an agent hangs on
> an RPC. This would require some additional handling by the agent side where we
> might have tie some additional action to the 0xFF sequence.
>
> Previously this scenario was handled by a hard-coded timeout mechanism in the
> agent, with a seperate thread handling the RPCs, but we've since dropped the
> thread due to potential for memory leaks (with plans to re-introduce using a
> child process).
>
> client-induced resets would be much nicer though, and a reserved byte is the
> best solution we've been able to come up with given the current virtio-serial
> semantics.

Yeah, we really need a "sane reset semantics" flag for virtio-serial that 
provides a guest and host initiated channel close mechanism.

I think you need to do this by using a single ring and using a simple session id 
with an explicit open/close message.  That way there is never ambiguity.

And yes, I can't help but think of Dave Millers comments long ago that any PV 
transport is eventually going to reinvent TCP, poorly.

Regards,

Anthony Liguori

>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> [...]
>>>
>>
>
>

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Anthony Liguori writes:

> On 12/07/2011 12:51 PM, Peter Maydell wrote:
>> 2011/12/7 Lluís Vilanova<vilanova@ac.upc.edu>:
>>> Anthony Liguori writes:
>>> [...]
>>>> Why should this analyzer live outside of QEMU in the first place?  I fail to see
>>>> the rationale for that other than not wanting to do the work of making it
>>>> suitable for upstream consumption.
>>> 
>>> For the same reason that SystemTap lets you add user-provided code into the
>>> trace hooks, you never know what the user actually wants. The difference is that
>>> when dealing with traces that require a high bandwidth, different people may be
>>> interested on analyzing different events and under different conditions, and
>>> having a JIT in QEMU comes in handy to optimize the traces. This includes the
>>> generation of extra tracing code at translation time (e.g., I generate checks in
>>> TCG to establish when I want to trace a specific event, and someone else may
>>> just want to increment a counter using TCG code).
>>> 
>>> Guest trace instrumentation turns QEMU into a highly-performant tool for dynamic
>>> binary instrumentation, with all the benefits that stem from QEMU (support for a
>>> myriad of target architectures, as well as support for both full-system and
>>> user-level applications).
>> 
>> I think this *is* useful. However I also think that it *is* effectively
>> defining an API for people writing this hook code, and as such we ought
>> to do it properly if we're going to do it. (ie nail down what we are
>> providing for hook authors and don't let them grub around in the internals
>> otherwise).

> I strongly suspect that you could define interfaces in QEMU such that you could
> do most of what you want without needing to link any code against QEMU itself.

> This is probably a case where LUA integration might make a lot of sense.

Sure you can provide an API for instrumentors. It's just that defining a stable
API for that would require me at least an order of magnitude more effort, and
this is time I don't have right now.

The other problem here is that of performance.

During translation, it is basically dependant on TB reusability, plus the
possibility to optimize the analyzer code when it's inlined into QEMU and some
of its arguments are known at compile-time. I just don't have numbers on that.

During execution it is way more sensitive, thus my approach to build the
analyzer code into QEMU. Otherwise you would go through a LUA interpreter (or
anything else) or, in the best case, a conditional indirect call
(dlopen).

>From my experience, these are the basics I need:

* Decide what to do when an event is translated (by default - no instrumentation
  -, it just generates a TCG call to an execution-time tracing routine).

  This includes:

  * Deciding whether to generate the TCG call to the execution-time tracing
    routine.

  * Access to generating other arbitrary TCG code (the API is pretty stable, but
    headers depend on target-specific defines).

    This could also include generating code like incrementing a counter (e.g.,
    when counting instructions), instead of calling the execution-time tracing
    routine.

    My library actually hides this from the user, who just provides conditions
    to establish whether to trace the event. Then the library automatically
    establishes whether it's best to evaluate each condition at translation
    time, using TCG code or when the execution-time tracing routine is invoked.

  * [still don't have it] Access to a map between target-specific abstractions
    (mainly register names) and TCG values (e.g., to generate code that gets or
    sets the value of a well-known register).

* Decide what to do when an event is executed, including calls to execution-time
  event tracing routines generated by TCG code (by default, it just calls the
  tracing backend).

  This includes:

  * Arbitrary user-provided code (the analysis itself when not inlined with
    TCG).

  * [still don't have it] Access to get and set the values of target-specific
    abstractions (mainly registers).

* Add a per-vCPU opaque pointer with data private to the analyzer code.


Other things might include the injection of interrupts or anything else related
to the guest.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Lluís Vilanova writes:
[...]
> From my experience, these are the basics I need:

> * Decide what to do when an event is translated (by default - no instrumentation
>   -, it just generates a TCG call to an execution-time tracing routine).

>   This includes:

>   * Deciding whether to generate the TCG call to the execution-time tracing
>     routine.

>   * Access to generating other arbitrary TCG code (the API is pretty stable, but
>     headers depend on target-specific defines).

>     This could also include generating code like incrementing a counter (e.g.,
>     when counting instructions), instead of calling the execution-time tracing
>     routine.

>     My library actually hides this from the user, who just provides conditions
>     to establish whether to trace the event. Then the library automatically
>     establishes whether it's best to evaluate each condition at translation
>     time, using TCG code or when the execution-time tracing routine is invoked.

>   * [still don't have it] Access to a map between target-specific abstractions
>     (mainly register names) and TCG values (e.g., to generate code that gets or
>     sets the value of a well-known register).

> * Decide what to do when an event is executed, including calls to execution-time
>   event tracing routines generated by TCG code (by default, it just calls the
>   tracing backend).

>   This includes:

>   * Arbitrary user-provided code (the analysis itself when not inlined with
>     TCG).

>   * [still don't have it] Access to get and set the values of target-specific
>     abstractions (mainly registers).

> * Add a per-vCPU opaque pointer with data private to the analyzer code.

I forgot to add that another nice addition is to let the translation-time
user-code specify which global registers might be accessed in the execution-time
user code (thus the translation can generate TCG_CALL_CONST by default).

Right now, it is established at compile time by having two event properties; one
with and one without TCG_CALL_CONST-enabled TCG helpers.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] Insane virtio-serial semantics

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 12/07/2011 01:44 PM, Michael Roth wrote:
>> On 12/07/2011 07:49 AM, Anthony Liguori wrote:
>>> On 12/07/2011 02:21 AM, Markus Armbruster wrote:
>>>> Anthony Liguori<anthony@codemonkey.ws> writes:
[...]
>>>>> They have the same purpose (which are both vague TBH). The only
>>>>> reason I'm sympathetic to this device is that virtio-serial has such
>>>>> insane semantics.
>>>>
>>>> Could you summarize what's wrong? Is it fixable?
>>>
>>> I don't think so as it's part of the userspace ABI now.
>>>
>>> Mike, please help me make sure I get this all right. A normal
>>> file/socket has the following guest semantics:
>>>
>>> 1) When a disconnect occurs, you will receive a return of '0' or -EPIPE
>>> depending on the platform. The fd is now unusable and you must
>>> close/reopen.
>>>
>>> 2) You can setup SIGIO/SIGPIPE to fire off whenever a file descriptor
>>> becomes readable/writable.
>>>
>>> virtio serial has the following semantics:
>>>
>>> 1) When a disconnect occurs, if you read() you will receive an -EPIPE.
>>>
>>> 2) However, if a reconnect occurs before you issue your read(), the read
>>> will complete with no indication that a disconnect occurred.
>>>
>>> 3) This makes it impossible to determine whether a disconnect has
>>> occurred which makes it very hard to reset your protocol stream. To deal
>>> with this, virtio-serial can issue a SIGIO signal upon disconnect.
>>>
>>> 4) Signals are asynchronous, so a reconnect may have occurred by the
>>> time you get the SIGIO signal. It's unclear that you can do anything
>>> useful with this.
>>
>> That about sums it up. There was a thread about this a while back where there
>> was some tentative agreement on a way to fix this by introducing QEMU flags that
>> invoke similar semantics to unix sockets:
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/94721/focus=95496
>>
>> But at this point we'd need to re-visit, since there's a fair number of
>> virtio-serial users now. It'd probably need to be something you could switch on
>> from the guest via an fcntl() or something.
>>
>>>
>>> So besides overloading the meaning of SIGIO, there's really no way to
>>> figure out in the guest when a reconnect has occurred. To deal with this
>>> in qemu-ga, we actually only allow 7-bit data transfers and use the 8th
>>> bit as an in-band message to tell the guest that a reset has occurred.
>>
>> Yup, it's not perfect though, due to a delayed/spurious response from an agent
>> that sent data before it read/handled the reset sequence. We don't get that
>> problem with unix sockets since they'd get an -EPIPE and would be blocked from
>> sending to a newly opened session.
>>
>> We try to account for this on the host by following up a reset sequences will
>> the guest-sync RPC, which contains a unique ID that the guest echos back to us.
>> That way we can throw away stale data on the host until we get the intended
>> response. In our case, it's not quite perfect since if the agent sent a "{"
>> before getting reset, subsequent transmission of the guest-sync response can be
>> lost. We'd need to precede responses to guest-sync with a 0xFF as well, so that
>> the host flushes it's rcv buffer/parser state...
>>
>> And, somewhat off-topic, but none of addresses the case where an agent hangs on
>> an RPC. This would require some additional handling by the agent side where we
>> might have tie some additional action to the 0xFF sequence.
>>
>> Previously this scenario was handled by a hard-coded timeout mechanism in the
>> agent, with a seperate thread handling the RPCs, but we've since dropped the
>> thread due to potential for memory leaks (with plans to re-introduce using a
>> child process).
>>
>> client-induced resets would be much nicer though, and a reserved byte is the
>> best solution we've been able to come up with given the current virtio-serial
>> semantics.
>
> Yeah, we really need a "sane reset semantics" flag for virtio-serial
> that provides a guest and host initiated channel close mechanism.
>
> I think you need to do this by using a single ring and using a simple
> session id with an explicit open/close message.  That way there is
> never ambiguity.

So it is fixable.

> And yes, I can't help but think of Dave Millers comments long ago that
> any PV transport is eventually going to reinvent TCP, poorly.

No doubt then, no doubt now.  But if I remember correctly, we didn't
create virtio-serial because we thought we could do better than TCP/IP.
We thought we need a zero-config communication channel that doesn't
interfere in any way with the guest's networking.  Since the network
folks were unwilling to give us one ("use TCP already"), we looked for
another bare metal thing to imitate, and chose serial lines.

Now, comparing serial lines to TCP/IP makes no sense.  Different layers.

Layering a real network protocol on top of serial line is possible; SLIP
exists.  But as long as we insist on "don't interfere in any way with
the guest's networking", we can't use TCP, and thus are doomed to
reinvent it, poorly.

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Stefan Hajnoczi]

2011/12/5 Lluís Vilanova <vilanova@ac.upc.edu>:
> Provides the ability for the guest to communicate with user-provided code inside
> QEMU itself, using a lightweight mechanism.
>
> See first commit for a full description.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---

First off, thank you for your continued efforts - including the
general tracing improvements you have contributed in the past.  I
think many people use QEMU as a tool for research and simulation, but
few end up working upstream.  So thanks for polishing up your work and
submitting it upstream.

Thoughts on the pieces you have described:

> * Support for tracing guest code through TCG.

I'm not clear on whether a backdoor mechanism is needed or not.  A
backdoor mechanism allows a modified guest to participate in tracing.

Depending on the type of analysis you are doing it might be possible
to achieve the same thing by emitting calls to custom instrumentation
in TCG (or is this too low-level for what you're trying to trace in
the guest?).  The advantage is also that you can do this to unmodified
guests.

> * Support for different sets of enabled guest tracing events on a per-vCPU basis.

This feature is very specific but potentially useful.  I haven't
reviewed patches yet but I can imagine that this would be merged.

> * Support for instrumenting tracing events (including those for guest code).

Although I typically use post-processing rather than filtering, I can
see your point about the power and flexibility in being able to react
on the spot to events inside QEMU.  Given the ad-hoc nature of the
instrumentation, as well as that this is really in the guts of QEMU
and TCG, I think we should make it easy to write instrumentation but
keep the infrastructure and APIs minimal.

Is there even a need to add "instrumentation" support on top of QEMU
tracing?  As an instrumentation implementor you are working with the
QEMU source tree and need to be somewhat familiar with the internals.
Why not just add custom functions into QEMU and call them from
relevant points?

The documentation and tracetool extensions you posted provide some
structure but IMO they don't cut down the workflow significantly over
adding plain old functions in places of interest.  i.e. by the time I
have read the docs and begun trying to add instrumentation I could
have already added a custom function in QEMU and built a binary.  Plus
it's harder to understand/debug instrumentation code if it sits on top
of the tracing instrumentation - yet another layer to understand.

So I'm suggesting that we don't *need* explicit support for
instrumenting trace events.  Instead add plain old functions where you
need them.  You may decide to invoke trace events from your
instrumentation function, but that's already covered today by
docs/tracing.txt.

Am I missing the point of instrumentation tracing events or do you
agree that we can work well without it?

Stefan

Re: [Qemu-devel] Insane virtio-serial semantics

[Anthony Liguori]

On 12/08/2011 04:11 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>> And yes, I can't help but think of Dave Millers comments long ago that
>> any PV transport is eventually going to reinvent TCP, poorly.
>
> No doubt then, no doubt now.  But if I remember correctly, we didn't
> create virtio-serial because we thought we could do better than TCP/IP.
> We thought we need a zero-config communication channel that doesn't
> interfere in any way with the guest's networking.  Since the network
> folks were unwilling to give us one ("use TCP already"), we looked for
> another bare metal thing to imitate, and chose serial lines.
>
> Now, comparing serial lines to TCP/IP makes no sense.  Different layers.

virtio-serial is not a serial line.  It attempts to have connection semantics 
(like a socket) which is what the fundamental problem is.

It probably makes sense to have a virtio-serial2 that is exposed to the guest as 
a tty device and stick strictly to serial semantics.

> Layering a real network protocol on top of serial line is possible; SLIP
> exists.  But as long as we insist on "don't interfere in any way with
> the guest's networking", we can't use TCP, and thus are doomed to
> reinvent it, poorly.

I think we just got too clever.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Stefan Hajnoczi writes:
[...]
>> * Support for tracing guest code through TCG.

> I'm not clear on whether a backdoor mechanism is needed or not.  A
> backdoor mechanism allows a modified guest to participate in tracing.

> Depending on the type of analysis you are doing it might be possible
> to achieve the same thing by emitting calls to custom instrumentation
> in TCG (or is this too low-level for what you're trying to trace in
> the guest?).  The advantage is also that you can do this to unmodified
> guests.

You're right with the first; it exists for two reasons:

* simplicity: it's easier to have, for example, a backdoor + linux tracepoints
  than obtaining the IP of an "interesting" function (think of the
  multiprogramming on a full system)

* performance: it's faster to let the guest tell you rather than (ab)using
  breakpoints or checking the IP of every instruction

The main use of this is to get semanticful events on the guest (my previous
example was about knowing when linux changes the current process, but you could
use anything else as well - e.g., a certain function of a specific thread on the
guest -).


>> * Support for different sets of enabled guest tracing events on a per-vCPU basis.

> This feature is very specific but potentially useful.  I haven't
> reviewed patches yet but I can imagine that this would be merged.

I'll send it after the initial TCG tracing support.


>> * Support for instrumenting tracing events (including those for guest code).

> Although I typically use post-processing rather than filtering, I can
> see your point about the power and flexibility in being able to react
> on the spot to events inside QEMU.  Given the ad-hoc nature of the
> instrumentation, as well as that this is really in the guts of QEMU
> and TCG, I think we should make it easy to write instrumentation but
> keep the infrastructure and APIs minimal.

> Is there even a need to add "instrumentation" support on top of QEMU
> tracing?  As an instrumentation implementor you are working with the
> QEMU source tree and need to be somewhat familiar with the internals.
> Why not just add custom functions into QEMU and call them from
> relevant points?

Well, the point is to just use the existing tracing points in QEMU. Of course, I
could just modify tracetool and add support for a new tracing backend that uses
my library.

The intention was just to make it brain-dead simple and repeatable (no manual
editing of the auto-generated tracing files), as well as able to reuse existing
backends when the user sees fit.


> The documentation and tracetool extensions you posted provide some
> structure but IMO they don't cut down the workflow significantly over
> adding plain old functions in places of interest.  i.e. by the time I
> have read the docs and begun trying to add instrumentation I could
> have already added a custom function in QEMU and built a binary.  Plus
> it's harder to understand/debug instrumentation code if it sits on top
> of the tracing instrumentation - yet another layer to understand.

True. Maybe an unofficial [1] instrumentation tracing backend would be simpler
to use and maintain.

[1] I'm assuming from Anthony's comments that such a thing would not be accepted
    upstream.


> So I'm suggesting that we don't *need* explicit support for
> instrumenting trace events.  Instead add plain old functions where you
> need them.  You may decide to invoke trace events from your
> instrumentation function, but that's already covered today by
> docs/tracing.txt.

> Am I missing the point of instrumentation tracing events or do you
> agree that we can work well without it?

The point is to avoid diving into QEMU's code and instead use the current
out-of-the-box tracing points as the instrumentation hooks, which can be
achieved both through the current approach or an unofficial tracing backend.

The nice thing about the current approach is that the user can do some extra
checks on the tracing event and then (maybe) call the original backend-generated
tracing routine (as a quick and easy way to extend what to actually trace).


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Blue Swirl]

On Wed, Dec 7, 2011 at 18:54, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 12/07/2011 12:51 PM, Peter Maydell wrote:
>>
>> 2011/12/7 Lluís Vilanova<vilanova@ac.upc.edu>:
>>>
>>> Anthony Liguori writes:
>>> [...]
>>>>
>>>> Why should this analyzer live outside of QEMU in the first place?  I
>>>> fail to see
>>>> the rationale for that other than not wanting to do the work of making
>>>> it
>>>> suitable for upstream consumption.
>>>
>>>
>>> For the same reason that SystemTap lets you add user-provided code into
>>> the
>>> trace hooks, you never know what the user actually wants. The difference
>>> is that
>>> when dealing with traces that require a high bandwidth, different people
>>> may be
>>> interested on analyzing different events and under different conditions,
>>> and
>>> having a JIT in QEMU comes in handy to optimize the traces. This includes
>>> the
>>> generation of extra tracing code at translation time (e.g., I generate
>>> checks in
>>> TCG to establish when I want to trace a specific event, and someone else
>>> may
>>> just want to increment a counter using TCG code).
>>>
>>> Guest trace instrumentation turns QEMU into a highly-performant tool for
>>> dynamic
>>> binary instrumentation, with all the benefits that stem from QEMU
>>> (support for a
>>> myriad of target architectures, as well as support for both full-system
>>> and
>>> user-level applications).
>>
>>
>> I think this *is* useful. However I also think that it *is* effectively
>> defining an API for people writing this hook code, and as such we ought
>> to do it properly if we're going to do it. (ie nail down what we are
>> providing for hook authors and don't let them grub around in the internals
>> otherwise).
>
>
> I strongly suspect that you could define interfaces in QEMU such that you
> could do most of what you want without needing to link any code against QEMU
> itself.

I don't see linking as the problem, the instrumenting user who
modifies QEMU needs to follow licensing if the result is ever
distributed.

But the API issue can be a problem. Reusing and extending tracepoints
should help API stability from an instrumenting user point of view and
a rich set of various static and dynamic tracepoint mechanisms should
only be helpful for development of QEMU. But also I agree that the
internals of QEMU shouldn't become an API for any user code which
happens to access them.

> This is probably a case where LUA integration might make a lot of sense.
>
> Regards,
>
> Anthony Liguori
>
>>
>> -- PMM
>>
>>
>

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Blue Swirl]

2011/12/8 Lluís Vilanova <vilanova@ac.upc.edu>:
> Stefan Hajnoczi writes:
> [...]
>>> * Support for tracing guest code through TCG.
>
>> I'm not clear on whether a backdoor mechanism is needed or not.  A
>> backdoor mechanism allows a modified guest to participate in tracing.
>
>> Depending on the type of analysis you are doing it might be possible
>> to achieve the same thing by emitting calls to custom instrumentation
>> in TCG (or is this too low-level for what you're trying to trace in
>> the guest?).  The advantage is also that you can do this to unmodified
>> guests.
>
> You're right with the first; it exists for two reasons:
>
> * simplicity: it's easier to have, for example, a backdoor + linux tracepoints
>  than obtaining the IP of an "interesting" function (think of the
>  multiprogramming on a full system)
>
> * performance: it's faster to let the guest tell you rather than (ab)using
>  breakpoints or checking the IP of every instruction

I still think that a breakpoint based system could be a useful method
in addition to others, simply because it is entirely invisible to the
guest and can then be used with unmodified and non-cooperating guests.
Properly implemented, it should not have much overhead.

> The main use of this is to get semanticful events on the guest (my previous
> example was about knowing when linux changes the current process, but you could
> use anything else as well - e.g., a certain function of a specific thread on the
> guest -).
>
>
>>> * Support for different sets of enabled guest tracing events on a per-vCPU basis.
>
>> This feature is very specific but potentially useful.  I haven't
>> reviewed patches yet but I can imagine that this would be merged.
>
> I'll send it after the initial TCG tracing support.
>
>
>>> * Support for instrumenting tracing events (including those for guest code).
>
>> Although I typically use post-processing rather than filtering, I can
>> see your point about the power and flexibility in being able to react
>> on the spot to events inside QEMU.  Given the ad-hoc nature of the
>> instrumentation, as well as that this is really in the guts of QEMU
>> and TCG, I think we should make it easy to write instrumentation but
>> keep the infrastructure and APIs minimal.
>
>> Is there even a need to add "instrumentation" support on top of QEMU
>> tracing?  As an instrumentation implementor you are working with the
>> QEMU source tree and need to be somewhat familiar with the internals.
>> Why not just add custom functions into QEMU and call them from
>> relevant points?
>
> Well, the point is to just use the existing tracing points in QEMU. Of course, I
> could just modify tracetool and add support for a new tracing backend that uses
> my library.
>
> The intention was just to make it brain-dead simple and repeatable (no manual
> editing of the auto-generated tracing files), as well as able to reuse existing
> backends when the user sees fit.
>
>
>> The documentation and tracetool extensions you posted provide some
>> structure but IMO they don't cut down the workflow significantly over
>> adding plain old functions in places of interest.  i.e. by the time I
>> have read the docs and begun trying to add instrumentation I could
>> have already added a custom function in QEMU and built a binary.  Plus
>> it's harder to understand/debug instrumentation code if it sits on top
>> of the tracing instrumentation - yet another layer to understand.
>
> True. Maybe an unofficial [1] instrumentation tracing backend would be simpler
> to use and maintain.
>
> [1] I'm assuming from Anthony's comments that such a thing would not be accepted
>    upstream.
>
>
>> So I'm suggesting that we don't *need* explicit support for
>> instrumenting trace events.  Instead add plain old functions where you
>> need them.  You may decide to invoke trace events from your
>> instrumentation function, but that's already covered today by
>> docs/tracing.txt.
>
>> Am I missing the point of instrumentation tracing events or do you
>> agree that we can work well without it?
>
> The point is to avoid diving into QEMU's code and instead use the current
> out-of-the-box tracing points as the instrumentation hooks, which can be
> achieved both through the current approach or an unofficial tracing backend.
>
> The nice thing about the current approach is that the user can do some extra
> checks on the tracing event and then (maybe) call the original backend-generated
> tracing routine (as a quick and easy way to extend what to actually trace).

Tracepoints shouldn't also suffer from bit rot so easily.

>
>
> Lluis
>
> --
>  "And it's much the same thing with knowledge, for whenever you learn
>  something new, the whole world becomes that much richer."
>  -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
>  Tollbooth

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Blue Swirl writes:

> 2011/12/8 Lluís Vilanova <vilanova@ac.upc.edu>:
>> Stefan Hajnoczi writes:
>> [...]
>>>> * Support for tracing guest code through TCG.
>> 
>>> I'm not clear on whether a backdoor mechanism is needed or not.  A
>>> backdoor mechanism allows a modified guest to participate in tracing.
>> 
>>> Depending on the type of analysis you are doing it might be possible
>>> to achieve the same thing by emitting calls to custom instrumentation
>>> in TCG (or is this too low-level for what you're trying to trace in
>>> the guest?).  The advantage is also that you can do this to unmodified
>>> guests.
>> 
>> You're right with the first; it exists for two reasons:
>> 
>> * simplicity: it's easier to have, for example, a backdoor + linux tracepoints
>>  than obtaining the IP of an "interesting" function (think of the
>>  multiprogramming on a full system)
>> 
>> * performance: it's faster to let the guest tell you rather than (ab)using
>>  breakpoints or checking the IP of every instruction

> I still think that a breakpoint based system could be a useful method
> in addition to others, simply because it is entirely invisible to the
> guest and can then be used with unmodified and non-cooperating guests.
> Properly implemented, it should not have much overhead.

Yup. My plan was to implement this sometime in the future :)

On the performance side, there's only the degenerated case of a very hot code
page that repeatedly triggers false mmu faults due to the (never reached)
breakpoint.

As for simplicity, I can use the backdoor to tell my analuzer to insert a
breakpoint in the given address, instead of having some complex system to get
symbol information from the outside (specially in softmmy mode).


[...]
>>> So I'm suggesting that we don't *need* explicit support for
>>> instrumenting trace events.  Instead add plain old functions where you
>>> need them.  You may decide to invoke trace events from your
>>> instrumentation function, but that's already covered today by
>>> docs/tracing.txt.
>> 
>>> Am I missing the point of instrumentation tracing events or do you
>>> agree that we can work well without it?
>> 
>> The point is to avoid diving into QEMU's code and instead use the current
>> out-of-the-box tracing points as the instrumentation hooks, which can be
>> achieved both through the current approach or an unofficial tracing backend.
>> 
>> The nice thing about the current approach is that the user can do some extra
>> checks on the tracing event and then (maybe) call the original backend-generated
>> tracing routine (as a quick and easy way to extend what to actually trace).

> Tracepoints shouldn't also suffer from bit rot so easily.

Exactly. That's why I thought that tracepoint instrumentation is not such a bad
option.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Stefan Hajnoczi]

2011/12/8 Lluís Vilanova <vilanova@ac.upc.edu>:
> Stefan Hajnoczi writes:
> [...]
>>> * Support for tracing guest code through TCG.
>
>> I'm not clear on whether a backdoor mechanism is needed or not.  A
>> backdoor mechanism allows a modified guest to participate in tracing.
>
>> Depending on the type of analysis you are doing it might be possible
>> to achieve the same thing by emitting calls to custom instrumentation
>> in TCG (or is this too low-level for what you're trying to trace in
>> the guest?).  The advantage is also that you can do this to unmodified
>> guests.
>
> You're right with the first; it exists for two reasons:
>
> * simplicity: it's easier to have, for example, a backdoor + linux tracepoints
>  than obtaining the IP of an "interesting" function (think of the
>  multiprogramming on a full system)
>
> * performance: it's faster to let the guest tell you rather than (ab)using
>  breakpoints or checking the IP of every instruction
>
> The main use of this is to get semanticful events on the guest (my previous
> example was about knowing when linux changes the current process, but you could
> use anything else as well - e.g., a certain function of a specific thread on the
> guest -).

I misunderstood the layer you were tracing.  A backdoor into the guest
indeed makes high-level events easy.

My thoughts were more along the lines of TCG planting calls to custom
functions when it translates an instruction of interest, which is much
lower level and would be more appropriate for code execution
statistics (branches, counters, etc).

>> So I'm suggesting that we don't *need* explicit support for
>> instrumenting trace events.  Instead add plain old functions where you
>> need them.  You may decide to invoke trace events from your
>> instrumentation function, but that's already covered today by
>> docs/tracing.txt.
>
>> Am I missing the point of instrumentation tracing events or do you
>> agree that we can work well without it?
>
> The point is to avoid diving into QEMU's code and instead use the current
> out-of-the-box tracing points as the instrumentation hooks, which can be
> achieved both through the current approach or an unofficial tracing backend.
>
> The nice thing about the current approach is that the user can do some extra
> checks on the tracing event and then (maybe) call the original backend-generated
> tracing routine (as a quick and easy way to extend what to actually trace).

The workflow with an instrumentation layer and trace events as API:
1. Look at available trace events.
2. Write function against trace event interface without using QEMU internals.
3. If you want tracing output make sure to invoke trace_*_backend()
after any processing/filtering.

The workflow in my mind is:
1. Look at QEMU source for a place to insert instrumentation.
2. Write a plain old function (nothing to do with QEMU tracing) to
perform your instrumentation.
3. If you want tracing output make sure to invoke trace_*() after any
processing/filtering.

Although I now understand the purpose of the instrumentation layer
better - it insulates from QEMU's guts - I'm not really of fan of this
approach because I don't see trace events as an API.  They are not
documented and require looking at call sites in QEMU source code to
understand their meaning.  My argument is that once you need to look
inside QEMU to understand how to instrument there is no benefit from
having an instrumentation layer.

We have no API on which to build the instrumentation interface.  Can
you give details about the APIs you'd like and how you use them (e.g.
mem_read()/mem_write() for instrumenting guest memory accesses)?

Stefan

Re: [Qemu-devel] [PATCH v2 0/5] backdoor: lightweight guest-to-QEMU backdoor channel

[Lluís Vilanova]

Stefan Hajnoczi writes:

> 2011/12/8 Lluís Vilanova <vilanova@ac.upc.edu>:
>> Stefan Hajnoczi writes:
>> [...]
>>>> * Support for tracing guest code through TCG.
>> 
>>> I'm not clear on whether a backdoor mechanism is needed or not.  A
>>> backdoor mechanism allows a modified guest to participate in tracing.
>> 
>>> Depending on the type of analysis you are doing it might be possible
>>> to achieve the same thing by emitting calls to custom instrumentation
>>> in TCG (or is this too low-level for what you're trying to trace in
>>> the guest?).  The advantage is also that you can do this to unmodified
>>> guests.
>> 
>> You're right with the first; it exists for two reasons:
>> 
>> * simplicity: it's easier to have, for example, a backdoor + linux tracepoints
>>  than obtaining the IP of an "interesting" function (think of the
>>  multiprogramming on a full system)
>> 
>> * performance: it's faster to let the guest tell you rather than (ab)using
>>  breakpoints or checking the IP of every instruction
>> 
>> The main use of this is to get semanticful events on the guest (my previous
>> example was about knowing when linux changes the current process, but you could
>> use anything else as well - e.g., a certain function of a specific thread on the
>> guest -).

> I misunderstood the layer you were tracing.  A backdoor into the guest
> indeed makes high-level events easy.

> My thoughts were more along the lines of TCG planting calls to custom
> functions when it translates an instruction of interest, which is much
> lower level and would be more appropriate for code execution
> statistics (branches, counters, etc).

Right. That's where instrumentation would come in handy.


>>> So I'm suggesting that we don't *need* explicit support for
>>> instrumenting trace events.  Instead add plain old functions where you
>>> need them.  You may decide to invoke trace events from your
>>> instrumentation function, but that's already covered today by
>>> docs/tracing.txt.
>> 
>>> Am I missing the point of instrumentation tracing events or do you
>>> agree that we can work well without it?
>> 
>> The point is to avoid diving into QEMU's code and instead use the current
>> out-of-the-box tracing points as the instrumentation hooks, which can be
>> achieved both through the current approach or an unofficial tracing backend.
>> 
>> The nice thing about the current approach is that the user can do some extra
>> checks on the tracing event and then (maybe) call the original backend-generated
>> tracing routine (as a quick and easy way to extend what to actually trace).

> The workflow with an instrumentation layer and trace events as API:
> 1. Look at available trace events.
> 2. Write function against trace event interface without using QEMU internals.
> 3. If you want tracing output make sure to invoke trace_*_backend()
> after any processing/filtering.

> The workflow in my mind is:
> 1. Look at QEMU source for a place to insert instrumentation.
> 2. Write a plain old function (nothing to do with QEMU tracing) to
> perform your instrumentation.
> 3. If you want tracing output make sure to invoke trace_*() after any
> processing/filtering.

> Although I now understand the purpose of the instrumentation layer
> better - it insulates from QEMU's guts - I'm not really of fan of this
> approach because I don't see trace events as an API.  They are not
> documented and require looking at call sites in QEMU source code to
> understand their meaning.  My argument is that once you need to look
> inside QEMU to understand how to instrument there is no benefit from
> having an instrumentation layer.

> We have no API on which to build the instrumentation interface.  Can
> you give details about the APIs you'd like and how you use them (e.g.
> mem_read()/mem_write() for instrumenting guest memory accesses)?

I agree that tracing events in general are not an API, but you can add some
sufficiently generic events that can be used as such.

The main idea is to have some well-documented and target-agnostic events that
can be easily instrumented. As an example of this, you can have a look at a
patch series I just sent with some simple TCG-aware events:

 * vbbl
   Virtual address of a basic block that is going to be executed (QEMU TB).

 * vfetch
   Virtual address of an instruction that is going to be executed.

 * vmem
   Virtual address, size and mode of a memory access that is going to be
   performed.

Some other events that are half-cooked on my branch:

 * plvl
   Privilege-level change (x86-only).

 * aspace
   Address-space (page table) change (x86-only).

 * intr
   SW/HW interrupt or exception.

 * vfetch_info
   Extended instruction information (instruction size and set of used/defined
   architectural registers).

Other info that might be useful to add:

 * pbbl, pfetch, pmem (to avoid adding all the information on a single
   heavy-weight event)
   Physical address information. I already had a look at it,
   and my thoughts were to add the guest physical address info in the TLB
   entries if there's an enabled event requiring it.

 * vfetch_info
   Instruction opcode category (e.g., floating point, branch, alu, etc.). Hard
   to get on some architectures like x86.

 * cflow
   Control-flow instruction and its type (conditional, indirect, taken, etc.)
   and target address (probably hard to get it right, like opcodes)

 * *_end (or similar)
   The after-the-action counterpart of most of the other events (e.g.,
   bbl_end). There could be problems to get it right when there's a branch in
   the guest; besides, the next bbl/fetch/intr event would already signal the
   end/error of the previous event.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth