Re: [Qemu-ppc] [PATCH 1/2] spapr_cpu_core: migrate per-CPU data

[Greg Kurz]

On Tue, 19 Jun 2018 10:09:05 +1000
David Gibson <address@hidden> wrote:

> On Mon, Jun 18, 2018 at 02:26:35PM +0200, Greg Kurz wrote:
> > A per-CPU machine data pointer was recently added to PowerPCCPU. The
> > motivation is to to hide platform specific details from the core CPU
> > code. This per-CPU data can hold state which is revelant to the guest
> > though, eg, Virtual Processor Areas, and we whould migrate this state.
> > 
> > This patch adds the plumbing so that we can migrate the per-CPU data
> > for PAPR guests. We only do this for newer machine types for the sake
> > of backword compatibility. No state is migrated for the moment: the
> > vmstate_spapr_cpu_state structure will be populated by subsequent
> > patches.
> > 
> > Signed-off-by: Greg Kurz <address@hidden>  
> 
> Applied, though I fixed some spelling and spacing errors along the way.
> 

Thanks but I now realize I have a nit...

> > ---
> >  hw/ppc/spapr.c                  |    5 +++++
> >  hw/ppc/spapr_cpu_core.c         |   27 +++++++++++++++++++++++----
> >  include/hw/ppc/spapr_cpu_core.h |    1 +
> >  3 files changed, 29 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index db0fb385d4e0..37db3e8bc6ca 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -4116,6 +4116,11 @@ DEFINE_SPAPR_MACHINE(3_0, "3.0", true);
> >      {                                                                  \
> >          .driver = TYPE_POWERPC_CPU,                                    \
> >          .property = "pre-3.0-migration",                              \
> > +        .value    = "on",                                              \
> > +            },                                                         \
> > +    {                                                                  \
> > +        .driver = TYPE_SPAPR_CPU_CORE,                                 \
> > +        .property = "pre-3.0-migration",                               \
> >          .value    = "on",                                              \
> >      },
> >  
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index aef3be33a3bb..96d1dfad00e1 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -129,6 +129,15 @@ static void spapr_cpu_core_unrealize(DeviceState *dev, 
> > Error **errp)
> >      g_free(sc->threads);
> >  }
> >  
> > +static const VMStateDescription vmstate_spapr_cpu_state = {
> > +    .name = "spapr_cpu",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_END_OF_LIST()
> > +    },
> > +};
> > +
> >  static void spapr_realize_vcpu(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >                                 Error **errp)
> >  {
> > @@ -164,7 +173,8 @@ error:
> >      error_propagate(errp, local_err);
> >  }
> >  
> > -static PowerPCCPU *spapr_create_vcpu(sPAPRCPUCore *sc, int i, Error **errp)
> > +static PowerPCCPU *spapr_create_vcpu(sPAPRCPUCore *sc, int i,
> > +                                     sPAPRMachineState *spapr, Error 
> > **errp)

... here. This spapr argument comes from a previous version. I didn't need it
in the end, but I forgot to remove it :(

Do you want me to resend or can you fix that in your tree ?

> >  {
> >      sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(sc);
> >      CPUCore *cc = CPU_CORE(sc);
> > @@ -194,6 +204,10 @@ static PowerPCCPU *spapr_create_vcpu(sPAPRCPUCore *sc, 
> > int i, Error **errp)
> >      }
> >  
> >      cpu->machine_data = g_new0(sPAPRCPUState, 1);
> > +    if (!sc->pre_3_0_migration) {
> > +        vmstate_register(NULL, cs->cpu_index, &vmstate_spapr_cpu_state,
> > +                         cpu->machine_data);
> > +    }
> >  
> >      object_unref(obj);
> >      return cpu;
> > @@ -204,10 +218,13 @@ err:
> >      return NULL;
> >  }
> >  
> > -static void spapr_delete_vcpu(PowerPCCPU *cpu)
> > +static void spapr_delete_vcpu(PowerPCCPU *cpu, sPAPRCPUCore *sc)
> >  {
> >      sPAPRCPUState *spapr_cpu = spapr_cpu_state(cpu);
> >  
> > +    if (!sc->pre_3_0_migration) {
> > +        vmstate_unregister(NULL, &vmstate_spapr_cpu_state, 
> > cpu->machine_data);
> > +    }
> >      cpu->machine_data = NULL;
> >      g_free(spapr_cpu);
> >      object_unparent(OBJECT(cpu));
> > @@ -233,7 +250,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> > Error **errp)
> >  
> >      sc->threads = g_new(PowerPCCPU *, cc->nr_threads);
> >      for (i = 0; i < cc->nr_threads; i++) {
> > -        sc->threads[i] = spapr_create_vcpu(sc, i, &local_err);
> > +        sc->threads[i] = spapr_create_vcpu(sc, i, spapr, &local_err);
> >          if (local_err) {
> >              goto err;
> >          }
> > @@ -253,7 +270,7 @@ err_unrealize:
> >      }
> >  err:
> >      while (--i >= 0) {
> > -        spapr_delete_vcpu(sc->threads[i]);
> > +        spapr_delete_vcpu(sc->threads[i], sc);
> >      }
> >      g_free(sc->threads);
> >      error_propagate(errp, local_err);
> > @@ -261,6 +278,8 @@ err:
> >  
> >  static Property spapr_cpu_core_properties[] = {
> >      DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, 
> > CPU_UNSET_NUMA_NODE_ID),
> > +    DEFINE_PROP_BOOL("pre-3.0-migration", sPAPRCPUCore, pre_3_0_migration,
> > +                     false),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >  
> > diff --git a/include/hw/ppc/spapr_cpu_core.h 
> > b/include/hw/ppc/spapr_cpu_core.h
> > index 8ceea2973a93..9e2821e4b31f 100644
> > --- a/include/hw/ppc/spapr_cpu_core.h
> > +++ b/include/hw/ppc/spapr_cpu_core.h
> > @@ -31,6 +31,7 @@ typedef struct sPAPRCPUCore {
> >      /*< public >*/
> >      PowerPCCPU **threads;
> >      int node_id;
> > +    bool pre_3_0_migration; /* older machine don't know about 
> > sPAPRCPUState */
> >  } sPAPRCPUCore;
> >  
> >  typedef struct sPAPRCPUCoreClass {
> >   
>

pgpxlAwfnR55Y.pgp
Description: OpenPGP digital signature