Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>,
	Stefan Hajnoczi <stefanha@gmail.com>,
	"Emilio G. Cota" <cota@braap.org>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
Date: Wed, 26 Jul 2017 12:22:10 +0100	[thread overview]
Message-ID: <20170726112210.GD18489@stefanha-x1.localdomain> (raw)
In-Reply-To: <87o9s88sb4.fsf@frigg.lan>

[-- 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 --]