All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
@ 2017-07-30 14:08 Lluís Vilanova
  2017-07-30 14:12 ` [Qemu-devel] [PATCH v8 1/5] hypertrace: Add documentation Lluís Vilanova
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Lluís Vilanova @ 2017-07-30 14:08 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 v8
=============

* Do not use 'seq' when there's no extra hypertrace arguments (BSD behaves
  differently for "seq 0").
* Fix compilation for bsd-user.


Changes in v7
=============

* Use 'expr' instead of assuming 'bash' when generating the "emit.c" file.
* Restore generation of trace-events-all.


Changes in v6
=============

* Fix compilation errors.


Changes in v5
=============

* Rebase on 5a477a7806.
* Fix typo in "bsd-user/main.c" [Stephan Hajnoczi].
* Replace abort() with exit() in command-line errors [Stephan Hajnoczi].
* Fix alignment of data and control channels [Stephan Hajnoczi].
* Fix signal reflection in user-mode (SIGINT, SIGABRT, SIGSEGV) [Stephan Hajnoczi].
* Clarify semantics of hypertrace_guest_mmap_check() [Stephan Hajnoczi].
* Use uintptr_t instead of unsigned long in SEGV handler [Stephan Hajnoczi].
* Emit hypertrace's event with host-endian arguments [Stephan Hajnoczi].
* Enable true concurrency between user-mode guest threads by using a spearate control channel page per client [Stephan Hajnoczi].
* Remove unused PAGE_SIZE define [Stephan Hajnoczi].
* Submit linux kernel API module separately to Linux upstream [Stephan Hajnoczi].
* Assume guest code events are always enabled.


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 (5):
      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


 Makefile                           |   11 +
 Makefile.objs                      |    6 +
 bsd-user/main.c                    |   17 +
 bsd-user/mmap.c                    |   15 +
 bsd-user/qemu.h                    |    3 
 bsd-user/syscall.c                 |   34 ++-
 configure                          |   36 +++
 docs/devel/tracing.txt             |    3 
 docs/hypertrace.txt                |  225 ++++++++++++++++++++
 hypertrace/Makefile.objs           |   25 ++
 hypertrace/common.c                |   55 +++++
 hypertrace/common.h                |   25 ++
 hypertrace/guest/Makefile          |   30 +++
 hypertrace/guest/common.c          |  301 ++++++++++++++++++++++++++
 hypertrace/guest/qemu-hypertrace.h |   80 +++++++
 hypertrace/softmmu.c               |  237 +++++++++++++++++++++
 hypertrace/user.c                  |  415 ++++++++++++++++++++++++++++++++++++
 hypertrace/user.h                  |   71 ++++++
 include/hw/pci/pci.h               |    2 
 include/qom/cpu.h                  |    4 
 linux-user/main.c                  |   19 ++
 linux-user/mmap.c                  |   16 +
 linux-user/qemu.h                  |    3 
 linux-user/signal.c                |   12 +
 linux-user/syscall.c               |   31 ++-
 rules.mak                          |    2 
 trace-events                       |   11 +
 27 files changed, 1660 insertions(+), 29 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/Makefile
 create mode 100644 hypertrace/guest/common.c
 create mode 100644 hypertrace/guest/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] 14+ messages in thread

* [Qemu-devel] [PATCH v8 1/5] hypertrace: Add documentation
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
@ 2017-07-30 14:12 ` Lluís Vilanova
  2017-08-04 17:50   ` Eric Blake
  2017-07-30 14:16 ` [Qemu-devel] [PATCH v8 2/5] hypertrace: Add tracing event "guest_hypertrace" Lluís Vilanova
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Lluís Vilanova @ 2017-07-30 14:12 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/devel/tracing.txt |    3 +
 docs/hypertrace.txt    |  225 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 228 insertions(+)
 create mode 100644 docs/hypertrace.txt

diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
index 5768a0b7a2..9178a308da 100644
--- a/docs/devel/tracing.txt
+++ b/docs/devel/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:
diff --git a/docs/hypertrace.txt b/docs/hypertrace.txt
new file mode 100644
index 0000000000..c3715db25b
--- /dev/null
+++ b/docs/hypertrace.txt
@@ -0,0 +1,225 @@
+= Hypertrace channel =
+
+Copyright (C) 2016-2017 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, 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 that
+  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" backend (SystemTap) 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.
+
+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. Compile QEMU:
+
+    make -C /tmp/qemu-build install -j
+
+3. Compile the guest support code:
+
+    make -C /tmp/qemu-build/x86_64-linux-user/hypertrace/guest
+    make -C /tmp/qemu-build/x86_64-softmmu/hypertrace/guest
+
+   If you need to cross-compile the guest library, set the 'CC' variable:
+
+    make -C /tmp/qemu-build/mipsel-linux-user/hypertrace/guest CC=mipsel-gnu-linux-gcc
+
+4. Create a guest application that interacts 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/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/libqemu-hypertrace-guest.a      \
+        -I/tmp/qemu-install/include -lpthread
+
+5. 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
+
+6. 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 instead run a guest in 'user' mode with hypertrace (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
+
+
+== 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
+(i.e., similar to what virtual devices achieve in SR-IOV).

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

* [Qemu-devel] [PATCH v8 2/5] hypertrace: Add tracing event "guest_hypertrace"
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
  2017-07-30 14:12 ` [Qemu-devel] [PATCH v8 1/5] hypertrace: Add documentation Lluís Vilanova
@ 2017-07-30 14:16 ` Lluís Vilanova
  2017-07-30 14:20 ` [Qemu-devel] [PATCH v8 3/5] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Lluís Vilanova @ 2017-07-30 14:16 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      |    6 ++++--
 Makefile.objs |    2 ++
 configure     |   34 ++++++++++++++++++++++++++++++++++
 rules.mak     |    2 +-
 trace-events  |   11 +++++++++++
 5 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 38814f9a61..f08f498f79 100644
--- a/Makefile
+++ b/Makefile
@@ -84,6 +84,7 @@ endif
 GENERATED_FILES += $(TRACE_HEADERS)
 GENERATED_FILES += $(TRACE_SOURCES)
 GENERATED_FILES += $(BUILD_DIR)/trace-events-all
+GENERATED_FILES += $(BUILD_DIR)/hypertrace/trace.h
 
 trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
 
@@ -92,7 +93,7 @@ tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
 
 %/trace.h: %/trace.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-%/trace.h-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
+%/trace.h-timestamp: %/trace-events $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--group=$(call trace-group-name,$@) \
 		--format=h \
@@ -101,7 +102,7 @@ tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
 
 %/trace.c: %/trace.c-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-%/trace.c-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
+%/trace.c-timestamp: %/trace-events $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--group=$(call trace-group-name,$@) \
 		--format=c \
@@ -534,6 +535,7 @@ distclean: clean
 	rm -Rf .sdk
 	if test -f pixman/config.log; then $(MAKE) -C pixman distclean; fi
 	if test -f dtc/version_gen.h; then $(MAKE) $(DTC_MAKE_ARGS) clean; fi
+	rm -f hypertrace/trace-events
 
 KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  modifiers  no  pt-br  sv \
 ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \
diff --git a/Makefile.objs b/Makefile.objs
index bfd5a6ceb1..ce9a60137b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -170,9 +170,11 @@ trace-events-subdirs += accel/kvm
 trace-events-subdirs += nbd
 
 trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events)
+trace-events-files += $(BUILD_DIR)/hypertrace/trace-events
 
 trace-obj-y = trace-root.o
 trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
 trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
 trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
 trace-obj-$(CONFIG_TRACE_DTRACE) += $(trace-events-subdirs:%=%/trace-dtrace.o)
+trace-obj-y += $(BUILD_DIR)/hypertrace/trace.o
diff --git a/configure b/configure
index a3f0522e8f..e6f752c242 100755
--- a/configure
+++ b/configure
@@ -351,6 +351,7 @@ pie=""
 qom_cast_debug="yes"
 trace_backends="log"
 trace_file="trace"
+hypertrace="1"
 spice=""
 rbd=""
 smartcard=""
@@ -895,6 +896,8 @@ for opt do
   ;;
   --with-trace-file=*) trace_file="$optarg"
   ;;
+  --with-hypertrace-args=*) hypertrace="$optarg"
+  ;;
   --enable-gprof) gprof="yes"
   ;;
   --enable-gcov) gcov="yes"
@@ -1425,6 +1428,8 @@ Advanced options (experts only):
                            Available backends: $trace_backend_list
   --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
@@ -4434,6 +4439,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
@@ -5282,6 +5297,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"
@@ -5989,6 +6005,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/rules.mak b/rules.mak
index 6e943335f3..8d3915070b 100644
--- a/rules.mak
+++ b/rules.mak
@@ -139,7 +139,7 @@ cc-option = $(if $(shell $(CC) $1 $2 -S -o /dev/null -xc /dev/null \
 cc-c-option = $(if $(shell $(CC) $1 $2 -c -o /dev/null -xc /dev/null \
                 >/dev/null 2>&1 && echo OK), $2, $3)
 
-VPATH_SUFFIXES = %.c %.h %.S %.cc %.cpp %.m %.mak %.texi %.sh %.rc
+VPATH_SUFFIXES = %.c %.h %.S %.cc %.cpp %.m %.mak %.texi %.sh %.rc %trace-events
 set-vpath = $(if $1,$(foreach PATTERN,$(VPATH_SUFFIXES),$(eval vpath $(PATTERN) $1)))
 
 # install-prog list, dir
diff --git a/trace-events b/trace-events
index f9dbd7f509..aeb7b6807d 100644
--- a/trace-events
+++ b/trace-events
@@ -88,6 +88,17 @@ vcpu guest_cpu_exit(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).
 #

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

* [Qemu-devel] [PATCH v8 3/5] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
  2017-07-30 14:12 ` [Qemu-devel] [PATCH v8 1/5] hypertrace: Add documentation Lluís Vilanova
  2017-07-30 14:16 ` [Qemu-devel] [PATCH v8 2/5] hypertrace: Add tracing event "guest_hypertrace" Lluís Vilanova
@ 2017-07-30 14:20 ` Lluís Vilanova
  2017-07-30 14:24 ` [Qemu-devel] [PATCH v8 4/5] hypertrace: [softmmu] " Lluís Vilanova
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Lluís Vilanova @ 2017-07-30 14:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Eric Blake, Stefan Hajnoczi,
	Riku Voipio, Laurent Vivier

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/qemu.h          |    3 
 bsd-user/syscall.c       |   34 ++--
 hypertrace/Makefile.objs |   21 ++
 hypertrace/common.c      |   55 ++++++
 hypertrace/common.h      |   25 +++
 hypertrace/user.c        |  415 ++++++++++++++++++++++++++++++++++++++++++++++
 hypertrace/user.h        |   71 ++++++++
 include/qom/cpu.h        |    4 
 linux-user/main.c        |   19 ++
 linux-user/mmap.c        |   16 ++
 linux-user/qemu.h        |    3 
 linux-user/signal.c      |   12 +
 linux-user/syscall.c     |   31 ++-
 16 files changed, 719 insertions(+), 26 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 ce9a60137b..57479fa738 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -98,6 +98,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 fa9c012c9f..50df757209 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -30,9 +30,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;
@@ -675,6 +678,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"
@@ -735,6 +740,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();
@@ -753,6 +760,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 (;;) {
@@ -840,6 +848,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_base);
+            hypertrace_opt_parse(optarg, &hypertrace_base,
+                                 &hypertrace_max_clients);
         } else {
             usage();
         }
@@ -974,6 +986,11 @@ int main(int argc, char **argv)
     target_set_brk(info->brk);
     syscall_init();
     signal_init();
+    if (atexit(hypertrace_fini) != 0) {
+        error_report("error: atexit: %s", 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
diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 7f2018ede0..6a549a3553 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
 
@@ -240,10 +241,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;
 
@@ -285,6 +293,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;
@@ -396,6 +408,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/qemu.h b/bsd-user/qemu.h
index 19b2b8fecb..55421e9d89 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -205,6 +205,9 @@ abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
 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/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..177230fe1d
--- /dev/null
+++ b/hypertrace/Makefile.objs
@@ -0,0 +1,21 @@
+# -*- mode: makefile -*-
+
+target-obj-$(CONFIG_USER_ONLY) += user.o
+target-obj-y += common.o
+
+$(obj)/user.o: $(obj)/emit.c
+$(obj)/common.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 do_hypertrace_emit(CPUState *cpu, uint64_t arg1, uint64_t *data)" >$@
+	@echo "{" >>$@
+	@echo -n "    trace_guest_hypertrace(cpu, arg1" >>$@
+ifneq ($(CONFIG_HYPERTRACE_ARGS),1)
+	for i in `seq $(shell expr $(CONFIG_HYPERTRACE_ARGS) - 1)`; do \
+	    echo -n ", data[$$i-1]" >>$@; \
+	done
+endif
+	@echo ");" >>$@
+	@echo "}" >>$@
diff --git a/hypertrace/common.c b/hypertrace/common.c
new file mode 100644
index 0000000000..dfa8e4e4c9
--- /dev/null
+++ b/hypertrace/common.c
@@ -0,0 +1,55 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016-2017 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 "cpu.h"
+#include "exec/cpu-all.h"
+#include "hypertrace/common.h"
+#include "hypertrace/trace.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);
+
+    /* Align for both, since they can be used on softmmu and user mode */
+    int page_size = 1;
+    page_size = QEMU_ALIGN_UP(page_size, getpagesize());
+    page_size = QEMU_ALIGN_UP(page_size, TARGET_PAGE_SIZE);
+
+#if defined(CONFIG_USER_ONLY)
+    /* Twice the number of clients (*in pages*) for the double-fault protocol */
+    config->control_size = QEMU_ALIGN_UP(
+        config->max_clients * TARGET_PAGE_SIZE * 2, page_size);
+#else
+    config->control_size = QEMU_ALIGN_UP(
+        config->max_clients * sizeof(uint64_t), page_size);
+#endif
+    config->data_size = QEMU_ALIGN_UP(
+        config->max_clients * config->client_data_size, page_size);
+}
+
+
+#include "hypertrace/emit.c"
+
+void hypertrace_emit(CPUState *cpu, uint64_t arg1, uint64_t *data)
+{
+    int i;
+    /* swap event arguments to host endianness */
+    arg1 = tswap64(arg1);
+    for (i = 0; i < CONFIG_HYPERTRACE_ARGS - 1; i++) {
+        data[i] = tswap64(data[i]);
+    }
+
+    /* emit event */
+    do_hypertrace_emit(cpu, arg1, data);
+}
diff --git a/hypertrace/common.h b/hypertrace/common.h
new file mode 100644
index 0000000000..cd295bdf76
--- /dev/null
+++ b/hypertrace/common.h
@@ -0,0 +1,25 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016-2017 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 "qemu/typedefs.h"
+
+/* NOTE: Linux's kernel headers must be synced with this */
+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);
+
+void hypertrace_emit(struct CPUState *cpu, uint64_t arg1, uint64_t *data);
diff --git a/hypertrace/user.c b/hypertrace/user.c
new file mode 100644
index 0000000000..7ff6749f7d
--- /dev/null
+++ b/hypertrace/user.c
@@ -0,0 +1,415 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016-2017 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;
+
+
+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 {
+        error_report("error: -hypertrace path is mandatory");
+        exit(EXIT_FAILURE);
+    }
+    max_clients = qemu_opt_get_number(opts, "max-clients", 1);
+    if (max_clients <= 0) {
+        error_report("error:"
+                     " -hypertrace 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));
+        exit(1);
+    }
+
+    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();
+        }
+    }
+}
+
+/* Host SIGSEGV cannot be set bu user-mode guests */
+static struct sigaction sigsegv_ours;
+static struct sigaction sigsegv_next;
+static void sigsegv_handler(int signum, siginfo_t *siginfo, void *sigctxt);
+
+static struct sigaction sigint_ours;
+static struct sigaction sigint_next;
+bool sigint_user_set;
+struct sigaction sigint_user;
+
+static struct sigaction sigabrt_ours;
+static struct sigaction sigabrt_next;
+bool sigabrt_user_set;
+struct sigaction sigabrt_user;
+
+typedef void (*sigaction_t)(int, siginfo_t *, void *);
+
+static void reflect_handler(struct sigaction *ours, struct sigaction *next,
+                            int signum, siginfo_t *siginfo, void *sigctxt)
+{
+    if (next->sa_flags & SA_SIGINFO) {
+        if (next->sa_sigaction != NULL) {
+            next->sa_sigaction(signum, siginfo, sigctxt);
+        } else if (next->sa_sigaction == (sigaction_t)SIG_IGN) {
+            /* ignore */
+        } else if (next->sa_sigaction == (sigaction_t)SIG_DFL) {
+            if (signal(signum, SIG_DFL) == SIG_ERR) {
+                error_report("error: signal: %s", strerror(errno));
+                abort();
+            }
+            if (raise(signum) != 0) {
+                error_report("error: raise: %s", strerror(errno));
+                abort();
+            }
+            struct sigaction tmp;
+            memcpy(&tmp, ours, sizeof(tmp));
+            if (sigaction(signum, &tmp, NULL) != 0) {
+                error_report("error: sigaction: %s", strerror(errno));
+                abort();
+            }
+        }
+    } else {
+        if (next->sa_handler != NULL) {
+            next->sa_handler(signum);
+        } else if (next->sa_handler == SIG_IGN) {
+            /* ignore */
+        } else if (next->sa_handler == SIG_DFL) {
+            if (signal(signum, SIG_DFL) == SIG_ERR) {
+                error_report("error: signal: %s", strerror(errno));
+                abort();
+            }
+            if (raise(signum) != 0) {
+                error_report("error: raise: %s", strerror(errno));
+                abort();
+            }
+            struct sigaction tmp;
+            memcpy(&tmp, ours, sizeof(tmp));
+            if (sigaction(signum, &tmp, NULL) != 0) {
+                error_report("error: sigaction: %s", strerror(errno));
+                abort();
+            }
+        }
+    }
+}
+
+static void sigint_handler(int signum, siginfo_t *siginfo, void *sigctxt)
+{
+    hypertrace_fini();
+    /* QEMU lets users override any signal handler */
+    if (sigint_user_set) {
+        reflect_handler(&sigint_ours, &sigint_user, signum, siginfo, sigctxt);
+    } else {
+        reflect_handler(&sigint_ours, &sigint_next, signum, siginfo, sigctxt);
+    }
+}
+
+static void sigabrt_handler(int signum, siginfo_t *siginfo, void *sigctxt)
+{
+    hypertrace_fini();
+    /* QEMU lets users override any signal handler */
+    if (sigabrt_user_set) {
+        reflect_handler(&sigabrt_ours, &sigabrt_user, signum, siginfo, sigctxt);
+    } else {
+        reflect_handler(&sigabrt_ours, &sigabrt_next, signum, siginfo, sigctxt);
+    }
+}
+
+void hypertrace_init(const char *base, unsigned int max_clients)
+{
+    struct hypertrace_config *pconfig;
+
+    if (base == NULL) {
+        return;
+    }
+
+    sigint_user_set = false;
+    memset(&sigint_ours, 0, sizeof(sigint_ours));
+    sigint_ours.sa_sigaction = sigint_handler;
+    sigint_ours.sa_flags = SA_SIGINFO | SA_RESTART;
+    sigemptyset(&sigint_ours.sa_mask);
+    if (sigaction(SIGINT, &sigint_ours, &sigint_next) != 0) {
+        error_report("error: sigaction(SIGINT): %s", strerror(errno));
+        abort();
+    }
+
+    sigabrt_user_set = false;
+    memset(&sigabrt_ours, 0, sizeof(sigabrt_ours));
+    sigabrt_ours.sa_sigaction = sigabrt_handler;
+    sigabrt_ours.sa_flags = SA_SIGINFO | SA_RESTART;
+    sigemptyset(&sigabrt_ours.sa_mask);
+    if (sigaction(SIGABRT, &sigabrt_ours, &sigabrt_next) != 0) {
+        error_report("error: sigaction(SIGABRT): %s", strerror(errno));
+        abort();
+    }
+
+    hypertrace_init_config(&config, max_clients);
+
+    init_channel(base, "-config", getpagesize(),
+                 &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();
+    }
+
+    memset(&sigsegv_ours, 0, sizeof(sigsegv_ours));
+    sigsegv_ours.sa_sigaction = sigsegv_handler;
+    sigsegv_ours.sa_flags = SA_SIGINFO | SA_RESTART;
+    sigemptyset(&sigsegv_ours.sa_mask);
+    if (sigaction(SIGSEGV, &sigsegv_ours, &sigsegv_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, &sigsegv_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) {
+        /* the control channel should never fail fstat() */
+        return true;
+    }
+
+    if (s.st_dev != control_fd_stat.st_dev ||
+        s.st_ino != control_fd_stat.st_ino) {
+        /* this is not the control channel */
+        return true;
+    }
+
+    /* check control channel is mapped correctly */
+    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();
+    }
+}
+
+static void sigsegv_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(((uintptr_t)siginfo->si_addr % sizeof(TARGET_PAGE_SIZE)) == 0);
+        swap_control(control_0, control_1);
+
+    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
+
+        /* 2nd fault (invoke) */
+        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];
+        assert(((uintptr_t)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 */
+        reflect_handler(&sigsegv_ours, &sigsegv_next, signum, siginfo, sigctxt);
+
+    }
+}
diff --git a/hypertrace/user.h b/hypertrace/user.h
new file mode 100644
index 0000000000..e3f131c675
--- /dev/null
+++ b/hypertrace/user.h
@@ -0,0 +1,71 @@
+/*
+ * QEMU-side management of hypertrace in user-level emulation.
+ *
+ * Copyright (C) 2016-2017 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:
+ *
+ * Check whether the mapped file is *not* hypertrace's control channel; if it
+ * is, check it is mapped correctly.
+ *
+ * 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);
+
+
+/* Internal signal management */
+extern bool sigint_user_set;
+extern struct sigaction sigint_user;
+extern bool sigabrt_user_set;
+extern struct sigaction sigabrt_user;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 25eefea7ab..bb382a0e6a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -305,6 +305,7 @@ struct qemu_work_item;
  * @trace_dstate_delayed: Delayed changes to trace_dstate (includes all changes
  *                        to @trace_dstate).
  * @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.
  */
@@ -377,6 +378,9 @@ struct CPUState {
     DECLARE_BITMAP(trace_dstate_delayed, CPU_TRACE_DSTATE_MAX_EVENTS);
     DECLARE_BITMAP(trace_dstate, CPU_TRACE_DSTATE_MAX_EVENTS);
 
+    /* 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 ad03c9e8b2..b9e31e925b 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;
 
@@ -4014,6 +4016,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;
@@ -4063,6 +4073,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}
@@ -4252,6 +4264,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);
 
@@ -4453,6 +4466,12 @@ int main(int argc, char **argv, char **envp)
     syscall_init();
     signal_init();
 
+    if (atexit(hypertrace_fini)) {
+        error_report("error: atexit: %s", 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 4888f53139..ade8759af9 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
 
@@ -357,10 +358,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();
@@ -442,6 +451,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;
@@ -553,6 +566,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/linux-user/qemu.h b/linux-user/qemu.h
index 4edd7d0c08..67cf051d27 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -425,6 +425,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/signal.c b/linux-user/signal.c
index 3d18d1b3ee..9a21302331 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -23,6 +23,7 @@
 
 #include "qemu.h"
 #include "qemu-common.h"
+#include "hypertrace/user.h"
 #include "target_signal.h"
 #include "trace.h"
 
@@ -813,7 +814,16 @@ int do_sigaction(int sig, const struct target_sigaction *act,
             } else {
                 act1.sa_sigaction = host_signal_handler;
             }
-            ret = sigaction(host_sig, &act1, NULL);
+
+            if (host_sig == SIGINT) {
+                memcpy(&sigint_user, &act1, sizeof(act1));
+                sigint_user_set = true;
+            } else if (host_sig == SIGABRT) {
+                memcpy(&sigabrt_user, &act1, sizeof(act1));
+                sigabrt_user_set = true;
+            } else {
+                ret = sigaction(host_sig, &act1, NULL);
+            }
         }
     }
     return ret;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 003943b736..17d76f5d39 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -115,6 +115,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 */
@@ -7765,6 +7766,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;
@@ -9231,15 +9233,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
@@ -9248,10 +9254,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:
@@ -9821,6 +9829,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] 14+ messages in thread

* [Qemu-devel] [PATCH v8 4/5] hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (2 preceding siblings ...)
  2017-07-30 14:20 ` [Qemu-devel] [PATCH v8 3/5] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
@ 2017-07-30 14:24 ` Lluís Vilanova
  2017-07-30 14:28 ` [Qemu-devel] [PATCH v8 5/5] hypertrace: Add guest-side user-level library Lluís Vilanova
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Lluís Vilanova @ 2017-07-30 14:24 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 |    4 +
 hypertrace/softmmu.c     |  237 ++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/pci/pci.h     |    2 
 3 files changed, 243 insertions(+)
 create mode 100644 hypertrace/softmmu.c

diff --git a/hypertrace/Makefile.objs b/hypertrace/Makefile.objs
index 177230fe1d..2ec1f04e79 100644
--- a/hypertrace/Makefile.objs
+++ b/hypertrace/Makefile.objs
@@ -1,10 +1,14 @@
 # -*- 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)/common.o: $(obj)/emit.c
+$(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..4048c7e0ef
--- /dev/null
+++ b/hypertrace/softmmu.c
@@ -0,0 +1,237 @@
+/*
+ * QEMU-side management of hypertrace in softmmu emulation.
+ *
+ * Copyright (C) 2016-2017 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 "hypertrace/common.h"
+#include "hypertrace/trace.h"
+#include "hw/pci/pci.h"
+#include "migration/blocker.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+
+
+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);
+        abort();
+    }
+
+    return res;
+}
+
+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);
+        abort();
+    }
+
+    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)
+{
+    Error *local_err = NULL;
+    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, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        return;
+    }
+
+    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 e598b095eb..3910a9b89f 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] 14+ messages in thread

* [Qemu-devel] [PATCH v8 5/5] hypertrace: Add guest-side user-level library
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (3 preceding siblings ...)
  2017-07-30 14:24 ` [Qemu-devel] [PATCH v8 4/5] hypertrace: [softmmu] " Lluís Vilanova
@ 2017-07-30 14:28 ` Lluís Vilanova
  2017-07-30 14:49 ` [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel no-reply
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Lluís Vilanova @ 2017-07-30 14:28 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                           |    5 +
 configure                          |    2 
 hypertrace/guest/Makefile          |   30 ++++
 hypertrace/guest/common.c          |  301 ++++++++++++++++++++++++++++++++++++
 hypertrace/guest/qemu-hypertrace.h |   80 ++++++++++
 5 files changed, 418 insertions(+)
 create mode 100644 hypertrace/guest/Makefile
 create mode 100644 hypertrace/guest/common.c
 create mode 100644 hypertrace/guest/qemu-hypertrace.h

diff --git a/Makefile b/Makefile
index f08f498f79..523885a0a5 100644
--- a/Makefile
+++ b/Makefile
@@ -602,8 +602,13 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
 endif
 endif
 
+install-hypertrace:
+	$(INSTALL_DIR) "$(DESTDIR)$(includedir)"
+	$(INSTALL_DATA) "$(SRC_PATH)/hypertrace/guest/qemu-hypertrace.h" "$(DESTDIR)$(includedir)/"
+
 
 install: all $(if $(BUILD_DOCS),install-doc) install-datadir install-localstatedir
+install: install-hypertrace
 ifneq ($(TOOLS),)
 	$(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
 endif
diff --git a/configure b/configure
index e6f752c242..9c7358d0b4 100755
--- a/configure
+++ b/configure
@@ -6286,6 +6286,8 @@ if [ "$TARGET_BASE_ARCH" = "" ]; then
 fi
 
 symlink "$source_path/Makefile.target" "$target_dir/Makefile"
+mkdir -p $target_dir/hypertrace/guest
+symlink $source_path/hypertrace/guest/Makefile $target_dir/hypertrace/guest/Makefile
 
 upper() {
     echo "$@"| LC_ALL=C tr '[a-z]' '[A-Z]'
diff --git a/hypertrace/guest/Makefile b/hypertrace/guest/Makefile
new file mode 100644
index 0000000000..cbc956bde1
--- /dev/null
+++ b/hypertrace/guest/Makefile
@@ -0,0 +1,30 @@
+include ../../../config-host.mak
+include ../../config-target.mak
+include $(SRC_PATH)/rules.mak
+
+vpath % $(SRC_PATH)/hypertrace/guest
+
+# 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/common.c b/hypertrace/guest/common.c
new file mode 100644
index 0000000000..96ae4464bc
--- /dev/null
+++ b/hypertrace/guest/common.c
@@ -0,0 +1,301 @@
+/*
+ * Guest-side management of hypertrace.
+ *
+ * Copyright (C) 2016-2017 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 page_size;
+
+
+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;
+        }
+
+        page_size = getpagesize();
+
+        res = init_channel_file(base, config_suff, page_size,
+                                &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 offset;
+#if defined(CONFIG_USER_ONLY)
+    offset = client * page_size;
+#endif
+    *(uint64_t *)((char *)control_addr + offset) = arg1;
+#if defined(CONFIG_USER_ONLY)
+    /* QEMU in 'user' mode uses two faulting pages to detect invocations */
+    *(uint64_t *)((char *)control_addr_1 + offset) = arg1;
+#endif
+}
diff --git a/hypertrace/guest/qemu-hypertrace.h b/hypertrace/guest/qemu-hypertrace.h
new file mode 100644
index 0000000000..7c7a17c64d
--- /dev/null
+++ b/hypertrace/guest/qemu-hypertrace.h
@@ -0,0 +1,80 @@
+/*
+ * Guest-side management of hypertrace.
+ *
+ * Copyright (C) 2016-2017 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] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (4 preceding siblings ...)
  2017-07-30 14:28 ` [Qemu-devel] [PATCH v8 5/5] hypertrace: Add guest-side user-level library Lluís Vilanova
@ 2017-07-30 14:49 ` no-reply
  2017-08-04 15:34 ` Stefan Hajnoczi
  2017-08-26  0:34 ` Emilio G. Cota
  7 siblings, 0 replies; 14+ messages in thread
From: no-reply @ 2017-07-30 14:49 UTC (permalink / raw)
  To: vilanova; +Cc: famz, qemu-devel, stefanha, lcapitulino

Hi,

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

Subject: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
Message-id: 150142369849.12995.11229612194223213120.stgit@frigg.lan
Type: series

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

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

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
Switched to a new branch 'test'
b727925815 hypertrace: Add guest-side user-level library
bb83766b6c hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event
b1e07ad5aa hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
add0b54fc3 hypertrace: Add tracing event "guest_hypertrace"
cecce1a6d5 hypertrace: Add documentation

=== OUTPUT BEGIN ===
Checking PATCH 1/5: hypertrace: Add documentation...
Checking PATCH 2/5: hypertrace: Add tracing event "guest_hypertrace"...
Checking PATCH 3/5: hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event...
Checking PATCH 4/5: hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event...
Checking PATCH 5/5: hypertrace: Add guest-side user-level library...
WARNING: architecture specific defines should be avoided
#171: FILE: hypertrace/guest/common.c:84:
+#if !defined(CONFIG_USER_ONLY) && defined(__linux__)

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

total: 1 errors, 1 warnings, 432 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.

=== 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] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (5 preceding siblings ...)
  2017-07-30 14:49 ` [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel no-reply
@ 2017-08-04 15:34 ` Stefan Hajnoczi
  2017-08-04 18:32   ` Lluís Vilanova
  2017-08-26  0:34 ` Emilio G. Cota
  7 siblings, 1 reply; 14+ messages in thread
From: Stefan Hajnoczi @ 2017-08-04 15:34 UTC (permalink / raw)
  To: Lluís Vilanova
  Cc: qemu-devel, Daniel P Berrange, Luiz Capitulino, Eric Blake

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

On Sun, Jul 30, 2017 at 05:08:18PM +0300, 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.
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> 
> Changes in v8
> =============
> 
> * Do not use 'seq' when there's no extra hypertrace arguments (BSD behaves
>   differently for "seq 0").
> * Fix compilation for bsd-user.

Hi Lluís,
Any changes regarding the fundamental approach since September 2016?

Back then I had the following concerns about hypertrace for full-system
virtualization:

Going to QEMU for every guest trace event has high overhead under
virtualization.  The alternative approach is recording separate traces
and merging them for analysis.  This is possible with trace-cmd [1] and
LTTng [2].

Merging traces eliminates the performance bottleneck and does not
require new paravirt interfaces or guest tracing libraries.  I think it
it would be a distraction to support hypertrace for the virtualization
use case because it fundamentally has a high overhead.

I see promise in using hypertrace for TCG because it is low-overhead
when running inside the QEMU process.  I'll review the patches again
with this in mind and not focus on virtualization.

[1] https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg00887.html
[2] http://archive.eclipse.org/tracecompass/doc/stable/org.eclipse.tracecompass.doc.user/Virtual-Machine-Analysis.html
    and also generic trace synchronization
    http://archive.eclipse.org/tracecompass/doc/stable/org.eclipse.tracecompass.doc.user/Trace-synchronization.html#Trace_synchronization

Stefan

> Changes in v7
> =============
> 
> * Use 'expr' instead of assuming 'bash' when generating the "emit.c" file.
> * Restore generation of trace-events-all.
> 
> 
> Changes in v6
> =============
> 
> * Fix compilation errors.
> 
> 
> Changes in v5
> =============
> 
> * Rebase on 5a477a7806.
> * Fix typo in "bsd-user/main.c" [Stephan Hajnoczi].
> * Replace abort() with exit() in command-line errors [Stephan Hajnoczi].
> * Fix alignment of data and control channels [Stephan Hajnoczi].
> * Fix signal reflection in user-mode (SIGINT, SIGABRT, SIGSEGV) [Stephan Hajnoczi].
> * Clarify semantics of hypertrace_guest_mmap_check() [Stephan Hajnoczi].
> * Use uintptr_t instead of unsigned long in SEGV handler [Stephan Hajnoczi].
> * Emit hypertrace's event with host-endian arguments [Stephan Hajnoczi].
> * Enable true concurrency between user-mode guest threads by using a spearate control channel page per client [Stephan Hajnoczi].
> * Remove unused PAGE_SIZE define [Stephan Hajnoczi].
> * Submit linux kernel API module separately to Linux upstream [Stephan Hajnoczi].
> * Assume guest code events are always enabled.
> 
> 
> 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 (5):
>       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
> 
> 
>  Makefile                           |   11 +
>  Makefile.objs                      |    6 +
>  bsd-user/main.c                    |   17 +
>  bsd-user/mmap.c                    |   15 +
>  bsd-user/qemu.h                    |    3 
>  bsd-user/syscall.c                 |   34 ++-
>  configure                          |   36 +++
>  docs/devel/tracing.txt             |    3 
>  docs/hypertrace.txt                |  225 ++++++++++++++++++++
>  hypertrace/Makefile.objs           |   25 ++
>  hypertrace/common.c                |   55 +++++
>  hypertrace/common.h                |   25 ++
>  hypertrace/guest/Makefile          |   30 +++
>  hypertrace/guest/common.c          |  301 ++++++++++++++++++++++++++
>  hypertrace/guest/qemu-hypertrace.h |   80 +++++++
>  hypertrace/softmmu.c               |  237 +++++++++++++++++++++
>  hypertrace/user.c                  |  415 ++++++++++++++++++++++++++++++++++++
>  hypertrace/user.h                  |   71 ++++++
>  include/hw/pci/pci.h               |    2 
>  include/qom/cpu.h                  |    4 
>  linux-user/main.c                  |   19 ++
>  linux-user/mmap.c                  |   16 +
>  linux-user/qemu.h                  |    3 
>  linux-user/signal.c                |   12 +
>  linux-user/syscall.c               |   31 ++-
>  rules.mak                          |    2 
>  trace-events                       |   11 +
>  27 files changed, 1660 insertions(+), 29 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/Makefile
>  create mode 100644 hypertrace/guest/common.c
>  create mode 100644 hypertrace/guest/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>

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

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

* Re: [Qemu-devel] [PATCH v8 1/5] hypertrace: Add documentation
  2017-07-30 14:12 ` [Qemu-devel] [PATCH v8 1/5] hypertrace: Add documentation Lluís Vilanova
@ 2017-08-04 17:50   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2017-08-04 17:50 UTC (permalink / raw)
  To: Lluís Vilanova, qemu-devel
  Cc: Daniel P Berrange, Luiz Capitulino, Stefan Hajnoczi

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

On 07/30/2017 09:12 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  docs/devel/tracing.txt |    3 +
>  docs/hypertrace.txt    |  225 ++++++++++++++++++++++++++++++++++++++++++++++++

Do we want the new document directly in docs/, or does it live in one of
the subdirectories?

>  2 files changed, 228 insertions(+)
>  create mode 100644 docs/hypertrace.txt
> 
> diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
> index 5768a0b7a2..9178a308da 100644
> --- a/docs/devel/tracing.txt
> +++ b/docs/devel/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.

Depending on where it should live, this may need a tweak.


> +
> +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'

Stale filename; needs adjusting.

> +and 'user' modes. Therefore, hypertrace is to tracing what hypercalls are to
> +system calls.
> +


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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

* Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
  2017-08-04 15:34 ` Stefan Hajnoczi
@ 2017-08-04 18:32   ` Lluís Vilanova
  2017-08-08 12:25     ` Stefan Hajnoczi
  0 siblings, 1 reply; 14+ messages in thread
From: Lluís Vilanova @ 2017-08-04 18:32 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel, Luiz Capitulino

Stefan Hajnoczi writes:

> On Sun, Jul 30, 2017 at 05:08:18PM +0300, 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.
>> 
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> 
>> Changes in v8
>> =============
>> 
>> * Do not use 'seq' when there's no extra hypertrace arguments (BSD behaves
>> differently for "seq 0").
>> * Fix compilation for bsd-user.

> Hi Lluís,
> Any changes regarding the fundamental approach since September 2016?

> Back then I had the following concerns about hypertrace for full-system
> virtualization:

> Going to QEMU for every guest trace event has high overhead under
> virtualization.  The alternative approach is recording separate traces
> and merging them for analysis.  This is possible with trace-cmd [1] and
> LTTng [2].

> Merging traces eliminates the performance bottleneck and does not
> require new paravirt interfaces or guest tracing libraries.  I think it
> it would be a distraction to support hypertrace for the virtualization
> use case because it fundamentally has a high overhead.

> I see promise in using hypertrace for TCG because it is low-overhead
> when running inside the QEMU process.  I'll review the patches again
> with this in mind and not focus on virtualization.

> [1] https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg00887.html
> [2] http://archive.eclipse.org/tracecompass/doc/stable/org.eclipse.tracecompass.doc.user/Virtual-Machine-Analysis.html
>     and also generic trace synchronization
>     http://archive.eclipse.org/tracecompass/doc/stable/org.eclipse.tracecompass.doc.user/Trace-synchronization.html#Trace_synchronization

There's been no fundamental changes since then (just the few bits listed in the
v5-v8 changelog).

But I'm kind of split on this one.

If you want high-performance trace correlation, this will work much better for
TCG than virtualization (where [1] will probably be more efficient).

If you want a hook to trigger other operations (like in the docs example), I
think this is the right approach. In fact, my initial interest in hypertrace was
for instrumentation, so maybe this should be subsumed into the proposal of a
stable instrumentation API.

What do you think?

Cheers,
  Lluis


> Stefan

>> Changes in v7
>> =============
>> 
>> * Use 'expr' instead of assuming 'bash' when generating the "emit.c" file.
>> * Restore generation of trace-events-all.
>> 
>> 
>> Changes in v6
>> =============
>> 
>> * Fix compilation errors.
>> 
>> 
>> Changes in v5
>> =============
>> 
>> * Rebase on 5a477a7806.
>> * Fix typo in "bsd-user/main.c" [Stephan Hajnoczi].
>> * Replace abort() with exit() in command-line errors [Stephan Hajnoczi].
>> * Fix alignment of data and control channels [Stephan Hajnoczi].
>> * Fix signal reflection in user-mode (SIGINT, SIGABRT, SIGSEGV) [Stephan Hajnoczi].
>> * Clarify semantics of hypertrace_guest_mmap_check() [Stephan Hajnoczi].
>> * Use uintptr_t instead of unsigned long in SEGV handler [Stephan Hajnoczi].
>> * Emit hypertrace's event with host-endian arguments [Stephan Hajnoczi].
>> * Enable true concurrency between user-mode guest threads by using a spearate control channel page per client [Stephan Hajnoczi].
>> * Remove unused PAGE_SIZE define [Stephan Hajnoczi].
>> * Submit linux kernel API module separately to Linux upstream [Stephan Hajnoczi].
>> * Assume guest code events are always enabled.
>> 
>> 
>> 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 (5):
>> 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
>> 
>> 
>> Makefile                           |   11 +
>> Makefile.objs                      |    6 +
>> bsd-user/main.c                    |   17 +
>> bsd-user/mmap.c                    |   15 +
>> bsd-user/qemu.h                    |    3 
>> bsd-user/syscall.c                 |   34 ++-
>> configure                          |   36 +++
>> docs/devel/tracing.txt             |    3 
>> docs/hypertrace.txt                |  225 ++++++++++++++++++++
>> hypertrace/Makefile.objs           |   25 ++
>> hypertrace/common.c                |   55 +++++
>> hypertrace/common.h                |   25 ++
>> hypertrace/guest/Makefile          |   30 +++
>> hypertrace/guest/common.c          |  301 ++++++++++++++++++++++++++
>> hypertrace/guest/qemu-hypertrace.h |   80 +++++++
>> hypertrace/softmmu.c               |  237 +++++++++++++++++++++
>> hypertrace/user.c                  |  415 ++++++++++++++++++++++++++++++++++++
>> hypertrace/user.h                  |   71 ++++++
>> include/hw/pci/pci.h               |    2 
>> include/qom/cpu.h                  |    4 
>> linux-user/main.c                  |   19 ++
>> linux-user/mmap.c                  |   16 +
>> linux-user/qemu.h                  |    3 
>> linux-user/signal.c                |   12 +
>> linux-user/syscall.c               |   31 ++-
>> rules.mak                          |    2 
>> trace-events                       |   11 +
>> 27 files changed, 1660 insertions(+), 29 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/Makefile
>> create mode 100644 hypertrace/guest/common.c
>> create mode 100644 hypertrace/guest/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] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
  2017-08-04 18:32   ` Lluís Vilanova
@ 2017-08-08 12:25     ` Stefan Hajnoczi
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2017-08-08 12:25 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel, Luiz Capitulino

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

On Fri, Aug 04, 2017 at 09:32:25PM +0300, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> 
> > On Sun, Jul 30, 2017 at 05:08:18PM +0300, 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.
> >> 
> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> >> ---
> >> 
> >> Changes in v8
> >> =============
> >> 
> >> * Do not use 'seq' when there's no extra hypertrace arguments (BSD behaves
> >> differently for "seq 0").
> >> * Fix compilation for bsd-user.
> 
> > Hi Lluís,
> > Any changes regarding the fundamental approach since September 2016?
> 
> > Back then I had the following concerns about hypertrace for full-system
> > virtualization:
> 
> > Going to QEMU for every guest trace event has high overhead under
> > virtualization.  The alternative approach is recording separate traces
> > and merging them for analysis.  This is possible with trace-cmd [1] and
> > LTTng [2].
> 
> > Merging traces eliminates the performance bottleneck and does not
> > require new paravirt interfaces or guest tracing libraries.  I think it
> > it would be a distraction to support hypertrace for the virtualization
> > use case because it fundamentally has a high overhead.
> 
> > I see promise in using hypertrace for TCG because it is low-overhead
> > when running inside the QEMU process.  I'll review the patches again
> > with this in mind and not focus on virtualization.
> 
> > [1] https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg00887.html
> > [2] http://archive.eclipse.org/tracecompass/doc/stable/org.eclipse.tracecompass.doc.user/Virtual-Machine-Analysis.html
> >     and also generic trace synchronization
> >     http://archive.eclipse.org/tracecompass/doc/stable/org.eclipse.tracecompass.doc.user/Trace-synchronization.html#Trace_synchronization
> 
> There's been no fundamental changes since then (just the few bits listed in the
> v5-v8 changelog).
> 
> But I'm kind of split on this one.
> 
> If you want high-performance trace correlation, this will work much better for
> TCG than virtualization (where [1] will probably be more efficient).
> 
> If you want a hook to trigger other operations (like in the docs example), I
> think this is the right approach. In fact, my initial interest in hypertrace was
> for instrumentation, so maybe this should be subsumed into the proposal of a
> stable instrumentation API.
> 
> What do you think?

That sounds good.

Stefan

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

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

* Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
  2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
                   ` (6 preceding siblings ...)
  2017-08-04 15:34 ` Stefan Hajnoczi
@ 2017-08-26  0:34 ` Emilio G. Cota
  2017-08-31 11:59   ` Stefan Hajnoczi
  7 siblings, 1 reply; 14+ messages in thread
From: Emilio G. Cota @ 2017-08-26  0:34 UTC (permalink / raw)
  To: Lluís Vilanova; +Cc: qemu-devel, Stefan Hajnoczi, Luiz Capitulino

On Sun, Jul 30, 2017 at 17:08:18 +0300, 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.
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>

This would be indeed very useful once TCG instrumentation is in place.

However, I'm not very excited about this being PCI-only and Linux-only for
system mode
I wonder how we could make this work on all hosts -- did you consider using
"magic" instructions? We'd need a different magic instruction for each
guest ISA, but the library would hide that anyway (and the library code
would be the same for user and system modes).

Thanks,

		Emilio

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

* Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
  2017-08-26  0:34 ` Emilio G. Cota
@ 2017-08-31 11:59   ` Stefan Hajnoczi
  2017-08-31 15:13     ` Lluís Vilanova
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Hajnoczi @ 2017-08-31 11:59 UTC (permalink / raw)
  To: Emilio G. Cota; +Cc: Lluís Vilanova, qemu-devel, Luiz Capitulino

On Fri, Aug 25, 2017 at 08:34:54PM -0400, Emilio G. Cota wrote:
> On Sun, Jul 30, 2017 at 17:08:18 +0300, 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.
> > 
> > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> 
> This would be indeed very useful once TCG instrumentation is in place.
> 
> However, I'm not very excited about this being PCI-only and Linux-only for
> system mode
> I wonder how we could make this work on all hosts -- did you consider using
> "magic" instructions? We'd need a different magic instruction for each
> guest ISA, but the library would hide that anyway (and the library code
> would be the same for user and system modes).

Magic instructions can potentially be implemented more efficiently in
TCG too.  They wouldn't work under KVM/HAX/hvf accelerators though.

Stefan

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

* Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel
  2017-08-31 11:59   ` Stefan Hajnoczi
@ 2017-08-31 15:13     ` Lluís Vilanova
  0 siblings, 0 replies; 14+ messages in thread
From: Lluís Vilanova @ 2017-08-31 15:13 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Emilio G. Cota, qemu-devel, Luiz Capitulino

Stefan Hajnoczi writes:

> On Fri, Aug 25, 2017 at 08:34:54PM -0400, Emilio G. Cota wrote:
>> On Sun, Jul 30, 2017 at 17:08:18 +0300, 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.
>> > 
>> > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> 
>> This would be indeed very useful once TCG instrumentation is in place.
>> 
>> However, I'm not very excited about this being PCI-only and Linux-only for
>> system mode
>> I wonder how we could make this work on all hosts -- did you consider using
>> "magic" instructions? We'd need a different magic instruction for each
>> guest ISA, but the library would hide that anyway (and the library code
>> would be the same for user and system modes).

> Magic instructions can potentially be implemented more efficiently in
> TCG too.  They wouldn't work under KVM/HAX/hvf accelerators though.

Yes, we discussed this on the first version of this series long ago. The special
device makes it work on both TCG and KVM (if it were possible to implement a
TCG-to/from-KVM mode switch that'd be awesome).

Also, both approaches have negligible performance impact on the *guest* time.


Cheers,
  Lluis

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

end of thread, other threads:[~2017-08-31 15:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-30 14:08 [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel Lluís Vilanova
2017-07-30 14:12 ` [Qemu-devel] [PATCH v8 1/5] hypertrace: Add documentation Lluís Vilanova
2017-08-04 17:50   ` Eric Blake
2017-07-30 14:16 ` [Qemu-devel] [PATCH v8 2/5] hypertrace: Add tracing event "guest_hypertrace" Lluís Vilanova
2017-07-30 14:20 ` [Qemu-devel] [PATCH v8 3/5] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event Lluís Vilanova
2017-07-30 14:24 ` [Qemu-devel] [PATCH v8 4/5] hypertrace: [softmmu] " Lluís Vilanova
2017-07-30 14:28 ` [Qemu-devel] [PATCH v8 5/5] hypertrace: Add guest-side user-level library Lluís Vilanova
2017-07-30 14:49 ` [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel no-reply
2017-08-04 15:34 ` Stefan Hajnoczi
2017-08-04 18:32   ` Lluís Vilanova
2017-08-08 12:25     ` Stefan Hajnoczi
2017-08-26  0:34 ` Emilio G. Cota
2017-08-31 11:59   ` Stefan Hajnoczi
2017-08-31 15:13     ` Lluís Vilanova

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.