Re: [Qemu-devel] [RFC PATCH v0 5/6] qmp, spapr: Show hot-plugged/pluggable CPU slots in the Machine

[Igor Mammedov]

On Thu, 25 Feb 2016 21:52:41 +0530
Bharata B Rao <address@hidden> wrote:

> Implement query cpu-slots that provides information about hot-plugged
> as well as hot-pluggable CPU slots that the machine supports.
> 
> TODO: As Eric suggested use enum for type instead of str.
> TODO: @hotplug-granularity probably isn't required.
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/core/machine.c   |  19 +++++++++
>  hw/ppc/spapr.c      | 112 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/boards.h |   4 ++
>  qapi-schema.json    |  85 +++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx     |  47 ++++++++++++++++++++++
>  5 files changed, 267 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 6d1a0d8..3055ef8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -17,6 +17,25 @@
>  #include "hw/sysbus.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
> +#include "qmp-commands.h"
> +
> +/*
> + * QMP: query-cpu-slots
> + *
> + * TODO: Ascertain if this is the right place to for this arch-neutral 
> routine.
> + */
> +CPUSlotInfoList *qmp_query_cpu_slots(Error **errp)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> +    if (!mc->cpu_slots) {
> +        error_setg(errp, QERR_UNSUPPORTED);
> +        return NULL;
> +    }
> +
> +    return mc->cpu_slots(ms);
> +}
>  
>  static char *machine_get_accel(Object *obj, Error **errp)
>  {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 780cd00..b76ed85 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2453,6 +2453,117 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned 
> cpu_index)
>      return cpu_index / smp_threads / smp_cores;
>  }
>  
> +static int spapr_cpuinfo_list(Object *obj, void *opaque)
> +{
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> +    CPUInfoList ***prev = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_CPU)) {
> +        CPUInfoList *elem = g_new0(CPUInfoList, 1);
> +        CPUInfo *s = g_new0(CPUInfo, 1);
> +        CPUState *cpu = CPU(obj);
> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
> +
> +        s->arch_id = ppc_get_vcpu_dt_id(pcpu);
> +        s->type = g_strdup(object_get_typename(obj));
> +        s->thread = cpu->cpu_index;
> +        s->has_thread = true;
> +        s->core = cpu->cpu_index / smp_threads;
> +        s->has_core = true;
> +        if (mc->cpu_index_to_socket_id) {
> +            s->socket = mc->cpu_index_to_socket_id(cpu->cpu_index);
> +        } else {
> +            s->socket = cpu->cpu_index / smp_threads / smp_cores;
> +        }
> +        s->has_socket = true;
> +        s->node = cpu->numa_node;
> +        s->has_node = true;
> +        s->qom_path = object_get_canonical_path(obj);
> +        s->has_qom_path = true;
> +
> +        elem->value = s;
> +        elem->next = NULL;
> +        **prev = elem;
> +        *prev = &elem->next;
> +    }
> +    object_child_foreach(obj, spapr_cpuinfo_list, opaque);
> +    return 0;
> +}
> +
> +static CPUSlotInfoList *spapr_cpu_slots(MachineState *machine)
> +{
> +    CPUSlotInfoList *head = NULL;
> +    CPUSlotInfoList **prev = &head;
> +    Object *root_container;
> +    ObjectProperty *prop;
> +    ObjectPropertyIterator iter;
> +
> +    /*
> +     * TODO: There surely must be a better/easier way to walk all
> +     * the link properties of an object ?
> +     */
> +    root_container = container_get(object_get_root(), "/machine");
> +    object_property_iter_init(&iter, root_container);
> +
> +    while ((prop = object_property_iter_next(&iter))) {
> +        Object *obj;
> +        DeviceState *dev;
> +        CPUSlotInfoList *elem;
> +        CPUSlotInfo *s;
> +        CPUInfoList *cpu_head = NULL;
> +        CPUInfoList **cpu_prev = &cpu_head;
> +        sPAPRCPUCore *core;
> +
> +        if (!strstart(prop->type, "link<", NULL)) {
> +            continue;
> +        }
> +
> +        if (!strstart(prop->name, SPAPR_MACHINE_CPU_CORE_PROP, NULL)) {
> +            continue;
> +        }
> +
> +        elem = g_new0(CPUSlotInfoList, 1);
> +        s = g_new0(CPUSlotInfo, 1);
> +
> +        obj = object_property_get_link(root_container, prop->name, NULL);
> +        if (obj) {
> +            /* Slot populated */
> +            dev = DEVICE(obj);
> +            core = SPAPR_CPU_CORE(obj);
> +
> +            if (dev->id) {
> +                s->has_id = true;
> +                s->id = g_strdup(dev->id);
> +            }
> +            s->realized = object_property_get_bool(obj, "realized", NULL);
> +            s->nr_cpus = core->nr_threads;
> +            s->has_nr_cpus = true;
> +            s->qom_path = object_get_canonical_path(obj);
> +            s->has_qom_path = true;
> +            if (s->realized) {
> +                spapr_cpuinfo_list(obj, &cpu_prev);
> +            }
> +            s->has_cpus = true;
> +        } else {
> +            /* Slot empty */
> +            s->has_id = false;
> +            s->has_nr_cpus = false;
> +            s->has_qom_path = false;
> +            s->has_cpus = false;
> +            s->realized = false;
> +        }
> +        s->type = g_strdup(TYPE_SPAPR_CPU_CORE);
> +        s->hotplug_granularity = g_strdup(SPAPR_MACHINE_CPU_CORE_PROP);
> +        s->slot_id = g_strdup(prop->name);
> +        s->cpus = cpu_head;
> +        elem->value = s;
> +        elem->next = NULL;
> +        *prev = elem;
> +        prev = &elem->next;
> +    }
> +    return head;
> +}
> +
>  static void spapr_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -2482,6 +2593,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>      hc->plug = spapr_machine_device_plug;
>      hc->unplug = spapr_machine_device_unplug;
>      mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
> +    mc->cpu_slots = spapr_cpu_slots;
>  
>      smc->dr_lmb_enabled = true;
>      smc->dr_cpu_enabled = true;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 0f30959..d888a02 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -57,6 +57,9 @@ bool machine_mem_merge(MachineState *machine);
>   *    Set only by old machines because they need to keep
>   *    compatibility on code that exposed QEMU_VERSION to guests in
>   *    the past (and now use qemu_hw_version()).
> + * @cpu_slots:
> + *    Provides information about populated and yet-to-be populated
> + *    CPU slots in the machine. Used by QMP query-cpu-slots.
>   */
>  struct MachineClass {
>      /*< private >*/
> @@ -99,6 +102,7 @@ struct MachineClass {
>      HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>                                             DeviceState *dev);
>      unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> +    CPUSlotInfoList *(*cpu_slots)(MachineState *machine);
>  };
>  
>  /**
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8d04897..e9a52a2 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4083,3 +4083,88 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @CPUInfo:
> +#
> +# Information about CPUs
> +#
> +# @arch-id: Arch-specific ID for the CPU.
> +#
> +# @type: QOM type of the CPU.
> +#
> +# @thread: Thread ID of the CPU.
> +#
> +# @core: Core ID of the CPU.
> +#
> +# @socket: Socket ID of the CPU.
> +#
> +# @node: NUMA node to which the CPU belongs.
> +#
> +# @qom-path: QOM path of the CPU object
> +#
> +# Since: 2.6
> +##
> +
> +{ 'struct': 'CPUInfo',
> +  'data': { 'arch-id': 'int',
> +            'type': 'str',
> +            '*thread': 'int',
> +            '*core': 'int',
> +            '*socket' : 'int',
> +            '*node' : 'int',
> +            '*qom-path': 'str'
> +          }
> +}
> +
> +##
> +# @CPUSlotInfo:
> +#
> +# Information about CPU Slots
> +#
> +# @id: Device ID of the CPU composite object that occupies the slot.
> +#
> +# @type: QOM type of the CPU composite object that occupies the slot.
> +#
> +# @hotplug-granularity: Granularity of CPU composite hotplug for this slot,
> +# can be thread, core or socket.
> +#
> +# @slot-id: Slot's identifier.
> +#
> +# @qom-path: QOM path of the CPU composite object that occupies the slot.
> +#
> +# @realized: A boolean that indicates whether the slot is filled or empty.
> +#
> +# @nr-cpus: Number of CPUs that are part of CPU composite object that 
> occupies
> +# this slot.
> +#
> +# @cpus: An array of @CPUInfo elements where each element describes one
> +# CPU that is part of this slot's CPU composite object.
> +#
> +# @type: QOM type
> +#
> +# Since: 2.6
> +##
> +
> +{ 'struct': 'CPUSlotInfo',
> +  'data': { '*id': 'str',
> +            'type': 'str',
> +            'hotplug-granularity' : 'str',
Does it convey any useful info, if yes how it will be used by mgmt?

> +            'slot-id' : 'str',
> +            '*qom-path': 'str',
> +            'realized': 'bool',
field's redundant, presence of qom-path answers this question

> +            '*nr-cpus': 'int',
> +            '*cpus' : ['CPUInfo']
I'd suggest to drop above 2 fields as it's impl dependent,
qom-path already point's to socket/core/thread object and
its internal composition can be explored by other means if needed.

Moreover 'CPUInfo' is confusing and sort of conflicts with existing
'CpuInfo'.
I'd drop CPUInfo altogether and introduce only 'CPUSlotInfo' here,
existing thread enumeration's already implemented query-cpus.
 
> +          }
> +}
What I miss here is that CPUSlotInfo doesn't provide any
information to about where CPU might be hotplugged to.

Maybe use following tuple instead of slot-id?

{ 'struct': 'CPUSlotProperties',
  'data': { '*node': 'int',
            '*socket': 'int',
            '*core': 'int',
            '*thread': 'int'
  }
}

it's generic for the most targets, which should work for spapr, s390, x86, ARM
and could be extended for other cases adding other board specific
properties if it's needed.
The all mgmt would have to do for hotplug is to execute:
 device_add ${type},${CPUSlotProperties}


> +
> +##
> +# @query-cpu-slots:
> +#
> +# Returns information for all CPU slots
> +#
> +# Returns: a list of @CPUSlotInfo
> +#
> +# Since: 2.6
> +##
> +{ 'command': 'query-cpu-slots', 'returns': ['CPUSlotInfo'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 085dc7d..185aa13 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4825,3 +4825,50 @@ Example:
>                   {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
>                    "pop-vlan": 1, "id": 251658240}
>     ]}
> +
> +EQMP
> +
> +    {
> +        .name       = "query-cpu-slots",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_query_cpu_slots,
> +    },
> +
> +SQMP
> address@hidden
> +--------------------
> +
> +Show CPU slots information
> +
> +Example:
> +-> { "execute": "query-cpu-slots" }
> +<- {"return": [{
> +                "slot-id": "core[2]",
> +                "hotplug-granularity": "core",
> +                "realized": false,
> +                "type": "spapr-cpu-core"
> +               },
> +               {
> +                "slot-id": "core[1]",
> +                "qom-path": "/machine/unattached/device[2]",
> +                "hotplug-granularity": "core",
> +                "realized": true,
> +                "type": "spapr-cpu-core"
> +                 "nr-cpus": 2,
> +                 "cpus": [
> +                   {"thread": 8,
> +                    "socket": 0,
> +                    "core": 1,
> +                    "arch-id": 8,
> +                    "qom-path": "/machine/unattached/device[2]/thread[0]",
> +                    "node": 0,
> +                    "type": "host-powerpc64-cpu"},
> +                   {"thread": 9,
> +                    "socket": 0,
> +                    "core": 1,
> +                    "arch-id": 9,
> +                    "qom-path": "/machine/unattached/device[2]/thread[2]",
> +                    "node": 0,
> +                    "type": "host-powerpc64-cpu"}]
> +               }
> +   ]}