Re: [PATCH v2] target/sh4: Fix TB_FLAG_UNALIGN

[Richard Henderson]

On 12/11/22 19:13, Guenter Roeck wrote:
> On Sat, Dec 10, 2022 at 07:27:46AM -0800, Guenter Roeck wrote:
> > Hi,
> >
> > On Thu, Sep 01, 2022 at 11:15:09AM +0100, Richard Henderson wrote:
> > > The value previously chosen overlaps GUSA_MASK.
> > >
> > > Rename all DELAY_SLOT_* and GUSA_* defines to emphasize
> > > that they are included in TB_FLAGs.  Add aliases for the
> > > FPSCR and SR bits that are included in TB_FLAGS, so that
> > > we don't accidentally reassign those bits.
> > >
> > > Fixes: 4da06fb3062 ("target/sh4: Implement prctl_unalign_sigbus")
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/856
> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >
> > I noticed that my sh4 emulations crash randomly with qemu v7.2-rc4.
> > This happens with all Linux kernel versions. Testing shows that this
> > patch is responsible. Reverting it fixes the problem.
>
> The patch below fixes the problem for me.

Thanks for the investigation.


> +++ b/target/sh4/cpu.c
> @@ -47,7 +47,7 @@ static void superh_cpu_synchronize_from_tb(CPUState *cs,
>       SuperHCPU *cpu = SUPERH_CPU(cs);
>   
>       cpu->env.pc = tb_pc(tb);
> -    cpu->env.flags = tb->flags;
> +    cpu->env.flags = tb->flags & TB_FLAG_ENVFLAGS_MASK;

Only this hunk should be necessary.



>   }
>   
>   static void superh_restore_state_to_opc(CPUState *cs,
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 7db3468b01..546c182463 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -2270,7 +2270,6 @@ static void sh4_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>                     (tbflags & (1 << SR_RB))) * 0x10;
>       ctx->fbank = tbflags & FPSCR_FR ? 0x10 : 0;
>   
> -#ifdef CONFIG_USER_ONLY
>       if (tbflags & TB_FLAG_GUSA_MASK) {
>           /* In gUSA exclusive region. */
>           uint32_t pc = ctx->base.pc_next;
> @@ -2290,7 +2289,6 @@ static void sh4_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>               return;
>           }
>       }
> -#endif

This one is actively wrong.


r~