Re: [Qemu-devel] [RFC v4 18/71] sh4: convert to cpu_halted

[Emilio G. Cota]

On Wed, Oct 31, 2018 at 13:54:14 +0000, Alex Bennée wrote:
> 
> Emilio G. Cota <address@hidden> writes:
> 
> > Cc: Aurelien Jarno <address@hidden>
> > Reviewed-by: Richard Henderson <address@hidden>
> > Signed-off-by: Emilio G. Cota <address@hidden>
> > ---
> >  target/sh4/op_helper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
> > index 4f825bae5a..57cc363ccc 100644
> > --- a/target/sh4/op_helper.c
> > +++ b/target/sh4/op_helper.c
> > @@ -105,7 +105,7 @@ void helper_sleep(CPUSH4State *env)
> >  {
> >      CPUState *cs = CPU(sh_env_get_cpu(env));
> >
> > -    cs->halted = 1;
> > +    cpu_halted_set(cs, 1);
> 
> Looks good:
> 
> Reviewed-by: Alex Bennée <address@hidden>

Thanks!

> >      env->in_sleep = 1;
> 
> I wonder if you could drop env->in_sleep from CPUSH4State?
> The test in superh_cpu_do_interrupt:
> 
>         if (do_irq && !env->in_sleep) {
>             return; /* masked */
>         }
>     }
>     env->in_sleep = 0;
> 
> maybe be simplified is we cpu_set_halted(cs, 0) when servicing a
> delivered irq?

I don't know this target well, but on a quick look I'd worry about
that change interfering with other (generic) setters of
cpu->halted, e.g. cpu_common_reset, as well as the CPU loop
where cpu->halted is read.

In any case I'd leave additional cleanups out of this series,
which is already so long that git rebase is choking a bit
every time I add an R-b tag :D

Thanks,

                Emilio