[Qemu-devel] [PATCH v13 00/26] translate: [tcg] Generic translation framework

Lluís Vilanova posted 26 patches 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/150002001195.22386.4679134058536830996.stgit@frigg.lan
Test FreeBSD passed
Test checkpatch failed
Test docker passed
Test s390x passed
accel/tcg/Makefile.objs       |    1
accel/tcg/translate-all.c     |    2
accel/tcg/translator.c        |  141 ++++++++++++++
include/exec/exec-all.h       |    8 -
include/exec/translator.h     |  134 +++++++++++++
target/alpha/translate.c      |    5
target/arm/translate-a64.c    |  271 +++++++++++++-------------
target/arm/translate.c        |  420 +++++++++++++++++++++--------------------
target/arm/translate.h        |   38 ++--
target/cris/translate.c       |   14 +
target/hppa/translate.c       |    5
target/i386/translate.c       |  300 +++++++++++++++--------------
target/lm32/translate.c       |   10 +
target/m68k/translate.c       |   12 +
target/microblaze/translate.c |   10 +
target/mips/translate.c       |    5
target/moxie/translate.c      |    4
target/nios2/translate.c      |   11 +
target/openrisc/translate.c   |   10 +
target/ppc/translate.c        |    5
target/s390x/translate.c      |    8 -
target/sh4/translate.c        |    5
target/sparc/translate.c      |    5
target/tilegx/translate.c     |    5
target/tricore/translate.c    |    5
target/unicore32/translate.c  |   12 +
target/xtensa/translate.c     |    9 +
27 files changed, 876 insertions(+), 579 deletions(-)
create mode 100644 accel/tcg/translator.c
create mode 100644 include/exec/translator.h
[Qemu-devel] [PATCH v13 00/26] translate: [tcg] Generic translation framework
Posted by Lluís Vilanova 6 years, 9 months ago
This series proposes a generic (target-agnostic) instruction translation
framework.

It basically provides a generic main loop for instruction disassembly, which
calls target-specific functions when necessary. This generalization makes
inserting new code in the main loop easier, and helps in keeping all targets in
synch as to the contents of it.

This series also paves the way towards adding events to trace guest code
execution (BBLs and instructions).

I've ported i386/x86-64 and arm/aarch64 as an example to see how it fits in the
current organization, but will port the rest when this series gets merged.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---

Changes in v13
==============

* Rebase on aa916e409c.
* Pass DisasContext whenever possible.
* Ensure DisasContextBase::is_jmp is set only in proper target hooks.
* Add generic value DISAS_NORETURN and handle it accordingly.
* Change breakpoint_check() hook to use a bool return + optional DISAS_NORETURN.
* Let targets set max_insns in generic loop through the tb_stop() hook.
* Move gen_io_end() before the tb_stop() hook.


Changes in v12
==============

* Do not rename cs -> cpu when using CPUState arg in gen_intermediate_code().
* Avoid merge conflicts with queued patches.
* Fix documentation typos.
* Rename translate_block -> translator_loop.
* Fix misplaced changes (wrong patch).
* Rename _trblock_ -> _tr_.
* Drop DISAS_SS and DISAS_PAGE_CROSS from arm targets.
* Refactor calculation of TranslatorOps in arm targets.
* Merge init_globals() into init_disas_context().
* Minor cosmetic changes.


Changes in v11
==============

* Convert gen_intermediate_code_a64 [Emilio G. Cota].
* Replace "cpu_env" argument in icount functions with "tcg_ctx.cpu_env"
  [Emilio G. Cota].
* Drop QTAILQ_FOREACH_CONTINUE and cpu_breakpoint_get in favour of an explicit
  breakpoint traversal [Richard Henderson].
* Rename translate-block.[ch] into translator.[ch] [Emilio G. Cota].
* Revert DJ_* names into DISAS_*, and provide generic DISAS_TARGET_* enum values
  instead of defining them as integers on each target.
* Do not use DisasContextBase directly in target code (helps the compiler's
  alias analysis) [Richard Henderson].
* Make all generic translator hooks mandatory [Richard Henderson].
* Rename TranslatorOps::disas_insn as TranslatorOps::translate_insn
  [Richard Henderson].
* Generalize TranslatorOps::disas_flags into TranslatorOps::disas_log to support
  future targets [Richard Henderson].
* Split arm and aarch64 changes in different patches [Emilio G. Cota].
* Make translator op structs constant [Richard Henderson].
* Write a single entry point for both arm and aarch64 translation
  [Richard Henderson].
* Change format of commit titles [Emilio G. Cota].
* Remove cross-page check from generic code (it's already embedded in more
  complex, and mandatory, checks in most targets).


Changes in v10
==============

* Rebase on 931892e8a6.
* Fix build errors for hppa, nios2 and openrisc.


Changes in v9
=============

* Further increase inter-mail sleep time during sending.


Changes in v8
=============

* Increase inter-mail sleep time during sending (list keeps refusing some emails
  due to an excessive send rate).


Changes in v7
=============

* Change BreakpointHitType (BH_*) for BreakpointCheckType (BC_*).
* Move target-specific translation functions to a struct (TranslatorOps).
* Split target-specific changes into multiple patches.
* Rebase on edf8bc9842.


Changes in v6
=============

* Rebase on upstream master (64175afc69).
* Reorder fields in DisasContextBase to minimize padding [Richard Henderson].


Changes in v5
=============

* Remove stray uses of "restrict" keyword.


Changes in v4
=============

* Document new macro QTAILQ_FOREACH_CONTINUE [Peter Maydell].
* Fix coding style errors reported by checkpatch.
* Remove use of "restrict" in added functions; it makes older gcc versions barf
  about compilation errors.


Changes in v3
=============

* Rebase on 0737f32daf.


Changes in v2
=============

* Port ARM and AARCH64 targets.
* Fold single-stepping checks into "max_insns" [Richard Henderson].
* Move instruction start marks to target code [Richard Henderson].
* Add target hook for TB start.
* Check for TCG temporary leaks.
* Move instruction disassembly into a target hook.
* Make breakpoint_hit() return an enum to accomodate target's needs (ARM).


Lluís Vilanova (26):
      Pass generic CPUState to gen_intermediate_code()
      target: [tcg] Use a generic enum for DISAS_ values
      target: [tcg] Add generic translation framework
      target/i386: [tcg] Port to DisasContextBase
      target/i386: [tcg] Port to init_disas_context
      target/i386: [tcg] Port to insn_start
      target/i386: [tcg] Port to breakpoint_check
      target/i386: [tcg] Port to translate_insn
      target/i386: [tcg] Port to tb_stop
      target/i386: [tcg] Port to disas_log
      target/i386: [tcg] Port to generic translation framework
      target/arm: [tcg] Port to DisasContextBase
      target/arm: [tcg] Port to init_disas_context
      target/arm: [tcg,a64] Port to init_disas_context
      target/arm: [tcg] Port to tb_start
      target/arm: [tcg] Port to insn_start
      target/arm: [tcg,a64] Port to insn_start
      target/arm: [tcg] Port to breakpoint_check
      target/arm: [tcg,a64] Port to breakpoint_check
      target/arm: [tcg] Port to translate_insn
      target/arm: [tcg,a64] Port to translate_insn
      target/arm: [tcg] Port to tb_stop
      target/arm: [tcg,a64] Port to tb_stop
      target/arm: [tcg] Port to disas_log
      target/arm: [tcg,a64] Port to disas_log
      target/arm: [tcg] Port to generic translation framework


 accel/tcg/Makefile.objs       |    1 
 accel/tcg/translate-all.c     |    2 
 accel/tcg/translator.c        |  141 ++++++++++++++
 include/exec/exec-all.h       |    8 -
 include/exec/translator.h     |  134 +++++++++++++
 target/alpha/translate.c      |    5 
 target/arm/translate-a64.c    |  271 +++++++++++++-------------
 target/arm/translate.c        |  420 +++++++++++++++++++++--------------------
 target/arm/translate.h        |   38 ++--
 target/cris/translate.c       |   14 +
 target/hppa/translate.c       |    5 
 target/i386/translate.c       |  300 +++++++++++++++--------------
 target/lm32/translate.c       |   10 +
 target/m68k/translate.c       |   12 +
 target/microblaze/translate.c |   10 +
 target/mips/translate.c       |    5 
 target/moxie/translate.c      |    4 
 target/nios2/translate.c      |   11 +
 target/openrisc/translate.c   |   10 +
 target/ppc/translate.c        |    5 
 target/s390x/translate.c      |    8 -
 target/sh4/translate.c        |    5 
 target/sparc/translate.c      |    5 
 target/tilegx/translate.c     |    5 
 target/tricore/translate.c    |    5 
 target/unicore32/translate.c  |   12 +
 target/xtensa/translate.c     |    9 +
 27 files changed, 876 insertions(+), 579 deletions(-)
 create mode 100644 accel/tcg/translator.c
 create mode 100644 include/exec/translator.h


To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Emilio G. Cota <cota@braap.org>

Re: [Qemu-devel] [PATCH v13 00/26] translate: [tcg] Generic translation framework
Posted by no-reply@patchew.org 6 years, 9 months ago
Hi,

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

Type: series
Message-id: 150002001195.22386.4679134058536830996.stgit@frigg.lan
Subject: [Qemu-devel] [PATCH v13 00/26] translate: [tcg] Generic translation framework

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
dc3923c target/arm: [tcg] Port to generic translation framework
cf50855 target/arm: [tcg, a64] Port to disas_log
d333f58 target/arm: [tcg] Port to disas_log
760a365 target/arm: [tcg, a64] Port to tb_stop
4ff55a7 target/arm: [tcg] Port to tb_stop
94a2eed target/arm: [tcg, a64] Port to translate_insn
d1a3990 target/arm: [tcg] Port to translate_insn
a8ccf26 target/arm: [tcg, a64] Port to breakpoint_check
ce8ef1c target/arm: [tcg] Port to breakpoint_check
a2a561e target/arm: [tcg, a64] Port to insn_start
5b77def target/arm: [tcg] Port to insn_start
033fd32 target/arm: [tcg] Port to tb_start
6d31278 target/arm: [tcg, a64] Port to init_disas_context
2b136cb target/arm: [tcg] Port to init_disas_context
4e52d9c target/arm: [tcg] Port to DisasContextBase
b3c0a87 target/i386: [tcg] Port to generic translation framework
e70a33c target/i386: [tcg] Port to disas_log
fff0ca1 target/i386: [tcg] Port to tb_stop
6ec3f13 target/i386: [tcg] Port to translate_insn
6d3aefa target/i386: [tcg] Port to breakpoint_check
6529981 target/i386: [tcg] Port to insn_start
9e05cd3 target/i386: [tcg] Port to init_disas_context
7323661 target/i386: [tcg] Port to DisasContextBase
fc4b55b target: [tcg] Add generic translation framework
7612ad2 target: [tcg] Use a generic enum for DISAS_ values
5e64387 Pass generic CPUState to gen_intermediate_code()

=== OUTPUT BEGIN ===
Checking PATCH 1/26: Pass generic CPUState to gen_intermediate_code()...
WARNING: line over 80 characters
#123: FILE: target/arm/translate.h:158:
+static inline void gen_intermediate_code_a64(CPUState *cpu, TranslationBlock *tb)

total: 0 errors, 1 warnings, 303 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/26: target: [tcg] Use a generic enum for DISAS_ values...
Checking PATCH 3/26: target: [tcg] Add generic translation framework...
Checking PATCH 4/26: target/i386: [tcg] Port to DisasContextBase...
WARNING: line over 80 characters
#500: FILE: target/i386/translate.c:8516:
+                != ((dc->base.pc_next + TARGET_MAX_INSN_SIZE - 1) & TARGET_PAGE_MASK)

WARNING: line over 80 characters
#540: FILE: target/i386/translate.c:8554:
+        log_target_disas(cs, dc->base.pc_first, dc->base.pc_next - dc->base.pc_first,

total: 0 errors, 2 warnings, 491 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 5/26: target/i386: [tcg] Port to init_disas_context...
Checking PATCH 6/26: target/i386: [tcg] Port to insn_start...
Checking PATCH 7/26: target/i386: [tcg] Port to breakpoint_check...
Checking PATCH 8/26: target/i386: [tcg] Port to translate_insn...
Checking PATCH 9/26: target/i386: [tcg] Port to tb_stop...
Checking PATCH 10/26: target/i386: [tcg] Port to disas_log...
WARNING: line over 80 characters
#37: FILE: target/i386/translate.c:8536:
+    log_target_disas(cpu, dc->base.pc_first, dc->base.pc_next - dc->base.pc_first,

total: 0 errors, 1 warnings, 42 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 11/26: target/i386: [tcg] Port to generic translation framework...
Checking PATCH 12/26: target/arm: [tcg] Port to DisasContextBase...
WARNING: line over 80 characters
#53: FILE: target/arm/translate-a64.c:351:
+    if (s->base.singlestep_enabled || s->ss_active || (s->base.tb->cflags & CF_LAST_IO)) {

WARNING: line over 80 characters
#220: FILE: target/arm/translate-a64.c:11223:
+    dc->mmu_idx = core_to_arm_mmu_idx(env, ARM_TBFLAG_MMUIDX(dc->base.tb->flags));

ERROR: line over 90 characters
#274: FILE: target/arm/translate-a64.c:11288:
+                           included in [dc->base.tb->pc, dc->base.tb->pc + dc->base.tb->size) in order

WARNING: line over 80 characters
#287: FILE: target/arm/translate-a64.c:11300:
+        if (dc->base.num_insns == max_insns && (dc->base.tb->cflags & CF_LAST_IO)) {

ERROR: space required before the open parenthesis '('
#775: FILE: target/arm/translate.c:12092:
+        switch(dc->base.is_jmp) {

WARNING: line over 80 characters
#848: FILE: target/arm/translate.h:164:
+static inline void gen_intermediate_code_a64(DisasContextBase *db, CPUState *cpu,

total: 2 errors, 4 warnings, 759 lines checked

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

Checking PATCH 13/26: target/arm: [tcg] Port to init_disas_context...
WARNING: line over 80 characters
#72: FILE: target/arm/translate.c:11810:
+    dc->mmu_idx = core_to_arm_mmu_idx(env, ARM_TBFLAG_MMUIDX(dc->base.tb->flags));

total: 0 errors, 1 warnings, 120 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 14/26: target/arm: [tcg, a64] Port to init_disas_context...
Checking PATCH 15/26: target/arm: [tcg] Port to tb_start...
Checking PATCH 16/26: target/arm: [tcg] Port to insn_start...
Checking PATCH 17/26: target/arm: [tcg, a64] Port to insn_start...
Checking PATCH 18/26: target/arm: [tcg] Port to breakpoint_check...
Checking PATCH 19/26: target/arm: [tcg, a64] Port to breakpoint_check...
Checking PATCH 20/26: target/arm: [tcg] Port to translate_insn...
ERROR: suspect code indent for conditional statements (4, 4)
#200: FILE: target/arm/translate.c:12095:
+    if (dc->base.is_jmp != DISAS_SKIP) {
     if (tb->cflags & CF_LAST_IO) {

total: 1 errors, 0 warnings, 216 lines checked

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

Checking PATCH 21/26: target/arm: [tcg, a64] Port to translate_insn...
Checking PATCH 22/26: target/arm: [tcg] Port to tb_stop...
Checking PATCH 23/26: target/arm: [tcg, a64] Port to tb_stop...
Checking PATCH 24/26: target/arm: [tcg] Port to disas_log...
Checking PATCH 25/26: target/arm: [tcg, a64] Port to disas_log...
Checking PATCH 26/26: target/arm: [tcg] Port to generic translation framework...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org