On Thu, Jun 01, 2017 at 03:44:40PM +1000, Suraj Jitindar Singh wrote:
> On Fri, 2017-05-26 at 15:23 +1000, David Gibson wrote:
> > Server class POWER CPUs have a "compat" property, which is used to
> > set the
> > backwards compatibility mode for the processor. However, this only
> > makes
> > sense for machine types which don't give the guest access to
> > hypervisor
> > privilege - otherwise the compatibility level is under the guest's
> > control.
> >
> > To reflect this, this removes the CPU 'compat' property and instead
> > creates a 'max-cpu-compat' property on the pseries machine. Strictly
> > speaking this breaks compatibility, but AFAIK the 'compat' option was
> > never (directly) used with -device or device_add.
> >
> > The option was used with -cpu. So, to maintain compatibility, this
> > patch adds a hack to the cpu option parsing to strip out any compat
> > options supplied with -cpu and set them on the machine property
> > instead of the now deprecated cpu property.
>
> Generally looks good, a couple of comments below.
>
> Suraj
>
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > hw/ppc/spapr.c | 6 ++-
> > hw/ppc/spapr_cpu_core.c | 56 +++++++++++++++++++++++-
> > hw/ppc/spapr_hcall.c | 8 ++--
> > include/hw/ppc/spapr.h | 12 ++++--
> > target/ppc/compat.c | 102
> > ++++++++++++++++++++++++++++++++++++++++++++
> > target/ppc/cpu.h | 5 ++-
> > target/ppc/translate_init.c | 86 +++++++++++-----------------------
> > ---
> > 7 files changed, 202 insertions(+), 73 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index ab3aab1..3c4e88f 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2134,7 +2134,7 @@ static void ppc_spapr_init(MachineState
> > *machine)
> > machine->cpu_model = kvm_enabled() ? "host" : smc-
> > >tcg_default_cpu;
> > }
> >
> > - ppc_cpu_parse_features(machine->cpu_model);
> > + spapr_cpu_parse_features(spapr);
> >
> > spapr_init_cpus(spapr);
> >
> > @@ -2497,6 +2497,10 @@ static void spapr_machine_initfn(Object *obj)
> > " place of standard EPOW events
> > when possible"
> > " (required for memory hot-
> > unplug support)",
> > NULL);
> > +
> > + ppc_compat_add_property(obj, "max-cpu-compat", &spapr-
> > >max_compat_pvr,
> > + "Maximum permitted CPU compatibility
> > mode",
> > + &error_fatal);
> > }
> >
> > static void spapr_machine_finalizefn(Object *obj)
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index ff7058e..ab4102b 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -20,6 +20,58 @@
> > #include "sysemu/numa.h"
> > #include "qemu/error-report.h"
> >
> > +void spapr_cpu_parse_features(sPAPRMachineState *spapr)
> > +{
> > + /*
> > + * Backwards compatibility hack:
> > + *
> > + * CPUs had a "compat=" property which didn't make sense for
> > + * anything except pseries. It was replaced by "max-cpu-
> > compat"
> > + * machine option. This supports old command lines like
> > + * -cpu POWER8,compat=power7
> > + * By stripping the compat option and applying it to the
> > machine
> > + * before passing it on to the cpu level parser.
> > + */
> > + gchar **inpieces;
> > + int i, j;
> > + gchar *compat_str = NULL;
> > +
> > + inpieces = g_strsplit(MACHINE(spapr)->cpu_model, ",", 0);
> > +
> > + /* inpieces[0] is the actual model string */
> > + i = 1;
> > + j = 1;
> > + while (inpieces[i]) {
> > + if (g_str_has_prefix(inpieces[i], "compat=")) {
> > + /* in case of multiple compat= optipons */
>
> s/optipons/options?
Oops, fixed.
> > + g_free(compat_str);
> > + compat_str = inpieces[i];
> > + } else {
> > + j++;
> > + }
> > +
> > + /* Excise compat options from list */
> > + inpieces[j] = inpieces[i];
>
> it's worth noting that where previously when specifying an invalid
> option you got:
>
> qemu-system-ppc64: Expected key=value format, found *blah*
>
> You now get a segfault here.
Sod. The joy of doing string manipulation in C. Ok, I think I've
found and fixed that too.
--
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
signature.asc
Description: PGP signature
