On Mon, Jun 27, 2016 at 04:51:34PM +1000, David Gibson wrote:
> On Mon, Jun 27, 2016 at 11:49:46AM +0530, Bharata B Rao wrote:
> > Compat CPU type is typically specified on -cpu cmdline option like:
> > -cpu host,compat=power7 or -cpu POWER8E,compat=power7 etc. When
> > compat is specified on -cpu cmdline, apply the same to all the
> > CPUs that get created as part of CPU core devices.
> >
> > This patch takes care of compat property that is specified with
> > -cpu cmdline option only. The other way to specify this property
> > is via -global cmdline option and that usage isn't addressed by
> > this patch because -global is already broken for some CPU class
> > in PowerPC. There are two issues with using -global on CPU class.
> >
> > - We specify alias names for CPUs commonly and use of alias names
> > won't work with -global seamlessly. For eg, When "-cpu host" is
> > specified, the actual CPU type that gets created
> > is host-powerpc64-cpu. Hence specifying -global host.compat=power7
> > will not work, so it has to be -global host-powerpc64-cpu.compat=power7.
> >
> > - PowerPC class names have . (dot) and opts parsing doesn't like it.
> > Specifying -global POWER8E_v2.1-powerpc64-cpu.compat=power7 will not
> > work as the driver name gets extracted as POWER8E_v2 and hence setting
> > of global property fails.
> >
> > The above two aspects could be considered/fixed separately from this
> > patch as this patch allows existing uses of -cpu cpuname,compat= to
> > work correctly after the introducton of sPAPR CPU cores.
> >
> > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
> > ---
> > Changes in v2:
> > - No need for a separate property named compat for cores.
> > - Simplified spapr_get_cpu_compat_type() based on David's review.
>
> I'm still not seeing why this is actually necessary. If the compat
> property is specified with -global, it will automatically be
> propagated to all threads without core intervention, yes?
>
> So shouldn't we just make the -cpu fallback effectively set the same
> -global property, rather than bouncing it through the core object?
Yes and the below patch against Igor's cpu_parse_into_global_props_V2
branch exactly does that for sPAPR.
===========
spapr: Parse CPU feature only once
From: Bharata B Rao <bhar...@linux.vnet.ibm.com>
Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0636642..be24a37 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1727,6 +1727,10 @@ static void ppc_spapr_init(MachineState *machine)
long load_limit, fw_size;
bool kernel_le = false;
char *filename;
+ CPUClass *cc;
+ ObjectClass *oc;
+ const char *typename;
+ gchar **model_pieces;
msi_nonbroken = true;
@@ -1785,6 +1789,24 @@ static void ppc_spapr_init(MachineState *machine)
if (machine->cpu_model == NULL) {
machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
}
+
+ model_pieces = g_strsplit(machine->cpu_model, ",", 2);
+ if (!model_pieces[0]) {
+ error_report("Invalid/empty CPU model name");
+ exit(1);
+ }
+
+ oc = cpu_class_by_name(TYPE_POWERPC_CPU, model_pieces[0]);
+ if (oc == NULL) {
+ error_report("Unable to find CPU definition: %s", model_pieces[0]);
+ exit(1);
+ }
+
+ typename = object_class_get_name(oc);
+ cc = CPU_CLASS(oc);
+ cc->parse_features(typename, model_pieces[1], &error_fatal);
+ g_strfreev(model_pieces);
+
for (i = 0; i < smp_cpus; i++) {
cpu = cpu_ppc_init(machine->cpu_model);
if (cpu == NULL) {
===========
So essentially this uses cc->parse_features() to parse the cpu feature string
even before any CPU is initialized and cpu_common_parse_features() will
ensure that compat= and any other properties that are part of -cpu will
be set globally only once. I am following what Igor did for x86 here.
Igor's tree doesn't yet contain PowerPC hotplug patches, hence I couldn't
verify this with spapr cpu cores, but I believe that this is all that is
required (on top of Igor's cpu features work) to ensure that compat= is
applied automatically to all the created CPU threads.
However as I note in the patch description of the original patch in this
mail, -global is broken for certain cpu type names.
-cpu host -global host-powerpc64-cpu.compat=power7
-cpu POWER8 -global POWER8-powerpc64-cpu.compat=power7
work, but
-cpu POWER8E -global POWER8E_v2.1-powerpc64-cpu.compat=power7
doesn't since cpu typename has . (dot) and this needs to be fixed.
Regards,
Bharata.
