Re: [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype

[Alex Bennée]

Pavel Dovgalyuk <address@hidden> writes:

> The following series implements dynamic binary instrumentation upon
> QEMU.

OK I've done a pass through the patches, final comments bellow.

<snip>
>
> For the current patches the plugins should provide the following
> callbacks:
>  - "needs" callback to check whether the specific instruction
>    should be instrumented by this plugin
>  - "run" callback which called before executing the instuction

I think with a little re-jigging we could reduce this to a single run
call which can then decide if it is going to deal with things in this
case.

> Our instrumentation subsystem exploits TCG helper mechanism to embed
> callbacks into the translation blocks. These callbacks may be inserted
> before the specific instructions.
>
> The aim of submission of this series at that early stage is to get
> the feedback which will guide the development process. We are faced
> the following questions:
>  1. Does every plugins should have its own callback embedded into the TB
>     (which will cause TB extra growth in case of multiple plugins),
>     or the instrumentation layer's callback should invoke the plugins
>     that wanted to instrument that specific instruction?

The common case is having one plugin and I'd rather give the flexibility
of what helper to call and how much information to give it to the plugin
and deal with the potential to have multiple helper calls per
instruction for the complex cases.

Now I can see arguments against it from an interface complexity point of
view but I think plugins should get access to the TCG functions so they
can generate their own op sequences if need be and not have to call a
helper at all if they don't need to.

>  2. How the plugins should function? Will they work as a binary dynamic
>     libraries or a script on some interpreted language?

I think plain C dynamic libraries with optional TCG ops would be a
sensible starting point. For the sort of analysis that potentially
involves lots of operations adding an interpreted language would be:

  - a bike-shedding operation (everyone has a favourite)
  - slower than C

>  3. Should the plugins reuse QEMU configuration script results?
>     Now there is no possibility for using platform-specific macros
>     generated by QEMU configure.

It depends but I'm of the opinion that you build your plugins for the
QEMU you have and not expect them to run with any other version.
Defining some sort of cross-version API stability is just making a rod
for our own back.

>  4. Maybe QEMU module infrastructure should be extended to support
>     plugins too?

Pass - I'm not very knowledgeable about what the existing module stuff
does.

>  5. How the GDB-related CPU inspection interface may be used better?
>     We should pass a register code to read the value. These codes
>     are not described in any of the files. Maybe a function for
>     accessing register by name should be added?

Personally it feels a bit hacky at the moment. Perhaps we should be
looking at exposing tcg_global to the plugin interface. They are already
registered with names and it would be more efficient to load from TCG
code into the helper.

>
>
> v2 changes:
>  - added a subsystem for the plugins
>  - added QEMU side API for plugins
>  - added sample plugins for simple tracing
>
> ---
>
> Pavel Dovgalyuk (7):
>       tcg: add headers for non-target helpers
>       Add plugin support
>       plugins: provide helper functions for plugins
>       tcg: add instrumenting module
>       plugins: add plugin template
>       plugin: add instruction execution logger
>       plugins: add syscall logging plugin sample
>
>
>  Makefile.target                   |    1
>  accel/tcg/translator.c            |    5 +
>  configure                         |   14 ++++
>  include/exec/helper-register.h    |   53 +++++++++++++++
>  include/qemu/instrument.h         |    7 ++
>  include/qemu/plugins.h            |    8 ++
>  plugins/exec-log/Makefile         |   19 +++++
>  plugins/exec-log/exec-log.c       |   18 +++++
>  plugins/helper.h                  |    1
>  plugins/include/plugins.h         |   18 +++++
>  plugins/plugins.c                 |  132 
> +++++++++++++++++++++++++++++++++++++
>  plugins/qemulib.c                 |   31 +++++++++
>  plugins/syscall-log/Makefile      |   19 +++++
>  plugins/syscall-log/syscall-log.c |   44 ++++++++++++
>  plugins/template/Makefile         |   19 +++++
>  plugins/template/template.c       |   19 +++++
>  qemu-options.hx                   |   10 +++
>  tcg/tcg.c                         |   12 +++
>  tcg/tcg.h                         |    3 +
>  vl.c                              |    8 ++
>  20 files changed, 440 insertions(+), 1 deletion(-)
>  create mode 100644 include/exec/helper-register.h
>  create mode 100644 include/qemu/instrument.h
>  create mode 100644 include/qemu/plugins.h
>  create mode 100644 plugins/exec-log/Makefile
>  create mode 100644 plugins/exec-log/exec-log.c
>  create mode 100644 plugins/helper.h
>  create mode 100644 plugins/include/plugins.h
>  create mode 100644 plugins/plugins.c
>  create mode 100644 plugins/qemulib.c
>  create mode 100644 plugins/syscall-log/Makefile
>  create mode 100644 plugins/syscall-log/syscall-log.c
>  create mode 100644 plugins/template/Makefile
>  create mode 100644 plugins/template/template.c


--
Alex Bennée