On Tue, Jun 14, 2016 at 12:00:26PM +1000, David Gibson wrote:
> On Fri, Jun 10, 2016 at 07:40:16PM +0200, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones <drjo...@redhat.com>
> > ---
> > hw/core/machine.c | 81
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/boards.h | 6 ++++
> > 2 files changed, 87 insertions(+)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 3dce9020e510a..2625044002e57 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -172,6 +172,53 @@ static void machine_set_dumpdtb(Object *obj, const
> > char *value, Error **errp)
> > ms->dumpdtb = g_strdup(value);
> > }
> >
> > +static void machine_get_smp(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + int64_t value;
> > +
> > + if (strncmp(name, "sockets", 7) == 0) {
> > + value = ms->sockets;
> > + } else if (strncmp(name, "cores", 5) == 0) {
> > + value = ms->cores;
> > + } else if (strncmp(name, "threads", 7) == 0) {
> > + value = ms->threads;
> > + } else if (strncmp(name, "maxcpus", 7) == 0) {
> > + value = ms->maxcpus;
> > + } else if (strncmp(name, "cpus", 4) == 0) {
> > + value = ms->cpus;
> > + }
> > +
> > + visit_type_int(v, name, &value, errp);
> > +}
>
> Any particular for multiplexing all the set / get, rather than having
> separate callbacks for each property?
Not really. This way just makes denser code. But I'll go whichever
direction people prefer. Actually I should probably add an
else { error_report(...) } in either case, which means the multifunction
direction would still contain strncmps.
>
> > +
> > +static void machine_set_smp(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + Error *error = NULL;
> > + int64_t value;
> > +
> > + visit_type_int(v, name, &value, &error);
> > + if (error) {
> > + error_propagate(errp, error);
> > + return;
> > + }
> > +
> > + if (strncmp(name, "sockets", 7) == 0) {
> > + ms->sockets = value;
> > + } else if (strncmp(name, "cores", 5) == 0) {
> > + ms->cores = value;;
> > + } else if (strncmp(name, "threads", 7) == 0) {
> > + ms->threads = value;
> > + } else if (strncmp(name, "maxcpus", 7) == 0) {
> > + ms->maxcpus = value;
> > + } else if (strncmp(name, "cpus", 4) == 0) {
> > + ms->cpus = value;
> > + }
> > +}
> > +
> > static void machine_get_phandle_start(Object *obj, Visitor *v,
> > const char *name, void *opaque,
> > Error **errp)
> > @@ -368,8 +415,18 @@ static void machine_init_notify(Notifier *notifier,
> > void *data)
> > foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
> > }
> >
> > +static void machine_set_smp_parameters(MachineState *ms)
> > +{
> > + if (ms->sockets != -1 || ms->cores != -1 || ms->threads != -1 ||
> > + ms->maxcpus != -1 || ms->cpus != -1) {
> > + error_report("warning: cpu topology: "
> > + "machine properties currently ignored");
> > + }
> > +}
> > +
> > static void machine_pre_init(MachineState *ms)
> > {
> > + machine_set_smp_parameters(ms);
> > }
> >
> > static void machine_class_init(ObjectClass *oc, void *data)
> > @@ -403,6 +460,11 @@ static void machine_initfn(Object *obj)
> > ms->dump_guest_core = true;
> > ms->mem_merge = true;
> > ms->enable_graphics = true;
> > + ms->sockets = -1;
> > + ms->cores = -1;
> > + ms->threads = -1;
> > + ms->maxcpus = -1;
> > + ms->cpus = -1;
> >
> > object_property_add_str(obj, "accel",
> > machine_get_accel, machine_set_accel, NULL);
> > @@ -462,6 +524,25 @@ static void machine_initfn(Object *obj)
> > object_property_set_description(obj, "dt-compatible",
> > "Overrides the \"compatible\" property
> > of the dt root node",
> > NULL);
> > + object_property_add(obj, "sockets", "int", machine_get_smp,
> > + machine_set_smp, NULL, NULL, NULL);
> > + object_property_set_description(obj, "sockets", "Number of sockets",
> > NULL);
> > + object_property_add(obj, "cores", "int", machine_get_smp,
> > + machine_set_smp, NULL, NULL, NULL);
> > + object_property_set_description(obj, "cores",
> > + "Number of cores per socket", NULL);
> > + object_property_add(obj, "threads", "int", machine_get_smp,
> > + machine_set_smp, NULL, NULL, NULL);
> > + object_property_set_description(obj, "threads",
> > + "Number of threads per core", NULL);
> > + object_property_add(obj, "maxcpus", "int", machine_get_smp,
> > + machine_set_smp, NULL, NULL, NULL);
> > + object_property_set_description(obj, "maxcpus", "Maximum number of
> > cpus",
> > + NULL);
> > + object_property_add(obj, "cpus", "int", machine_get_smp,
> > + machine_set_smp, NULL, NULL, NULL);
> > + object_property_set_description(obj, "cpus", "Number of online cpus",
> > + NULL);
> > object_property_add_bool(obj, "dump-guest-core",
> > machine_get_dump_guest_core,
> > machine_set_dump_guest_core,
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 4e8dc68b07a24..53adbfe2a3099 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -166,6 +166,12 @@ struct MachineState {
> > char *initrd_filename;
> > const char *cpu_model;
> > AccelState *accelerator;
> > +
> > + int sockets;
> > + int cores;
> > + int threads;
> > + int maxcpus;
> > + int cpus;
>
> Hrm.. as the tests added in earlier patches highlight, essentially one
> of these properties is redundant. Is there a reason to include them
> all, rather than to pick one to drop?
Well, IMO, only maxcpus could be dropped. I'd prefer not to though
because it's a convenient state to have pre-calculated, and possibly
error prone to leave to all users to re-calculate.
Thanks,
drew
>
> > };
> >
> > #define DEFINE_MACHINE(namestr, machine_initfn) \
>
> --
> 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
