1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v4 00/54] plugins for TCG

Alex Bennée posted 54 patches 4 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190731160719.11396-1-alex.bennee@linaro.org
Test docker-clang@ubuntu failed
Test asan failed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test s390x passed
Test checkpatch failed
Makefile                                  |  10 +-
Makefile.target                           |   2 +
accel/stubs/tcg-stub.c                    |   3 -
accel/tcg/Makefile.objs                   |   1 +
accel/tcg/atomic_common.inc.c             |  54 ++
accel/tcg/atomic_template.h               |  96 ++-
accel/tcg/cpu-exec.c                      |   8 +-
accel/tcg/cputlb.c                        |  69 +-
accel/tcg/plugin-gen.c                    | 977 ++++++++++++++++++++++
accel/tcg/plugin-helpers.h                |   5 +
accel/tcg/translate-all.c                 |  15 +-
accel/tcg/translator.c                    |  20 +
accel/tcg/user-exec.c                     |   3 +
bsd-user/syscall.c                        |  24 +-
configure                                 |  86 +-
cpus-common.c                             |   4 +
cpus.c                                    |  10 +
disas.c                                   | 103 +++
docs/devel/index.rst                      |   1 +
docs/devel/plugins.rst                    | 107 +++
exec.c                                    |   2 +
include/disas/disas.h                     |   2 +
include/exec/cpu-defs.h                   |   3 +-
include/exec/cpu_ldst.h                   |  11 +
include/exec/cpu_ldst_template.h          |  37 +-
include/exec/cpu_ldst_useronly_template.h |  29 +-
include/exec/exec-all.h                   |  81 +-
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                 |  58 +-
include/qemu/bswap.h                      |   5 +
include/qemu/plugin.h                     | 261 ++++++
include/qemu/qemu-plugin.h                | 360 ++++++++
include/qemu/queue.h                      |  10 +
include/qom/cpu.h                         |  19 +
include/sysemu/sysemu.h                   |   2 -
include/user/syscall-trace.h              |  40 +
linux-user/exit.c                         |   1 +
linux-user/main.c                         |  18 +
linux-user/syscall.c                      |   7 +-
plugins/.gitignore                        |   2 +
plugins/Makefile.objs                     |  21 +
plugins/api.c                             | 338 ++++++++
plugins/core.c                            | 499 +++++++++++
plugins/loader.c                          | 353 ++++++++
plugins/plugin.h                          |  95 +++
plugins/qemu-plugins.symbols              |  39 +
qemu-options.hx                           |  17 +
qom/cpu.c                                 |   2 +
scripts/tracetool/transform.py            |   1 +
target/alpha/translate.c                  |   2 +-
target/arm/arm_ldst.h                     |  15 +-
target/arm/helper.c                       |  90 +-
target/arm/m_helper.c                     |  18 +-
target/arm/translate.c                    |  64 +-
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/tcg-op.c                              |  40 +-
tcg/tcg-op.h                              |  16 +
tcg/tcg-opc.h                             |   3 +
tcg/tcg.c                                 |  22 +
tcg/tcg.h                                 |  23 +
tests/Makefile.include                    |  10 +-
tests/plugin/Makefile                     |  31 +
tests/plugin/bb.c                         |  65 ++
tests/plugin/empty.c                      |  29 +
tests/plugin/hotblocks.c                  | 146 ++++
tests/plugin/hotpages.c                   | 179 ++++
tests/plugin/howvec.c                     | 301 +++++++
tests/plugin/insn.c                       |  62 ++
tests/plugin/mem.c                        |  96 +++
tests/tcg/Makefile                        |  34 +
tests/tcg/arm/Makefile.softmmu-target     |   1 +
trace-events                              |   8 +-
trace/mem-internal.h                      |  31 +-
trace/mem.h                               |   7 +-
vl.c                                      |  11 +
87 files changed, 5067 insertions(+), 260 deletions(-)
create mode 100644 accel/tcg/atomic_common.inc.c
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 include/user/syscall-trace.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
[Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by Alex Bennée 4 years, 8 months ago 
Hi,

This is the latest iteration of the plugins series. The main changes
from the last version are:

  - dropped passing of haddr to plugins

This makes the code for handling the plugins less invasive in the
softmmu path at the cost of offloading processing to the plugin if it
wants the value. We rely on the fact that the TLB is per vCPU so a
helper can just trigger a re-query of the TLB to get the final
address.

Part of that change involved embedding the MMU index in the meminfo
field for tracing. I see there are some other patches on the list for
messing with TCGMemOp so there might be a clash coming up.

  - translator_ld goes direct to softmmu/user functions

I also mark the [SOFTMMU_]CODE_ACCESS helpers as deprecated. There is
more work to be done to clean up all the current uses of code access
helpers but ideally the only thing that should be peaking at code is
the translator loop itself. However a bunch of helpers have taken to
using code loading functions to peak at the instruction just executed
to figure out what to do. Once those have been fixed then we can
remove those helpers.

Other more minor fixes can be found documented bellow the --- in the
individual patches.

This series also includes the semihosting patches as they are a
pre-requisite for the translator_ld patches for ARM.

Once the tree opens up for development again I hope to get the
semihosting and trivial clean-up patches merged quickly so the patch
count for the plugins patches proper can be reduced a bit.

The following patches need review
  patch 0004/target arm remove run time semihosting checks.patch
  patch 0005/includes remove stale smp max _cpus externs.patch
  patch 0007/trace add mmu_index to mem_info.patch
  patch 0011/docs devel add plugins.rst design document.patch
  patch 0012/configure add enable plugins MOVE TO END.patch
  patch 0015/plugin add implementation of the api.patch
  patch 0024/plugins implement helpers for resolving hwaddr.patch
  patch 0047/tests tcg enable plugin testing.patch
  patch 0048/tests plugin add a hotblocks plugin.patch
  patch 0050/tests plugin add instruction execution breakdown.patch
  patch 0051/tests plugin add hotpages plugin to breakdown mem.patch
  patch 0053/include exec wrap cpu_ldst.h in CONFIG_TCG.patch

Alex Bennée (18):
  target/arm: handle M-profile semihosting at translate time
  target/arm: handle A-profile T32 semihosting at translate time
  target/arm: handle A-profile A32 semihosting at translate time
  target/arm: remove run time semihosting checks
  includes: remove stale [smp|max]_cpus externs
  trace: add mmu_index to mem_info
  docs/devel: add plugins.rst design document
  configure: add --enable-plugins (MOVE TO END)
  plugin: add implementation of the api
  plugins: implement helpers for resolving hwaddr
  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

Emilio G. Cota (34):
  trace: expand mem_info:size_shift to 4 bits
  tcg/README: fix typo s/afterwise/afterwards/
  cpu: introduce cpu_in_exclusive_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
  tcg: add tcg_gen_st_ptr
  plugin-gen: add module for TCG-related code
  atomic_template: fix indentation in GEN_ATOMIC_HELPER
  atomic_template: add inline trace/plugin helpers
  tcg: let plugins instrument virtual 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: fetch code with translator_ld
  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                                  |  10 +-
 Makefile.target                           |   2 +
 accel/stubs/tcg-stub.c                    |   3 -
 accel/tcg/Makefile.objs                   |   1 +
 accel/tcg/atomic_common.inc.c             |  54 ++
 accel/tcg/atomic_template.h               |  96 ++-
 accel/tcg/cpu-exec.c                      |   8 +-
 accel/tcg/cputlb.c                        |  69 +-
 accel/tcg/plugin-gen.c                    | 977 ++++++++++++++++++++++
 accel/tcg/plugin-helpers.h                |   5 +
 accel/tcg/translate-all.c                 |  15 +-
 accel/tcg/translator.c                    |  20 +
 accel/tcg/user-exec.c                     |   3 +
 bsd-user/syscall.c                        |  24 +-
 configure                                 |  86 +-
 cpus-common.c                             |   4 +
 cpus.c                                    |  10 +
 disas.c                                   | 103 +++
 docs/devel/index.rst                      |   1 +
 docs/devel/plugins.rst                    | 107 +++
 exec.c                                    |   2 +
 include/disas/disas.h                     |   2 +
 include/exec/cpu-defs.h                   |   3 +-
 include/exec/cpu_ldst.h                   |  11 +
 include/exec/cpu_ldst_template.h          |  37 +-
 include/exec/cpu_ldst_useronly_template.h |  29 +-
 include/exec/exec-all.h                   |  81 +-
 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                 |  58 +-
 include/qemu/bswap.h                      |   5 +
 include/qemu/plugin.h                     | 261 ++++++
 include/qemu/qemu-plugin.h                | 360 ++++++++
 include/qemu/queue.h                      |  10 +
 include/qom/cpu.h                         |  19 +
 include/sysemu/sysemu.h                   |   2 -
 include/user/syscall-trace.h              |  40 +
 linux-user/exit.c                         |   1 +
 linux-user/main.c                         |  18 +
 linux-user/syscall.c                      |   7 +-
 plugins/.gitignore                        |   2 +
 plugins/Makefile.objs                     |  21 +
 plugins/api.c                             | 338 ++++++++
 plugins/core.c                            | 499 +++++++++++
 plugins/loader.c                          | 353 ++++++++
 plugins/plugin.h                          |  95 +++
 plugins/qemu-plugins.symbols              |  39 +
 qemu-options.hx                           |  17 +
 qom/cpu.c                                 |   2 +
 scripts/tracetool/transform.py            |   1 +
 target/alpha/translate.c                  |   2 +-
 target/arm/arm_ldst.h                     |  15 +-
 target/arm/helper.c                       |  90 +-
 target/arm/m_helper.c                     |  18 +-
 target/arm/translate.c                    |  64 +-
 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/tcg-op.c                              |  40 +-
 tcg/tcg-op.h                              |  16 +
 tcg/tcg-opc.h                             |   3 +
 tcg/tcg.c                                 |  22 +
 tcg/tcg.h                                 |  23 +
 tests/Makefile.include                    |  10 +-
 tests/plugin/Makefile                     |  31 +
 tests/plugin/bb.c                         |  65 ++
 tests/plugin/empty.c                      |  29 +
 tests/plugin/hotblocks.c                  | 146 ++++
 tests/plugin/hotpages.c                   | 179 ++++
 tests/plugin/howvec.c                     | 301 +++++++
 tests/plugin/insn.c                       |  62 ++
 tests/plugin/mem.c                        |  96 +++
 tests/tcg/Makefile                        |  34 +
 tests/tcg/arm/Makefile.softmmu-target     |   1 +
 trace-events                              |   8 +-
 trace/mem-internal.h                      |  31 +-
 trace/mem.h                               |   7 +-
 vl.c                                      |  11 +
 87 files changed, 5067 insertions(+), 260 deletions(-)
 create mode 100644 accel/tcg/atomic_common.inc.c
 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 include/user/syscall-trace.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
Re: [Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by no-reply@patchew.org 4 years, 8 months ago 
Patchew URL: https://patchew.org/QEMU/20190731160719.11396-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/cpus.o
  CC      x86_64-softmmu/gdbstub.o
  CC      x86_64-softmmu/balloon.o
/tmp/qemu-test/src/disas.c:503:5: error: use of undeclared identifier 'csh'
    csh handle;
    ^
/tmp/qemu-test/src/disas.c:504:5: error: use of undeclared identifier 'cs_insn'
    cs_insn *insn;
    ^
/tmp/qemu-test/src/disas.c:504:14: error: use of undeclared identifier 'insn'; did you mean 'info'?
    cs_insn *insn;
             ^~~~
             info
/tmp/qemu-test/src/disas.c:500:46: note: 'info' declared here
bool plugin_cap_disas_insn(disassemble_info *info, uint64_t pc, size_t size)
                                             ^
/tmp/qemu-test/src/disas.c:509: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:509:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/tmp/qemu-test/src/disas.c:509:32: error: use of undeclared identifier 'handle'
    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
                               ^
/tmp/qemu-test/src/disas.c:509:43: error: use of undeclared identifier 'CS_ERR_OK'
    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
                                          ^
/tmp/qemu-test/src/disas.c:512:5: error: use of undeclared identifier 'insn'
    insn = cap_insn;
    ^
/tmp/qemu-test/src/disas.c:512:12: error: use of undeclared identifier 'cap_insn'
    insn = cap_insn;
           ^
/tmp/qemu-test/src/disas.c:518: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:518:13: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/tmp/qemu-test/src/disas.c:518:23: error: use of undeclared identifier 'handle'
    count = cs_disasm(handle, cbuf, size, 0, 1, &insn);
                      ^
/tmp/qemu-test/src/disas.c:518:50: error: use of undeclared identifier 'insn'
    count = cs_disasm(handle, cbuf, size, 0, 1, &insn);
                                                 ^
/tmp/qemu-test/src/disas.c:521:37: error: use of undeclared identifier 'insn'
        g_string_printf(s, "%s %s", insn->mnemonic, insn->op_str);
                                    ^
/tmp/qemu-test/src/disas.c:521:53: error: use of undeclared identifier 'insn'
        g_string_printf(s, "%s %s", insn->mnemonic, insn->op_str);
                                                    ^
/tmp/qemu-test/src/disas.c:526: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:526:5: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/tmp/qemu-test/src/disas.c:526:15: error: use of undeclared identifier 'handle'
    cs_close(&handle);
              ^
18 errors generated.


The full log is available at
http://patchew.org/logs/20190731160719.11396-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
Re: [Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by no-reply@patchew.org 4 years, 8 months ago 
Patchew URL: https://patchew.org/QEMU/20190731160719.11396-1-alex.bennee@linaro.org/



Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH  v4 00/54] plugins for TCG
Message-id: 20190731160719.11396-1-alex.bennee@linaro.org

=== 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
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20190731075652.17053-1-thuth@redhat.com -> patchew/20190731075652.17053-1-thuth@redhat.com
 * [new tag]         patchew/20190731160719.11396-1-alex.bennee@linaro.org -> patchew/20190731160719.11396-1-alex.bennee@linaro.org
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' (https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out '20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' (https://github.com/openssl/openssl) registered for path 'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out '50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out '09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out 'c79e0ecb84f4f1ee3f73f521622e264edd1bf174'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/opensbi'...
Submodule path 'roms/opensbi': checked out 'ce228ee0919deb9957192d723eecc8aaae2697c6'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out 'bf0e13698872450164fa7040da36a95d2d4b326f'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a5cab58e9a3fb6e168aba919c5669bea406573b4'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '0f4fe84658165e96ce35870fd19fc634e182e77b'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out '261ca8e779e5138869a45f174caa49be6a274501'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd3689267f92c5956e09cc7d1baa4700141662bff'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'slirp'...
Submodule path 'slirp': checked out 'f0da6726207b740f6101028b2992f918477a4b08'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
4618434 include/exec/cpu-defs.h: fix typo
d260302 include/exec: wrap cpu_ldst.h in CONFIG_TCG
4a08e81 accel/stubs: reduce headers from tcg-stub
dc047a8 tests/plugin: add hotpages plugin to breakdown memory access patterns
0031177 tests/plugin: add instruction execution breakdown
bd83073 plugin: add qemu_plugin_insn_disas helper
719f27a tests/plugin: add a hotblocks plugin
0c2193f tests/tcg: enable plugin testing
4fda032 tests/plugin: add sample plugins
fd7f586 linux-user: support -plugin option
b484c3f vl: support -plugin option
720fb2d plugin: add API symbols to qemu-plugins.symbols
75048c5 translator: inject instrumentation from plugins
90d92af target/openrisc: fetch code with translator_ld
bf3c443 target/xtensa: fetch code with translator_ld
1b3b92b target/sparc: fetch code with translator_ld
f41eb29 target/riscv: fetch code with translator_ld
35e338c target/alpha: fetch code with translator_ld
9014aab target/m68k: fetch code with translator_ld
604625d target/hppa: fetch code with translator_ld
f86b279 target/i386: fetch code with translator_ld
5bfcc20 target/sh4: fetch code with translator_ld
65656f9 target/ppc: fetch code with translator_ld
1338aac target/arm: fetch code with translator_ld
6ad7980 translator: add translator_ld{ub, sw, uw, l, q}
123a7b0 plugin-gen: add plugin_insn_append
2fc0fb1 cpu: hook plugin vcpu events
2ad986c *-user: plugin syscalls
12cf148 *-user: notify plugin of exit
5ef3c3d translate-all: notify plugin code of tb_flush
ceffbf5 plugins: implement helpers for resolving hwaddr
bfd0bef tcg: let plugins instrument virtual memory accesses
8c70962 atomic_template: add inline trace/plugin helpers
b6d0321 atomic_template: fix indentation in GEN_ATOMIC_HELPER
59e11a6 plugin-gen: add module for TCG-related code
b7ff8fb tcg: add tcg_gen_st_ptr
57dce44 cputlb: introduce get_page_addr_code_hostp
c21fd8d cputlb: document get_page_addr_code
0156ea7 queue: add QTAILQ_REMOVE_SEVERAL
98bb420 plugin: add implementation of the api
90dff04 plugin: add core code
dfc8157 plugin: add user-facing API
c508aec configure: add --enable-plugins (MOVE TO END)
efd9ca8 docs/devel: add plugins.rst design document
7e445c2 translate-all: use cpu_in_exclusive_work_context() in tb_flush
e3927dd cpu: introduce cpu_in_exclusive_context()
2c1c22b tcg/README: fix typo s/afterwise/afterwards/
0082b05 trace: add mmu_index to mem_info
43a9cd6 trace: expand mem_info:size_shift to 4 bits
92f8098 includes: remove stale [smp|max]_cpus externs
b12d00f target/arm: remove run time semihosting checks
3964cfe target/arm: handle A-profile A32 semihosting at translate time
354ccbf target/arm: handle A-profile T32 semihosting at translate time
c736511 target/arm: handle M-profile semihosting at translate time

=== OUTPUT BEGIN ===
1/54 Checking commit c736511afb73 (target/arm: handle M-profile semihosting at translate time)
2/54 Checking commit 354ccbf7836a (target/arm: handle A-profile T32 semihosting at translate time)
3/54 Checking commit 3964cfed151e (target/arm: handle A-profile A32 semihosting at translate time)
4/54 Checking commit b12d00f866ff (target/arm: remove run time semihosting checks)
5/54 Checking commit 92f80985c6b7 (includes: remove stale [smp|max]_cpus externs)
6/54 Checking commit 43a9cd64afe9 (trace: expand mem_info:size_shift to 4 bits)
7/54 Checking commit 0082b05e9d82 (trace: add mmu_index to mem_info)
ERROR: line over 90 characters
#24: FILE: accel/tcg/atomic_template.h:63:
+        uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, false, ATOMIC_MMU_IDX); \

ERROR: line over 90 characters
#33: FILE: accel/tcg/atomic_template.h:71:
+        uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, false, ATOMIC_MMU_IDX); \

ERROR: line over 90 characters
#40: FILE: accel/tcg/atomic_template.h:77:
+        uint16_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, true, ATOMIC_MMU_IDX); \

WARNING: line over 80 characters
#321: FILE: trace/mem.h:21:
+static uint16_t trace_mem_get_info(TCGMemOp op, unsigned int mmu_idx, bool store);

total: 3 errors, 1 warnings, 261 lines checked

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

8/54 Checking commit 2c1c22bc16e9 (tcg/README: fix typo s/afterwise/afterwards/)
9/54 Checking commit e3927dda7720 (cpu: introduce cpu_in_exclusive_context())
10/54 Checking commit 7e445c2738b4 (translate-all: use cpu_in_exclusive_work_context() in tb_flush)
11/54 Checking commit efd9ca87d2ad (docs/devel: add plugins.rst design document)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#24: 
new file mode 100644

total: 0 errors, 1 warnings, 111 lines checked

Patch 11/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/54 Checking commit c508aec11118 (configure: add --enable-plugins (MOVE TO END))
13/54 Checking commit dfc81576c19a (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, 351 lines checked

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

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

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

WARNING: line over 80 characters
#618: 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
#909: FILE: plugins/loader.c:42:
+        { /* end of list */ }

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

total: 3 errors, 3 warnings, 1261 lines checked

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

15/54 Checking commit 98bb4205f8d1 (plugin: add implementation of the api)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100644

ERROR: if this code is redundant consider removing it
#282: FILE: plugins/api.c:256:
+#if 0 /* XXX FIXME should be SOFTMMU */

ERROR: "foo * bar" should be "foo *bar"
#303: FILE: plugins/api.c:277:
+static MachineState * get_ms(void)

total: 2 errors, 1 warnings, 303 lines checked

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

16/54 Checking commit 0156ea779e67 (queue: add QTAILQ_REMOVE_SEVERAL)
WARNING: Block comments use a leading /* on a separate line
#31: FILE: include/qemu/queue.h:433:
+    } while (/*CONSTCOND*/0)

total: 0 errors, 1 warnings, 16 lines checked

Patch 16/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/54 Checking commit c21fd8dfb8e0 (cputlb: document get_page_addr_code)
18/54 Checking commit 57dce445a292 (cputlb: introduce get_page_addr_code_hostp)
19/54 Checking commit b7ff8fb737a6 (tcg: add tcg_gen_st_ptr)
20/54 Checking commit 59e11a6c6781 (plugin-gen: add module for TCG-related code)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#48: 
new file mode 100644

ERROR: if this code is redundant consider removing it
#420: FILE: accel/tcg/plugin-gen.c:368:
+#if 0 /* XXX: no longer needed? */

total: 1 errors, 1 warnings, 1168 lines checked

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

21/54 Checking commit b6d032138a56 (atomic_template: fix indentation in GEN_ATOMIC_HELPER)
22/54 Checking commit 8c70962547d5 (atomic_template: add inline trace/plugin helpers)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#15: 
new file mode 100644

total: 0 errors, 1 warnings, 296 lines checked

Patch 22/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
23/54 Checking commit bfd0bef6abd9 (tcg: let plugins instrument virtual memory accesses)
WARNING: line over 80 characters
#171: FILE: include/exec/cpu_ldst_template.h:91:
+    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, false, mmu_idx);

total: 0 errors, 1 warnings, 372 lines checked

Patch 23/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
24/54 Checking commit ceffbf57661b (plugins: implement helpers for resolving hwaddr)
25/54 Checking commit 5ef3c3d45dd0 (translate-all: notify plugin code of tb_flush)
26/54 Checking commit 12cf1480ae24 (*-user: notify plugin of exit)
27/54 Checking commit 2ad986cc69bd (*-user: plugin syscalls)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#93: 
new file mode 100644

total: 0 errors, 1 warnings, 127 lines checked

Patch 27/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
28/54 Checking commit 2fc0fb11c7c5 (cpu: hook plugin vcpu events)
29/54 Checking commit 123a7b0c3078 (plugin-gen: add plugin_insn_append)
30/54 Checking commit 6ad7980f6247 (translator: add translator_ld{ub, sw, uw, l, q})
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#76: FILE: include/exec/translator.h:161:
+#define DO_LOAD(type, name, shift)               \
+    set_helper_retaddr(1);                       \
+    ret = name ## _p(g2h(pc));                   \
+    clear_helper_retaddr();

WARNING: Block comments use a leading /* on a separate line
#109: FILE: include/exec/translator.h:194:
+GEN_TRANSLATOR_LD(translator_ldub, ldb, uint8_t, 1, /* no swap needed */)

total: 1 errors, 1 warnings, 116 lines checked

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

31/54 Checking commit 1338aac82722 (target/arm: fetch code with translator_ld)
32/54 Checking commit 65656f9c3eea (target/ppc: fetch code with translator_ld)
33/54 Checking commit 5bfcc20d11c6 (target/sh4: fetch code with translator_ld)
34/54 Checking commit f86b27916216 (target/i386: fetch code with translator_ld)
35/54 Checking commit 604625d4780e (target/hppa: fetch code with translator_ld)
36/54 Checking commit 9014aab24680 (target/m68k: fetch code with translator_ld)
37/54 Checking commit 35e338c60b8f (target/alpha: fetch code with translator_ld)
38/54 Checking commit f41eb298b626 (target/riscv: fetch code with translator_ld)
39/54 Checking commit 1b3b92b10053 (target/sparc: fetch code with translator_ld)
40/54 Checking commit bf3c443ec239 (target/xtensa: fetch code with translator_ld)
41/54 Checking commit 90d92af84114 (target/openrisc: fetch code with translator_ld)
42/54 Checking commit 75048c56cd83 (translator: inject instrumentation from plugins)
43/54 Checking commit 720fb2d03f34 (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, 151 lines checked

Patch 43/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
44/54 Checking commit b484c3fd133c (vl: support -plugin option)
45/54 Checking commit fd7f58630fad (linux-user: support -plugin option)
46/54 Checking commit 4fda03223154 (tests/plugin: add sample plugins)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 296 lines checked

Patch 46/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
47/54 Checking commit 0c2193fd7aa2 (tests/tcg: enable plugin testing)
48/54 Checking commit 719f27a8e85d (tests/plugin: add a hotblocks plugin)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

total: 0 errors, 1 warnings, 153 lines checked

Patch 48/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
49/54 Checking commit bd8307360460 (plugin: add qemu_plugin_insn_disas helper)
ERROR: "foo * bar" should be "foo *bar"
#74: FILE: disas.c:530:
+char * plugin_disas(CPUState *cpu, uint64_t addr, size_t size)

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

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

total: 3 errors, 0 warnings, 158 lines checked

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

50/54 Checking commit 0031177b24bf (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},
                             ^

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

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

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

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

WARNING: line over 80 characters
#246: FILE: tests/plugin/howvec.c:216:
+                                                       GUINT_TO_POINTER(opcode));

total: 8 errors, 4 warnings, 308 lines checked

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

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

total: 0 errors, 1 warnings, 186 lines checked

Patch 51/54 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
52/54 Checking commit 4a08e81629be (accel/stubs: reduce headers from tcg-stub)
53/54 Checking commit d260302d62f5 (include/exec: wrap cpu_ldst.h in CONFIG_TCG)
54/54 Checking commit 4618434e2366 (include/exec/cpu-defs.h: fix typo)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190731160719.11396-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
Re: [Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by Markus Armbruster 4 years, 8 months ago 
Alex Bennée <alex.bennee@linaro.org> writes:

> Hi,
>
> This is the latest iteration of the plugins series. The main changes
> from the last version are:
>
>   - dropped passing of haddr to plugins
>
> This makes the code for handling the plugins less invasive in the
> softmmu path at the cost of offloading processing to the plugin if it
> wants the value. We rely on the fact that the TLB is per vCPU so a
> helper can just trigger a re-query of the TLB to get the final
> address.
>
> Part of that change involved embedding the MMU index in the meminfo
> field for tracing. I see there are some other patches on the list for
> messing with TCGMemOp so there might be a clash coming up.
>
>   - translator_ld goes direct to softmmu/user functions
>
> I also mark the [SOFTMMU_]CODE_ACCESS helpers as deprecated. There is
> more work to be done to clean up all the current uses of code access
> helpers but ideally the only thing that should be peaking at code is
> the translator loop itself. However a bunch of helpers have taken to
> using code loading functions to peak at the instruction just executed
> to figure out what to do. Once those have been fixed then we can
> remove those helpers.
>
> Other more minor fixes can be found documented bellow the --- in the
> individual patches.
>
> This series also includes the semihosting patches as they are a
> pre-requisite for the translator_ld patches for ARM.
>
> Once the tree opens up for development again I hope to get the
> semihosting and trivial clean-up patches merged quickly so the patch
> count for the plugins patches proper can be reduced a bit.

Next time, please explain briefly what TCG plugins are about right in
your cover letter.  I had to go hunting for this.  Found "[PATCH v4
11/54] docs/devel: add plugins.rst design document".

Please advise why TCG plugins don't undermine the GPL.  Any proposal to
add a plugin interface needs to do that.
Re: [Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by Alex Bennée 4 years, 7 months ago 
Markus Armbruster <armbru@redhat.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Hi,
>>
>> This is the latest iteration of the plugins series. The main changes
>> from the last version are:
>>
>>   - dropped passing of haddr to plugins
>>
>> This makes the code for handling the plugins less invasive in the
>> softmmu path at the cost of offloading processing to the plugin if it
>> wants the value. We rely on the fact that the TLB is per vCPU so a
>> helper can just trigger a re-query of the TLB to get the final
>> address.
>>
>> Part of that change involved embedding the MMU index in the meminfo
>> field for tracing. I see there are some other patches on the list for
>> messing with TCGMemOp so there might be a clash coming up.
>>
>>   - translator_ld goes direct to softmmu/user functions
>>
>> I also mark the [SOFTMMU_]CODE_ACCESS helpers as deprecated. There is
>> more work to be done to clean up all the current uses of code access
>> helpers but ideally the only thing that should be peaking at code is
>> the translator loop itself. However a bunch of helpers have taken to
>> using code loading functions to peak at the instruction just executed
>> to figure out what to do. Once those have been fixed then we can
>> remove those helpers.
>>
>> Other more minor fixes can be found documented bellow the --- in the
>> individual patches.
>>
>> This series also includes the semihosting patches as they are a
>> pre-requisite for the translator_ld patches for ARM.
>>
>> Once the tree opens up for development again I hope to get the
>> semihosting and trivial clean-up patches merged quickly so the patch
>> count for the plugins patches proper can be reduced a bit.
>
> Next time, please explain briefly what TCG plugins are about right in
> your cover letter.  I had to go hunting for this.  Found "[PATCH v4
> 11/54] docs/devel: add plugins.rst design document".

I'll provide a better overview in my next cover letter.

> Please advise why TCG plugins don't undermine the GPL.  Any proposal to
> add a plugin interface needs to do that.

I'm not sure what we can say about this apart from "ask your lawyer".
I'm certainly not proposing we add any sort of language about what
should and shouldn't be allowed to use the plugin interface. I find it
hard to see how anyone could argue code written to interface with the
plugin API couldn't be considered a derived work.

There are two use cases I have in mind:

The first is FLOSS developers writing interesting tools that can take
advantage of QEMU's control of the system to do experiments that are
tricky with other setups (Valgrind is limited to same-arch, Dynamo/Pin
are user-space only). I want these experiments to be easy to do without
having to keep hacking and re-hacking QEMU's core code. I would hope
QEMU developers would up-stream theirs into the QEMU source tree but I
can imagine academics will have open source code that will only ever sit
in their paper's repository.

The other is users who currently maintain hacked up internal copies of
QEMU as a test bed for whatever piece of silicon they are brewing behind
closed doors. This code would never be distributed (hence never be a GPL
issue) and is generally kept private because it's IP sensitive
(e.g: experimenting with different cache models). If we can provide an
interface that allows them to keep their experiments private and
separate from changes to the core code then maybe apart from making
their lives a bit easier we will see some non-IP sensitive contributions
come back to the upstream. I live in hope ;-)

--
Alex Bennée
Re: [Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by Aaron Lindsay OS via Qemu-devel 4 years, 7 months ago 
On Sep 06 20:52, Alex Bennée wrote:
> 
> Markus Armbruster <armbru@redhat.com> writes:
> > Please advise why TCG plugins don't undermine the GPL.  Any proposal to
> > add a plugin interface needs to do that.
> 
> I'm not sure what we can say about this apart from "ask your lawyer".
> I'm certainly not proposing we add any sort of language about what
> should and shouldn't be allowed to use the plugin interface. I find it
> hard to see how anyone could argue code written to interface with the
> plugin API couldn't be considered a derived work.

I am not a lawyer, but I would not have expected software merely using a
well-defined API to be considered a derivative work of the software
defining it. Unless, of course, it is a derivative work of another
plugin using the same interface in a way that is not necessitated by the
structure of the API.

What's your reasoning for why it would be a derivative work? Is your
belief that the plugin API is complex enough that anything using it has
to be a derivative work, or something else?

That said, I'm not sure I understand in what way adding a plugin
interface would undermine the GPL, so maybe I'm missing the point.

-Aaron
Re: [Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by Alex Bennée 4 years, 7 months ago 
Aaron Lindsay OS <aaron@os.amperecomputing.com> writes:

> On Sep 06 20:52, Alex Bennée wrote:
>>
>> Markus Armbruster <armbru@redhat.com> writes:
>> > Please advise why TCG plugins don't undermine the GPL.  Any proposal to
>> > add a plugin interface needs to do that.
>>
>> I'm not sure what we can say about this apart from "ask your lawyer".
>> I'm certainly not proposing we add any sort of language about what
>> should and shouldn't be allowed to use the plugin interface. I find it
>> hard to see how anyone could argue code written to interface with the
>> plugin API couldn't be considered a derived work.
>
> I am not a lawyer, but I would not have expected software merely using a
> well-defined API to be considered a derivative work of the software
> defining it. Unless, of course, it is a derivative work of another
> plugin using the same interface in a way that is not necessitated by the
> structure of the API.
>
> What's your reasoning for why it would be a derivative work? Is your
> belief that the plugin API is complex enough that anything using it has
> to be a derivative work, or something else?

Well it's derivative if the code couldn't have been written on it's own
- so is "derived" from a design in GPL code. The endless arguments about
derivation w.r.t the nvidia kernel drivers hinge on the fact the drivers
are multi-OS and shimmed to into the kernel - not explicitly written for
Linux. However no one really knows where the line is because it's the
courts that will ultimately decide.

OTOH the kernel has explicit language about the syscall layer:

   NOTE! This copyright does *not* cover user programs that use kernel
 services by normal system calls - this is merely considered normal use
 of the kernel, and does *not* fall under the heading of "derived work".

Anyway I don't really care either way and don't intend to become an arm
chair lawyer as the arguments can become rather circular and tedious. We
might want to make an explicit statement in the QEMU tree about
reserving the right to change the API in the future and the best way to
keep plugins in sync is to have them in the upstream source tree.

> That said, I'm not sure I understand in what way adding a plugin
> interface would undermine the GPL, so maybe I'm missing the point.

I suspect there is a similar concern as GCC had when they were asked to
expose their IR to tools. They were explicitly concerned that people
would ship proprietary compilers consisting of a GPL GCC shell with all
the clever optimisations in proprietary plugins that glued the front and
backends together. I don't think this is a problem for QEMU because at
the moment plugins can't be used to improve code generation - just
observe it. You could theoretically implement device models this way but
there are definitely easier more efficient ways of doing it - as shown
by other 3rd party forks of QEMU.

As I mentioned in the other message I'm sure there will be proprietary
- or rather secret behind closed doors plugins but I don't view them any
differently to all the various secret and external forks of the main
QEMU source.

>
> -Aaron


--
Alex Bennée
Re: [Qemu-devel] [PATCH v4 00/54] plugins for TCG
Posted by Peter Maydell 4 years, 7 months ago 
On Fri, 6 Sep 2019 at 20:53, Alex Bennée <alex.bennee@linaro.org> wrote:
> Markus Armbruster <armbru@redhat.com> writes:
>
> > Please advise why TCG plugins don't undermine the GPL.  Any proposal to
> > add a plugin interface needs to do that.
>
> I'm not sure what we can say about this apart from "ask your lawyer".

I suspect the underlying confusion here is "the term 'plugin'
has a very wide scope and you and Markus are probably not on
the same page about what this API/ABI is actually doing".

Specifically, this is not the 'plugin ABI that lets you write
device models out of tree' that is sometimes mooted, and the ABI
does not (unless it's changed since I last looked) expose arbitrary
bits of QEMU internals to the plugin or let plugins make calls to
random QEMU functions. It's a very limited scope ABI that allows
QEMU users to write code that can do useful introspection on what
running guest code is doing without having to modify QEMU itself
in non-sustainable non-upstreamable ways.
[If it doesn't have that kind of protection against misuse
then we should add it.]

More generally, your cover letter really needs to be much
more descriptive about what we're trying to do here, and
what the purpose, limitations, etc are here. If you say
"plugins" without giving any detail then you're going to
trigger a lot of (reasonable) reactions from people who
associate that word with a much more generalized "provide
arbitrary bits of QEMU functionality as 3rd-party blobs"
concept...

thanks
-- PMM
[Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Markus Armbruster 4 years, 7 months ago 
Alex Bennée <alex.bennee@linaro.org> writes:

> Markus Armbruster <armbru@redhat.com> writes:
[...]
>> Please advise why TCG plugins don't undermine the GPL.  Any proposal to
>> add a plugin interface needs to do that.
>
> I'm not sure what we can say about this apart from "ask your lawyer".

I'm not asking for a legal argument, I'm asking for a pragmatic one.

> I'm certainly not proposing we add any sort of language about what
> should and shouldn't be allowed to use the plugin interface. I find it
> hard to see how anyone could argue code written to interface with the
> plugin API couldn't be considered a derived work.

What makes that so?  Is writing a plugin without linking with QEMU code
impractical?

> There are two use cases I have in mind:
>
> The first is FLOSS developers writing interesting tools that can take
> advantage of QEMU's control of the system to do experiments that are
> tricky with other setups (Valgrind is limited to same-arch, Dynamo/Pin
> are user-space only). I want these experiments to be easy to do without
> having to keep hacking and re-hacking QEMU's core code. I would hope
> QEMU developers would up-stream theirs into the QEMU source tree but I
> can imagine academics will have open source code that will only ever sit
> in their paper's repository.

GPL'ed code that's not for upstream is 100% legitimate.

> The other is users who currently maintain hacked up internal copies of
> QEMU as a test bed for whatever piece of silicon they are brewing behind
> closed doors. This code would never be distributed (hence never be a GPL
> issue)

Correct.  We can't force anybody to distribute, and that's only proper.

>        and is generally kept private because it's IP sensitive
> (e.g: experimenting with different cache models). If we can provide an
> interface that allows them to keep their experiments private and
> separate from changes to the core code then maybe apart from making
> their lives a bit easier we will see some non-IP sensitive contributions
> come back to the upstream. I live in hope ;-)

I'm concerned about a third case: imlementing stuff as a plugin so you
can distribute it with a GPL-incompatible license.  Particularly
pernicious when that stuff could be useful upstream.

Are there any technical difficulties that could make distributing a
plugins in binary form impractical?
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Alex Bennée 4 years, 7 months ago 
Markus Armbruster <armbru@redhat.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Markus Armbruster <armbru@redhat.com> writes:
> [...]
>>> Please advise why TCG plugins don't undermine the GPL.  Any proposal to
>>> add a plugin interface needs to do that.
>>
>> I'm not sure what we can say about this apart from "ask your lawyer".
>
> I'm not asking for a legal argument, I'm asking for a pragmatic one.
>
>> I'm certainly not proposing we add any sort of language about what
>> should and shouldn't be allowed to use the plugin interface. I find it
>> hard to see how anyone could argue code written to interface with the
>> plugin API couldn't be considered a derived work.
>
> What makes that so?  Is writing a plugin without linking with QEMU code
> impractical?

The way a plugin works is by linking. The plugin itself would be useless
unless combined with the QEMU code to do its thing. It is a more
intimate binding than an IPC interface using some sort of protocol. The
argument goes that pretty much any kernel module is derived code - but
again it has never really been litigated in the courts which would be
the people to set the precedent.

>
>> There are two use cases I have in mind:
>>
>> The first is FLOSS developers writing interesting tools that can take
>> advantage of QEMU's control of the system to do experiments that are
>> tricky with other setups (Valgrind is limited to same-arch, Dynamo/Pin
>> are user-space only). I want these experiments to be easy to do without
>> having to keep hacking and re-hacking QEMU's core code. I would hope
>> QEMU developers would up-stream theirs into the QEMU source tree but I
>> can imagine academics will have open source code that will only ever sit
>> in their paper's repository.
>
> GPL'ed code that's not for upstream is 100% legitimate.
>
>> The other is users who currently maintain hacked up internal copies of
>> QEMU as a test bed for whatever piece of silicon they are brewing behind
>> closed doors. This code would never be distributed (hence never be a GPL
>> issue)
>
> Correct.  We can't force anybody to distribute, and that's only proper.
>
>>        and is generally kept private because it's IP sensitive
>> (e.g: experimenting with different cache models). If we can provide an
>> interface that allows them to keep their experiments private and
>> separate from changes to the core code then maybe apart from making
>> their lives a bit easier we will see some non-IP sensitive contributions
>> come back to the upstream. I live in hope ;-)
>
> I'm concerned about a third case: imlementing stuff as a plugin so you
> can distribute it with a GPL-incompatible license.  Particularly
> pernicious when that stuff could be useful upstream.

If someone were to do that it would depend on a copyright holder (i.e.
one of us) being willing to challenge that licensing. AIUI GCC used
additional language in the runtime exception clause:

  https://www.gnu.org/licenses/gcc-exception-3.1.html

which only allows use of the runtime exception if the code has gone
through GPL compatible code:

  A Compilation Process is "Eligible" if it is done using GCC, alone or
  with other GPL-compatible software, or if it is done without using any
  work based on GCC. For example, using non-GPL-compatible Software to
  optimize any GCC intermediate representations would not qualify as an
  Eligible Compilation Process.

> Are there any technical difficulties that could make distributing a
> plugins in binary form impractical?

Well the first thing will be we are not intending to offer a guaranteed
ABI. While we don't want to be changing it at a whim there shouldn't be
an expectation that the plugin interface will maintain backwards
compatibility (unlike the command line interface ;-). There should be an
expectation that plugins will likely need to be rebuilt against the
current source tree from time to time.

We could implement a more technical measure analogous to the kernels
module signing that would require the plugin to be rebuilt with
reference to the current QEMU source tree although that will be a pain
even for internally distributed blobs. I'm loathed to implement such a
system from v1 though given the problem of publicly distributed binary
blobs is currently only a theoretical problem.

--
Alex Bennée
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Peter Maydell 4 years, 7 months ago 
On Thu, 12 Sep 2019 at 10:03, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Well the first thing will be we are not intending to offer a guaranteed
> ABI. While we don't want to be changing it at a whim there shouldn't be
> an expectation that the plugin interface will maintain backwards
> compatibility (unlike the command line interface ;-). There should be an
> expectation that plugins will likely need to be rebuilt against the
> current source tree from time to time.

Wait, what? From my perspective the whole point of the plugin
interface is that it should be stable, in that at least there's
a good chance that a plugin you built will work against multiple
versions of QEMU, and if it doesn't then it should fail with
a reasonable error message telling you to update. I'm not
sure we should be landing the plugins infrastructure if we
don't have that much stability.

thanks
-- PMM
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Alex Bennée 4 years, 7 months ago 
Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 12 Sep 2019 at 10:03, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Well the first thing will be we are not intending to offer a guaranteed
>> ABI. While we don't want to be changing it at a whim there shouldn't be
>> an expectation that the plugin interface will maintain backwards
>> compatibility (unlike the command line interface ;-). There should be an
>> expectation that plugins will likely need to be rebuilt against the
>> current source tree from time to time.
>
> Wait, what? From my perspective the whole point of the plugin
> interface is that it should be stable, in that at least there's
> a good chance that a plugin you built will work against multiple
> versions of QEMU, and if it doesn't then it should fail with
> a reasonable error message telling you to update. I'm not
> sure we should be landing the plugins infrastructure if we
> don't have that much stability.

There is a big fat blurry line between "set in stone" and "not requiring
you to re-engineer the plugin every QEMU release". I'm saying we should
reserve the right to extend and change the plugin API as required but
the expectation would be the plugins will continue to work the same way
but maybe with tweaks to the API hooks to support additional features.

It's also a pretty young interface so I would expect some evolution once
it is released into the field.

One problem with the anti-license circumvention measures being suggested
is it will mean having to recompile plugins for any given release. This
isn't a problem for plugins we write but it does add an extra step for
out of tree plugins. Maybe being forced to re-compile (but not change
the source) is a hurdle we are willing to accept?

--
Alex Bennée
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Daniel P. Berrangé 4 years, 7 months ago 
On Thu, Sep 12, 2019 at 11:07:07AM +0100, Alex Bennée wrote:
> 
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
> > On Thu, 12 Sep 2019 at 10:03, Alex Bennée <alex.bennee@linaro.org> wrote:
> >>
> >> Well the first thing will be we are not intending to offer a guaranteed
> >> ABI. While we don't want to be changing it at a whim there shouldn't be
> >> an expectation that the plugin interface will maintain backwards
> >> compatibility (unlike the command line interface ;-). There should be an
> >> expectation that plugins will likely need to be rebuilt against the
> >> current source tree from time to time.
> >
> > Wait, what? From my perspective the whole point of the plugin
> > interface is that it should be stable, in that at least there's
> > a good chance that a plugin you built will work against multiple
> > versions of QEMU, and if it doesn't then it should fail with
> > a reasonable error message telling you to update. I'm not
> > sure we should be landing the plugins infrastructure if we
> > don't have that much stability.
> 
> There is a big fat blurry line between "set in stone" and "not requiring
> you to re-engineer the plugin every QEMU release". I'm saying we should
> reserve the right to extend and change the plugin API as required but
> the expectation would be the plugins will continue to work the same way
> but maybe with tweaks to the API hooks to support additional features.
> 
> It's also a pretty young interface so I would expect some evolution once
> it is released into the field.
> 
> One problem with the anti-license circumvention measures being suggested
> is it will mean having to recompile plugins for any given release. This
> isn't a problem for plugins we write but it does add an extra step for
> out of tree plugins. Maybe being forced to re-compile (but not change
> the source) is a hurdle we are willing to accept?

I can understand & totally support not wishing to break the compilation
of plugins for every release, by having a reasonably stable API.

I think forcing recompile for each release is reasonable protection
to make it less atttractive to create license violating closed source
plugins.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Peter Maydell 4 years, 7 months ago 
On Thu, 12 Sep 2019 at 11:17, Daniel P. Berrangé <berrange@redhat.com> wrote:
> I think forcing recompile for each release is reasonable protection
> to make it less atttractive to create license violating closed source
> plugins.

I'm just not sure that a plugin that, for instance, does
"whenever the guest makes a memory access print the address
and data", is in any reasonable sense a derived work of QEMU
that it makes sense to extend the GPL to. We're providing
a convenient introspection interface here, similar to but
different from the way gdbstub lets you introspect guest
binary behaviour, not an arbitrary mechanism for extending
QEMU itself.

thanks
-- PMM
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Peter Maydell 4 years, 7 months ago 
On Thu, 12 Sep 2019 at 11:07, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> > Wait, what? From my perspective the whole point of the plugin
> > interface is that it should be stable, in that at least there's
> > a good chance that a plugin you built will work against multiple
> > versions of QEMU, and if it doesn't then it should fail with
> > a reasonable error message telling you to update. I'm not
> > sure we should be landing the plugins infrastructure if we
> > don't have that much stability.
>
> There is a big fat blurry line between "set in stone" and "not requiring
> you to re-engineer the plugin every QEMU release". I'm saying we should
> reserve the right to extend and change the plugin API as required but
> the expectation would be the plugins will continue to work the same way
> but maybe with tweaks to the API hooks to support additional features.
>
> It's also a pretty young interface so I would expect some evolution once
> it is released into the field.

Sure. But I think we should document that we at least intend to
have some approximation to a compatability/deprecation policy
here, and some mechanisms for versioning so you get a helpful
error rather than weird misbehaviour if your plugin is too old.

> One problem with the anti-license circumvention measures being suggested
> is it will mean having to recompile plugins for any given release.

Why should we do this? I think this is making life hard for our
users for no good reason. We *do* have this check for modules,
because a module is just a random .so that can do anything in
QEMU. I thought we had the TCG-plugin interface much more locked
down than that?

thanks
-- PMM
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Alex Bennée 4 years, 7 months ago 
Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 12 Sep 2019 at 11:07, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > Wait, what? From my perspective the whole point of the plugin
>> > interface is that it should be stable, in that at least there's
>> > a good chance that a plugin you built will work against multiple
>> > versions of QEMU, and if it doesn't then it should fail with
>> > a reasonable error message telling you to update. I'm not
>> > sure we should be landing the plugins infrastructure if we
>> > don't have that much stability.
>>
>> There is a big fat blurry line between "set in stone" and "not requiring
>> you to re-engineer the plugin every QEMU release". I'm saying we should
>> reserve the right to extend and change the plugin API as required but
>> the expectation would be the plugins will continue to work the same way
>> but maybe with tweaks to the API hooks to support additional features.
>>
>> It's also a pretty young interface so I would expect some evolution once
>> it is released into the field.
>
> Sure. But I think we should document that we at least intend to
> have some approximation to a compatability/deprecation policy
> here, and some mechanisms for versioning so you get a helpful
> error rather than weird misbehaviour if your plugin is too old.
>
>> One problem with the anti-license circumvention measures being suggested
>> is it will mean having to recompile plugins for any given release.
>
> Why should we do this? I think this is making life hard for our
> users for no good reason. We *do* have this check for modules,
> because a module is just a random .so that can do anything in
> QEMU. I thought we had the TCG-plugin interface much more locked
> down than that?

It is, there are only a few set calls the plugin can make into QEMU,
mostly to register callbacks to events. Currently it can examine the
state of the system (again through the API) but can't change it's
behaviour (although a register access interface has been requested
although I'd initially intended to make it read only).

>
> thanks
> -- PMM


--
Alex Bennée
Re: [Qemu-devel] TCG plugins and the GPL (was: [PATCH v4 00/54] plugins for TCG)
Posted by Daniel P. Berrangé 4 years, 7 months ago 
On Thu, Sep 12, 2019 at 10:03:48AM +0100, Alex Bennée wrote:
> 
> Markus Armbruster <armbru@redhat.com> writes:
> 
> > Alex Bennée <alex.bennee@linaro.org> writes:
> >
> >> Markus Armbruster <armbru@redhat.com> writes:
> > [...]
> >>> Please advise why TCG plugins don't undermine the GPL.  Any proposal to
> >>> add a plugin interface needs to do that.
> >>
> >> I'm not sure what we can say about this apart from "ask your lawyer".
> >
> > I'm not asking for a legal argument, I'm asking for a pragmatic one.
> >
> >> I'm certainly not proposing we add any sort of language about what
> >> should and shouldn't be allowed to use the plugin interface. I find it
> >> hard to see how anyone could argue code written to interface with the
> >> plugin API couldn't be considered a derived work.
> >
> > What makes that so?  Is writing a plugin without linking with QEMU code
> > impractical?
> 
> The way a plugin works is by linking. The plugin itself would be useless
> unless combined with the QEMU code to do its thing. It is a more
> intimate binding than an IPC interface using some sort of protocol. The
> argument goes that pretty much any kernel module is derived code - but
> again it has never really been litigated in the courts which would be
> the people to set the precedent.

Part of the problem with the kernel is the historical precedent they
had set. The issue of GPL compliance only become prominent much later
after (closed source) loadable modules were already widely in use. They
tried to lock the door after the horse had already bolted by adding
EXPORT_SYMBOL_GPL.

We can avoid this trapped by clearly documenting our license expectations
from the very start. ie state that we consider any plugins to be derived
works and to be bound by the terms of the GPL. This doesn't mean the
plugins themselves have to be GPL, but they have to be under terms that
allow relicensing to the GPL, in order to be license compatible. 

We could even go as far as having the plugin registration API require
that the plugin explicitly declare its license and we can log this
license at time of loading. If people use a non-GPL compatible license
it will be clearly visible as non-compatible, or if they lie and pretend
to be GPL then they would be willfully violating.

> >> There are two use cases I have in mind:
> >>
> >> The first is FLOSS developers writing interesting tools that can take
> >> advantage of QEMU's control of the system to do experiments that are
> >> tricky with other setups (Valgrind is limited to same-arch, Dynamo/Pin
> >> are user-space only). I want these experiments to be easy to do without
> >> having to keep hacking and re-hacking QEMU's core code. I would hope
> >> QEMU developers would up-stream theirs into the QEMU source tree but I
> >> can imagine academics will have open source code that will only ever sit
> >> in their paper's repository.
> >
> > GPL'ed code that's not for upstream is 100% legitimate.

Yep, the code only has to be provided to the people who receive the
plugin binary. Those people can't be prevented from redistributing
it further though.

> >> The other is users who currently maintain hacked up internal copies of
> >> QEMU as a test bed for whatever piece of silicon they are brewing behind
> >> closed doors. This code would never be distributed (hence never be a GPL
> >> issue)
> >
> > Correct.  We can't force anybody to distribute, and that's only proper.
> >

> > Are there any technical difficulties that could make distributing a
> > plugins in binary form impractical?
> 
> Well the first thing will be we are not intending to offer a guaranteed
> ABI. While we don't want to be changing it at a whim there shouldn't be
> an expectation that the plugin interface will maintain backwards
> compatibility (unlike the command line interface ;-). There should be an
> expectation that plugins will likely need to be rebuilt against the
> current source tree from time to time.
> 
> We could implement a more technical measure analogous to the kernels
> module signing that would require the plugin to be rebuilt with
> reference to the current QEMU source tree although that will be a pain
> even for internally distributed blobs. I'm loathed to implement such a
> system from v1 though given the problem of publicly distributed binary
> blobs is currently only a theoretical problem.

The problem with waiting for a problem to arise is that you have set a
precedent that its ok. We are in a stronger enforcement position if we
can set expectations accurately from day one, avoiding the trap the
kernel had which needed to try to enforce after the fact.

So from that POV, I'd be strongly in favour of technical measures that
force the plugin to be rebuilt against each new QEMU version.

This doesn't need to be module signing - it could be as simple as
requiring the plugin to export a symbol "module_version" which must
exactly match the QEMU version, or refuse to load it. If we want to
be even more strict we could generate a random hash in each QEMU
rebuild, which is the similar level of strictness to kernel signing

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.