This is my take on Joelle's patch set:
https://lists.nongnu.org/archive/html/qemu-devel/2020-10/msg07837.html

First, lots more patches.  For the most part, I convert one interface
at a time, instead of trying to do it all at once.  Then, convert the
tcg backends one at a time, allowing for a backend to say that it has
not been updated and not to use the split.  This takes care of TCI,
for one, which would never be converted, as it makes no sense.  But I
don't expect to ever try to convert mips either -- the memory mapping
constraints there are ugly.

There are many more places that "const" could logically be pushed.
I stopped at several major interfaces and left TODO comments.

I have only converted tcg/i386 and tcg/aarch64 so far.  That should
certainly be sufficient for immediate darwin/iOS testing.

Second, I've taken the start with rw and offset to rx approach, which
is the opposite of Joelle's patch set.  It's a close call, but this
direction may be slightly cleaner.

Third, there are almost no ifdefs.  The only ones are related to host
specific support.  That means that this is always available, modulo
the actual tcg backend support.  When the feature is disabled, we will
be adding and subtracting a 0 stored in a global variable.

Fourth, I have renamed the command-line parameter to "split-rwx".
I don't think this is perfect, and I'm not even sure if it's better
than "mirror-jit".  What this has done, though, is left the code
with inconsistant language -- "mirror" in some places, "split" in
others.  I'll clean that up once we know decide on naming.

Fifth, I have auto-enabled the feature for CONFIG_DEBUG_TCG, so that
it will fall-back to disabled without error.  But if you try to enable
it from the command-line without complete host support a fatal error
will be generated.  But this will make sure that the feature is
regularly tested.


r~


Richard Henderson (19):
  tcg: Enhance flush_icache_range with separate data pointer
  tcg: Move tcg prologue pointer out of TCGContext
  tcg: Move tcg epilogue pointer out of TCGContext
  tcg: Introduce tcg_mirror_rw_to_rx/tcg_mirror_rx_to_rw
  tcg: Adjust tcg_out_call for const
  tcg: Adjust tcg_out_label for const
  tcg: Adjust tcg_register_jit for const
  tcg: Adjust tb_target_set_jmp_target for split rwx
  tcg: Make DisasContextBase.tb const
  tcg: Make tb arg to synchronize_from_tb const
  tcg: Use Error with alloc_code_gen_buffer
  tcg: Add --accel tcg,split-rwx property
  accel/tcg: Support split-rwx for linux with memfd
  RFC: accel/tcg: Support split-rwx for darwin/iOS with vm_remap
  tcg: Return the rx mirror of TranslationBlock from exit_tb
  tcg/i386: Support split-rwx code generation
  tcg/aarch64: Use B not BL for tcg_out_goto_long
  tcg/aarch64: Implement flush_idcache_range manually
  tcg/aarch64: Support split-rwx code generation

 accel/tcg/tcg-runtime.h      |   2 +-
 include/disas/disas.h        |   2 +-
 include/exec/exec-all.h      |   2 +-
 include/exec/gen-icount.h    |   4 +-
 include/exec/log.h           |   2 +-
 include/exec/translator.h    |   2 +-
 include/hw/core/cpu.h        |   3 +-
 include/sysemu/tcg.h         |   2 +-
 include/tcg/tcg-op.h         |   2 +-
 include/tcg/tcg.h            |  37 +++--
 tcg/aarch64/tcg-target.h     |   9 +-
 tcg/arm/tcg-target.h         |  11 +-
 tcg/i386/tcg-target.h        |  10 +-
 tcg/mips/tcg-target.h        |  11 +-
 tcg/ppc/tcg-target.h         |   5 +-
 tcg/riscv/tcg-target.h       |  11 +-
 tcg/s390/tcg-target.h        |  12 +-
 tcg/sparc/tcg-target.h       |  11 +-
 tcg/tci/tcg-target.h         |  12 +-
 accel/tcg/cpu-exec.c         |  41 +++---
 accel/tcg/tcg-all.c          |  26 +++-
 accel/tcg/tcg-runtime.c      |   4 +-
 accel/tcg/translate-all.c    | 255 ++++++++++++++++++++++++++++-------
 accel/tcg/translator.c       |   4 +-
 bsd-user/main.c              |   2 +-
 disas.c                      |   4 +-
 linux-user/main.c            |   2 +-
 softmmu/physmem.c            |   9 +-
 target/arm/cpu.c             |   3 +-
 target/arm/translate-a64.c   |   2 +-
 target/avr/cpu.c             |   3 +-
 target/hppa/cpu.c            |   3 +-
 target/i386/cpu.c            |   3 +-
 target/microblaze/cpu.c      |   3 +-
 target/mips/cpu.c            |   3 +-
 target/riscv/cpu.c           |   3 +-
 target/rx/cpu.c              |   3 +-
 target/sh4/cpu.c             |   3 +-
 target/sparc/cpu.c           |   3 +-
 target/tricore/cpu.c         |   2 +-
 tcg/tcg-op.c                 |  15 ++-
 tcg/tcg.c                    |  85 +++++++++---
 tcg/tci.c                    |   4 +-
 accel/tcg/trace-events       |   2 +-
 tcg/aarch64/tcg-target.c.inc | 130 +++++++++++++-----
 tcg/arm/tcg-target.c.inc     |   6 +-
 tcg/i386/tcg-target.c.inc    |  38 +++---
 tcg/mips/tcg-target.c.inc    |  18 +--
 tcg/ppc/tcg-target.c.inc     |  45 ++++---
 tcg/riscv/tcg-target.c.inc   |  12 +-
 tcg/s390/tcg-target.c.inc    |   8 +-
 tcg/sparc/tcg-target.c.inc   |  22 +--
 tcg/tcg-pool.c.inc           |   6 +-
 tcg/tci/tcg-target.c.inc     |   2 +-
 54 files changed, 655 insertions(+), 269 deletions(-)

-- 
2.25.1

Patchew URL: https://patchew.org/QEMU/20201030004921.721096-1-richard.henderson@linaro.org/



Hi,

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

Type: series
Message-id: 20201030004921.721096-1-richard.henderson@linaro.org
Subject: [PATCH v2 00/19] Mirror map JIT memory for TCG

=== 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
 * [new tag]         patchew/20201030004921.721096-1-richard.henderson@linaro.org -> patchew/20201030004921.721096-1-richard.henderson@linaro.org
Switched to a new branch 'test'
1cae0aa tcg/aarch64: Support split-rwx code generation
2a8bb6f tcg/aarch64: Implement flush_idcache_range manually
5aac937 tcg/aarch64: Use B not BL for tcg_out_goto_long
9f81275 tcg/i386: Support split-rwx code generation
5a11dd0 tcg: Return the rx mirror of TranslationBlock from exit_tb
af71330 RFC: accel/tcg: Support split-rwx for darwin/iOS with vm_remap
bf60965 accel/tcg: Support split-rwx for linux with memfd
90c1e77 tcg: Add --accel tcg,split-rwx property
d4805b8 tcg: Use Error with alloc_code_gen_buffer
9bfafc6 tcg: Make tb arg to synchronize_from_tb const
27ebfc9 tcg: Make DisasContextBase.tb const
5a7bc51 tcg: Adjust tb_target_set_jmp_target for split rwx
6a7c2c6 tcg: Adjust tcg_register_jit for const
51a0884 tcg: Adjust tcg_out_label for const
79f0c8e tcg: Adjust tcg_out_call for const
cd13bcf tcg: Introduce tcg_mirror_rw_to_rx/tcg_mirror_rx_to_rw
43a4727 tcg: Move tcg epilogue pointer out of TCGContext
5c5b85a tcg: Move tcg prologue pointer out of TCGContext
e320c51 tcg: Enhance flush_icache_range with separate data pointer

=== OUTPUT BEGIN ===
1/19 Checking commit e320c51e3e4a (tcg: Enhance flush_icache_range with separate data pointer)
2/19 Checking commit 5c5b85a1a024 (tcg: Move tcg prologue pointer out of TCGContext)
3/19 Checking commit 43a47275e60e (tcg: Move tcg epilogue pointer out of TCGContext)
4/19 Checking commit cd13bcf48f36 (tcg: Introduce tcg_mirror_rw_to_rx/tcg_mirror_rx_to_rw)
5/19 Checking commit 79f0c8e8dd04 (tcg: Adjust tcg_out_call for const)
6/19 Checking commit 51a088446659 (tcg: Adjust tcg_out_label for const)
7/19 Checking commit 6a7c2c61ac7c (tcg: Adjust tcg_register_jit for const)
8/19 Checking commit 5a7bc5100680 (tcg: Adjust tb_target_set_jmp_target for split rwx)
9/19 Checking commit 27ebfc9c7710 (tcg: Make DisasContextBase.tb const)
10/19 Checking commit 9bfafc6a1e66 (tcg: Make tb arg to synchronize_from_tb const)
11/19 Checking commit d4805b8c7ba9 (tcg: Use Error with alloc_code_gen_buffer)
12/19 Checking commit 90c1e773778d (tcg: Add --accel tcg,split-rwx property)
13/19 Checking commit bf60965b715c (accel/tcg: Support split-rwx for linux with memfd)
14/19 Checking commit af71330a9b77 (RFC: accel/tcg: Support split-rwx for darwin/iOS with vm_remap)
ERROR: externs should be avoided in .c files
#39: FILE: accel/tcg/translate-all.c:1195:
+extern kern_return_t mach_vm_remap(vm_map_t target_task,

total: 1 errors, 0 warnings, 86 lines checked

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

15/19 Checking commit 5a11dd063165 (tcg: Return the rx mirror of TranslationBlock from exit_tb)
16/19 Checking commit 9f812754c6c2 (tcg/i386: Support split-rwx code generation)
17/19 Checking commit 5aac937754cc (tcg/aarch64: Use B not BL for tcg_out_goto_long)
18/19 Checking commit 2a8bb6f3e71e (tcg/aarch64: Implement flush_idcache_range manually)
19/19 Checking commit 1cae0aa42840 (tcg/aarch64: Support split-rwx code generation)
=== OUTPUT END ===

Test command exited with code: 1


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

On 30/10/20 01:49, Richard Henderson wrote:
> Fourth, I have renamed the command-line parameter to "split-rwx".

Stupid observation, but wouldn't it be "split-wx"?

Thanks,

Paolo

> I don't think this is perfect, and I'm not even sure if it's better
> than "mirror-jit".  What this has done, though, is left the code
> with inconsistant language -- "mirror" in some places, "split" in
> others.  I'll clean that up once we know decide on naming.

On 10/30/20 11:26 AM, Paolo Bonzini wrote:
> On 30/10/20 01:49, Richard Henderson wrote:
>> Fourth, I have renamed the command-line parameter to "split-rwx".
> 
> Stupid observation, but wouldn't it be "split-wx"?

Um, yes.  ;-)


r~