Re: [Qemu-devel] [PATCH v2 03/20] target-ppc: move back cpu_exec_init() to init

[Bharata B Rao]

On Fri, Oct 14, 2016 at 03:07:51PM +1100, David Gibson wrote:
> On Thu, Oct 13, 2016 at 06:24:45PM +0200, Laurent Vivier wrote:
> > We have now the cpu_exec_realize() in realize,
> > so the init part must be in init.
> > 
> > As cpu_exec_unrealize() is called from cpu_common_finalize(),
> > remove the call from ppc_cpu_unrealizefn().
> > 
> > CC: Bharata B Rao <address@hidden>
> > CC: Alexander Graf <address@hidden>
> > CC: address@hidden
> > Signed-off-by: Laurent Vivier <address@hidden>
> > ---
> >  target-ppc/translate_init.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> > index 094f28a..bbca8b5 100644
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -9678,7 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
> > **errp)
> >      }
> >  #endif
> >  
> > -    cpu_exec_init(cs);
> >      cpu_exec_realize(cs, &local_err);
> >      if (local_err != NULL) {
> >          error_propagate(errp, local_err);
> > @@ -9911,8 +9910,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, 
> > Error **errp)
> >      opc_handler_t **table, **table_2;
> >      int i, j, k;
> >  
> > -    cpu_exec_unrealize(CPU(dev));
> > -
> 
> This doesn't seem right.  As you said in 0/20, cpu_exec_unrealize() is
> called from cpu_common_finalize().  But finalize should mirror init,
> not unrealize().  So it seems that unrealize() really should belong
> here, not in finalize.

For archs like PowerPC, cpu_exec_exit() was being called twice: once
from PowerPC CPU's unrealize function and once from cpu_common_finalize().
cpu_exec_exit() had two vmstate_unregister() calls and it used to ensure
that they are not called twice, but looks like this got changed sometime
back and we are now executing these two vmstate_unregister() calls twice.

While you are here, could you please take care of this ?

Regards,
Bharata.