Re: [Qemu-ppc] [Qemu-devel] [PATCH v7 12/13] qmp: Add query-ppc-cpu-cores command

[Igor Mammedov]

On Thu, 28 Jan 2016 11:19:54 +0530
Bharata B Rao <address@hidden> wrote:

> Show the details of PPC CPU cores via a new QMP command.
> 
> TODO: update qmp-commands.hx with example
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/ppc/cpu-core.c               | 77 
> +++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json                | 31 +++++++++++++++++
>  qmp-commands.hx                 | 51 +++++++++++++++++++++++++++
>  stubs/Makefile.objs             |  1 +
>  stubs/qmp_query_ppc_cpu_cores.c | 10 ++++++
>  5 files changed, 170 insertions(+)
>  create mode 100644 stubs/qmp_query_ppc_cpu_cores.c
> 
> diff --git a/hw/ppc/cpu-core.c b/hw/ppc/cpu-core.c
> index aa96e79..652a5aa 100644
> --- a/hw/ppc/cpu-core.c
> +++ b/hw/ppc/cpu-core.c
> @@ -9,7 +9,84 @@
>  #include "hw/ppc/cpu-core.h"
>  #include "hw/boards.h"
>  #include <sysemu/cpus.h>
> +#include <sysemu/kvm.h>
>  #include "qemu/error-report.h"
> +#include "qmp-commands.h"
> +
> +/*
> + * QMP: info ppc-cpu-cores
> + */
> +static int qmp_ppc_cpu_list(Object *obj, void *opaque)
> +{
> +    CpuInfoList ***prev = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_POWERPC_CPU)) {
> +        CpuInfoList *elem = g_new0(CpuInfoList, 1);
> +        CpuInfo *s = g_new0(CpuInfo, 1);
> +        CPUState *cs = CPU(obj);
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +        CPUPPCState *env = &cpu->env;
> +
> +        cpu_synchronize_state(cs);
> +        s->arch = CPU_INFO_ARCH_PPC;
> +        s->current = (cs == first_cpu);
> +        s->CPU = cs->cpu_index;
> +        s->qom_path = object_get_canonical_path(obj);
> +        s->halted = cs->halted;
> +        s->thread_id = cs->thread_id;
> +        s->u.ppc = g_new0(CpuInfoPPC, 1);
> +        s->u.ppc->nip = env->nip;
> +
> +        elem->value = s;
> +        elem->next = NULL;
> +        **prev = elem;
> +        *prev = &elem->next;
> +    }
> +    object_child_foreach(obj, qmp_ppc_cpu_list, opaque);
> +    return 0;
> +}
> +
> +static int qmp_ppc_cpu_core_list(Object *obj, void *opaque)
> +{
> +    PPCCPUCoreList ***prev = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_POWERPC_CPU_CORE)) {
> +        DeviceClass *dc = DEVICE_GET_CLASS(obj);
> +        DeviceState *dev = DEVICE(obj);
> +
> +        if (dev->realized) {
> +            PPCCPUCoreList *elem = g_new0(PPCCPUCoreList, 1);
> +            PPCCPUCore *s = g_new0(PPCCPUCore, 1);
> +            CpuInfoList *cpu_head = NULL;
> +            CpuInfoList **cpu_prev = &cpu_head;
> +
> +            if (dev->id) {
> +                s->has_id = true;
> +                s->id = g_strdup(dev->id);
> +            }
> +            s->hotplugged = dev->hotplugged;
> +            s->hotpluggable = dc->hotpluggable;
> +            qmp_ppc_cpu_list(obj, &cpu_prev);
> +            s->threads = cpu_head;
> +            elem->value = s;
> +            elem->next = NULL;
> +            **prev = elem;
> +            *prev = &elem->next;
> +        }
> +    }
> +
> +    object_child_foreach(obj, qmp_ppc_cpu_core_list, opaque);
> +    return 0;
> +}
> +
> +PPCCPUCoreList *qmp_query_ppc_cpu_cores(Error **errp)
> +{
> +    PPCCPUCoreList *head = NULL;
> +    PPCCPUCoreList **prev = &head;
> +
> +    qmp_ppc_cpu_core_list(qdev_get_machine(), &prev);
> +    return head;
> +}
>  
>  static int ppc_cpu_core_realize_child(Object *child, void *opaque)
>  {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8d04897..0902697 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4083,3 +4083,34 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @PPCCPUCore:
> +#
> +# Information about PPC CPU core devices
> +#
> +# @hotplugged: true if device was hotplugged
> +#
> +# @hotpluggable: true if device if could be added/removed while machine is 
> running
> +#
> +# Since: 2.6
> +##
> +
> +{ 'struct': 'PPCCPUCore',
> +  'data': { '*id': 'str',
> +            'hotplugged': 'bool',
> +            'hotpluggable': 'bool',
> +            'threads' : ['CpuInfo']
> +          }
> +}
Could it be made more arch independent?
Perhaps it might make sense to replace 'threads'
with qom-path so tools could inspect it in more detail
if needed?

Also looking from cpu hotplug pov it would be nice
to have at top level
  - device type that tools could use with device_add
  - display supported least granularity from topology pov
    like node,socket[,core,[thread]] 'address' parameters
  - display in CPU list also possible CPUs where only
    'type' and 'address' parameters are present.

so above could look like:
{ 'struct': 'CPU',
  'data': {
            'type': 'str'
            'node': 'int',
            'socket': 'int',
            '*core' : 'int',
            '*thread' : 'int',
            '*id': 'str',
            '*hotplugged': 'bool',
            '*hotpluggable': 'bool',
            '*qom-path' : 'str'
          }
}

in addition qom-path could replaced with generic {CPUCore{CPUThread,...},...},
where CPUThread is CPUInfo, I'm not sure if CPUCore could be made generic.

> +
> +##
> +# @query-ppc-cpu-core:
> +#
> +# Returns information for all PPC CPU core devices
> +#
> +# Returns: a list of @PPCCPUCore.
> +#
> +# Since: 2.6
> +##
> +{ 'command': 'query-ppc-cpu-cores', 'returns': ['PPCCPUCore'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index db072a6..77cda3c 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4795,3 +4795,54 @@ Example:
>                   {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
>                    "pop-vlan": 1, "id": 251658240}
>     ]}
> +
> +EQMP
> +
> +#if defined TARGET_PPC64
> +    {
> +        .name       = "query-ppc-cpu-cores",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_query_ppc_cpu_cores,
> +    },
> +#endif
> +
> +SQMP
> address@hidden
> +--------------------
> +
> +Show PowerPC CPU core devices information.
> +
> +Example:
> +-> { "execute": "query-ppc-cpu-cores" }
> +<- {"return": [{"threads": [
> +                 {"arch": "ppc",
> +                  "current": false,
> +                  "CPU": 16,
> +                  "nip": -4611686018426944644,
> +                  "qom_path": "/machine/peripheral/core2/thread[0]",
> +                  "halted": false,
> +                  "thread_id": 32636},
> +                 {"arch": "ppc",
> +                  "current": false",
> +                  "CPU": 17,
> +                  "nip": -4611686018426944644,
> +                  "qom_path": "/machine/peripheral/core2/thread[1]",
> +                  "halted": false, "thread_id": 32637},
> +                 {"arch": "ppc",
> +                  "current": false,
> +                  "CPU": 18,
> +                  "nip": -4611686018426944644,
> +                  "qom_path": "/machine/peripheral/core2/thread[2]",
> +                  "halted": false,
> +                  "thread_id": 32638},
> +                 {"arch": "ppc",
> +                  "current": false,
> +                  "CPU": 19,
> +                  "nip": -4611686018426944644,
> +                  "qom_path": "/machine/peripheral/core2/thread[3]",
> +                  "halted": false,
> +                  "thread_id": 32639}],
> +                "hotplugged": false,
> +                "hotpluggable": true,
> +                "id": "core2"}
> +              ]}
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index d7898a0..1d65999 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -38,3 +38,4 @@ stub-obj-y += qmp_pc_dimm_device_list.o
>  stub-obj-y += target-monitor-defs.o
>  stub-obj-y += target-get-monitor-def.o
>  stub-obj-y += vhost.o
> +stub-obj-y += qmp_query_ppc_cpu_cores.o
> diff --git a/stubs/qmp_query_ppc_cpu_cores.c b/stubs/qmp_query_ppc_cpu_cores.c
> new file mode 100644
> index 0000000..6a875f0
> --- /dev/null
> +++ b/stubs/qmp_query_ppc_cpu_cores.c
> @@ -0,0 +1,10 @@
> +#include "qom/object.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qemu/typedefs.h"
> +#include "qmp-commands.h"
> +
> +PPCCPUCoreList *qmp_query_ppc_cpu_cores(Error **errp)
> +{
> +    error_setg(errp, QERR_UNSUPPORTED);
> +    return 0;
> +}