diff mbox series

[v2] sh4: fix use_icount with linux-user

Message ID 20180811082328.11268-1-laurent@vivier.eu
State New
Headers show
Series [v2] sh4: fix use_icount with linux-user | expand

Commit Message

Laurent Vivier Aug. 11, 2018, 8:23 a.m. UTC
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(+)

Comments

John Paul Adrian Glaubitz Aug. 11, 2018, 3:22 p.m. UTC | #1
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
Richard Henderson Aug. 11, 2018, 3:26 p.m. UTC | #2
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~
Laurent Vivier Aug. 16, 2018, 6:58 p.m. UTC | #3
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
Aurelien Jarno Aug. 19, 2018, 9:07 p.m. UTC | #4
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
diff mbox series

Patch

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;
     }