All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel
@ 2016-12-26 20:34 Lluís Vilanova
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 1/6] hypertrace: Add documentation Lluís Vilanova
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Lluís Vilanova @ 2016-12-26 20:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi

The hypertrace channel allows guest code to emit events in QEMU (the host) using
its tracing infrastructure (see "docs/trace.txt"). This works in both 'system'
and 'user' modes, is architecture-agnostic and introduces minimal noise on the
guest.

See first commit for a full description, use-cases and an example.

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

Changes in v4
=============

* Fix typo in stap script example.
* Fix compilation instructions in doc/hypertrace.txt.
* Rebase on 0737f32daf.


Changes in v3
=============

* Rebase on 4a58f35.
* Remove debugging printf's.
* Fix style issues identified by checkpatch.
* Fix control channel mapping in guest linux module.
* Add a short event description in "trace-events".
* Polish documentation in 1st patch.


Changes in v2
=============

* Remove unnecessary casts for g2h() [Eric Blake].
* Use perror() [Eric Blake].
* Avoid expansions in application example [Eric Blake].
* Add copyright in document "hypertrace.txt" [Eric Blake].
* Make the user-mode hypertrace invocations thread-safe [Stefan Hajnoczi].
* Split dynamic hypertrace configuration into a separate "config" channel.

Lluís Vilanova (6):
      hypertrace: Add documentation
      hypertrace: Add tracing event "guest_hypertrace"
      hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
      hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event
      hypertrace: Add guest-side user-level library
      hypertrace: Add guest-side Linux module


 Makefile                                           |    8 
 Makefile.objs                                      |    6 
 bsd-user/main.c                                    |   17 +
 bsd-user/mmap.c                                    |   15 +
 bsd-user/syscall.c                                 |   34 +-
 configure                                          |   40 ++
 docs/hypertrace.txt                                |  237 ++++++++++++++
 docs/tracing.txt                                   |    3 
 hypertrace/Makefile.objs                           |   21 +
 hypertrace/common.c                                |   26 ++
 hypertrace/common.h                                |   23 +
 hypertrace/guest/linux-module/Kbuild.in            |    7 
 hypertrace/guest/linux-module/Makefile             |   23 +
 .../include/linux/qemu-hypertrace-internal.h       |   46 +++
 .../linux-module/include/linux/qemu-hypertrace.h   |   73 ++++
 hypertrace/guest/linux-module/qemu-hypertrace.c    |  149 +++++++++
 hypertrace/guest/user/Makefile                     |   30 ++
 hypertrace/guest/user/common.c                     |  296 ++++++++++++++++++
 hypertrace/guest/user/qemu-hypertrace.h            |   80 +++++
 hypertrace/softmmu.c                               |  235 ++++++++++++++
 hypertrace/user.c                                  |  332 ++++++++++++++++++++
 hypertrace/user.h                                  |   63 ++++
 include/hw/pci/pci.h                               |    2 
 include/qom/cpu.h                                  |    4 
 linux-user/main.c                                  |   19 +
 linux-user/mmap.c                                  |   17 +
 linux-user/qemu.h                                  |    3 
 linux-user/syscall.c                               |   31 +-
 trace-events                                       |   11 +
 trace/Makefile.objs                                |    2 
 30 files changed, 1826 insertions(+), 27 deletions(-)
 create mode 100644 docs/hypertrace.txt
 create mode 100644 hypertrace/Makefile.objs
 create mode 100644 hypertrace/common.c
 create mode 100644 hypertrace/common.h
 create mode 100644 hypertrace/guest/linux-module/Kbuild.in
 create mode 100644 hypertrace/guest/linux-module/Makefile
 create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h
 create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h
 create mode 100644 hypertrace/guest/linux-module/qemu-hypertrace.c
 create mode 100644 hypertrace/guest/user/Makefile
 create mode 100644 hypertrace/guest/user/common.c
 create mode 100644 hypertrace/guest/user/qemu-hypertrace.h
 create mode 100644 hypertrace/softmmu.c
 create mode 100644 hypertrace/user.c
 create mode 100644 hypertrace/user.h


To: qemu-devel@nongnu.org
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Daniel P Berrange <berrange@redhat.com>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v4 1/6] hypertrace: Add documentation
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
@ 2016-12-26 20:34 ` Lluís Vilanova
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 2/6] hypertrace: Add tracing event "guest_hypertrace" Lluís Vilanova
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Lluís Vilanova @ 2016-12-26 20:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 docs/hypertrace.txt |  237 +++++++++++++++++++++++++++++++++++++++++++++++++++
 docs/tracing.txt    |    3 +
 2 files changed, 240 insertions(+)
 create mode 100644 docs/hypertrace.txt

diff --git a/docs/hypertrace.txt b/docs/hypertrace.txt
new file mode 100644
index 0000000000..1d5d41461c
--- /dev/null
+++ b/docs/hypertrace.txt
@@ -0,0 +1,237 @@
+= Hypertrace channel =
+
+Copyright (C) 2016 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.
+
+
+The hypertrace channel allows guest code to emit events in QEMU (the host) using
+its tracing infrastructure (see "docs/trace.txt"). This works in both 'system'
+and 'user' modes. Therefore, hypertrace is to tracing what hypercalls are to
+system calls.
+
+The hypertrace channel can be used for various purposes:
+
+* Using guest code semantics to guide which QEMU events to trace at each point
+  in time. The example "Quick guide" below shows how to use this to identify
+  "regions of interest" in your guest code. It then uses these regions to trace
+  QEMU's behaviour during their execution (the example traces guest memory
+  accesses), without paying the price of tracing events outside the interest
+  regions.
+
+* Mark "progress points" in guest code (e.g., processed client requests,
+  scheduled processes, etc), so that they can be easily traced and correlated
+  between QEMU's various tracing events and the guest's own tracing
+  infrastructure (e.g., Linux's tracepoints).
+
+* You can also use regions of interest and progress points on the guest code to
+  time the performance of new TCG optimizations. Each hypertrace event comes
+  with a host timestamp, making it easy to compare the host execution times of
+  interesting guest code.
+
+Hypertrace features:
+
+* Works with 'system' and 'user' mode.
+
+* Minimal setup for the guest; QEMU provides support guest code libraries to
+  make 'system' mode with Linux and 'user' mode work out of the box.
+
+* Independent of guest architecture; the guest code uses accesses to special
+  memory regions, as opposed to redefining instruction semantics.
+
+* Negligible guest overhead; emitting a hypertrace event requires a single guest
+  memory access, making it as unobtrusive as possible.
+
+Warning: The hypertrace channel in 'system' mode only works in systems with
+support for PCI. You can get the list of guests with PCI support with 'grep
+pci.mak default-configs/*'.
+
+
+== Quick guide ==
+
+This shows an example of using the hypertrace channel to trace the guest memory
+accesses only in a specific guest code region, which is identified by calls to
+the hypertrace channel.
+
+We are going to trace memory accesses to disk using QEMU's "log" backend, and
+will use QEMU's "dtrace" (SystemTap) backend to ensure memory accesses are only
+traced in the guest code region of interest. The first time the guest code
+invokes the hypertrace channel, we will start tracing the
+"guest_mem_before_exec" event using dtrace, and then will disable it the second
+time around.
+
+Tracing is done with "log" because it is more efficient than using "dtrace" in
+high-volume events like memory accesses. If the event rate is low, you can
+ignore the "log" backend and simply use "dtrace"'s printf, while still using the
+hypertrace events to identify guest semantics.
+
+1. Set the tracing backends and number of arguments for the hypertrace events:
+
+    mkdir /tmp/qemu-build
+    cd /tmp/qemu-build
+    /path/to/qemu-source/configure              \
+        --enable-trace-backends=dtrace,log      \
+        --with-hypertrace-args=4                \
+        --prefix=/tmp/qemu-install
+    make -j install
+
+2. Enable the event "guest_mem_before_exec":
+
+    sed -i -e 's/disable vcpu tcg guest_mem_before/vcpu tcg guest_mem_before/g' /path/to/qemu-source/trace-events
+
+3. Compile QEMU:
+
+    make -C /tmp/qemu-build install -j
+
+4. Compile the guest support code:
+
+    make -C /tmp/qemu-build/x86_64-linux-user/hypertrace/guest/user
+    make -C /tmp/qemu-build/x86_64-softmmu/hypertrace/guest/user
+    make -C /tmp/qemu-build/x86_64-softmmu/hypertrace/guest/linux-module
+
+   If you need to cross-compile the guest library, set the 'CC' variable:
+
+    make -C /tmp/qemu-build/mipsel-linux-user/hypertrace/guest/user CC=mipsel-gnu-linux-gcc
+
+5. Create a guest application using "qemu-hypertrace.h" to interact with the
+   hypertrace channel:
+
+    cat > /tmp/my-hypertrace.c <<\EOF
+    #include <stdio.h>
+    #include <errno.h>
+    #include <stdlib.h>
+    #include <string.h>
+    #include <qemu-hypertrace.h>
+    
+    
+    int main(int argc, char **argv)
+    {
+        char *base = NULL;
+        if (argc > 1) {
+            base = argv[1];
+        }
+    
+        /* In 'user' mode this path must be the same we will use to start QEMU. */
+        if (qemu_hypertrace_init(base) != 0) {
+            perror("error: qemu_hypertrace_init");
+            abort();
+        }
+    
+        /* Set additional event arguments (unused in this example) */
+        uint64_t client  = 0;
+        uint64_t *data = qemu_hypertrace_data(client);
+        data[0] = 0xcafe;
+        data[1] = 0xdead;
+        data[2] = 0xbeef;
+    
+        /* Emit event to start tracing */
+        qemu_hypertrace(client, 1);
+
+        /* Computation in between that we want to trace */
+        printf("Some computation...\n");
+
+        /* Emit event to stop tracing */
+        qemu_hypertrace(client, 0);
+    }
+    EOF
+
+    gcc -o /tmp/my-hypertrace-user /tmp/my-hypertrace.c                                    \
+        /tmp/qemu-build/x86_64-linux-user/hypertrace/guest/user/libqemu-hypertrace-guest.a \
+        -I/tmp/qemu-install/include -lpthread
+
+    gcc -o /tmp/my-hypertrace-softmmu /tmp/my-hypertrace.c                              \
+        /tmp/qemu-build/x86_64-softmmu/hypertrace/guest/user/libqemu-hypertrace-guest.a \
+        -I/tmp/qemu-install/include -lpthread
+
+6. Create a SystemTap script to control event tracing:
+
+    cat > /tmp/my-hypertrace-script.stp <<\EOF
+    #!/usr/bin/env stap
+    
+    %{
+    #include <linux/delay.h>
+    %}
+    
+    function enable_mem:long()
+    %{
+        /* Tell QEMU's monitor to enable tracing */
+        char *argv[4] = {"/bin/sh", "-c", "echo 'trace-event guest_mem_before_exec on' | telnet localhost 1234", NULL};
+        printk(KERN_ERR "enable\n");
+        call_usermodehelper(argv[0], argv, NULL, UMH_WAIT_EXEC);
+        /* Wait for changes to apply */
+        msleep(1000);
+        printk(KERN_ERR "enabled\n");
+        STAP_RETURN(0);
+    %}
+    
+    function disable_mem:long()
+    %{
+        char *argv[4] = {"/bin/sh", "-c", "echo 'trace-event guest_mem_before_exec off' | telnet localhost 1234", NULL};
+        printk(KERN_ERR "disable\n");
+        call_usermodehelper(argv[0], argv, NULL, UMH_WAIT_EXEC);
+        msleep(1000);
+        printk(KERN_ERR "disabled\n");
+        STAP_RETURN(0);
+    %}
+    
+    probe process("/tmp/qemu-install/bin/qemu-*").mark("guest_hypertrace")
+    {
+        if ($arg1 == 1) {
+            enable_mem()
+        } else if ($arg1 == 0) {
+            disable_mem()
+        }
+    }
+    EOF
+
+7. Run a guest system with access to QEMU's hypertrace:
+
+    stap -g /tmp/my-hypertrace-script.stp -c            \
+    '/tmp/qemu-install/bin/qemu-system-x86_64           \
+      -device hypertrace                                \
+      -monitor tcp:localhost:1234,server,nowait         \
+      -trace enable=guest_hypertrace -D /dev/stdout     \
+      ...'
+
+   And inside the VM:
+
+    sudo /tmp/my-hypertrace-softmmu
+
+   The result will be something like this:
+
+    VNC server running on ::1:5900
+    23071@1473096085.744211:guest_hypertrace cpu=0x5602e1f49c10  arg1=1 arg2=0x000000000000cafe arg3=0x000000000000dead arg4=0x000000000000beef
+    23071@1473096085.745763:guest_mem_before_trans cpu=0x5602e1f49c10 info=19
+    23071@1473096085.745907:guest_mem_before_trans cpu=0x5602e1f49c10 info=3
+    23071@1473096085.752368:guest_mem_before_trans cpu=0x5602e1f49c10 info=3
+    23071@1473096085.752384:guest_mem_before_trans cpu=0x5602e1f49c10 info=19
+    23071@1473096086.756117:guest_hypertrace cpu=0x5602e1f49c10  arg1=0 arg2=0x000000000000cafe arg3=0x000000000000dead arg4=0x000000000000beef
+
+To enable hypertrace when running in 'user' mode (standalone guest applications):
+
+    /tmp/qemu-install/bin/qemu-x86_64          \
+      -hypertrace /tmp/hypertrace              \
+      -trace enable=guest* -D /dev/stdout      \
+      /tmp/my-hypertrace-user /tmp/hypertrace
+
+You can also use hypertrace inside the guest's Linux kernel code with the
+provided guest module (see "/tmp/qemu-install/include/linux/qemu-hypertrace.h"):
+
+    sudo insmod /tmp/qemu-build/x86_64-softmmu/hypertrace/guest/linux-module/qemu-hypertrace.ko
+
+
+== Details ==
+
+To make it more efficient in terms of guest and host time, hypertrace provides
+two different memory areas (channels).
+
+The control channel is used by the guest to tell QEMU that new data is ready to
+be processed in the data channel. Writes to the control channel are intercepted
+by QEMU, which emits the "hypertrace" tracing event.
+
+The data channel is a regular memory buffer used by the guest to write
+additional event arguments before raising the event through the control channel.
+
+Both channels accept different "per-client offsets" to enable multiple guest
+threads or CPUs to use the hypertrace channel without having to synchronize.
diff --git a/docs/tracing.txt b/docs/tracing.txt
index f351998a4e..7f62051e4d 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -5,6 +5,9 @@
 This document describes the tracing infrastructure in QEMU and how to use it
 for debugging, profiling, and observing execution.
 
+See "docs/hypertrace.txt" to correlate guest tracing events with those in the
+QEMU host.
+
 == Quickstart ==
 
 1. Build with the 'simple' trace backend:

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v4 2/6] hypertrace: Add tracing event "guest_hypertrace"
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 1/6] hypertrace: Add documentation Lluís Vilanova
@ 2016-12-26 20:34 ` Lluís Vilanova
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Lluís Vilanova @ 2016-12-26 20:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi

Generates the "guest_hypertrace" event with a user-configurable number
of arguments.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile.objs       |    2 ++
 configure           |   34 ++++++++++++++++++++++++++++++++++
 trace-events        |   11 +++++++++++
 trace/Makefile.objs |    2 +-
 4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/Makefile.objs b/Makefile.objs
index 06f74b8b99..5f0be2c3fb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -163,3 +163,5 @@ trace-events-y += target-ppc/trace-events
 trace-events-y += qom/trace-events
 trace-events-y += linux-user/trace-events
 trace-events-y += qapi/trace-events
+
+trace-events-gen-y = hypertrace/trace-events
diff --git a/configure b/configure
index 3770d7c263..afc7071e77 100755
--- a/configure
+++ b/configure
@@ -274,6 +274,7 @@ pie=""
 qom_cast_debug="yes"
 trace_backends="log"
 trace_file="trace"
+hypertrace="1"
 spice=""
 rbd=""
 smartcard=""
@@ -789,6 +790,8 @@ for opt do
   ;;
   --with-trace-file=*) trace_file="$optarg"
   ;;
+  --with-hypertrace-args=*) hypertrace="$optarg"
+  ;;
   --enable-gprof) gprof="yes"
   ;;
   --enable-gcov) gcov="yes"
@@ -1322,6 +1325,8 @@ Advanced options (experts only):
                            Available backends: $($python $source_path/scripts/tracetool.py --list-backends)
   --with-trace-file=NAME   Full PATH,NAME of file to store traces
                            Default:trace-<pid>
+  --with-hypertrace-args=NUMBER
+                           number of hypertrace arguments (default: $hypertrace)
   --disable-slirp          disable SLIRP userspace network connectivity
   --enable-tcg-interpreter enable TCG with bytecode interpreter (TCI)
   --oss-lib                path to OSS library
@@ -4299,6 +4304,16 @@ if test "$?" -ne 0 ; then
 fi
 
 ##########################################
+# check hypertrace arguments
+case "$hypertrace" in
+    ''|*[!0-9]*) error_exit "invalid number of hypertrace arguments" ;;
+    *) ;;
+esac
+if test $hypertrace = 0; then
+    error_exit "hypertrace arguments must be greater than zero"
+fi
+
+##########################################
 # For 'ust' backend, test if ust headers are present
 if have_backend "ust"; then
   cat > $TMPC << EOF
@@ -5066,6 +5081,7 @@ echo "Trace backends    $trace_backends"
 if have_backend "simple"; then
 echo "Trace output file $trace_file-<pid>"
 fi
+echo "Hypertrace arguments  $hypertrace"
 echo "spice support     $spice $(echo_version $spice $spice_protocol_version/$spice_server_version)"
 echo "rbd support       $rbd"
 echo "xfsctl support    $xfs"
@@ -5731,6 +5747,24 @@ else
 fi
 QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
 
+# hypertrace
+echo "CONFIG_HYPERTRACE_ARGS=$hypertrace" >> $config_host_mak
+hypertrace_events=hypertrace/trace-events
+mkdir -p $(dirname $hypertrace_events)
+echo "# See docs/trace-events.txt for syntax documentation." >$hypertrace_events
+echo -n 'vcpu guest_hypertrace(' >>$hypertrace_events
+for i in `seq $hypertrace`; do
+    if test $i != 1; then
+        echo -n ", " >>$hypertrace_events
+    fi
+    echo -n "uint64_t arg$i" >>$hypertrace_events
+done
+echo -n ') ' >>$hypertrace_events
+for i in `seq $hypertrace`; do
+    echo -n "\" arg$i=0x%016\"PRIx64" >>$hypertrace_events
+done
+echo >>$hypertrace_events
+
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
 echo "MAKE=$make" >> $config_host_mak
diff --git a/trace-events b/trace-events
index f74e1d3d22..fe6e5af880 100644
--- a/trace-events
+++ b/trace-events
@@ -141,6 +141,17 @@ vcpu guest_cpu_enter(void)
 # Targets: all
 vcpu guest_cpu_reset(void)
 
+# @argX: Hypertrace arguments.
+#
+# Event explicitly triggered by guest code. The event definition is
+# auto-generated from the "--with-hypertrace-args" configuration argument (see
+# "docs/hypertrace.txt"):
+#
+#   vcpu guest_hypertrace(uint64_t arg0, ..., uint64_t argN-1)
+#
+# Mode: all
+# Targets: all
+
 # @vaddr: Access' virtual address.
 # @info : Access' information (see below).
 #
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 1e1ce7479d..7ae6c9fdc7 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -8,7 +8,7 @@
 tracetool-y = $(SRC_PATH)/scripts/tracetool.py
 tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
 
-$(BUILD_DIR)/trace-events-all: $(trace-events-y:%=$(SRC_PATH)/%)
+$(BUILD_DIR)/trace-events-all: $(trace-events-y:%=$(SRC_PATH)/%) $(trace-events-gen-y:%=$(BUILD_DIR)/%)
 	$(call quiet-command,cat $^ > $@)
 
 ######################################################################

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 1/6] hypertrace: Add documentation Lluís Vilanova
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 2/6] hypertrace: Add tracing event "guest_hypertrace" Lluís Vilanova
@ 2016-12-26 20:34 ` Lluís Vilanova
  2017-01-09 15:44   ` Stefan Hajnoczi
  2017-01-09 16:35   ` Stefan Hajnoczi
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 4/6] hypertrace: [softmmu] " Lluís Vilanova
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Lluís Vilanova @ 2016-12-26 20:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi,
	Riku Voipio

QEMU detects when the guest uses 'mmap' on hypertrace's control channel
file, and then uses 'mprotect' to detect accesses to it, which are used
to trigger tracing event "guest_hypertrace".

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile.objs            |    4 +
 bsd-user/main.c          |   17 ++
 bsd-user/mmap.c          |   15 ++
 bsd-user/syscall.c       |   34 +++--
 hypertrace/Makefile.objs |   18 ++
 hypertrace/common.c      |   26 ++++
 hypertrace/common.h      |   23 +++
 hypertrace/user.c        |  332 ++++++++++++++++++++++++++++++++++++++++++++++
 hypertrace/user.h        |   63 +++++++++
 include/qom/cpu.h        |    4 +
 linux-user/main.c        |   19 +++
 linux-user/mmap.c        |   17 ++
 linux-user/qemu.h        |    3 
 linux-user/syscall.c     |   31 +++-
 14 files changed, 581 insertions(+), 25 deletions(-)
 create mode 100644 hypertrace/Makefile.objs
 create mode 100644 hypertrace/common.c
 create mode 100644 hypertrace/common.h
 create mode 100644 hypertrace/user.c
 create mode 100644 hypertrace/user.h

diff --git a/Makefile.objs b/Makefile.objs
index 5f0be2c3fb..5f3a68ca48 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -105,6 +105,10 @@ util-obj-y +=  trace/
 target-obj-y += trace/
 
 ######################################################################
+# hypertrace
+target-obj-y += hypertrace/
+
+######################################################################
 # guest agent
 
 # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 714a692e6f..d42ded4fc8 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -31,9 +31,12 @@
 #include "tcg.h"
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
+#include "qemu/error-report.h"
 #include "exec/log.h"
 #include "trace/control.h"
 #include "glib-compat.h"
+#include "hypertrace/user.h"
+
 
 int singlestep;
 unsigned long mmap_min_addr;
@@ -676,6 +679,8 @@ static void usage(void)
            "-strace           log system calls\n"
            "-trace            [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
            "                  specify tracing options\n"
+           "-hypertrace       [[base=]<path>][,max-clients=<uint>]\n"
+           "                  specify hypertrace options\n"
            "\n"
            "Environment variables:\n"
            "QEMU_STRACE       Print system calls and arguments similar to the\n"
@@ -736,6 +741,8 @@ int main(int argc, char **argv)
     envlist_t *envlist = NULL;
     char *trace_file = NULL;
     bsd_type = target_openbsd;
+    char *hypertrace_base = NULL;
+    unsigned int hypertrace_max_clients = 0;
 
     if (argc <= 1)
         usage();
@@ -757,6 +764,7 @@ int main(int argc, char **argv)
     cpu_model = NULL;
 
     qemu_add_opts(&qemu_trace_opts);
+    qemu_add_opts(&qemu_hypertrace_opts);
 
     optind = 1;
     for (;;) {
@@ -847,6 +855,10 @@ int main(int argc, char **argv)
         } else if (!strcmp(r, "trace")) {
             g_free(trace_file);
             trace_file = trace_opt_parse(optarg);
+        } else if (!strcmp(r, "hypertrace")) {
+            g_free(hypertrace_file);
+            hypertrace_opt_parse(optarg, &hypertrace_base,
+                                 &hypertrace_max_clients);
         } else {
             usage();
         }
@@ -981,6 +993,11 @@ int main(int argc, char **argv)
     target_set_brk(info->brk);
     syscall_init();
     signal_init();
+    if (atexit(hypertrace_fini) != 0) {
+        fprintf(stderr, "error: atexit: %s\n", strerror(errno));
+        abort();
+    }
+    hypertrace_init(hypertrace_base, hypertrace_size);
 
     /* Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
        generating the prologue until now so that the prologue can take
diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index ee5907330f..650b8203fa 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -21,6 +21,7 @@
 #include "qemu.h"
 #include "qemu-common.h"
 #include "bsd-mman.h"
+#include "hypertrace/user.h"
 
 //#define DEBUG_MMAP
 
@@ -256,10 +257,17 @@ static abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
     return addr;
 }
 
-/* NOTE: all the constants are the HOST ones */
 abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                      int flags, int fd, abi_ulong offset)
 {
+    return target_mmap_cpu(start, len, prot, flags, fd, offset, NULL);
+}
+
+/* NOTE: all the constants are the HOST ones */
+abi_long target_mmap_cpu(abi_ulong start, abi_ulong len, int prot,
+                         int flags, int fd, abi_ulong offset,
+                         CPUState *cpu)
+{
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
     unsigned long host_start;
 
@@ -301,6 +309,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         goto the_end;
     real_start = start & qemu_host_page_mask;
 
+    if (!hypertrace_guest_mmap_check(fd, len, offset)) {
+        goto fail;
+    }
+
     if (!(flags & MAP_FIXED)) {
         abi_ulong mmap_start;
         void *p;
@@ -412,6 +424,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         }
     }
  the_end1:
+    hypertrace_guest_mmap_apply(fd, g2h(start), cpu);
     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 66492aaf5d..f88f21876c 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -26,6 +26,7 @@
 
 #include "qemu.h"
 #include "qemu-common.h"
+#include "hypertrace/user.h"
 
 //#define DEBUG
 
@@ -332,6 +333,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+        hypertrace_fini();
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
         ret = 0; /* avoid warning */
@@ -369,10 +371,12 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg1, 0);
         break;
     case TARGET_FREEBSD_NR_mmap:
-        ret = get_errno(target_mmap(arg1, arg2, arg3,
-                                    target_to_host_bitmask(arg4, mmap_flags_tbl),
-                                    arg5,
-                                    arg6));
+        ret = get_errno(target_mmap_cpu(
+                            arg1, arg2, arg3,
+                            target_to_host_bitmask(arg4, mmap_flags_tbl),
+                            arg5,
+                            arg6,
+                            cpu));
         break;
     case TARGET_FREEBSD_NR_mprotect:
         ret = get_errno(target_mprotect(arg1, arg2, arg3));
@@ -430,6 +434,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+        hypertrace_fini();
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
         ret = 0; /* avoid warning */
@@ -455,10 +460,12 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg1, 0);
         break;
     case TARGET_NETBSD_NR_mmap:
-        ret = get_errno(target_mmap(arg1, arg2, arg3,
-                                    target_to_host_bitmask(arg4, mmap_flags_tbl),
-                                    arg5,
-                                    arg6));
+        ret = get_errno(target_mmap_cpu(
+                            arg1, arg2, arg3,
+                            target_to_host_bitmask(arg4, mmap_flags_tbl),
+                            arg5,
+                            arg6,
+                            cpu));
         break;
     case TARGET_NETBSD_NR_mprotect:
         ret = get_errno(target_mprotect(arg1, arg2, arg3));
@@ -505,6 +512,7 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+        hypertrace_fini();
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
         ret = 0; /* avoid warning */
@@ -530,10 +538,12 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg1, 0);
         break;
     case TARGET_OPENBSD_NR_mmap:
-        ret = get_errno(target_mmap(arg1, arg2, arg3,
-                                    target_to_host_bitmask(arg4, mmap_flags_tbl),
-                                    arg5,
-                                    arg6));
+        ret = get_errno(target_mmap_cpu(
+                            arg1, arg2, arg3,
+                            target_to_host_bitmask(arg4, mmap_flags_tbl),
+                            arg5,
+                            arg6,
+                            cpu));
         break;
     case TARGET_OPENBSD_NR_mprotect:
         ret = get_errno(target_mprotect(arg1, arg2, arg3));
diff --git a/hypertrace/Makefile.objs b/hypertrace/Makefile.objs
new file mode 100644
index 0000000000..24e8fb4eea
--- /dev/null
+++ b/hypertrace/Makefile.objs
@@ -0,0 +1,18 @@
+# -*- mode: makefile -*-
+
+target-obj-$(CONFIG_USER_ONLY) += user.o
+target-obj-y += common.o
+
+$(obj)/user.o: $(obj)/emit.c
+
+$(obj)/emit.c: $(obj)/emit.c-timestamp $(BUILD_DIR)/config-host.mak
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
+$(obj)/emit.c-timestamp: $(BUILD_DIR)/config-host.mak
+	@echo "static void hypertrace_emit(CPUState *cpu, uint64_t arg1, uint64_t *data)" >$@
+	@echo "{" >>$@
+	@echo -n "    trace_guest_hypertrace(cpu, arg1" >>$@
+	@for i in `seq $$(( $(CONFIG_HYPERTRACE_ARGS) - 1 ))`; do \
+	    echo -n ", data[$$i-1]" >>$@; \
+	done
+	@echo ");" >>$@
+	@echo "}" >>$@
diff --git a/hypertrace/common.c b/hypertrace/common.c
new file mode 100644
index 0000000000..baca098aa0
--- /dev/null
+++ b/hypertrace/common.c
@@ -0,0 +1,26 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016 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/osdep.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "hypertrace/common.h"
+#include "qemu/osdep.h"
+
+void hypertrace_init_config(struct hypertrace_config *config,
+                            unsigned int max_clients)
+{
+    config->max_clients = max_clients;
+    config->client_args = CONFIG_HYPERTRACE_ARGS;
+    config->client_data_size = config->client_args * sizeof(uint64_t);
+    config->control_size = QEMU_ALIGN_UP(
+        config->max_clients * sizeof(uint64_t), TARGET_PAGE_SIZE);
+    config->data_size = QEMU_ALIGN_UP(
+        config->max_clients * config->client_data_size, TARGET_PAGE_SIZE);
+}
diff --git a/hypertrace/common.h b/hypertrace/common.h
new file mode 100644
index 0000000000..6eabbd6551
--- /dev/null
+++ b/hypertrace/common.h
@@ -0,0 +1,23 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016 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.
+ */
+
+#if !defined(__KERNEL__)
+#include <stdint.h>
+#endif
+
+struct hypertrace_config {
+    uint64_t max_clients;
+    uint64_t client_args;
+    uint64_t client_data_size;
+    uint64_t control_size;
+    uint64_t data_size;
+};
+
+void hypertrace_init_config(struct hypertrace_config *config,
+                            unsigned int max_clients);
diff --git a/hypertrace/user.c b/hypertrace/user.c
new file mode 100644
index 0000000000..5649c0369a
--- /dev/null
+++ b/hypertrace/user.c
@@ -0,0 +1,332 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016 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.
+ */
+
+/*
+ * Implementation details
+ * ======================
+ *
+ * There are 3 channels, each a regular file in the host system, and mmap'ed by
+ * the guest application.
+ *
+ * - Configuration channel: Exposes configuration parameters. Mapped once and
+ *   directly readable.
+ *
+ * - Data channel: Lets guests write argument values. Each guest thread should
+ *   use a different offset to avoid concurrency problems. Mapped once and
+ *   directly accessible.
+ *
+ * - Control channel: Triggers the hypertrace event on a write, providing the
+ *   first argument. Offset in the control channel sets the offset in the data
+ *   channel. Mapped once per thread, using two pages to reliably detect
+ *   accesses and their written value through a SEGV handler.
+ */
+
+#include <assert.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <sys/mman.h>
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+
+#include "hypertrace/common.h"
+#include "hypertrace/user.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+#include "trace.h"
+
+
+static struct hypertrace_config config;
+static char *config_path;
+static int config_fd = -1;
+static uint64_t *qemu_config;
+
+static char *data_path;
+static int data_fd = -1;
+static uint64_t *qemu_data;
+
+static char *control_path;
+static int control_fd = -1;
+static uint64_t *qemu_control;
+static struct stat control_fd_stat;
+
+struct sigaction segv_next;
+static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt);
+
+
+QemuOptsList qemu_hypertrace_opts = {
+    .name = "hypertrace",
+    .implied_opt_name = "path",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_hypertrace_opts.head),
+    .desc = {
+        {
+            .name = "path",
+            .type = QEMU_OPT_STRING,
+        },
+        {
+            .name = "max-clients",
+            .type = QEMU_OPT_NUMBER,
+            .def_value_str = "1",
+        },
+        { /* end of list */ }
+    },
+};
+
+void hypertrace_opt_parse(const char *optarg, char **base,
+                          unsigned int *max_clients_)
+{
+    int max_clients;
+    QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("hypertrace"),
+                                             optarg, true);
+    if (!opts) {
+        exit(1);
+    }
+    if (qemu_opt_get(opts, "path")) {
+        *base = g_strdup(qemu_opt_get(opts, "path"));
+    } else {
+        *base = NULL;
+    }
+    max_clients = qemu_opt_get_number(opts, "pages", 1);
+    if (max_clients <= 0) {
+        error_report("Parameter 'max-clients' expects a positive number");
+        exit(EXIT_FAILURE);
+    }
+    *max_clients_ = max_clients;
+}
+
+static void init_channel(const char *base, const char *suffix, size_t size,
+                         char **path, int *fd, uint64_t **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) {
+        error_report("error: open(%s): %s", *path, strerror(errno));
+        abort();
+    }
+
+    off_t lres = lseek(*fd, size - 1, SEEK_SET);
+    if (lres == (off_t)-1) {
+        error_report("error: lseek(%s): %s", *path, strerror(errno));
+        abort();
+    }
+
+    char tmp;
+    ssize_t wres = write(*fd, &tmp, 1);
+    if (wres == -1) {
+        error_report("error: write(%s): %s", *path, strerror(errno));
+        abort();
+    }
+
+    if (addr) {
+        *addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, *fd, 0);
+        if (*addr == MAP_FAILED) {
+            error_report("error: mmap(%s): %s", *path, strerror(errno));
+            abort();
+        }
+    }
+}
+
+static void fini_handler(int signum, siginfo_t *siginfo, void *sigctxt)
+{
+    hypertrace_fini();
+}
+
+void hypertrace_init(const char *base, unsigned int max_clients)
+{
+    struct sigaction sigint;
+    struct hypertrace_config *pconfig;
+
+    if (base == NULL) {
+        return;
+    }
+
+    memset(&sigint, 0, sizeof(sigint));
+    sigint.sa_sigaction = fini_handler;
+    sigint.sa_flags = SA_SIGINFO | SA_RESTART;
+    if (sigaction(SIGINT, &sigint, NULL) != 0) {
+        error_report("error: sigaction(SIGINT): %s", strerror(errno));
+        abort();
+    }
+    if (sigaction(SIGABRT, &sigint, NULL) != 0) {
+        error_report("error: sigaction(SIGABRT): %s", strerror(errno));
+        abort();
+    }
+
+    hypertrace_init_config(&config, max_clients);
+    /* We need twice the space for the double-fault protocol */
+    config.control_size *= 2;
+
+    init_channel(base, "-config", TARGET_PAGE_SIZE,
+                 &config_path, &config_fd, &qemu_config);
+    pconfig = (struct hypertrace_config *)qemu_config;
+    pconfig->max_clients = tswap64(config.max_clients);
+    pconfig->client_args = tswap64(config.client_args);
+    pconfig->client_data_size = tswap64(config.client_data_size);
+    pconfig->control_size = tswap64(config.control_size);
+    pconfig->data_size = tswap64(config.data_size);
+
+    init_channel(base, "-data", config.data_size,
+                 &data_path, &data_fd, &qemu_data);
+    if (fstat(data_fd, &control_fd_stat) == -1) {
+        error_report("error: fstat(hypertrace_control): %s", strerror(errno));
+        abort();
+    }
+
+    init_channel(base, "-control", config.control_size,
+                 &control_path, &control_fd, &qemu_control);
+
+    if (fstat(control_fd, &control_fd_stat) == -1) {
+        error_report("error: fstat(hypertrace_control): %s", 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) {
+        error_report("error: sigaction(SIGSEGV): %s", strerror(errno));
+        abort();
+    }
+}
+
+
+static void fini_channel(int *fd, char **path)
+{
+    if (*fd != -1) {
+        if (close(*fd) == -1) {
+            error_report("error: close: %s", strerror(errno));
+            abort();
+        }
+        if (unlink(*path) == -1) {
+            error_report("error: unlink(%s): %s", *path, strerror(errno));
+            abort();
+        }
+        *fd = -1;
+    }
+    if (*path != NULL) {
+        g_free(*path);
+        *path =  NULL;
+    }
+}
+
+void hypertrace_fini(void)
+{
+    static bool atexit_in;
+    if (atexit_in) {
+        return;
+    }
+    atexit_in = true;
+
+    if (sigaction(SIGSEGV, &segv_next, NULL) != 0) {
+        error_report("error: sigaction(SIGSEGV): %s", strerror(errno));
+        abort();
+    }
+    fini_channel(&config_fd, &config_path);
+    fini_channel(&data_fd, &data_path);
+    fini_channel(&control_fd, &control_path);
+}
+
+
+bool hypertrace_guest_mmap_check(int fd, unsigned long len,
+                                 unsigned long offset)
+{
+    struct stat s;
+    if (fstat(fd, &s) < 0) {
+        return true;
+    }
+
+    if (s.st_dev != control_fd_stat.st_dev ||
+        s.st_ino != control_fd_stat.st_ino) {
+        return true;
+    }
+
+    return len == (config.control_size) && offset == 0;
+}
+
+void hypertrace_guest_mmap_apply(int fd, void *qemu_addr, CPUState *vcpu)
+{
+    struct stat s;
+
+    if (vcpu == NULL) {
+        return;
+    }
+
+    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)
+     */
+    vcpu->hypertrace_control = qemu_addr;
+    if (mprotect(qemu_addr, config.control_size / 2, PROT_READ) == -1) {
+        error_report("error: mprotect(hypertrace_control): %s",
+                     strerror(errno));
+        abort();
+    }
+}
+
+static void swap_control(void *from, void *to)
+{
+    if (mprotect(from, config.control_size / 2, PROT_READ | PROT_WRITE) == -1) {
+        error_report("error: mprotect(from): %s", strerror(errno));
+        abort();
+    }
+    if (mprotect(to, config.control_size / 2, PROT_READ) == -1) {
+        error_report("error: mprotect(to): %s", strerror(errno));
+        abort();
+    }
+}
+
+#include "hypertrace/emit.c"
+
+static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
+{
+    CPUState *vcpu = current_cpu;
+    void *control_0 = vcpu->hypertrace_control;
+    void *control_1 = vcpu->hypertrace_control + config.control_size / 2;
+    void *control_2 = control_1 + config.control_size / 2;
+
+    if (control_0 <= siginfo->si_addr && siginfo->si_addr < control_1) {
+
+        /* 1st fault (guest will write cmd) */
+        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
+        swap_control(control_0, control_1);
+
+    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
+        size_t client = (siginfo->si_addr - control_1) / sizeof(uint64_t);
+        uint64_t vcontrol = ((uint64_t *)control_0)[client];
+        uint64_t *data_ptr = &qemu_data[client * config.client_data_size];
+
+        /* 2nd fault (invoke) */
+        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
+        hypertrace_emit(current_cpu, vcontrol, data_ptr);
+        swap_control(control_1, 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/hypertrace/user.h b/hypertrace/user.h
new file mode 100644
index 0000000000..b2bc60d507
--- /dev/null
+++ b/hypertrace/user.h
@@ -0,0 +1,63 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016 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>
+
+
+/**
+ * Definition of QEMU options describing hypertrace subsystem configuration
+ */
+extern QemuOptsList qemu_hypertrace_opts;
+
+/**
+ * hypertrace_opt_parse:
+ * @optarg: Input arguments.
+ * @base: Output base path for the hypertrace channel files.
+ * @max_clients: Output maximum number of concurrent clients.
+ *
+ * Parse the commandline arguments for hypertrace.
+ */
+void hypertrace_opt_parse(const char *optarg, char **base,
+                          unsigned int *max_clients);
+
+/**
+ * hypertrace_init:
+ * @base: Base path for the hypertrace channel files.
+ * @max_clients: Maximum number of concurrent clients.
+ *
+ * Initialize the backing files for the hypertrace channel.
+ */
+void hypertrace_init(const char *base, unsigned int max_clients);
+
+/**
+ * hypertrace_guest_mmap_check:
+ *
+ * Verify argument validity when mapping the control channel.
+ *
+ * Precondition: defined(CONFIG_USER_ONLY)
+ */
+bool hypertrace_guest_mmap_check(int fd, unsigned long len,
+                                 unsigned long offset);
+
+/**
+ * hypertrace_guest_mmap_apply:
+ *
+ * Configure initial mprotect if mapping the control channel.
+ *
+ * Precondition: defined(CONFIG_USER_ONLY)
+ */
+void hypertrace_guest_mmap_apply(int fd, void *qemu_addr, CPUState *vcpu);
+
+/**
+ * hypertrace_fini:
+ *
+ * Remove the backing files for the hypertrace channel.
+ */
+void hypertrace_fini(void);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 3f79a8e955..8e7b579083 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -296,6 +296,7 @@ struct qemu_work_item;
  * @work_mutex: Lock to prevent multiple access to queued_work_*.
  * @queued_work_first: First asynchronous work pending.
  * @trace_dstate: Dynamic tracing state of events for this vCPU (bitmask).
+ * @hypertrace_control: Per-vCPU address of the hypertrace control channel.
  *
  * State of one CPU core or thread.
  */
@@ -372,6 +373,9 @@ struct CPUState {
      */
     unsigned long *trace_dstate;
 
+    /* Only used when defined(CONFIG_USER_ONLY) */
+    void *hypertrace_control;
+
     /* TODO Move common fields from CPUArchState here. */
     int cpu_index; /* used by alpha TCG */
     uint32_t halted; /* used by alpha, cris, ppc TCG */
diff --git a/linux-user/main.c b/linux-user/main.c
index 75b199f274..8006abb102 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -32,10 +32,12 @@
 #include "tcg.h"
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
+#include "qemu/error-report.h"
 #include "elf.h"
 #include "exec/log.h"
 #include "trace/control.h"
 #include "glib-compat.h"
+#include "hypertrace/user.h"
 
 char *exec_path;
 
@@ -3748,6 +3750,14 @@ static void handle_arg_trace(const char *arg)
     trace_file = trace_opt_parse(arg);
 }
 
+static char *hypertrace_base;
+static unsigned int hypertrace_max_clients;
+static void handle_arg_hypertrace(const char *arg)
+{
+    g_free(hypertrace_base);
+    hypertrace_opt_parse(arg, &hypertrace_base, &hypertrace_max_clients);
+}
+
 struct qemu_argument {
     const char *argv;
     const char *env;
@@ -3797,6 +3807,8 @@ static const struct qemu_argument arg_table[] = {
      "",           "Seed for pseudo-random number generator"},
     {"trace",      "QEMU_TRACE",       true,  handle_arg_trace,
      "",           "[[enable=]<pattern>][,events=<file>][,file=<file>]"},
+    {"hypertrace", "QEMU_HYPERTRACE",  true,  handle_arg_hypertrace,
+     "",           "[[base=]<path>][,max-clients=<uint>]"},
     {"version",    "QEMU_VERSION",     false, handle_arg_version,
      "",           "display version information and exit"},
     {NULL, NULL, false, NULL, NULL, NULL}
@@ -3989,6 +4001,7 @@ int main(int argc, char **argv, char **envp)
     srand(time(NULL));
 
     qemu_add_opts(&qemu_trace_opts);
+    qemu_add_opts(&qemu_hypertrace_opts);
 
     optind = parse_args(argc, argv);
 
@@ -4187,6 +4200,12 @@ int main(int argc, char **argv, char **envp)
     syscall_init();
     signal_init();
 
+    if (atexit(hypertrace_fini)) {
+        fprintf(stderr, "error: atexit: %s\n", strerror(errno));
+        abort();
+    }
+    hypertrace_init(hypertrace_base, hypertrace_max_clients);
+
     /* Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
        generating the prologue until now so that the prologue can take
        the real value of GUEST_BASE into account.  */
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 61685bf79e..52f1a54281 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -21,6 +21,7 @@
 #include "qemu.h"
 #include "qemu-common.h"
 #include "translate-all.h"
+#include "hypertrace/user.h"
 
 //#define DEBUG_MMAP
 
@@ -360,10 +361,18 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
     }
 }
 
-/* NOTE: all the constants are the HOST ones */
 abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                      int flags, int fd, abi_ulong offset)
 {
+    return target_mmap_cpu(start, len, prot, flags, fd, offset, NULL);
+}
+
+
+/* NOTE: all the constants are the HOST ones */
+abi_long target_mmap_cpu(abi_ulong start, abi_ulong len, int prot,
+                         int flags, int fd, abi_ulong offset,
+                         CPUState *cpu)
+{
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
 
     mmap_lock();
@@ -445,6 +454,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
        }
     }
 
+    if (!hypertrace_guest_mmap_check(fd, len, offset)) {
+        goto fail;
+    }
+
     if (!(flags & MAP_FIXED)) {
         unsigned long host_start;
         void *p;
@@ -556,6 +569,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         }
     }
  the_end1:
+    printf("mmap "TARGET_ABI_FMT_lx" "TARGET_ABI_FMT_lu"\n", start, len);
+    hypertrace_guest_mmap_apply(fd, g2h(start), cpu);
     page_set_flags(start, start + len, prot | PAGE_VALID);
  the_end:
 #ifdef DEBUG_MMAP
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index da73a01106..da10920907 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -422,6 +422,9 @@ void sparc64_get_context(CPUSPARCState *env);
 int target_mprotect(abi_ulong start, abi_ulong len, int prot);
 abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                      int flags, int fd, abi_ulong offset);
+abi_long target_mmap_cpu(abi_ulong start, abi_ulong len, int prot,
+                         int flags, int fd, abi_ulong offset,
+                         CPUState *cpu);
 int target_munmap(abi_ulong start, abi_ulong len);
 abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
                        abi_ulong new_size, unsigned long flags,
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 7b77503f94..0432001cda 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -113,6 +113,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
 #include "uname.h"
 
 #include "qemu.h"
+#include "hypertrace/user.h"
 
 #ifndef CLONE_IO
 #define CLONE_IO                0x80000000      /* Clone io context */
@@ -7603,6 +7604,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+        hypertrace_fini();
         _exit(arg1);
         ret = 0; /* avoid warning */
         break;
@@ -9051,15 +9053,19 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             v5 = tswapal(v[4]);
             v6 = tswapal(v[5]);
             unlock_user(v, arg1, 0);
-            ret = get_errno(target_mmap(v1, v2, v3,
-                                        target_to_host_bitmask(v4, mmap_flags_tbl),
-                                        v5, v6));
+            ret = get_errno(target_mmap_cpu(
+                                v1, v2, v3,
+                                target_to_host_bitmask(v4, mmap_flags_tbl),
+                                v5, v6,
+                                cpu));
         }
 #else
-        ret = get_errno(target_mmap(arg1, arg2, arg3,
-                                    target_to_host_bitmask(arg4, mmap_flags_tbl),
-                                    arg5,
-                                    arg6));
+        ret = get_errno(target_mmap_cpu(
+                            arg1, arg2, arg3,
+                            target_to_host_bitmask(arg4, mmap_flags_tbl),
+                            arg5,
+                            arg6,
+                            cpu));
 #endif
         break;
 #endif
@@ -9068,10 +9074,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifndef MMAP_SHIFT
 #define MMAP_SHIFT 12
 #endif
-        ret = get_errno(target_mmap(arg1, arg2, arg3,
-                                    target_to_host_bitmask(arg4, mmap_flags_tbl),
-                                    arg5,
-                                    arg6 << MMAP_SHIFT));
+        ret = get_errno(target_mmap_cpu(
+                            arg1, arg2, arg3,
+                            target_to_host_bitmask(arg4, mmap_flags_tbl),
+                            arg5,
+                            arg6 << MMAP_SHIFT,
+                            cpu));
         break;
 #endif
     case TARGET_NR_munmap:
@@ -9642,6 +9650,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         _mcleanup();
 #endif
         gdb_exit(cpu_env, arg1);
+        hypertrace_fini();
         ret = get_errno(exit_group(arg1));
         break;
 #endif

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v4 4/6] hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (2 preceding siblings ...)
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
@ 2016-12-26 20:34 ` Lluís Vilanova
  2017-01-09 16:12   ` Stefan Hajnoczi
  2016-12-26 20:35 ` [Qemu-devel] [PATCH v4 5/6] hypertrace: Add guest-side user-level library Lluís Vilanova
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Lluís Vilanova @ 2016-12-26 20:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi,
	Michael S. Tsirkin, Marcel Apfelbaum

Uses a virtual device to trigger the hypertrace channel event.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 hypertrace/Makefile.objs |    5 +
 hypertrace/softmmu.c     |  235 ++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/pci/pci.h     |    2 
 3 files changed, 241 insertions(+), 1 deletion(-)
 create mode 100644 hypertrace/softmmu.c

diff --git a/hypertrace/Makefile.objs b/hypertrace/Makefile.objs
index 24e8fb4eea..0025207a4f 100644
--- a/hypertrace/Makefile.objs
+++ b/hypertrace/Makefile.objs
@@ -1,9 +1,12 @@
 # -*- mode: makefile -*-
 
 target-obj-$(CONFIG_USER_ONLY) += user.o
+ifdef CONFIG_PCI
+target-obj-$(CONFIG_SOFTMMU) += softmmu.o
+endif
 target-obj-y += common.o
 
-$(obj)/user.o: $(obj)/emit.c
+$(obj)/user.o $(obj)/softmmu.o: $(obj)/emit.c
 
 $(obj)/emit.c: $(obj)/emit.c-timestamp $(BUILD_DIR)/config-host.mak
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
diff --git a/hypertrace/softmmu.c b/hypertrace/softmmu.c
new file mode 100644
index 0000000000..4a6008cccd
--- /dev/null
+++ b/hypertrace/softmmu.c
@@ -0,0 +1,235 @@
+/*
+ * QEMU-side management of hypertrace in softmmu emulation.
+ *
+ * Copyright (C) 2016 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.
+ */
+
+/*
+ * Implementation details
+ * ======================
+ *
+ * There are 3 channels, each a BAR of a virtual device that can be used through
+ * MMIO.
+ *
+ *
+ * - Configuration channel: Exposes configuration parameters.
+ *
+ * - Data channel: Lets guests write argument values. Each guest client should
+ *   use a different offset to avoid concurrency problems.
+ *
+ * - Control channel: Triggers the hypertrace event on a write, providing the
+ *   first argument. Offset in the control channel sets the offset in the data
+ *   channel.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/ram_addr.h"
+#include "hypertrace/common.h"
+#include "hw/pci/pci.h"
+#include "migration/migration.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "trace.h"
+
+
+#define PAGE_SIZE TARGET_PAGE_SIZE
+
+
+typedef struct HypertraceState {
+    PCIDevice dev;
+
+    uint64_t max_clients;
+    struct hypertrace_config hconfig;
+
+    MemoryRegion config;
+    void *config_ptr;
+    MemoryRegion data;
+    void *data_ptr;
+    MemoryRegion control;
+    void *control_ptr;
+
+    Error *migration_blocker;
+} HypertraceState;
+
+
+static uint64_t hypertrace_control_io_read(void *opaque, hwaddr addr,
+                                           unsigned size)
+{
+    uint64_t res;
+    HypertraceState *s = opaque;
+    char *mem = &((char *)s->control_ptr)[addr];
+
+    switch (size) {
+    case 1:
+    {
+        res = ((uint8_t *)mem)[0];
+        break;
+    }
+    case 2:
+    {
+        res = ((uint16_t *)mem)[0];
+        break;
+    }
+    case 4:
+    {
+        res = ((uint32_t *)mem)[0];
+        break;
+    }
+    case 8:
+    {
+        res = ((uint64_t *)mem)[0];
+        break;
+    }
+    default:
+        error_report("error: hypertrace: Unexpected read of size %d", size);
+    }
+
+    return res;
+}
+
+#include "hypertrace/emit.c"
+
+static void hypertrace_control_io_write(void *opaque, hwaddr addr,
+                                        uint64_t data, unsigned size)
+{
+    HypertraceState *s = opaque;
+    char *mem = &((char *)s->control_ptr)[addr];
+
+    switch (size) {
+    case 1:
+    {
+        uint8_t *res = (uint8_t *)mem;
+        *res = (uint8_t)data;
+        break;
+    }
+    case 2:
+    {
+        uint16_t *res = (uint16_t *)mem;
+        *res = (uint16_t)data;
+        break;
+    }
+    case 4:
+    {
+        uint32_t *res = (uint32_t *)mem;
+        *res = (uint32_t)data;
+        break;
+    }
+    case 8:
+    {
+        uint64_t *res = (uint64_t *)mem;
+        *res = (uint64_t)data;
+        break;
+    }
+    default:
+        error_report("error: hypertrace: Unexpected write of size %d", size);
+    }
+
+    if ((addr + size) % sizeof(uint64_t) == 0) {
+        uint64_t client = addr / sizeof(uint64_t);
+        uint64_t vcontrol = ((uint64_t *)s->control_ptr)[client];
+        uint64_t *data_ptr = (uint64_t *)s->data_ptr;
+        data_ptr = &data_ptr[client * s->hconfig.client_data_size];
+        hypertrace_emit(current_cpu, vcontrol, data_ptr);
+    }
+}
+
+static const MemoryRegionOps hypertrace_control_ops = {
+    .read = &hypertrace_control_io_read,
+    .write = &hypertrace_control_io_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+    },
+};
+
+
+static void hypertrace_realize(PCIDevice *dev, Error **errp)
+{
+    struct hypertrace_config *config;
+    HypertraceState *s = DO_UPCAST(HypertraceState, dev, dev);
+    Error *err = NULL;
+
+    if (s->max_clients < 1) {
+        error_setg(errp, "hypertrace: must have at least one client");
+        return;
+    }
+
+    hypertrace_init_config(&s->hconfig, s->max_clients);
+
+    error_setg(&s->migration_blocker,
+               "The 'hypertrace' device cannot be migrated");
+    migrate_add_blocker(s->migration_blocker);
+
+    pci_set_word(s->dev.config + PCI_COMMAND,
+                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
+
+    /* config channel */
+    memory_region_init_ram(&s->config, OBJECT(s), "hypertrace.config",
+                           TARGET_PAGE_SIZE, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->config);
+    s->config_ptr = qemu_map_ram_ptr(s->config.ram_block, 0);
+    config = s->config_ptr;
+    config->max_clients = tswap64(s->hconfig.max_clients);
+    config->client_args = tswap64(s->hconfig.client_args);
+    config->client_data_size = tswap64(s->hconfig.client_data_size);
+    config->control_size = tswap64(s->hconfig.control_size);
+    config->data_size = tswap64(s->hconfig.data_size);
+
+    /* data channel */
+    memory_region_init_ram(&s->data, OBJECT(s), "hypertrace.data",
+                           s->hconfig.data_size, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->data);
+    s->data_ptr = qemu_map_ram_ptr(s->data.ram_block, 0);
+
+    /* control channel */
+    memory_region_init_io(&s->control, OBJECT(s), &hypertrace_control_ops, s,
+                          "hypertrace.control", s->hconfig.control_size);
+    pci_register_bar(&s->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->control);
+    s->control_ptr = qemu_map_ram_ptr(s->control.ram_block, 0);
+}
+
+
+static Property hypertrace_properties[] = {
+    DEFINE_PROP_UINT64("max-clients", HypertraceState, max_clients, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void hypertrace_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->realize = hypertrace_realize;
+    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+    k->device_id = PCI_DEVICE_ID_HYPERTRACE;
+    k->class_id = PCI_CLASS_MEMORY_RAM;
+    dc->desc  = "Hypertrace communication channel",
+    dc->props = hypertrace_properties;
+}
+
+static TypeInfo hypertrace_info = {
+    .name          = "hypertrace",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(HypertraceState),
+    .class_init    = hypertrace_class_init,
+};
+
+static void hypertrace_register_types(void)
+{
+    type_register_static(&hypertrace_info);
+}
+
+type_init(hypertrace_register_types)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 772692f1b2..b162b582cf 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -84,6 +84,8 @@
 #define PCI_DEVICE_ID_VIRTIO_9P          0x1009
 #define PCI_DEVICE_ID_VIRTIO_VSOCK       0x1012
 
+#define PCI_DEVICE_ID_HYPERTRACE         0x10f0
+
 #define PCI_VENDOR_ID_REDHAT             0x1b36
 #define PCI_DEVICE_ID_REDHAT_BRIDGE      0x0001
 #define PCI_DEVICE_ID_REDHAT_SERIAL      0x0002

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v4 5/6] hypertrace: Add guest-side user-level library
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (3 preceding siblings ...)
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 4/6] hypertrace: [softmmu] " Lluís Vilanova
@ 2016-12-26 20:35 ` Lluís Vilanova
  2016-12-26 20:35 ` [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module Lluís Vilanova
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Lluís Vilanova @ 2016-12-26 20:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi

Provides guest library "libqemu-hypertrace-guest.a" to abstract access
to the hypertrace channel.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile                                |    6 +
 configure                               |    2 
 hypertrace/guest/user/Makefile          |   30 +++
 hypertrace/guest/user/common.c          |  296 +++++++++++++++++++++++++++++++
 hypertrace/guest/user/qemu-hypertrace.h |   80 ++++++++
 5 files changed, 413 insertions(+), 1 deletion(-)
 create mode 100644 hypertrace/guest/user/Makefile
 create mode 100644 hypertrace/guest/user/common.c
 create mode 100644 hypertrace/guest/user/qemu-hypertrace.h

diff --git a/Makefile b/Makefile
index 474cc5e66a..aa3fd4a79b 100644
--- a/Makefile
+++ b/Makefile
@@ -459,9 +459,13 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
 endif
 endif
 
+install-hypertrace:
+	$(INSTALL_DIR) "$(DESTDIR)$(includedir)"
+	$(INSTALL_DATA) "$(SRC_PATH)/hypertrace/guest/user/qemu-hypertrace.h" "$(DESTDIR)$(includedir)/"
+
 
 install: all $(if $(BUILD_DOCS),install-doc) \
-install-datadir install-localstatedir
+install-datadir install-localstatedir install-hypertrace
 ifneq ($(TOOLS),)
 	$(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
 endif
diff --git a/configure b/configure
index afc7071e77..4dfb9ff586 100755
--- a/configure
+++ b/configure
@@ -6021,6 +6021,8 @@ if [ "$TARGET_BASE_ARCH" = "" ]; then
 fi
 
 symlink "$source_path/Makefile.target" "$target_dir/Makefile"
+mkdir -p $target_dir/hypertrace/guest/user
+symlink $source_path/hypertrace/guest/user/Makefile $target_dir/hypertrace/guest/user/Makefile
 
 upper() {
     echo "$@"| LC_ALL=C tr '[a-z]' '[A-Z]'
diff --git a/hypertrace/guest/user/Makefile b/hypertrace/guest/user/Makefile
new file mode 100644
index 0000000000..85d08390db
--- /dev/null
+++ b/hypertrace/guest/user/Makefile
@@ -0,0 +1,30 @@
+include ../../../../config-host.mak
+include ../../../config-target.mak
+include $(SRC_PATH)/rules.mak
+
+vpath % $(SRC_PATH)/hypertrace/guest/user
+
+# do not use QEMU's per-host cflags when building guest code
+QEMU_CFLAGS  = -Werror -Wall
+
+QEMU_CFLAGS += $(GLIB_CFLAGS)
+QEMU_CFLAGS += -I$(SRC_PATH)/include
+QEMU_CFLAGS += -I../../../../linux-headers
+QEMU_CFLAGS += -I../../../../
+QEMU_CFLAGS += -I../../../
+
+ifdef CONFIG_SOFTMMU
+QEMU_CFLAGS += -DNEED_CPU_H
+QEMU_CFLAGS += -I$(SRC_PATH)/target-$(TARGET_BASE_ARCH)
+endif
+
+QEMU_CFLAGS += -g -O2
+
+obj-y = common.o
+
+libqemu-hypertrace-guest.a: $(obj-y)
+
+all: libqemu-hypertrace-guest.a
+
+clean:
+	rm -f $(obj-y) libqemu-hypertrace-guest.a
diff --git a/hypertrace/guest/user/common.c b/hypertrace/guest/user/common.c
new file mode 100644
index 0000000000..caa2bfce20
--- /dev/null
+++ b/hypertrace/guest/user/common.c
@@ -0,0 +1,296 @@
+/*
+ * Guest-side management of hypertrace.
+ *
+ * Copyright (C) 2016 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-hypertrace.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.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-host.h"
+#include "config-target.h"
+#if defined(CONFIG_SOFTMMU)
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#endif
+#include "hypertrace/common.h"
+
+static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+static char *config_path;
+static int config_fd = -1;
+static uint64_t *config_addr;
+static struct hypertrace_config *config;
+
+
+static char *data_path;
+static int data_fd = -1;
+static uint64_t *data_addr;
+
+static char *control_path;
+static int control_fd = -1;
+#if defined(CONFIG_USER_ONLY)
+static __thread uint64_t *control_addr;
+static __thread uint64_t *control_addr_1;
+#else
+static uint64_t *control_addr;
+#endif
+
+
+static int init_channel_file(const char *base, const char *suffix, size_t size,
+                             char **path, int *fd, uint64_t **addr, bool write)
+{
+    int prot;
+
+    *path = malloc(strlen(base) + strlen(suffix) + 1);
+    sprintf(*path, "%s%s", base, suffix);
+
+    prot = O_RDONLY;
+    if (write) {
+        prot = O_RDWR;
+    }
+    *fd = open(*path, prot);
+    if (*fd == -1) {
+        return -1;
+    }
+
+    prot = PROT_READ;
+    if (write) {
+        prot |= PROT_WRITE;
+    }
+    *addr = mmap(NULL, size, prot, 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 *res;
+    uint64_t vv = strtoull(v, &res, 16);
+    if (*res == '\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_HYPERTRACE) < 0) {
+            continue;
+        }
+
+        sprintf(tmp, "%s", path);
+        res = tmp;
+        break;
+    }
+
+    globfree(&g);
+
+    return res;
+}
+#endif
+
+int qemu_hypertrace_init(const char *base)
+{
+#if defined(CONFIG_USER_ONLY)
+    const char *config_suff = "-config";
+    const char *data_suff = "-data";
+    const char *control_suff = "-control";
+#elif defined(__linux__)
+    const char *config_suff = "/resource0";
+    const char *data_suff = "/resource1";
+    const char *control_suff = "/resource2";
+#else
+#error Unsupported OS
+#endif
+
+#if defined(CONFIG_USER_ONLY)
+    if (base == NULL) {
+        errno = ENOENT;
+        return -1;
+    }
+#elif defined(__linux__)
+    if (base == NULL) {
+        /* try to guess the base path */
+        base = find_device();
+        if (base == NULL) {
+            errno = ENOENT;
+            return -1;
+        }
+    }
+#endif
+
+    if (config_addr == NULL) {
+        int res;
+
+        if (pthread_mutex_lock(&init_mutex)) {
+            return -1;
+        }
+
+        res = init_channel_file(base, config_suff, getpagesize(),
+                                &config_path, &config_fd, &config_addr,
+                                false);
+        if (res != 0) {
+            return res;
+        }
+
+        config = (struct hypertrace_config *)config_addr;
+
+        if (pthread_mutex_unlock(&init_mutex)) {
+            return -1;
+        }
+    }
+
+    if (data_addr == NULL) {
+        int res;
+
+        if (pthread_mutex_lock(&init_mutex)) {
+            return -1;
+        }
+
+        res = init_channel_file(base, data_suff, config->data_size,
+                                &data_path, &data_fd, &data_addr,
+                                true);
+        if (res != 0) {
+            return res;
+        }
+
+        if (pthread_mutex_unlock(&init_mutex)) {
+            return -1;
+        }
+    }
+
+    if (control_addr == NULL) {
+        int res;
+        uint64_t control_size = config->control_size;
+
+        if (pthread_mutex_lock(&init_mutex)) {
+            return -1;
+        }
+
+        res = init_channel_file(base, control_suff, control_size,
+                                &control_path, &control_fd, &control_addr,
+                                true);
+        if (res != 0) {
+            return res;
+        }
+
+#if defined(CONFIG_USER_ONLY)
+        control_addr_1 = (uint64_t *)((char *)control_addr +
+                                      config->control_size / 2);
+#endif
+
+        if (pthread_mutex_unlock(&init_mutex)) {
+            return -1;
+        }
+    }
+
+    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_hypertrace_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_hypertrace_max_clients(void)
+{
+    return config->max_clients;
+}
+
+uint64_t qemu_hypertrace_num_args(void)
+{
+    return config->client_args;
+}
+
+uint64_t *qemu_hypertrace_data(uint64_t client)
+{
+    return &data_addr[client * CONFIG_HYPERTRACE_ARGS * sizeof(uint64_t)];
+}
+
+void qemu_hypertrace(uint64_t client, uint64_t arg1)
+{
+    uint64_t *ctrl = control_addr;
+    ctrl[client] = arg1;
+#if defined(CONFIG_USER_ONLY)
+    /* QEMU in 'user' mode uses two faulting pages to detect invocations */
+    ctrl = control_addr_1;
+    ctrl[client] = arg1;
+#endif
+}
diff --git a/hypertrace/guest/user/qemu-hypertrace.h b/hypertrace/guest/user/qemu-hypertrace.h
new file mode 100644
index 0000000000..aa78bb3355
--- /dev/null
+++ b/hypertrace/guest/user/qemu-hypertrace.h
@@ -0,0 +1,80 @@
+/*
+ * Guest-side management of hypertrace.
+ *
+ * Copyright (C) 2016 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>
+
+
+/**
+ * qemu_hypertrace_init:
+ * @base: Base path to the hypertrace channel.
+ *
+ * Initialize the hypertrace channel. The operation is idempotent, and must be
+ * called once per thread if running in QEMU's "user" mode.
+ *
+ * The base path to the hypertrace channel depends on the type of QEMU target:
+ *
+ * - User (single-application)
+ *   The base path provided when starting QEMU ("-hypertrace" commandline
+ *   option).
+ *
+ * - System (OS-dependant)
+ *   + Linux
+ *     The base path to the hypertrace channel virtual device; on a default QEMU
+ *     device setup for x86 this is "/sys/devices/pci0000:00/0000:00:04.0". If
+ *     NULL is provided, the hypertrace device will be automatically detected.
+ *
+ * Returns: Zero on success.
+ */
+int qemu_hypertrace_init(const char *base);
+
+/**
+ * qemu_hypertrace_fini:
+ *
+ * Deinitialize the hypertrace channel.
+ *
+ * Returns: Zero on success.
+ */
+int qemu_hypertrace_fini(void);
+
+/**
+ * qemu_hypertrace_max_clients:
+ *
+ * Maximum number of concurrent clients accepted by other calls.
+ */
+uint64_t qemu_hypertrace_max_clients(void);
+
+/**
+ * qemu_hypertrace_num_args:
+ *
+ * Number of uint64_t values read by each call to qemu_hypertrace().
+ */
+uint64_t qemu_hypertrace_num_args(void);
+
+/**
+ * qemu_hypertrace_data:
+ * @client: Client identifier.
+ *
+ * Pointer to the start of the data channel for the given client. Clients must
+ * write their arguments there (all but the first one).
+ */
+uint64_t *qemu_hypertrace_data(uint64_t client);
+
+/**
+ * qemu_hypertrace:
+ * @client: Client identifier.
+ * @arg1: First argument of the hypertrace event.
+ *
+ * Emit a hypertrace event.
+ *
+ * Each of the clients (e.g., thread) must use a different client identifier to
+ * ensure they can work concurrently without using locks (i.e., each uses a
+ * different portion of the data channel).
+ */
+void qemu_hypertrace(uint64_t client, uint64_t arg1);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (4 preceding siblings ...)
  2016-12-26 20:35 ` [Qemu-devel] [PATCH v4 5/6] hypertrace: Add guest-side user-level library Lluís Vilanova
@ 2016-12-26 20:35 ` Lluís Vilanova
  2017-01-09 16:17   ` Stefan Hajnoczi
  2016-12-26 20:47 ` [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel no-reply
  2017-01-09 16:28 ` Stefan Hajnoczi
  7 siblings, 1 reply; 21+ messages in thread
From: Lluís Vilanova @ 2016-12-26 20:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi

Provides guest Linux kernel module "qemu-hypertrace.ko" to abstract
access to the hypertrace channel.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile                                           |    4 -
 configure                                          |    4 +
 hypertrace/guest/linux-module/Kbuild.in            |    7 +
 hypertrace/guest/linux-module/Makefile             |   23 +++
 .../include/linux/qemu-hypertrace-internal.h       |   46 ++++++
 .../linux-module/include/linux/qemu-hypertrace.h   |   73 ++++++++++
 hypertrace/guest/linux-module/qemu-hypertrace.c    |  149 ++++++++++++++++++++
 7 files changed, 305 insertions(+), 1 deletion(-)
 create mode 100644 hypertrace/guest/linux-module/Kbuild.in
 create mode 100644 hypertrace/guest/linux-module/Makefile
 create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h
 create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h
 create mode 100644 hypertrace/guest/linux-module/qemu-hypertrace.c

diff --git a/Makefile b/Makefile
index aa3fd4a79b..1a18c570cf 100644
--- a/Makefile
+++ b/Makefile
@@ -460,8 +460,10 @@ endif
 endif
 
 install-hypertrace:
-	$(INSTALL_DIR) "$(DESTDIR)$(includedir)"
+	$(INSTALL_DIR) "$(DESTDIR)$(includedir)/linux"
 	$(INSTALL_DATA) "$(SRC_PATH)/hypertrace/guest/user/qemu-hypertrace.h" "$(DESTDIR)$(includedir)/"
+	$(INSTALL_DATA) "$(SRC_PATH)/hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h" "$(DESTDIR)$(includedir)/linux"
+	$(INSTALL_DATA) "$(SRC_PATH)/hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h" "$(DESTDIR)$(includedir)/linux"
 
 
 install: all $(if $(BUILD_DOCS),install-doc) \
diff --git a/configure b/configure
index 4dfb9ff586..28dd570033 100755
--- a/configure
+++ b/configure
@@ -6023,6 +6023,10 @@ fi
 symlink "$source_path/Makefile.target" "$target_dir/Makefile"
 mkdir -p $target_dir/hypertrace/guest/user
 symlink $source_path/hypertrace/guest/user/Makefile $target_dir/hypertrace/guest/user/Makefile
+if test "$target_softmmu" = "yes"; then
+    mkdir -p $target_dir/hypertrace/guest/linux-module
+    symlink $source_path/hypertrace/guest/linux-module/Makefile $target_dir/hypertrace/guest/linux-module/Makefile
+fi
 
 upper() {
     echo "$@"| LC_ALL=C tr '[a-z]' '[A-Z]'
diff --git a/hypertrace/guest/linux-module/Kbuild.in b/hypertrace/guest/linux-module/Kbuild.in
new file mode 100644
index 0000000000..56abaeac8c
--- /dev/null
+++ b/hypertrace/guest/linux-module/Kbuild.in
@@ -0,0 +1,7 @@
+# -*- mode: makefile -*-
+
+COMMON_H = $(shell readlink -f $(MOD_SRC_PATH)/../../common.h)
+
+src = $(MOD_SRC_PATH)
+ccflags-y := -I$(src)/include/ -DCOMMON_H=$(COMMON_H)
+obj-m := qemu-hypertrace.o
diff --git a/hypertrace/guest/linux-module/Makefile b/hypertrace/guest/linux-module/Makefile
new file mode 100644
index 0000000000..b30cdeb00b
--- /dev/null
+++ b/hypertrace/guest/linux-module/Makefile
@@ -0,0 +1,23 @@
+include ../../../../config-host.mak
+include ../../../config-target.mak
+include $(SRC_PATH)/rules.mak
+
+MOD_SRC_PATH = $(SRC_PATH)/hypertrace/guest/linux-module
+
+LINUX_BUILD_PATH = /lib/modules/$(shell uname -r)/build
+
+vpath % $(MOD_SRC_PATH)
+
+
+all: Kbuild
+	$(MAKE) -C $(LINUX_BUILD_PATH) M=$(shell pwd) \
+	  MOD_SRC_PATH=$(MOD_SRC_PATH) \
+	  modules
+
+clean: Kbuild
+	$(MAKE) -C $(LINUX_BUILD_PATH) M=$(shell pwd) clean
+	rm -f $<
+
+Kbuild: $(MOD_SRC_PATH)/Kbuild.in Makefile
+	rm -f $@
+	cp $< $@
diff --git a/hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h b/hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h
new file mode 100644
index 0000000000..c439107efc
--- /dev/null
+++ b/hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h
@@ -0,0 +1,46 @@
+/* -*- C -*-
+ *
+ * Guest-side management of hypertrace.
+ *
+ * Copyright (C) 2016 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+extern uint64_t _qemu_hypertrace_channel_max_clients;
+extern uint64_t _qemu_hypertrace_channel_num_args;
+extern uint64_t *_qemu_hypertrace_channel_data;
+extern uint64_t *_qemu_hypertrace_channel_control;
+
+static inline uint64_t qemu_hypertrace_max_clients(void)
+{
+    return _qemu_hypertrace_channel_max_clients;
+}
+
+static inline uint64_t qemu_hypertrace_num_args(void)
+{
+    return _qemu_hypertrace_channel_num_args;
+}
+
+static inline uint64_t *qemu_hypertrace_data(uint64_t client)
+{
+    size_t args_size = qemu_hypertrace_num_args() * sizeof(uint64_t);
+    return &_qemu_hypertrace_channel_data[client * args_size];
+}
+
+static inline void qemu_hypertrace(uint64_t client, uint64_t arg1)
+{
+    uint64_t *ctrlp = _qemu_hypertrace_channel_control;
+    ctrlp[client] = arg1;
+}
diff --git a/hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h b/hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h
new file mode 100644
index 0000000000..f4b70c90d2
--- /dev/null
+++ b/hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h
@@ -0,0 +1,73 @@
+/* -*- C -*-
+ *
+ * Guest-side management of hypertrace.
+ *
+ * Copyright (C) 2016 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#ifndef QEMU_HYPERTRACE_H
+#define QEMU_HYPERTRACE_H
+
+#include <linux/types.h>
+
+
+/**
+ * qemu_hypertrace_max_clients:
+ *
+ * Maximum number of concurrent clients accepted by other calls.
+ */
+static uint64_t qemu_hypertrace_max_clients(void);
+
+/**
+ * qemu_hypertrace_num_args:
+ *
+ * Number of uint64_t values read by each call to qemu_hypertrace().
+ */
+static uint64_t qemu_hypertrace_num_args(void);
+
+/**
+ * qemu_hypertrace_data:
+ * @client: Client identifier.
+ *
+ * Pointer to the start of the data channel for the given client. Clients must
+ * write their arguments there (all but the first one).
+ */
+static uint64_t *qemu_hypertrace_data(uint64_t client);
+
+/**
+ * qemu_hypertrace:
+ * @client: Client identifier.
+ * @arg1: First argument of the hypertrace event.
+ *
+ * Emit a hypertrace event.
+ *
+ * Each of the clients (e.g., CPU) must use a different client identifier to
+ * ensure they can work concurrently without using locks (i.e., each uses a
+ * different portion of the data channel).
+ *
+ * Note: You should use wmb() before writing into the control channel iff you
+ * have written into the data channel.
+ *
+ * Note: Use preempt_disable() and preempt_enable() if you're using data offsets
+ * based on the CPU identifiers (or else data might be mixed if a task is
+ * re-scheduled).
+ */
+static void qemu_hypertrace(uint64_t client, uint64_t arg1);
+
+
+#include <linux/qemu-hypertrace-internal.h>
+
+#endif  /* QEMU_HYPERTRACE_H */
diff --git a/hypertrace/guest/linux-module/qemu-hypertrace.c b/hypertrace/guest/linux-module/qemu-hypertrace.c
new file mode 100644
index 0000000000..5f16ef00c2
--- /dev/null
+++ b/hypertrace/guest/linux-module/qemu-hypertrace.c
@@ -0,0 +1,149 @@
+/*
+ * Guest-side management of hypertrace.
+ *
+ * Copyright (C) 2016 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#include <linux/qemu-hypertrace.h>
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/uaccess.h>
+
+#define xstr(s) #s
+#define str(s) xstr(s)
+#include str(COMMON_H)
+
+
+#define VERSION_STR "0.1"
+#define PCI_VENDOR_ID_REDHAT_QUMRANET    0x1af4
+#define PCI_DEVICE_ID_HYPERTRACE         0x10f0
+
+
+MODULE_DESCRIPTION("Kernel interface to QEMU's hypertrace device");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Lluís Vilanova");
+MODULE_VERSION(VERSION_STR);
+
+
+/*********************************************************************
+ * Kernel interface
+ */
+
+uint64_t _qemu_hypertrace_channel_max_clients;
+uint64_t _qemu_hypertrace_channel_num_args;
+static uint64_t *_qemu_hypertrace_channel_config;
+uint64_t *_qemu_hypertrace_channel_data;
+uint64_t *_qemu_hypertrace_channel_control;
+
+EXPORT_SYMBOL(_qemu_hypertrace_channel_max_clients);
+EXPORT_SYMBOL(_qemu_hypertrace_channel_num_args);
+EXPORT_SYMBOL(_qemu_hypertrace_channel_data);
+EXPORT_SYMBOL(_qemu_hypertrace_channel_control);
+
+
+/*********************************************************************
+ * Channel initialization
+ */
+
+static int init_channel(uint64_t **vaddr, struct pci_dev *dev, int bar)
+{
+    void *res;
+    resource_size_t start, size;
+
+    start = pci_resource_start(dev, bar);
+    size = pci_resource_len(dev, bar);
+
+    if (start == 0 || size == 0) {
+        return -ENOENT;
+    }
+
+    res = ioremap(start, size);
+    if (res == 0) {
+        return -EINVAL;
+    }
+
+    *vaddr = res;
+    return 0;
+}
+
+/*********************************************************************
+ * Module (de)initialization
+ */
+
+int init_module(void)
+{
+    int res = 0;
+    struct pci_dev *dev = NULL;
+    struct hypertrace_config *config;
+
+    printk(KERN_NOTICE "Loading QEMU hypertrace module (version %s)\n",
+           VERSION_STR);
+
+    dev = pci_get_device(PCI_VENDOR_ID_REDHAT_QUMRANET,
+                         PCI_DEVICE_ID_HYPERTRACE,
+                         NULL);
+    if (dev == NULL) {
+        res = -ENOENT;
+        printk(KERN_ERR "Unable to find hypertrace device\n");
+        goto error;
+    }
+
+    res = init_channel(&_qemu_hypertrace_channel_config, dev, 0);
+    if (res != 0) {
+        printk(KERN_ERR "Unable to find hypertrace config channel\n");
+        goto error;
+    }
+
+    config = (struct hypertrace_config *)_qemu_hypertrace_channel_config;
+    _qemu_hypertrace_channel_max_clients = config->max_clients;
+    _qemu_hypertrace_channel_num_args = config->client_args;
+
+    res = init_channel(&_qemu_hypertrace_channel_data, dev, 1);
+    if (res != 0) {
+        printk(KERN_ERR "Unable to find hypertrace data channel\n");
+        goto error_config;
+    }
+
+    res = init_channel(&_qemu_hypertrace_channel_control, dev, 2);
+    if (res != 0) {
+        printk(KERN_ERR "Unable to find hypertrace control channel\n");
+        goto error_data;
+    }
+
+    goto ok;
+
+error_data:
+    iounmap(_qemu_hypertrace_channel_data);
+
+error_config:
+    iounmap(_qemu_hypertrace_channel_config);
+    _qemu_hypertrace_channel_config = NULL;
+    _qemu_hypertrace_channel_data = NULL;
+
+error:
+ok:
+    return res;
+}
+
+void cleanup_module(void)
+{
+    printk(KERN_NOTICE "Unloading QEMU hypertrace module\n");
+
+    iounmap(_qemu_hypertrace_channel_config);
+    iounmap(_qemu_hypertrace_channel_data);
+    iounmap(_qemu_hypertrace_channel_control);
+}

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (5 preceding siblings ...)
  2016-12-26 20:35 ` [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module Lluís Vilanova
@ 2016-12-26 20:47 ` no-reply
  2017-01-09 16:28 ` Stefan Hajnoczi
  7 siblings, 0 replies; 21+ messages in thread
From: no-reply @ 2016-12-26 20:47 UTC (permalink / raw)
  To: vilanova; +Cc: famz, qemu-devel, stefanha, lcapitulino

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel
Message-id: 148278447806.8988.12706825771606357198.stgit@fimbulvetr.bsc.es

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/148278447806.8988.12706825771606357198.stgit@fimbulvetr.bsc.es -> patchew/148278447806.8988.12706825771606357198.stgit@fimbulvetr.bsc.es
Switched to a new branch 'test'
fb46318 hypertrace: Add guest-side Linux module
499a17f hypertrace: Add guest-side user-level library
9eb68af hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event
be70f02 hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
7cc5978 hypertrace: Add tracing event "guest_hypertrace"
8d71b28 hypertrace: Add documentation

=== OUTPUT BEGIN ===
Checking PATCH 1/6: hypertrace: Add documentation...
Checking PATCH 2/6: hypertrace: Add tracing event "guest_hypertrace"...
Checking PATCH 3/6: hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event...
WARNING: architecture specific defines should be avoided
#306: FILE: hypertrace/common.h:10:
+#if !defined(__KERNEL__)

total: 0 errors, 1 warnings, 831 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/6: hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event...
Checking PATCH 5/6: hypertrace: Add guest-side user-level library...
WARNING: architecture specific defines should be avoided
#171: FILE: hypertrace/guest/user/common.c:83:
+#if !defined(CONFIG_USER_ONLY) && defined(__linux__)

ERROR: consider using qemu_strtoull in preference to strtoull
#191: FILE: hypertrace/guest/user/common.c:103:
+    uint64_t vv = strtoull(v, &res, 16);

total: 1 errors, 1 warnings, 428 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 6/6: hypertrace: Add guest-side Linux module...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
@ 2017-01-09 15:44   ` Stefan Hajnoczi
  2017-01-16 17:05     ` Lluís Vilanova
  2017-01-16 17:10     ` Lluís Vilanova
  2017-01-09 16:35   ` Stefan Hajnoczi
  1 sibling, 2 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-09 15:44 UTC (permalink / raw)
  To: Lluís Vilanova
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake, Riku Voipio

[-- Attachment #1: Type: text/plain, Size: 4860 bytes --]

On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
> @@ -847,6 +855,10 @@ int main(int argc, char **argv)
>          } else if (!strcmp(r, "trace")) {
>              g_free(trace_file);
>              trace_file = trace_opt_parse(optarg);
> +        } else if (!strcmp(r, "hypertrace")) {
> +            g_free(hypertrace_file);

This variable hasn't been declared yet.  Perhaps it's in a later patch.
Please reorder things to avoid the compilation error.

Or was this supposed to be hypertrace_base?

> +void hypertrace_init_config(struct hypertrace_config *config,
> +                            unsigned int max_clients)
> +{
> +    config->max_clients = max_clients;
> +    config->client_args = CONFIG_HYPERTRACE_ARGS;
> +    config->client_data_size = config->client_args * sizeof(uint64_t);
> +    config->control_size = QEMU_ALIGN_UP(
> +        config->max_clients * sizeof(uint64_t), TARGET_PAGE_SIZE);

This needs to be host page size aligned, too.  Otherwise protect will
affect bytes beyond the end of the control region.

> +static void init_channel(const char *base, const char *suffix, size_t size,
> +                         char **path, int *fd, uint64_t **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) {
> +        error_report("error: open(%s): %s", *path, strerror(errno));
> +        abort();
> +    }

open() can fail for reasons outside QEMU's control.  This isn't an
internal error.  Please exit cleanly instead of using abort(3).

> +void hypertrace_init(const char *base, unsigned int max_clients)
> +{
> +    struct sigaction sigint;
> +    struct hypertrace_config *pconfig;
> +
> +    if (base == NULL) {
> +        return;
> +    }
> +
> +    memset(&sigint, 0, sizeof(sigint));
> +    sigint.sa_sigaction = fini_handler;
> +    sigint.sa_flags = SA_SIGINFO | SA_RESTART;
> +    if (sigaction(SIGINT, &sigint, NULL) != 0) {
> +        error_report("error: sigaction(SIGINT): %s", strerror(errno));
> +        abort();
> +    }
> +    if (sigaction(SIGABRT, &sigint, NULL) != 0) {
> +        error_report("error: sigaction(SIGABRT): %s", strerror(errno));
> +        abort();
> +    }

I don't know whether it's okay to set up signal handlers in user mode.

Will this break guest code SIGINT/SIGABRT handling?

> +bool hypertrace_guest_mmap_check(int fd, unsigned long len,
> +                                 unsigned long offset)
> +{
> +    struct stat s;
> +    if (fstat(fd, &s) < 0) {
> +        return true;

Should this be return false?

> +    }
> +
> +    if (s.st_dev != control_fd_stat.st_dev ||
> +        s.st_ino != control_fd_stat.st_ino) {
> +        return true;

Here too.

> +static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
> +{
> +    CPUState *vcpu = current_cpu;
> +    void *control_0 = vcpu->hypertrace_control;
> +    void *control_1 = vcpu->hypertrace_control + config.control_size / 2;
> +    void *control_2 = control_1 + config.control_size / 2;
> +
> +    if (control_0 <= siginfo->si_addr && siginfo->si_addr < control_1) {
> +
> +        /* 1st fault (guest will write cmd) */
> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);

Please use uintptr_t instead of unsigned long.  It's more portable
because it doesn't assume sizeof(void*) == sizeof(unsigned long).  On
Windows the 64-bit data model is LLP64, not LP64:
https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models

> +        swap_control(control_0, control_1);
> +
> +    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
> +        size_t client = (siginfo->si_addr - control_1) / sizeof(uint64_t);
> +        uint64_t vcontrol = ((uint64_t *)control_0)[client];
> +        uint64_t *data_ptr = &qemu_data[client * config.client_data_size];

Is byte swapping required?

> +
> +        /* 2nd fault (invoke) */
> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
> +        hypertrace_emit(current_cpu, vcontrol, data_ptr);
> +        swap_control(control_1, control_0);

I don't understand how this scheme works for multi-threaded programs.
If two threads are both writing at the same time can we miss events due
to swap_control() changing mprotect?

> +
> +    } 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);
> +        }

Is there a case when no signal handler was installed (i.e. default
action)?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/6] hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 4/6] hypertrace: [softmmu] " Lluís Vilanova
@ 2017-01-09 16:12   ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-09 16:12 UTC (permalink / raw)
  To: Lluís Vilanova
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake,
	Michael S. Tsirkin, Marcel Apfelbaum

[-- Attachment #1: Type: text/plain, Size: 131 bytes --]

On Mon, Dec 26, 2016 at 09:34:59PM +0100, Lluís Vilanova wrote:
> +#define PAGE_SIZE TARGET_PAGE_SIZE

Unused.  Please remove.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module
  2016-12-26 20:35 ` [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module Lluís Vilanova
@ 2017-01-09 16:17   ` Stefan Hajnoczi
  2017-01-15  2:10     ` Lluís Vilanova
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-09 16:17 UTC (permalink / raw)
  To: Lluís Vilanova
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

On Mon, Dec 26, 2016 at 09:35:10PM +0100, Lluís Vilanova wrote:
> Provides guest Linux kernel module "qemu-hypertrace.ko" to abstract
> access to the hypertrace channel.
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  Makefile                                           |    4 -
>  configure                                          |    4 +
>  hypertrace/guest/linux-module/Kbuild.in            |    7 +
>  hypertrace/guest/linux-module/Makefile             |   23 +++
>  .../include/linux/qemu-hypertrace-internal.h       |   46 ++++++
>  .../linux-module/include/linux/qemu-hypertrace.h   |   73 ++++++++++
>  hypertrace/guest/linux-module/qemu-hypertrace.c    |  149 ++++++++++++++++++++
>  7 files changed, 305 insertions(+), 1 deletion(-)
>  create mode 100644 hypertrace/guest/linux-module/Kbuild.in
>  create mode 100644 hypertrace/guest/linux-module/Makefile
>  create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h
>  create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h
>  create mode 100644 hypertrace/guest/linux-module/qemu-hypertrace.c

This should be submitted to Linux, the module cannot live in the QEMU
tree (we don't carry out-of-tree kernel modules).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel
  2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (6 preceding siblings ...)
  2016-12-26 20:47 ` [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel no-reply
@ 2017-01-09 16:28 ` Stefan Hajnoczi
  7 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-09 16:28 UTC (permalink / raw)
  To: Lluís Vilanova
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On Mon, Dec 26, 2016 at 09:34:38PM +0100, Lluís Vilanova wrote:
> The hypertrace channel allows guest code to emit events in QEMU (the host) using
> its tracing infrastructure (see "docs/trace.txt"). This works in both 'system'
> and 'user' modes, is architecture-agnostic and introduces minimal noise on the
> guest.
> 
> See first commit for a full description, use-cases and an example.

As mentioned previously, the -system mode "1 vmexit per trace event"
approach is too heavyweight for for performance analysis or high
frequency tracing.  It might be useful for things like system bringup or
TCG development though.

The -user approach is more promising because overheads are lower when
guest code really just jumps into QEMU code.

I have left comments on the patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
  2017-01-09 15:44   ` Stefan Hajnoczi
@ 2017-01-09 16:35   ` Stefan Hajnoczi
  2017-01-09 18:20     ` Lluís Vilanova
  1 sibling, 1 reply; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-09 16:35 UTC (permalink / raw)
  To: Lluís Vilanova
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake, Riku Voipio

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
> +static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
> +{
> +    CPUState *vcpu = current_cpu;
> +    void *control_0 = vcpu->hypertrace_control;
> +    void *control_1 = vcpu->hypertrace_control + config.control_size / 2;
> +    void *control_2 = control_1 + config.control_size / 2;
> +
> +    if (control_0 <= siginfo->si_addr && siginfo->si_addr < control_1) {
> +
> +        /* 1st fault (guest will write cmd) */
> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
> +        swap_control(control_0, control_1);
> +
> +    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
> +        size_t client = (siginfo->si_addr - control_1) / sizeof(uint64_t);
> +        uint64_t vcontrol = ((uint64_t *)control_0)[client];
> +        uint64_t *data_ptr = &qemu_data[client * config.client_data_size];
> +
> +        /* 2nd fault (invoke) */
> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
> +        hypertrace_emit(current_cpu, vcontrol, data_ptr);
> +        swap_control(control_1, control_0);

A simpler and faster approach is to permanently mprotect just one region
and load all arguments from data[] (including the first argument).  Then
swapping isn't necessary.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2017-01-09 16:35   ` Stefan Hajnoczi
@ 2017-01-09 18:20     ` Lluís Vilanova
  2017-01-10 14:47       ` Stefan Hajnoczi
  0 siblings, 1 reply; 21+ messages in thread
From: Lluís Vilanova @ 2017-01-09 18:20 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Riku Voipio, qemu-devel, Luiz Capitulino

Stefan Hajnoczi writes:

> On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
>> +static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
>> +{
>> +    CPUState *vcpu = current_cpu;
>> +    void *control_0 = vcpu->hypertrace_control;
>> +    void *control_1 = vcpu->hypertrace_control + config.control_size / 2;
>> +    void *control_2 = control_1 + config.control_size / 2;
>> +
>> +    if (control_0 <= siginfo->si_addr && siginfo->si_addr < control_1) {
>> +
>> +        /* 1st fault (guest will write cmd) */
>> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
>> +        swap_control(control_0, control_1);
>> +
>> +    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
>> +        size_t client = (siginfo->si_addr - control_1) / sizeof(uint64_t);
>> +        uint64_t vcontrol = ((uint64_t *)control_0)[client];
>> +        uint64_t *data_ptr = &qemu_data[client * config.client_data_size];
>> +
>> +        /* 2nd fault (invoke) */
>> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
>> +        hypertrace_emit(current_cpu, vcontrol, data_ptr);
>> +        swap_control(control_1, control_0);

> A simpler and faster approach is to permanently mprotect just one region
> and load all arguments from data[] (including the first argument).  Then
> swapping isn't necessary.

I'm don't understand what you propose.

With a single protected region, you don't know when to restore protection of it
so that later accesses will be detected too. That could be solved if we used
single-stepping (maybe that's what you meant):

* trap access
* unprotect memory region
* single-step guest
* read written data and emit event
* protect memory region again
* resume guest

If the single-stepping can be done without too much complexity, that'd be a
faster option, and that piece of code might be cleaner too.

We could only avoid the protect/unprotect sequence if we added target-specific
code to "skip" the failed instruction (assuming all useful writes go to the data
channel), but I wanted to make all code target-agnostic.


Cheers,
  Lluis

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2017-01-09 18:20     ` Lluís Vilanova
@ 2017-01-10 14:47       ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-10 14:47 UTC (permalink / raw)
  To: Riku Voipio, qemu-devel, Luiz Capitulino

[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]

On Mon, Jan 09, 2017 at 07:20:07PM +0100, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> 
> > On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
> >> +static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
> >> +{
> >> +    CPUState *vcpu = current_cpu;
> >> +    void *control_0 = vcpu->hypertrace_control;
> >> +    void *control_1 = vcpu->hypertrace_control + config.control_size / 2;
> >> +    void *control_2 = control_1 + config.control_size / 2;
> >> +
> >> +    if (control_0 <= siginfo->si_addr && siginfo->si_addr < control_1) {
> >> +
> >> +        /* 1st fault (guest will write cmd) */
> >> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
> >> +        swap_control(control_0, control_1);
> >> +
> >> +    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
> >> +        size_t client = (siginfo->si_addr - control_1) / sizeof(uint64_t);
> >> +        uint64_t vcontrol = ((uint64_t *)control_0)[client];
> >> +        uint64_t *data_ptr = &qemu_data[client * config.client_data_size];
> >> +
> >> +        /* 2nd fault (invoke) */
> >> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
> >> +        hypertrace_emit(current_cpu, vcontrol, data_ptr);
> >> +        swap_control(control_1, control_0);
> 
> > A simpler and faster approach is to permanently mprotect just one region
> > and load all arguments from data[] (including the first argument).  Then
> > swapping isn't necessary.
> 
> I'm don't understand what you propose.
> 
> With a single protected region, you don't know when to restore protection of it
> so that later accesses will be detected too. That could be solved if we used
> single-stepping (maybe that's what you meant):
> 
> * trap access
> * unprotect memory region
> * single-step guest
> * read written data and emit event
> * protect memory region again
> * resume guest
> 
> If the single-stepping can be done without too much complexity, that'd be a
> faster option, and that piece of code might be cleaner too.
> 
> We could only avoid the protect/unprotect sequence if we added target-specific
> code to "skip" the failed instruction (assuming all useful writes go to the data
> channel), but I wanted to make all code target-agnostic.

Okay, I didn't realize the instruction would be restarted.  I thought
swapping was solely to allow the guest to write vcontrol =
control_0[client].

Assuming for a second that the instruction isn't restarted, my
suggestion was to use si_addr to identify which client and then load all
args out of the (read/write) data region.  This eliminates vcontrol.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module
  2017-01-09 16:17   ` Stefan Hajnoczi
@ 2017-01-15  2:10     ` Lluís Vilanova
  2017-01-16 10:37       ` Stefan Hajnoczi
  0 siblings, 1 reply; 21+ messages in thread
From: Lluís Vilanova @ 2017-01-15  2:10 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel, Luiz Capitulino

Stefan Hajnoczi writes:

> On Mon, Dec 26, 2016 at 09:35:10PM +0100, Lluís Vilanova wrote:
>> Provides guest Linux kernel module "qemu-hypertrace.ko" to abstract
>> access to the hypertrace channel.
>> 
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> Makefile                                           |    4 -
>> configure                                          |    4 +
>> hypertrace/guest/linux-module/Kbuild.in            |    7 +
>> hypertrace/guest/linux-module/Makefile             |   23 +++
>> .../include/linux/qemu-hypertrace-internal.h       |   46 ++++++
>> .../linux-module/include/linux/qemu-hypertrace.h   |   73 ++++++++++
>> hypertrace/guest/linux-module/qemu-hypertrace.c    |  149 ++++++++++++++++++++
>> 7 files changed, 305 insertions(+), 1 deletion(-)
>> create mode 100644 hypertrace/guest/linux-module/Kbuild.in
>> create mode 100644 hypertrace/guest/linux-module/Makefile
>> create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h
>> create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h
>> create mode 100644 hypertrace/guest/linux-module/qemu-hypertrace.c

> This should be submitted to Linux, the module cannot live in the QEMU
> tree (we don't carry out-of-tree kernel modules).

I thought it made sense to have it here, the same way we have the user-mode
guest library. If not, I will split it into a separate repo and point at it from
the documentation file. Once the rest is merged into QEMU, I can post the kernel
module repo to the linux list.

Thanks,
  LLuis

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module
  2017-01-15  2:10     ` Lluís Vilanova
@ 2017-01-16 10:37       ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-16 10:37 UTC (permalink / raw)
  To: qemu-devel, Luiz Capitulino

[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]

On Sun, Jan 15, 2017 at 03:10:13AM +0100, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> > On Mon, Dec 26, 2016 at 09:35:10PM +0100, Lluís Vilanova wrote:
> >> Provides guest Linux kernel module "qemu-hypertrace.ko" to abstract
> >> access to the hypertrace channel.
> >> 
> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> >> ---
> >> Makefile                                           |    4 -
> >> configure                                          |    4 +
> >> hypertrace/guest/linux-module/Kbuild.in            |    7 +
> >> hypertrace/guest/linux-module/Makefile             |   23 +++
> >> .../include/linux/qemu-hypertrace-internal.h       |   46 ++++++
> >> .../linux-module/include/linux/qemu-hypertrace.h   |   73 ++++++++++
> >> hypertrace/guest/linux-module/qemu-hypertrace.c    |  149 ++++++++++++++++++++
> >> 7 files changed, 305 insertions(+), 1 deletion(-)
> >> create mode 100644 hypertrace/guest/linux-module/Kbuild.in
> >> create mode 100644 hypertrace/guest/linux-module/Makefile
> >> create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h
> >> create mode 100644 hypertrace/guest/linux-module/include/linux/qemu-hypertrace.h
> >> create mode 100644 hypertrace/guest/linux-module/qemu-hypertrace.c
> 
> > This should be submitted to Linux, the module cannot live in the QEMU
> > tree (we don't carry out-of-tree kernel modules).
> 
> I thought it made sense to have it here, the same way we have the user-mode
> guest library. If not, I will split it into a separate repo and point at it from
> the documentation file. Once the rest is merged into QEMU, I can post the kernel
> module repo to the linux list.

Linux doesn't have a stable kernel module API.  Out-of-tree modules
bitrot if they are not actively rebased.  Getting the module into
linux.git avoids these problems.

The Linux maintainers may give you feedback that requires changes to the
current design, so please send the patch to them now instead of waiting.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2017-01-09 15:44   ` Stefan Hajnoczi
@ 2017-01-16 17:05     ` Lluís Vilanova
  2017-01-17  9:46       ` Stefan Hajnoczi
  2017-01-16 17:10     ` Lluís Vilanova
  1 sibling, 1 reply; 21+ messages in thread
From: Lluís Vilanova @ 2017-01-16 17:05 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake, Riku Voipio

Stefan Hajnoczi writes:

> On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
>> @@ -847,6 +855,10 @@ int main(int argc, char **argv)
>> } else if (!strcmp(r, "trace")) {
>> g_free(trace_file);
>> trace_file = trace_opt_parse(optarg);
>> +        } else if (!strcmp(r, "hypertrace")) {
>> +            g_free(hypertrace_file);

> This variable hasn't been declared yet.  Perhaps it's in a later patch.
> Please reorder things to avoid the compilation error.

> Or was this supposed to be hypertrace_base?

It's hypertrace_base, yes.


>> +void hypertrace_init_config(struct hypertrace_config *config,
>> +                            unsigned int max_clients)
>> +{
>> +    config->max_clients = max_clients;
>> +    config->client_args = CONFIG_HYPERTRACE_ARGS;
>> +    config->client_data_size = config->client_args * sizeof(uint64_t);
>> +    config->control_size = QEMU_ALIGN_UP(
>> +        config->max_clients * sizeof(uint64_t), TARGET_PAGE_SIZE);

> This needs to be host page size aligned, too.  Otherwise protect will
> affect bytes beyond the end of the control region.

Ummm, so right. Although I think only host page alignment is required (there's
no soft TLB in user-mode, right?).


>> +static void init_channel(const char *base, const char *suffix, size_t size,
>> +                         char **path, int *fd, uint64_t **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) {
>> +        error_report("error: open(%s): %s", *path, strerror(errno));
>> +        abort();
>> +    }

> open() can fail for reasons outside QEMU's control.  This isn't an
> internal error.  Please exit cleanly instead of using abort(3).

By cleanly you mean exit with a non-zero code, right? It still is an error that
cannot be recovered.

Also, if this goes with exit() what about the abort()s I have added in other
places? (e.g., on a failed call to sigaction)


>> +void hypertrace_init(const char *base, unsigned int max_clients)
>> +{
>> +    struct sigaction sigint;
>> +    struct hypertrace_config *pconfig;
>> +
>> +    if (base == NULL) {
>> +        return;
>> +    }
>> +
>> +    memset(&sigint, 0, sizeof(sigint));
>> +    sigint.sa_sigaction = fini_handler;
>> +    sigint.sa_flags = SA_SIGINFO | SA_RESTART;
>> +    if (sigaction(SIGINT, &sigint, NULL) != 0) {
>> +        error_report("error: sigaction(SIGINT): %s", strerror(errno));
>> +        abort();
>> +    }
>> +    if (sigaction(SIGABRT, &sigint, NULL) != 0) {
>> +        error_report("error: sigaction(SIGABRT): %s", strerror(errno));
>> +        abort();
>> +    }

> I don't know whether it's okay to set up signal handlers in user mode.

> Will this break guest code SIGINT/SIGABRT handling?

Yes, I should reflect the signal back to the guest.


>> +bool hypertrace_guest_mmap_check(int fd, unsigned long len,
>> +                                 unsigned long offset)
>> +{
>> +    struct stat s;
>> +    if (fstat(fd, &s) < 0) {
>> +        return true;

> Should this be return false?

>> +    }
>> +
>> +    if (s.st_dev != control_fd_stat.st_dev ||
>> +        s.st_ino != control_fd_stat.st_ino) {
>> +        return true;

> Here too.

Yes, that's so embarrassing.


>> +static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
>> +{
>> +    CPUState *vcpu = current_cpu;
>> +    void *control_0 = vcpu->hypertrace_control;
>> +    void *control_1 = vcpu->hypertrace_control + config.control_size / 2;
>> +    void *control_2 = control_1 + config.control_size / 2;
>> +
>> +    if (control_0 <= siginfo->si_addr && siginfo->si_addr < control_1) {
>> +
>> +        /* 1st fault (guest will write cmd) */
>> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);

> Please use uintptr_t instead of unsigned long.  It's more portable
> because it doesn't assume sizeof(void*) == sizeof(unsigned long).  On
> Windows the 64-bit data model is LLP64, not LP64:
> https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models

Got it.


>> +        swap_control(control_0, control_1);
>> +
>> +    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
>> +        size_t client = (siginfo->si_addr - control_1) / sizeof(uint64_t);
>> +        uint64_t vcontrol = ((uint64_t *)control_0)[client];
>> +        uint64_t *data_ptr = &qemu_data[client * config.client_data_size];

> Is byte swapping required?

Ummmm, these are values passed to the trace emitter, so either is "correct".

But for the sake of people's sanity when looking at traces, it might be better
to always swap to host endianness.


>> +
>> +        /* 2nd fault (invoke) */
>> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
>> +        hypertrace_emit(current_cpu, vcontrol, data_ptr);
>> +        swap_control(control_1, control_0);

> I don't understand how this scheme works for multi-threaded programs.
> If two threads are both writing at the same time can we miss events due
> to swap_control() changing mprotect?

Dang, with the version changes I forgot to add the per-client padding to place
each on a separate page.


>> +
>> +    } 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);
>> +        }

> Is there a case when no signal handler was installed (i.e. default
> action)?

Yes, before calling hypertrace_init() or if it is called without a
"hypertrace_base" argument set (i.e., the user has not enabled hypertrace in the
command line).


Thanks,
  Lluis

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2017-01-09 15:44   ` Stefan Hajnoczi
  2017-01-16 17:05     ` Lluís Vilanova
@ 2017-01-16 17:10     ` Lluís Vilanova
  1 sibling, 0 replies; 21+ messages in thread
From: Lluís Vilanova @ 2017-01-16 17:10 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake, Riku Voipio

Stefan Hajnoczi writes:

> On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
[...]
>> +
>> +    } 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);
>> +        }

> Is there a case when no signal handler was installed (i.e. default
> action)?

Sorry, in my previous email I meant no, there is none. If hypertrace is not
initialized, the segv handler will never be installed; otherwise segv_next will
always have one of the two values.


Thanks,
  Lluis

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2017-01-16 17:05     ` Lluís Vilanova
@ 2017-01-17  9:46       ` Stefan Hajnoczi
  2017-01-17 23:35         ` Lluís Vilanova
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-17  9:46 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel, Daniel P Berrange, Luiz Capitulino,
	Eric Blake, Riku Voipio

[-- Attachment #1: Type: text/plain, Size: 3135 bytes --]

On Mon, Jan 16, 2017 at 06:05:26PM +0100, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> > On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
> >> +void hypertrace_init_config(struct hypertrace_config *config,
> >> +                            unsigned int max_clients)
> >> +{
> >> +    config->max_clients = max_clients;
> >> +    config->client_args = CONFIG_HYPERTRACE_ARGS;
> >> +    config->client_data_size = config->client_args * sizeof(uint64_t);
> >> +    config->control_size = QEMU_ALIGN_UP(
> >> +        config->max_clients * sizeof(uint64_t), TARGET_PAGE_SIZE);
> 
> > This needs to be host page size aligned, too.  Otherwise protect will
> > affect bytes beyond the end of the control region.
> 
> Ummm, so right. Although I think only host page alignment is required (there's
> no soft TLB in user-mode, right?).

Yes.

> >> +static void init_channel(const char *base, const char *suffix, size_t size,
> >> +                         char **path, int *fd, uint64_t **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) {
> >> +        error_report("error: open(%s): %s", *path, strerror(errno));
> >> +        abort();
> >> +    }
> 
> > open() can fail for reasons outside QEMU's control.  This isn't an
> > internal error.  Please exit cleanly instead of using abort(3).
> 
> By cleanly you mean exit with a non-zero code, right? It still is an error that
> cannot be recovered.

Right, it's an error.

> Also, if this goes with exit() what about the abort()s I have added in other
> places? (e.g., on a failed call to sigaction)

abort(3) is useful for internal errors where a core dump and debugging
are required.

exit(3) is useful for graceful exit (both successful and unsuccessful).
Over the past few years the codebase has been moving towards using Error
**errp and letting the top-level functions handle errors instead of
exiting deep inside QEMU.  This is necessary because lots of things can
be initialized at runtime (like device hotplug) and shouldn't bring down
QEMU.  But it's okay to exit in initialization code that will only be
called once.

> >> +
> >> +    } 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);
> >> +        }
> 
> > Is there a case when no signal handler was installed (i.e. default
> > action)?
> 
> Yes, before calling hypertrace_init() or if it is called without a
> "hypertrace_base" argument set (i.e., the user has not enabled hypertrace in the
> command line).

I meant "what happens if !segv_next.sa_action &&
!segv_next.sa_handler?".  The default signal disposition should take
effect.  This code is ignoring that case, turning everything into
SIG_IGN but there is also SIG_DFL.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2017-01-17  9:46       ` Stefan Hajnoczi
@ 2017-01-17 23:35         ` Lluís Vilanova
  0 siblings, 0 replies; 21+ messages in thread
From: Lluís Vilanova @ 2017-01-17 23:35 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Stefan Hajnoczi, qemu-devel, Daniel P Berrange, Luiz Capitulino,
	Eric Blake, Riku Voipio

Stefan Hajnoczi writes:

> On Mon, Jan 16, 2017 at 06:05:26PM +0100, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>> > On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
[...]
>> >> +
>> >> +    } 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);
>> >> +        }
>> 
>> > Is there a case when no signal handler was installed (i.e. default
>> > action)?
>> 
>> Yes, before calling hypertrace_init() or if it is called without a
>> "hypertrace_base" argument set (i.e., the user has not enabled hypertrace in the
>> command line).

> I meant "what happens if !segv_next.sa_action &&
> !segv_next.sa_handler?".  The default signal disposition should take
> effect.  This code is ignoring that case, turning everything into
> SIG_IGN but there is also SIG_DFL.

I see, I didn't take SIG_DFL and SIG_IGN into account, and if both are null (no
handler was installed by the user), I should cleanup my handler and raise() the
signal again to let it go through its default system action.

Sorry for the overall messy patch.


Thanks,
  Lluis

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-01-17 23:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-26 20:34 [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 1/6] hypertrace: Add documentation Lluís Vilanova
2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 2/6] hypertrace: Add tracing event "guest_hypertrace" Lluís Vilanova
2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
2017-01-09 15:44   ` Stefan Hajnoczi
2017-01-16 17:05     ` Lluís Vilanova
2017-01-17  9:46       ` Stefan Hajnoczi
2017-01-17 23:35         ` Lluís Vilanova
2017-01-16 17:10     ` Lluís Vilanova
2017-01-09 16:35   ` Stefan Hajnoczi
2017-01-09 18:20     ` Lluís Vilanova
2017-01-10 14:47       ` Stefan Hajnoczi
2016-12-26 20:34 ` [Qemu-devel] [PATCH v4 4/6] hypertrace: [softmmu] " Lluís Vilanova
2017-01-09 16:12   ` Stefan Hajnoczi
2016-12-26 20:35 ` [Qemu-devel] [PATCH v4 5/6] hypertrace: Add guest-side user-level library Lluís Vilanova
2016-12-26 20:35 ` [Qemu-devel] [PATCH v4 6/6] hypertrace: Add guest-side Linux module Lluís Vilanova
2017-01-09 16:17   ` Stefan Hajnoczi
2017-01-15  2:10     ` Lluís Vilanova
2017-01-16 10:37       ` Stefan Hajnoczi
2016-12-26 20:47 ` [Qemu-devel] [PATCH v4 0/6] hypertrace: Lightweight guest-to-QEMU trace channel no-reply
2017-01-09 16:28 ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.