On Mon, 25 Sep 2017 17:48:57 +0200
Greg Kurz <gr...@kaod.org> wrote:
> On Mon, 25 Sep 2017 15:41:34 +0200
> Igor Mammedov <imamm...@redhat.com> wrote:
>
> > On Mon, 25 Sep 2017 11:47:33 +0200
> > Greg Kurz <gr...@kaod.org> wrote:
> >
> > > The CPU core abstraction belongs to the machine code. This also gets
> > > rid of some code duplication.
> > >
> > > Signed-off-by: Greg Kurz <gr...@kaod.org>
> > > ---
> > >
> > > hw/ppc/spapr_cpu_core.h is also included elsewhere in target/ppc/kvm.c
> > > but this is already handled by the following cleanup patch:
> > >
> > > https://patchwork.ozlabs.org/patch/817598/
> > > ---
> > > hw/ppc/spapr.c | 4 ++++
> > > hw/ppc/spapr_cpu_core.c | 34 ++++++++++++++++++++++------------
> > > include/hw/ppc/spapr_cpu_core.h | 2 +-
> > > target/ppc/kvm.c | 12 ------------
> > > 4 files changed, 27 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 0ce3ec87ac59..e82c8532ffb0 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2349,6 +2349,10 @@ static void ppc_spapr_init(MachineState *machine)
> > > }
> > >
> > > /* init CPUs */
> > > + if (kvm_enabled()) {
> > > + spapr_cpu_core_register_host_type();
> > > + }
> > why don't we create it statically in hw/ppc/spapr_cpu_core.c
> > like it's done in x86, i.e.
> >
> > static void x86_cpu_register_types(void)
> >
> > {
> >
> > ...
> > #ifdef CONFIG_KVM
> >
> > type_register_static(&host_x86_cpu_type_info);
> >
> > #endif
> >
> > }
> > type_init(x86_cpu_register_types)
> >
> > and do the same for host CPU as well?
> >
>
> Hi Igor,
>
> Not sure yet why we use dynamic types, but I'd be glad to dig a bit more.
So the problem is that it was decided to make the host CPU class a
subclass of the host's CPU model, and this requires all the CPU model
classes to be registered beforehand.
commit 5ba4576b858c0d6056f59abb7e17a2b63f7905f3
Author: Andreas Färber <afaer...@suse.de>
Date: Sat Feb 23 11:22:12 2013 +0000
target-ppc: Make host CPU a subclass of the host's CPU model
This avoids assigning individual class fields and contributors
forgetting to add field assignments in KVM-only code.
ppc_cpu_class_find_by_pvr() requires the CPU model classes to be
registered, so defer host CPU type registration to kvm_arch_init().
Only register the host CPU type if there is a class with matching PVR.
This lets us drop error handling from instance_init.
Signed-off-by: Andreas Färber <afaer...@suse.de>
Signed-off-by: Alexander Graf <ag...@suse.de>
I can't think of an alternate way to do this. Any suggestion ?
> Maybe I'll do this in a followup patch though, since the present patch
> is mostly about not referencing sPAPRCPUCore from the KVM code anymore.
>
> > my gripe with 'dynamic' types is that it creates problems
> > if one needs to access type information before type is created.
> > In my use-case it's before machine_init() is called but
> > after kvm is initialized, so I was sort of 'fine' with way
> > it's currently handled /i.e. still too much convoluted with
> > aliases redefinition and all but sort of 'manageable' /.
> >
> > However I'd very much prefer static type info so it
> > could be used regardless of place/time it's accessed.
> >
>
> I agree that it would be better.
>
> > I can add a couple patches to that effect into upcomming
> > cpu_model removal series, if that's ok for PPC folks.
> >
>
> Oh, and can you share an url to see how it looks like ?
>
> Cheers,
>
> --
> Greg
>
> >
> > > +
> > > if (machine->cpu_model == NULL) {
> > > machine->cpu_model = kvm_enabled() ? "host" :
> > > smc->tcg_default_cpu;
> > > }
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index c08ee7571a50..6e224ba029ec 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -317,7 +317,7 @@ static Property spapr_cpu_core_properties[] = {
> > > 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);
> > > @@ -337,6 +337,20 @@ static const TypeInfo spapr_cpu_core_type_info = {
> > > .class_size = sizeof(sPAPRCPUCoreClass),
> > > };
> > >
> > > +static void spapr_cpu_core_register_type(const char *model_name)
> > > +{
> > > + TypeInfo type_info = {
> > > + .parent = TYPE_SPAPR_CPU_CORE,
> > > + .instance_size = sizeof(sPAPRCPUCore),
> > > + .class_init = spapr_cpu_core_class_init,
> > > + .class_data = (void *) model_name,
> > > + };
> > > +
> > > + type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE,
> > > model_name);
> > > + type_register(&type_info);
> > > + g_free((void *)type_info.name);
> > > +}
> > > +
> > > static void spapr_cpu_core_register_types(void)
> > > {
> > > int i;
> > > @@ -344,18 +358,14 @@ static void spapr_cpu_core_register_types(void)
> > > 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);
> > > + spapr_cpu_core_register_type(spapr_core_models[i]);
> > > }
> > > +
> > > +}
> > > +
> > > +void spapr_cpu_core_register_host_type(void)
> > > +{
> > > + spapr_cpu_core_register_type("host");
> > > }
> > >
> > > type_init(spapr_cpu_core_register_types)
> > > diff --git a/include/hw/ppc/spapr_cpu_core.h
> > > b/include/hw/ppc/spapr_cpu_core.h
> > > index 93051e9ecf56..e3e906343048 100644
> > > --- a/include/hw/ppc/spapr_cpu_core.h
> > > +++ b/include/hw/ppc/spapr_cpu_core.h
> > > @@ -36,5 +36,5 @@ typedef struct sPAPRCPUCoreClass {
> > > } sPAPRCPUCoreClass;
> > >
> > > char *spapr_get_cpu_core_type(const char *model);
> > > -void spapr_cpu_core_class_init(ObjectClass *oc, void *data);
> > > +void spapr_cpu_core_register_host_type(void);
> > > #endif
> > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > > index 5b281b2f1b6d..8dd80993ec9e 100644
> > > --- a/target/ppc/kvm.c
> > > +++ b/target/ppc/kvm.c
> > > @@ -37,7 +37,6 @@
> > > #include "hw/sysbus.h"
> > > #include "hw/ppc/spapr.h"
> > > #include "hw/ppc/spapr_vio.h"
> > > -#include "hw/ppc/spapr_cpu_core.h"
> > > #include "hw/ppc/ppc.h"
> > > #include "sysemu/watchdog.h"
> > > #include "trace.h"
> > > @@ -2502,17 +2501,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
> > >
> > >
> >
>
>
>
--
Gregory Kurz kurzg...@fr.ibm.com
gk...@linux.vnet.ibm.com
Software Engineer @ IBM/LTC http://www.ibm.com
Tel 33-5-6218-1607
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
