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

[Stefan Hajnoczi <stefanha@gmail.com>]

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