[v1,00/12] fix plugins double counting with mmio, cleanup CF_ flags

Hi,

Aaron reported an issue with TCG plugins when interacting with the
cpu_io_recompile code during icount. The ultimate fix was to avoid
instrumenting the re-executed block but along the way we clean-up a
bunch of the code by getting rid of CF_NOCACHE. I've also included
Richard's recently posted recompile hook cleanups at the start of the
series because it makes the improves the diffstat by pushing more arch
specific black magic to the targets. In fact without the additional
tests this removes more code than it adds ;-)

I've added some acceptance tests to detect the failure mode as well as
manually testing with a test Peter had lying around that exercises the
trixy "executing out of MMIO" code path which I've touched.

Please test and review.

Alex Bennée (8):
  tests/plugin: expand insn test to detect duplicate instructions
  tests/acceptance: add a new set of tests to exercise plugins
  accel/tcg: actually cache our partial icount TB
  accel/tcg: cache single instruction TB on pending replay exception
  accel/tcg: re-factor non-RAM execution code
  accel/tcg: remove CF_NOCACHE and special cases
  accel/tcg: allow plugin instrumentation to be disable via cflags
  tests/acceptance: add a new tests to detect counting errors

Richard Henderson (4):
  exec: Move TranslationBlock typedef to qemu/typedefs.h
  accel/tcg: Create io_recompile_replay_branch hook
  target/mips: Create mips_io_recompile_replay_branch
  target/sh4: Create superh_io_recompile_replay_branch

 include/exec/exec-all.h         |   6 +-
 include/exec/tb-context.h       |   1 -
 include/hw/core/cpu.h           |   4 +-
 include/hw/core/tcg-cpu-ops.h   |  13 +++-
 include/qemu/typedefs.h         |   1 +
 target/arm/internals.h          |   3 +-
 accel/tcg/cpu-exec.c            |  62 ++++-----------
 accel/tcg/translate-all.c       | 128 +++++++++++-------------------
 accel/tcg/translator.c          |   2 +-
 target/cris/translate.c         |   2 +-
 target/lm32/translate.c         |   2 +-
 target/mips/cpu.c               |  18 +++++
 target/moxie/translate.c        |   2 +-
 target/sh4/cpu.c                |  18 +++++
 target/unicore32/translate.c    |   2 +-
 tests/plugin/insn.c             |  12 ++-
 tests/acceptance/tcg_plugins.py | 134 ++++++++++++++++++++++++++++++++
 17 files changed, 263 insertions(+), 147 deletions(-)
 create mode 100644 tests/acceptance/tcg_plugins.py

-- 
2.20.1

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



Hi,

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

Type: series
Message-id: 20210209182749.31323-1-alex.bennee@linaro.org
Subject: [PATCH  v1 00/12] fix plugins double counting with mmio, cleanup CF_ flags

=== 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/20210209190224.62827-1-dgilbert@redhat.com -> patchew/20210209190224.62827-1-dgilbert@redhat.com
 - [tag update]      patchew/20210211045455.456371-1-thuth@redhat.com -> patchew/20210211045455.456371-1-thuth@redhat.com
Switched to a new branch 'test'
a9637ea tests/acceptance: add a new tests to detect counting errors
fa2e5c6 accel/tcg: allow plugin instrumentation to be disable via cflags
feba470 accel/tcg: remove CF_NOCACHE and special cases
efc2b45 accel/tcg: re-factor non-RAM execution code
8fa939a accel/tcg: cache single instruction TB on pending replay exception
3950a33 accel/tcg: actually cache our partial icount TB
999a79a tests/acceptance: add a new set of tests to exercise plugins
cd71497 tests/plugin: expand insn test to detect duplicate instructions
be5dad9 target/sh4: Create superh_io_recompile_replay_branch
ee10c4e target/mips: Create mips_io_recompile_replay_branch
69ecbdf accel/tcg: Create io_recompile_replay_branch hook
827fd40 exec: Move TranslationBlock typedef to qemu/typedefs.h

=== OUTPUT BEGIN ===
1/12 Checking commit 827fd4086c80 (exec: Move TranslationBlock typedef to qemu/typedefs.h)
2/12 Checking commit 69ecbdf64517 (accel/tcg: Create io_recompile_replay_branch hook)
3/12 Checking commit ee10c4e8c405 (target/mips: Create mips_io_recompile_replay_branch)
4/12 Checking commit be5dad9f333f (target/sh4: Create superh_io_recompile_replay_branch)
5/12 Checking commit cd71497cb2a2 (tests/plugin: expand insn test to detect duplicate instructions)
WARNING: line over 80 characters
#30: FILE: tests/plugin/insn.c:27:
+        g_autofree gchar *out = g_strdup_printf("detected repeat execution @ 0x%"

total: 0 errors, 1 warnings, 25 lines checked

Patch 5/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/12 Checking commit 999a79a49265 (tests/acceptance: add a new set of tests to exercise plugins)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#18: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 6/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/12 Checking commit 3950a3391d8c (accel/tcg: actually cache our partial icount TB)
8/12 Checking commit 8fa939a360c2 (accel/tcg: cache single instruction TB on pending replay exception)
WARNING: line over 80 characters
#89: FILE: accel/tcg/cpu-exec.c:654:
+            && (cpu->cflags_next_tb == -1 || cpu->cflags_next_tb & CF_USE_ICOUNT)

total: 0 errors, 1 warnings, 65 lines checked

Patch 8/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/12 Checking commit efc2b45722f4 (accel/tcg: re-factor non-RAM execution code)
WARNING: Block comments use a leading /* on a separate line
#26: FILE: accel/tcg/translate-all.c:1781:
+/* Add a new TB and link it to the physical page tables. phys_page2 is

total: 0 errors, 1 warnings, 53 lines checked

Patch 9/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/12 Checking commit feba47079ea4 (accel/tcg: remove CF_NOCACHE and special cases)
11/12 Checking commit fa2e5c6fe34b (accel/tcg: allow plugin instrumentation to be disable via cflags)
WARNING: line over 80 characters
#75: FILE: accel/tcg/translator.c:61:
+    plugin_enabled = !(tb_cflags(db->tb) & CF_NOINSTR) && plugin_gen_tb_start(cpu, tb);

ERROR: line over 90 characters
#96: FILE: include/exec/exec-all.h:465:
+    (CF_COUNT_MASK | CF_LAST_IO | CF_NOINSTR | CF_USE_ICOUNT | CF_PARALLEL | CF_CLUSTER_MASK)

total: 1 errors, 1 warnings, 57 lines checked

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

12/12 Checking commit a9637eabc2c8 (tests/acceptance: add a new tests to detect counting errors)
WARNING: line over 80 characters
#50: FILE: tests/acceptance/tcg_plugins.py:129:
+            m = re.search(br"detected repeat execution @ (?P<addr>0x[0-9A-Fa-f]+)", s)

total: 0 errors, 1 warnings, 34 lines checked

Patch 12/12 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/20210209182749.31323-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