tcg: Fix launchpad 1824853

[Qemu-devel] [PATCH 0/2] tcg: Fix launchpad 1824853

Richard Henderson posted 2 patches 5 years ago

Download series mbox

Test docker-mingw@fedora passed

Test docker-clang@ubuntu passed

Test checkpatch passed

Test asan passed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190416083150.19649-1-richard.henderson@linaro.org

Maintainers: Sagar Karandikar <sagark@eecs.berkeley.edu>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Guan Xuetao <gxt@mprc.pku.edu.cn>, Cornelia Huck <cohuck@redhat.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Alistair Francis <Alistair.Francis@wdc.com>, Stafford Horne <shorne@gmail.com>, Max Filippov <jcmvbkbc@gmail.com>, Laurent Vivier <laurent@vivier.eu>, Eduardo Habkost <ehabkost@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Peter Maydell <peter.maydell@linaro.org>, Marek Vasut <marex@denx.de>, Michael Walle <michael@walle.cc>, Richard Henderson <rth@twiddle.net>, Aleksandar Rikalo <arikalo@wavecomp.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Paolo Bonzini <pbonzini@redhat.com>, Chris Wulff <crwulff@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Anthony Green <green@moxielogic.com>, David Hildenbrand <david@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Markovic <amarkovic@wavecomp.com>, Palmer Dabbelt <palmer@sifive.com>

include/exec/exec-all.h       |  4 +--
include/exec/translator.h     |  3 +-
accel/tcg/translate-all.c     | 54 +++++++++++++++++++++++++++++------
accel/tcg/translator.c        | 15 ++--------
target/alpha/translate.c      |  4 +--
target/arm/translate.c        |  4 +--
target/cris/translate.c       | 10 +------
target/hppa/translate.c       |  5 ++--
target/i386/translate.c       |  4 +--
target/lm32/translate.c       | 10 +------
target/m68k/translate.c       |  4 +--
target/microblaze/translate.c | 10 +------
target/mips/translate.c       |  4 +--
target/moxie/translate.c      | 11 ++-----
target/nios2/translate.c      | 14 ++-------
target/openrisc/translate.c   |  4 +--
target/ppc/translate.c        |  4 +--
target/riscv/translate.c      |  4 +--
target/s390x/translate.c      |  4 +--
target/sh4/translate.c        |  4 +--
target/sparc/translate.c      |  4 +--
target/tilegx/translate.c     | 12 +-------
target/tricore/translate.c    | 16 ++---------
target/unicore32/translate.c  | 10 +------
target/xtensa/translate.c     |  4 +--
tcg/tcg.c                     |  4 +++
26 files changed, 93 insertions(+), 133 deletions(-)

Expand all Fold all

[Qemu-devel] [PATCH 0/2] tcg: Fix launchpad 1824853

Posted by Richard Henderson 5 years ago

This is a case where we generate more than 64k code for a mere 231
guest instructions.  This hits some assertions within TCG that we're
not overflowing the uint16_t that we use for representing our
unwind info.

Fix this by returning an error indication, rather than asserting.
This lets us try again from tb_gen_code with a lower max_insns.

This should resolve the problem for x86 as a host.  There are other
failure modes wrt out-of-range relocations that might affect the
RISC hosts.  I'm going to leave those for a different patch set.


r~


Richard Henderson (2):
  tcg: Hoist max_insns computation to tb_gen_code
  tcg: Restart after TB code generation overflow

 include/exec/exec-all.h       |  4 +--
 include/exec/translator.h     |  3 +-
 accel/tcg/translate-all.c     | 54 +++++++++++++++++++++++++++++------
 accel/tcg/translator.c        | 15 ++--------
 target/alpha/translate.c      |  4 +--
 target/arm/translate.c        |  4 +--
 target/cris/translate.c       | 10 +------
 target/hppa/translate.c       |  5 ++--
 target/i386/translate.c       |  4 +--
 target/lm32/translate.c       | 10 +------
 target/m68k/translate.c       |  4 +--
 target/microblaze/translate.c | 10 +------
 target/mips/translate.c       |  4 +--
 target/moxie/translate.c      | 11 ++-----
 target/nios2/translate.c      | 14 ++-------
 target/openrisc/translate.c   |  4 +--
 target/ppc/translate.c        |  4 +--
 target/riscv/translate.c      |  4 +--
 target/s390x/translate.c      |  4 +--
 target/sh4/translate.c        |  4 +--
 target/sparc/translate.c      |  4 +--
 target/tilegx/translate.c     | 12 +-------
 target/tricore/translate.c    | 16 ++---------
 target/unicore32/translate.c  | 10 +------
 target/xtensa/translate.c     |  4 +--
 tcg/tcg.c                     |  4 +++
 26 files changed, 93 insertions(+), 133 deletions(-)

-- 
2.17.1

Re: [Qemu-devel] [PATCH 0/2] tcg: Fix launchpad 1824853

Posted by Philippe Mathieu-Daudé 5 years ago

On 4/16/19 10:31 AM, Richard Henderson wrote:
> This is a case where we generate more than 64k code for a mere 231
> guest instructions.  This hits some assertions within TCG that we're
> not overflowing the uint16_t that we use for representing our
> unwind info.
> 
> Fix this by returning an error indication, rather than asserting.
> This lets us try again from tb_gen_code with a lower max_insns.
> 
> This should resolve the problem for x86 as a host.  There are other
> failure modes wrt out-of-range relocations that might affect the
> RISC hosts.  I'm going to leave those for a different patch set.

Very clean way to solve this. Nice cleanup in patch #1 (interesting
Nios2 case), Simple fix in patch #2. It would be fun to trigger the
assert(max_insns > 1) =)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> Richard Henderson (2):
>   tcg: Hoist max_insns computation to tb_gen_code
>   tcg: Restart after TB code generation overflow
> 
>  include/exec/exec-all.h       |  4 +--
>  include/exec/translator.h     |  3 +-
>  accel/tcg/translate-all.c     | 54 +++++++++++++++++++++++++++++------
>  accel/tcg/translator.c        | 15 ++--------
>  target/alpha/translate.c      |  4 +--
>  target/arm/translate.c        |  4 +--
>  target/cris/translate.c       | 10 +------
>  target/hppa/translate.c       |  5 ++--
>  target/i386/translate.c       |  4 +--
>  target/lm32/translate.c       | 10 +------
>  target/m68k/translate.c       |  4 +--
>  target/microblaze/translate.c | 10 +------
>  target/mips/translate.c       |  4 +--
>  target/moxie/translate.c      | 11 ++-----
>  target/nios2/translate.c      | 14 ++-------
>  target/openrisc/translate.c   |  4 +--
>  target/ppc/translate.c        |  4 +--
>  target/riscv/translate.c      |  4 +--
>  target/s390x/translate.c      |  4 +--
>  target/sh4/translate.c        |  4 +--
>  target/sparc/translate.c      |  4 +--
>  target/tilegx/translate.c     | 12 +-------
>  target/tricore/translate.c    | 16 ++---------
>  target/unicore32/translate.c  | 10 +------
>  target/xtensa/translate.c     |  4 +--
>  tcg/tcg.c                     |  4 +++
>  26 files changed, 93 insertions(+), 133 deletions(-)
>

Re: [Qemu-devel] [PATCH 0/2] tcg: Fix launchpad 1824853

Posted by Alex Bennée 4 years, 12 months ago

Richard Henderson <richard.henderson@linaro.org> writes:

> This is a case where we generate more than 64k code for a mere 231
> guest instructions.

I would like to know more! Are these unrolled vector ops or something else?

> This hits some assertions within TCG that we're
> not overflowing the uint16_t that we use for representing our
> unwind info.
>
> Fix this by returning an error indication, rather than asserting.
> This lets us try again from tb_gen_code with a lower max_insns.
>
> This should resolve the problem for x86 as a host.  There are other
> failure modes wrt out-of-range relocations that might affect the
> RISC hosts.  I'm going to leave those for a different patch set.
>
>
> r~
>
>
> Richard Henderson (2):
>   tcg: Hoist max_insns computation to tb_gen_code
>   tcg: Restart after TB code generation overflow
>
>  include/exec/exec-all.h       |  4 +--
>  include/exec/translator.h     |  3 +-
>  accel/tcg/translate-all.c     | 54 +++++++++++++++++++++++++++++------
>  accel/tcg/translator.c        | 15 ++--------
>  target/alpha/translate.c      |  4 +--
>  target/arm/translate.c        |  4 +--
>  target/cris/translate.c       | 10 +------
>  target/hppa/translate.c       |  5 ++--
>  target/i386/translate.c       |  4 +--
>  target/lm32/translate.c       | 10 +------
>  target/m68k/translate.c       |  4 +--
>  target/microblaze/translate.c | 10 +------
>  target/mips/translate.c       |  4 +--
>  target/moxie/translate.c      | 11 ++-----
>  target/nios2/translate.c      | 14 ++-------
>  target/openrisc/translate.c   |  4 +--
>  target/ppc/translate.c        |  4 +--
>  target/riscv/translate.c      |  4 +--
>  target/s390x/translate.c      |  4 +--
>  target/sh4/translate.c        |  4 +--
>  target/sparc/translate.c      |  4 +--
>  target/tilegx/translate.c     | 12 +-------
>  target/tricore/translate.c    | 16 ++---------
>  target/unicore32/translate.c  | 10 +------
>  target/xtensa/translate.c     |  4 +--
>  tcg/tcg.c                     |  4 +++
>  26 files changed, 93 insertions(+), 133 deletions(-)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 0/2] tcg: Fix launchpad 1824853

Posted by Richard Henderson 4 years, 12 months ago

On 4/19/19 1:07 PM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> This is a case where we generate more than 64k code for a mere 231
>> guest instructions.
> 
> I would like to know more! Are these unrolled vector ops or something else?

Yes.  E.g.

  ld4  { v0.16b - v3.16b }, [x0]

will generate 64 guest byte loads.  Given the size of the code
generated for each guest memory operation, we should probably
change this to use 64-bit loads and dole out the bytes manually.

Even for linux-user, with direct host memory ops this converts
to 1k code.

r~