[Qemu-devel] [PULL v2.5 00/19] tcg queued patches

[Qemu-devel] [PULL v2.5 00/19] tcg queued patches

[Richard Henderson]

Re-issuing the v2 pull request with the proper form.


r~


The following changes since commit 42747d6abb5035473e5585fa17620c1e8983a70b:

  Merge remote-tracking branch 'remotes/edgar/tags/edgar/xilinx-next-2018-06-15.for-upstream' into staging (2018-06-15 17:28:37 +0100)

are available in the Git repository at:

  https://github.com/rth7680/qemu.git tags/pull-tcg-20180615

for you to fetch changes up to 9f754620651d3432114f4bb89c7f12cbea814b3e:

  tcg: Reduce max TB opcode count (2018-06-15 09:39:53 -1000)

----------------------------------------------------------------
TCG patch queue:

Workaround macos assembler lossage.
Eliminate tb_lock.
Fix TB code generation overflow.

----------------------------------------------------------------
Emilio G. Cota (17):
      qht: require a default comparison function
      qht: return existing entry when qht_insert fails
      tcg: track TBs with per-region BST's
      tcg: move tb_ctx.tb_phys_invalidate_count to tcg_ctx
      translate-all: iterate over TBs in a page with PAGE_FOR_EACH_TB
      translate-all: make l1_map lockless
      translate-all: remove hole in PageDesc
      translate-all: work page-by-page in tb_invalidate_phys_range_1
      translate-all: move tb_invalidate_phys_page_range up in the file
      translate-all: use per-page locking in !user-mode
      translate-all: add page_locked assertions
      translate-all: introduce assert_no_pages_locked
      translate-all: discard TB when tb_link_page returns an existing matching TB
      translate-all: protect TB jumps with a per-destination-TB lock
      cputlb: remove tb_lock from tlb_flush functions
      translate-all: remove tb_lock mention from cpu_restore_state_from_tb
      tcg: remove tb_lock

John Arbuckle (1):
      tcg/i386: Use byte form of xgetbv instruction

Richard Henderson (1):
      tcg: Reduce max TB opcode count

 accel/tcg/translate-all.h       |    6 +-
 include/exec/cpu-common.h       |    2 +-
 include/exec/exec-all.h         |   51 +-
 include/exec/memory-internal.h  |    6 +-
 include/exec/tb-context.h       |    4 -
 include/qemu/qht.h              |   32 +-
 tcg/tcg.h                       |   19 +-
 accel/tcg/cpu-exec.c            |   96 ++--
 accel/tcg/cputlb.c              |    8 -
 accel/tcg/translate-all.c       | 1045 +++++++++++++++++++++++++++------------
 exec.c                          |   26 +-
 linux-user/main.c               |    3 -
 tcg/aarch64/tcg-target.inc.c    |    2 +-
 tcg/arm/tcg-target.inc.c        |    2 +-
 tcg/i386/tcg-target.inc.c       |    7 +-
 tcg/mips/tcg-target.inc.c       |    2 +-
 tcg/ppc/tcg-target.inc.c        |    4 +-
 tcg/s390/tcg-target.inc.c       |    2 +-
 tcg/sparc/tcg-target.inc.c      |    4 +-
 tcg/tcg.c                       |  218 +++++++-
 tcg/tci/tcg-target.inc.c        |    2 +-
 tests/qht-bench.c               |   18 +-
 tests/test-qht.c                |   23 +-
 util/qht.c                      |   41 +-
 docs/devel/multi-thread-tcg.txt |   24 +-
 25 files changed, 1157 insertions(+), 490 deletions(-)

Re: [Qemu-devel] [PULL v2.5 00/19] tcg queued patches

[Peter Maydell]

On 21 June 2018 at 15:37, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Re-issuing the v2 pull request with the proper form.
>
>
> r~
>
>
> The following changes since commit 42747d6abb5035473e5585fa17620c1e8983a70b:
>
>   Merge remote-tracking branch 'remotes/edgar/tags/edgar/xilinx-next-2018-06-15.for-upstream' into staging (2018-06-15 17:28:37 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/rth7680/qemu.git tags/pull-tcg-20180615
>
> for you to fetch changes up to 9f754620651d3432114f4bb89c7f12cbea814b3e:
>
>   tcg: Reduce max TB opcode count (2018-06-15 09:39:53 -1000)
>
> ----------------------------------------------------------------
> TCG patch queue:
>
> Workaround macos assembler lossage.
> Eliminate tb_lock.
> Fix TB code generation overflow.
>
Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL v2.5 00/19] tcg queued patches

[Peter Maydell]

On 22 June 2018 at 09:58, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 21 June 2018 at 15:37, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>> The following changes since commit 42747d6abb5035473e5585fa17620c1e8983a70b:
>>
>>   Merge remote-tracking branch 'remotes/edgar/tags/edgar/xilinx-next-2018-06-15.for-upstream' into staging (2018-06-15 17:28:37 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/rth7680/qemu.git tags/pull-tcg-20180615
>>
>> for you to fetch changes up to 9f754620651d3432114f4bb89c7f12cbea814b3e:
>>
>>   tcg: Reduce max TB opcode count (2018-06-15 09:39:53 -1000)
>>
>> ----------------------------------------------------------------
>> TCG patch queue:
>>
>> Workaround macos assembler lossage.
>> Eliminate tb_lock.
>> Fix TB code generation overflow.
>>
> Applied, thanks.

...looks like it's broken builds with --disable-tcg, though.
Travis builds with that config fail:

/home/travis/build/qemu/qemu/accel/stubs/tcg-stub.c:24:6: error: no
previous prototype for "tb_unlock" [-Werror=missing-prototypes]
 void tb_unlock(void)
      ^
cc1: all warnings being treated as errors

(sample failing job log: https://api.travis-ci.org/v3/job/395399220/log.txt)

Should just be a matter of deleting the stub function, I guess.

thanks
-- PMM

Re: [Qemu-devel] [PULL v2.5 00/19] tcg queued patches

[Peter Maydell]

On 22 June 2018 at 13:46, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 June 2018 at 09:58, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 21 June 2018 at 15:37, Richard Henderson
>> <richard.henderson@linaro.org> wrote:
>>> The following changes since commit 42747d6abb5035473e5585fa17620c1e8983a70b:
>>>
>>>   Merge remote-tracking branch 'remotes/edgar/tags/edgar/xilinx-next-2018-06-15.for-upstream' into staging (2018-06-15 17:28:37 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>   https://github.com/rth7680/qemu.git tags/pull-tcg-20180615
>>>
>>> for you to fetch changes up to 9f754620651d3432114f4bb89c7f12cbea814b3e:
>>>
>>>   tcg: Reduce max TB opcode count (2018-06-15 09:39:53 -1000)
>>>
>>> ----------------------------------------------------------------
>>> TCG patch queue:
>>>
>>> Workaround macos assembler lossage.
>>> Eliminate tb_lock.
>>> Fix TB code generation overflow.
>>>
>> Applied, thanks.
>
> ...looks like it's broken builds with --disable-tcg, though.
> Travis builds with that config fail:
>
> /home/travis/build/qemu/qemu/accel/stubs/tcg-stub.c:24:6: error: no
> previous prototype for "tb_unlock" [-Werror=missing-prototypes]
>  void tb_unlock(void)
>       ^
> cc1: all warnings being treated as errors
>
> (sample failing job log: https://api.travis-ci.org/v3/job/395399220/log.txt)
>
> Should just be a matter of deleting the stub function, I guess.

Hmm, not quite that easy. After that the link fails:

  LINK    x86_64-softmmu/qemu-system-x86_64
exec.o: In function `tlb_reset_dirty_range_all':
/home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:1334: undefined
reference to `tlb_reset_dirty'
exec.o: In function `memory_notdirty_write_complete':
/home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:2658: undefined
reference to `page_collection_unlock'
exec.o: In function `tcg_commit':
/home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:3052: undefined
reference to `cpu_reloading_memory_map'
cpus.o: In function `tcg_cpu_exec':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1362: undefined
reference to `cpu_exec'
cpus.o: In function `qemu_tcg_rr_cpu_thread_fn':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1401: undefined
reference to `tcg_register_thread'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1471: undefined
reference to `cpu_exec_step_atomic'
cpus.o: In function `qemu_tcg_cpu_thread_fn':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1637: undefined
reference to `tcg_register_thread'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1673: undefined
reference to `cpu_exec_step_atomic'
cpus.o: In function `qemu_tcg_init_vcpu':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1865: undefined
reference to `tcg_region_init'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1875: undefined
reference to `parallel_cpus'

Some of those are probably best handled with stub functions
(eg tcg_register_thread()), but some of them might be better
off handled by guarding functions with #ifndef CONFIG_TCG
or moving them to source files in accel/tcg ?

This is probably best fixed by somebody familiar with the
commits in question...

thanks
-- PMM

[Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal

[Emilio G. Cota]

Tested to build x86_64-softmmu and i386-softmmu targets.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 accel/stubs/tcg-stub.c | 4 ----
 exec.c                 | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index ee575a8..76ae461 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -21,10 +21,6 @@ void tb_flush(CPUState *cpu)
 {
 }
 
-void tb_unlock(void)
-{
-}
-
 void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
 {
 }
diff --git a/exec.c b/exec.c
index 28f9bdc..3baa3dc 100644
--- a/exec.c
+++ b/exec.c
@@ -2645,18 +2645,22 @@ void memory_notdirty_write_prepare(NotDirtyInfo *ndi,
     ndi->pages = NULL;
 
     assert(tcg_enabled());
+#ifdef CONFIG_TCG
     if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
         ndi->pages = page_collection_lock(ram_addr, ram_addr + size);
         tb_invalidate_phys_page_fast(ndi->pages, ram_addr, size);
     }
+#endif
 }
 
 /* Called within RCU critical section. */
 void memory_notdirty_write_complete(NotDirtyInfo *ndi)
 {
     if (ndi->pages) {
+#ifdef CONFIG_TCG
         page_collection_unlock(ndi->pages);
         ndi->pages = NULL;
+#endif
     }
 
     /* Set both VGA and migration bits for simplicity and to remove
-- 
2.7.4

Re: [Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal

[Peter Maydell]

On 22 June 2018 at 17:16, Emilio G. Cota <cota@braap.org> wrote:
> Tested to build x86_64-softmmu and i386-softmmu targets.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  accel/stubs/tcg-stub.c | 4 ----
>  exec.c                 | 4 ++++
>  2 files changed, 4 insertions(+), 4 deletions(-)

This still doesn't link for me:

  LINK    x86_64-softmmu/qemu-system-x86_64
exec.o: In function `tlb_reset_dirty_range_all':
/home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:1334: undefined
reference to `tlb_reset_dirty'
exec.o: In function `tcg_commit':
/home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:3056: undefined
reference to `cpu_reloading_memory_map'
cpus.o: In function `tcg_cpu_exec':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1362: undefined
reference to `cpu_exec'
cpus.o: In function `qemu_tcg_rr_cpu_thread_fn':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1401: undefined
reference to `tcg_register_thread'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1471: undefined
reference to `cpu_exec_step_atomic'
cpus.o: In function `qemu_tcg_cpu_thread_fn':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1637: undefined
reference to `tcg_register_thread'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1673: undefined
reference to `cpu_exec_step_atomic'
cpus.o: In function `qemu_tcg_init_vcpu':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1865: undefined
reference to `tcg_region_init'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1875: undefined
reference to `parallel_cpus'
collect2: error: ld returned 1 exit status

Configured with --target-list=x86_64-softmmu --enable-debug --disable-tcg

It's probably the --enable-debug that makes the difference:
for instance cpu_reloading_memory_map() is referenced from
tcg_commit(), which is only called from within an "if (tcg_enabled())"
guard; with debug disabled the compiler probably figures out that
tcg_commit() is unreachable and doesn't put it in the .o file.

Possibly this has always been broken and wasn't a regression?
If so I guess we should apply your patch and then fix this
separately...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal

[Peter Maydell]

On 22 June 2018 at 17:24, Peter Maydell <peter.maydell@linaro.org> wrote:
> Possibly this has always been broken and wasn't a regression?
> If so I guess we should apply your patch and then fix this
> separately...

Yep, before your tb_lock changes on my system I can build
--disable-tcg, but not --disable-tcg --enable-debug. After
your tb_lock changes, both fail, and this patch fixes the
--disable-tcg case again. So I think we could reasonably
apply this (to fix the travis builds) and then look at
the longer-standing "can't build --disable-tcg --enable-debug"
issue separately.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal

[Emilio G. Cota]

On Fri, Jun 22, 2018 at 17:24:33 +0100, Peter Maydell wrote:
> On 22 June 2018 at 17:16, Emilio G. Cota <cota@braap.org> wrote:
> It's probably the --enable-debug that makes the difference:
> for instance cpu_reloading_memory_map() is referenced from
> tcg_commit(), which is only called from within an "if (tcg_enabled())"
> guard; with debug disabled the compiler probably figures out that
> tcg_commit() is unreachable and doesn't put it in the .o file.

Yes, I get the same errors with both --enable-debug and --disable-tcg.

> Possibly this has always been broken and wasn't a regression?
> If so I guess we should apply your patch and then fix this
> separately...

v2.12.0 doesn't build either with the options above, so the
regression is fixed with the patch.

I'll send a v2 patch as a separate thread with an updated commit
message to reflect the above.

Thanks,

		Emilio

Re: [Qemu-devel] [PULL v2.5 00/19] tcg queued patches

[Emilio G. Cota]

On Fri, Jun 22, 2018 at 15:40:33 +0100, Peter Maydell wrote:
(snip)
> /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1637: undefined
> reference to `tcg_register_thread'
> /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1673: undefined
> reference to `cpu_exec_step_atomic'
> cpus.o: In function `qemu_tcg_init_vcpu':
> /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1865: undefined
> reference to `tcg_region_init'
> /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1875: undefined
> reference to `parallel_cpus'
> 
> Some of those are probably best handled with stub functions
> (eg tcg_register_thread()), but some of them might be better
> off handled by guarding functions with #ifndef CONFIG_TCG
> or moving them to source files in accel/tcg ?
> 
> This is probably best fixed by somebody familiar with the
> commits in question...

Just sent a patch that fixes the --disable-tcg build for me,
tested for x86_64-softmmu and i386-softmmu targets.

However, I'm puzzled by the fact that I cannot replicate all
of the link errors you report above. My build (tried with gcc
5.4.0 and 8.1.0) completes even though I don't see how
some symbols are resolved (I don't see them in the final
binary), e.g. tcg_register_thread().
Is the compiler somehow determining that it doesn't need those?

Thanks,

		Emilio