[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU t
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores |
Date: |
Wed, 22 Jun 2016 13:03:44 +1000 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Wed, Jun 22, 2016 at 08:06:50AM +0530, Bharata B Rao wrote:
> On Tue, Jun 21, 2016 at 03:10:00PM +1000, David Gibson wrote:
> > On Sat, Jun 18, 2016 at 02:04:06PM +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.
> > > With the introduction of sPAPR CPU core devices, we need to support
> > > the same for core devices too.
> > >
> > > Support the specification of CPU compat type on device_add command for
> > > sPAPRCPUCore devices like:
> > > (qemu) device_add POWER8E-spapr-cpu-core,id=core3,compat=power7,core-id=24
> > >
> > > Signed-off-by: Bharata B Rao <address@hidden>
> > > ---
> > > Applies on ppc-for-2.7 branch of David Gibson's tree.
> >
> > The implementation looks ok apart from a few nits noted below.
> >
> > There's a larger problem here, though, in that this doesn't advertise
> > the necessary compat= property via query-hotpluggable-cpus qmp and hmp
> > interfaces. Which means that management has no good way of knowing
> > it's necessary.
> >
> > >
> > > hw/ppc/spapr.c | 8 +++++
> > > hw/ppc/spapr_cpu_core.c | 73
> > > +++++++++++++++++++++++++++++++++++++++++
> > > include/hw/ppc/spapr_cpu_core.h | 2 ++
> > > 3 files changed, 83 insertions(+)
> > >
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 778fa25..2049d7d 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1807,6 +1807,7 @@ static void ppc_spapr_init(MachineState *machine)
> > > if (i < spapr_cores) {
> > > char *type = spapr_get_cpu_core_type(machine->cpu_model);
> > > Object *core;
> > > + char *compat;
> > >
> > > if (!object_class_by_name(type)) {
> > > error_report("Unable to find sPAPR CPU Core
> > > definition");
> > > @@ -1818,6 +1819,13 @@ static void ppc_spapr_init(MachineState *machine)
> > > &error_fatal);
> > > object_property_set_int(core, core_dt_id,
> > > CPU_CORE_PROP_CORE_ID,
> > > &error_fatal);
> > > + compat = spapr_get_cpu_compat_type(machine->cpu_model);
> > > + if (compat) {
> > > + object_property_set_str(core, compat, "compat",
> > > + &error_fatal);
> > > + g_free(compat);
> > > + }
> > > +
> > > object_property_set_bool(core, true, "realized",
> > > &error_fatal);
> > > }
> > > }
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index 3a5da09..9eb63cc 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -96,6 +96,24 @@ char *spapr_get_cpu_core_type(const char *model)
> > > return core_type;
> > > }
> > >
> > > +/*
> > > + * Returns the CPU compat type specified in -cpu @model.
> > > + */
> > > +char *spapr_get_cpu_compat_type(const char *model)
> > > +{
> > > + char *compat_type = NULL;
> > > + gchar **model_pieces = g_strsplit(model, ",", 2);
> > > +
> > > + if (model_pieces[1]) {
> > > + gchar **compat_pieces = g_strsplit(model_pieces[1], "=", 2);
> > > +
> > > + compat_type = g_strdup_printf("%s", compat_pieces[1]);
> > > + }
> > > +
> > > + g_strfreev(model_pieces);
> > > + return compat_type;
> > > +}
> > > +
> > > static void spapr_core_release(DeviceState *dev, void *opaque)
> > > {
> > > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> > > @@ -223,12 +241,31 @@ void spapr_core_pre_plug(HotplugHandler
> > > *hotplug_dev, DeviceState *dev,
> > > CPUCore *cc = CPU_CORE(dev);
> > > char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model);
> > > const char *type = object_get_typename(OBJECT(dev));
> > > + char *base_compat_type = NULL;
> > > + char *compat = NULL;
> > > + bool compat_set;
> > >
> > > if (strcmp(base_core_type, type)) {
> > > error_setg(&local_err, "CPU core type should be %s",
> > > base_core_type);
> > > goto out;
> > > }
> > >
> > > + base_compat_type = spapr_get_cpu_compat_type(machine->cpu_model);
> >
> > This can go in the initializer to match the base_core_type.
>
> Had it that way, but since there was an error exit possibility when
> base_core_type is not matching, I thought better to initialize
> base_compat_type
> after that check.
Ah, good point. I forgot that get_cpu_compat_type() had an implicit
allocation that needs cleanup.
> > > @@ -298,9 +339,19 @@ static void spapr_cpu_core_realize(DeviceState *dev,
> > > Error **errp)
> > > snprintf(id, sizeof(id), "thread[%d]", i);
> > > object_property_add_child(OBJECT(sc), id, obj, &local_err);
> > > if (local_err) {
> > > + g_free(compat);
> > > goto err;
> > > }
> > > + if (compat_set) {
> > > + CPUClass *cc = CPU_GET_CLASS(CPU(obj));
> > > + char *featurestr = g_strdup_printf("compat=%s", compat);
> > > +
> > > + cc->parse_features(CPU(obj), featurestr, &local_err);
> >
> > Hmm.. would it make more sense to just do an object_property_set()
> > rather than calling into parse_features?
>
> It would work, but I guess better to use ->parse_features() to ensure
> future additional properties would work seamlessly.
Hmm.. except that the string you're forming to pass to parse_features
is always just "compat=%s". So you'd have to change that bit anyway,
so I don't see that using parse_features really buys you anything.
--
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
- [Qemu-ppc] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Bharata B Rao, 2016/06/18
- Re: [Qemu-ppc] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, David Gibson, 2016/06/21
- Re: [Qemu-ppc] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Thomas Huth, 2016/06/21
- Re: [Qemu-ppc] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Igor Mammedov, 2016/06/21
- Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Igor Mammedov, 2016/06/21
- Re: [Qemu-ppc] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Bharata B Rao, 2016/06/21
- Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Igor Mammedov, 2016/06/22
- Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Eduardo Habkost, 2016/06/23
- Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Bharata B Rao, 2016/06/23
- Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, David Gibson, 2016/06/23
- Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores, Eduardo Habkost, 2016/06/24