Re: [Qemu-devel] [PATCH RFC 05/16] hw/core/machine: add smp properites

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6088 bytes --]

On Fri, Jun 10, 2016 at 07:40:16PM +0200, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@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?

> +
> +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?

>  };
>  
>  #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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]