Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 13/24] ppc: spapr: define core types statically

[Igor Mammedov]

On Thu, 12 Oct 2017 15:10:30 +0200
Greg Kurz <address@hidden> wrote:

> On Tue, 10 Oct 2017 09:29:46 +0200
> Greg Kurz <address@hidden> wrote:
> 
> > On Mon,  9 Oct 2017 21:51:00 +0200
> > Igor Mammedov <address@hidden> wrote:
> >   
> > > spapr core type definition doesn't have any fields that
> > > require it to be defined at runtime. So replace code
> > > that fills in TypeInfo at runtime with static TypeInfo
> > > array that does the same at complie time.
> > > 
> > > Signed-off-by: Igor Mammedov <address@hidden>
> > > ---
> > > v2:
> > >   - fix rebase conflict: add newly added power9_v2 core
> > > ---    
> > 
> > Reviewed-by: Greg Kurz <address@hidden>
> >   
> 
> Oops, this patch causes a regression when you try to pass a sPAPR core to
> a non-pseries machine.
> 
> Without this patch:
> 
> $ ppc64-softmmu/qemu-system-ppc64 -M none -device host-spapr-cpu-core 
> qemu-system-ppc64: -device host-spapr-cpu-core:
>  'host-spapr-cpu-core' is not a valid device model name
> 
> With this patch:
> 
> $ ppc64-softmmu/qemu-system-ppc64 -M none -device host-spapr-cpu-core 
> **
> ERROR:qom/object.c:217:object_type_get_instance_size:
>  assertion failed: (type != NULL)
> Aborted
there shouldn't be type host-spapr-cpu-core with this patch since you don't 
have -enable-kvm on CLI
maybe you are talking about
    [PATCH v2 15/24] ppc: spapr: register 'host' core type  along with the rest 
of core types


[...]

 
> I think that this patch should be reverted from David's staging tree for now, 
> I'll
> repost it when I have a proposal for the host CPU.
let me find ppc host and look into it before reverting,
maybe there is easier way around this 'very corner' case.


> 
> Cheers,
> 
> --
> Greg
> 
> > >  include/hw/ppc/spapr_cpu_core.h |  2 +
> > >  hw/ppc/spapr_cpu_core.c         | 87 
> > > +++++++++++++----------------------------
> > >  2 files changed, 30 insertions(+), 59 deletions(-)
> > > 
> > > diff --git a/include/hw/ppc/spapr_cpu_core.h 
> > > b/include/hw/ppc/spapr_cpu_core.h
> > > index 93051e9..66dcf52 100644
> > > --- a/include/hw/ppc/spapr_cpu_core.h
> > > +++ b/include/hw/ppc/spapr_cpu_core.h
> > > @@ -21,6 +21,8 @@
> > >  #define SPAPR_CPU_CORE_GET_CLASS(obj) \
> > >       OBJECT_GET_CLASS(sPAPRCPUCoreClass, (obj), TYPE_SPAPR_CPU_CORE)
> > >  
> > > +#define SPAPR_CPU_CORE_TYPE_NAME(model) model "-" TYPE_SPAPR_CPU_CORE
> > > +
> > >  typedef struct sPAPRCPUCore {
> > >      /*< private >*/
> > >      CPUCore parent_obj;
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index b6610dd..550d320 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -217,37 +217,6 @@ err:
> > >      error_propagate(errp, local_err);
> > >  }
> > >  
> > > -static const char *spapr_core_models[] = {
> > > -    /* 970 */
> > > -    "970_v2.2",
> > > -
> > > -    /* 970MP variants */
> > > -    "970mp_v1.0",
> > > -    "970mp_v1.1",
> > > -
> > > -    /* POWER5+ */
> > > -    "power5+_v2.1",
> > > -
> > > -    /* POWER7 */
> > > -    "power7_v2.3",
> > > -
> > > -    /* POWER7+ */
> > > -    "power7+_v2.1",
> > > -
> > > -    /* POWER8 */
> > > -    "power8_v2.0",
> > > -
> > > -    /* POWER8E */
> > > -    "power8e_v2.1",
> > > -
> > > -    /* POWER8NVL */
> > > -    "power8nvl_v1.0",
> > > -
> > > -    /* POWER9 */
> > > -    "power9_v1.0",
> > > -    "power9_v2.0",
> > > -};
> > > -
> > >  static Property spapr_cpu_core_properties[] = {
> > >      DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, 
> > > CPU_UNSET_NUMA_NODE_ID),
> > >      DEFINE_PROP_END_OF_LIST()
> > > @@ -265,33 +234,33 @@ void spapr_cpu_core_class_init(ObjectClass *oc, 
> > > void *data)
> > >      g_assert(scc->cpu_class);
> > >  }
> > >  
> > > -static const TypeInfo spapr_cpu_core_type_info = {
> > > -    .name = TYPE_SPAPR_CPU_CORE,
> > > -    .parent = TYPE_CPU_CORE,
> > > -    .abstract = true,
> > > -    .instance_size = sizeof(sPAPRCPUCore),
> > > -    .class_size = sizeof(sPAPRCPUCoreClass),
> > > -};
> > > -
> > > -static void spapr_cpu_core_register_types(void)
> > > -{
> > > -    int i;
> > > -
> > > -    type_register_static(&spapr_cpu_core_type_info);
> > > -
> > > -    for (i = 0; i < ARRAY_SIZE(spapr_core_models); i++) {
> > > -        TypeInfo type_info = {
> > > -            .parent = TYPE_SPAPR_CPU_CORE,
> > > -            .instance_size = sizeof(sPAPRCPUCore),
> > > -            .class_init = spapr_cpu_core_class_init,
> > > -            .class_data = (void *) spapr_core_models[i],
> > > -        };
> > > -
> > > -        type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE,
> > > -                                         spapr_core_models[i]);
> > > -        type_register(&type_info);
> > > -        g_free((void *)type_info.name);
> > > +#define DEFINE_SPAPR_CPU_CORE_TYPE(cpu_model) \
> > > +    {                                                   \
> > > +        .parent = TYPE_SPAPR_CPU_CORE,                  \587
> > > +        .class_data = (void *) cpu_model,               \
> > > +        .class_init = spapr_cpu_core_class_init,        \
> > > +        .name = SPAPR_CPU_CORE_TYPE_NAME(cpu_model),    \
> > >      }
> > > -}
> > >  
> > > -type_init(spapr_cpu_core_register_types)
> > > +static const TypeInfo spapr_cpu_core_type_infos[] = {
> > > +    {
> > > +        .name = TYPE_SPAPR_CPU_CORE,
> > > +        .parent = TYPE_CPU_CORE,
> > > +        .abstract = true,
> > > +        .instance_size = sizeof(sPAPRCPUCore),
> > > +        .class_size = sizeof(sPAPRCPUCoreClass),
> > > +    },
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("970_v2.2"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.0"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.1"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power5+_v2.1"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power7_v2.3"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power7+_v2.1"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power8_v2.0"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power8e_v2.1"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
> > > +    DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
> > > +};
> > > +
> > > +DEFINE_TYPES(spapr_cpu_core_type_infos)    
> > 
> >   
>