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

[Alex Bennée]

Emilio G. Cota <address@hidden> writes:

> 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

No worries, it was more a general query thrown in the direction of the
sh4 maintainers ;-)

--
Alex Bennée