Hi,

This is v3 of the proposed plugins API for QEMU. As Emilio is busy
having finished his time at Columbia I have volunteered to take the
patch series forward. Emilio's RFC v2 was posted last year:

  Subject: [RFC v2 00/38] Plugin support
  Date: Sun,  9 Dec 2018 14:37:11 -0500
  Message-Id: <20181209193749.12277-1-cota@braap.org>

The idea is to present a plugin interface that allows for interesting
experiments to be written while not exposing the internal details of
how the TCG works to the wider world.

Changes from last year

  - re-basing and fixing up conflicts
  - moved to plugins/ and split files up (core/api/loader)
  - added a design document (docs/devel/plugins.rst)
  - replaced auto-growing structures with glib GArray and friends
  - expanded API to include access to disassembly
  - wrote some more example plugins
  - dropped the RFC tag ;-)

There are a few things left to sort out. Currently the haddr
calculation relies on the softmmu TLB code for each architecture to
dump it in a cpu_env location. Currently this is only done on x86 and
we would require some tweaking to each backend. I think it would
probably be better to just have a wrapper helper which could do the
lookup of the (hot) TLB in C to calculate the address before calling
the helpers.

I also ended up adding a bunch of miscellaneous fixes at the end while
I was re-spinning. If they get some review I'll try and get the
non-controversial stuff up-streamed first.

The area that needs the most review is the single pass TCG op
insertion code. When plugins are enabled each instruction gets dummy
ops inserted and then at the end we swap those out for the actual
calls or remove them from the stream. Hopefully this is not too much
of an abuse of TCG mechanisms.

I'm aiming to get this in for 4.1 but I guess that will depend on how
the review and iteration goes.

Alex Bennée (12):
  docs/devel: add plugins.rst design document
  configure: add --enable-plugins (MOVE TO END)
  plugin: add implementation of the api
  tests/tcg: enable plugin testing
  tests/plugin: add a hotblocks plugin
  plugin: add qemu_plugin_insn_disas helper
  tests/plugin: add instruction execution breakdown
  tests/plugin: add hotpages plugin to breakdown memory access patterns
  accel/stubs: reduce headers from tcg-stub
  include/exec: wrap cpu_ldst.h in CONFIG_TCG
  include/exec/cpu-defs.h: fix typo
  TODO: API changes to make?

Emilio G. Cota (36):
  trace: expand mem_info:size_shift to 3 bits
  tcg/README: fix typo s/afterwise/afterwards/
  cpu: introduce cpu_in_exclusive_work_context()
  translate-all: use cpu_in_exclusive_work_context() in tb_flush
  plugin: add user-facing API
  plugin: add core code
  queue: add QTAILQ_REMOVE_SEVERAL
  cputlb: document get_page_addr_code
  cputlb: introduce get_page_addr_code_hostp
  plugin-gen: add module for TCG-related code
  tcg: add tcg_gen_st_ptr
  tcg: add MO_HADDR to TCGMemOp
  atomic_template: fix indentation in GEN_ATOMIC_HELPER
  atomic_template: add inline trace/plugin helpers
  cpu_ldst_useronly_template: remove redundant #ifndef CODE_ACCESS
  tcg: let plugins instrument memory accesses
  translate-all: notify plugin code of tb_flush
  *-user: notify plugin of exit
  *-user: plugin syscalls
  cpu: hook plugin vcpu events
  plugin-gen: add plugin_insn_append
  translator: add translator_ld{ub,sw,uw,l,q}
  target/arm: call qemu_plugin_insn_append
  target/ppc: fetch code with translator_ld
  target/sh4: fetch code with translator_ld
  target/i386: fetch code with translator_ld
  target/hppa: fetch code with translator_ld
  target/m68k: fetch code with translator_ld
  target/alpha: fetch code with translator_ld
  target/riscv: fetch code with translator_ld
  target/sparc: fetch code with translator_ld
  target/xtensa: fetch code with translator_ld
  target/openrisc: fetch code with translator_ld
  translator: inject instrumentation from plugins
  plugin: add API symbols to qemu-plugins.symbols
  tests/plugin: add sample plugins

Lluís Vilanova (2):
  vl: support -plugin option
  linux-user: support -plugin option

 Makefile                                  |   11 +-
 Makefile.target                           |    2 +
 accel/stubs/tcg-stub.c                    |    3 -
 accel/tcg/Makefile.objs                   |    1 +
 accel/tcg/atomic_template.h               |  117 ++-
 accel/tcg/cpu-exec.c                      |    3 +
 accel/tcg/cputlb.c                        |   56 +-
 accel/tcg/plugin-gen.c                    | 1079 +++++++++++++++++++++
 accel/tcg/plugin-helpers.h                |    6 +
 accel/tcg/translate-all.c                 |   15 +-
 accel/tcg/translator.c                    |   16 +
 bsd-user/syscall.c                        |   12 +
 configure                                 |   86 +-
 cpus-common.c                             |    2 +
 cpus.c                                    |   10 +
 disas.c                                   |  103 ++
 docs/devel/index.rst                      |    1 +
 docs/devel/plugins.rst                    |   99 ++
 exec.c                                    |    2 +
 include/disas/disas.h                     |    2 +
 include/exec/cpu-defs.h                   |   11 +-
 include/exec/cpu_ldst.h                   |    9 +
 include/exec/cpu_ldst_template.h          |   40 +-
 include/exec/cpu_ldst_useronly_template.h |   36 +-
 include/exec/exec-all.h                   |   64 +-
 include/exec/helper-gen.h                 |    1 +
 include/exec/helper-proto.h               |    1 +
 include/exec/helper-tcg.h                 |    1 +
 include/exec/plugin-gen.h                 |   71 ++
 include/exec/translator.h                 |   30 +-
 include/qemu/plugin.h                     |  261 +++++
 include/qemu/qemu-plugin.h                |  349 +++++++
 include/qemu/queue.h                      |   10 +
 include/qom/cpu.h                         |   19 +
 linux-user/exit.c                         |    1 +
 linux-user/main.c                         |   18 +
 linux-user/syscall.c                      |    3 +
 plugins/.gitignore                        |    2 +
 plugins/Makefile.objs                     |   21 +
 plugins/api.c                             |  274 ++++++
 plugins/core.c                            |  524 ++++++++++
 plugins/loader.c                          |  353 +++++++
 plugins/plugin.h                          |   95 ++
 plugins/qemu-plugins.symbols              |   36 +
 qemu-options.hx                           |   17 +
 qom/cpu.c                                 |    2 +
 target/alpha/translate.c                  |    2 +-
 target/arm/translate-a64.c                |    2 +
 target/arm/translate.c                    |    8 +-
 target/hppa/translate.c                   |    2 +-
 target/i386/translate.c                   |   10 +-
 target/m68k/translate.c                   |    2 +-
 target/openrisc/translate.c               |    2 +-
 target/ppc/translate.c                    |    8 +-
 target/riscv/translate.c                  |    2 +-
 target/sh4/translate.c                    |    4 +-
 target/sparc/translate.c                  |    2 +-
 target/xtensa/translate.c                 |    4 +-
 tcg/README                                |    2 +-
 tcg/i386/tcg-target.inc.c                 |    8 +
 tcg/tcg-op.c                              |   40 +-
 tcg/tcg-op.h                              |   16 +
 tcg/tcg-opc.h                             |    3 +
 tcg/tcg.c                                 |   22 +
 tcg/tcg.h                                 |   28 +
 tests/Makefile.include                    |   10 +-
 tests/plugin/Makefile                     |   31 +
 tests/plugin/bb.c                         |   66 ++
 tests/plugin/empty.c                      |   30 +
 tests/plugin/hotblocks.c                  |  145 +++
 tests/plugin/hotpages.c                   |  152 +++
 tests/plugin/howvec.c                     |  297 ++++++
 tests/plugin/insn.c                       |   63 ++
 tests/plugin/mem.c                        |   93 ++
 tests/tcg/Makefile                        |   34 +
 tests/tcg/arm/Makefile.softmmu-target     |    1 +
 trace-events                              |    2 +-
 vl.c                                      |   11 +
 78 files changed, 4855 insertions(+), 122 deletions(-)
 create mode 100644 accel/tcg/plugin-gen.c
 create mode 100644 accel/tcg/plugin-helpers.h
 create mode 100644 docs/devel/plugins.rst
 create mode 100644 include/exec/plugin-gen.h
 create mode 100644 include/qemu/plugin.h
 create mode 100644 include/qemu/qemu-plugin.h
 create mode 100644 plugins/.gitignore
 create mode 100644 plugins/Makefile.objs
 create mode 100644 plugins/api.c
 create mode 100644 plugins/core.c
 create mode 100644 plugins/loader.c
 create mode 100644 plugins/plugin.h
 create mode 100644 plugins/qemu-plugins.symbols
 create mode 100644 tests/plugin/Makefile
 create mode 100644 tests/plugin/bb.c
 create mode 100644 tests/plugin/empty.c
 create mode 100644 tests/plugin/hotblocks.c
 create mode 100644 tests/plugin/hotpages.c
 create mode 100644 tests/plugin/howvec.c
 create mode 100644 tests/plugin/insn.c
 create mode 100644 tests/plugin/mem.c

-- 
2.20.1

On Fri, Jun 14, 2019 at 7:22 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Hi,
>
> This is v3 of the proposed plugins API for QEMU. As Emilio is busy
> having finished his time at Columbia I have volunteered to take the
> patch series forward. Emilio's RFC v2 was posted last year:
>
>   Subject: [RFC v2 00/38] Plugin support
>   Date: Sun,  9 Dec 2018 14:37:11 -0500
>   Message-Id: <20181209193749.12277-1-cota@braap.org>
>
> The idea is to present a plugin interface that allows for interesting
> experiments to be written while not exposing the internal details of
> how the TCG works to the wider world.
>
> Changes from last year
>
>   - re-basing and fixing up conflicts
>   - moved to plugins/ and split files up (core/api/loader)
>   - added a design document (docs/devel/plugins.rst)
>   - replaced auto-growing structures with glib GArray and friends
>   - expanded API to include access to disassembly
>   - wrote some more example plugins
>   - dropped the RFC tag ;-)
>
> There are a few things left to sort out. Currently the haddr
> calculation relies on the softmmu TLB code for each architecture to
> dump it in a cpu_env location. Currently this is only done on x86 and
> we would require some tweaking to each backend. I think it would
> probably be better to just have a wrapper helper which could do the
> lookup of the (hot) TLB in C to calculate the address before calling
> the helpers.
>

Great to see this series!

Is there any particular conceptual reason why not all targets are included
(i.e. mips, s390, etc.)? Or it was just, let's say, lack of your time resources
to complete everything?

Yours,
Aleksandar

> I also ended up adding a bunch of miscellaneous fixes at the end while
> I was re-spinning. If they get some review I'll try and get the
> non-controversial stuff up-streamed first.
>
> The area that needs the most review is the single pass TCG op
> insertion code. When plugins are enabled each instruction gets dummy
> ops inserted and then at the end we swap those out for the actual
> calls or remove them from the stream. Hopefully this is not too much
> of an abuse of TCG mechanisms.
>
> I'm aiming to get this in for 4.1 but I guess that will depend on how
> the review and iteration goes.
>
> Alex Bennée (12):
>   docs/devel: add plugins.rst design document
>   configure: add --enable-plugins (MOVE TO END)
>   plugin: add implementation of the api
>   tests/tcg: enable plugin testing
>   tests/plugin: add a hotblocks plugin
>   plugin: add qemu_plugin_insn_disas helper
>   tests/plugin: add instruction execution breakdown
>   tests/plugin: add hotpages plugin to breakdown memory access patterns
>   accel/stubs: reduce headers from tcg-stub
>   include/exec: wrap cpu_ldst.h in CONFIG_TCG
>   include/exec/cpu-defs.h: fix typo
>   TODO: API changes to make?
>
> Emilio G. Cota (36):
>   trace: expand mem_info:size_shift to 3 bits
>   tcg/README: fix typo s/afterwise/afterwards/
>   cpu: introduce cpu_in_exclusive_work_context()
>   translate-all: use cpu_in_exclusive_work_context() in tb_flush
>   plugin: add user-facing API
>   plugin: add core code
>   queue: add QTAILQ_REMOVE_SEVERAL
>   cputlb: document get_page_addr_code
>   cputlb: introduce get_page_addr_code_hostp
>   plugin-gen: add module for TCG-related code
>   tcg: add tcg_gen_st_ptr
>   tcg: add MO_HADDR to TCGMemOp
>   atomic_template: fix indentation in GEN_ATOMIC_HELPER
>   atomic_template: add inline trace/plugin helpers
>   cpu_ldst_useronly_template: remove redundant #ifndef CODE_ACCESS
>   tcg: let plugins instrument memory accesses
>   translate-all: notify plugin code of tb_flush
>   *-user: notify plugin of exit
>   *-user: plugin syscalls
>   cpu: hook plugin vcpu events
>   plugin-gen: add plugin_insn_append
>   translator: add translator_ld{ub,sw,uw,l,q}
>   target/arm: call qemu_plugin_insn_append
>   target/ppc: fetch code with translator_ld
>   target/sh4: fetch code with translator_ld
>   target/i386: fetch code with translator_ld
>   target/hppa: fetch code with translator_ld
>   target/m68k: fetch code with translator_ld
>   target/alpha: fetch code with translator_ld
>   target/riscv: fetch code with translator_ld
>   target/sparc: fetch code with translator_ld
>   target/xtensa: fetch code with translator_ld
>   target/openrisc: fetch code with translator_ld
>   translator: inject instrumentation from plugins
>   plugin: add API symbols to qemu-plugins.symbols
>   tests/plugin: add sample plugins
>
> Lluís Vilanova (2):
>   vl: support -plugin option
>   linux-user: support -plugin option
>
>  Makefile                                  |   11 +-
>  Makefile.target                           |    2 +
>  accel/stubs/tcg-stub.c                    |    3 -
>  accel/tcg/Makefile.objs                   |    1 +
>  accel/tcg/atomic_template.h               |  117 ++-
>  accel/tcg/cpu-exec.c                      |    3 +
>  accel/tcg/cputlb.c                        |   56 +-
>  accel/tcg/plugin-gen.c                    | 1079 +++++++++++++++++++++
>  accel/tcg/plugin-helpers.h                |    6 +
>  accel/tcg/translate-all.c                 |   15 +-
>  accel/tcg/translator.c                    |   16 +
>  bsd-user/syscall.c                        |   12 +
>  configure                                 |   86 +-
>  cpus-common.c                             |    2 +
>  cpus.c                                    |   10 +
>  disas.c                                   |  103 ++
>  docs/devel/index.rst                      |    1 +
>  docs/devel/plugins.rst                    |   99 ++
>  exec.c                                    |    2 +
>  include/disas/disas.h                     |    2 +
>  include/exec/cpu-defs.h                   |   11 +-
>  include/exec/cpu_ldst.h                   |    9 +
>  include/exec/cpu_ldst_template.h          |   40 +-
>  include/exec/cpu_ldst_useronly_template.h |   36 +-
>  include/exec/exec-all.h                   |   64 +-
>  include/exec/helper-gen.h                 |    1 +
>  include/exec/helper-proto.h               |    1 +
>  include/exec/helper-tcg.h                 |    1 +
>  include/exec/plugin-gen.h                 |   71 ++
>  include/exec/translator.h                 |   30 +-
>  include/qemu/plugin.h                     |  261 +++++
>  include/qemu/qemu-plugin.h                |  349 +++++++
>  include/qemu/queue.h                      |   10 +
>  include/qom/cpu.h                         |   19 +
>  linux-user/exit.c                         |    1 +
>  linux-user/main.c                         |   18 +
>  linux-user/syscall.c                      |    3 +
>  plugins/.gitignore                        |    2 +
>  plugins/Makefile.objs                     |   21 +
>  plugins/api.c                             |  274 ++++++
>  plugins/core.c                            |  524 ++++++++++
>  plugins/loader.c                          |  353 +++++++
>  plugins/plugin.h                          |   95 ++
>  plugins/qemu-plugins.symbols              |   36 +
>  qemu-options.hx                           |   17 +
>  qom/cpu.c                                 |    2 +
>  target/alpha/translate.c                  |    2 +-
>  target/arm/translate-a64.c                |    2 +
>  target/arm/translate.c                    |    8 +-
>  target/hppa/translate.c                   |    2 +-
>  target/i386/translate.c                   |   10 +-
>  target/m68k/translate.c                   |    2 +-
>  target/openrisc/translate.c               |    2 +-
>  target/ppc/translate.c                    |    8 +-
>  target/riscv/translate.c                  |    2 +-
>  target/sh4/translate.c                    |    4 +-
>  target/sparc/translate.c                  |    2 +-
>  target/xtensa/translate.c                 |    4 +-
>  tcg/README                                |    2 +-
>  tcg/i386/tcg-target.inc.c                 |    8 +
>  tcg/tcg-op.c                              |   40 +-
>  tcg/tcg-op.h                              |   16 +
>  tcg/tcg-opc.h                             |    3 +
>  tcg/tcg.c                                 |   22 +
>  tcg/tcg.h                                 |   28 +
>  tests/Makefile.include                    |   10 +-
>  tests/plugin/Makefile                     |   31 +
>  tests/plugin/bb.c                         |   66 ++
>  tests/plugin/empty.c                      |   30 +
>  tests/plugin/hotblocks.c                  |  145 +++
>  tests/plugin/hotpages.c                   |  152 +++
>  tests/plugin/howvec.c                     |  297 ++++++
>  tests/plugin/insn.c                       |   63 ++
>  tests/plugin/mem.c                        |   93 ++
>  tests/tcg/Makefile                        |   34 +
>  tests/tcg/arm/Makefile.softmmu-target     |    1 +
>  trace-events                              |    2 +-
>  vl.c                                      |   11 +
>  78 files changed, 4855 insertions(+), 122 deletions(-)
>  create mode 100644 accel/tcg/plugin-gen.c
>  create mode 100644 accel/tcg/plugin-helpers.h
>  create mode 100644 docs/devel/plugins.rst
>  create mode 100644 include/exec/plugin-gen.h
>  create mode 100644 include/qemu/plugin.h
>  create mode 100644 include/qemu/qemu-plugin.h
>  create mode 100644 plugins/.gitignore
>  create mode 100644 plugins/Makefile.objs
>  create mode 100644 plugins/api.c
>  create mode 100644 plugins/core.c
>  create mode 100644 plugins/loader.c
>  create mode 100644 plugins/plugin.h
>  create mode 100644 plugins/qemu-plugins.symbols
>  create mode 100644 tests/plugin/Makefile
>  create mode 100644 tests/plugin/bb.c
>  create mode 100644 tests/plugin/empty.c
>  create mode 100644 tests/plugin/hotblocks.c
>  create mode 100644 tests/plugin/hotpages.c
>  create mode 100644 tests/plugin/howvec.c
>  create mode 100644 tests/plugin/insn.c
>  create mode 100644 tests/plugin/mem.c
>
> --
> 2.20.1
>
>

Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:

> On Fri, Jun 14, 2019 at 7:22 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Hi,
>>
>> This is v3 of the proposed plugins API for QEMU. As Emilio is busy
>> having finished his time at Columbia I have volunteered to take the
>> patch series forward. Emilio's RFC v2 was posted last year:
>>
>>   Subject: [RFC v2 00/38] Plugin support
>>   Date: Sun,  9 Dec 2018 14:37:11 -0500
>>   Message-Id: <20181209193749.12277-1-cota@braap.org>
>>
>> The idea is to present a plugin interface that allows for interesting
>> experiments to be written while not exposing the internal details of
>> how the TCG works to the wider world.
>>
>> Changes from last year
>>
>>   - re-basing and fixing up conflicts
>>   - moved to plugins/ and split files up (core/api/loader)
>>   - added a design document (docs/devel/plugins.rst)
>>   - replaced auto-growing structures with glib GArray and friends
>>   - expanded API to include access to disassembly
>>   - wrote some more example plugins
>>   - dropped the RFC tag ;-)
>>
>> There are a few things left to sort out. Currently the haddr
>> calculation relies on the softmmu TLB code for each architecture to
>> dump it in a cpu_env location. Currently this is only done on x86 and
>> we would require some tweaking to each backend. I think it would
>> probably be better to just have a wrapper helper which could do the
>> lookup of the (hot) TLB in C to calculate the address before calling
>> the helpers.
>>
>
> Great to see this series!
>
> Is there any particular conceptual reason why not all targets are included
> (i.e. mips, s390, etc.)? Or it was just, let's say, lack of your time resources
> to complete everything?

There are two things.

Firstly this only supports guests that have been converted to the common
translator_loop code. This is to avoid having hooks in all the various
guest front-ends. I think we are slowly moving to having the common
translator_loop as mandatory and once that's done the guest gets plugins
"for free".

The only other change needed for each guest is to use the common
translator_ld function to read instructions. This again avoids us having
to have too many tendrils into the guests. arm's handling of endian
switching being the exception in this case.

AFAICT both mips and s390 are using the common loop stuff so it should
just be a case of adding the relevant translator_ld calls. The only
minor wrinkle would be if they do anything fancy like SH4's gUSA
sequence.

>
> Yours,
> Aleksandar
>
>> I also ended up adding a bunch of miscellaneous fixes at the end while
>> I was re-spinning. If they get some review I'll try and get the
>> non-controversial stuff up-streamed first.
>>
>> The area that needs the most review is the single pass TCG op
>> insertion code. When plugins are enabled each instruction gets dummy
>> ops inserted and then at the end we swap those out for the actual
>> calls or remove them from the stream. Hopefully this is not too much
>> of an abuse of TCG mechanisms.
>>
>> I'm aiming to get this in for 4.1 but I guess that will depend on how
>> the review and iteration goes.
>>
>> Alex Bennée (12):
>>   docs/devel: add plugins.rst design document
>>   configure: add --enable-plugins (MOVE TO END)
>>   plugin: add implementation of the api
>>   tests/tcg: enable plugin testing
>>   tests/plugin: add a hotblocks plugin
>>   plugin: add qemu_plugin_insn_disas helper
>>   tests/plugin: add instruction execution breakdown
>>   tests/plugin: add hotpages plugin to breakdown memory access patterns
>>   accel/stubs: reduce headers from tcg-stub
>>   include/exec: wrap cpu_ldst.h in CONFIG_TCG
>>   include/exec/cpu-defs.h: fix typo
>>   TODO: API changes to make?
>>
>> Emilio G. Cota (36):
>>   trace: expand mem_info:size_shift to 3 bits
>>   tcg/README: fix typo s/afterwise/afterwards/
>>   cpu: introduce cpu_in_exclusive_work_context()
>>   translate-all: use cpu_in_exclusive_work_context() in tb_flush
>>   plugin: add user-facing API
>>   plugin: add core code
>>   queue: add QTAILQ_REMOVE_SEVERAL
>>   cputlb: document get_page_addr_code
>>   cputlb: introduce get_page_addr_code_hostp
>>   plugin-gen: add module for TCG-related code
>>   tcg: add tcg_gen_st_ptr
>>   tcg: add MO_HADDR to TCGMemOp
>>   atomic_template: fix indentation in GEN_ATOMIC_HELPER
>>   atomic_template: add inline trace/plugin helpers
>>   cpu_ldst_useronly_template: remove redundant #ifndef CODE_ACCESS
>>   tcg: let plugins instrument memory accesses
>>   translate-all: notify plugin code of tb_flush
>>   *-user: notify plugin of exit
>>   *-user: plugin syscalls
>>   cpu: hook plugin vcpu events
>>   plugin-gen: add plugin_insn_append
>>   translator: add translator_ld{ub,sw,uw,l,q}
>>   target/arm: call qemu_plugin_insn_append
>>   target/ppc: fetch code with translator_ld
>>   target/sh4: fetch code with translator_ld
>>   target/i386: fetch code with translator_ld
>>   target/hppa: fetch code with translator_ld
>>   target/m68k: fetch code with translator_ld
>>   target/alpha: fetch code with translator_ld
>>   target/riscv: fetch code with translator_ld
>>   target/sparc: fetch code with translator_ld
>>   target/xtensa: fetch code with translator_ld
>>   target/openrisc: fetch code with translator_ld
>>   translator: inject instrumentation from plugins
>>   plugin: add API symbols to qemu-plugins.symbols
>>   tests/plugin: add sample plugins
>>
>> Lluís Vilanova (2):
>>   vl: support -plugin option
>>   linux-user: support -plugin option
>>
>>  Makefile                                  |   11 +-
>>  Makefile.target                           |    2 +
>>  accel/stubs/tcg-stub.c                    |    3 -
>>  accel/tcg/Makefile.objs                   |    1 +
>>  accel/tcg/atomic_template.h               |  117 ++-
>>  accel/tcg/cpu-exec.c                      |    3 +
>>  accel/tcg/cputlb.c                        |   56 +-
>>  accel/tcg/plugin-gen.c                    | 1079 +++++++++++++++++++++
>>  accel/tcg/plugin-helpers.h                |    6 +
>>  accel/tcg/translate-all.c                 |   15 +-
>>  accel/tcg/translator.c                    |   16 +
>>  bsd-user/syscall.c                        |   12 +
>>  configure                                 |   86 +-
>>  cpus-common.c                             |    2 +
>>  cpus.c                                    |   10 +
>>  disas.c                                   |  103 ++
>>  docs/devel/index.rst                      |    1 +
>>  docs/devel/plugins.rst                    |   99 ++
>>  exec.c                                    |    2 +
>>  include/disas/disas.h                     |    2 +
>>  include/exec/cpu-defs.h                   |   11 +-
>>  include/exec/cpu_ldst.h                   |    9 +
>>  include/exec/cpu_ldst_template.h          |   40 +-
>>  include/exec/cpu_ldst_useronly_template.h |   36 +-
>>  include/exec/exec-all.h                   |   64 +-
>>  include/exec/helper-gen.h                 |    1 +
>>  include/exec/helper-proto.h               |    1 +
>>  include/exec/helper-tcg.h                 |    1 +
>>  include/exec/plugin-gen.h                 |   71 ++
>>  include/exec/translator.h                 |   30 +-
>>  include/qemu/plugin.h                     |  261 +++++
>>  include/qemu/qemu-plugin.h                |  349 +++++++
>>  include/qemu/queue.h                      |   10 +
>>  include/qom/cpu.h                         |   19 +
>>  linux-user/exit.c                         |    1 +
>>  linux-user/main.c                         |   18 +
>>  linux-user/syscall.c                      |    3 +
>>  plugins/.gitignore                        |    2 +
>>  plugins/Makefile.objs                     |   21 +
>>  plugins/api.c                             |  274 ++++++
>>  plugins/core.c                            |  524 ++++++++++
>>  plugins/loader.c                          |  353 +++++++
>>  plugins/plugin.h                          |   95 ++
>>  plugins/qemu-plugins.symbols              |   36 +
>>  qemu-options.hx                           |   17 +
>>  qom/cpu.c                                 |    2 +
>>  target/alpha/translate.c                  |    2 +-
>>  target/arm/translate-a64.c                |    2 +
>>  target/arm/translate.c                    |    8 +-
>>  target/hppa/translate.c                   |    2 +-
>>  target/i386/translate.c                   |   10 +-
>>  target/m68k/translate.c                   |    2 +-
>>  target/openrisc/translate.c               |    2 +-
>>  target/ppc/translate.c                    |    8 +-
>>  target/riscv/translate.c                  |    2 +-
>>  target/sh4/translate.c                    |    4 +-
>>  target/sparc/translate.c                  |    2 +-
>>  target/xtensa/translate.c                 |    4 +-
>>  tcg/README                                |    2 +-
>>  tcg/i386/tcg-target.inc.c                 |    8 +
>>  tcg/tcg-op.c                              |   40 +-
>>  tcg/tcg-op.h                              |   16 +
>>  tcg/tcg-opc.h                             |    3 +
>>  tcg/tcg.c                                 |   22 +
>>  tcg/tcg.h                                 |   28 +
>>  tests/Makefile.include                    |   10 +-
>>  tests/plugin/Makefile                     |   31 +
>>  tests/plugin/bb.c                         |   66 ++
>>  tests/plugin/empty.c                      |   30 +
>>  tests/plugin/hotblocks.c                  |  145 +++
>>  tests/plugin/hotpages.c                   |  152 +++
>>  tests/plugin/howvec.c                     |  297 ++++++
>>  tests/plugin/insn.c                       |   63 ++
>>  tests/plugin/mem.c                        |   93 ++
>>  tests/tcg/Makefile                        |   34 +
>>  tests/tcg/arm/Makefile.softmmu-target     |    1 +
>>  trace-events                              |    2 +-
>>  vl.c                                      |   11 +
>>  78 files changed, 4855 insertions(+), 122 deletions(-)
>>  create mode 100644 accel/tcg/plugin-gen.c
>>  create mode 100644 accel/tcg/plugin-helpers.h
>>  create mode 100644 docs/devel/plugins.rst
>>  create mode 100644 include/exec/plugin-gen.h
>>  create mode 100644 include/qemu/plugin.h
>>  create mode 100644 include/qemu/qemu-plugin.h
>>  create mode 100644 plugins/.gitignore
>>  create mode 100644 plugins/Makefile.objs
>>  create mode 100644 plugins/api.c
>>  create mode 100644 plugins/core.c
>>  create mode 100644 plugins/loader.c
>>  create mode 100644 plugins/plugin.h
>>  create mode 100644 plugins/qemu-plugins.symbols
>>  create mode 100644 tests/plugin/Makefile
>>  create mode 100644 tests/plugin/bb.c
>>  create mode 100644 tests/plugin/empty.c
>>  create mode 100644 tests/plugin/hotblocks.c
>>  create mode 100644 tests/plugin/hotpages.c
>>  create mode 100644 tests/plugin/howvec.c
>>  create mode 100644 tests/plugin/insn.c
>>  create mode 100644 tests/plugin/mem.c
>>
>> --
>> 2.20.1
>>
>>


--
Alex Bennée

Hi Alex/Emilio,

I am really happy to see the progress you made on the plugin feature. Looking
forward to seeing it merged soon! Please CC me on future versions of the
patchset. I am happy to help review and contribute to this effort.

I have a few general comments from experience writing a very similar system
(qsim) below.

On Fri, Jun 14, 2019 at 10:23 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Hi,
>
> This is v3 of the proposed plugins API for QEMU. As Emilio is busy
> having finished his time at Columbia I have volunteered to take the
> patch series forward. Emilio's RFC v2 was posted last year:
>
>   Subject: [RFC v2 00/38] Plugin support
>   Date: Sun,  9 Dec 2018 14:37:11 -0500
>   Message-Id: <20181209193749.12277-1-cota@braap.org>
>

* Register and memory read/write API

  It would be great to have register and memory read/write API i.e., ability
  to read/write to registers/memory from within the callback. This gives the
  plugin ability to do system introspection. (Not sure if the current patchset
  implements this already).

* Register callbacks

  A callback needs to be invoked whenever a specified registers is read or
  written to.

* Where do new plugins live in the tree?

  The current source files in plugins (api, core etc.,) I think are better if
  moved to tcg/plugins/.  The various plugins we write would then live in the
  plugins/ folder instead of the current tests/plugin/ folder.

* Timer interrupts

  What I observed is that the system execution is affected by what you do in
  the callbacks because of timer interrupts. For example, if you spend time in
  the memory callback doing a bit of processing and writing to a file, you
  will see more timer interrupt instructions. One solution to this would be to
  use 'icount', but it does not work that well. I think we need to do
  something similar to what gdb does in debug mode. How would you handle MTTCG
  guests in that case?

  Another approach would be to offload callback generation to a separate
  plugin thread. The main thread will copy data required by a callback and
  invoke the callback asynchronously (std::async in C++ if you are familiar).

* Starting and stopping callback generation

  It would be great if we have a mechanism to dynamically start/stop callbacks
  when a sequence of code (magic instruction) is executed. This would be
  useful to annotate region-of-interest (ROI) in benchmarks to
generate callbacks.

  Also, the return value from a callback can be used to decide further course
  of action. For example, if our plugin needs 10000 callbacks, it can indicate
  to stop generating further callbacks in the return value of the callback
  once it got the necessary callbacks.

* State saving API

  An API to save the state of the VM from the plugin code.

Let me know your thoughts and any other ideas you might have.

Thanks,
--
Pranith

Pranith Kumar <bobby.prani@gmail.com> writes:

> Hi Alex/Emilio,
>
> I am really happy to see the progress you made on the plugin feature. Looking
> forward to seeing it merged soon! Please CC me on future versions of the
> patchset. I am happy to help review and contribute to this effort.

Will do.

>
> I have a few general comments from experience writing a very similar system
> (qsim) below.
>
> On Fri, Jun 14, 2019 at 10:23 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Hi,
>>
>> This is v3 of the proposed plugins API for QEMU. As Emilio is busy
>> having finished his time at Columbia I have volunteered to take the
>> patch series forward. Emilio's RFC v2 was posted last year:
>>
>>   Subject: [RFC v2 00/38] Plugin support
>>   Date: Sun,  9 Dec 2018 14:37:11 -0500
>>   Message-Id: <20181209193749.12277-1-cota@braap.org>
>>
>
> * Register and memory read/write API
>
>   It would be great to have register and memory read/write API i.e., ability
>   to read/write to registers/memory from within the callback. This gives the
>   plugin ability to do system introspection. (Not sure if the current patchset
>   implements this already).

Not currently. The trick is to have something flexible enough without
exposing internals. I guess we could consider the gdb register
enumeration or maybe hook into stuff declared as
tcg_global_mem_new_i[64|32]. That won't get every "register" but it
should certainly cover the main general purpose ones. Things like SVE
and AdvSIMD vector registers wouldn't be seen though.

> * Register callbacks
>
>   A callback needs to be invoked whenever a specified registers is read or
>   written to.

Again tricky as not every register read/write is obvious from TCG -
vector registers tweaked from helpers would be a good example.

>
> * Where do new plugins live in the tree?
>
>   The current source files in plugins (api, core etc.,) I think are better if
>   moved to tcg/plugins/.  The various plugins we write would then live in the
>   plugins/ folder instead of the current tests/plugin/ folder.

The example plugins are really just toys for experimenting with the API
- I don't see too much problem with them being in tests. However the
howvec plugin is very guest architecture specific so we could consider a
bit more of a hierarchy. Maybe these should all live in tests/tcg?

>
> * Timer interrupts
>
>   What I observed is that the system execution is affected by what you do in
>   the callbacks because of timer interrupts. For example, if you spend time in
>   the memory callback doing a bit of processing and writing to a file, you
>   will see more timer interrupt instructions. One solution to this would be to
>   use 'icount', but it does not work that well. I think we need to do
>   something similar to what gdb does in debug mode. How would you handle MTTCG
>   guests in that case?

icount is going to be the best you can get for deterministic time -
other efforts to pause/restart virtual time going in and out of plugins
are just going to add a lot of overhead.

Remember QEMU doesn't even try to be a cycle accurate emulation so
expecting to get reasonable timing information out of these plugins is a
stretch. Maybe I should make that clearer in the design document?

The gdb behaviour is just a massive hack. When single-stepping in GDB we
prevent timer IRQs from being delivered - they have still fired and are
pending and will execute as soon as you hit continue.

>   Another approach would be to offload callback generation to a separate
>   plugin thread. The main thread will copy data required by a callback and
>   invoke the callback asynchronously (std::async in C++ if you are
>   familiar).

This would complicate things - the current iteration I'm working on
drops the haddr cb in favour of dynamically resolving the vaddr in the
callback. But that approach is only valid during the callback before
something else potentially pollutes the TLB.

>
> * Starting and stopping callback generation
>
>   It would be great if we have a mechanism to dynamically start/stop callbacks
>   when a sequence of code (magic instruction) is executed. This would be
>   useful to annotate region-of-interest (ROI) in benchmarks to
> generate callbacks.

Well we have that now. At each TB generation event the callback is free to register
as many or few callbacks as it likes dynamically.

>
>   Also, the return value from a callback can be used to decide further course
>   of action. For example, if our plugin needs 10000 callbacks, it can indicate
>   to stop generating further callbacks in the return value of the callback
>   once it got the necessary callbacks.

This can be done internally - a plugin can unregister itself once it has
collected everything it wanted. If it wants to hang around you can
qemu_plugin_reset to reset all callbacks.

Do you want something finer grained?

> * State saving API
>
>   An API to save the state of the VM from the plugin code.

This would be tricky for -smp > 1 because other threads will have run on
while you are making your decision. It would certainly be useful for
analyzing failed states though.

>
> Let me know your thoughts and any other ideas you might have.
>
> Thanks,


--
Alex Bennée

On Fri, Jun 21, 2019 at 1:21 AM Alex Bennée <alex.bennee@linaro.org> wrote:

> > * Register and memory read/write API
> >
> >   It would be great to have register and memory read/write API i.e., ability
> >   to read/write to registers/memory from within the callback. This gives the
> >   plugin ability to do system introspection. (Not sure if the current patchset
> >   implements this already).
>
> Not currently. The trick is to have something flexible enough without
> exposing internals. I guess we could consider the gdb register
> enumeration or maybe hook into stuff declared as
> tcg_global_mem_new_i[64|32]. That won't get every "register" but it
> should certainly cover the main general purpose ones. Things like SVE
> and AdvSIMD vector registers wouldn't be seen though.

I guess general registers could be a good starting point. We can then
implement arch specific register access APIs.

>
> > * Register callbacks
> >
> >   A callback needs to be invoked whenever a specified registers is read or
> >   written to.
>
> Again tricky as not every register read/write is obvious from TCG -
> vector registers tweaked from helpers would be a good example.
>
> >
> > * Where do new plugins live in the tree?
> >
> >   The current source files in plugins (api, core etc.,) I think are better if
> >   moved to tcg/plugins/.  The various plugins we write would then live in the
> >   plugins/ folder instead of the current tests/plugin/ folder.
>
> The example plugins are really just toys for experimenting with the API
> - I don't see too much problem with them being in tests. However the
> howvec plugin is very guest architecture specific so we could consider a
> bit more of a hierarchy. Maybe these should all live in tests/tcg?
>

So where do you want 'real' plugins to live in the tree? It would be
good to think about the structure for those.

> >
> > * Timer interrupts
> >
> >   What I observed is that the system execution is affected by what you do in
> >   the callbacks because of timer interrupts. For example, if you spend time in
> >   the memory callback doing a bit of processing and writing to a file, you
> >   will see more timer interrupt instructions. One solution to this would be to
> >   use 'icount', but it does not work that well. I think we need to do
> >   something similar to what gdb does in debug mode. How would you handle MTTCG
> >   guests in that case?
>
> icount is going to be the best you can get for deterministic time -
> other efforts to pause/restart virtual time going in and out of plugins
> are just going to add a lot of overhead.

I wonder why using icount is not working in this case. Are there any
timers that fire non-deterministically even when icount is used?

>
> Remember QEMU doesn't even try to be a cycle accurate emulation so
> expecting to get reasonable timing information out of these plugins is a
> stretch. Maybe I should make that clearer in the design document?

It is less about being cycle accurate and more about being
deterministic. For example, when tracing using plugins+callbacks, you
will see a lot more interrupt code in the trace than when if you
execute without tracing. How do we get them to be more similar?

Another idea would be to provide an API for the plugin to generate the
timer interrupt. This allows the plugin to generate regular interrupts
irrespective of what is being done in the callbacks.

>
> The gdb behaviour is just a massive hack. When single-stepping in GDB we
> prevent timer IRQs from being delivered - they have still fired and are
> pending and will execute as soon as you hit continue.
>
> >   Another approach would be to offload callback generation to a separate
> >   plugin thread. The main thread will copy data required by a callback and
> >   invoke the callback asynchronously (std::async in C++ if you are
> >   familiar).
>
> This would complicate things - the current iteration I'm working on
> drops the haddr cb in favour of dynamically resolving the vaddr in the
> callback. But that approach is only valid during the callback before
> something else potentially pollutes the TLB.
>

> >
> > * Starting and stopping callback generation
> >
> >   It would be great if we have a mechanism to dynamically start/stop callbacks
> >   when a sequence of code (magic instruction) is executed. This would be
> >   useful to annotate region-of-interest (ROI) in benchmarks to
> > generate callbacks.
>
> Well we have that now. At each TB generation event the callback is free to register
> as many or few callbacks as it likes dynamically.

But how does the plugin know that the TB being generated is the first
TB in the ROI?
Similarly the plugin needs to know the then end of ROI has been reached.

Also, please note that there can be multiple ROIs. It would be good to
know if we can assign ids to each ROI for the plugin.

Thanks,
-- 
Pranith

Pranith Kumar <bobby.prani@gmail.com> writes:

> On Fri, Jun 21, 2019 at 1:21 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> > * Register and memory read/write API
>> >
>> >   It would be great to have register and memory read/write API i.e., ability
>> >   to read/write to registers/memory from within the callback. This gives the
>> >   plugin ability to do system introspection. (Not sure if the current patchset
>> >   implements this already).
>>
>> Not currently. The trick is to have something flexible enough without
>> exposing internals. I guess we could consider the gdb register
>> enumeration or maybe hook into stuff declared as
>> tcg_global_mem_new_i[64|32]. That won't get every "register" but it
>> should certainly cover the main general purpose ones. Things like SVE
>> and AdvSIMD vector registers wouldn't be seen though.
>
> I guess general registers could be a good starting point. We can then
> implement arch specific register access APIs.
>
>>
>> > * Register callbacks
>> >
>> >   A callback needs to be invoked whenever a specified registers is read or
>> >   written to.
>>
>> Again tricky as not every register read/write is obvious from TCG -
>> vector registers tweaked from helpers would be a good example.
>>
>> >
>> > * Where do new plugins live in the tree?
>> >
>> >   The current source files in plugins (api, core etc.,) I think are better if
>> >   moved to tcg/plugins/.  The various plugins we write would then live in the
>> >   plugins/ folder instead of the current tests/plugin/ folder.
>>
>> The example plugins are really just toys for experimenting with the API
>> - I don't see too much problem with them being in tests. However the
>> howvec plugin is very guest architecture specific so we could consider a
>> bit more of a hierarchy. Maybe these should all live in tests/tcg?
>>
>
> So where do you want 'real' plugins to live in the tree? It would be
> good to think about the structure for those.

I don't see whats wrong with tests/plugins for this. For the upstream
point of view they are there to test and exercise the plugin code.

>
>> >
>> > * Timer interrupts
>> >
>> >   What I observed is that the system execution is affected by what you do in
>> >   the callbacks because of timer interrupts. For example, if you spend time in
>> >   the memory callback doing a bit of processing and writing to a file, you
>> >   will see more timer interrupt instructions. One solution to this would be to
>> >   use 'icount', but it does not work that well. I think we need to do
>> >   something similar to what gdb does in debug mode. How would you handle MTTCG
>> >   guests in that case?
>>
>> icount is going to be the best you can get for deterministic time -
>> other efforts to pause/restart virtual time going in and out of plugins
>> are just going to add a lot of overhead.
>
> I wonder why using icount is not working in this case. Are there any
> timers that fire non-deterministically even when icount is used?

Shouldn't be. What sort of problems are you seeing?

>> Remember QEMU doesn't even try to be a cycle accurate emulation so
>> expecting to get reasonable timing information out of these plugins is a
>> stretch. Maybe I should make that clearer in the design document?
>
> It is less about being cycle accurate and more about being
> deterministic. For example, when tracing using plugins+callbacks, you
> will see a lot more interrupt code in the trace than when if you
> execute without tracing. How do we get them to be more similar?
>
> Another idea would be to provide an API for the plugin to generate the
> timer interrupt. This allows the plugin to generate regular interrupts
> irrespective of what is being done in the callbacks.

I don't think allowing plugins to change behaviour of the code is
something we want to consider - at least for v1.

>
>>
>> The gdb behaviour is just a massive hack. When single-stepping in GDB we
>> prevent timer IRQs from being delivered - they have still fired and are
>> pending and will execute as soon as you hit continue.
>>
>> >   Another approach would be to offload callback generation to a separate
>> >   plugin thread. The main thread will copy data required by a callback and
>> >   invoke the callback asynchronously (std::async in C++ if you are
>> >   familiar).
>>
>> This would complicate things - the current iteration I'm working on
>> drops the haddr cb in favour of dynamically resolving the vaddr in the
>> callback. But that approach is only valid during the callback before
>> something else potentially pollutes the TLB.
>>
>
>> >
>> > * Starting and stopping callback generation
>> >
>> >   It would be great if we have a mechanism to dynamically start/stop callbacks
>> >   when a sequence of code (magic instruction) is executed. This would be
>> >   useful to annotate region-of-interest (ROI) in benchmarks to
>> > generate callbacks.
>>
>> Well we have that now. At each TB generation event the callback is free to register
>> as many or few callbacks as it likes dynamically.
>
> But how does the plugin know that the TB being generated is the first
> TB in the ROI?

It has to track the information - I assume by a combination of looking
at addresses or looking through the instruction patterns.

> Similarly the plugin needs to know the then end of ROI has been reached.
>
> Also, please note that there can be multiple ROIs. It would be good to
> know if we can assign ids to each ROI for the plugin.

This all sounds like stuff the plugin can do for itself. What else does
it need to know from QEMU?

--
Alex Bennée

Patchew URL: https://patchew.org/QEMU/20190614171200.21078-1-alex.bennee@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190614171200.21078-1-alex.bennee@linaro.org
Type: series
Subject: [Qemu-devel] [PATCH  v3 00/50] tcg plugin support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
049048f TODO: API changes to make?
ed124e9 include/exec/cpu-defs.h: fix typo
f5a1e5b include/exec: wrap cpu_ldst.h in CONFIG_TCG
9ba123b accel/stubs: reduce headers from tcg-stub
df9793f tests/plugin: add hotpages plugin to breakdown memory access patterns
d1f355e tests/plugin: add instruction execution breakdown
6d4683c plugin: add qemu_plugin_insn_disas helper
e3145c6 tests/plugin: add a hotblocks plugin
473cace tests/tcg: enable plugin testing
c99d2ca tests/plugin: add sample plugins
f92b641 linux-user: support -plugin option
f81dc94 vl: support -plugin option
11d158f plugin: add API symbols to qemu-plugins.symbols
d0af116 translator: inject instrumentation from plugins
075f779 target/openrisc: fetch code with translator_ld
6426594 target/xtensa: fetch code with translator_ld
38fa0bd target/sparc: fetch code with translator_ld
96a7c87 target/riscv: fetch code with translator_ld
2bbeaea target/alpha: fetch code with translator_ld
2a3f91f target/m68k: fetch code with translator_ld
86b0923 target/hppa: fetch code with translator_ld
206cd1e target/i386: fetch code with translator_ld
c7ffbb8 target/sh4: fetch code with translator_ld
23bae27 target/ppc: fetch code with translator_ld
04e9634 target/arm: call qemu_plugin_insn_append
90c2eaa translator: add translator_ld{ub, sw, uw, l, q}
9681e1f plugin-gen: add plugin_insn_append
6c03f40 cpu: hook plugin vcpu events
aa70581 *-user: plugin syscalls
d9de19e *-user: notify plugin of exit
2a222c5 translate-all: notify plugin code of tb_flush
fa91333 tcg: let plugins instrument memory accesses
3644ead cpu_ldst_useronly_template: remove redundant #ifndef CODE_ACCESS
ab9dbec atomic_template: add inline trace/plugin helpers
e4aef56 atomic_template: fix indentation in GEN_ATOMIC_HELPER
ee3eba8 tcg: add MO_HADDR to TCGMemOp
3760a15 tcg: add tcg_gen_st_ptr
a4af748 plugin-gen: add module for TCG-related code
7f9020a cputlb: introduce get_page_addr_code_hostp
9a46037 cputlb: document get_page_addr_code
6d8e505 queue: add QTAILQ_REMOVE_SEVERAL
a3f7fb1 plugin: add implementation of the api
1bd3802 plugin: add core code
eb7a5d5 plugin: add user-facing API
33d885f configure: add --enable-plugins (MOVE TO END)
3fd3aeb docs/devel: add plugins.rst design document
dd4b2c0 translate-all: use cpu_in_exclusive_work_context() in tb_flush
6ef20c7 cpu: introduce cpu_in_exclusive_work_context()
8995c5a tcg/README: fix typo s/afterwise/afterwards/
b13029e trace: expand mem_info:size_shift to 3 bits

=== OUTPUT BEGIN ===
1/50 Checking commit b13029ec3c61 (trace: expand mem_info:size_shift to 3 bits)
2/50 Checking commit 8995c5a5814a (tcg/README: fix typo s/afterwise/afterwards/)
3/50 Checking commit 6ef20c7ac84b (cpu: introduce cpu_in_exclusive_work_context())
4/50 Checking commit dd4b2c0b5d58 (translate-all: use cpu_in_exclusive_work_context() in tb_flush)
5/50 Checking commit 3fd3aeb92699 (docs/devel: add plugins.rst design document)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#23: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 5/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/50 Checking commit 33d885f24794 (configure: add --enable-plugins (MOVE TO END))
7/50 Checking commit eb7a5d579131 (plugin: add user-facing API)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#15: 
new file mode 100644

WARNING: architecture specific defines should be avoided
#41: FILE: include/qemu/qemu-plugin.h:22:
+#if defined _WIN32 || defined __CYGWIN__

WARNING: architecture specific defines should be avoided
#49: FILE: include/qemu/qemu-plugin.h:30:
+  #if __GNUC__ >= 4

total: 0 errors, 3 warnings, 339 lines checked

Patch 7/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/50 Checking commit 1bd3802f7e55 (plugin: add core code)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#56: 
new file mode 100644

ERROR: "foo * bar" should be "foo *bar"
#197: FILE: include/qemu/plugin.h:137:
+static inline struct qemu_plugin_insn * qemu_plugin_insn_alloc(void)

WARNING: Block comments use a leading /* on a separate line
#371: FILE: plugins/core.c:1:
+/* plugin.c - QEMU Plugin interface

ERROR: named QLIST_HEAD should be typedefed separately
#407: FILE: plugins/core.c:37:
+QLIST_HEAD(qemu_plugin_cb_head, qemu_plugin_cb);

WARNING: line over 80 characters
#626: FILE: plugins/core.c:256:
+        cbs = g_array_sized_new(false, false, sizeof(struct qemu_plugin_dyn_cb), 1);

WARNING: Block comments use a leading /* on a separate line
#942: FILE: plugins/loader.c:42:
+        { /* end of list */ }

ERROR: externs should be avoided in .c files
#948: FILE: plugins/loader.c:48:
+extern struct qemu_plugin_state plugin;

total: 3 errors, 4 warnings, 1294 lines checked

Patch 8/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

9/50 Checking commit a3f7fb1c3afd (plugin: add implementation of the api)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#21: 
new file mode 100644

total: 0 errors, 1 warnings, 271 lines checked

Patch 9/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/50 Checking commit 6d8e50569c13 (queue: add QTAILQ_REMOVE_SEVERAL)
WARNING: Block comments use a leading /* on a separate line
#30: FILE: include/qemu/queue.h:433:
+    } while (/*CONSTCOND*/0)

total: 0 errors, 1 warnings, 16 lines checked

Patch 10/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/50 Checking commit 9a46037bfb3b (cputlb: document get_page_addr_code)
12/50 Checking commit 7f9020af50d0 (cputlb: introduce get_page_addr_code_hostp)
13/50 Checking commit a4af74838a09 (plugin-gen: add module for TCG-related code)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#48: 
new file mode 100644

total: 0 errors, 1 warnings, 1273 lines checked

Patch 13/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/50 Checking commit 3760a1583150 (tcg: add tcg_gen_st_ptr)
15/50 Checking commit ee3eba82d0e6 (tcg: add MO_HADDR to TCGMemOp)
16/50 Checking commit e4aef56e6d7b (atomic_template: fix indentation in GEN_ATOMIC_HELPER)
17/50 Checking commit ab9dbecd0947 (atomic_template: add inline trace/plugin helpers)
18/50 Checking commit 3644ead62d44 (cpu_ldst_useronly_template: remove redundant #ifndef CODE_ACCESS)
19/50 Checking commit fa91333b879c (tcg: let plugins instrument memory accesses)
ERROR: line over 90 characters
#224: FILE: accel/tcg/plugin-gen.c:98:
+    tcg_gen_ld_i32(cpu_index, cpu_env, -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));

total: 1 errors, 0 warnings, 542 lines checked

Patch 19/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

20/50 Checking commit 2a222c5a56d6 (translate-all: notify plugin code of tb_flush)
21/50 Checking commit d9de19e4841b (*-user: notify plugin of exit)
22/50 Checking commit aa7058138134 (*-user: plugin syscalls)
23/50 Checking commit 6c03f4039c79 (cpu: hook plugin vcpu events)
24/50 Checking commit 9681e1f77508 (plugin-gen: add plugin_insn_append)
25/50 Checking commit 90c2eaa2247d (translator: add translator_ld{ub, sw, uw, l, q})
WARNING: Block comments use a leading /* on a separate line
#49: FILE: include/exec/translator.h:166:
+GEN_TRANSLATOR_LD(translator_ldub, ldub, uint8_t, /* no swap needed */)

total: 0 errors, 1 warnings, 40 lines checked

Patch 25/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/50 Checking commit 04e963494144 (target/arm: call qemu_plugin_insn_append)
27/50 Checking commit 23bae27d37b4 (target/ppc: fetch code with translator_ld)
28/50 Checking commit c7ffbb8780fb (target/sh4: fetch code with translator_ld)
29/50 Checking commit 206cd1e53cc9 (target/i386: fetch code with translator_ld)
30/50 Checking commit 86b0923359a8 (target/hppa: fetch code with translator_ld)
31/50 Checking commit 2a3f91fd39e9 (target/m68k: fetch code with translator_ld)
32/50 Checking commit 2bbeaea354dc (target/alpha: fetch code with translator_ld)
33/50 Checking commit 96a7c87ae19f (target/riscv: fetch code with translator_ld)
34/50 Checking commit 38fa0bd596fd (target/sparc: fetch code with translator_ld)
35/50 Checking commit 642659499636 (target/xtensa: fetch code with translator_ld)
36/50 Checking commit 075f7795fe2c (target/openrisc: fetch code with translator_ld)
37/50 Checking commit d0af1163d422 (translator: inject instrumentation from plugins)
38/50 Checking commit 11d158f19449 (plugin: add API symbols to qemu-plugins.symbols)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#120: 
new file mode 100644

total: 0 errors, 1 warnings, 148 lines checked

Patch 38/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
39/50 Checking commit f81dc947d361 (vl: support -plugin option)
40/50 Checking commit f92b641790b2 (linux-user: support -plugin option)
41/50 Checking commit c99d2cab03ae (tests/plugin: add sample plugins)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#34: 
new file mode 100644

total: 0 errors, 1 warnings, 296 lines checked

Patch 41/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
42/50 Checking commit 473cace55471 (tests/tcg: enable plugin testing)
43/50 Checking commit e3145c6f3945 (tests/plugin: add a hotblocks plugin)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: line over 90 characters
#97: FILE: tests/plugin/hotblocks.c:61:
+        g_string_append_printf(report, "  pc: %#" PRIx64 " (%d block%s, %ld insns, %" PRId64" total hits)\n",

WARNING: line over 80 characters
#99: FILE: tests/plugin/hotblocks.c:63:
+                               rec->trans_count, rec->trans_count < 2 ? "" : "s",

WARNING: line over 80 characters
#121: FILE: tests/plugin/hotblocks.c:85:
+    cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) cheap_hash);

WARNING: line over 80 characters
#142: FILE: tests/plugin/hotblocks.c:106:
+    cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) cheap_hash);

total: 1 errors, 4 warnings, 152 lines checked

Patch 43/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

44/50 Checking commit 6d4683cf10dc (plugin: add qemu_plugin_insn_disas helper)
ERROR: space prohibited between function name and open parenthesis '('
#36: FILE: disas.c:493:
+static void plugin_print_address (bfd_vma addr, struct disassemble_info *info)

WARNING: line over 80 characters
#42: FILE: disas.c:499:
+static bool plugin_cap_disas_insn(disassemble_info *info, uint64_t pc, size_t size)

ERROR: "foo * bar" should be "foo *bar"
#72: FILE: disas.c:529:
+char * plugin_disas(CPUState *cpu, uint64_t addr, size_t size)

ERROR: braces {} are necessary for all arms of this statement
#115: FILE: disas.c:572:
+        if (count < 0)
[...]

ERROR: "foo * bar" should be "foo *bar"
#135: FILE: include/disas/disas.h:16:
+char * plugin_disas(CPUState *cpu, uint64_t addr, size_t size);

ERROR: "foo * bar" should be "foo *bar"
#154: FILE: include/qemu/qemu-plugin.h:322:
+char * qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn);

total: 5 errors, 1 warnings, 158 lines checked

Patch 44/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

45/50 Checking commit d1f355eb9db4 (tests/plugin: add instruction execution breakdown)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

ERROR: space required after that ',' (ctx:VxV)
#99: FILE: tests/plugin/howvec.c:69:
+    { "  Add/Sub (imm,tags)","asit",   0x1f800000, 0x11800000, COUNT_CLASS},
                             ^

ERROR: space required after that ',' (ctx:VxV)
#122: FILE: tests/plugin/howvec.c:92:
+    { "  AdvSimd ldstmult++","advlsmp",0xbfb00000, 0x0c800000, COUNT_CLASS},
                             ^

ERROR: space required after that ',' (ctx:VxV)
#122: FILE: tests/plugin/howvec.c:92:
+    { "  AdvSimd ldstmult++","advlsmp",0xbfb00000, 0x0c800000, COUNT_CLASS},
                                       ^

ERROR: space required after that ',' (ctx:VxV)
#124: FILE: tests/plugin/howvec.c:94:
+    { "  AdvSimd ldst++",    "advlssp",0xbf800000, 0x0d800000, COUNT_CLASS},
                                       ^

ERROR: space required after that ',' (ctx:VxV)
#128: FILE: tests/plugin/howvec.c:98:
+    { "  ldst noalloc pair", "ldstnap",0x3b800000, 0x28000000, COUNT_CLASS},
                                       ^

ERROR: space required after that ',' (ctx:VxV)
#132: FILE: tests/plugin/howvec.c:102:
+    { "  ldst reg (reg off)","ldstro", 0x3b200b00, 0x38200800, COUNT_CLASS},
                             ^

ERROR: space required after that ',' (ctx:VxV)
#161: FILE: tests/plugin/howvec.c:131:
+                g_string_append_printf(report,"Class: %-24s\t(%ld hits)\n",
                                              ^

WARNING: line over 80 characters
#162: FILE: tests/plugin/howvec.c:132:
+                                       insn_classes[i].class, insn_classes[i].count);

WARNING: line over 80 characters
#166: FILE: tests/plugin/howvec.c:136:
+            g_string_append_printf(report,"Class: %-24s\tcounted individually\n",

ERROR: space required after that ',' (ctx:VxV)
#166: FILE: tests/plugin/howvec.c:136:
+            g_string_append_printf(report,"Class: %-24s\tcounted individually\n",
                                          ^

ERROR: space required after that ',' (ctx:VxV)
#170: FILE: tests/plugin/howvec.c:140:
+            g_string_append_printf(report,"Class: %-24s\tnot counted\n",
                                          ^

ERROR: space required after that ',' (ctx:VxV)
#182: FILE: tests/plugin/howvec.c:152:
+        g_string_append_printf(report,"Individual Instructions:\n");
                                      ^

WARNING: line over 80 characters
#188: FILE: tests/plugin/howvec.c:158:
+            g_string_append_printf(report, "Instr: %-24s\t(%ld hits)\t(op=%#08x/%s)\n",

WARNING: line over 80 characters
#192: FILE: tests/plugin/howvec.c:162:
+                                   rec->class ? rec->class->class : "un-categorised");

ERROR: "foo * bar" should be "foo *bar"
#212: FILE: tests/plugin/howvec.c:182:
+static uint64_t * find_counter(struct qemu_plugin_insn *insn)

WARNING: line over 80 characters
#243: FILE: tests/plugin/howvec.c:213:
+        icount = (InsnExecCount *) g_hash_table_lookup(insns, GUINT_TO_POINTER(opcode));

ERROR: line over 90 characters
#252: FILE: tests/plugin/howvec.c:222:
+                dprintf(stdout_fd, "adding instrumentation for %s (%#08x @ %#20lx from %s)\n",

WARNING: line over 80 characters
#253: FILE: tests/plugin/howvec.c:223:
+                        icount->insn, opcode, qemu_plugin_insn_vaddr(insn), class->class);

WARNING: line over 80 characters
#255: FILE: tests/plugin/howvec.c:225:
+            g_hash_table_insert(insns, GUINT_TO_POINTER(opcode), (gpointer) icount);

total: 12 errors, 8 warnings, 304 lines checked

Patch 45/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

46/50 Checking commit df9793f9866c (tests/plugin: add hotpages plugin to breakdown memory access patterns)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
new file mode 100644

ERROR: space required after that ',' (ctx:VxV)
#88: FILE: tests/plugin/hotpages.c:59:
+        g_string_append_printf(report,"Individual Instructions:\n");
                                      ^

ERROR: line over 90 characters
#94: FILE: tests/plugin/hotpages.c:65:
+            g_string_append_printf(report, "Page: %"PRIx64" cpus:%#04x/%#04x %"PRId64"/%"PRId64"\n",

ERROR: that open brace { should be on the previous line
#152: FILE: tests/plugin/hotpages.c:123:
+static GOptionEntry entries[] =
+{

WARNING: line over 80 characters
#154: FILE: tests/plugin/hotpages.c:125:
+    { "inline", 'i', 0, G_OPTION_ARG_NONE, &do_inline, "Inline counters", NULL },

ERROR: space prohibited between function name and open parenthesis '('
#164: FILE: tests/plugin/hotpages.c:135:
+    context = g_option_context_new ("- hot pages tool");

ERROR: space prohibited between function name and open parenthesis '('
#165: FILE: tests/plugin/hotpages.c:136:
+    g_option_context_add_main_entries (context, entries, NULL);

ERROR: that open brace { should be on the previous line
#166: FILE: tests/plugin/hotpages.c:137:
+    if (!g_option_context_parse (context, &argc, &argv, &error))
+    {

ERROR: space prohibited between function name and open parenthesis '('
#166: FILE: tests/plugin/hotpages.c:137:
+    if (!g_option_context_parse (context, &argc, &argv, &error))

total: 7 errors, 2 warnings, 159 lines checked

Patch 46/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

47/50 Checking commit 9ba123bb95e6 (accel/stubs: reduce headers from tcg-stub)
48/50 Checking commit f5a1e5b54136 (include/exec: wrap cpu_ldst.h in CONFIG_TCG)
49/50 Checking commit ed124e922b01 (include/exec/cpu-defs.h: fix typo)
50/50 Checking commit 049048fdb87a (TODO: API changes to make?)
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 7 lines checked

Patch 50/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190614171200.21078-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Patchew URL: https://patchew.org/QEMU/20190614171200.21078-1-alex.bennee@linaro.org/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      x86_64-softmmu/balloon.o
  CC      x86_64-softmmu/ioport.o
  CC      x86_64-softmmu/numa.o
/tmp/qemu-test/src/disas.c:502:5: error: use of undeclared identifier 'csh'
    csh handle;
    ^
/tmp/qemu-test/src/disas.c:503:5: error: use of undeclared identifier 'cs_insn'
    cs_insn *insn;
    ^
/tmp/qemu-test/src/disas.c:503:14: error: use of undeclared identifier 'insn'; did you mean 'info'?
    cs_insn *insn;
             ^~~~
             info
/tmp/qemu-test/src/disas.c:499:53: note: 'info' declared here
static bool plugin_cap_disas_insn(disassemble_info *info, uint64_t pc, size_t size)
                                                    ^
/tmp/qemu-test/src/disas.c:508:9: error: implicit declaration of function 'cap_disas_start' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
        ^
/tmp/qemu-test/src/disas.c:508:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/tmp/qemu-test/src/disas.c:508:32: error: use of undeclared identifier 'handle'
    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
                               ^
/tmp/qemu-test/src/disas.c:508:43: error: use of undeclared identifier 'CS_ERR_OK'
    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
                                          ^
/tmp/qemu-test/src/disas.c:511:5: error: use of undeclared identifier 'insn'
    insn = cap_insn;
    ^
/tmp/qemu-test/src/disas.c:511:12: error: use of undeclared identifier 'cap_insn'
    insn = cap_insn;
           ^
/tmp/qemu-test/src/disas.c:517:13: error: implicit declaration of function 'cs_disasm' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    count = cs_disasm(handle, cbuf, size, 0, 1, &insn);
            ^
/tmp/qemu-test/src/disas.c:517:13: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/tmp/qemu-test/src/disas.c:517:23: error: use of undeclared identifier 'handle'
    count = cs_disasm(handle, cbuf, size, 0, 1, &insn);
                      ^
/tmp/qemu-test/src/disas.c:517:50: error: use of undeclared identifier 'insn'
    count = cs_disasm(handle, cbuf, size, 0, 1, &insn);
                                                 ^
/tmp/qemu-test/src/disas.c:520:37: error: use of undeclared identifier 'insn'
        g_string_printf(s, "%s %s", insn->mnemonic, insn->op_str);
                                    ^
/tmp/qemu-test/src/disas.c:520:53: error: use of undeclared identifier 'insn'
        g_string_printf(s, "%s %s", insn->mnemonic, insn->op_str);
                                                    ^
/tmp/qemu-test/src/disas.c:525:5: error: implicit declaration of function 'cs_close' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    cs_close(&handle);
    ^
/tmp/qemu-test/src/disas.c:525:5: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/tmp/qemu-test/src/disas.c:525:15: error: use of undeclared identifier 'handle'
    cs_close(&handle);
              ^
18 errors generated.


The full log is available at
http://patchew.org/logs/20190614171200.21078-1-alex.bennee@linaro.org/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com