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

["Emilio G. Cota" <cota@braap.org>]

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