[Qemu-devel] [PATCH v2] sh4: fix use_icount with linux-user

[Qemu-devel] [PATCH v2] sh4: fix use_icount with linux-user

[Laurent Vivier]

This fixes java in a linux-user chroot:
  $ java --version
  qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
  qemu: uncaught target signal 6 (Aborted) - core dumped
  Aborted (core dumped)

In gen_conditional_jump() in the GUSA_EXCLUSIVE part, we must reset
base.is_jmp to DISAS_NEXT after the gen_goto_tb() as it is done in
gen_delayed_conditional_jump() after the gen_jump().

Bug: https://bugs.launchpad.net/qemu/+bug/1768246
Fixes: 4834871bc95b67343248100e2a75ae0d287bc08b
       ("target/sh4: Convert to DisasJumpType")
Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---

Notes:
    v2:
      don't revert the part of the original patch,
      but fixes the state problem in gen_conditional_jump()

 target/sh4/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 1b9a201d6d..ab254b0e8d 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -293,6 +293,7 @@ static void gen_conditional_jump(DisasContext *ctx, target_ulong dest,
            disallow it in use_goto_tb, but it handles exit + singlestep.  */
         gen_goto_tb(ctx, 0, dest);
         gen_set_label(l1);
+        ctx->base.is_jmp = DISAS_NEXT;
         return;
     }
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2] sh4: fix use_icount with linux-user

[John Paul Adrian Glaubitz]

On 08/11/2018 10:23 AM, Laurent Vivier wrote:
> This fixes java in a linux-user chroot:
>   $ java --version
>   qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
>   qemu: uncaught target signal 6 (Aborted) - core dumped
>   Aborted (core dumped)
> 
> In gen_conditional_jump() in the GUSA_EXCLUSIVE part, we must reset
> base.is_jmp to DISAS_NEXT after the gen_goto_tb() as it is done in
> gen_delayed_conditional_jump() after the gen_jump().
> 
> Bug: https://bugs.launchpad.net/qemu/+bug/1768246
> Fixes: 4834871bc95b67343248100e2a75ae0d287bc08b
>        ("target/sh4: Convert to DisasJumpType")
> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Thanks, testing this revision now as well.

Both patches finally allow me to use much newer QEMU versions for SH4,
before that I was stuck to versions from before the regression was
introduced.

So far, the overall improvement is quite spectacular and even the
Haskell compiler GHC now works much more reliable on qemu-sh4 than
it did in the past.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [Qemu-devel] [PATCH v2] sh4: fix use_icount with linux-user

[Richard Henderson]

On 08/11/2018 01:23 AM, Laurent Vivier wrote:
> This fixes java in a linux-user chroot:
>   $ java --version
>   qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
>   qemu: uncaught target signal 6 (Aborted) - core dumped
>   Aborted (core dumped)
> 
> In gen_conditional_jump() in the GUSA_EXCLUSIVE part, we must reset
> base.is_jmp to DISAS_NEXT after the gen_goto_tb() as it is done in
> gen_delayed_conditional_jump() after the gen_jump().
> 
> Bug: https://bugs.launchpad.net/qemu/+bug/1768246
> Fixes: 4834871bc95b67343248100e2a75ae0d287bc08b
>        ("target/sh4: Convert to DisasJumpType")
> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> 
> Notes:
>     v2:
>       don't revert the part of the original patch,
>       but fixes the state problem in gen_conditional_jump()

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v2] sh4: fix use_icount with linux-user

[Laurent Vivier]

Le 11/08/2018 à 17:26, Richard Henderson a écrit :
> On 08/11/2018 01:23 AM, Laurent Vivier wrote:
>> This fixes java in a linux-user chroot:
>>   $ java --version
>>   qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
>>   qemu: uncaught target signal 6 (Aborted) - core dumped
>>   Aborted (core dumped)
>>
>> In gen_conditional_jump() in the GUSA_EXCLUSIVE part, we must reset
>> base.is_jmp to DISAS_NEXT after the gen_goto_tb() as it is done in
>> gen_delayed_conditional_jump() after the gen_jump().
>>
>> Bug: https://bugs.launchpad.net/qemu/+bug/1768246
>> Fixes: 4834871bc95b67343248100e2a75ae0d287bc08b
>>        ("target/sh4: Convert to DisasJumpType")
>> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>
>> Notes:
>>     v2:
>>       don't revert the part of the original patch,
>>       but fixes the state problem in gen_conditional_jump()
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Aurélien,

do you agree if I push this patch through a linux-user pull request?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2] sh4: fix use_icount with linux-user

[Aurelien Jarno]

On 2018-08-16 20:58, Laurent Vivier wrote:
> Le 11/08/2018 à 17:26, Richard Henderson a écrit :
> > On 08/11/2018 01:23 AM, Laurent Vivier wrote:
> >> This fixes java in a linux-user chroot:
> >>   $ java --version
> >>   qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
> >>   qemu: uncaught target signal 6 (Aborted) - core dumped
> >>   Aborted (core dumped)
> >>
> >> In gen_conditional_jump() in the GUSA_EXCLUSIVE part, we must reset
> >> base.is_jmp to DISAS_NEXT after the gen_goto_tb() as it is done in
> >> gen_delayed_conditional_jump() after the gen_jump().
> >>
> >> Bug: https://bugs.launchpad.net/qemu/+bug/1768246
> >> Fixes: 4834871bc95b67343248100e2a75ae0d287bc08b
> >>        ("target/sh4: Convert to DisasJumpType")
> >> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> >> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> >> ---
> >>
> >> Notes:
> >>     v2:
> >>       don't revert the part of the original patch,
> >>       but fixes the state problem in gen_conditional_jump()
> > 
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

> Aurélien,
> 
> do you agree if I push this patch through a linux-user pull request?

Yes, that's fine with me.

Thanks,
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net