All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Greg Kurz <groug@kaod.org>
Cc: Igor Mammedov <imammedo@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC] ppc: define spapr core types statically
Date: Thu, 28 Sep 2017 14:22:56 +1000	[thread overview]
Message-ID: <20170928042256.GE12504@umbus> (raw)
In-Reply-To: <20170927181834.76c5d2ef@bahia.lan>

[-- Attachment #1: Type: text/plain, Size: 9612 bytes --]

On Wed, Sep 27, 2017 at 06:18:34PM +0200, Greg Kurz wrote:
> On Wed, 27 Sep 2017 13:49:17 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> 
> This looks nice indeed (just one remark, see below).
> 
> Will you re-post as a patch series or do you prefer I do it ?

Looks good to me as well.

> 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/ppc/spapr_cpu_core.h |  5 ++-
> >  hw/ppc/spapr.c                  |  6 +--
> >  hw/ppc/spapr_cpu_core.c         | 93 ++++++++++++++++-------------------------
> >  target/ppc/kvm.c                | 11 -----
> >  4 files changed, 41 insertions(+), 74 deletions(-)
> > 
> > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> > index 93051e9..42765de 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;
> > @@ -32,9 +34,8 @@ typedef struct sPAPRCPUCore {
> >  
> >  typedef struct sPAPRCPUCoreClass {
> >      DeviceClass parent_class;
> > -    ObjectClass *cpu_class;
> > +    const char *cpu_type;
> >  } sPAPRCPUCoreClass;
> >  
> >  char *spapr_get_cpu_core_type(const char *model);
> > -void spapr_cpu_core_class_init(ObjectClass *oc, void *data);
> >  #endif
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 8c1a437..00cfe9a 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3125,8 +3125,7 @@ void spapr_core_release(DeviceState *dev)
> >      if (smc->pre_2_10_has_unused_icps) {
> >          sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> >          sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> > -        const char *typename = object_class_get_name(scc->cpu_class);
> > -        size_t size = object_type_get_instance_size(typename);
> > +        size_t size = object_type_get_instance_size(scc->cpu_type);
> >          int i;
> >  
> >          for (i = 0; i < cc->nr_threads; i++) {
> > @@ -3222,8 +3221,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> >  
> >      if (smc->pre_2_10_has_unused_icps) {
> >          sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> > -        const char *typename = object_class_get_name(scc->cpu_class);
> > -        size_t size = object_type_get_instance_size(typename);
> > +        size_t size = object_type_get_instance_size(scc->cpu_type);
> >          int i;
> >  
> >          for (i = 0; i < cc->nr_threads; i++) {
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 5bea4c9..8a18eaf 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -104,8 +104,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp)
> >  {
> >      sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> >      sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
> > -    const char *typename = object_class_get_name(scc->cpu_class);
> > -    size_t size = object_type_get_instance_size(typename);
> > +    size_t size = object_type_get_instance_size(scc->cpu_type);
> >      CPUCore *cc = CPU_CORE(dev);
> >      int i;
> >  
> > @@ -166,8 +165,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> >      sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> >      sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
> >      CPUCore *cc = CPU_CORE(OBJECT(dev));
> > -    const char *typename = object_class_get_name(scc->cpu_class);
> > -    size_t size = object_type_get_instance_size(typename);
> > +    size_t size = object_type_get_instance_size(scc->cpu_type);
> >      Error *local_err = NULL;
> >      void *obj;
> >      int i, j;
> > @@ -186,7 +184,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> >  
> >          obj = sc->threads + i * size;
> >  
> > -        object_initialize(obj, size, typename);
> > +        object_initialize(obj, size, scc->cpu_type);
> >          cs = CPU(obj);
> >          cpu = POWERPC_CPU(cs);
> >          cs->cpu_index = cc->core_id + i;
> > @@ -231,42 +229,12 @@ 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",
> > -};
> > -
> >  static Property spapr_cpu_core_properties[] = {
> >      DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, CPU_UNSET_NUMA_NODE_ID),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >  
> > -void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> > +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(oc);
> >      sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
> > @@ -274,36 +242,47 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> >      dc->realize = spapr_cpu_core_realize;
> >      dc->unrealize = spapr_cpu_core_unrealizefn;
> >      dc->props = spapr_cpu_core_properties;
> > -    scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data);
> > -    g_assert(scc->cpu_class);
> > +    scc->cpu_type = data;
> 
> Maybe assert data isn't NULL ?
> 
> >  }
> >  
> > -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),
> > +#define DEFINE_SPAPR_CPU_CORE_TYPE(cpu_model) \
> > +    {                                                   \
> > +        .parent = TYPE_SPAPR_CPU_CORE,                  \
> > +        .class_data = (void *) POWERPC_CPU_TYPE_NAME(cpu_model), \
> > +        .class_init = spapr_cpu_core_class_init,        \
> > +        .name = SPAPR_CPU_CORE_TYPE_NAME(cpu_model),    \
> > +    }
> > +
> > +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"),
> > +#ifdef CONFIG_KVM
> > +    DEFINE_SPAPR_CPU_CORE_TYPE("host"),
> > +#endif
> >  };
> >  
> >  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);
> > +    for (i = 0; i < ARRAY_SIZE(spapr_cpu_core_type_infos); i++) {
> > +        type_register_static(&spapr_cpu_core_type_infos[i]);
> >      }
> >  }
> >  
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 1deaf10..7e5c041 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -2507,17 +2507,6 @@ static int kvm_ppc_register_host_cpu_type(void)
> >      oc = object_class_by_name(type_info.name);
> >      g_assert(oc);
> >  
> > -#if defined(TARGET_PPC64)
> > -    type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host");
> > -    type_info.parent = TYPE_SPAPR_CPU_CORE,
> > -    type_info.instance_size = sizeof(sPAPRCPUCore);
> > -    type_info.instance_init = NULL;
> > -    type_info.class_init = spapr_cpu_core_class_init;
> > -    type_info.class_data = (void *) "host";
> > -    type_register(&type_info);
> > -    g_free((void *)type_info.name);
> > -#endif
> > -
> >      /*
> >       * Update generic CPU family class alias (e.g. on a POWER8NVL host,
> >       * we want "POWER8" to be a "family" alias that points to the current
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2017-09-28  5:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-25  9:47 [Qemu-devel] [PATCH] spapr: move registration of "host" CPU core type to machine code Greg Kurz
2017-09-25 13:41 ` Igor Mammedov
2017-09-25 15:48   ` Greg Kurz
2017-09-25 21:47     ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2017-09-27 12:19       ` Igor Mammedov
2017-09-27 15:32         ` Greg Kurz
2017-09-29  6:41           ` Igor Mammedov
2017-09-29  7:25             ` Greg Kurz
2017-09-26  2:57 ` [Qemu-devel] " David Gibson
2017-09-26  7:19   ` Greg Kurz
2017-09-26  8:29     ` Igor Mammedov
2017-09-27  6:39       ` David Gibson
2017-09-27 12:11         ` Igor Mammedov
2017-09-28  4:01           ` David Gibson
2017-09-27  6:21     ` David Gibson
2017-09-27  8:13       ` Igor Mammedov
2017-09-27 11:49       ` [Qemu-devel] [RFC] ppc: define spapr core types statically Igor Mammedov
2017-09-27 16:18         ` Greg Kurz
2017-09-28  4:22           ` David Gibson [this message]
2017-09-29  6:44           ` Igor Mammedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170928042256.GE12504@umbus \
    --to=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=imammedo@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.