On Fri, 6 Oct 2017 16:04:39 +1100
David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Thu, Oct 05, 2017 at 06:24:44PM +0200, Igor Mammedov wrote:
> > use generic cpu_model parsing introduced by
> > (6063d4c0f vl.c: convert cpu_model to cpu type and set of global
> > properties before machine_init())
> >
> > it allows to:
> > * replace sPAPRMachineClass::tcg_default_cpu with
> > MachineClass::default_cpu_type
> > * drop cpu_parse_cpu_model() from hw/ppc/spapr.c and reuse
> > one in vl.c
> > * simplify spapr_get_cpu_core_type() by removing
> > not needed anymore recurrsion since alias look up
> > happens earlier at vl.c and spapr_get_cpu_core_type()
> > works only with resulted from that cpu type.
> > * spapr no more needs to parse/depend on being phased out
> > MachineState::cpu_model, all tha parsing done by generic
> > code and target specific callback.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > PS:
> > patch is a little bit big but it's hard to split it due
> > to dependencies, so pls bear with it.
>
> [snip]
> > -static int kvm_ppc_register_host_cpu_type(void)
> > +static int kvm_ppc_register_host_cpu_type(MachineState *ms)
> > {
> > TypeInfo type_info = {
> > .name = TYPE_HOST_POWERPC_CPU,
> > .class_init = kvmppc_host_cpu_class_init,
> > };
> > + MachineClass *mc = MACHINE_GET_CLASS(ms);
> > PowerPCCPUClass *pvr_pcc;
> > ObjectClass *oc;
> > DeviceClass *dc;
> > @@ -2504,6 +2505,8 @@ static int kvm_ppc_register_host_cpu_type(void)
> > }
> > type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
> > type_register(&type_info);
> > + /* override TCG default cpu type with 'host' cpu model */
> > + mc->default_cpu_type = TYPE_HOST_POWERPC_CPU;
>
> Ugh. I dislike this, it changes the default cpu type to 'host' for
> *all* machine types, whereas previously it was just true for pseries.
> That wasn't by accident.
>
> In general, I think we want there to be a single default CPU type for
> a machine, regardless of TCG vs. KVM. This is particularly true for
> many of the ppc machine types - they will only work with a CPU in the
> right "family" (e500 / 440 / POWERx, wahtever); in many of these cases
> KVM PR is usable, but using -cpu host could easily break the machine
> type.
>
> 'pseries' is different, it (and only it) can be used with KVM HV - and
> usually is. And KVM HV *only* works with -cpu host, due to hardware
> limitations.
I'd keep override in spapr_machine_class_init() but it's not possible
since it's called before kvm option is parsed and machine is prerequisite for
kvm option
how about following fixup?
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index abc222e..48dc3f7 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2505,8 +2505,10 @@ static int kvm_ppc_register_host_cpu_type(MachineState
*ms)
}
type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
type_register(&type_info);
- /* override TCG default cpu type with 'host' cpu model */
- mc->default_cpu_type = TYPE_HOST_POWERPC_CPU;
+ if (object_dynamic_cast(ms, TYPE_SPAPR_MACHINE)) {
+ /* override TCG default cpu type with 'host' cpu model */
+ mc->default_cpu_type = TYPE_HOST_POWERPC_CPU;
+ }
oc = object_class_by_name(type_info.name);
g_assert(oc);
> > oc = object_class_by_name(type_info.name);
> > g_assert(oc);
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index 5ee91e8..a968e86 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -10277,6 +10277,19 @@ PowerPCCPUClass
> > *ppc_cpu_class_by_pvr_mask(uint32_t pvr)
> > return pcc;
> > }
> >
> > +static const char *ppc_cpu_lookup_alias(const char *alias)
> > +{
> > + int ai;
> > +
> > + for (ai = 0; ppc_cpu_aliases[ai].alias != NULL; ai++) {
> > + if (strcmp(ppc_cpu_aliases[ai].alias, alias) == 0) {
> > + return ppc_cpu_aliases[ai].model;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > static ObjectClass *ppc_cpu_class_by_name(const char *name)
> > {
> > char *cpu_model, *typename;
> > @@ -10370,19 +10383,6 @@ static void ppc_cpu_parse_featurestr(const char
> > *typename, char *features,
> > pcc->parent_parse_features(typename, features, errp);
> > }
> >
> > -const char *ppc_cpu_lookup_alias(const char *alias)
> > -{
> > - int ai;
> > -
> > - for (ai = 0; ppc_cpu_aliases[ai].alias != NULL; ai++) {
> > - if (strcmp(ppc_cpu_aliases[ai].alias, alias) == 0) {
> > - return ppc_cpu_aliases[ai].model;
> > - }
> > - }
> > -
> > - return NULL;
> > -}
> > -
>
> This code motion of ppc_cpu_lookup_alias appears to be unrelated to
> the rest of the patch.
>
> > PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
> > {
> > ObjectClass *oc = OBJECT_CLASS(pcc);
>
