* [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
@ 2017-07-24 17:02 Lluís Vilanova
2017-07-24 17:06 ` [Qemu-devel] [PATCH 01/13] instrument: Add documentation Lluís Vilanova
` (13 more replies)
0 siblings, 14 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
This series adds a basic interface to instrument tracing events and control
their tracing state.
The instrumentation code is dynamically loaded into QEMU (either when it starts
or later using its remote control interfaces).
All events can be instrumented, but the instrumentable events must be explicitly
specified at configure time.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Lluís Vilanova (13):
instrument: Add documentation
instrument: [none] Add null instrumentation mode
instrument: [dynamic] Add dynamic instrumentation mode
instrument: Allow adding the "instrument" property without modifying event files
instrument: [dynamic] Add default public per-event functions
instrument: Add event control interface
instrument: Add generic command line library loader
instrument: [linux-user] Add command line library loader
instrument: [bsd-user] Add command line library loader
instrument: [softmmu] Add command line library loader
instrument: [qapi] Add library loader
instrument: [hmp] Add library loader
trace: Rename C++-specific names in event arguments
.gitignore | 7
Makefile | 65 ++++-
Makefile.objs | 8 +
bsd-user/main.c | 16 +
bsd-user/syscall.c | 5
configure | 40 +++
docs/devel/tracing.txt | 9 +
docs/instrumentation.txt | 264 ++++++++++++++++++
hmp-commands.hx | 28 ++
include/trace-tcg.h | 1
instrument/Makefile.objs | 47 +++
instrument/cmdline.c | 117 ++++++++
instrument/cmdline.h | 58 ++++
instrument/control.c | 316 ++++++++++++++++++++++
instrument/control.h | 86 ++++++
instrument/qemu-instr/control.h | 128 +++++++++
instrument/qemu-instr/types.h | 81 ++++++
instrument/qemu-instr/visibility-internal.h | 58 ++++
instrument/qmp.c | 71 +++++
linux-user/main.c | 20 +
linux-user/syscall.c | 4
monitor.c | 55 ++++
qapi-schema.json | 3
qapi/instrument.json | 92 ++++++
qemu-options.hx | 17 +
rules.mak | 3
scripts/tracetool.py | 11 +
scripts/tracetool/__init__.py | 43 +++
scripts/tracetool/backend/instr_dynamic.py | 161 +++++++++++
scripts/tracetool/backend/instr_none.py | 52 ++++
scripts/tracetool/format/c.py | 37 ++-
scripts/tracetool/format/h.py | 8 -
scripts/tracetool/format/instr_api_h.py | 76 +++++
scripts/tracetool/format/instr_c.py | 41 +++
scripts/tracetool/format/instr_h.py | 48 +++
scripts/tracetool/format/instr_tcg_c.py | 37 +++
scripts/tracetool/format/instr_tcg_h.py | 56 ++++
scripts/tracetool/format/tcg_h.py | 6
scripts/tracetool/format/tcg_helper_c.py | 4
scripts/tracetool/format/tcg_helper_h.py | 2
scripts/tracetool/format/tcg_helper_wrapper_h.py | 2
scripts/tracetool/transform.py | 13 +
trace/event-internal.h | 10 +
vl.c | 11 +
44 files changed, 2197 insertions(+), 20 deletions(-)
create mode 100644 docs/instrumentation.txt
create mode 100644 instrument/Makefile.objs
create mode 100644 instrument/cmdline.c
create mode 100644 instrument/cmdline.h
create mode 100644 instrument/control.c
create mode 100644 instrument/control.h
create mode 100644 instrument/qemu-instr/control.h
create mode 100644 instrument/qemu-instr/types.h
create mode 100644 instrument/qemu-instr/visibility-internal.h
create mode 100644 instrument/qmp.c
create mode 100644 qapi/instrument.json
create mode 100644 scripts/tracetool/backend/instr_dynamic.py
create mode 100644 scripts/tracetool/backend/instr_none.py
create mode 100644 scripts/tracetool/format/instr_api_h.py
create mode 100644 scripts/tracetool/format/instr_c.py
create mode 100644 scripts/tracetool/format/instr_h.py
create mode 100644 scripts/tracetool/format/instr_tcg_c.py
create mode 100644 scripts/tracetool/format/instr_tcg_h.py
To: qemu-devel@nongnu.org
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Emilio G. Cota <cota@braap.org>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 01/13] instrument: Add documentation
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
@ 2017-07-24 17:06 ` Lluís Vilanova
2017-07-24 17:10 ` [Qemu-devel] [PATCH 02/13] instrument: [none] Add null instrumentation mode Lluís Vilanova
` (12 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
docs/devel/tracing.txt | 9 ++
docs/instrumentation.txt | 264 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 273 insertions(+)
create mode 100644 docs/instrumentation.txt
diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
index 5768a0b7a2..17fcd12469 100644
--- a/docs/devel/tracing.txt
+++ b/docs/devel/tracing.txt
@@ -439,3 +439,12 @@ If the translating vCPU has address 0xc1 and code is later executed by vCPU
baz_trans cpu=0xc1 a=0xd3
// at guest code execution
baz_exec cpu=0xc2 a=0xd3
+
+=== "instrument" ===
+
+When compiling QEMU with trace instrumentation enabled, the "instrument"
+property lets you provide your own implementation for that trace event. This
+implementation can override and/or wrap the backend-specific tracing code
+(regardless of the tracing backend).
+
+See "docs/instrumentation.txt" for more information.
diff --git a/docs/instrumentation.txt b/docs/instrumentation.txt
new file mode 100644
index 0000000000..3d650ed09e
--- /dev/null
+++ b/docs/instrumentation.txt
@@ -0,0 +1,264 @@
+= Trace instrumentation =
+
+== Introduction ==
+
+Trace instrumentation allows users to execute their own code when QEMU raises
+one of its tracing events (see "docs/devel/tracing.txt"). This is more efficient
+than instrumenting events with the "dtrace" backend, since the user will run
+native instrumentation code and has more options to interact with the dynamic
+tracing and instrumentation facilities of QEMU.
+
+When applied to guest code events (i.e., those with the "guest_" prefix, like
+guest memory accesses), this turns QEMU into a fairly efficient and guest
+architecture-agnostic dynamic binary instrumentation framework. It works on all
+QEMU-supported architectures, as well as works in both 'user' (standalone
+application) and 'system' (full-system emulation) modes.
+
+Look at the headers installed by QEMU on the "qemu-instr" directory for further
+information beyond this document.
+
+
+== Selecting the events to instrument ==
+
+You must first select which events must be instrumentable before compiling QEMU
+by prefixing them with the "instrument" property, and removing the "disable"
+property if it is present.
+
+To get the full list of files defining events:
+
+ find /path/to/qemu-source -name trace-events
+
+To avoid modifying QEMU's sources, you can pass the "--with-instrument-events"
+argument to configure with one event name per line.
+
+
+== Instrumenting guest code ==
+
+QEMU emulates all guest instructions when executing in TCG mode (as opposed to
+using native hardware virtualization with KVM). Instructions are decompiled and
+translated into the intermediate TCG language. Then, the TCG compiler translates
+TCG code into the native host code that QEMU will execute.
+
+All events relating to guest code are named "guest_*". In addition, all events
+with the "tcg" property (see "docs/devel/tracing.txt") can be instrumented at
+two levels:
+
+* Translation
+
+ Raised when generating TCG code (e.g., translate a memory access instruction
+ from the guest).
+
+ Note: This level only exists for events with the "tcg" property.
+
+* Execution
+
+ Raised when executing the native host code generated by the TCG compiler
+ (e.g., execute a memory access instruction from the guest).
+
+ Note: All events without the "tcg" property are raised at execution time
+ (e.g., CPU hotplug).
+
+Note: Events with the "tcg" property (e.g., 'guest_mem_before') are internally
+ translated into two events to differentiate the translation and execution
+ levels (e.g., 'guest_mem_before_trans' and 'guest_mem_before_exec').
+
+Note: All guest events have a "Mode" and "Target" line describing when they are
+ available (e.g., TCG, KVM, etc.).
+
+
+== Setting instrumentation callbacks ==
+
+Function qi_ctrl_event_set() in "qemu-instr/control.h" can be used to set the
+instrumentation callback on each event to a user-specified function. Header
+"qemu-instr/events.h" provides the event identifiers and some pre-defined
+callbacks:
+
+* QI_EVENT_${EVENT}
+
+ Event identifier, passed to functions in "qemu-instr/control.h".
+
+* qi_event_${event}_nop
+
+ Do nothing.
+
+* qi_event_${event}_trace
+
+ Trace the event using whatever tracing backend QEMU has been configured with.
+
+* qi_event_${event}_gen_exec
+
+ Generate TCG code to raise the corresponding event when the TCG-generated code
+ is executed. Otherwise, the event will not be instrumented at execution time,
+ resulting in zero-overhead when executing the guest code.
+
+ Only available for translation-time events.
+
+* qi_event_${event}_trace_and_gen_exec
+
+ Combines 'qi_event_${event}_trace' and 'qi_event_${event}_gen_exec' in a
+ single call.
+
+ Only available for translation-time events.
+
+
+== Loading an instrumentation library ==
+
+There are two ways two load an instrumentation library:
+
+* Using the command-line "-instr" argument.
+
+* Using the "instr-load" and "instr-unload" commands in the HMP and QMP
+ interfaces.
+
+
+=== Example ===
+
+1. Configure QEMU with the selected events to instrument:
+
+ # instrument guest_cpu_enter and guest_mem_before
+ cat >/tmp/my-events <<EOF
+ guest_cpu_enter
+ guest_mem_before
+ EOF
+ mkdir -p /path/to/qemu-build
+ cd /path/to/qemu-build
+ /path/to/qemu-source/configure \
+ --enable-trace-instrument \
+ --with-instrument-events=/tmp/my-events \
+ --prefix=/path/to/qemu-install
+
+2. Build and install QEMU:
+
+ make install
+
+3. Create the "Makefile" to build the instrumentation library:
+
+ mkdir -p /tmp/my-instrument
+
+ cat > /tmp/my-instrument/Makefile <<EOF
+ QEMU_PATH=/tmp/qemu-install/
+
+ CFLAGS += -g
+ CFLAGS += -O3
+ CFLAGS += -Werror -Wall
+ CFLAGS += -I$(QEMU_PATH)/include
+
+ all: libtrace-instrument.la
+
+ libtrace-instrument.la: instrument.lo
+ libtool --mode=link --tag=CC $(CC) -module -rpath /usr/local/lib -o $@ $^
+
+ %.lo: %.c
+ libtool --mode=compile --tag=CC $(CC) $(CFLAGS) -c $^
+
+ clean:
+ $(RM) -f *.o *.so *.lo
+ $(RM) -Rf .libs
+ EOF
+
+4. Write your instrumentation library:
+
+ cat > /tmp/my-instrument/instrument.c <<EOF
+ #include <stdio.h>
+ #include <assert.h>
+
+ #include <qemu-instr/events.h> /* get event declarations */
+ #include <qemu-instr/control.h> /* manipulate events */
+
+ /* as documented in QEMU's event description */
+ struct mem_info {
+ uint8_t size_shift : 2;
+ bool sign_extend: 1;
+ uint8_t endianness : 1;
+ bool store : 1;
+ };
+
+ /* the address for the memory access is not known at translation time */
+ void guest_mem_before_trans(QICPU *cpu, QITCGv_cpu tcg_cpu,
+ QITCGv vaddr, uint8_t info)
+ {
+ struct mem_info *mi = (struct mem_info*)&info;
+ qi_event_guest_mem_before_trans_trace(cpu, tcg_cpu, vaddr, info);
+ if (mi->store) {
+ /* generate at execution time only for memory writes */
+ qi_event_guest_mem_before_trans_gen_exec(cpu, tcg_cpu, vaddr, info);
+ }
+ }
+
+ /* called when QEMU executes a memory access */
+ void guest_mem_before_exec(QICPU *cpu, uint64_t vaddr, uint8_t info)
+ {
+ struct mem_info *mi = (struct mem_info*)&info;
+ if (mi->store) {
+ /* if called by TCG code, we'll only get writes (see above) */
+ qi_event_guest_mem_before_exec_trace(cpu, vaddr, info);
+ }
+ }
+
+ /* called every time QEMU hotplugs a CPU */
+ void guest_cpu_enter(QICPU *cpu)
+ {
+ /* call the original tracing routine */
+ qi_event_guest_cpu_enter_trace(cpu);
+
+ /* disable instrumentation and tracing after the first call */
+ static bool found = false;
+ if (found) {
+ QIEvent *ev = QI_EVENT_GUEST_CPU_ENTER;
+ qi_ctrl_event_set(ev, NULL);
+ qi_trace_event_set_state_dynamic(ev, false);
+ } else {
+ found = true;
+ }
+ }
+
+
+ /* mandatory initialization callback */
+ void qi_init(int argc, const char **argv)
+ {
+ int i;
+ printf("init!\n");
+ printf(" argc :: %d\n", argc);
+ for (i = 0; i < argc; i++) {
+ printf(" -> %s\n", argv[i]);
+ }
+
+ /* instrument and trace events */
+ QIEvent *ev;
+
+ ev = QI_EVENT_GUEST_CPU_ENTER;
+ qi_ctrl_event_set(ev, guest_cpu_enter);
+ qi_trace_event_set_state_dynamic(ev, true);
+
+ ev = QI_EVENT_GUEST_MEM_BEFORE_TRANS;
+ qi_ctrl_event_set(ev, guest_mem_before_trans);
+ qi_trace_event_set_state_dynamic(ev, true);
+
+ ev = QI_EVENT_GUEST_MEM_BEFORE_EXEC;
+ qi_ctrl_event_set(ev, guest_mem_before_exec);
+ qi_trace_event_set_state_dynamic(ev, true);
+ }
+
+ /* mandatory finalization callback */
+ void qi_fini(void)
+ {
+ fprintf(stderr, "fini!\n");
+
+ /* ensure all tracing is disabled */
+ qi_trace_event_set_state_dynamic(QI_EVENT_GUEST_CPU_ENTER, false);
+ qi_trace_event_set_state_dynamic(QI_EVENT_GUEST_MEM_BEFORE_TRANS, false);
+ qi_trace_event_set_state_dynamic(QI_EVENT_GUEST_MEM_BEFORE_EXEC, false);
+
+ /* instrumentation callbacks are automatically reset by QEMU */
+ }
+ EOF
+
+5. Compile the instrumentation library:
+
+ make -C /tmp/my-instrument
+
+6. Start QEMU with the instrumentation library:
+
+ /tmp/qemu-install/bin/qemu-system-x86_64 \
+ -instr file=/tmp/my-dinstrument/.libs/libtrace-instrument.so, \
+ arg=foo,arg=bar
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 02/13] instrument: [none] Add null instrumentation mode
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
2017-07-24 17:06 ` [Qemu-devel] [PATCH 01/13] instrument: Add documentation Lluís Vilanova
@ 2017-07-24 17:10 ` Lluís Vilanova
2017-07-24 17:14 ` [Qemu-devel] [PATCH 03/13] instrument: [dynamic] Add dynamic " Lluís Vilanova
` (11 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
Renames existing tracing routines into "_backend__trace_*", and instead adds an
instrumentation-aware "trace_*".
Since we're in a "null" instrumentation mode for tracing events, the new
routines simply call the original tracing code.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
.gitignore | 3 +
Makefile | 26 ++++++++++
Makefile.objs | 4 ++
configure | 8 +++
include/trace-tcg.h | 1
instrument/Makefile.objs | 14 ++++++
scripts/tracetool/__init__.py | 6 ++
scripts/tracetool/backend/instr_none.py | 34 +++++++++++++
scripts/tracetool/format/h.py | 8 +++
scripts/tracetool/format/instr_h.py | 48 +++++++++++++++++++
scripts/tracetool/format/instr_tcg_h.py | 56 ++++++++++++++++++++++
scripts/tracetool/format/tcg_h.py | 6 +-
scripts/tracetool/format/tcg_helper_c.py | 2 -
scripts/tracetool/format/tcg_helper_h.py | 2 -
scripts/tracetool/format/tcg_helper_wrapper_h.py | 2 -
15 files changed, 210 insertions(+), 10 deletions(-)
create mode 100644 instrument/Makefile.objs
create mode 100644 scripts/tracetool/backend/instr_none.py
create mode 100644 scripts/tracetool/format/instr_h.py
create mode 100644 scripts/tracetool/format/instr_tcg_h.py
diff --git a/.gitignore b/.gitignore
index 09c2363acf..068dc1c1ae 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,6 +5,8 @@
/config-target.*
/config.status
/config-temp
+/instrument-root.h
+/instrument/generated-tcg-tracers.h
/trace-events-all
/trace/generated-events.h
/trace/generated-events.c
@@ -116,6 +118,7 @@ tags
TAGS
docker-src.*
*~
+instrument.h
trace.h
trace.c
trace-ust.h
diff --git a/Makefile b/Makefile
index 38814f9a61..375bd313d7 100644
--- a/Makefile
+++ b/Makefile
@@ -191,6 +191,29 @@ trace-dtrace-root.h: trace-dtrace-root.dtrace
trace-dtrace-root.o: trace-dtrace-root.dtrace
+INSTRUMENT_HEADERS = instrument-root.h $(trace-events-subdirs:%=%/instrument.h)
+INSTRUMENT_HEADERS += instrument/generated-tcg-tracers.h
+
+GENERATED_FILES += $(INSTRUMENT_HEADERS)
+
+%/instrument.h: %/instrument.h-timestamp
+ @cmp $< $@ >/dev/null 2>&1 || cp $< $@
+%/instrument.h-timestamp: $(SRC_PATH)/%/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=$(call trace-group-name,$@) \
+ --format=instr-h \
+ --backends=$(TRACE_INSTRUMENT_BACKEND) \
+ $< > $@,"GEN","$(@:%-timestamp=%)")
+
+instrument-root.h: instrument-root.h-timestamp
+ @cmp $< $@ >/dev/null 2>&1 || cp $< $@
+instrument-root.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=root \
+ --format=instr-h \
+ --backends=$(TRACE_INSTRUMENT_BACKEND) \
+ $< > $@,"GEN","$(@:%-timestamp=%)")
+
# Don't try to regenerate Makefile or configure
# We don't generate any of them
Makefile: ;
@@ -806,7 +829,8 @@ endif
.SECONDARY: $(TRACE_HEADERS) $(TRACE_HEADERS:%=%-timestamp) \
$(TRACE_SOURCES) $(TRACE_SOURCES:%=%-timestamp) \
- $(TRACE_DTRACE) $(TRACE_DTRACE:%=%-timestamp)
+ $(TRACE_DTRACE) $(TRACE_DTRACE:%=%-timestamp) \
+ $(INSTRUMENT_HEADERS) $(INSTRUMENT_HEADERS:%=%-timestamp)
# Include automatically generated dependency files
# Dependencies in Makefile.objs files come from our recursive subdir rules
diff --git a/Makefile.objs b/Makefile.objs
index bfd5a6ceb1..8e2b3770c4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -98,6 +98,10 @@ util-obj-y += trace/
target-obj-y += trace/
######################################################################
+# instrumentation
+util-obj-y += instrument/
+
+######################################################################
# guest agent
# FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed
diff --git a/configure b/configure
index a3f0522e8f..75d899b40c 100755
--- a/configure
+++ b/configure
@@ -351,6 +351,8 @@ pie=""
qom_cast_debug="yes"
trace_backends="log"
trace_file="trace"
+trace_instrument="no"
+trace_instrument_backend="none"
spice=""
rbd=""
smartcard=""
@@ -5282,6 +5284,7 @@ echo "Trace backends $trace_backends"
if have_backend "simple"; then
echo "Trace output file $trace_file-<pid>"
fi
+echo "Trace instrumentation $trace_instrument"
echo "spice support $spice $(echo_version $spice $spice_protocol_version/$spice_server_version)"
echo "rbd support $rbd"
echo "xfsctl support $xfs"
@@ -5989,6 +5992,11 @@ else
fi
QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
+##########################################
+# trace instrumentation
+echo "TRACE_INSTRUMENT_BACKEND=instr-$trace_instrument_backend" >> $config_host_mak
+##########################################
+
echo "TOOLS=$tools" >> $config_host_mak
echo "ROMS=$roms" >> $config_host_mak
echo "MAKE=$make" >> $config_host_mak
diff --git a/include/trace-tcg.h b/include/trace-tcg.h
index da68608c85..a626c5020c 100644
--- a/include/trace-tcg.h
+++ b/include/trace-tcg.h
@@ -2,5 +2,6 @@
#define TRACE_TCG_H
#include "trace/generated-tcg-tracers.h"
+#include "instrument/generated-tcg-tracers.h"
#endif /* TRACE_TCG_H */
diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs
new file mode 100644
index 0000000000..d1edd1696e
--- /dev/null
+++ b/instrument/Makefile.objs
@@ -0,0 +1,14 @@
+# -*- mode: makefile -*-
+
+######################################################################
+# QEMU trace->instrument interface
+
+$(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
+ @cmp $< $@ >/dev/null 2>&1 || cp $< $@
+$(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+ @mkdir -p $(dir $@)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=root \
+ --format=instr-tcg-h \
+ --backend=$(TRACE_INSTRUMENT_BACKEND) \
+ $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index d4c204a472..6d382157c0 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -161,7 +161,7 @@ class Event(object):
"(?:(?:(?P<fmt_trans>\".+),)?\s*(?P<fmt>\".+))?"
"\s*")
- _VALID_PROPS = set(["disable", "tcg", "tcg-trans", "tcg-exec", "vcpu"])
+ _VALID_PROPS = set(["disable", "tcg", "tcg-trans", "tcg-exec", "vcpu", "instrument"])
def __init__(self, name, props, fmt, args, orig=None,
event_trans=None, event_exec=None):
@@ -269,13 +269,15 @@ class Event(object):
QEMU_TRACE = "trace_%(name)s"
QEMU_TRACE_NOCHECK = "_nocheck__" + QEMU_TRACE
+ QEMU_TRACE_BACKEND = "_backend__" + QEMU_TRACE
QEMU_TRACE_TCG = QEMU_TRACE + "_tcg"
+ QEMU_TRACE_TCG_BACKEND = "_backend__" + QEMU_TRACE_TCG
QEMU_DSTATE = "_TRACE_%(NAME)s_DSTATE"
QEMU_EVENT = "_TRACE_%(NAME)s_EVENT"
def api(self, fmt=None):
if fmt is None:
- fmt = Event.QEMU_TRACE
+ fmt = Event.QEMU_TRACE_BACKEND
return fmt % {"name": self.name, "NAME": self.name.upper()}
def transform(self, *trans):
diff --git a/scripts/tracetool/backend/instr_none.py b/scripts/tracetool/backend/instr_none.py
new file mode 100644
index 0000000000..56508cfcbe
--- /dev/null
+++ b/scripts/tracetool/backend/instr_none.py
@@ -0,0 +1,34 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+No-instrumentation proxy.
+"""
+
+__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+
+
+##################################################
+# instr-tcg-h
+
+def generate_instr_tcg_h(event, group):
+ out(' %(backend)s(%(argnames)s);',
+ backend=event.api(event.QEMU_TRACE_TCG_BACKEND),
+ argnames=", ".join(event.args.names()))
+
+
+##################################################
+# instr-h
+
+def generate_instr_h(event, group):
+ out(' %(backend)s(%(argnames)s);',
+ backend=event.api(event.QEMU_TRACE_BACKEND),
+ argnames=", ".join(event.args.names()))
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index aecf249d66..2e423079db 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -87,4 +87,10 @@ def generate(events, backend, group):
backend.generate_end(events, group)
- out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
+ out('')
+ if group == "root":
+ out('#include "instrument-root.h"')
+ else:
+ out('#include "instrument.h"')
+ out('',
+ '#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
diff --git a/scripts/tracetool/format/instr_h.py b/scripts/tracetool/format/instr_h.py
new file mode 100644
index 0000000000..2b80b3d038
--- /dev/null
+++ b/scripts/tracetool/format/instr_h.py
@@ -0,0 +1,48 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+/instrument-root.h
+.../instrument.h
+
+"""
+
+__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+from tracetool.transform import *
+
+
+def generate(events, backend, group):
+ out('/* This file is autogenerated by tracetool, do not edit. */',
+ '',
+ '#ifndef INSTRUMENT_%s_GENERATED_TRACERS_H' % group.upper(),
+ '#define INSTRUMENT_%s_GENERATED_TRACERS_H' % group.upper(),
+ '')
+ backend.generate_begin(events, group)
+
+ for e in events:
+ out('static inline void %(qemu)s(%(args)s)',
+ '{',
+ qemu=e.api(e.QEMU_TRACE),
+ args=e.args)
+
+ if "tcg-trans" in e.properties or "instrument" not in e.properties:
+ # "*_trans" never called directly (and never instrumented)
+ out(' %(backend)s(%(argnames)s);',
+ backend=e.api(e.QEMU_TRACE_BACKEND),
+ argnames=", ".join(e.args.names()))
+ else:
+ backend.generate(e, group)
+
+ out('}')
+
+ backend.generate_end(events, group)
+ out('',
+ '#endif /* INSTRUMENT_%s_GENERATED_TRACERS_H */' % group.upper())
diff --git a/scripts/tracetool/format/instr_tcg_h.py b/scripts/tracetool/format/instr_tcg_h.py
new file mode 100644
index 0000000000..6abf97d368
--- /dev/null
+++ b/scripts/tracetool/format/instr_tcg_h.py
@@ -0,0 +1,56 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+instrument/generated-tcg-tracers.h
+
+"""
+
+__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out, Arguments
+import tracetool.vcpu
+
+
+API = True
+
+
+def generate(events, backend, group):
+ events = [e.original for e in events
+ if "tcg-trans" in e.properties]
+
+ out('/* This file is autogenerated by tracetool, do not edit. */',
+ '',
+ '#ifndef INSTRUMENT_%s_GENERATED_TCG_TRACERS_H' % group.upper(),
+ '#define INSTRUMENT_%s_GENERATED_TCG_TRACERS_H' % group.upper(),
+ '')
+ backend.generate_begin(events, group)
+
+ for e in events:
+ args_api = tracetool.vcpu.transform_args("tcg_h", e)
+
+ out('static inline void %(qemu)s(%(args)s)',
+ '{',
+ qemu=e.api(e.QEMU_TRACE_TCG),
+ args=args_api)
+
+ if "instrument" in e.properties:
+ e = e.copy()
+ e.args = args_api
+ backend.generate(e, group)
+ else:
+ out(' %(backend)s(%(argnames)s);',
+ backend=e.api(e.QEMU_TRACE_TCG_BACKEND),
+ argnames=", ".join(args_api.names()))
+
+ out('}')
+
+ backend.generate_end(events, group)
+ out('',
+ '#endif /* INSTRUMENT_%s_GENERATED_TCG_TRACERS_H */' % group.upper())
diff --git a/scripts/tracetool/format/tcg_h.py b/scripts/tracetool/format/tcg_h.py
index 1651cc3f71..3363502ffc 100644
--- a/scripts/tracetool/format/tcg_h.py
+++ b/scripts/tracetool/format/tcg_h.py
@@ -51,7 +51,7 @@ def generate(events, backend, group):
out('static inline void %(name_tcg)s(%(args)s)',
'{',
- name_tcg=e.original.api(e.QEMU_TRACE_TCG),
+ name_tcg=e.original.api(e.QEMU_TRACE_TCG_BACKEND),
args=tracetool.vcpu.transform_args("tcg_h", e.original))
if "disable" not in e.properties:
@@ -72,8 +72,8 @@ def generate(events, backend, group):
' if (%(cond)s) {',
' gen_helper_%(name_exec)s(%(argnames_exec)s);',
' }',
- name_trans=e.original.event_trans.api(e.QEMU_TRACE),
- name_exec=e.original.event_exec.api(e.QEMU_TRACE),
+ name_trans=e.original.event_trans.api(e.QEMU_TRACE_BACKEND),
+ name_exec=e.original.event_exec.api(e.QEMU_TRACE_BACKEND),
argnames_trans=", ".join(args_trans.names()),
argnames_exec=", ".join(args_exec.names()),
cond=cond)
diff --git a/scripts/tracetool/format/tcg_helper_c.py b/scripts/tracetool/format/tcg_helper_c.py
index bbbd6ad0f4..e9ba62a136 100644
--- a/scripts/tracetool/format/tcg_helper_c.py
+++ b/scripts/tracetool/format/tcg_helper_c.py
@@ -74,7 +74,7 @@ def generate(events, backend, group):
# NOTE: the check was already performed at TCG-generation time
' %(name)s(%(args_call)s);',
'}',
- name_tcg="helper_%s_proxy" % e.api(),
+ name_tcg="helper_%s_proxy" % e.api(e.QEMU_TRACE_BACKEND),
name=e.api(e.QEMU_TRACE_NOCHECK),
args_api=e_args_api,
args_call=", ".join(e_args_call.casted()),
diff --git a/scripts/tracetool/format/tcg_helper_h.py b/scripts/tracetool/format/tcg_helper_h.py
index 6b184b641b..17ef0021eb 100644
--- a/scripts/tracetool/format/tcg_helper_h.py
+++ b/scripts/tracetool/format/tcg_helper_h.py
@@ -44,6 +44,6 @@ def generate(events, backend, group):
out(fmt,
flags=flags,
argc=len(args),
- name=e.api() + "_proxy",
+ name=e.api(e.QEMU_TRACE_BACKEND) + "_proxy",
types=types,
)
diff --git a/scripts/tracetool/format/tcg_helper_wrapper_h.py b/scripts/tracetool/format/tcg_helper_wrapper_h.py
index ff53447512..377af4615b 100644
--- a/scripts/tracetool/format/tcg_helper_wrapper_h.py
+++ b/scripts/tracetool/format/tcg_helper_wrapper_h.py
@@ -54,7 +54,7 @@ def generate(events, backend, group):
for (type_, name) in args_tcg_compat
]
- gen_name = "gen_helper_" + e.api()
+ gen_name = "gen_helper_" + e.api(e.QEMU_TRACE_BACKEND)
out('static inline void %(name)s(%(args)s)',
'{',
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 03/13] instrument: [dynamic] Add dynamic instrumentation mode
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
2017-07-24 17:06 ` [Qemu-devel] [PATCH 01/13] instrument: Add documentation Lluís Vilanova
2017-07-24 17:10 ` [Qemu-devel] [PATCH 02/13] instrument: [none] Add null instrumentation mode Lluís Vilanova
@ 2017-07-24 17:14 ` Lluís Vilanova
2017-07-24 17:18 ` [Qemu-devel] [PATCH 04/13] instrument: Allow adding the "instrument" property without modifying event files Lluís Vilanova
` (10 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
This mode uses a function pointer in "trace_*" routines. By default, it points
to the original tracing function (now "_backend__trace_*").
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
.gitignore | 3 +
Makefile | 26 ++++++++++-
Makefile.objs | 4 ++
configure | 9 ++++
instrument/Makefile.objs | 12 +++++
scripts/tracetool/__init__.py | 3 +
scripts/tracetool/backend/instr_dynamic.py | 67 ++++++++++++++++++++++++++++
scripts/tracetool/format/instr_c.py | 44 ++++++++++++++++++
scripts/tracetool/format/instr_tcg_c.py | 40 +++++++++++++++++
9 files changed, 206 insertions(+), 2 deletions(-)
create mode 100644 scripts/tracetool/backend/instr_dynamic.py
create mode 100644 scripts/tracetool/format/instr_c.py
create mode 100644 scripts/tracetool/format/instr_tcg_c.py
diff --git a/.gitignore b/.gitignore
index 068dc1c1ae..ee2768cb05 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,7 +6,9 @@
/config.status
/config-temp
/instrument-root.h
+/instrument-root.c
/instrument/generated-tcg-tracers.h
+/instrument/generated-tcg-tracers.c
/trace-events-all
/trace/generated-events.h
/trace/generated-events.c
@@ -119,6 +121,7 @@ TAGS
docker-src.*
*~
instrument.h
+instrument.c
trace.h
trace.c
trace-ust.h
diff --git a/Makefile b/Makefile
index 375bd313d7..fb226bf54b 100644
--- a/Makefile
+++ b/Makefile
@@ -193,8 +193,11 @@ trace-dtrace-root.o: trace-dtrace-root.dtrace
INSTRUMENT_HEADERS = instrument-root.h $(trace-events-subdirs:%=%/instrument.h)
INSTRUMENT_HEADERS += instrument/generated-tcg-tracers.h
+INSTRUMENT_SOURCES = instrument-root.c $(trace-events-subdirs:%=%/instrument.c)
+INSTRUMENT_SOURCES += instrument/generated-tcg-tracers.c
GENERATED_FILES += $(INSTRUMENT_HEADERS)
+GENERATED_FILES += $(INSTRUMENT_SOURCES)
%/instrument.h: %/instrument.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
@@ -205,6 +208,15 @@ GENERATED_FILES += $(INSTRUMENT_HEADERS)
--backends=$(TRACE_INSTRUMENT_BACKEND) \
$< > $@,"GEN","$(@:%-timestamp=%)")
+%/instrument.c: %/instrument.c-timestamp
+ @cmp $< $@ >/dev/null 2>&1 || cp $< $@
+%/instrument.c-timestamp: $(SRC_PATH)/%/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=$(call trace-group-name,$@) \
+ --format=instr-c \
+ --backends=$(TRACE_INSTRUMENT_BACKEND) \
+ $< > $@,"GEN","$(@:%-timestamp=%)")
+
instrument-root.h: instrument-root.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
instrument-root.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
@@ -214,6 +226,15 @@ instrument-root.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.m
--backends=$(TRACE_INSTRUMENT_BACKEND) \
$< > $@,"GEN","$(@:%-timestamp=%)")
+instrument-root.c: instrument-root.c-timestamp
+ @cmp $< $@ >/dev/null 2>&1 || cp $< $@
+instrument-root.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=root \
+ --format=instr-c \
+ --backends=$(TRACE_INSTRUMENT_BACKEND) \
+ $< > $@,"GEN","$(@:%-timestamp=%)")
+
# Don't try to regenerate Makefile or configure
# We don't generate any of them
Makefile: ;
@@ -390,7 +411,7 @@ Makefile: $(version-obj-y)
# Build libraries
libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y) $(trace-obj-y)
+libqemuutil.a: $(util-obj-y) $(trace-obj-y) $(instr-obj-y)
######################################################################
@@ -830,7 +851,8 @@ endif
.SECONDARY: $(TRACE_HEADERS) $(TRACE_HEADERS:%=%-timestamp) \
$(TRACE_SOURCES) $(TRACE_SOURCES:%=%-timestamp) \
$(TRACE_DTRACE) $(TRACE_DTRACE:%=%-timestamp) \
- $(INSTRUMENT_HEADERS) $(INSTRUMENT_HEADERS:%=%-timestamp)
+ $(INSTRUMENT_HEADERS) $(INSTRUMENT_HEADERS:%=%-timestamp) \
+ $(INSTRUMENT_SOURCES) $(INSTRUMENT_SOURCES:%=%-timestamp)
# Include automatically generated dependency files
# Dependencies in Makefile.objs files come from our recursive subdir rules
diff --git a/Makefile.objs b/Makefile.objs
index 8e2b3770c4..e36a519952 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -100,6 +100,7 @@ target-obj-y += trace/
######################################################################
# instrumentation
util-obj-y += instrument/
+target-obj-y += instrument/
######################################################################
# guest agent
@@ -180,3 +181,6 @@ 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)
+
+instr-obj-$(CONFIG_INSTRUMENT) = instrument-root.o
+instr-obj-y += $(trace-events-subdirs:%=%/instrument.o)
diff --git a/configure b/configure
index 75d899b40c..13191052cb 100755
--- a/configure
+++ b/configure
@@ -897,6 +897,10 @@ for opt do
;;
--with-trace-file=*) trace_file="$optarg"
;;
+ --enable-trace-instrument)
+ trace_instrument="yes"
+ trace_instrument_backend="dynamic"
+ ;;
--enable-gprof) gprof="yes"
;;
--enable-gcov) gcov="yes"
@@ -1427,6 +1431,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>
+ --enable-trace-instrument
+ Enable trace instrumentation
--disable-slirp disable SLIRP userspace network connectivity
--enable-tcg-interpreter enable TCG with bytecode interpreter (TCI)
--oss-lib path to OSS library
@@ -5995,6 +6001,9 @@ QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
##########################################
# trace instrumentation
echo "TRACE_INSTRUMENT_BACKEND=instr-$trace_instrument_backend" >> $config_host_mak
+if test "$trace_instrument" = "yes"; then
+ echo "CONFIG_INSTRUMENT=y" >> $config_host_mak
+fi
##########################################
echo "TOOLS=$tools" >> $config_host_mak
diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs
index d1edd1696e..c548bbdd8a 100644
--- a/instrument/Makefile.objs
+++ b/instrument/Makefile.objs
@@ -12,3 +12,15 @@ $(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/
--format=instr-tcg-h \
--backend=$(TRACE_INSTRUMENT_BACKEND) \
$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+
+$(obj)/generated-tcg-tracers.c: $(obj)/generated-tcg-tracers.c-timestamp
+ @cmp $< $@ >/dev/null 2>&1 || cp $< $@
+$(obj)/generated-tcg-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
+ @mkdir -p $(dir $@)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=root \
+ --format=instr-tcg-c \
+ --backend=$(TRACE_INSTRUMENT_BACKEND) \
+ $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+
+target-obj-$(CONFIG_INSTRUMENT) += generated-tcg-tracers.o
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 6d382157c0..e65349bc33 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -275,6 +275,9 @@ class Event(object):
QEMU_DSTATE = "_TRACE_%(NAME)s_DSTATE"
QEMU_EVENT = "_TRACE_%(NAME)s_EVENT"
+ QI_TRACE_INSTRUMENT = "qi_event_%(name)s"
+ QI_TRACE_INSTRUMENT_TCG = QI_TRACE_INSTRUMENT + "_tcg"
+
def api(self, fmt=None):
if fmt is None:
fmt = Event.QEMU_TRACE_BACKEND
diff --git a/scripts/tracetool/backend/instr_dynamic.py b/scripts/tracetool/backend/instr_dynamic.py
new file mode 100644
index 0000000000..f42d3afa8f
--- /dev/null
+++ b/scripts/tracetool/backend/instr_dynamic.py
@@ -0,0 +1,67 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Dynamic instrumentation proxy.
+
+"""
+
+__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+from tracetool.transform import *
+import tracetool.vcpu
+
+
+##################################################
+# instr-h
+
+def generate_instr_h_begin(events, group):
+ for event in events:
+ if "instrument" not in event.properties:
+ continue
+ out('extern void * %(qi)s_cb;',
+ qi=event.api(event.QI_TRACE_INSTRUMENT))
+ out('')
+
+
+def generate_instr_h(event, group):
+ argtypes = ", ".join(event.args.types())
+ if argtypes == "":
+ argtypes = "void"
+ out(' void (*func)(%(argtypes)s) = %(qi)s_cb;',
+ ' func(%(argnames)s);',
+ qi=event.api(event.QI_TRACE_INSTRUMENT),
+ args=event.args,
+ argnames=", ".join(event.args.names()),
+ argtypes=argtypes)
+
+
+##################################################
+# instr-tcg-h
+
+def generate_instr_tcg_h_begin(events, group):
+ for event in events:
+ if "instrument" not in event.properties:
+ continue
+ out('extern void * %(qi)s_cb;',
+ qi=event.api(event.QI_TRACE_INSTRUMENT_TCG))
+ out('')
+
+
+def generate_instr_tcg_h(event, group):
+ argtypes = ", ".join(event.args.types())
+ if argtypes == "":
+ argtypes = "void"
+ out(' void (*func)(%(argtypes)s) = %(qi)s_cb;',
+ ' func(%(argnames)s);',
+ qi=event.api(event.QI_TRACE_INSTRUMENT_TCG),
+ args=event.args,
+ argnames=", ".join(event.args.names()),
+ argtypes=argtypes)
diff --git a/scripts/tracetool/format/instr_c.py b/scripts/tracetool/format/instr_c.py
new file mode 100644
index 0000000000..987ecbf5c2
--- /dev/null
+++ b/scripts/tracetool/format/instr_c.py
@@ -0,0 +1,44 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+/instrument-root.c
+.../instrument.c
+
+"""
+
+__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+from tracetool.transform import *
+
+
+def generate(events, backend, group):
+ events = [e for e in events
+ if "tcg-trans" not in e.properties and
+ "instrument" in e.properties]
+
+ if group == "root":
+ header = "trace-root.h"
+ else:
+ header = "trace.h"
+
+ out('/* This file is autogenerated by tracetool, do not edit. */',
+ '',
+ '#include "qemu/osdep.h"',
+ '#include "%s"' % header,
+ '')
+ backend.generate_begin(events, group)
+
+ for e in events:
+ out('void *%(qi_cb)s_cb = %(qi_trace)s;',
+ qi_cb=e.api(e.QI_TRACE_INSTRUMENT),
+ qi_trace=e.api(e.QEMU_TRACE_BACKEND))
+
+ backend.generate_end(events, group)
diff --git a/scripts/tracetool/format/instr_tcg_c.py b/scripts/tracetool/format/instr_tcg_c.py
new file mode 100644
index 0000000000..382709d0bd
--- /dev/null
+++ b/scripts/tracetool/format/instr_tcg_c.py
@@ -0,0 +1,40 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+instrument/generated-tcg-tracers.c
+
+"""
+
+__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+from tracetool.transform import *
+
+
+def generate(events, backend, group):
+ events = [e.original for e in events
+ if "tcg-trans" in e.properties and
+ "instrument" in e.properties]
+
+ out('/* This file is autogenerated by tracetool, do not edit. */',
+ '',
+ '#include "qemu/osdep.h"',
+ '#include "cpu.h"',
+ '#include "tcg-op.h"',
+ '#include "trace-tcg.h"',
+ '')
+ backend.generate_begin(events, group)
+
+ for e in events:
+ out('void *%(qi_cb)s_cb = %(qi_trace)s;',
+ qi_cb=e.api(e.QI_TRACE_INSTRUMENT_TCG),
+ qi_trace=e.api(e.QEMU_TRACE_TCG_BACKEND))
+
+ backend.generate_end(events, group)
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 04/13] instrument: Allow adding the "instrument" property without modifying event files
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (2 preceding siblings ...)
2017-07-24 17:14 ` [Qemu-devel] [PATCH 03/13] instrument: [dynamic] Add dynamic " Lluís Vilanova
@ 2017-07-24 17:18 ` Lluís Vilanova
2017-07-24 17:22 ` [Qemu-devel] [PATCH 05/13] instrument: [dynamic] Add default public per-event functions Lluís Vilanova
` (9 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
Adds a configure argument to allow users to add the instrument property
to events without having to modify any trace event files.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
configure | 17 +++++++++++++++++
rules.mak | 3 +++
scripts/tracetool.py | 11 +++++++++--
scripts/tracetool/__init__.py | 20 +++++++++++++++++++-
4 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 13191052cb..8ab2a36130 100755
--- a/configure
+++ b/configure
@@ -353,6 +353,7 @@ trace_backends="log"
trace_file="trace"
trace_instrument="no"
trace_instrument_backend="none"
+instrument_events=""
spice=""
rbd=""
smartcard=""
@@ -901,6 +902,13 @@ for opt do
trace_instrument="yes"
trace_instrument_backend="dynamic"
;;
+ --with-instrument-events=*)
+ if test ! -f "$optarg"; then
+ echo "ERROR: no such file: $optarg"
+ exit 1
+ fi
+ instrument_events="`realpath $optarg`"
+ ;;
--enable-gprof) gprof="yes"
;;
--enable-gcov) gcov="yes"
@@ -1433,6 +1441,9 @@ Advanced options (experts only):
Default:trace-<pid>
--enable-trace-instrument
Enable trace instrumentation
+ --with-instrument-events=FILE
+ File with a list of events to enable instrumentation
+ on (one per line)
--disable-slirp disable SLIRP userspace network connectivity
--enable-tcg-interpreter enable TCG with bytecode interpreter (TCI)
--oss-lib path to OSS library
@@ -5291,6 +5302,9 @@ if have_backend "simple"; then
echo "Trace output file $trace_file-<pid>"
fi
echo "Trace instrumentation $trace_instrument"
+if test -n "$instrument_events"; then
+echo "Instrument events $instrument_events"
+fi
echo "spice support $spice $(echo_version $spice $spice_protocol_version/$spice_server_version)"
echo "rbd support $rbd"
echo "xfsctl support $xfs"
@@ -6004,6 +6018,9 @@ echo "TRACE_INSTRUMENT_BACKEND=instr-$trace_instrument_backend" >> $config_host_
if test "$trace_instrument" = "yes"; then
echo "CONFIG_INSTRUMENT=y" >> $config_host_mak
fi
+if test -n "$instrument_events"; then
+ echo "CONFIG_INSTRUMENT_EVENTS=$instrument_events" >> $config_host_mak
+fi
##########################################
echo "TOOLS=$tools" >> $config_host_mak
diff --git a/rules.mak b/rules.mak
index 6e943335f3..fbeda68022 100644
--- a/rules.mak
+++ b/rules.mak
@@ -188,6 +188,9 @@ notempty = $(if $1,y,n)
# Generate files with tracetool
TRACETOOL=$(PYTHON) $(SRC_PATH)/scripts/tracetool.py
+ifdef CONFIG_INSTRUMENT_EVENTS
+TRACETOOL += --instrument=$(CONFIG_INSTRUMENT_EVENTS)
+endif
# Generate timestamp files for .h include files
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index c55a21518b..d5d212e98f 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -6,7 +6,7 @@ Command-line wrapper for the tracetool machinery.
"""
__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__ = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
__license__ = "GPL version 2 or (at your option) any later version"
__maintainer__ = "Stefan Hajnoczi"
@@ -70,6 +70,7 @@ def main(args):
long_opts = ["backends=", "format=", "help", "list-backends",
"check-backends", "group="]
long_opts += ["binary=", "target-type=", "target-name=", "probe-prefix="]
+ long_opts += ["instrument="]
try:
opts, args = getopt.getopt(args[1:], "", long_opts)
@@ -84,6 +85,7 @@ def main(args):
target_type = None
target_name = None
probe_prefix = None
+ instrument = None
for opt, arg in opts:
if opt == "--help":
error_opt()
@@ -111,6 +113,9 @@ def main(args):
elif opt == '--probe-prefix':
probe_prefix = arg
+ elif opt == "--instrument":
+ instrument = arg
+
else:
error_opt("unhandled option: %s" % opt)
@@ -137,12 +142,14 @@ def main(args):
if probe_prefix is None:
probe_prefix = ".".join(["qemu", target_type, target_name])
+ instrument_cre = tracetool.read_instrument(instrument)
+
if len(args) < 1:
error_opt("missing trace-events filepath")
events = []
for arg in args:
with open(arg, "r") as fh:
- events.extend(tracetool.read_events(fh))
+ events.extend(tracetool.read_events(fh, instrument_cre))
try:
tracetool.generate(events, arg_group, arg_format, arg_backends,
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index e65349bc33..b9ddf8fbf9 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -292,13 +292,26 @@ class Event(object):
self)
-def read_events(fobj):
+def read_instrument(path):
+ if path is None:
+ return None
+
+ with open(path) as f:
+ patterns = f.readlines()
+ patterns = [p.replace("\n", "") for p in patterns]
+ patterns = [p.replace("*", ".*") for p in patterns]
+ pattern = "|".join(patterns)
+ return re.compile(pattern)
+
+def read_events(fobj, instrument):
"""Generate the output for the given (format, backends) pair.
Parameters
----------
fobj : file
Event description file.
+ instrument : cre or None
+ Event patterns to instrument.
Returns a list of Event objects
"""
@@ -312,6 +325,11 @@ def read_events(fobj):
event = Event.build(line)
+ if instrument is not None and instrument.match(event.name):
+ event.properties = [p for p in event.properties
+ if p != "disable"]
+ event.properties.append("instrument")
+
# transform TCG-enabled events
if "tcg" not in event.properties:
events.append(event)
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 05/13] instrument: [dynamic] Add default public per-event functions
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (3 preceding siblings ...)
2017-07-24 17:18 ` [Qemu-devel] [PATCH 04/13] instrument: Allow adding the "instrument" property without modifying event files Lluís Vilanova
@ 2017-07-24 17:22 ` Lluís Vilanova
2017-07-24 17:26 ` [Qemu-devel] [PATCH 06/13] instrument: Add event control interface Lluís Vilanova
` (8 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
For each event, adds a set of public routines available to instrumentation
clients:
* qi_event_*_nop:
Do nothing (to disable an instrumentation event).
* qi_event_*_trace
Trace event using QEMU's tracing system. Default for execution-time events.
* qi_event_*_gen_exec
Generate TCG code to raise the event at execution time. Only available for
translation-time events.
* qi_event_*_trace_and_gen_exec
Combination of qi_event_*_trace and qi_event_*_gen_exec. Only available for
translation-time events. Default for translation-time events.
The functions are defined for all events, regardless of whether they are
instrumented or disabled (used later).
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
.gitignore | 1
Makefile | 14 +++++
configure | 2 +
instrument/Makefile.objs | 14 +++++
instrument/qemu-instr/types.h | 73 +++++++++++++++++++++++
instrument/qemu-instr/visibility-internal.h | 58 +++++++++++++++++++
scripts/tracetool/__init__.py | 4 +
scripts/tracetool/backend/instr_dynamic.py | 84 +++++++++++++++++++++++++++
scripts/tracetool/backend/instr_none.py | 18 ++++++
scripts/tracetool/format/instr_api_h.py | 65 +++++++++++++++++++++
scripts/tracetool/format/instr_c.py | 7 +-
scripts/tracetool/format/instr_tcg_c.py | 7 +-
scripts/tracetool/format/tcg_helper_c.py | 2 -
scripts/tracetool/transform.py | 13 ++++
14 files changed, 351 insertions(+), 11 deletions(-)
create mode 100644 instrument/qemu-instr/types.h
create mode 100644 instrument/qemu-instr/visibility-internal.h
create mode 100644 scripts/tracetool/format/instr_api_h.py
diff --git a/.gitignore b/.gitignore
index ee2768cb05..d3aa8286b0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,6 +9,7 @@
/instrument-root.c
/instrument/generated-tcg-tracers.h
/instrument/generated-tcg-tracers.c
+/instrument/qemu-instr/events.h
/trace-events-all
/trace/generated-events.h
/trace/generated-events.c
diff --git a/Makefile b/Makefile
index fb226bf54b..c8be326790 100644
--- a/Makefile
+++ b/Makefile
@@ -193,6 +193,9 @@ trace-dtrace-root.o: trace-dtrace-root.dtrace
INSTRUMENT_HEADERS = instrument-root.h $(trace-events-subdirs:%=%/instrument.h)
INSTRUMENT_HEADERS += instrument/generated-tcg-tracers.h
+ifeq ($(TRACE_INSTRUMENT_BACKEND),instr-dynamic)
+INSTRUMENT_HEADERS += instrument/qemu-instr/events.h
+endif
INSTRUMENT_SOURCES = instrument-root.c $(trace-events-subdirs:%=%/instrument.c)
INSTRUMENT_SOURCES += instrument/generated-tcg-tracers.c
@@ -644,8 +647,13 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
endif
endif
+install-includedir:
+ $(INSTALL_DIR) "$(DESTDIR)$(includedir)"
install: all $(if $(BUILD_DOCS),install-doc) install-datadir install-localstatedir
+ifeq ($(TRACE_INSTRUMENT_BACKEND),instr-dynamic)
+install: install-includedir
+endif
ifneq ($(TOOLS),)
$(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
endif
@@ -676,6 +684,12 @@ endif
for d in $(TARGET_DIRS); do \
$(MAKE) $(SUBDIR_MAKEFLAGS) TARGET_DIR=$$d/ -C $$d $@ || exit 1 ; \
done
+ifeq ($(TRACE_INSTRUMENT_BACKEND),instr-dynamic)
+ $(INSTALL_DIR) "$(DESTDIR)$(includedir)/qemu-instr"
+ $(INSTALL_DATA) $(SRC_PATH)/instrument/qemu-instr/types.h "$(DESTDIR)$(includedir)/qemu-instr/"
+ $(INSTALL_DATA) $(SRC_PATH)/instrument/qemu-instr/visibility-internal.h "$(DESTDIR)$(includedir)/qemu-instr/"
+ $(INSTALL_DATA) $(BUILD_DIR)/instrument/qemu-instr/events.h "$(DESTDIR)$(includedir)/qemu-instr/"
+endif
# various test targets
test speed: all
diff --git a/configure b/configure
index 8ab2a36130..ca61665874 100755
--- a/configure
+++ b/configure
@@ -489,6 +489,8 @@ QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
+QEMU_INCLUDES="$QEMU_INCLUDES -I\$(SRC_PATH)/instrument"
+QEMU_INCLUDES="$QEMU_INCLUDES -I\$(BUILD_DIR)/instrument"
if test "$debug_info" = "yes"; then
CFLAGS="-g $CFLAGS"
LDFLAGS="-g $LDFLAGS"
diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs
index c548bbdd8a..c2289aba85 100644
--- a/instrument/Makefile.objs
+++ b/instrument/Makefile.objs
@@ -24,3 +24,17 @@ $(obj)/generated-tcg-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/
$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
target-obj-$(CONFIG_INSTRUMENT) += generated-tcg-tracers.o
+
+######################################################################
+# User interface
+
+$(obj)/qemu-instr/events.h: $(obj)/qemu-instr/events.h-timestamp
+ @cmp -s $< $@ || cp $< $@
+$(obj)/qemu-instr/events.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)/config-host.mak $(tracetool-y)
+ @mkdir -p $(dir $@)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=root \
+ --format=instr-api-h \
+ --backend=$(TRACE_INSTRUMENT_BACKEND) \
+ $(TRACETOOL_INSTR_STATIC) \
+ $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
diff --git a/instrument/qemu-instr/types.h b/instrument/qemu-instr/types.h
new file mode 100644
index 0000000000..d3d26bbf73
--- /dev/null
+++ b/instrument/qemu-instr/types.h
@@ -0,0 +1,73 @@
+/*
+ * QI-specific types for instrumentation clients.
+ *
+ * Copyright (C) 2012-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.
+ */
+
+#ifndef QI__TYPES_H
+#define QI__TYPES_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * SECTION: types
+ * @section_id: qi-types
+ * @title: Common types
+ *
+ * Data of architecture-specific length is always passed as an #int64_t to
+ * provide binary compatibility between the instrumentation library and QEMU,
+ * regardless of the guest architecture being instrumented.
+ */
+
+/**
+ * QICPU:
+ *
+ * Opaque guest CPU pointer.
+ */
+typedef struct QICPU QICPU;
+
+/**
+ * QITCGv_cpu:
+ *
+ * TCG register with QICPU.
+ */
+typedef struct QITCGv_cpu_d *QITCGv_cpu;
+
+/**
+ * QITCGv:
+ *
+ * TCG register with data of architecture-specific length.
+ */
+typedef struct QITCGv_d *QITCGv;
+
+/**
+ * QITCGv_i32:
+ *
+ * TCG register with 32-bit data.
+ */
+typedef struct QITCGv_i32_d *QITCGv_i32;
+
+/**
+ * QITCGv_i64:
+ *
+ * TCG register with 64-bit data.
+ */
+typedef struct QITCGv_i64_d *QITCGv_i64;
+
+/**
+ * QITCGv_ptr:
+ *
+ * TCG register with pointer of architecture-specific length.
+ */
+typedef struct QITCGv_ptr_d *QITCGv_ptr;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* QI__TYPES_H */
diff --git a/instrument/qemu-instr/visibility-internal.h b/instrument/qemu-instr/visibility-internal.h
new file mode 100644
index 0000000000..3bbbdc34cd
--- /dev/null
+++ b/instrument/qemu-instr/visibility-internal.h
@@ -0,0 +1,58 @@
+/*
+ * Macros for symbol visibility.
+ *
+ * Copyright (C) 2012-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 of QEMU.
+ */
+
+#ifndef QI__VISIBILITY_INTERNAL_H
+#define QI__VISIBILITY_INTERNAL_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * SECTION:visibility
+ * @section_id: qi-visibility
+ * @title: Symbol visibility
+ *
+ * This code is taken from http://gcc.gnu.org/wiki/Visibility.
+ */
+
+/**
+ * QI_VPUBLIC:
+ *
+ * Make an element public to user's instrumentation code.
+ */
+
+/**
+ * QI_VLOCAL:
+ *
+ * Make an element not visible to user's instrumentation code.
+ */
+
+#if defined _WIN32 || defined __CYGWIN__
+ #ifdef __GNUC__
+ #define QI_VPUBLIC __attribute__ ((dllimport))
+ #else
+ #define QI_VPUBLIC __declspec(dllimport)
+ #endif
+ #define QI_VLOCAL
+#else
+ #if __GNUC__ >= 4
+ #define QI_VPUBLIC __attribute__ ((visibility ("default")))
+ #define QI_VLOCAL __attribute__ ((visibility ("hidden")))
+ #else
+ #define QI_VPUBLIC
+ #define QI_VLOCAL
+ #endif
+#endif
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* QI__VISIBILITY_INTERNAL_H */
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index b9ddf8fbf9..7c19dc8c94 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -277,6 +277,10 @@ class Event(object):
QI_TRACE_INSTRUMENT = "qi_event_%(name)s"
QI_TRACE_INSTRUMENT_TCG = QI_TRACE_INSTRUMENT + "_tcg"
+ QI_TRACE_NOP = QI_TRACE_INSTRUMENT + "_nop"
+ QI_TRACE_TRACE = QI_TRACE_INSTRUMENT + "_trace"
+ QI_TRACE_GEN = QI_TRACE_INSTRUMENT + "_gen_exec"
+ QI_TRACE_TRACEGEN = QI_TRACE_INSTRUMENT + "_trace_and_gen_exec"
def api(self, fmt=None):
if fmt is None:
diff --git a/scripts/tracetool/backend/instr_dynamic.py b/scripts/tracetool/backend/instr_dynamic.py
index f42d3afa8f..6f9f8e49fb 100644
--- a/scripts/tracetool/backend/instr_dynamic.py
+++ b/scripts/tracetool/backend/instr_dynamic.py
@@ -65,3 +65,87 @@ def generate_instr_tcg_h(event, group):
args=event.args,
argnames=", ".join(event.args.names()),
argtypes=argtypes)
+
+
+##################################################
+# instr-tcg-c
+
+def generate_instr_tcg_c_begin(events, group):
+ out('#include "qemu-instr/events.h"',
+ '')
+
+def generate_instr_tcg_c(event, group):
+ if "tcg" in event.properties:
+ event = event.event_trans
+
+ iargs = tracetool.vcpu.transform_args("tcg_h", event.original)
+ api_args = iargs.transform(TCG_2_QI_TCG)
+
+ qtargs = ["(%s)%s" % (arg[0], arg[1])
+ for arg in event.args]
+ gargs = tracetool.vcpu.transform_args(
+ "tcg_helper_c", event.original, "wrapper")
+ qgargs = ["(%s)%s" % (arg[0], arg[1])
+ for arg in gargs]
+ qargs = ["(%s)%s" % (arg[0], arg[1])
+ for arg in iargs]
+
+ out('QI_VPUBLIC void %(nop)s(%(api_args)s)',
+ '{',
+ '}',
+ 'QI_VPUBLIC void %(trace)s(%(api_args)s)',
+ '{',
+ ' %(b_trace)s(%(qtargs)s);',
+ '}',
+ 'QI_VPUBLIC void %(gen)s(%(api_args)s)',
+ '{',
+ ' %(b_gen)s(%(qgargs)s);',
+ '}',
+ 'QI_VPUBLIC void %(trace_gen)s(%(api_args)s)',
+ '{',
+ ' %(b_trace_gen)s(%(qargs)s);',
+ '}',
+ 'void *%(qi_cb)s_cb = %(trace_gen)s;',
+ '',
+ api_args=api_args,
+ nop=event.api(event.QI_TRACE_NOP),
+ trace=event.api(event.QI_TRACE_TRACE),
+ b_trace=event.api(event.QEMU_TRACE_BACKEND),
+ gen=event.api(event.QI_TRACE_GEN),
+ b_gen="gen_helper_" + event.original.event_exec.api(event.QEMU_TRACE_BACKEND),
+ trace_gen=event.api(event.QI_TRACE_TRACEGEN),
+ b_trace_gen=event.original.api(event.QEMU_TRACE_TCG_BACKEND),
+ iargs=iargs,
+ qargs=", ".join(qargs),
+ qtargs=", ".join(qtargs),
+ qgargs=", ".join(qgargs),
+ qi_cb=event.original.api(event.QI_TRACE_INSTRUMENT_TCG))
+
+
+##################################################
+# instr-c
+
+def generate_instr_c_begin(events, group):
+ out('#include "qemu-instr/events.h"',
+ '')
+
+def generate_instr_c(event, group):
+ args = event.args.transform(TCG_2_QI_TCG)
+ qargs = ["(%s)%s" % (arg[0], arg[1])
+ for arg in event.args]
+ out('QI_VPUBLIC void %(nop)s(%(args)s)',
+ '{',
+ '}',
+ 'QI_VPUBLIC void %(trace)s(%(args)s)',
+ '{',
+ ' %(backend)s(%(qargs)s);',
+ '}',
+ 'void *%(qi_cb)s_cb = %(qi_trace)s;',
+ '',
+ args=args,
+ nop=event.api(event.QI_TRACE_NOP),
+ trace=event.api(event.QI_TRACE_TRACE),
+ backend=event.api(event.QEMU_TRACE_BACKEND),
+ qargs=", ".join(qargs),
+ qi_cb=event.api(event.QI_TRACE_INSTRUMENT),
+ qi_trace=event.api(event.QI_TRACE_TRACE))
diff --git a/scripts/tracetool/backend/instr_none.py b/scripts/tracetool/backend/instr_none.py
index 56508cfcbe..95aed5e96f 100644
--- a/scripts/tracetool/backend/instr_none.py
+++ b/scripts/tracetool/backend/instr_none.py
@@ -32,3 +32,21 @@ def generate_instr_h(event, group):
out(' %(backend)s(%(argnames)s);',
backend=event.api(event.QEMU_TRACE_BACKEND),
argnames=", ".join(event.args.names()))
+
+
+##################################################
+# instr-tcg-c
+
+def generate_instr_tcg_c(event, group):
+ out('void *%(qi_cb)s_cb = %(qi_trace)s;',
+ qi_cb=event.api(event.QI_TRACE_INSTRUMENT_TCG),
+ qi_trace=event.api(event.QEMU_TRACE_TCG_BACKEND))
+
+
+##################################################
+# instr-c
+
+def generate_instr_c(event, group):
+ out('void *%(qi_cb)s_cb = %(qi_trace)s;',
+ qi_cb=event.api(event.QI_TRACE_INSTRUMENT),
+ qi_trace=event.api(event.QEMU_TRACE_BACKEND))
diff --git a/scripts/tracetool/format/instr_api_h.py b/scripts/tracetool/format/instr_api_h.py
new file mode 100644
index 0000000000..d85f64df45
--- /dev/null
+++ b/scripts/tracetool/format/instr_api_h.py
@@ -0,0 +1,65 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+instrument/qemu-instr/events.h
+
+"""
+
+__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+from tracetool.transform import *
+import tracetool.vcpu
+
+
+def generate(events, backend, group):
+ # We won't be using any backend, because this code is only compiled in when
+ # dynamic instrumentation mode is enabled
+
+ out('/* This file is autogenerated by tracetool, do not edit. */',
+ '',
+ '#ifndef QI__EVENTS_H',
+ '#define QI__EVENTS_H',
+ '',
+ '#ifdef __cplusplus',
+ 'extern "C" {',
+ '#endif',
+ '',
+ '#include <stdbool.h>',
+ '#include <stdint.h>',
+ '#include <qemu-instr/types.h>',
+ '#include <qemu-instr/visibility-internal.h>',
+ '',
+ '')
+
+ for e in events:
+ if "tcg-trans" in e.properties:
+ args = tracetool.vcpu.transform_args("tcg_h", e.original)
+ args = args.transform(TCG_2_QI_TCG)
+ else:
+ args = e.args.transform(TCG_2_QI_TCG)
+ out('QI_VPUBLIC void %(nop)s(%(args)s);',
+ 'QI_VPUBLIC void %(trace)s(%(args)s);',
+ args=args,
+ nop=e.api(e.QI_TRACE_NOP),
+ trace=e.api(e.QI_TRACE_TRACE))
+ if "tcg-trans" in e.properties:
+ out('QI_VPUBLIC void %(gen)s(%(args)s);',
+ 'QI_VPUBLIC void %(trace_gen)s(%(args)s);',
+ args=args,
+ gen=e.api(e.QI_TRACE_GEN),
+ trace_gen=e.api(e.QI_TRACE_TRACEGEN))
+ out('')
+
+ out('#ifdef __cplusplus',
+ '}',
+ '#endif',
+ '',
+ '#endif /* QI__EVENTS_H */')
diff --git a/scripts/tracetool/format/instr_c.py b/scripts/tracetool/format/instr_c.py
index 987ecbf5c2..35b7955641 100644
--- a/scripts/tracetool/format/instr_c.py
+++ b/scripts/tracetool/format/instr_c.py
@@ -21,8 +21,7 @@ from tracetool.transform import *
def generate(events, backend, group):
events = [e for e in events
- if "tcg-trans" not in e.properties and
- "instrument" in e.properties]
+ if "tcg-trans" not in e.properties]
if group == "root":
header = "trace-root.h"
@@ -37,8 +36,6 @@ def generate(events, backend, group):
backend.generate_begin(events, group)
for e in events:
- out('void *%(qi_cb)s_cb = %(qi_trace)s;',
- qi_cb=e.api(e.QI_TRACE_INSTRUMENT),
- qi_trace=e.api(e.QEMU_TRACE_BACKEND))
+ backend.generate(e, group)
backend.generate_end(events, group)
diff --git a/scripts/tracetool/format/instr_tcg_c.py b/scripts/tracetool/format/instr_tcg_c.py
index 382709d0bd..61407a68c0 100644
--- a/scripts/tracetool/format/instr_tcg_c.py
+++ b/scripts/tracetool/format/instr_tcg_c.py
@@ -20,8 +20,7 @@ from tracetool.transform import *
def generate(events, backend, group):
events = [e.original for e in events
- if "tcg-trans" in e.properties and
- "instrument" in e.properties]
+ if "tcg-trans" in e.properties]
out('/* This file is autogenerated by tracetool, do not edit. */',
'',
@@ -33,8 +32,6 @@ def generate(events, backend, group):
backend.generate_begin(events, group)
for e in events:
- out('void *%(qi_cb)s_cb = %(qi_trace)s;',
- qi_cb=e.api(e.QI_TRACE_INSTRUMENT_TCG),
- qi_trace=e.api(e.QEMU_TRACE_TCG_BACKEND))
+ backend.generate(e, group)
backend.generate_end(events, group)
diff --git a/scripts/tracetool/format/tcg_helper_c.py b/scripts/tracetool/format/tcg_helper_c.py
index e9ba62a136..db431797d9 100644
--- a/scripts/tracetool/format/tcg_helper_c.py
+++ b/scripts/tracetool/format/tcg_helper_c.py
@@ -75,7 +75,7 @@ def generate(events, backend, group):
' %(name)s(%(args_call)s);',
'}',
name_tcg="helper_%s_proxy" % e.api(e.QEMU_TRACE_BACKEND),
- name=e.api(e.QEMU_TRACE_NOCHECK),
+ name=e.api(e.QEMU_TRACE),
args_api=e_args_api,
args_call=", ".join(e_args_call.casted()),
)
diff --git a/scripts/tracetool/transform.py b/scripts/tracetool/transform.py
index e18b05315e..400d36a6ed 100644
--- a/scripts/tracetool/transform.py
+++ b/scripts/tracetool/transform.py
@@ -137,6 +137,19 @@ TCG_2_TCG_HELPER_DECL = {
##################################################
+# host/tcg -> instrumentation-client tcg type
+
+def TCG_2_QI_TCG(type_):
+ if type_ == "TCGv_env":
+ return "QITCGv_cpu"
+ elif type_.startswith("TCGv"):
+ return "QI" + type_
+ elif type_.startswith("CPUState"):
+ return type_.replace("CPUState", "QICPU", 1)
+ else:
+ return type_
+
+##################################################
# host/tcg -> tcg temporal constant allocation
def _host_2_tcg_tmp_new(type_):
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 06/13] instrument: Add event control interface
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (4 preceding siblings ...)
2017-07-24 17:22 ` [Qemu-devel] [PATCH 05/13] instrument: [dynamic] Add default public per-event functions Lluís Vilanova
@ 2017-07-24 17:26 ` Lluís Vilanova
2017-07-24 17:30 ` [Qemu-devel] [PATCH 07/13] instrument: Add generic command line library loader Lluís Vilanova
` (7 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
Adds public and internal APIs to control event instrumentation.
It also adds an event object (QI_EVENT_${NAME}) and define to know if it's staically enabled (QI_EVENT_${NAME}_ENABLED).
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Makefile | 1
configure | 4
instrument/Makefile.objs | 5
instrument/control.c | 316 ++++++++++++++++++++++++++++
instrument/control.h | 86 ++++++++
instrument/qemu-instr/control.h | 128 +++++++++++
instrument/qemu-instr/types.h | 8 +
scripts/tracetool/backend/instr_dynamic.py | 14 +
scripts/tracetool/format/c.py | 37 +++
scripts/tracetool/format/instr_api_h.py | 13 +
trace/event-internal.h | 10 +
11 files changed, 615 insertions(+), 7 deletions(-)
create mode 100644 instrument/control.c
create mode 100644 instrument/control.h
create mode 100644 instrument/qemu-instr/control.h
diff --git a/Makefile b/Makefile
index c8be326790..ee54b7e17a 100644
--- a/Makefile
+++ b/Makefile
@@ -688,6 +688,7 @@ ifeq ($(TRACE_INSTRUMENT_BACKEND),instr-dynamic)
$(INSTALL_DIR) "$(DESTDIR)$(includedir)/qemu-instr"
$(INSTALL_DATA) $(SRC_PATH)/instrument/qemu-instr/types.h "$(DESTDIR)$(includedir)/qemu-instr/"
$(INSTALL_DATA) $(SRC_PATH)/instrument/qemu-instr/visibility-internal.h "$(DESTDIR)$(includedir)/qemu-instr/"
+ $(INSTALL_DATA) $(SRC_PATH)/instrument/qemu-instr/control.h "$(DESTDIR)$(includedir)/qemu-instr/"
$(INSTALL_DATA) $(BUILD_DIR)/instrument/qemu-instr/events.h "$(DESTDIR)$(includedir)/qemu-instr/"
endif
diff --git a/configure b/configure
index ca61665874..320309e0c0 100755
--- a/configure
+++ b/configure
@@ -6019,10 +6019,14 @@ QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
echo "TRACE_INSTRUMENT_BACKEND=instr-$trace_instrument_backend" >> $config_host_mak
if test "$trace_instrument" = "yes"; then
echo "CONFIG_INSTRUMENT=y" >> $config_host_mak
+ libs_qga="-ldl $libs_qga"
+ LDFLAGS="-rdynamic $LDFLAGS"
fi
if test -n "$instrument_events"; then
echo "CONFIG_INSTRUMENT_EVENTS=$instrument_events" >> $config_host_mak
fi
+# code requiring it is always compiled-in
+LIBS="-ldl $LIBS"
##########################################
echo "TOOLS=$tools" >> $config_host_mak
diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs
index c2289aba85..244936aa8c 100644
--- a/instrument/Makefile.objs
+++ b/instrument/Makefile.objs
@@ -38,3 +38,8 @@ $(obj)/qemu-instr/events.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
--backend=$(TRACE_INSTRUMENT_BACKEND) \
$(TRACETOOL_INSTR_STATIC) \
$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+
+######################################################################
+# Control code
+
+target-obj-y += control.o
diff --git a/instrument/control.c b/instrument/control.c
new file mode 100644
index 0000000000..309228f15d
--- /dev/null
+++ b/instrument/control.c
@@ -0,0 +1,316 @@
+/*
+ * Interface for controlling dynamic trace instrumentation.
+ *
+ * Copyright (C) 2012-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 "qemu-common.h"
+
+#include <dlfcn.h>
+#include "instrument/control.h"
+#include "qemu/thread.h"
+#include "qemu-instr/control.h"
+#include "qemu/error-report.h"
+
+
+typedef int64_t HandleID;
+
+typedef struct Handle
+{
+ HandleID id;
+ void *dlhandle;
+ void (*init)(int, const char **);
+ void (*fini)(void);
+ QSLIST_ENTRY(Handle) list;
+} Handle;
+
+HandleID handle_last_id;
+QSLIST_HEAD(, Handle) handles = QSLIST_HEAD_INITIALIZER(handles);
+static QemuMutex instr_lock;
+
+
+static Handle *handle_get(void)
+{
+ Handle *res = g_malloc0(sizeof(Handle));
+ res->id = handle_last_id++;
+ QSLIST_INSERT_HEAD(&handles, res, list);
+ return res;
+}
+
+static bool handle_put(HandleID id)
+{
+ Handle *prev = NULL;
+ Handle *handle;
+ QSLIST_FOREACH(handle, &handles, list) {
+ if (handle->id == id) {
+ break;
+ }
+ prev = handle;
+ }
+ if (handle == NULL) {
+ return false;
+ } else {
+ if (prev == NULL) {
+ QSLIST_REMOVE_HEAD(&handles, list);
+ } else {
+ QSLIST_REMOVE_AFTER(prev, list);
+ }
+ g_free(handle);
+ return true;
+ }
+}
+
+#if defined(CONFIG_INSTRUMENT)
+static Handle *handle_find(HandleID id)
+{
+ Handle *handle;
+ QSLIST_FOREACH(handle, &handles, list) {
+ if (handle->id == id) {
+ return handle;
+ }
+ }
+ return NULL;
+}
+#endif
+
+static bool instr_available(void)
+{
+#if defined(CONFIG_INSTRUMENT)
+ return true;
+#else
+ return false;
+#endif
+}
+
+InstrLoadError instr_load(const char * path, int argc, const char ** argv,
+ int64_t *handle_id)
+{
+ InstrLoadError res;
+
+ qemu_rec_mutex_lock(&instr_lock);
+
+ *handle_id = -1;
+ if (!instr_available()) {
+ res = INSTR_LOAD_UNAVAILABLE;
+ goto out;
+ }
+
+ if (!QSLIST_EMPTY(&handles) > 0) {
+ /* XXX: This is in fact a hard-coded limit, but there's no reason why a
+ * real multi-library implementation should fail with something like
+ * "too many open libraries".
+ */
+ res = INSTR_LOAD_UNAVAILABLE;
+ goto out;
+ }
+
+ Handle * handle = handle_get();
+ handle->dlhandle = dlopen(path, RTLD_NOW);
+ if (handle->dlhandle == NULL) {
+ goto err;
+ }
+
+ handle->init = dlsym(handle->dlhandle, "qi_init");
+ if (handle->init == NULL) {
+ goto err;
+ }
+ handle->fini = dlsym(handle->dlhandle, "qi_fini");
+ if (handle->fini == NULL) {
+ goto err;
+ }
+
+ handle->init(argc, argv);
+
+ *handle_id = handle->id;
+ res = INSTR_LOAD_OK;
+ goto out;
+
+err:
+ handle_put(handle->id);
+ res = INSTR_LOAD_ERROR;
+out:
+ qemu_rec_mutex_unlock(&instr_lock);
+ return res;
+}
+
+static inline bool instr_event_is_instrument(TraceEvent *ev)
+{
+#if defined(CONFIG_INSTRUMENT)
+ return ev->is_instr;
+#else
+ return false;
+#endif
+}
+
+InstrUnloadError instr_unload(int64_t handle_id)
+{
+ InstrLoadError res;
+
+ qemu_rec_mutex_lock(&instr_lock);
+
+ if (!instr_available()) {
+ res = INSTR_UNLOAD_UNAVAILABLE;
+ goto out;
+ }
+
+#if defined(CONFIG_INSTRUMENT)
+ Handle *handle = handle_find(handle_id);
+ if (handle == NULL) {
+ res = INSTR_UNLOAD_INVALID;
+ goto out;
+ }
+
+ handle->fini();
+
+ TraceEventIter iter;
+ TraceEvent *ev = NULL;
+ trace_event_iter_init(&iter, NULL);
+ while ((ev = trace_event_iter_next(&iter)) != NULL) {
+ if (instr_event_is_instrument(ev)) {
+ *ev->instr_cb = ev->instr_cb_default;
+ }
+ }
+
+ /* this should never fail */
+ if (dlclose(handle->dlhandle) < 0) {
+ res = INSTR_UNLOAD_ERROR;
+ } else {
+ res = INSTR_UNLOAD_OK;
+ }
+ handle_put(handle->id);
+#endif
+
+out:
+ qemu_rec_mutex_unlock(&instr_lock);
+ return res;
+}
+
+InstrUnloadError instr_unload_all(void)
+{
+ InstrUnloadError res = INSTR_UNLOAD_OK;
+
+ qemu_rec_mutex_lock(&instr_lock);
+ while (true) {
+ Handle *handle = QSLIST_FIRST(&handles);
+ if (handle == NULL) {
+ break;
+ } else {
+ res = instr_unload(handle->id);
+ if (res != INSTR_UNLOAD_OK) {
+ break;
+ }
+ }
+ }
+ qemu_rec_mutex_unlock(&instr_lock);
+
+ return res;
+}
+
+void qi_ctrl_event_set(QIEvent *ev, void *cb)
+{
+ TraceEvent *tev = (TraceEvent *)ev;
+
+ if (unlikely(tev == NULL)) {
+ error_report("qi_ctrl_event_set: event is NULL");
+ return;
+ }
+
+#if defined(CONFIG_INSTRUMENT)
+ if (unlikely(!tev->is_instr)) {
+ error_report("qi_ctrl_event_set: event is not instrumentable");
+ return;
+ }
+
+ if (cb == NULL) {
+ *tev->instr_cb = tev->instr_cb_default;
+ } else {
+ *tev->instr_cb = cb;
+ }
+#else
+ assert(false);
+#endif /* defined(CONFIG_INSTRUMENT) */
+}
+
+bool qi_trace_event_get_state_static(QIEvent *ev)
+{
+ if (unlikely(ev == NULL)) {
+ error_report("qi_trace_event_get_state_static: event is NULL");
+ return false;
+ }
+ return trace_event_get_state_static((TraceEvent *)ev);
+}
+
+bool qi_trace_event_get_state_dynamic(QIEvent *ev)
+{
+ if (unlikely(ev == NULL)) {
+ error_report("qi_trace_event_get_state_dynamic: event is NULL");
+ return false;
+ }
+ return trace_event_get_state_dynamic((TraceEvent *)ev);
+}
+
+bool qi_trace_event_get_vcpu_state_dynamic(QICPU *vcpu, QIEvent *ev)
+{
+ TraceEvent *tev = (TraceEvent *)ev;
+
+ if (unlikely(vcpu == NULL)) {
+ error_report("qi_trace_event_get_vcpu_state_dynamic: vcpu is NULL");
+ return false;
+ }
+ if (unlikely(tev == NULL)) {
+ error_report("qi_trace_event_get_vcpu_state_dynamic: event is NULL");
+ return false;
+ }
+ if (unlikely(!trace_event_is_vcpu(tev))) {
+ error_report("qi_trace_event_get_vcpu_state_dynamic: event does not have 'vcpu' property");
+ return false;
+ }
+ return trace_event_get_vcpu_state_dynamic((CPUState *)vcpu, tev);
+}
+
+void qi_trace_event_set_state_dynamic(QIEvent *ev, bool state)
+{
+ TraceEvent *tev = (TraceEvent *)ev;
+
+ if (unlikely(tev == NULL)) {
+ error_report("qi_trace_event_set_state_dynamic: event is NULL");
+ return;
+ }
+ if (unlikely(!trace_event_get_state_static(tev))) {
+ error_report("qi_trace_event_set_state_dynamic: event is statically disabled");
+ return;
+ }
+ trace_event_set_state_dynamic(tev, state);
+}
+
+void qi_trace_event_set_vcpu_state_dynamic(QICPU *vcpu, QIEvent *ev, bool state)
+{
+ TraceEvent *tev = (TraceEvent *)ev;
+
+ if (unlikely(vcpu == NULL)) {
+ error_report("qi_trace_event_set_vcpu_state_dynamic: vcpu is NULL");
+ return;
+ }
+ if (unlikely(tev == NULL)) {
+ error_report("qi_trace_event_set_vcpu_state_dynamic: event is NULL");
+ return;
+ }
+ if (unlikely(!trace_event_is_vcpu(tev))) {
+ error_report("qi_trace_event_set_vcpu_state_dynamic: event does not have 'vcpu' property");
+ return;
+ }
+ if (unlikely(!trace_event_get_state_static(tev))) {
+ error_report("qi_trace_event_set_vcpu_state_dynamic: event is statically disabled");
+ return;
+ }
+ trace_event_set_vcpu_state_dynamic((CPUState *)vcpu, tev, state);
+}
+
+static void __attribute__((constructor)) instr_lock_init(void)
+{
+ qemu_rec_mutex_init(&instr_lock);
+}
diff --git a/instrument/control.h b/instrument/control.h
new file mode 100644
index 0000000000..a83a350faf
--- /dev/null
+++ b/instrument/control.h
@@ -0,0 +1,86 @@
+/*
+ * Interface for controlling dynamic trace instrumentation.
+ *
+ * Copyright (C) 2012-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.
+ */
+
+#ifndef INSTRUMENT__CONTROL_H
+#define INSTRUMENT__CONTROL_H
+
+#include <stdint.h>
+#include <stdbool.h>
+#include "qapi-types.h"
+#include "trace/control.h"
+
+
+/**
+ * InstrLoadError:
+ * @INSTR_LOAD_OK: Correctly loaded.
+ * @INSTR_LOAD_UNAVAILABLE: Service not available.
+ * @INSTR_LOAD_ERROR: Error with libdl (see dlerror).
+ *
+ * Error codes for instr_load().
+ *
+ * Warning: Keep in sync with #InstrLoadCode
+ */
+typedef enum {
+ INSTR_LOAD_OK,
+ INSTR_LOAD_UNAVAILABLE,
+ INSTR_LOAD_ERROR,
+} InstrLoadError;
+
+/**
+ * InstrUnloadError:
+ * @INSTR_UNLOAD_OK: Correctly unloaded.
+ * @INSTR_UNLOAD_UNAVAILABLE: Service not available.
+ * @INSTR_UNLOAD_INVALID: Invalid handle.
+ * @INSTR_UNLOAD_ERROR: Error with libdl (see dlerror).
+ *
+ * Error codes for instr_unload().
+ *
+ * Warning: Keep in sync with #InstrUnloadCode
+ */
+typedef enum {
+ INSTR_UNLOAD_OK,
+ INSTR_UNLOAD_UNAVAILABLE,
+ INSTR_UNLOAD_INVALID,
+ INSTR_UNLOAD_ERROR,
+} InstrUnloadError;
+
+/**
+ * instr_load:
+ * @path: Path to the shared library to load.
+ * @argc: Number of arguments passed to the initialization function of the library.
+ * @argv: Arguments passed to the initialization function of the library.
+ * @handle: Instrumentation library handle (undefined in case of error).
+ *
+ * Load a dynamic trace instrumentation library.
+ *
+ * Returns: Whether the library could be loaded.
+ */
+InstrLoadError instr_load(const char * path, int argc, const char ** argv,
+ int64_t *handle);
+
+/**
+ * instr_unload:
+ * @handle: Instrumentation library handle returned by instr_load().
+ *
+ * Unload the given instrumentation library.
+ *
+ * Returns: Whether the library could be unloaded.
+ */
+InstrUnloadError instr_unload(int64_t handle);
+
+/**
+ * instr_unload_all:
+ *
+ * Unload all instrumentation libraries.
+ *
+ * Returns: Whether any library could not be unloaded.
+ */
+InstrUnloadError instr_unload_all(void);
+
+#endif /* INSTRUMENT__CONTROL_H */
diff --git a/instrument/qemu-instr/control.h b/instrument/qemu-instr/control.h
new file mode 100644
index 0000000000..83f20d42f2
--- /dev/null
+++ b/instrument/qemu-instr/control.h
@@ -0,0 +1,128 @@
+/*
+ * Interface for controlling the state of events.
+ *
+ * Copyright (C) 2012-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.
+ */
+
+#ifndef QI__CONTROL_H
+#define QI__CONTROL_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdbool.h>
+#include <stddef.h>
+
+#include <qemu-instr/types.h>
+#include <qemu-instr/visibility-internal.h>
+
+
+/**
+ * SECTION:control
+ * @section_id: qi-control
+ * @title: Event control API for QEMU event instrumentation
+ */
+
+/**
+ * qi_ctrl_event_set:
+ * @ev: Event descriptor.
+ * @cb: Pointer to instrumentation callback.
+ *
+ * Set the instrumentation callback for the given event.
+ *
+ * @cb can also be set to #NULL to restore the default callback.
+ */
+QI_VPUBLIC void qi_ctrl_event_set(QIEvent *ev, void *cb);
+
+/**
+ * qi_trace_event_get_state:
+ * @ev: Event identifier name.
+ *
+ * Get the tracing state of an event (both static and dynamic).
+ *
+ * If the event has the disabled property, the check will have no performance
+ * impact.
+ */
+#define qi_trace_event_get_state(ev) \
+ ((ev ##_ENABLED) && qi_trace_event_get_state_dynamic(ev))
+
+/**
+ * qi_trace_event_get_vcpu_state:
+ * @vcpu: Target vCPU.
+ * @ev: Event identifier name.
+ *
+ * Get the tracing state of an event (both static and dynamic) for the given
+ * vCPU.
+ *
+ * If the event has the disabled property, the check will have no performance
+ * impact.
+ */
+#define qi_trace_event_get_vcpu_state(vcpu, ev) \
+ ((ev ##_ENABLED) && \
+ qi_trace_event_get_vcpu_state_dynamic(vcpu, ev))
+
+/**
+ * qi_trace_event_get_state_static:
+ * @ev: Event identifier.
+ *
+ * Get the static tracing state of an event.
+ *
+ * Use the define 'QI_EVENT_${EVENT}_ENABLED' for compile-time checks (it will
+ * be set to 1 or 0 according to the presence of the disabled property).
+ */
+QI_VPUBLIC bool qi_trace_event_get_state_static(QIEvent *ev);
+
+/**
+ * qi_trace_event_get_state_dynamic:
+ * @ev: Event identifier.
+ *
+ * Get the dynamic tracing state of an event.
+ */
+QI_VPUBLIC bool qi_trace_event_get_state_dynamic(QIEvent *ev);
+
+/**
+ * qi_trace_event_get_vcpu_state_dynamic:
+ * @vcpu: Target vCPU.
+ * @ev: Event identifier.
+ *
+ * Get the dynamic tracing state of an event for the given vCPU.
+ */
+QI_VPUBLIC bool qi_trace_event_get_vcpu_state_dynamic(QICPU *vcpu,
+ QIEvent *ev);
+
+/**
+ * qi_trace_event_set_state_dynamic:
+ * @ev: Event identifier.
+ * @state: Target tracing state.
+ *
+ * Set the dynamic tracing state of an event.
+ *
+ * Pre-condition: qi_trace_event_get_state_static(ev) == true
+ */
+QI_VPUBLIC void qi_trace_event_set_state_dynamic(QIEvent *ev, bool state);
+
+/**
+ * qi_trace_event_set_vcpu_state_dynamic:
+ * @vcpu: Target vCPU.
+ * @ev: Event identifier.
+ * @state: Target tracing state.
+ *
+ * Set the dynamic tracing state of an event for the given vCPU.
+ *
+ * Pre-condition: qi_trace_event_get_state_static(ev) == true
+ *
+ * Note: Changes for execution-time events with the 'tcg' property will not be
+ * propagated until the next TB is executed (iff executing in TCG mode).
+ */
+QI_VPUBLIC void qi_trace_event_set_vcpu_state_dynamic(QICPU *vcpu,
+ QIEvent *ev, bool state);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* QI__CONTROL_H */
diff --git a/instrument/qemu-instr/types.h b/instrument/qemu-instr/types.h
index d3d26bbf73..58a84b6d01 100644
--- a/instrument/qemu-instr/types.h
+++ b/instrument/qemu-instr/types.h
@@ -25,6 +25,14 @@ extern "C" {
*/
/**
+ * QIEvent:
+ *
+ * Opaque structure defining an instrumentation event.
+ */
+struct QIEvent;
+typedef struct QIEvent QIEvent;
+
+/**
* QICPU:
*
* Opaque guest CPU pointer.
diff --git a/scripts/tracetool/backend/instr_dynamic.py b/scripts/tracetool/backend/instr_dynamic.py
index 6f9f8e49fb..2efd4a7eb1 100644
--- a/scripts/tracetool/backend/instr_dynamic.py
+++ b/scripts/tracetool/backend/instr_dynamic.py
@@ -90,7 +90,8 @@ def generate_instr_tcg_c(event, group):
qargs = ["(%s)%s" % (arg[0], arg[1])
for arg in iargs]
- out('QI_VPUBLIC void %(nop)s(%(api_args)s)',
+ out('QIEvent *%(qevent)s = (QIEvent *)&%(tevent)s;',
+ 'QI_VPUBLIC void %(nop)s(%(api_args)s)',
'{',
'}',
'QI_VPUBLIC void %(trace)s(%(api_args)s)',
@@ -99,7 +100,10 @@ def generate_instr_tcg_c(event, group):
'}',
'QI_VPUBLIC void %(gen)s(%(api_args)s)',
'{',
+ # does not exist when event is disabled
+ '#if %(teventu)s_ENABLED',
' %(b_gen)s(%(qgargs)s);',
+ '#endif',
'}',
'QI_VPUBLIC void %(trace_gen)s(%(api_args)s)',
'{',
@@ -107,6 +111,9 @@ def generate_instr_tcg_c(event, group):
'}',
'void *%(qi_cb)s_cb = %(trace_gen)s;',
'',
+ qevent=event.api(event.QI_TRACE_INSTRUMENT).upper(),
+ tevent=event.api(event.QEMU_EVENT),
+ teventu=event.api(event.QEMU_TRACE).upper(),
api_args=api_args,
nop=event.api(event.QI_TRACE_NOP),
trace=event.api(event.QI_TRACE_TRACE),
@@ -133,7 +140,8 @@ def generate_instr_c(event, group):
args = event.args.transform(TCG_2_QI_TCG)
qargs = ["(%s)%s" % (arg[0], arg[1])
for arg in event.args]
- out('QI_VPUBLIC void %(nop)s(%(args)s)',
+ out('QIEvent *%(qevent)s = (QIEvent *)&%(tevent)s;',
+ 'QI_VPUBLIC void %(nop)s(%(args)s)',
'{',
'}',
'QI_VPUBLIC void %(trace)s(%(args)s)',
@@ -142,6 +150,8 @@ def generate_instr_c(event, group):
'}',
'void *%(qi_cb)s_cb = %(qi_trace)s;',
'',
+ qevent=event.api(event.QI_TRACE_INSTRUMENT).upper(),
+ tevent=event.api(event.QEMU_EVENT),
args=args,
nop=event.api(event.QI_TRACE_NOP),
trace=event.api(event.QI_TRACE_TRACE),
diff --git a/scripts/tracetool/format/c.py b/scripts/tracetool/format/c.py
index 833c05a022..0b8bb98d24 100644
--- a/scripts/tracetool/format/c.py
+++ b/scripts/tracetool/format/c.py
@@ -6,7 +6,7 @@ trace/generated-tracers.c
"""
__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__ = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>"
__license__ = "GPL version 2 or (at your option) any later version"
__maintainer__ = "Stefan Hajnoczi"
@@ -39,18 +39,49 @@ def generate(events, backend, group):
vcpu_id = 0
else:
vcpu_id = "TRACE_VCPU_EVENT_NONE"
+
+ if "instrument" in e.properties:
+ is_instr = "true"
+ out('#if defined(CONFIG_INSTRUMENT)')
+ if "tcg-trans" in e.properties:
+ instr_cb = e.original.api(e.QI_TRACE_INSTRUMENT_TCG) + "_cb"
+ instr_cb_default = e.api(e.QI_TRACE_TRACEGEN)
+ out('static void *_tmp__%(name1)s;',
+ 'extern void *%(name1)s __attribute__((weak, alias("_tmp__%(name1)s")));',
+ name1=instr_cb)
+ else:
+ instr_cb = e.api(e.QI_TRACE_INSTRUMENT) + "_cb"
+ instr_cb_default = e.api(e.QI_TRACE_TRACE)
+ out('static void _tmp__%(name2)s(void) {};',
+ 'void %(name2)s(void) __attribute__((weak, alias("_tmp__%(name2)s")));',
+ '#endif',
+ name2=instr_cb_default)
+ instr_cb = "&" + instr_cb
+ else:
+ is_instr = "false"
+ instr_cb = "NULL"
+ instr_cb_default = "NULL"
+
out('TraceEvent %(event)s = {',
' .id = 0,',
' .vcpu_id = %(vcpu_id)s,',
' .name = \"%(name)s\",',
' .sstate = %(sstate)s,',
- ' .dstate = &%(dstate)s ',
+ ' .dstate = &%(dstate)s,',
+ '#if defined(CONFIG_INSTRUMENT)',
+ ' .is_instr = %(is_instr)s,',
+ ' .instr_cb = %(instr_cb)s,',
+ ' .instr_cb_default = %(instr_cb_default)s,',
+ '#endif',
'};',
event = e.api(e.QEMU_EVENT),
vcpu_id = vcpu_id,
name = e.name,
sstate = "TRACE_%s_ENABLED" % e.name.upper(),
- dstate = e.api(e.QEMU_DSTATE))
+ dstate = e.api(e.QEMU_DSTATE),
+ is_instr=is_instr,
+ instr_cb=instr_cb,
+ instr_cb_default=instr_cb_default)
out('TraceEvent *%(group)s_trace_events[] = {',
group = group.lower())
diff --git a/scripts/tracetool/format/instr_api_h.py b/scripts/tracetool/format/instr_api_h.py
index d85f64df45..812a5d44ec 100644
--- a/scripts/tracetool/format/instr_api_h.py
+++ b/scripts/tracetool/format/instr_api_h.py
@@ -40,13 +40,24 @@ def generate(events, backend, group):
'')
for e in events:
+ if "disable" in e.properties:
+ enabled = 0
+ else:
+ enabled = 1
+
if "tcg-trans" in e.properties:
args = tracetool.vcpu.transform_args("tcg_h", e.original)
args = args.transform(TCG_2_QI_TCG)
else:
args = e.args.transform(TCG_2_QI_TCG)
- out('QI_VPUBLIC void %(nop)s(%(args)s);',
+
+ out('#define %(qeventu)s_ENABLED %(enabled)d',
+ 'extern QI_VPUBLIC QIEvent *%(event)s;',
+ 'QI_VPUBLIC void %(nop)s(%(args)s);',
'QI_VPUBLIC void %(trace)s(%(args)s);',
+ qeventu=e.api(e.QI_TRACE_INSTRUMENT).upper(),
+ enabled=enabled,
+ event=e.api(e.QI_TRACE_INSTRUMENT).upper(),
args=args,
nop=e.api(e.QI_TRACE_NOP),
trace=e.api(e.QI_TRACE_TRACE))
diff --git a/trace/event-internal.h b/trace/event-internal.h
index f63500b37e..b12cc110b6 100644
--- a/trace/event-internal.h
+++ b/trace/event-internal.h
@@ -1,7 +1,7 @@
/*
* Interface for configuring and controlling the state of tracing events.
*
- * Copyright (C) 2012-2016 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2012-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.
@@ -23,6 +23,9 @@
* @name: Event name.
* @sstate: Static tracing state.
* @dstate: Dynamic tracing state
+ * @is_instr: Whether the event is instrumentable.
+ * @instr_cb: Current instrumentation callback.
+ * @instr_cb_default: Default instrumentation callback.
*
* Interpretation of @dstate depends on whether the event has the 'vcpu'
* property:
@@ -37,6 +40,11 @@ typedef struct TraceEvent {
const char * name;
const bool sstate;
uint16_t *dstate;
+#if defined(CONFIG_INSTRUMENT)
+ bool is_instr;
+ void **instr_cb;
+ void *instr_cb_default;
+#endif
} TraceEvent;
void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state);
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 07/13] instrument: Add generic command line library loader
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (5 preceding siblings ...)
2017-07-24 17:26 ` [Qemu-devel] [PATCH 06/13] instrument: Add event control interface Lluís Vilanova
@ 2017-07-24 17:30 ` Lluís Vilanova
2017-07-24 17:34 ` [Qemu-devel] [PATCH 08/13] instrument: [linux-user] Add " Lluís Vilanova
` (6 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
instrument/Makefile.objs | 1
instrument/cmdline.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++
instrument/cmdline.h | 58 +++++++++++++++++++++++
3 files changed, 176 insertions(+)
create mode 100644 instrument/cmdline.c
create mode 100644 instrument/cmdline.h
diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs
index 244936aa8c..aa6db29ff4 100644
--- a/instrument/Makefile.objs
+++ b/instrument/Makefile.objs
@@ -43,3 +43,4 @@ $(obj)/qemu-instr/events.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
# Control code
target-obj-y += control.o
+target-obj-y += cmdline.o
diff --git a/instrument/cmdline.c b/instrument/cmdline.c
new file mode 100644
index 0000000000..e0c0e9ce1b
--- /dev/null
+++ b/instrument/cmdline.c
@@ -0,0 +1,117 @@
+/*
+ * Control dynamic trace instrumentation during program (de)initialization.
+ *
+ * Copyright (C) 2012-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 <dlfcn.h>
+#include "instrument/cmdline.h"
+#include "instrument/control.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+
+
+QemuOptsList qemu_instr_opts = {
+ .name = "instrument",
+ .implied_opt_name = "file",
+ .merge_lists = true,
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_instr_opts.head),
+ .desc = {
+ {
+ .name = "file",
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = "arg",
+ .type = QEMU_OPT_STRING,
+ },
+ { /* end of list */ }
+ },
+};
+
+void instr_opt_parse(const char *optarg, char **path,
+ int *argc, const char ***argv)
+{
+ const char *arg;
+ QemuOptsIter iter;
+ QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("instrument"),
+ optarg, true);
+ if (!opts) {
+ exit(1);
+ } else {
+#if !defined(CONFIG_INSTRUMENT)
+ error_report("instrumentation not enabled on this build");
+ exit(1);
+#endif
+ }
+
+
+ arg = qemu_opt_get(opts, "file");
+ if (arg != NULL) {
+ g_free(*path);
+ *path = g_strdup(arg);
+ }
+
+ qemu_opt_iter_init(&iter, opts, "arg");
+ while ((arg = qemu_opt_iter_next(&iter)) != NULL) {
+ *argv = realloc(*argv, sizeof(**argv) * (*argc + 1));
+ (*argv)[*argc] = g_strdup(arg);
+ (*argc)++;
+ }
+
+ qemu_opts_del(opts);
+}
+
+
+void instr_init(const char *path, int argc, const char **argv)
+{
+ InstrLoadError err;
+ int64_t handle;
+
+ if (path == NULL) {
+ return;
+ }
+
+ if (atexit(instr_fini) != 0) {
+ fprintf(stderr, "error: atexit: %s\n", strerror(errno));
+ abort();
+ }
+
+ err = instr_load(path, argc, argv, &handle);
+ switch (err) {
+ case INSTR_LOAD_OK:
+ return;
+ case INSTR_LOAD_UNAVAILABLE:
+ error_report("instrument: not available");
+ break;
+ case INSTR_LOAD_ERROR:
+ error_report("instrument: error loading library: %s", dlerror());
+ break;
+ }
+
+ exit(1);
+}
+
+void instr_fini(void)
+{
+ InstrUnloadError err = instr_unload_all();
+
+ switch (err) {
+ case INSTR_UNLOAD_OK:
+ return;
+ case INSTR_UNLOAD_UNAVAILABLE:
+ error_report("instrument: not available");
+ break;
+ case INSTR_UNLOAD_INVALID:
+ /* the user might have already unloaded it */
+ return;
+ case INSTR_UNLOAD_ERROR:
+ error_report("instrument: error unloading library: %s", dlerror());
+ break;
+ }
+
+ exit(1);
+}
diff --git a/instrument/cmdline.h b/instrument/cmdline.h
new file mode 100644
index 0000000000..c5eebf8b58
--- /dev/null
+++ b/instrument/cmdline.h
@@ -0,0 +1,58 @@
+/*
+ * Control dynamic trace instrumentation during program (de)initialization.
+ *
+ * Copyright (C) 2012-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.
+ */
+
+#ifndef INSTRUMENT__CMDLINE_H
+#define INSTRUMENT__CMDLINE_H
+
+
+/**
+ * Definition of QEMU options describing instrumentation subsystem
+ * configuration.
+ */
+extern QemuOptsList qemu_instr_opts;
+
+/**
+ * instr_opt_parse:
+ * @optarg: A string argument of --instrument command line argument
+ *
+ * Initialize instrument subsystem.
+ */
+void instr_opt_parse(const char *optarg, char **path,
+ int *argc, const char ***argv);
+
+/**
+ * instr_init:
+ * @path: Path to dynamic trace instrumentation library.
+ * @argc: Number of arguments to the library's #qi_init routine.
+ * @argv: Arguments to the library's #qi_init routine.
+ *
+ * Load and initialize the given instrumentation library.
+ *
+ * Automatically installs #instr_fini as an atexit callback.
+ *
+ * If path is %NULL and we're running with static instrumentation, the library
+ * is not loaded but just initialized.
+ *
+ * Pre-condition: There is no library already loaded.
+ */
+void instr_init(const char *path, int argc, const char **argv);
+
+/**
+ * instr_fini:
+ *
+ * Deinitialize and unload the current instrumentation library.
+ *
+ * If we're running with static instrumentation, the library is not unloaded but
+ * just deinitialized.
+ *
+ * Pre-condition: There is an already loaded library.
+ */
+void instr_fini(void);
+
+#endif /* INSTRUMENT__CMDLINE_H */
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 08/13] instrument: [linux-user] Add command line library loader
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (6 preceding siblings ...)
2017-07-24 17:30 ` [Qemu-devel] [PATCH 07/13] instrument: Add generic command line library loader Lluís Vilanova
@ 2017-07-24 17:34 ` Lluís Vilanova
2017-07-24 17:38 ` [Qemu-devel] [PATCH 09/13] instrument: [bsd-user] " Lluís Vilanova
` (5 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi, Riku Voipio, Laurent Vivier
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
linux-user/main.c | 20 ++++++++++++++++++++
linux-user/syscall.c | 4 ++++
2 files changed, 24 insertions(+)
diff --git a/linux-user/main.c b/linux-user/main.c
index ad03c9e8b2..fdb7ddb199 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -36,6 +36,8 @@
#include "exec/log.h"
#include "trace/control.h"
#include "glib-compat.h"
+#include "instrument/cmdline.h"
+#include "instrument/control.h"
char *exec_path;
@@ -4014,6 +4016,17 @@ static void handle_arg_trace(const char *arg)
trace_file = trace_opt_parse(arg);
}
+static char *instrument_path;
+static int instrument_argc;
+static const char **instrument_argv;
+#if defined(CONFIG_INSTRUMENT)
+static void handle_arg_instrument(const char *arg)
+{
+ instr_opt_parse(arg, &instrument_path,
+ &instrument_argc, &instrument_argv);
+}
+#endif
+
struct qemu_argument {
const char *argv;
const char *env;
@@ -4063,6 +4076,10 @@ 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>]"},
+#if defined(CONFIG_INSTRUMENT)
+ {"instr", "QEMU_INSTR", true, handle_arg_instrument,
+ "", "[file=]<file>[,arg=<string>]"},
+#endif
{"version", "QEMU_VERSION", false, handle_arg_version,
"", "display version information and exit"},
{NULL, NULL, false, NULL, NULL, NULL}
@@ -4252,6 +4269,7 @@ int main(int argc, char **argv, char **envp)
srand(time(NULL));
qemu_add_opts(&qemu_trace_opts);
+ qemu_add_opts(&qemu_instr_opts);
optind = parse_args(argc, argv);
@@ -4260,6 +4278,8 @@ int main(int argc, char **argv, char **envp)
}
trace_init_file(trace_file);
+ instr_init(instrument_path, instrument_argc, instrument_argv);
+
/* Zero out regs */
memset(regs, 0, sizeof(struct target_pt_regs));
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 003943b736..cf7071bd35 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -115,6 +115,8 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
#include "uname.h"
#include "qemu.h"
+#include "instrument/cmdline.h"
+
#ifndef CLONE_IO
#define CLONE_IO 0x80000000 /* Clone io context */
@@ -7765,6 +7767,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
_mcleanup();
#endif
gdb_exit(cpu_env, arg1);
+ instr_fini();
_exit(arg1);
ret = 0; /* avoid warning */
break;
@@ -9821,6 +9824,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
_mcleanup();
#endif
gdb_exit(cpu_env, arg1);
+ instr_fini();
ret = get_errno(exit_group(arg1));
break;
#endif
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 09/13] instrument: [bsd-user] Add command line library loader
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (7 preceding siblings ...)
2017-07-24 17:34 ` [Qemu-devel] [PATCH 08/13] instrument: [linux-user] Add " Lluís Vilanova
@ 2017-07-24 17:38 ` Lluís Vilanova
2017-07-24 17:42 ` [Qemu-devel] [PATCH 10/13] instrument: [softmmu] " Lluís Vilanova
` (4 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
bsd-user/main.c | 16 ++++++++++++++++
bsd-user/syscall.c | 5 +++++
2 files changed, 21 insertions(+)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index fa9c012c9f..37bf6a435c 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -33,6 +33,8 @@
#include "exec/log.h"
#include "trace/control.h"
#include "glib-compat.h"
+#include "instrument/cmdline.h"
+#include "instrument/control.h"
int singlestep;
unsigned long mmap_min_addr;
@@ -666,6 +668,11 @@ static void usage(void)
"-B address set guest_base address to address\n"
"-bsd type select emulated BSD type FreeBSD/NetBSD/OpenBSD (default)\n"
"\n"
+#if defined(CONFIG_INSTRUMENT)
+ "-instr [file=]<file>[,arg=<string>]\n"
+ " load an instrumentation library\n"
+ "\n"
+#endif
"Debug options:\n"
"-d item1[,...] enable logging of specified items\n"
" (use '-d help' for a list of log items)\n"
@@ -735,6 +742,9 @@ int main(int argc, char **argv)
envlist_t *envlist = NULL;
char *trace_file = NULL;
bsd_type = target_openbsd;
+ char *instrument_path = NULL;
+ int instrument_argc = 0;
+ const char **instrument_argv = NULL;
if (argc <= 1)
usage();
@@ -753,6 +763,7 @@ int main(int argc, char **argv)
cpu_model = NULL;
qemu_add_opts(&qemu_trace_opts);
+ qemu_add_opts(&qemu_instr_opts);
optind = 1;
for (;;) {
@@ -840,6 +851,9 @@ 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, "instr")) {
+ instr_opt_parse(optarg, &instrument_path,
+ &instrument_argc, &instrument_argv);
} else {
usage();
}
@@ -869,6 +883,8 @@ int main(int argc, char **argv)
}
trace_init_file(trace_file);
+ instr_init(instrument_path, instrument_argc, instrument_argv);
+
/* Zero out regs */
memset(regs, 0, sizeof(struct target_pt_regs));
diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
index 66492aaf5d..3230f722f3 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -26,6 +26,8 @@
#include "qemu.h"
#include "qemu-common.h"
+#include "instrument/cmdline.h"
+
//#define DEBUG
@@ -332,6 +334,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
_mcleanup();
#endif
gdb_exit(cpu_env, arg1);
+ instr_fini();
/* XXX: should free thread stack and CPU env */
_exit(arg1);
ret = 0; /* avoid warning */
@@ -430,6 +433,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long arg1,
_mcleanup();
#endif
gdb_exit(cpu_env, arg1);
+ instr_fini();
/* XXX: should free thread stack and CPU env */
_exit(arg1);
ret = 0; /* avoid warning */
@@ -505,6 +509,7 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, abi_long arg1,
_mcleanup();
#endif
gdb_exit(cpu_env, arg1);
+ instr_fini();
/* XXX: should free thread stack and CPU env */
_exit(arg1);
ret = 0; /* avoid warning */
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 10/13] instrument: [softmmu] Add command line library loader
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (8 preceding siblings ...)
2017-07-24 17:38 ` [Qemu-devel] [PATCH 09/13] instrument: [bsd-user] " Lluís Vilanova
@ 2017-07-24 17:42 ` Lluís Vilanova
2017-07-24 17:46 ` [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add " Lluís Vilanova
` (3 subsequent siblings)
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi, Paolo Bonzini
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
qemu-options.hx | 17 +++++++++++++++++
vl.c | 11 +++++++++++
2 files changed, 28 insertions(+)
diff --git a/qemu-options.hx b/qemu-options.hx
index 9bd6bf0eee..aacd01cee7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4064,6 +4064,23 @@ HXCOMM HX does not support conditional compilation of text.
@findex -trace
@include qemu-option-trace.texi
ETEXI
+DEF("instr", HAS_ARG, QEMU_OPTION_instr,
+ "-instr [file=]<file>[,arg=<string>]\n"
+ " load an instrumentation library\n",
+ QEMU_ARCH_ALL)
+STEXI
+@item -instr file=@var{file}[,arg=@var{string}]
+@findex -instr
+
+Load a dynamic trace instrumentation library.
+
+@table @option
+@item file=@var{file}
+Load the given dynamic trace instrumentation library.
+@item arg=@var{string}
+String argument passed as to the library's @code{qi_init} routine (can be given multiple times).
+@end table
+ETEXI
HXCOMM Internal use
DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index fb6b2efafa..1a24b34d47 100644
--- a/vl.c
+++ b/vl.c
@@ -118,6 +118,7 @@ int main(int argc, char **argv)
#include "trace-root.h"
#include "trace/control.h"
+#include "instrument/cmdline.h"
#include "qemu/queue.h"
#include "sysemu/arch_init.h"
@@ -3031,6 +3032,9 @@ int main(int argc, char **argv, char **envp)
} BlockdevOptions_queue;
QSIMPLEQ_HEAD(, BlockdevOptions_queue) bdo_queue
= QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
+ char *instrument_path = NULL;
+ int instrument_argc = 0;
+ const char **instrument_argv = NULL;
module_call_init(MODULE_INIT_TRACE);
@@ -3058,6 +3062,7 @@ int main(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_global_opts);
qemu_add_opts(&qemu_mon_opts);
qemu_add_opts(&qemu_trace_opts);
+ qemu_add_opts(&qemu_instr_opts);
qemu_add_opts(&qemu_option_rom_opts);
qemu_add_opts(&qemu_machine_opts);
qemu_add_opts(&qemu_accel_opts);
@@ -3999,6 +4004,10 @@ int main(int argc, char **argv, char **envp)
g_free(trace_file);
trace_file = trace_opt_parse(optarg);
break;
+ case QEMU_OPTION_instr:
+ instr_opt_parse(optarg, &instrument_path,
+ &instrument_argc, &instrument_argv);
+ break;
case QEMU_OPTION_readconfig:
{
int ret = qemu_read_config_file(optarg);
@@ -4185,6 +4194,8 @@ int main(int argc, char **argv, char **envp)
}
trace_init_file(trace_file);
+ instr_init(instrument_path, instrument_argc, instrument_argv);
+
/* Open the logfile at this point and set the log mask if necessary.
*/
if (log_file) {
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (9 preceding siblings ...)
2017-07-24 17:42 ` [Qemu-devel] [PATCH 10/13] instrument: [softmmu] " Lluís Vilanova
@ 2017-07-24 17:46 ` Lluís Vilanova
2017-07-24 18:03 ` Eric Blake
2017-07-24 17:50 ` [Qemu-devel] [PATCH 12/13] instrument: [hmp] " Lluís Vilanova
` (2 subsequent siblings)
13 siblings, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi, Eric Blake, Markus Armbruster
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
instrument/Makefile.objs | 1 +
instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
qapi-schema.json | 3 ++
qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 167 insertions(+)
create mode 100644 instrument/qmp.c
create mode 100644 qapi/instrument.json
diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs
index aa6db29ff4..757a247321 100644
--- a/instrument/Makefile.objs
+++ b/instrument/Makefile.objs
@@ -44,3 +44,4 @@ $(obj)/qemu-instr/events.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
target-obj-y += control.o
target-obj-y += cmdline.o
+target-obj-y += qmp.o
diff --git a/instrument/qmp.c b/instrument/qmp.c
new file mode 100644
index 0000000000..3f577e0c78
--- /dev/null
+++ b/instrument/qmp.c
@@ -0,0 +1,71 @@
+/*
+ * QMP interface for dynamic trace instrumentation control commands.
+ *
+ * Copyright (C) 2012-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 "qemu-common.h"
+#include "qapi/qmp/qerror.h"
+#include "qmp-commands.h"
+
+#include <dlfcn.h>
+
+#include "instrument/control.h"
+
+
+
+InstrLoadResult *qmp_instr_load(const char * path,
+ bool have_args, StringList * args,
+ Error **errp)
+{
+ int argc = 0;
+ const char **argv = NULL;
+
+ StringList *entry = have_args ? args : NULL;
+ while (entry != NULL) {
+ argv = realloc(argv, sizeof(*argv) * (argc + 1));
+ argv[argc] = entry->value->str;
+ argc++;
+ entry = entry->next;
+ }
+
+ InstrLoadResult *res = g_malloc0(sizeof(*res));
+ res->code = instr_load(path, argc, argv, &res->handle);
+ switch (res->code) {
+ case INSTR_LOAD_CODE_OK:
+ case INSTR_LOAD_CODE_UNAVAILABLE:
+ break;
+ case INSTR_LOAD_CODE_ERROR:
+ res->msg = dlerror();
+ break;
+ default:
+ fprintf(stderr, "Unknown instrumentation load code: %d", res->code);
+ exit(1);
+ break;
+ }
+ return res;
+}
+
+InstrUnloadResult *qmp_instr_unload(int64_t handle, Error **errp)
+{
+ InstrUnloadResult *res = g_malloc0(sizeof(*res));
+ res->code = instr_unload(handle);
+ switch (res->code) {
+ case INSTR_UNLOAD_OK:
+ case INSTR_UNLOAD_UNAVAILABLE:
+ case INSTR_UNLOAD_INVALID:
+ break;
+ case INSTR_UNLOAD_CODE_ERROR:
+ res->msg = dlerror();
+ break;
+ default:
+ fprintf(stderr, "Unknown instrumentation unload code: %d", res->code);
+ exit(1);
+ break;
+ }
+ return res;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index ab438ead70..6c4f237af8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -90,6 +90,9 @@
# QAPI introspection
{ 'include': 'qapi/introspect.json' }
+# Instrumentation commands
+{ 'include': 'qapi/instrument.json' }
+
##
# = QMP commands
##
diff --git a/qapi/instrument.json b/qapi/instrument.json
new file mode 100644
index 0000000000..f0d725e9a9
--- /dev/null
+++ b/qapi/instrument.json
@@ -0,0 +1,92 @@
+# *-*- Mode: Python -*-*
+#
+# QAPI trace instrumentation control commands.
+#
+# Copyright (C) 2012-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.
+
+##
+# @InstrLoadCode:
+#
+# Result code of an 'instr-load' command.
+#
+# @ok: Correctly loaded.
+# @unavailable: Service not available.
+# @error: Error with libdl (see 'msg').
+#
+# Since: 2.10
+##
+{ 'enum': 'InstrLoadCode',
+ 'data': [ 'ok', 'unavailable', 'error' ] }
+
+##
+# @InstrLoadResult:
+#
+# Result of an 'instr-load' command.
+#
+# @code: Result code.
+# @msg: Additional error message.
+# @handle: Instrumentation library identifier (undefined in case of error).
+#
+# Since: 2.10
+##
+{ 'struct': 'InstrLoadResult',
+ 'data': { 'code': 'InstrLoadCode', 'msg': 'str', 'handle': 'int' } }
+
+##
+# @instr-load:
+#
+# Load an instrumentation library.
+#
+# @path: path to the dynamic instrumentation library
+# @args: arguments to the dynamic instrumentation library
+#
+# Since: 2.10
+##
+{ 'command': 'instr-load',
+ 'data': { 'path': 'str', '*args': ['String'] },
+ 'returns': 'InstrLoadResult' }
+
+
+##
+# @InstrUnloadCode:
+#
+# Result code of an 'instr-unload' command.
+#
+# @ok: Correctly unloaded.
+# @unavailable: Service not available.
+# @invalid: Invalid handle.
+# @error: Error with libdl (see 'msg').
+#
+# Since: 2.10
+##
+{ 'enum': 'InstrUnloadCode',
+ 'data': [ 'ok', 'unavailable', 'invalid', 'error' ] }
+
+##
+# @InstrUnloadResult:
+#
+# Result of an 'instr-unload' command.
+#
+# @code: Result code.
+# @msg: Additional error message.
+#
+# Since: 2.10
+##
+{ 'struct': 'InstrUnloadResult',
+ 'data': { 'code': 'InstrUnloadCode', 'msg': 'str' } }
+
+##
+# @instr-unload:
+#
+# Unload an instrumentation library.
+#
+# @handle: Instrumentation library identifier (see #InstrLoadResult).
+#
+# Since: 2.10
+##
+{ 'command': 'instr-unload',
+ 'data': { 'handle': 'int' },
+ 'returns': 'InstrUnloadResult' }
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 12/13] instrument: [hmp] Add library loader
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (10 preceding siblings ...)
2017-07-24 17:46 ` [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add " Lluís Vilanova
@ 2017-07-24 17:50 ` Lluís Vilanova
2017-07-24 17:54 ` [Qemu-devel] [PATCH 13/13] trace: Rename C++-specific names in event arguments Lluís Vilanova
2017-07-25 13:19 ` [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Stefan Hajnoczi
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:50 UTC (permalink / raw)
To: qemu-devel
Cc: Emilio G. Cota, Stefan Hajnoczi, Dr. David Alan Gilbert,
Markus Armbruster
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
hmp-commands.hx | 28 ++++++++++++++++++++++++++++
monitor.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1941e19932..b1f3a75ab0 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1858,6 +1858,34 @@ ETEXI
.sub_table = info_cmds,
},
+ {
+ .name = "instr-load",
+ .args_type = "path:F,args:s?",
+ .params = "path [args]",
+ .help = "load an instrumentation library",
+ .cmd = hmp_instr_load,
+ },
+
+STEXI
+@item instr-load @var{path} [args=value[,...]]
+@findex instr-load
+Load an instrumentation library.
+ETEXI
+
+ {
+ .name = "instr-unload",
+ .args_type = "handle:i",
+ .params = "",
+ .help = "unload an instrumentation library",
+ .cmd = hmp_instr_unload,
+ },
+
+STEXI
+@item instr-unload
+@findex instr-unload
+Unload an instrumentation library.
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/monitor.c b/monitor.c
index 6d040e620f..4784356798 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2319,6 +2319,61 @@ int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp)
return fd;
}
+static void hmp_instr_load(Monitor *mon, const QDict *qdict)
+{
+ const char *path = qdict_get_str(qdict, "path");
+ String str;
+ str.str = (char *)qdict_get_try_str(qdict, "args");
+ StringList args;
+ args.value = str.str == NULL ? NULL : &str;
+ args.next = NULL;
+ InstrLoadResult *res = qmp_instr_load(path, args.value != NULL,
+ args.value != NULL ? &args : NULL,
+ NULL);
+ switch (res->code) {
+ case INSTR_LOAD_CODE_OK:
+ monitor_printf(mon, "Handle: %"PRId64"\n", res->handle);
+ monitor_printf(mon, "OK\n");
+ break;
+ case INSTR_LOAD_CODE_UNAVAILABLE:
+ monitor_printf(mon, "Not available\n");
+ break;
+ case INSTR_LOAD_CODE_ERROR:
+ monitor_printf(mon, "Error loading library: %s\n", res->msg);
+ break;
+ default:
+ fprintf(stderr, "Unknown instrumentation load code: %d", res->code);
+ exit(1);
+ break;
+ }
+ qapi_free_InstrLoadResult(res);
+}
+
+static void hmp_instr_unload(Monitor *mon, const QDict *qdict)
+{
+ int64_t handle = qdict_get_int(qdict, "handle");
+ InstrUnloadResult *res = qmp_instr_unload(handle, NULL);
+ switch (res->code) {
+ case INSTR_UNLOAD_CODE_OK:
+ monitor_printf(mon, "OK\n");
+ break;
+ case INSTR_UNLOAD_CODE_UNAVAILABLE:
+ monitor_printf(mon, "Not available\n");
+ break;
+ case INSTR_UNLOAD_CODE_INVALID:
+ monitor_printf(mon, "Invalid handle\n");
+ break;
+ case INSTR_UNLOAD_CODE_ERROR:
+ monitor_printf(mon, "Error unloading library: %s\n", res->msg);
+ break;
+ default:
+ fprintf(stderr, "Unknown instrumentation unload code: %d", res->code);
+ exit(1);
+ break;
+ }
+ qapi_free_InstrUnloadResult(res);
+}
+
/* Please update hmp-commands.hx when adding or changing commands */
static mon_cmd_t info_cmds[] = {
#include "hmp-commands-info.h"
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [PATCH 13/13] trace: Rename C++-specific names in event arguments
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (11 preceding siblings ...)
2017-07-24 17:50 ` [Qemu-devel] [PATCH 12/13] instrument: [hmp] " Lluís Vilanova
@ 2017-07-24 17:54 ` Lluís Vilanova
2017-07-25 13:19 ` [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Stefan Hajnoczi
13 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-24 17:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota, Stefan Hajnoczi
Allows instrumentation clients in C++ to use the auto-generated headers.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
scripts/tracetool/__init__.py | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 7c19dc8c94..be7f28d2be 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -42,6 +42,8 @@ def out(*lines, **kwargs):
sys.stdout.writelines("\n".join(lines) + "\n")
+FORBIDDEN = ["new", "class"]
+
class Arguments:
"""Event arguments description."""
@@ -85,6 +87,9 @@ class Arguments:
else:
arg_type, identifier = arg.rsplit(None, 1)
+ if identifier in FORBIDDEN:
+ identifier += "_"
+
res.append((arg_type, identifier))
return Arguments(res)
@@ -230,7 +235,10 @@ class Event(object):
fmt_trans = groups["fmt_trans"]
if len(fmt_trans) > 0:
fmt = [fmt_trans, fmt]
- args = Arguments.build(groups["args"])
+ try:
+ args = Arguments.build(groups["args"])
+ except ForbiddenArgumentError as e:
+ error("Error: event '%s' uses forbidden argument name '%s'" % (name, e.name))
if "tcg-trans" in props:
raise ValueError("Invalid property 'tcg-trans'")
^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader
2017-07-24 17:46 ` [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add " Lluís Vilanova
@ 2017-07-24 18:03 ` Eric Blake
2017-07-25 8:24 ` Lluís Vilanova
0 siblings, 1 reply; 59+ messages in thread
From: Eric Blake @ 2017-07-24 18:03 UTC (permalink / raw)
To: Lluís Vilanova, qemu-devel
Cc: Emilio G. Cota, Stefan Hajnoczi, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 3786 bytes --]
On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> instrument/Makefile.objs | 1 +
> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
> qapi-schema.json | 3 ++
> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 167 insertions(+)
> create mode 100644 instrument/qmp.c
> create mode 100644 qapi/instrument.json
Adding new files; but I don't see a patch to MAINTAINERS to cover
instrument/*.
> +++ b/qapi/instrument.json
> @@ -0,0 +1,92 @@
> +# *-*- Mode: Python -*-*
> +#
> +# QAPI trace instrumentation control commands.
> +#
> +# Copyright (C) 2012-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.
> +
> +##
> +# @InstrLoadCode:
> +#
> +# Result code of an 'instr-load' command.
> +#
> +# @ok: Correctly loaded.
> +# @unavailable: Service not available.
> +# @error: Error with libdl (see 'msg').
> +#
> +# Since: 2.10
This is a new feature, and you've missed soft freeze. You'll want to
use 2.11 throughout the file.
> +##
> +{ 'enum': 'InstrLoadCode',
> + 'data': [ 'ok', 'unavailable', 'error' ] }
> +
> +##
> +# @InstrLoadResult:
> +#
> +# Result of an 'instr-load' command.
> +#
> +# @code: Result code.
> +# @msg: Additional error message.
Worth a comment that the message is for human consumption, and should
not be further parsed by machine?
Should msg be optional, present only when there is an error?
> +# @handle: Instrumentation library identifier (undefined in case of error).
Should it be an optional member, omitted when there is an error?
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'InstrLoadResult',
> + 'data': { 'code': 'InstrLoadCode', 'msg': 'str', 'handle': 'int' } }
> +
> +##
> +# @instr-load:
> +#
> +# Load an instrumentation library.
> +#
> +# @path: path to the dynamic instrumentation library
> +# @args: arguments to the dynamic instrumentation library
> +#
> +# Since: 2.10
> +##
> +{ 'command': 'instr-load',
> + 'data': { 'path': 'str', '*args': ['String'] },
Why are you double-nesting things? It's a lot nicer to use ['str']
(that is, your way requires
"arguments":{"path":"/some/path",
"args": [ { "str": "string1" }, { "str": "string2" } ] }
whereas mine only needs:
"arguments":{"path":"/some/path", "args":[ "string1", "string2" ]}
> + 'returns': 'InstrLoadResult' }
> +
> +
> +##
> +# @InstrUnloadCode:
> +#
> +# Result code of an 'instr-unload' command.
> +#
> +# @ok: Correctly unloaded.
> +# @unavailable: Service not available.
> +# @invalid: Invalid handle.
> +# @error: Error with libdl (see 'msg').
> +#
> +# Since: 2.10
> +##
> +{ 'enum': 'InstrUnloadCode',
> + 'data': [ 'ok', 'unavailable', 'invalid', 'error' ] }
> +
> +##
> +# @InstrUnloadResult:
> +#
> +# Result of an 'instr-unload' command.
> +#
> +# @code: Result code.
> +# @msg: Additional error message.
Again, should msg be optional? Document that it is only for human
consumption.
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'InstrUnloadResult',
> + 'data': { 'code': 'InstrUnloadCode', 'msg': 'str' } }
> +
> +##
> +# @instr-unload:
> +#
> +# Unload an instrumentation library.
> +#
> +# @handle: Instrumentation library identifier (see #InstrLoadResult).
> +#
> +# Since: 2.10
> +##
> +{ 'command': 'instr-unload',
> + 'data': { 'handle': 'int' },
> + 'returns': 'InstrUnloadResult' }
>
>
--
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] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader
2017-07-24 18:03 ` Eric Blake
@ 2017-07-25 8:24 ` Lluís Vilanova
2017-07-25 11:30 ` Eric Blake
0 siblings, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-25 8:24 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Emilio G. Cota, Markus Armbruster, Stefan Hajnoczi
Eric Blake writes:
> On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> instrument/Makefile.objs | 1 +
>> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
>> qapi-schema.json | 3 ++
>> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 167 insertions(+)
>> create mode 100644 instrument/qmp.c
>> create mode 100644 qapi/instrument.json
> Adding new files; but I don't see a patch to MAINTAINERS to cover
> instrument/*.
Who should I put as a maintainer? Or does this go to the general maintainer(s)?
>> +++ b/qapi/instrument.json
>> @@ -0,0 +1,92 @@
>> +# *-*- Mode: Python -*-*
>> +#
>> +# QAPI trace instrumentation control commands.
>> +#
>> +# Copyright (C) 2012-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.
>> +
>> +##
>> +# @InstrLoadCode:
>> +#
>> +# Result code of an 'instr-load' command.
>> +#
>> +# @ok: Correctly loaded.
>> +# @unavailable: Service not available.
>> +# @error: Error with libdl (see 'msg').
>> +#
>> +# Since: 2.10
> This is a new feature, and you've missed soft freeze. You'll want to
> use 2.11 throughout the file.
Thx!
>> +##
>> +{ 'enum': 'InstrLoadCode',
>> + 'data': [ 'ok', 'unavailable', 'error' ] }
>> +
>> +##
>> +# @InstrLoadResult:
>> +#
>> +# Result of an 'instr-load' command.
>> +#
>> +# @code: Result code.
>> +# @msg: Additional error message.
> Worth a comment that the message is for human consumption, and should
> not be further parsed by machine?
> Should msg be optional, present only when there is an error?
True.
>> +# @handle: Instrumentation library identifier (undefined in case of error).
> Should it be an optional member, omitted when there is an error?
Also true.
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'struct': 'InstrLoadResult',
>> + 'data': { 'code': 'InstrLoadCode', 'msg': 'str', 'handle': 'int' } }
>> +
>> +##
>> +# @instr-load:
>> +#
>> +# Load an instrumentation library.
>> +#
>> +# @path: path to the dynamic instrumentation library
>> +# @args: arguments to the dynamic instrumentation library
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'command': 'instr-load',
>> + 'data': { 'path': 'str', '*args': ['String'] },
> Why are you double-nesting things? It's a lot nicer to use ['str']
> (that is, your way requires
> "arguments":{"path":"/some/path",
> "args": [ { "str": "string1" }, { "str": "string2" } ] }
> whereas mine only needs:
> "arguments":{"path":"/some/path", "args":[ "string1", "string2" ]}
Aha, you mean the definition should be this instead?
{ 'command': 'instr-load',
'data': { 'path': 'str', '*args': ['str'] },
'returns': 'InstrLoadResult' }
>> + 'returns': 'InstrLoadResult' }
>> +
>> +
>> +##
>> +# @InstrUnloadCode:
>> +#
>> +# Result code of an 'instr-unload' command.
>> +#
>> +# @ok: Correctly unloaded.
>> +# @unavailable: Service not available.
>> +# @invalid: Invalid handle.
>> +# @error: Error with libdl (see 'msg').
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'enum': 'InstrUnloadCode',
>> + 'data': [ 'ok', 'unavailable', 'invalid', 'error' ] }
>> +
>> +##
>> +# @InstrUnloadResult:
>> +#
>> +# Result of an 'instr-unload' command.
>> +#
>> +# @code: Result code.
>> +# @msg: Additional error message.
> Again, should msg be optional? Document that it is only for human
> consumption.
Ok.
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'struct': 'InstrUnloadResult',
>> + 'data': { 'code': 'InstrUnloadCode', 'msg': 'str' } }
>> +
>> +##
>> +# @instr-unload:
>> +#
>> +# Unload an instrumentation library.
>> +#
>> +# @handle: Instrumentation library identifier (see #InstrLoadResult).
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'command': 'instr-unload',
>> + 'data': { 'handle': 'int' },
>> + 'returns': 'InstrUnloadResult' }
>>
>>
Thanks,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader
2017-07-25 8:24 ` Lluís Vilanova
@ 2017-07-25 11:30 ` Eric Blake
2017-07-25 11:51 ` Lluís Vilanova
0 siblings, 1 reply; 59+ messages in thread
From: Eric Blake @ 2017-07-25 11:30 UTC (permalink / raw)
To: qemu-devel, Emilio G. Cota, Markus Armbruster, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]
On 07/25/2017 03:24 AM, Lluís Vilanova wrote:
> Eric Blake writes:
>
>> On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> ---
>>> instrument/Makefile.objs | 1 +
>>> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
>>> qapi-schema.json | 3 ++
>>> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 167 insertions(+)
>>> create mode 100644 instrument/qmp.c
>>> create mode 100644 qapi/instrument.json
>
>> Adding new files; but I don't see a patch to MAINTAINERS to cover
>> instrument/*.
>
> Who should I put as a maintainer? Or does this go to the general maintainer(s)?
You can be the maintainer if you'd like; or see if Stefan is okay
including it as part of the trace files, since it trace-related.
>>> +##
>>> +{ 'command': 'instr-load',
>>> + 'data': { 'path': 'str', '*args': ['String'] },
>
>> Why are you double-nesting things? It's a lot nicer to use ['str']
> Aha, you mean the definition should be this instead?
>
> { 'command': 'instr-load',
> 'data': { 'path': 'str', '*args': ['str'] },
> 'returns': 'InstrLoadResult' }
Yes.
--
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] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader
2017-07-25 11:30 ` Eric Blake
@ 2017-07-25 11:51 ` Lluís Vilanova
0 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-25 11:51 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Emilio G. Cota, Markus Armbruster, Stefan Hajnoczi
Eric Blake writes:
> On 07/25/2017 03:24 AM, Lluís Vilanova wrote:
>> Eric Blake writes:
>>
>>> On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>> ---
>>>> instrument/Makefile.objs | 1 +
>>>> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
>>>> qapi-schema.json | 3 ++
>>>> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 167 insertions(+)
>>>> create mode 100644 instrument/qmp.c
>>>> create mode 100644 qapi/instrument.json
>>
>>> Adding new files; but I don't see a patch to MAINTAINERS to cover
>>> instrument/*.
>>
>> Who should I put as a maintainer? Or does this go to the general maintainer(s)?
> You can be the maintainer if you'd like; or see if Stefan is okay
> including it as part of the trace files, since it trace-related.
I can certainly do that. I was just wondering if the project prefers to have
someone on the "core team" as a maintainer. Stefan?
Thanks,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
` (12 preceding siblings ...)
2017-07-24 17:54 ` [Qemu-devel] [PATCH 13/13] trace: Rename C++-specific names in event arguments Lluís Vilanova
@ 2017-07-25 13:19 ` Stefan Hajnoczi
2017-07-25 13:30 ` Peter Maydell
2017-07-25 14:47 ` Lluís Vilanova
13 siblings, 2 replies; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-07-25 13:19 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: qemu-devel, Emilio G. Cota, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]
On Mon, Jul 24, 2017 at 08:02:24PM +0300, Lluís Vilanova wrote:
> This series adds a basic interface to instrument tracing events and control
> their tracing state.
>
> The instrumentation code is dynamically loaded into QEMU (either when it starts
> or later using its remote control interfaces).
>
> All events can be instrumented, but the instrumentable events must be explicitly
> specified at configure time.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Hi Lluís,
I'm concerned that the shared library interface will be abused to monkey
patch code into QEMU far beyond instrumentation use cases and/or avoid
the responsibilities of the GPL license.
Instead I suggest adding a trace backend generates calls to registered
"callback" functions:
$ cat >my-instrumentation.c
#include "trace/control.h"
static void my_cpu_in(unsigned int addr, char size, unsigned int val)
{
printf("my_cpu_in\n");
}
static void my_init(void)
{
trace_register_event_callback("cpu_in", my_cpu_in);
trace_enable_events("cpu_in");
}
trace_init(my_init);
$ ./configure --enable-trace-backends=log,callback && make -j4
This is still a clean interface that allows instrumentation code to be
kept separate from the trace event call sites.
The instrumentation code gets compiled into QEMU, but that shouldn't be
a huge burden since QEMU's Makefiles only recompile changed source
files (only the first build is slow).
Does this alternative sound reasonable to you?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-25 13:19 ` [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Stefan Hajnoczi
@ 2017-07-25 13:30 ` Peter Maydell
2017-07-25 15:11 ` Lluís Vilanova
2017-07-26 11:26 ` Stefan Hajnoczi
2017-07-25 14:47 ` Lluís Vilanova
1 sibling, 2 replies; 59+ messages in thread
From: Peter Maydell @ 2017-07-25 13:30 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Lluís Vilanova, Emilio G. Cota, QEMU Developers, Stefan Hajnoczi
On 25 July 2017 at 14:19, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> Instead I suggest adding a trace backend generates calls to registered
> "callback" functions:
>
> $ cat >my-instrumentation.c
> #include "trace/control.h"
>
> static void my_cpu_in(unsigned int addr, char size, unsigned int val)
> {
> printf("my_cpu_in\n");
> }
>
> static void my_init(void)
> {
> trace_register_event_callback("cpu_in", my_cpu_in);
> trace_enable_events("cpu_in");
> }
> trace_init(my_init);
>
> $ ./configure --enable-trace-backends=log,callback && make -j4
>
> This is still a clean interface that allows instrumentation code to be
> kept separate from the trace event call sites.
>
> The instrumentation code gets compiled into QEMU, but that shouldn't be
> a huge burden since QEMU's Makefiles only recompile changed source
> files (only the first build is slow).
Is your proposal that my-instrumentation.c gets compiled into
QEMU at this point? That doesn't seem like a great idea to
me, because it means you can only use this tracing if you
build QEMU yourself, and distros won't enable it and so
lots of our users won't have convenient access to it.
I'd much rather see a cleanly designed plugin interface
(which we should be able to implement in a manner that doesn't
let the plugin monkey patch arbitrary parts of QEMU beyond
what can already be achieved via LD_PRELOAD).
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-25 13:19 ` [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Stefan Hajnoczi
2017-07-25 13:30 ` Peter Maydell
@ 2017-07-25 14:47 ` Lluís Vilanova
2017-07-26 11:29 ` Stefan Hajnoczi
1 sibling, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-25 14:47 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Emilio G. Cota, Stefan Hajnoczi
Stefan Hajnoczi writes:
> On Mon, Jul 24, 2017 at 08:02:24PM +0300, Lluís Vilanova wrote:
>> This series adds a basic interface to instrument tracing events and control
>> their tracing state.
>>
>> The instrumentation code is dynamically loaded into QEMU (either when it starts
>> or later using its remote control interfaces).
>>
>> All events can be instrumented, but the instrumentable events must be explicitly
>> specified at configure time.
>>
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> Hi Lluís,
> I'm concerned that the shared library interface will be abused to monkey
> patch code into QEMU far beyond instrumentation use cases and/or avoid
> the responsibilities of the GPL license.
> Instead I suggest adding a trace backend generates calls to registered
> "callback" functions:
> $ cat >my-instrumentation.c
> #include "trace/control.h"
> static void my_cpu_in(unsigned int addr, char size, unsigned int val)
> {
> printf("my_cpu_in\n");
> }
> static void my_init(void)
> {
> trace_register_event_callback("cpu_in", my_cpu_in);
> trace_enable_events("cpu_in");
> }
> trace_init(my_init);
> $ ./configure --enable-trace-backends=log,callback && make -j4
> This is still a clean interface that allows instrumentation code to be
> kept separate from the trace event call sites.
> The instrumentation code gets compiled into QEMU, but that shouldn't be
> a huge burden since QEMU's Makefiles only recompile changed source
> files (only the first build is slow).
> Does this alternative sound reasonable to you?
You mean to add a user-provided .c file to QEMU at compile-time? (I'm assuming
we can keep the "user API" proposed in this series, instead of the one you
showed).
First, a user might want to provide more than just a .c, so we might have to
accept a directory that produces a library that is included into QEMU at link
time (a bit more complicated to do portably).
Second, the user can still do the same actions you want to shield from,
regardless of whether it's a dynamically loaded library (i.e., access any
fuction in QEMU).
What I propose to do instead is:
* For the monkey-patch part, we can limit symbol resolution to the
instrumentation API functions when loading the library (e.g., compile QEMU
with -fvisibility=hidden).
* For the license part, that is a legal issue that can be handled by the API
header license, right? (the "public" headers I added are GPL, not
LGPL). Besides, if only the intended API is available, I'm not sure if that
matters (e.g., we don't care about the license of a dtrace script, since it
only has the API provided by QEMU+dtrace).
This would be similar to Linux's module support; only selected functions are
available to modules, and we could add a license check (e.g., QI_LICENSE("GPL")
must be on the instrumentation library or it won't be loaded).
Thanks,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-25 13:30 ` Peter Maydell
@ 2017-07-25 15:11 ` Lluís Vilanova
2017-07-26 11:22 ` Stefan Hajnoczi
2017-07-26 11:26 ` Stefan Hajnoczi
1 sibling, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-25 15:11 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Emilio G. Cota, Stefan Hajnoczi, QEMU Developers
Peter Maydell writes:
> On 25 July 2017 at 14:19, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> Instead I suggest adding a trace backend generates calls to registered
>> "callback" functions:
>>
>> $ cat >my-instrumentation.c
>> #include "trace/control.h"
>>
>> static void my_cpu_in(unsigned int addr, char size, unsigned int val)
>> {
>> printf("my_cpu_in\n");
>> }
>>
>> static void my_init(void)
>> {
>> trace_register_event_callback("cpu_in", my_cpu_in);
>> trace_enable_events("cpu_in");
>> }
>> trace_init(my_init);
>>
>> $ ./configure --enable-trace-backends=log,callback && make -j4
>>
>> This is still a clean interface that allows instrumentation code to be
>> kept separate from the trace event call sites.
>>
>> The instrumentation code gets compiled into QEMU, but that shouldn't be
>> a huge burden since QEMU's Makefiles only recompile changed source
>> files (only the first build is slow).
> Is your proposal that my-instrumentation.c gets compiled into
> QEMU at this point? That doesn't seem like a great idea to
> me, because it means you can only use this tracing if you
> build QEMU yourself, and distros won't enable it and so
> lots of our users won't have convenient access to it.
> I'd much rather see a cleanly designed plugin interface
> (which we should be able to implement in a manner that doesn't
> let the plugin monkey patch arbitrary parts of QEMU beyond
> what can already be achieved via LD_PRELOAD).
Just to be clear, what do you both mean by monkey-patching?
* Accessing unintended symbols in QEMU from the library (and from there
modifying QEMU's behavior).
* QEMU using symbols on the library instead of its own just because they have
the same name.
As I said, the former can be accomplished by compiling QEMU with
"-fvisibility=hidden".
The latter is already achieved by using dlopen with RTLD_LOCAL (the default).
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-25 15:11 ` Lluís Vilanova
@ 2017-07-26 11:22 ` Stefan Hajnoczi
2017-07-26 12:44 ` Lluís Vilanova
0 siblings, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-07-26 11:22 UTC (permalink / raw)
To: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]
On Tue, Jul 25, 2017 at 06:11:43PM +0300, Lluís Vilanova wrote:
> Peter Maydell writes:
>
> > On 25 July 2017 at 14:19, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> >> Instead I suggest adding a trace backend generates calls to registered
> >> "callback" functions:
> >>
> >> $ cat >my-instrumentation.c
> >> #include "trace/control.h"
> >>
> >> static void my_cpu_in(unsigned int addr, char size, unsigned int val)
> >> {
> >> printf("my_cpu_in\n");
> >> }
> >>
> >> static void my_init(void)
> >> {
> >> trace_register_event_callback("cpu_in", my_cpu_in);
> >> trace_enable_events("cpu_in");
> >> }
> >> trace_init(my_init);
> >>
> >> $ ./configure --enable-trace-backends=log,callback && make -j4
> >>
> >> This is still a clean interface that allows instrumentation code to be
> >> kept separate from the trace event call sites.
> >>
> >> The instrumentation code gets compiled into QEMU, but that shouldn't be
> >> a huge burden since QEMU's Makefiles only recompile changed source
> >> files (only the first build is slow).
>
> > Is your proposal that my-instrumentation.c gets compiled into
> > QEMU at this point? That doesn't seem like a great idea to
> > me, because it means you can only use this tracing if you
> > build QEMU yourself, and distros won't enable it and so
> > lots of our users won't have convenient access to it.
> > I'd much rather see a cleanly designed plugin interface
> > (which we should be able to implement in a manner that doesn't
> > let the plugin monkey patch arbitrary parts of QEMU beyond
> > what can already be achieved via LD_PRELOAD).
>
> Just to be clear, what do you both mean by monkey-patching?
>
> * Accessing unintended symbols in QEMU from the library (and from there
> modifying QEMU's behavior).
> * QEMU using symbols on the library instead of its own just because they have
> the same name.
>
> As I said, the former can be accomplished by compiling QEMU with
> "-fvisibility=hidden".
>
> The latter is already achieved by using dlopen with RTLD_LOCAL (the default).
Instrumentation functions are invoked in the same memory space as QEMU,
so pointer arguments can be modified.
Think of all the void *s arguments we trace. Instrumentation code can
access those structs, hijack function pointers, etc. No symbols are
required.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-25 13:30 ` Peter Maydell
2017-07-25 15:11 ` Lluís Vilanova
@ 2017-07-26 11:26 ` Stefan Hajnoczi
2017-07-26 11:49 ` Peter Maydell
1 sibling, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-07-26 11:26 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Lluís Vilanova, Emilio G. Cota, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]
On Tue, Jul 25, 2017 at 02:30:06PM +0100, Peter Maydell wrote:
> On 25 July 2017 at 14:19, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > Instead I suggest adding a trace backend generates calls to registered
> > "callback" functions:
> >
> > $ cat >my-instrumentation.c
> > #include "trace/control.h"
> >
> > static void my_cpu_in(unsigned int addr, char size, unsigned int val)
> > {
> > printf("my_cpu_in\n");
> > }
> >
> > static void my_init(void)
> > {
> > trace_register_event_callback("cpu_in", my_cpu_in);
> > trace_enable_events("cpu_in");
> > }
> > trace_init(my_init);
> >
> > $ ./configure --enable-trace-backends=log,callback && make -j4
> >
> > This is still a clean interface that allows instrumentation code to be
> > kept separate from the trace event call sites.
> >
> > The instrumentation code gets compiled into QEMU, but that shouldn't be
> > a huge burden since QEMU's Makefiles only recompile changed source
> > files (only the first build is slow).
>
> Is your proposal that my-instrumentation.c gets compiled into
> QEMU at this point? That doesn't seem like a great idea to
> me, because it means you can only use this tracing if you
> build QEMU yourself, and distros won't enable it and so
> lots of our users won't have convenient access to it.
> I'd much rather see a cleanly designed plugin interface
> (which we should be able to implement in a manner that doesn't
> let the plugin monkey patch arbitrary parts of QEMU beyond
> what can already be achieved via LD_PRELOAD).
Trace events are not a stable public API. They change between releases
and need to be interpreted together with the QEMU source code.
It doesn't make sense to be doing instrumentation inside the QEMU
process but not want to compile QEMU.
What is the use case?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-25 14:47 ` Lluís Vilanova
@ 2017-07-26 11:29 ` Stefan Hajnoczi
2017-07-26 12:31 ` Lluís Vilanova
0 siblings, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-07-26 11:29 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel, Emilio G. Cota
[-- Attachment #1: Type: text/plain, Size: 3456 bytes --]
On Tue, Jul 25, 2017 at 05:47:08PM +0300, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
>
> > On Mon, Jul 24, 2017 at 08:02:24PM +0300, Lluís Vilanova wrote:
> >> This series adds a basic interface to instrument tracing events and control
> >> their tracing state.
> >>
> >> The instrumentation code is dynamically loaded into QEMU (either when it starts
> >> or later using its remote control interfaces).
> >>
> >> All events can be instrumented, but the instrumentable events must be explicitly
> >> specified at configure time.
> >>
> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>
> > Hi Lluís,
> > I'm concerned that the shared library interface will be abused to monkey
> > patch code into QEMU far beyond instrumentation use cases and/or avoid
> > the responsibilities of the GPL license.
>
> > Instead I suggest adding a trace backend generates calls to registered
> > "callback" functions:
>
> > $ cat >my-instrumentation.c
> > #include "trace/control.h"
>
> > static void my_cpu_in(unsigned int addr, char size, unsigned int val)
> > {
> > printf("my_cpu_in\n");
> > }
>
> > static void my_init(void)
> > {
> > trace_register_event_callback("cpu_in", my_cpu_in);
> > trace_enable_events("cpu_in");
> > }
> > trace_init(my_init);
>
> > $ ./configure --enable-trace-backends=log,callback && make -j4
>
> > This is still a clean interface that allows instrumentation code to be
> > kept separate from the trace event call sites.
>
> > The instrumentation code gets compiled into QEMU, but that shouldn't be
> > a huge burden since QEMU's Makefiles only recompile changed source
> > files (only the first build is slow).
>
> > Does this alternative sound reasonable to you?
>
> You mean to add a user-provided .c file to QEMU at compile-time? (I'm assuming
> we can keep the "user API" proposed in this series, instead of the one you
> showed).
>
> First, a user might want to provide more than just a .c, so we might have to
> accept a directory that produces a library that is included into QEMU at link
> time (a bit more complicated to do portably).
>
> Second, the user can still do the same actions you want to shield from,
> regardless of whether it's a dynamically loaded library (i.e., access any
> fuction in QEMU).
>
> What I propose to do instead is:
>
> * For the monkey-patch part, we can limit symbol resolution to the
> instrumentation API functions when loading the library (e.g., compile QEMU
> with -fvisibility=hidden).
>
> * For the license part, that is a legal issue that can be handled by the API
> header license, right? (the "public" headers I added are GPL, not
> LGPL). Besides, if only the intended API is available, I'm not sure if that
> matters (e.g., we don't care about the license of a dtrace script, since it
> only has the API provided by QEMU+dtrace).
>
> This would be similar to Linux's module support; only selected functions are
> available to modules, and we could add a license check (e.g., QI_LICENSE("GPL")
> must be on the instrumentation library or it won't be loaded).
Proprietary Linux kernel modules are controversial and some still
consider them license violations - especially when an "open source" shim
module is used to interface between proprietary code and the kernel.
What is the use case for this instrumentation?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-26 11:26 ` Stefan Hajnoczi
@ 2017-07-26 11:49 ` Peter Maydell
2017-07-26 12:26 ` Lluís Vilanova
2017-07-27 10:43 ` Daniel P. Berrange
0 siblings, 2 replies; 59+ messages in thread
From: Peter Maydell @ 2017-07-26 11:49 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Stefan Hajnoczi, Lluís Vilanova, Emilio G. Cota, QEMU Developers
On 26 July 2017 at 12:26, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Tue, Jul 25, 2017 at 02:30:06PM +0100, Peter Maydell wrote:
>> Is your proposal that my-instrumentation.c gets compiled into
>> QEMU at this point? That doesn't seem like a great idea to
>> me, because it means you can only use this tracing if you
>> build QEMU yourself, and distros won't enable it and so
>> lots of our users won't have convenient access to it.
>> I'd much rather see a cleanly designed plugin interface
>> (which we should be able to implement in a manner that doesn't
>> let the plugin monkey patch arbitrary parts of QEMU beyond
>> what can already be achieved via LD_PRELOAD).
>
> Trace events are not a stable public API. They change between releases
> and need to be interpreted together with the QEMU source code.
>
> It doesn't make sense to be doing instrumentation inside the QEMU
> process but not want to compile QEMU.
>
> What is the use case?
I want the continuous stream of people who come along
and want to do interesting things (or even just straightforward
things like "show me all the memory accesses") with tracing
their guest binary to be able to do so without having to
rebuild QEMU. We don't require people to rebuild QEMU
themselves to attach gdb to it, and tracing of guest
behaviour is a similar "I just want to introspect what
my guest is doing" use case.
I think this is distinct from use of the trace API
for qemu-internal events as used in a lot of the C
code.
Some simple use cases:
* a plugin which prints out instruction execution traces
by hooking into "insn executed" and printing something
helpful (you can extend this into producing traces
in whatever standard format some other program wants
to inhale, but at a minimum it would be something we
could point people at that makes more sense than our
current "only print insns at translate time" debug);
it's also easy to add bells and whistles for "start
tracing at time T" and so on
* a plugin that make cachegrind-style guesses about
cache usage by tracking memory accesses and simulating
cache effects
* a plugin that prints all the system calls the
guest makes (by hooking into "insn executed" and
printing info when the insn is an SVC)
* trivial statistics like "count executed instructions
and branches taken"
(I also think that we should aim for that API for
things like guest register changes and guest memory
changes to not be particularly unstable even if we
allow ourselves to make non-back-compatible changes
to it.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-26 11:49 ` Peter Maydell
@ 2017-07-26 12:26 ` Lluís Vilanova
2017-07-27 10:43 ` Daniel P. Berrange
1 sibling, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-26 12:26 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
Peter Maydell writes:
> On 26 July 2017 at 12:26, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> On Tue, Jul 25, 2017 at 02:30:06PM +0100, Peter Maydell wrote:
>>> Is your proposal that my-instrumentation.c gets compiled into
>>> QEMU at this point? That doesn't seem like a great idea to
>>> me, because it means you can only use this tracing if you
>>> build QEMU yourself, and distros won't enable it and so
>>> lots of our users won't have convenient access to it.
>>> I'd much rather see a cleanly designed plugin interface
>>> (which we should be able to implement in a manner that doesn't
>>> let the plugin monkey patch arbitrary parts of QEMU beyond
>>> what can already be achieved via LD_PRELOAD).
>>
>> Trace events are not a stable public API. They change between releases
>> and need to be interpreted together with the QEMU source code.
>>
>> It doesn't make sense to be doing instrumentation inside the QEMU
>> process but not want to compile QEMU.
>>
>> What is the use case?
> I want the continuous stream of people who come along
> and want to do interesting things (or even just straightforward
> things like "show me all the memory accesses") with tracing
> their guest binary to be able to do so without having to
> rebuild QEMU. We don't require people to rebuild QEMU
> themselves to attach gdb to it, and tracing of guest
> behaviour is a similar "I just want to introspect what
> my guest is doing" use case.
> I think this is distinct from use of the trace API
> for qemu-internal events as used in a lot of the C
> code.
> Some simple use cases:
> * a plugin which prints out instruction execution traces
> by hooking into "insn executed" and printing something
> helpful (you can extend this into producing traces
> in whatever standard format some other program wants
> to inhale, but at a minimum it would be something we
> could point people at that makes more sense than our
> current "only print insns at translate time" debug);
> it's also easy to add bells and whistles for "start
> tracing at time T" and so on
> * a plugin that make cachegrind-style guesses about
> cache usage by tracking memory accesses and simulating
> cache effects
> * a plugin that prints all the system calls the
> guest makes (by hooking into "insn executed" and
> printing info when the insn is an SVC)
> * trivial statistics like "count executed instructions
> and branches taken"
> (I also think that we should aim for that API for
> things like guest register changes and guest memory
> changes to not be particularly unstable even if we
> allow ourselves to make non-back-compatible changes
> to it.)
That's what I was thinking about (i.e., try to keep a minimally useful and
stable set of guest_* tracepoints). And I guess this also answer's Stefan's
other email about instrumentation use-cases.
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-26 11:29 ` Stefan Hajnoczi
@ 2017-07-26 12:31 ` Lluís Vilanova
0 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-26 12:31 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Stefan Hajnoczi, qemu-devel, Emilio G. Cota
Stefan Hajnoczi writes:
> On Tue, Jul 25, 2017 at 05:47:08PM +0300, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>>
>> > On Mon, Jul 24, 2017 at 08:02:24PM +0300, Lluís Vilanova wrote:
>> >> This series adds a basic interface to instrument tracing events and control
>> >> their tracing state.
>> >>
>> >> The instrumentation code is dynamically loaded into QEMU (either when it starts
>> >> or later using its remote control interfaces).
>> >>
>> >> All events can be instrumented, but the instrumentable events must be explicitly
>> >> specified at configure time.
>> >>
>> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>
>> > Hi Lluís,
>> > I'm concerned that the shared library interface will be abused to monkey
>> > patch code into QEMU far beyond instrumentation use cases and/or avoid
>> > the responsibilities of the GPL license.
>>
>> > Instead I suggest adding a trace backend generates calls to registered
>> > "callback" functions:
>>
>> > $ cat >my-instrumentation.c
>> > #include "trace/control.h"
>>
>> > static void my_cpu_in(unsigned int addr, char size, unsigned int val)
>> > {
>> > printf("my_cpu_in\n");
>> > }
>>
>> > static void my_init(void)
>> > {
>> > trace_register_event_callback("cpu_in", my_cpu_in);
>> > trace_enable_events("cpu_in");
>> > }
>> > trace_init(my_init);
>>
>> > $ ./configure --enable-trace-backends=log,callback && make -j4
>>
>> > This is still a clean interface that allows instrumentation code to be
>> > kept separate from the trace event call sites.
>>
>> > The instrumentation code gets compiled into QEMU, but that shouldn't be
>> > a huge burden since QEMU's Makefiles only recompile changed source
>> > files (only the first build is slow).
>>
>> > Does this alternative sound reasonable to you?
>>
>> You mean to add a user-provided .c file to QEMU at compile-time? (I'm assuming
>> we can keep the "user API" proposed in this series, instead of the one you
>> showed).
>>
>> First, a user might want to provide more than just a .c, so we might have to
>> accept a directory that produces a library that is included into QEMU at link
>> time (a bit more complicated to do portably).
>>
>> Second, the user can still do the same actions you want to shield from,
>> regardless of whether it's a dynamically loaded library (i.e., access any
>> fuction in QEMU).
>>
>> What I propose to do instead is:
>>
>> * For the monkey-patch part, we can limit symbol resolution to the
>> instrumentation API functions when loading the library (e.g., compile QEMU
>> with -fvisibility=hidden).
>>
>> * For the license part, that is a legal issue that can be handled by the API
>> header license, right? (the "public" headers I added are GPL, not
>> LGPL). Besides, if only the intended API is available, I'm not sure if that
>> matters (e.g., we don't care about the license of a dtrace script, since it
>> only has the API provided by QEMU+dtrace).
>>
>> This would be similar to Linux's module support; only selected functions are
>> available to modules, and we could add a license check (e.g., QI_LICENSE("GPL")
>> must be on the instrumentation library or it won't be loaded).
> Proprietary Linux kernel modules are controversial and some still
> consider them license violations - especially when an "open source" shim
> module is used to interface between proprietary code and the kernel.
> What is the use case for this instrumentation?
As I said, we can require instrumentation libraries to be GPL (and nothing
else), so no proprietary code is possible without a license violation (then
we're on the same field as if someone ships a modified QEMU binary, which
technically is just as doable).
As for use-cases, see Peter's email and my response.
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-26 11:22 ` Stefan Hajnoczi
@ 2017-07-26 12:44 ` Lluís Vilanova
2017-07-27 10:32 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-26 12:44 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
Stefan Hajnoczi writes:
> On Tue, Jul 25, 2017 at 06:11:43PM +0300, Lluís Vilanova wrote:
>> Peter Maydell writes:
>>
>> > On 25 July 2017 at 14:19, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> >> Instead I suggest adding a trace backend generates calls to registered
>> >> "callback" functions:
>> >>
>> >> $ cat >my-instrumentation.c
>> >> #include "trace/control.h"
>> >>
>> >> static void my_cpu_in(unsigned int addr, char size, unsigned int val)
>> >> {
>> >> printf("my_cpu_in\n");
>> >> }
>> >>
>> >> static void my_init(void)
>> >> {
>> >> trace_register_event_callback("cpu_in", my_cpu_in);
>> >> trace_enable_events("cpu_in");
>> >> }
>> >> trace_init(my_init);
>> >>
>> >> $ ./configure --enable-trace-backends=log,callback && make -j4
>> >>
>> >> This is still a clean interface that allows instrumentation code to be
>> >> kept separate from the trace event call sites.
>> >>
>> >> The instrumentation code gets compiled into QEMU, but that shouldn't be
>> >> a huge burden since QEMU's Makefiles only recompile changed source
>> >> files (only the first build is slow).
>>
>> > Is your proposal that my-instrumentation.c gets compiled into
>> > QEMU at this point? That doesn't seem like a great idea to
>> > me, because it means you can only use this tracing if you
>> > build QEMU yourself, and distros won't enable it and so
>> > lots of our users won't have convenient access to it.
>> > I'd much rather see a cleanly designed plugin interface
>> > (which we should be able to implement in a manner that doesn't
>> > let the plugin monkey patch arbitrary parts of QEMU beyond
>> > what can already be achieved via LD_PRELOAD).
>>
>> Just to be clear, what do you both mean by monkey-patching?
>>
>> * Accessing unintended symbols in QEMU from the library (and from there
>> modifying QEMU's behavior).
>> * QEMU using symbols on the library instead of its own just because they have
>> the same name.
>>
>> As I said, the former can be accomplished by compiling QEMU with
>> "-fvisibility=hidden".
>>
>> The latter is already achieved by using dlopen with RTLD_LOCAL (the default).
> Instrumentation functions are invoked in the same memory space as QEMU,
> so pointer arguments can be modified.
> Think of all the void *s arguments we trace. Instrumentation code can
> access those structs, hijack function pointers, etc. No symbols are
> required.
And why exactly is this a threat? Because it can be used to "extend" QEMU
without touching its sources? Is this a realistic threat? (it's a rather brittle
way to do it, so I'm not sure it's practical)
If that's the case, forcing instrumentation libraries to be exclusively GPL
seems like a solution to me, just as GPL protects QEMU itself.
Do we agree on that? Or did I miss something else?
As a side note, I find instrumentation to be most useful for guest code events,
which mostly contain non-pointer values (except for the CPUState*).
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-26 12:44 ` Lluís Vilanova
@ 2017-07-27 10:32 ` Stefan Hajnoczi
2017-07-27 10:40 ` Peter Maydell
0 siblings, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-07-27 10:32 UTC (permalink / raw)
To: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 4239 bytes --]
On Wed, Jul 26, 2017 at 03:44:39PM +0300, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
>
> > On Tue, Jul 25, 2017 at 06:11:43PM +0300, Lluís Vilanova wrote:
> >> Peter Maydell writes:
> >>
> >> > On 25 July 2017 at 14:19, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> >> >> Instead I suggest adding a trace backend generates calls to registered
> >> >> "callback" functions:
> >> >>
> >> >> $ cat >my-instrumentation.c
> >> >> #include "trace/control.h"
> >> >>
> >> >> static void my_cpu_in(unsigned int addr, char size, unsigned int val)
> >> >> {
> >> >> printf("my_cpu_in\n");
> >> >> }
> >> >>
> >> >> static void my_init(void)
> >> >> {
> >> >> trace_register_event_callback("cpu_in", my_cpu_in);
> >> >> trace_enable_events("cpu_in");
> >> >> }
> >> >> trace_init(my_init);
> >> >>
> >> >> $ ./configure --enable-trace-backends=log,callback && make -j4
> >> >>
> >> >> This is still a clean interface that allows instrumentation code to be
> >> >> kept separate from the trace event call sites.
> >> >>
> >> >> The instrumentation code gets compiled into QEMU, but that shouldn't be
> >> >> a huge burden since QEMU's Makefiles only recompile changed source
> >> >> files (only the first build is slow).
> >>
> >> > Is your proposal that my-instrumentation.c gets compiled into
> >> > QEMU at this point? That doesn't seem like a great idea to
> >> > me, because it means you can only use this tracing if you
> >> > build QEMU yourself, and distros won't enable it and so
> >> > lots of our users won't have convenient access to it.
> >> > I'd much rather see a cleanly designed plugin interface
> >> > (which we should be able to implement in a manner that doesn't
> >> > let the plugin monkey patch arbitrary parts of QEMU beyond
> >> > what can already be achieved via LD_PRELOAD).
> >>
> >> Just to be clear, what do you both mean by monkey-patching?
> >>
> >> * Accessing unintended symbols in QEMU from the library (and from there
> >> modifying QEMU's behavior).
> >> * QEMU using symbols on the library instead of its own just because they have
> >> the same name.
> >>
> >> As I said, the former can be accomplished by compiling QEMU with
> >> "-fvisibility=hidden".
> >>
> >> The latter is already achieved by using dlopen with RTLD_LOCAL (the default).
>
> > Instrumentation functions are invoked in the same memory space as QEMU,
> > so pointer arguments can be modified.
>
> > Think of all the void *s arguments we trace. Instrumentation code can
> > access those structs, hijack function pointers, etc. No symbols are
> > required.
>
> And why exactly is this a threat? Because it can be used to "extend" QEMU
> without touching its sources? Is this a realistic threat? (it's a rather brittle
> way to do it, so I'm not sure it's practical)
Unfortunately it is a problem. I recently came across a product that
was using LD_PRELOAD= to "integrate" with QEMU. People really abuse
these interfaces instead of integrating their features cleanly into
QEMU.
I see the use cases that Peter has been describing and am sure we can
come up with good solutions. What I care about is that it doesn't allow
loading a .so that connects to arbitrary trace events.
> If that's the case, forcing instrumentation libraries to be exclusively GPL
> seems like a solution to me, just as GPL protects QEMU itself.
>
> Do we agree on that? Or did I miss something else?
How the license applies depends on how user instrumentation code
interfaces with QEMU. We probably need to ask the QEMU project's
lawyers for confirmation. I don't think it's as simple as saying "the
interface will be protected by GPL".
Let's discuss the use cases and how instrumentation should work in
another subthread first. Then towards the end we can come back to GPL
again.
> As a side note, I find instrumentation to be most useful for guest code events,
> which mostly contain non-pointer values (except for the CPUState*).
That's great. I'll read the entire series to get a feel for what
instrumentation code needs to be able to do. I'm wondering if a stable
public API is possible that does not leave room for abuse.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 10:32 ` Stefan Hajnoczi
@ 2017-07-27 10:40 ` Peter Maydell
2017-07-28 13:42 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Peter Maydell @ 2017-07-27 10:40 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
On 27 July 2017 at 11:32, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Wed, Jul 26, 2017 at 03:44:39PM +0300, Lluís Vilanova wrote:
>> And why exactly is this a threat? Because it can be used to "extend" QEMU
>> without touching its sources? Is this a realistic threat? (it's a rather brittle
>> way to do it, so I'm not sure it's practical)
>
> Unfortunately it is a problem. I recently came across a product that
> was using LD_PRELOAD= to "integrate" with QEMU. People really abuse
> these interfaces instead of integrating their features cleanly into
> QEMU.
...if people who want to do this kind of thing already can and
do use LD_PRELOAD for it, I don't think we should worry too much
about trying to make the instrumentation plugin API bulletproof
against similar abuse.
> I see the use cases that Peter has been describing and am sure we can
> come up with good solutions. What I care about is that it doesn't allow
> loading a .so that connects to arbitrary trace events.
That said, I agree that we don't really need an arbitrary-trace-event
setup here, and we should probably design our API so that it isn't
handing the trace plugin hooks pointers into QEMU's internals.
We want an API that makes it easy for people to do things based on
changes of the guest binary's state (registers, insns, etc etc)
and which makes it hard for them to accidentally trip themselves up
(eg by prodding around in QEMU internal data structures).
This will have the secondary benefit that it's unlikely that future
changes to QEMU will break plugin code.
>> As a side note, I find instrumentation to be most useful for guest code events,
>> which mostly contain non-pointer values (except for the CPUState*).
For instance we definitely should not be passing a CPUState* to
any plugin function.
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-26 11:49 ` Peter Maydell
2017-07-26 12:26 ` Lluís Vilanova
@ 2017-07-27 10:43 ` Daniel P. Berrange
2017-07-27 10:54 ` Peter Maydell
1 sibling, 1 reply; 59+ messages in thread
From: Daniel P. Berrange @ 2017-07-27 10:43 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On Wed, Jul 26, 2017 at 12:49:00PM +0100, Peter Maydell wrote:
> On 26 July 2017 at 12:26, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Tue, Jul 25, 2017 at 02:30:06PM +0100, Peter Maydell wrote:
> >> Is your proposal that my-instrumentation.c gets compiled into
> >> QEMU at this point? That doesn't seem like a great idea to
> >> me, because it means you can only use this tracing if you
> >> build QEMU yourself, and distros won't enable it and so
> >> lots of our users won't have convenient access to it.
> >> I'd much rather see a cleanly designed plugin interface
> >> (which we should be able to implement in a manner that doesn't
> >> let the plugin monkey patch arbitrary parts of QEMU beyond
> >> what can already be achieved via LD_PRELOAD).
> >
> > Trace events are not a stable public API. They change between releases
> > and need to be interpreted together with the QEMU source code.
> >
> > It doesn't make sense to be doing instrumentation inside the QEMU
> > process but not want to compile QEMU.
> >
> > What is the use case?
>
> I want the continuous stream of people who come along
> and want to do interesting things (or even just straightforward
> things like "show me all the memory accesses") with tracing
> their guest binary to be able to do so without having to
> rebuild QEMU. We don't require people to rebuild QEMU
> themselves to attach gdb to it, and tracing of guest
> behaviour is a similar "I just want to introspect what
> my guest is doing" use case.
>
> I think this is distinct from use of the trace API
> for qemu-internal events as used in a lot of the C
> code.
>
> Some simple use cases:
> * a plugin which prints out instruction execution traces
> by hooking into "insn executed" and printing something
> helpful (you can extend this into producing traces
> in whatever standard format some other program wants
> to inhale, but at a minimum it would be something we
> could point people at that makes more sense than our
> current "only print insns at translate time" debug);
> it's also easy to add bells and whistles for "start
> tracing at time T" and so on
> * a plugin that make cachegrind-style guesses about
> cache usage by tracking memory accesses and simulating
> cache effects
> * a plugin that prints all the system calls the
> guest makes (by hooking into "insn executed" and
> printing info when the insn is an SVC)
> * trivial statistics like "count executed instructions
> and branches taken"
Maybe I'm missing something, but aren't all these things
already possible via either the statically defined tracepoints
QEMU exposes, or by placing dynamic tracepoints on arbitrary
code locations using dtrace/systemtap/lttng-ust.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 10:43 ` Daniel P. Berrange
@ 2017-07-27 10:54 ` Peter Maydell
2017-07-27 14:58 ` Lluís Vilanova
2017-07-27 15:21 ` Daniel P. Berrange
0 siblings, 2 replies; 59+ messages in thread
From: Peter Maydell @ 2017-07-27 10:54 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Stefan Hajnoczi, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On 27 July 2017 at 11:43, Daniel P. Berrange <berrange@redhat.com> wrote:
> Maybe I'm missing something, but aren't all these things
> already possible via either the statically defined tracepoints
> QEMU exposes, or by placing dynamic tracepoints on arbitrary
> code locations using dtrace/systemtap/lttng-ust.
Last time I looked we didn't have tracepoints on most of
the events you'd be interested in.
That said, yes, I was going to ask if we could do this via
leveraging the tracepoint infrastructure and whatever scripting
facilities it provides. Are there any good worked examples of
this sort of thing? Can you do it as an ordinary non-root user?
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 10:54 ` Peter Maydell
@ 2017-07-27 14:58 ` Lluís Vilanova
2017-07-27 15:21 ` Daniel P. Berrange
1 sibling, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-27 14:58 UTC (permalink / raw)
To: Peter Maydell
Cc: Daniel P. Berrange, Stefan Hajnoczi, Emilio G. Cota,
Stefan Hajnoczi, QEMU Developers
Peter Maydell writes:
> On 27 July 2017 at 11:43, Daniel P. Berrange <berrange@redhat.com> wrote:
>> Maybe I'm missing something, but aren't all these things
>> already possible via either the statically defined tracepoints
>> QEMU exposes, or by placing dynamic tracepoints on arbitrary
>> code locations using dtrace/systemtap/lttng-ust.
> Last time I looked we didn't have tracepoints on most of
> the events you'd be interested in.
> That said, yes, I was going to ask if we could do this via
> leveraging the tracepoint infrastructure and whatever scripting
> facilities it provides. Are there any good worked examples of
> this sort of thing? Can you do it as an ordinary non-root user?
The hypertrace series I just sent contains an example that uses dtrace to
trigger changes on the tracing state of events based on information from other
events [1].
AFAIK it requires root privileges, and it's much less efficient than loading the
instrumentation library proposed in this series (see that there's also an API to
control event tracing).
[1] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg08458.html
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 10:54 ` Peter Maydell
2017-07-27 14:58 ` Lluís Vilanova
@ 2017-07-27 15:21 ` Daniel P. Berrange
2017-07-27 15:33 ` Peter Maydell
2017-07-27 19:55 ` Lluís Vilanova
1 sibling, 2 replies; 59+ messages in thread
From: Daniel P. Berrange @ 2017-07-27 15:21 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
> On 27 July 2017 at 11:43, Daniel P. Berrange <berrange@redhat.com> wrote:
> > Maybe I'm missing something, but aren't all these things
> > already possible via either the statically defined tracepoints
> > QEMU exposes, or by placing dynamic tracepoints on arbitrary
> > code locations using dtrace/systemtap/lttng-ust.
>
> Last time I looked we didn't have tracepoints on most of
> the events you'd be interested in.
>
> That said, yes, I was going to ask if we could do this via
> leveraging the tracepoint infrastructure and whatever scripting
> facilities it provides. Are there any good worked examples of
> this sort of thing? Can you do it as an ordinary non-root user?
Do you have a particular thing you'd like to see an example of ?
To dynamically probe a function which doesn't have a tracepoint
defined you can do:
probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
printf("syscall stasrt\n")
}
but getting access to the function args is not as easy as with
pre-defined tracepoints.
You can't typically run this as root, however, I don't think that's a
huge issue, because most QEMU deployments are not running as your own
user account anyway, so you can't directly interact with them no
matter what.
If the goal is to be easy to instrument without havig to rebuild
QEMU, then I think using one of the existing trace backends is
the best viable option, as those are already enabled by distros.
I find it very unlikely that Fedora/RHEL would ever enable a
trace backend that lets you load arbitrary code into the QEMU
process, so you'd be back to having to rebuild QEMU again even
with that approach.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 15:21 ` Daniel P. Berrange
@ 2017-07-27 15:33 ` Peter Maydell
2017-07-27 15:45 ` Daniel P. Berrange
2017-07-27 19:55 ` Lluís Vilanova
1 sibling, 1 reply; 59+ messages in thread
From: Peter Maydell @ 2017-07-27 15:33 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Stefan Hajnoczi, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On 27 July 2017 at 16:21, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
>> That said, yes, I was going to ask if we could do this via
>> leveraging the tracepoint infrastructure and whatever scripting
>> facilities it provides. Are there any good worked examples of
>> this sort of thing? Can you do it as an ordinary non-root user?
>
> Do you have a particular thing you'd like to see an example of ?
>
> To dynamically probe a function which doesn't have a tracepoint
> defined you can do:
>
> probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
> printf("syscall stasrt\n")
> }
>
> but getting access to the function args is not as easy as with
> pre-defined tracepoints.
How do I go about actually running that script? What I
have in mind by "worked example" is something like a blog
post that says "ok, here's a problem, we want to find out
what QEMU is doing in situation X, here's how you do this
with $TRACING_THINGY" and generally steps you through how
it works assuming you know nothing at all about whatever
the tracing facility you're using is.
> You can't typically run this as root,
Do you mean "non-root" ?
> however, I don't think that's a
> huge issue, because most QEMU deployments are not running as your own
> user account anyway, so you can't directly interact with them no
> matter what.
It is important, because almost all uses of TCG QEMU are
running it from the command line as non-root normal users,
especially if they're trying to debug what's going on with a
guest binary. So any tracing solution for this kind of usecase
must work without requiring root access, I think.
I think the users for this are pretty much completely
distinct from anybody who would use the term "deployment"
for their usage of QEMU :-)
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 15:33 ` Peter Maydell
@ 2017-07-27 15:45 ` Daniel P. Berrange
2017-07-28 13:34 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Daniel P. Berrange @ 2017-07-27 15:45 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On Thu, Jul 27, 2017 at 04:33:01PM +0100, Peter Maydell wrote:
> On 27 July 2017 at 16:21, Daniel P. Berrange <berrange@redhat.com> wrote:
> > On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
> >> That said, yes, I was going to ask if we could do this via
> >> leveraging the tracepoint infrastructure and whatever scripting
> >> facilities it provides. Are there any good worked examples of
> >> this sort of thing? Can you do it as an ordinary non-root user?
> >
> > Do you have a particular thing you'd like to see an example of ?
> >
> > To dynamically probe a function which doesn't have a tracepoint
> > defined you can do:
> >
> > probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
> > printf("syscall stasrt\n")
> > }
> >
> > but getting access to the function args is not as easy as with
> > pre-defined tracepoints.
>
> How do I go about actually running that script? What I
> have in mind by "worked example" is something like a blog
> post that says "ok, here's a problem, we want to find out
> what QEMU is doing in situation X, here's how you do this
> with $TRACING_THINGY" and generally steps you through how
> it works assuming you know nothing at all about whatever
> the tracing facility you're using is.
Ok, so something like this example that I wrote for libvirt a
while back then
https://www.berrange.com/posts/2011/11/30/watching-the-libvirt-rpc-protocol-using-systemtap/
> > You can't typically run this as root,
>
> Do you mean "non-root" ?
Sigh, yes, of course.
> > however, I don't think that's a
> > huge issue, because most QEMU deployments are not running as your own
> > user account anyway, so you can't directly interact with them no
> > matter what.
>
> It is important, because almost all uses of TCG QEMU are
> running it from the command line as non-root normal users,
> especially if they're trying to debug what's going on with a
> guest binary. So any tracing solution for this kind of usecase
> must work without requiring root access, I think.
None of the Linux integrated tracing tools allow direct non-root access
afaik. systemtap has ability to launch probes as non-root, via a privileged
daemon, but it is restricted to probe scripts that the administrator has
pre-defined.
That pretty much leaves re-building QEMU, LD_PRELOADS, or something
ptrace(), or qemu's built-in simpletrace feature, as the remaining
options. We have a scripts/simpletrace.py that lets you load a
trace file into python and process it, but as written that's aimed
as post-processing a tracefile you've previously collected.
It would be desirable to write a more advanced simpletrace python
module that could collect & process the trace data live, and also
interact with the qemu monitor to change what events are enabled
dynamically. Basically we'd need a way for the simpletrace backend
to output its data to a fifo, instead of creating an file on disk,
then you could dynanically consume it.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 15:21 ` Daniel P. Berrange
2017-07-27 15:33 ` Peter Maydell
@ 2017-07-27 19:55 ` Lluís Vilanova
1 sibling, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-27 19:55 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota, Stefan Hajnoczi,
QEMU Developers
Daniel P Berrange writes:
> On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
>> On 27 July 2017 at 11:43, Daniel P. Berrange <berrange@redhat.com> wrote:
>> > Maybe I'm missing something, but aren't all these things
>> > already possible via either the statically defined tracepoints
>> > QEMU exposes, or by placing dynamic tracepoints on arbitrary
>> > code locations using dtrace/systemtap/lttng-ust.
>>
>> Last time I looked we didn't have tracepoints on most of
>> the events you'd be interested in.
>>
>> That said, yes, I was going to ask if we could do this via
>> leveraging the tracepoint infrastructure and whatever scripting
>> facilities it provides. Are there any good worked examples of
>> this sort of thing? Can you do it as an ordinary non-root user?
> Do you have a particular thing you'd like to see an example of ?
> To dynamically probe a function which doesn't have a tracepoint
> defined you can do:
> probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
> printf("syscall stasrt\n")
> }
> but getting access to the function args is not as easy as with
> pre-defined tracepoints.
> You can't typically run this as root, however, I don't think that's a
> huge issue, because most QEMU deployments are not running as your own
> user account anyway, so you can't directly interact with them no
> matter what.
> If the goal is to be easy to instrument without havig to rebuild
> QEMU, then I think using one of the existing trace backends is
> the best viable option, as those are already enabled by distros.
> I find it very unlikely that Fedora/RHEL would ever enable a
> trace backend that lets you load arbitrary code into the QEMU
> process, so you'd be back to having to rebuild QEMU again even
> with that approach.
My idea is to use the guest code tracing events to perform some complex analysis
of the executed guest code. E.g., tracing executed instructions to a file to
calculate SimPoints [1], analysing guest instructions and memory accesses to
track information flow or feeding that info to a processor simulator. In fact,
I've used my non-upstream version of QEMU+instrumentation to do the first two
and a few others.
My problem with dtrace is efficiency of the instrumentation code, and the lack
of APIs to perform a few more complex analysis like peek/poke on guest memory,
registers or even control event tracing states. I'm not proposing to have such
APIs on QEMU upstream (unless there's interest), but they are easy enough for me
to maintain for my particular needs if we're talking about the instrumentation
support in this series.
So Stefan, could you elaborate on your proposal of having to build the
instrumentation code into QEMU? I would prefer a dynamic library if we can craft
it in a way that ensures the proper license restrictions, but a "static library"
could work just as well for me (I initially had a branch with both solutions,
but it got too tedious to rebase).
Thanks,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 15:45 ` Daniel P. Berrange
@ 2017-07-28 13:34 ` Stefan Hajnoczi
2017-07-28 13:41 ` Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-07-28 13:34 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]
On Thu, Jul 27, 2017 at 04:45:35PM +0100, Daniel P. Berrange wrote:
> On Thu, Jul 27, 2017 at 04:33:01PM +0100, Peter Maydell wrote:
> > On 27 July 2017 at 16:21, Daniel P. Berrange <berrange@redhat.com> wrote:
> > > On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
> > >> That said, yes, I was going to ask if we could do this via
> > >> leveraging the tracepoint infrastructure and whatever scripting
> > >> facilities it provides. Are there any good worked examples of
> > >> this sort of thing? Can you do it as an ordinary non-root user?
> > >
> > > Do you have a particular thing you'd like to see an example of ?
> > >
> > > To dynamically probe a function which doesn't have a tracepoint
> > > defined you can do:
> > >
> > > probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
> > > printf("syscall stasrt\n")
> > > }
> > >
> > > but getting access to the function args is not as easy as with
> > > pre-defined tracepoints.
> >
> > How do I go about actually running that script? What I
> > have in mind by "worked example" is something like a blog
> > post that says "ok, here's a problem, we want to find out
> > what QEMU is doing in situation X, here's how you do this
> > with $TRACING_THINGY" and generally steps you through how
> > it works assuming you know nothing at all about whatever
> > the tracing facility you're using is.
>
> Ok, so something like this example that I wrote for libvirt a
> while back then
>
> https://www.berrange.com/posts/2011/11/30/watching-the-libvirt-rpc-protocol-using-systemtap/
>
>
> > > You can't typically run this as root,
> >
> > Do you mean "non-root" ?
>
> Sigh, yes, of course.
>
> > > however, I don't think that's a
> > > huge issue, because most QEMU deployments are not running as your own
> > > user account anyway, so you can't directly interact with them no
> > > matter what.
> >
> > It is important, because almost all uses of TCG QEMU are
> > running it from the command line as non-root normal users,
> > especially if they're trying to debug what's going on with a
> > guest binary. So any tracing solution for this kind of usecase
> > must work without requiring root access, I think.
>
> None of the Linux integrated tracing tools allow direct non-root access
> afaik. systemtap has ability to launch probes as non-root, via a privileged
> daemon, but it is restricted to probe scripts that the administrator has
> pre-defined.
One exception is gdb's static userspace probes support. If you can run
gdb on QEMU then you can trace the same events as SystemTap. I have
never tried this GDB feature:
https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
It should work out of the box if your distro builds QEMU with the
'dtrace' backend enabled.
> That pretty much leaves re-building QEMU, LD_PRELOADS, or something
> ptrace(), or qemu's built-in simpletrace feature, as the remaining
> options. We have a scripts/simpletrace.py that lets you load a
> trace file into python and process it, but as written that's aimed
> as post-processing a tracefile you've previously collected.
>
> It would be desirable to write a more advanced simpletrace python
> module that could collect & process the trace data live, and also
> interact with the qemu monitor to change what events are enabled
> dynamically. Basically we'd need a way for the simpletrace backend
> to output its data to a fifo, instead of creating an file on disk,
> then you could dynanically consume it.
That would be interesting, I know Alex Bennee has wrangled with large
(~10 GB?) simpletrace files and it's not a pleasant experience :).
Lluís/Peter: What are the requirements for instrumentation code
interacting with the running QEMU instance? simpletrace is
asynchronous, meaning it does not wait for anyone handle the trace event
before continuing execution, and is therefore not suitable for
SystemTap-style scripts that can interact with the program while
handling a trace event.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 13:34 ` Stefan Hajnoczi
@ 2017-07-28 13:41 ` Peter Maydell
2017-07-28 14:06 ` Daniel P. Berrange
2017-07-28 13:52 ` Daniel P. Berrange
2017-07-28 15:10 ` Lluís Vilanova
2 siblings, 1 reply; 59+ messages in thread
From: Peter Maydell @ 2017-07-28 13:41 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Daniel P. Berrange, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On 28 July 2017 at 14:34, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Lluís/Peter: What are the requirements for instrumentation code
> interacting with the running QEMU instance? simpletrace is
> asynchronous, meaning it does not wait for anyone handle the trace event
> before continuing execution, and is therefore not suitable for
> SystemTap-style scripts that can interact with the program while
> handling a trace event.
I think you'd probably want synchronous -- it's pretty helpful
to be able to say "register a trace event hook that doesn't
fire very often, and use that to get to the region of
execution that's of interest to you, then enable more hooks
to get more detail at that point". (For instance, "wait til
we've executed 5,000,000 instructions, then turn on the
tracing of all instruction execution, register modification
and memory accesses".)
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-27 10:40 ` Peter Maydell
@ 2017-07-28 13:42 ` Stefan Hajnoczi
2017-07-28 16:21 ` Lluís Vilanova
0 siblings, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-07-28 13:42 UTC (permalink / raw)
To: Peter Maydell; +Cc: Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]
On Thu, Jul 27, 2017 at 11:40:17AM +0100, Peter Maydell wrote:
> On 27 July 2017 at 11:32, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Wed, Jul 26, 2017 at 03:44:39PM +0300, Lluís Vilanova wrote:
> >> And why exactly is this a threat? Because it can be used to "extend" QEMU
> >> without touching its sources? Is this a realistic threat? (it's a rather brittle
> >> way to do it, so I'm not sure it's practical)
> >
> > Unfortunately it is a problem. I recently came across a product that
> > was using LD_PRELOAD= to "integrate" with QEMU. People really abuse
> > these interfaces instead of integrating their features cleanly into
> > QEMU.
>
> ...if people who want to do this kind of thing already can and
> do use LD_PRELOAD for it, I don't think we should worry too much
> about trying to make the instrumentation plugin API bulletproof
> against similar abuse.
>
> > I see the use cases that Peter has been describing and am sure we can
> > come up with good solutions. What I care about is that it doesn't allow
> > loading a .so that connects to arbitrary trace events.
>
> That said, I agree that we don't really need an arbitrary-trace-event
> setup here, and we should probably design our API so that it isn't
> handing the trace plugin hooks pointers into QEMU's internals.
> We want an API that makes it easy for people to do things based on
> changes of the guest binary's state (registers, insns, etc etc)
> and which makes it hard for them to accidentally trip themselves up
> (eg by prodding around in QEMU internal data structures).
> This will have the secondary benefit that it's unlikely that future
> changes to QEMU will break plugin code.
>
> >> As a side note, I find instrumentation to be most useful for guest code events,
> >> which mostly contain non-pointer values (except for the CPUState*).
>
> For instance we definitely should not be passing a CPUState* to
> any plugin function.
The gdbstub protocol has relevant features for accessing guest memory,
registers, etc. Perhaps a set of QEMU-specific events can be added
(e.g. tb generated) so it's possible to instrument and control the
guest from an instrumentation program (written in any language).
Perhaps there is a fundamental reason why this isn't possible due to the
protocol design, because using gdbstub halts all vcpus, etc. I don't
know.
Do you think this is an interesting direction? It definitely seems like
a powerful approach though performance would be less than running native
code inside the QEMU process.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 13:34 ` Stefan Hajnoczi
2017-07-28 13:41 ` Peter Maydell
@ 2017-07-28 13:52 ` Daniel P. Berrange
2017-07-28 16:14 ` Lluís Vilanova
2017-07-28 15:10 ` Lluís Vilanova
2 siblings, 1 reply; 59+ messages in thread
From: Daniel P. Berrange @ 2017-07-28 13:52 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On Fri, Jul 28, 2017 at 02:34:30PM +0100, Stefan Hajnoczi wrote:
> On Thu, Jul 27, 2017 at 04:45:35PM +0100, Daniel P. Berrange wrote:
> > On Thu, Jul 27, 2017 at 04:33:01PM +0100, Peter Maydell wrote:
> > > On 27 July 2017 at 16:21, Daniel P. Berrange <berrange@redhat.com> wrote:
> > > > On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
> > > >> That said, yes, I was going to ask if we could do this via
> > > >> leveraging the tracepoint infrastructure and whatever scripting
> > > >> facilities it provides. Are there any good worked examples of
> > > >> this sort of thing? Can you do it as an ordinary non-root user?
> > > >
> > > > Do you have a particular thing you'd like to see an example of ?
> > > >
> > > > To dynamically probe a function which doesn't have a tracepoint
> > > > defined you can do:
> > > >
> > > > probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
> > > > printf("syscall stasrt\n")
> > > > }
> > > >
> > > > but getting access to the function args is not as easy as with
> > > > pre-defined tracepoints.
> > >
> > > How do I go about actually running that script? What I
> > > have in mind by "worked example" is something like a blog
> > > post that says "ok, here's a problem, we want to find out
> > > what QEMU is doing in situation X, here's how you do this
> > > with $TRACING_THINGY" and generally steps you through how
> > > it works assuming you know nothing at all about whatever
> > > the tracing facility you're using is.
> >
> > Ok, so something like this example that I wrote for libvirt a
> > while back then
> >
> > https://www.berrange.com/posts/2011/11/30/watching-the-libvirt-rpc-protocol-using-systemtap/
> >
> >
> > > > You can't typically run this as root,
> > >
> > > Do you mean "non-root" ?
> >
> > Sigh, yes, of course.
> >
> > > > however, I don't think that's a
> > > > huge issue, because most QEMU deployments are not running as your own
> > > > user account anyway, so you can't directly interact with them no
> > > > matter what.
> > >
> > > It is important, because almost all uses of TCG QEMU are
> > > running it from the command line as non-root normal users,
> > > especially if they're trying to debug what's going on with a
> > > guest binary. So any tracing solution for this kind of usecase
> > > must work without requiring root access, I think.
> >
> > None of the Linux integrated tracing tools allow direct non-root access
> > afaik. systemtap has ability to launch probes as non-root, via a privileged
> > daemon, but it is restricted to probe scripts that the administrator has
> > pre-defined.
>
> One exception is gdb's static userspace probes support. If you can run
> gdb on QEMU then you can trace the same events as SystemTap. I have
> never tried this GDB feature:
>
> https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
>
> It should work out of the box if your distro builds QEMU with the
> 'dtrace' backend enabled.
Wow, that's great to learn about. It does indeed work !
If you knew alot about ptrace() you could probably build something
that use ptrace() and these probe points to call your dynamic
instrumentation code with reasonable low overheads.
> > That pretty much leaves re-building QEMU, LD_PRELOADS, or something
> > ptrace(), or qemu's built-in simpletrace feature, as the remaining
> > options. We have a scripts/simpletrace.py that lets you load a
> > trace file into python and process it, but as written that's aimed
> > as post-processing a tracefile you've previously collected.
> >
> > It would be desirable to write a more advanced simpletrace python
> > module that could collect & process the trace data live, and also
> > interact with the qemu monitor to change what events are enabled
> > dynamically. Basically we'd need a way for the simpletrace backend
> > to output its data to a fifo, instead of creating an file on disk,
> > then you could dynanically consume it.
>
> That would be interesting, I know Alex Bennee has wrangled with large
> (~10 GB?) simpletrace files and it's not a pleasant experience :).
>
> Lluís/Peter: What are the requirements for instrumentation code
> interacting with the running QEMU instance? simpletrace is
> asynchronous, meaning it does not wait for anyone handle the trace event
> before continuing execution, and is therefore not suitable for
> SystemTap-style scripts that can interact with the program while
> handling a trace event.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 13:41 ` Peter Maydell
@ 2017-07-28 14:06 ` Daniel P. Berrange
2017-07-28 16:05 ` Lluís Vilanova
0 siblings, 1 reply; 59+ messages in thread
From: Daniel P. Berrange @ 2017-07-28 14:06 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Stefan Hajnoczi, Emilio G. Cota,
Lluís Vilanova, QEMU Developers
On Fri, Jul 28, 2017 at 02:41:19PM +0100, Peter Maydell wrote:
> On 28 July 2017 at 14:34, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > Lluís/Peter: What are the requirements for instrumentation code
> > interacting with the running QEMU instance? simpletrace is
> > asynchronous, meaning it does not wait for anyone handle the trace event
> > before continuing execution, and is therefore not suitable for
> > SystemTap-style scripts that can interact with the program while
> > handling a trace event.
>
> I think you'd probably want synchronous -- it's pretty helpful
> to be able to say "register a trace event hook that doesn't
> fire very often, and use that to get to the region of
> execution that's of interest to you, then enable more hooks
> to get more detail at that point". (For instance, "wait til
> we've executed 5,000,000 instructions, then turn on the
> tracing of all instruction execution, register modification
> and memory accesses".)
Currently simpletrace probes have a fixed action when they are enabled,
namely to print state to the trace log file. Perhaps we can make the
action more flexible, if we create a more formal protocol for simpletrace
to let it talk over a UNIX socket. By default it could send probe data
asynchronously as now, but you could mark probes such that they require
a synchronous ACK, thus pausing execution until that ACK is received
from the instrumenting program.
For that to be useful, we would need to have allow probes to be turned
on/off via this trace socket, since the normal HMP/QMP monitor execution
would be blocked while this probe is running.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 13:34 ` Stefan Hajnoczi
2017-07-28 13:41 ` Peter Maydell
2017-07-28 13:52 ` Daniel P. Berrange
@ 2017-07-28 15:10 ` Lluís Vilanova
2 siblings, 0 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-28 15:10 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Daniel P. Berrange, QEMU Developers, Peter Maydell,
Emilio G. Cota, Stefan Hajnoczi
Stefan Hajnoczi writes:
> On Thu, Jul 27, 2017 at 04:45:35PM +0100, Daniel P. Berrange wrote:
>> On Thu, Jul 27, 2017 at 04:33:01PM +0100, Peter Maydell wrote:
>> > On 27 July 2017 at 16:21, Daniel P. Berrange <berrange@redhat.com> wrote:
>> > > On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
>> > >> That said, yes, I was going to ask if we could do this via
>> > >> leveraging the tracepoint infrastructure and whatever scripting
>> > >> facilities it provides. Are there any good worked examples of
>> > >> this sort of thing? Can you do it as an ordinary non-root user?
>> > >
>> > > Do you have a particular thing you'd like to see an example of ?
>> > >
>> > > To dynamically probe a function which doesn't have a tracepoint
>> > > defined you can do:
>> > >
>> > > probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
>> > > printf("syscall stasrt\n")
>> > > }
>> > >
>> > > but getting access to the function args is not as easy as with
>> > > pre-defined tracepoints.
>> >
>> > How do I go about actually running that script? What I
>> > have in mind by "worked example" is something like a blog
>> > post that says "ok, here's a problem, we want to find out
>> > what QEMU is doing in situation X, here's how you do this
>> > with $TRACING_THINGY" and generally steps you through how
>> > it works assuming you know nothing at all about whatever
>> > the tracing facility you're using is.
>>
>> Ok, so something like this example that I wrote for libvirt a
>> while back then
>>
>> https://www.berrange.com/posts/2011/11/30/watching-the-libvirt-rpc-protocol-using-systemtap/
>>
>>
>> > > You can't typically run this as root,
>> >
>> > Do you mean "non-root" ?
>>
>> Sigh, yes, of course.
>>
>> > > however, I don't think that's a
>> > > huge issue, because most QEMU deployments are not running as your own
>> > > user account anyway, so you can't directly interact with them no
>> > > matter what.
>> >
>> > It is important, because almost all uses of TCG QEMU are
>> > running it from the command line as non-root normal users,
>> > especially if they're trying to debug what's going on with a
>> > guest binary. So any tracing solution for this kind of usecase
>> > must work without requiring root access, I think.
>>
>> None of the Linux integrated tracing tools allow direct non-root access
>> afaik. systemtap has ability to launch probes as non-root, via a privileged
>> daemon, but it is restricted to probe scripts that the administrator has
>> pre-defined.
> One exception is gdb's static userspace probes support. If you can run
> gdb on QEMU then you can trace the same events as SystemTap. I have
> never tried this GDB feature:
> https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
> It should work out of the box if your distro builds QEMU with the
> 'dtrace' backend enabled.
I tried it once long time ago and didn't work for me. Maybe I just missed
something.
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 14:06 ` Daniel P. Berrange
@ 2017-07-28 16:05 ` Lluís Vilanova
2017-08-01 13:48 ` Stefan Hajnoczi
2017-08-26 0:02 ` Emilio G. Cota
0 siblings, 2 replies; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-28 16:05 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota, Stefan Hajnoczi,
QEMU Developers
Daniel P Berrange writes:
> On Fri, Jul 28, 2017 at 02:41:19PM +0100, Peter Maydell wrote:
>> On 28 July 2017 at 14:34, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> > Lluís/Peter: What are the requirements for instrumentation code
>> > interacting with the running QEMU instance? simpletrace is
>> > asynchronous, meaning it does not wait for anyone handle the trace event
>> > before continuing execution, and is therefore not suitable for
>> > SystemTap-style scripts that can interact with the program while
>> > handling a trace event.
>>
>> I think you'd probably want synchronous -- it's pretty helpful
>> to be able to say "register a trace event hook that doesn't
>> fire very often, and use that to get to the region of
>> execution that's of interest to you, then enable more hooks
>> to get more detail at that point". (For instance, "wait til
>> we've executed 5,000,000 instructions, then turn on the
>> tracing of all instruction execution, register modification
>> and memory accesses".)
> Currently simpletrace probes have a fixed action when they are enabled,
> namely to print state to the trace log file. Perhaps we can make the
> action more flexible, if we create a more formal protocol for simpletrace
> to let it talk over a UNIX socket. By default it could send probe data
> asynchronously as now, but you could mark probes such that they require
> a synchronous ACK, thus pausing execution until that ACK is received
> from the instrumenting program.
> For that to be useful, we would need to have allow probes to be turned
> on/off via this trace socket, since the normal HMP/QMP monitor execution
> would be blocked while this probe is running.
This sounds like much more work to me for much lower performance (copy through
fifo/socket & process synchronization). Especially if many interesting events
become "synchronous". Also, with MTTCG using fifos becomes more complex on both
sides (select on the client & synchronization of events between streams on both
sides).
As a performance example, I've used the approach of this series to perform some
non-trivial instrumentation and I can get performance on par with PIN (a
state-of-the-art tool from intel) on most SPEC benchmarks.
Honestly, I would try to see if there's a GPL-preserving way to allow native
instrumentation libraries to run inside QEMU (either dynamic or static, that's
less important to me).
As for the (minimum) requirements I've collected:
* Peek at register values and guest memory.
* Enumerate guest cpus.
* Control when events are raised to minimize overheads (e.g., avoid generating
TCG code to trace a guest code event we don't care about just now).
* When a guest code event is raised at translation time, let instrumentor decide
if it needs to be raised at execution time too (e.g., might be decided on
instruction opcode)
* Minimum overhead when instrumenting (I think the proposed fifo/socket approach
does not meet this; point 2 helps a lot here, which is what the current
tracing code does)
* Let instrumentor know when TBs are being freed (i.e., to free internal per-TB
structures; I have a patch queued adding this event).
Nice to have:
* Avoid recompilation for each new instrumentation logic.
* Poke at register values and guest memory.
* [synchronous for sure] Let user annotate guest programs to raise additional
events with guest-specified information (the hypertrace series I sent).
* [synchronous for sure] Make guest breakpoints/watchpoints raise an event (for
when we cannot annotate the guest code; I have some patches).
* Trigger QEMU's event tracing routine when the instrumentation code
decides. This is what this series does now, as then lazy instrumentors don't
need to write their own tracing-to-disk code. Otherwise, per-event tracing
states would need to be independent of instrumentation states (but the logic
is exactly the same).
* Let instrumentor define additional arguments to execution-time events during
translation-time (e.g., to quickly index into an instrumentor struct when the
execution-time event comes that references info collected during translation).
* Attach an instrumentor-specified value to each guest CPU (e.g., pointing to
the instrumentor's cpu state). Might be less necessary if the point above is
met (if we only look at overall performance).
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 13:52 ` Daniel P. Berrange
@ 2017-07-28 16:14 ` Lluís Vilanova
2017-08-01 13:13 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-28 16:14 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Stefan Hajnoczi, QEMU Developers, Peter Maydell, Emilio G. Cota,
Stefan Hajnoczi
Daniel P Berrange writes:
> On Fri, Jul 28, 2017 at 02:34:30PM +0100, Stefan Hajnoczi wrote:
>> On Thu, Jul 27, 2017 at 04:45:35PM +0100, Daniel P. Berrange wrote:
>> > On Thu, Jul 27, 2017 at 04:33:01PM +0100, Peter Maydell wrote:
>> > > On 27 July 2017 at 16:21, Daniel P. Berrange <berrange@redhat.com> wrote:
>> > > > On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
>> > > >> That said, yes, I was going to ask if we could do this via
>> > > >> leveraging the tracepoint infrastructure and whatever scripting
>> > > >> facilities it provides. Are there any good worked examples of
>> > > >> this sort of thing? Can you do it as an ordinary non-root user?
>> > > >
>> > > > Do you have a particular thing you'd like to see an example of ?
>> > > >
>> > > > To dynamically probe a function which doesn't have a tracepoint
>> > > > defined you can do:
>> > > >
>> > > > probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
>> > > > printf("syscall stasrt\n")
>> > > > }
>> > > >
>> > > > but getting access to the function args is not as easy as with
>> > > > pre-defined tracepoints.
>> > >
>> > > How do I go about actually running that script? What I
>> > > have in mind by "worked example" is something like a blog
>> > > post that says "ok, here's a problem, we want to find out
>> > > what QEMU is doing in situation X, here's how you do this
>> > > with $TRACING_THINGY" and generally steps you through how
>> > > it works assuming you know nothing at all about whatever
>> > > the tracing facility you're using is.
>> >
>> > Ok, so something like this example that I wrote for libvirt a
>> > while back then
>> >
>> > https://www.berrange.com/posts/2011/11/30/watching-the-libvirt-rpc-protocol-using-systemtap/
>> >
>> >
>> > > > You can't typically run this as root,
>> > >
>> > > Do you mean "non-root" ?
>> >
>> > Sigh, yes, of course.
>> >
>> > > > however, I don't think that's a
>> > > > huge issue, because most QEMU deployments are not running as your own
>> > > > user account anyway, so you can't directly interact with them no
>> > > > matter what.
>> > >
>> > > It is important, because almost all uses of TCG QEMU are
>> > > running it from the command line as non-root normal users,
>> > > especially if they're trying to debug what's going on with a
>> > > guest binary. So any tracing solution for this kind of usecase
>> > > must work without requiring root access, I think.
>> >
>> > None of the Linux integrated tracing tools allow direct non-root access
>> > afaik. systemtap has ability to launch probes as non-root, via a privileged
>> > daemon, but it is restricted to probe scripts that the administrator has
>> > pre-defined.
>>
>> One exception is gdb's static userspace probes support. If you can run
>> gdb on QEMU then you can trace the same events as SystemTap. I have
>> never tried this GDB feature:
>>
>> https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
>>
>> It should work out of the box if your distro builds QEMU with the
>> 'dtrace' backend enabled.
> Wow, that's great to learn about. It does indeed work !
> If you knew alot about ptrace() you could probably build something
> that use ptrace() and these probe points to call your dynamic
> instrumentation code with reasonable low overheads.
I don't think so. Ptrace traps into the kernel and stops the process while a
separate process decides what to do. That's between 3 and 4 orders of magnitude
slower than calling an instrumentor function.
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 13:42 ` Stefan Hajnoczi
@ 2017-07-28 16:21 ` Lluís Vilanova
2017-08-02 11:04 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-07-28 16:21 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
Stefan Hajnoczi writes:
> On Thu, Jul 27, 2017 at 11:40:17AM +0100, Peter Maydell wrote:
>> On 27 July 2017 at 11:32, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> > On Wed, Jul 26, 2017 at 03:44:39PM +0300, Lluís Vilanova wrote:
>> >> And why exactly is this a threat? Because it can be used to "extend" QEMU
>> >> without touching its sources? Is this a realistic threat? (it's a rather brittle
>> >> way to do it, so I'm not sure it's practical)
>> >
>> > Unfortunately it is a problem. I recently came across a product that
>> > was using LD_PRELOAD= to "integrate" with QEMU. People really abuse
>> > these interfaces instead of integrating their features cleanly into
>> > QEMU.
>>
>> ...if people who want to do this kind of thing already can and
>> do use LD_PRELOAD for it, I don't think we should worry too much
>> about trying to make the instrumentation plugin API bulletproof
>> against similar abuse.
>>
>> > I see the use cases that Peter has been describing and am sure we can
>> > come up with good solutions. What I care about is that it doesn't allow
>> > loading a .so that connects to arbitrary trace events.
>>
>> That said, I agree that we don't really need an arbitrary-trace-event
>> setup here, and we should probably design our API so that it isn't
>> handing the trace plugin hooks pointers into QEMU's internals.
>> We want an API that makes it easy for people to do things based on
>> changes of the guest binary's state (registers, insns, etc etc)
>> and which makes it hard for them to accidentally trip themselves up
>> (eg by prodding around in QEMU internal data structures).
>> This will have the secondary benefit that it's unlikely that future
>> changes to QEMU will break plugin code.
>>
>> >> As a side note, I find instrumentation to be most useful for guest code events,
>> >> which mostly contain non-pointer values (except for the CPUState*).
>>
>> For instance we definitely should not be passing a CPUState* to
>> any plugin function.
> The gdbstub protocol has relevant features for accessing guest memory,
> registers, etc. Perhaps a set of QEMU-specific events can be added
> (e.g. tb generated) so it's possible to instrument and control the
> guest from an instrumentation program (written in any language).
> Perhaps there is a fundamental reason why this isn't possible due to the
> protocol design, because using gdbstub halts all vcpus, etc. I don't
> know.
> Do you think this is an interesting direction? It definitely seems like
> a powerful approach though performance would be less than running native
> code inside the QEMU process.
That's the same approach someone else dubbed as using a fifo with "synchronous"
events, right? I have some measurements on this using a pipe, and overheads are
1000x to 2300x for each communication event (compared to a function call, and
depending on whether each process/thread is pinned to the same or different
CPU).
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 16:14 ` Lluís Vilanova
@ 2017-08-01 13:13 ` Stefan Hajnoczi
0 siblings, 0 replies; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-08-01 13:13 UTC (permalink / raw)
To: Daniel P. Berrange, Stefan Hajnoczi, QEMU Developers,
Peter Maydell, Emilio G. Cota
[-- Attachment #1: Type: text/plain, Size: 4457 bytes --]
On Fri, Jul 28, 2017 at 07:14:33PM +0300, Lluís Vilanova wrote:
> Daniel P Berrange writes:
>
> > On Fri, Jul 28, 2017 at 02:34:30PM +0100, Stefan Hajnoczi wrote:
> >> On Thu, Jul 27, 2017 at 04:45:35PM +0100, Daniel P. Berrange wrote:
> >> > On Thu, Jul 27, 2017 at 04:33:01PM +0100, Peter Maydell wrote:
> >> > > On 27 July 2017 at 16:21, Daniel P. Berrange <berrange@redhat.com> wrote:
> >> > > > On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
> >> > > >> That said, yes, I was going to ask if we could do this via
> >> > > >> leveraging the tracepoint infrastructure and whatever scripting
> >> > > >> facilities it provides. Are there any good worked examples of
> >> > > >> this sort of thing? Can you do it as an ordinary non-root user?
> >> > > >
> >> > > > Do you have a particular thing you'd like to see an example of ?
> >> > > >
> >> > > > To dynamically probe a function which doesn't have a tracepoint
> >> > > > defined you can do:
> >> > > >
> >> > > > probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
> >> > > > printf("syscall stasrt\n")
> >> > > > }
> >> > > >
> >> > > > but getting access to the function args is not as easy as with
> >> > > > pre-defined tracepoints.
> >> > >
> >> > > How do I go about actually running that script? What I
> >> > > have in mind by "worked example" is something like a blog
> >> > > post that says "ok, here's a problem, we want to find out
> >> > > what QEMU is doing in situation X, here's how you do this
> >> > > with $TRACING_THINGY" and generally steps you through how
> >> > > it works assuming you know nothing at all about whatever
> >> > > the tracing facility you're using is.
> >> >
> >> > Ok, so something like this example that I wrote for libvirt a
> >> > while back then
> >> >
> >> > https://www.berrange.com/posts/2011/11/30/watching-the-libvirt-rpc-protocol-using-systemtap/
> >> >
> >> >
> >> > > > You can't typically run this as root,
> >> > >
> >> > > Do you mean "non-root" ?
> >> >
> >> > Sigh, yes, of course.
> >> >
> >> > > > however, I don't think that's a
> >> > > > huge issue, because most QEMU deployments are not running as your own
> >> > > > user account anyway, so you can't directly interact with them no
> >> > > > matter what.
> >> > >
> >> > > It is important, because almost all uses of TCG QEMU are
> >> > > running it from the command line as non-root normal users,
> >> > > especially if they're trying to debug what's going on with a
> >> > > guest binary. So any tracing solution for this kind of usecase
> >> > > must work without requiring root access, I think.
> >> >
> >> > None of the Linux integrated tracing tools allow direct non-root access
> >> > afaik. systemtap has ability to launch probes as non-root, via a privileged
> >> > daemon, but it is restricted to probe scripts that the administrator has
> >> > pre-defined.
> >>
> >> One exception is gdb's static userspace probes support. If you can run
> >> gdb on QEMU then you can trace the same events as SystemTap. I have
> >> never tried this GDB feature:
> >>
> >> https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
> >>
> >> It should work out of the box if your distro builds QEMU with the
> >> 'dtrace' backend enabled.
>
> > Wow, that's great to learn about. It does indeed work !
>
> > If you knew alot about ptrace() you could probably build something
> > that use ptrace() and these probe points to call your dynamic
> > instrumentation code with reasonable low overheads.
>
> I don't think so. Ptrace traps into the kernel and stops the process while a
> separate process decides what to do. That's between 3 and 4 orders of magnitude
> slower than calling an instrumentor function.
Dan might be referring to dynamic patching a jump to the instrumentation
function.
A static userspace probe is a single nop instruction (plus metadata
stored in a separate ELF section). Using ptrace you can binary patch
the nop instruction.
Unfortunately a single nop instruction cannot hold most x86
instructions. uprobes places a breakpoint instruction (INT $3 - 0xcc)
there. That works because it's just one byte.
This technique would be way out of scope for qemu.git but perhaps
perf(1) or a stand-alone tool could implement it. There are libraries
for binary patching like http://www.dyninst.org/dyninst.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 16:05 ` Lluís Vilanova
@ 2017-08-01 13:48 ` Stefan Hajnoczi
2017-08-01 13:54 ` Peter Maydell
2017-08-26 0:02 ` Emilio G. Cota
1 sibling, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-08-01 13:48 UTC (permalink / raw)
To: Lluís Vilanova
Cc: ale+qemu, Daniel P. Berrange, Peter Maydell, Emilio G. Cota,
Stefan Hajnoczi, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 6094 bytes --]
On Fri, Jul 28, 2017 at 07:05:43PM +0300, Lluís Vilanova wrote:
> Daniel P Berrange writes:
>
> > On Fri, Jul 28, 2017 at 02:41:19PM +0100, Peter Maydell wrote:
> >> On 28 July 2017 at 14:34, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >> > Lluís/Peter: What are the requirements for instrumentation code
> >> > interacting with the running QEMU instance? simpletrace is
> >> > asynchronous, meaning it does not wait for anyone handle the trace event
> >> > before continuing execution, and is therefore not suitable for
> >> > SystemTap-style scripts that can interact with the program while
> >> > handling a trace event.
> >>
> >> I think you'd probably want synchronous -- it's pretty helpful
> >> to be able to say "register a trace event hook that doesn't
> >> fire very often, and use that to get to the region of
> >> execution that's of interest to you, then enable more hooks
> >> to get more detail at that point". (For instance, "wait til
> >> we've executed 5,000,000 instructions, then turn on the
> >> tracing of all instruction execution, register modification
> >> and memory accesses".)
>
> > Currently simpletrace probes have a fixed action when they are enabled,
> > namely to print state to the trace log file. Perhaps we can make the
> > action more flexible, if we create a more formal protocol for simpletrace
> > to let it talk over a UNIX socket. By default it could send probe data
> > asynchronously as now, but you could mark probes such that they require
> > a synchronous ACK, thus pausing execution until that ACK is received
> > from the instrumenting program.
>
> > For that to be useful, we would need to have allow probes to be turned
> > on/off via this trace socket, since the normal HMP/QMP monitor execution
> > would be blocked while this probe is running.
>
> This sounds like much more work to me for much lower performance (copy through
> fifo/socket & process synchronization). Especially if many interesting events
> become "synchronous". Also, with MTTCG using fifos becomes more complex on both
> sides (select on the client & synchronization of events between streams on both
> sides).
>
> As a performance example, I've used the approach of this series to perform some
> non-trivial instrumentation and I can get performance on par with PIN (a
> state-of-the-art tool from intel) on most SPEC benchmarks.
>
> Honestly, I would try to see if there's a GPL-preserving way to allow native
> instrumentation libraries to run inside QEMU (either dynamic or static, that's
> less important to me).
>
> As for the (minimum) requirements I've collected:
>
> * Peek at register values and guest memory.
> * Enumerate guest cpus.
> * Control when events are raised to minimize overheads (e.g., avoid generating
> TCG code to trace a guest code event we don't care about just now).
> * When a guest code event is raised at translation time, let instrumentor decide
> if it needs to be raised at execution time too (e.g., might be decided on
> instruction opcode)
> * Minimum overhead when instrumenting (I think the proposed fifo/socket approach
> does not meet this; point 2 helps a lot here, which is what the current
> tracing code does)
> * Let instrumentor know when TBs are being freed (i.e., to free internal per-TB
> structures; I have a patch queued adding this event).
>
> Nice to have:
>
> * Avoid recompilation for each new instrumentation logic.
> * Poke at register values and guest memory.
> * [synchronous for sure] Let user annotate guest programs to raise additional
> events with guest-specified information (the hypertrace series I sent).
> * [synchronous for sure] Make guest breakpoints/watchpoints raise an event (for
> when we cannot annotate the guest code; I have some patches).
> * Trigger QEMU's event tracing routine when the instrumentation code
> decides. This is what this series does now, as then lazy instrumentors don't
> need to write their own tracing-to-disk code. Otherwise, per-event tracing
> states would need to be independent of instrumentation states (but the logic
> is exactly the same).
> * Let instrumentor define additional arguments to execution-time events during
> translation-time (e.g., to quickly index into an instrumentor struct when the
> execution-time event comes that references info collected during translation).
> * Attach an instrumentor-specified value to each guest CPU (e.g., pointing to
> the instrumentor's cpu state). Might be less necessary if the point above is
> met (if we only look at overall performance).
Thanks for sharing the requirements. A stable API is necessary for
providing these features.
We're essentially talking about libqemu. That means QEMU in library
form with an API for JIT engine, reverse engineering, instrumentation,
etc tasks.
The user must write a driver program:
#include <stdio.h>
#include <libqemu.h>
static void my_syscall(QemuVcpu *vcpu, uint64_t num, uint64_t arg1, ...)
{
if (num == 0x123) {
printf("r1 = %x\n", qemu_vcpu_get_reg32(vcpu, QEMU_REG_R1));
}
}
int main(int argc, char **argv)
{
qemu_init(argc, argv);
qemu_add_syscall_handler(my_syscall);
qemu_run();
return 0;
}
There should be real C functions with API documentation instead of
hooking trace events by name. Raw QEMU trace event arguments are not
suitable for libqemu users anyway (libqemu QemuVcpu and similar stable
API types are needed instead).
Maintaining libqemu will take ongoing effort and no one has committed.
The last discussion about libqemu was here:
https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg04847.html
I think think meets your requirements - native instrumentation code, no
recompiling QEMU, and flexible/powerful APIs possible.
Perhaps if you start with a focus on your use cases other people will
step in to help maintain and broaden the scope of libqemu for other
tasks.
What do you think?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-01 13:48 ` Stefan Hajnoczi
@ 2017-08-01 13:54 ` Peter Maydell
2017-08-02 11:04 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Peter Maydell @ 2017-08-01 13:54 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Lluís Vilanova, Alessandro Di Federico, Daniel P. Berrange,
Emilio G. Cota, Stefan Hajnoczi, QEMU Developers
On 1 August 2017 at 14:48, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> Thanks for sharing the requirements. A stable API is necessary for
> providing these features.
>
> We're essentially talking about libqemu. That means QEMU in library
> form with an API for JIT engine, reverse engineering, instrumentation,
> etc tasks.
> Maintaining libqemu will take ongoing effort and no one has committed.
> The last discussion about libqemu was here:
> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg04847.html
That thread seems to be focused on trying to extract TCG
from the rest of QEMU, which definitely isn't a requirement
for instrumentation, and I would suggest is something
of a distraction from it. I want to be able to say "just
instrument this setup that QEMU already provides as a
board model", not have to write a driver that duplicates
all the work vl.c and our board models do for us today,
and I don't need the TCG engine to be a library to do that...
I agree that we want to provide something that is at least
closer to a stable API than "just expose trace events",
though.
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-01 13:54 ` Peter Maydell
@ 2017-08-02 11:04 ` Stefan Hajnoczi
2017-08-02 11:10 ` Peter Maydell
0 siblings, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-08-02 11:04 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Lluís Vilanova, Alessandro Di Federico,
Daniel P. Berrange, Emilio G. Cota, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]
On Tue, Aug 01, 2017 at 02:54:29PM +0100, Peter Maydell wrote:
> On 1 August 2017 at 14:48, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > Thanks for sharing the requirements. A stable API is necessary for
> > providing these features.
> >
> > We're essentially talking about libqemu. That means QEMU in library
> > form with an API for JIT engine, reverse engineering, instrumentation,
> > etc tasks.
>
> > Maintaining libqemu will take ongoing effort and no one has committed.
> > The last discussion about libqemu was here:
> > https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg04847.html
>
> That thread seems to be focused on trying to extract TCG
> from the rest of QEMU, which definitely isn't a requirement
> for instrumentation, and I would suggest is something
> of a distraction from it.
You are right, it's a different use case. I just wanted to reference
previous discussion about libqemu. There have been more in the past for
different use cases.
> I want to be able to say "just
> instrument this setup that QEMU already provides as a
> board model", not have to write a driver that duplicates
> all the work vl.c and our board models do for us today,
Calling qemu_init(argc, argv) and qemu_run() isn't too onerous. For the
price of that you get an environment where we can offer stable APIs.
> and I don't need the TCG engine to be a library to do that...
You do need TCG APIs if you want TCG-level instrumentation, tuning
options, callbacks, etc.
> I agree that we want to provide something that is at least
> closer to a stable API than "just expose trace events",
> though.
libqemu has at least three parts:
1. VM API (i.e. qemu_init(argc, argv), qemu_run(), qemu_vcpu_get_reg32())
2. TCG engine
3. Device models
Like I said in my email, start with what matters for the instrumentation
use case (VM API at a minimum to control guest execution). Other people
can flesh out the other parts later, as needed.
Other attempts to provide a stable API will be essentially the same
thing as libqemu.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 16:21 ` Lluís Vilanova
@ 2017-08-02 11:04 ` Stefan Hajnoczi
0 siblings, 0 replies; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-08-02 11:04 UTC (permalink / raw)
To: Peter Maydell, Stefan Hajnoczi, Emilio G. Cota, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 3277 bytes --]
On Fri, Jul 28, 2017 at 07:21:04PM +0300, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
>
> > On Thu, Jul 27, 2017 at 11:40:17AM +0100, Peter Maydell wrote:
> >> On 27 July 2017 at 11:32, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >> > On Wed, Jul 26, 2017 at 03:44:39PM +0300, Lluís Vilanova wrote:
> >> >> And why exactly is this a threat? Because it can be used to "extend" QEMU
> >> >> without touching its sources? Is this a realistic threat? (it's a rather brittle
> >> >> way to do it, so I'm not sure it's practical)
> >> >
> >> > Unfortunately it is a problem. I recently came across a product that
> >> > was using LD_PRELOAD= to "integrate" with QEMU. People really abuse
> >> > these interfaces instead of integrating their features cleanly into
> >> > QEMU.
> >>
> >> ...if people who want to do this kind of thing already can and
> >> do use LD_PRELOAD for it, I don't think we should worry too much
> >> about trying to make the instrumentation plugin API bulletproof
> >> against similar abuse.
> >>
> >> > I see the use cases that Peter has been describing and am sure we can
> >> > come up with good solutions. What I care about is that it doesn't allow
> >> > loading a .so that connects to arbitrary trace events.
> >>
> >> That said, I agree that we don't really need an arbitrary-trace-event
> >> setup here, and we should probably design our API so that it isn't
> >> handing the trace plugin hooks pointers into QEMU's internals.
> >> We want an API that makes it easy for people to do things based on
> >> changes of the guest binary's state (registers, insns, etc etc)
> >> and which makes it hard for them to accidentally trip themselves up
> >> (eg by prodding around in QEMU internal data structures).
> >> This will have the secondary benefit that it's unlikely that future
> >> changes to QEMU will break plugin code.
> >>
> >> >> As a side note, I find instrumentation to be most useful for guest code events,
> >> >> which mostly contain non-pointer values (except for the CPUState*).
> >>
> >> For instance we definitely should not be passing a CPUState* to
> >> any plugin function.
>
> > The gdbstub protocol has relevant features for accessing guest memory,
> > registers, etc. Perhaps a set of QEMU-specific events can be added
> > (e.g. tb generated) so it's possible to instrument and control the
> > guest from an instrumentation program (written in any language).
>
> > Perhaps there is a fundamental reason why this isn't possible due to the
> > protocol design, because using gdbstub halts all vcpus, etc. I don't
> > know.
>
> > Do you think this is an interesting direction? It definitely seems like
> > a powerful approach though performance would be less than running native
> > code inside the QEMU process.
>
> That's the same approach someone else dubbed as using a fifo with "synchronous"
> events, right? I have some measurements on this using a pipe, and overheads are
> 1000x to 2300x for each communication event (compared to a function call, and
> depending on whether each process/thread is pinned to the same or different
> CPU).
You are right. I understand the need for native code without
interprocess communication now.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-02 11:04 ` Stefan Hajnoczi
@ 2017-08-02 11:10 ` Peter Maydell
2017-08-02 14:49 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Peter Maydell @ 2017-08-02 11:10 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Stefan Hajnoczi, Lluís Vilanova, Alessandro Di Federico,
Daniel P. Berrange, Emilio G. Cota, QEMU Developers
On 2 August 2017 at 12:04, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Tue, Aug 01, 2017 at 02:54:29PM +0100, Peter Maydell wrote:
>> and I don't need the TCG engine to be a library to do that...
>
> You do need TCG APIs if you want TCG-level instrumentation, tuning
> options, callbacks, etc.
I need an API; that doesn't necessarily look like the kind
of API you want to be able to embed the TCG engine into
other things, I think.
>> I agree that we want to provide something that is at least
>> closer to a stable API than "just expose trace events",
>> though.
>
> libqemu has at least three parts:
>
> 1. VM API (i.e. qemu_init(argc, argv), qemu_run(), qemu_vcpu_get_reg32())
> 2. TCG engine
> 3. Device models
>
> Like I said in my email, start with what matters for the instrumentation
> use case (VM API at a minimum to control guest execution). Other people
> can flesh out the other parts later, as needed.
>
> Other attempts to provide a stable API will be essentially the same
> thing as libqemu.
I don't think this is the case -- you could have a stable
instrumentation API without it looking anything like
libqemu. In particular I don't think you need to have
something that sits at the top level and says 'run'.
In particular I think that pulling TCG out of QEMU
is an enormous and painful undertaking that you just
don't need to do at all to allow this kind of
instrumentation API.
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-02 11:10 ` Peter Maydell
@ 2017-08-02 14:49 ` Stefan Hajnoczi
2017-08-02 15:19 ` Lluís Vilanova
0 siblings, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-08-02 14:49 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Lluís Vilanova, Alessandro Di Federico,
Daniel P. Berrange, Emilio G. Cota, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]
On Wed, Aug 02, 2017 at 12:10:14PM +0100, Peter Maydell wrote:
> On 2 August 2017 at 12:04, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Tue, Aug 01, 2017 at 02:54:29PM +0100, Peter Maydell wrote:
> >> and I don't need the TCG engine to be a library to do that...
> >
> > You do need TCG APIs if you want TCG-level instrumentation, tuning
> > options, callbacks, etc.
>
> I need an API; that doesn't necessarily look like the kind
> of API you want to be able to embed the TCG engine into
> other things, I think.
>
> >> I agree that we want to provide something that is at least
> >> closer to a stable API than "just expose trace events",
> >> though.
> >
> > libqemu has at least three parts:
> >
> > 1. VM API (i.e. qemu_init(argc, argv), qemu_run(), qemu_vcpu_get_reg32())
> > 2. TCG engine
> > 3. Device models
> >
> > Like I said in my email, start with what matters for the instrumentation
> > use case (VM API at a minimum to control guest execution). Other people
> > can flesh out the other parts later, as needed.
> >
> > Other attempts to provide a stable API will be essentially the same
> > thing as libqemu.
>
> I don't think this is the case -- you could have a stable
> instrumentation API without it looking anything like
> libqemu. In particular I don't think you need to have
> something that sits at the top level and says 'run'.
>
> In particular I think that pulling TCG out of QEMU
> is an enormous and painful undertaking that you just
> don't need to do at all to allow this kind of
> instrumentation API.
Please post an example of the API you'd like.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-02 14:49 ` Stefan Hajnoczi
@ 2017-08-02 15:19 ` Lluís Vilanova
2017-08-03 11:54 ` Stefan Hajnoczi
0 siblings, 1 reply; 59+ messages in thread
From: Lluís Vilanova @ 2017-08-02 15:19 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Peter Maydell, Alessandro Di Federico, Stefan Hajnoczi,
QEMU Developers, Emilio G. Cota
Stefan Hajnoczi writes:
> On Wed, Aug 02, 2017 at 12:10:14PM +0100, Peter Maydell wrote:
>> On 2 August 2017 at 12:04, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> > On Tue, Aug 01, 2017 at 02:54:29PM +0100, Peter Maydell wrote:
>> >> and I don't need the TCG engine to be a library to do that...
>> >
>> > You do need TCG APIs if you want TCG-level instrumentation, tuning
>> > options, callbacks, etc.
>>
>> I need an API; that doesn't necessarily look like the kind
>> of API you want to be able to embed the TCG engine into
>> other things, I think.
>>
>> >> I agree that we want to provide something that is at least
>> >> closer to a stable API than "just expose trace events",
>> >> though.
>> >
>> > libqemu has at least three parts:
>> >
>> > 1. VM API (i.e. qemu_init(argc, argv), qemu_run(), qemu_vcpu_get_reg32())
>> > 2. TCG engine
>> > 3. Device models
>> >
>> > Like I said in my email, start with what matters for the instrumentation
>> > use case (VM API at a minimum to control guest execution). Other people
>> > can flesh out the other parts later, as needed.
>> >
>> > Other attempts to provide a stable API will be essentially the same
>> > thing as libqemu.
>>
>> I don't think this is the case -- you could have a stable
>> instrumentation API without it looking anything like
>> libqemu. In particular I don't think you need to have
>> something that sits at the top level and says 'run'.
>>
>> In particular I think that pulling TCG out of QEMU
>> is an enormous and painful undertaking that you just
>> don't need to do at all to allow this kind of
>> instrumentation API.
> Please post an example of the API you'd like.
In my opinion, the instrumentation support in this series provides an API that
works in the opposite way you're suggesting (let's ignore the fact that it's
built on tracing events).
When QEMU loads an instrumentation library (which can happen at any time), some
initialization function is called on the library so that it can establish what
events to instrument. This also has the advantage that a user can hook into a
running QEMU instance at any time to perform some instrumentation.
I think this is the bare minimum necessary to make it work, and has the upside
of being completely orthogonal to the libqemu approach. We could reuse most of
the stable instrumentation API there too, except for the instrumentation code
initialization.
That being said, the libqemu approach *might* make it a bit easier to provide an
API for things such as "run for this many instructions and return control to
instrumentor", but I don't think that's mandatory for a first prototype (and can
definitely be implemented using both approaches).
Cheers,
Lluis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-02 15:19 ` Lluís Vilanova
@ 2017-08-03 11:54 ` Stefan Hajnoczi
2017-08-26 0:14 ` Emilio G. Cota
0 siblings, 1 reply; 59+ messages in thread
From: Stefan Hajnoczi @ 2017-08-03 11:54 UTC (permalink / raw)
To: Peter Maydell, Alessandro Di Federico, Stefan Hajnoczi,
QEMU Developers, Emilio G. Cota
[-- Attachment #1: Type: text/plain, Size: 3647 bytes --]
On Wed, Aug 02, 2017 at 06:19:29PM +0300, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
>
> > On Wed, Aug 02, 2017 at 12:10:14PM +0100, Peter Maydell wrote:
> >> On 2 August 2017 at 12:04, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >> > On Tue, Aug 01, 2017 at 02:54:29PM +0100, Peter Maydell wrote:
> >> >> and I don't need the TCG engine to be a library to do that...
> >> >
> >> > You do need TCG APIs if you want TCG-level instrumentation, tuning
> >> > options, callbacks, etc.
> >>
> >> I need an API; that doesn't necessarily look like the kind
> >> of API you want to be able to embed the TCG engine into
> >> other things, I think.
> >>
> >> >> I agree that we want to provide something that is at least
> >> >> closer to a stable API than "just expose trace events",
> >> >> though.
> >> >
> >> > libqemu has at least three parts:
> >> >
> >> > 1. VM API (i.e. qemu_init(argc, argv), qemu_run(), qemu_vcpu_get_reg32())
> >> > 2. TCG engine
> >> > 3. Device models
> >> >
> >> > Like I said in my email, start with what matters for the instrumentation
> >> > use case (VM API at a minimum to control guest execution). Other people
> >> > can flesh out the other parts later, as needed.
> >> >
> >> > Other attempts to provide a stable API will be essentially the same
> >> > thing as libqemu.
> >>
> >> I don't think this is the case -- you could have a stable
> >> instrumentation API without it looking anything like
> >> libqemu. In particular I don't think you need to have
> >> something that sits at the top level and says 'run'.
> >>
> >> In particular I think that pulling TCG out of QEMU
> >> is an enormous and painful undertaking that you just
> >> don't need to do at all to allow this kind of
> >> instrumentation API.
>
> > Please post an example of the API you'd like.
>
> In my opinion, the instrumentation support in this series provides an API that
> works in the opposite way you're suggesting (let's ignore the fact that it's
> built on tracing events).
>
> When QEMU loads an instrumentation library (which can happen at any time), some
> initialization function is called on the library so that it can establish what
> events to instrument. This also has the advantage that a user can hook into a
> running QEMU instance at any time to perform some instrumentation.
>
> I think this is the bare minimum necessary to make it work, and has the upside
> of being completely orthogonal to the libqemu approach. We could reuse most of
> the stable instrumentation API there too, except for the instrumentation code
> initialization.
>
> That being said, the libqemu approach *might* make it a bit easier to provide an
> API for things such as "run for this many instructions and return control to
> instrumentor", but I don't think that's mandatory for a first prototype (and can
> definitely be implemented using both approaches).
The main concern I have is that a feature for loading shared libraries
and hooking QEMU will be abused. This can be mostly solved by offering
only a stable API without the ability to hook trace events. There are
still ways to abuse this and that's why I prefer the libqemu approach.
The libqemu approach also avoids the "how do I enable my
instrumentation?" step for new users because they just need to run their
program after compiling it. The workflow is simpler.
As a next step I suggest defining the stable instrumentation API and
dropping the tracing hooks. We can still work out whether shared
library loading or libqemu is okay later but the stable API is most
important.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-07-28 16:05 ` Lluís Vilanova
2017-08-01 13:48 ` Stefan Hajnoczi
@ 2017-08-26 0:02 ` Emilio G. Cota
2017-08-29 9:19 ` Peter Maydell
1 sibling, 1 reply; 59+ messages in thread
From: Emilio G. Cota @ 2017-08-26 0:02 UTC (permalink / raw)
To: Daniel P. Berrange, Peter Maydell, Stefan Hajnoczi,
Stefan Hajnoczi, QEMU Developers
On Fri, Jul 28, 2017 at 19:05:43 +0300, Lluís Vilanova wrote:
> As for the (minimum) requirements I've collected:
>
> * Peek at register values and guest memory.
> * Enumerate guest cpus.
> * Control when events are raised to minimize overheads (e.g., avoid generating
> TCG code to trace a guest code event we don't care about just now).
> * When a guest code event is raised at translation time, let instrumentor decide
> if it needs to be raised at execution time too (e.g., might be decided on
> instruction opcode)
> * Minimum overhead when instrumenting (I think the proposed fifo/socket approach
> does not meet this; point 2 helps a lot here, which is what the current
> tracing code does)
> * Let instrumentor know when TBs are being freed (i.e., to free internal per-TB
> structures; I have a patch queued adding this event).
>
> Nice to have:
>
> * Avoid recompilation for each new instrumentation logic.
> * Poke at register values and guest memory.
> * [synchronous for sure] Let user annotate guest programs to raise additional
> events with guest-specified information (the hypertrace series I sent).
> * [synchronous for sure] Make guest breakpoints/watchpoints raise an event (for
> when we cannot annotate the guest code; I have some patches).
> * Trigger QEMU's event tracing routine when the instrumentation code
> decides. This is what this series does now, as then lazy instrumentors don't
> need to write their own tracing-to-disk code. Otherwise, per-event tracing
> states would need to be independent of instrumentation states (but the logic
> is exactly the same).
> * Let instrumentor define additional arguments to execution-time events during
> translation-time (e.g., to quickly index into an instrumentor struct when the
> execution-time event comes that references info collected during translation).
> * Attach an instrumentor-specified value to each guest CPU (e.g., pointing to
> the instrumentor's cpu state). Might be less necessary if the point above is
> met (if we only look at overall performance).
Agreed.
An additional "nice to have" would be:
* Allow inlining of TCG code by the instrumenter. Example use case:
the instrumenter wants to increment a counter every time a
basic block is executed. Instead of calling a callback function on every block's
execution, we could just have a translation-time callback to emit at the beginning
of the translated block the counter increment. This would be much faster, and
is something that all other tools (e.g. DynamoRIO/Pin) implement.
E.
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-03 11:54 ` Stefan Hajnoczi
@ 2017-08-26 0:14 ` Emilio G. Cota
0 siblings, 0 replies; 59+ messages in thread
From: Emilio G. Cota @ 2017-08-26 0:14 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Peter Maydell, Alessandro Di Federico, Stefan Hajnoczi, QEMU Developers
On Thu, Aug 03, 2017 at 12:54:57 +0100, Stefan Hajnoczi wrote:
> > > Please post an example of the API you'd like.
> >
> > In my opinion, the instrumentation support in this series provides an API that
> > works in the opposite way you're suggesting (let's ignore the fact that it's
> > built on tracing events).
> >
> > When QEMU loads an instrumentation library (which can happen at any time), some
> > initialization function is called on the library so that it can establish what
> > events to instrument. This also has the advantage that a user can hook into a
> > running QEMU instance at any time to perform some instrumentation.
> >
> > I think this is the bare minimum necessary to make it work, and has the upside
> > of being completely orthogonal to the libqemu approach. We could reuse most of
> > the stable instrumentation API there too, except for the instrumentation code
> > initialization.
> >
> > That being said, the libqemu approach *might* make it a bit easier to provide an
> > API for things such as "run for this many instructions and return control to
> > instrumentor", but I don't think that's mandatory for a first prototype (and can
> > definitely be implemented using both approaches).
>
> The main concern I have is that a feature for loading shared libraries
> and hooking QEMU will be abused. This can be mostly solved by offering
> only a stable API without the ability to hook trace events. There are
> still ways to abuse this and that's why I prefer the libqemu approach.
>
> The libqemu approach also avoids the "how do I enable my
> instrumentation?" step for new users because they just need to run their
> program after compiling it. The workflow is simpler.
Widely used instrumentation tools, such as DynamoRIO and Pin, follow the
same approach Lluis took here, i.e. your plugin is loaded at run-time by the tool.
So from a usability viewpoint I don't think this approach would be controversial.
> As a next step I suggest defining the stable instrumentation API and
> dropping the tracing hooks. We can still work out whether shared
> library loading or libqemu is okay later but the stable API is most
> important.
I agree with dropping the tracing hooks. However, I wonder whether we'd
want to add another field to struct TranslationBlock to keep track of
the dynamic instrumentation state, or just reuse trace_vcpu_dstate.
I gravitate towards the former approach--would be nice to have the
instrumentation code completely separate from the tracing code, and
hopefully not rely on python nor trace-events files :-)
Emilio
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
2017-08-26 0:02 ` Emilio G. Cota
@ 2017-08-29 9:19 ` Peter Maydell
0 siblings, 0 replies; 59+ messages in thread
From: Peter Maydell @ 2017-08-29 9:19 UTC (permalink / raw)
To: Emilio G. Cota
Cc: Daniel P. Berrange, Stefan Hajnoczi, Stefan Hajnoczi, QEMU Developers
On 26 August 2017 at 01:02, Emilio G. Cota <cota@braap.org> wrote:
> An additional "nice to have" would be:
>
> * Allow inlining of TCG code by the instrumenter. Example use case:
> the instrumenter wants to increment a counter every time a
> basic block is executed. Instead of calling a callback function on every block's
> execution, we could just have a translation-time callback to emit at the beginning
> of the translated block the counter increment. This would be much faster, and
> is something that all other tools (e.g. DynamoRIO/Pin) implement.
This is a feature I would strongly prefer us not to implement.
It exposes too much of QEMU's internals (ie TCG) to the
instrumentation, and it would be pretty complicated to use.
thanks
-- PMM
^ permalink raw reply [flat|nested] 59+ messages in thread
end of thread, other threads:[~2017-08-29 9:19 UTC | newest]
Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-24 17:02 [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Lluís Vilanova
2017-07-24 17:06 ` [Qemu-devel] [PATCH 01/13] instrument: Add documentation Lluís Vilanova
2017-07-24 17:10 ` [Qemu-devel] [PATCH 02/13] instrument: [none] Add null instrumentation mode Lluís Vilanova
2017-07-24 17:14 ` [Qemu-devel] [PATCH 03/13] instrument: [dynamic] Add dynamic " Lluís Vilanova
2017-07-24 17:18 ` [Qemu-devel] [PATCH 04/13] instrument: Allow adding the "instrument" property without modifying event files Lluís Vilanova
2017-07-24 17:22 ` [Qemu-devel] [PATCH 05/13] instrument: [dynamic] Add default public per-event functions Lluís Vilanova
2017-07-24 17:26 ` [Qemu-devel] [PATCH 06/13] instrument: Add event control interface Lluís Vilanova
2017-07-24 17:30 ` [Qemu-devel] [PATCH 07/13] instrument: Add generic command line library loader Lluís Vilanova
2017-07-24 17:34 ` [Qemu-devel] [PATCH 08/13] instrument: [linux-user] Add " Lluís Vilanova
2017-07-24 17:38 ` [Qemu-devel] [PATCH 09/13] instrument: [bsd-user] " Lluís Vilanova
2017-07-24 17:42 ` [Qemu-devel] [PATCH 10/13] instrument: [softmmu] " Lluís Vilanova
2017-07-24 17:46 ` [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add " Lluís Vilanova
2017-07-24 18:03 ` Eric Blake
2017-07-25 8:24 ` Lluís Vilanova
2017-07-25 11:30 ` Eric Blake
2017-07-25 11:51 ` Lluís Vilanova
2017-07-24 17:50 ` [Qemu-devel] [PATCH 12/13] instrument: [hmp] " Lluís Vilanova
2017-07-24 17:54 ` [Qemu-devel] [PATCH 13/13] trace: Rename C++-specific names in event arguments Lluís Vilanova
2017-07-25 13:19 ` [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation Stefan Hajnoczi
2017-07-25 13:30 ` Peter Maydell
2017-07-25 15:11 ` Lluís Vilanova
2017-07-26 11:22 ` Stefan Hajnoczi
2017-07-26 12:44 ` Lluís Vilanova
2017-07-27 10:32 ` Stefan Hajnoczi
2017-07-27 10:40 ` Peter Maydell
2017-07-28 13:42 ` Stefan Hajnoczi
2017-07-28 16:21 ` Lluís Vilanova
2017-08-02 11:04 ` Stefan Hajnoczi
2017-07-26 11:26 ` Stefan Hajnoczi
2017-07-26 11:49 ` Peter Maydell
2017-07-26 12:26 ` Lluís Vilanova
2017-07-27 10:43 ` Daniel P. Berrange
2017-07-27 10:54 ` Peter Maydell
2017-07-27 14:58 ` Lluís Vilanova
2017-07-27 15:21 ` Daniel P. Berrange
2017-07-27 15:33 ` Peter Maydell
2017-07-27 15:45 ` Daniel P. Berrange
2017-07-28 13:34 ` Stefan Hajnoczi
2017-07-28 13:41 ` Peter Maydell
2017-07-28 14:06 ` Daniel P. Berrange
2017-07-28 16:05 ` Lluís Vilanova
2017-08-01 13:48 ` Stefan Hajnoczi
2017-08-01 13:54 ` Peter Maydell
2017-08-02 11:04 ` Stefan Hajnoczi
2017-08-02 11:10 ` Peter Maydell
2017-08-02 14:49 ` Stefan Hajnoczi
2017-08-02 15:19 ` Lluís Vilanova
2017-08-03 11:54 ` Stefan Hajnoczi
2017-08-26 0:14 ` Emilio G. Cota
2017-08-26 0:02 ` Emilio G. Cota
2017-08-29 9:19 ` Peter Maydell
2017-07-28 13:52 ` Daniel P. Berrange
2017-07-28 16:14 ` Lluís Vilanova
2017-08-01 13:13 ` Stefan Hajnoczi
2017-07-28 15:10 ` Lluís Vilanova
2017-07-27 19:55 ` Lluís Vilanova
2017-07-25 14:47 ` Lluís Vilanova
2017-07-26 11:29 ` Stefan Hajnoczi
2017-07-26 12:31 ` 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.