diff mbox

[v7,12/13] qmp: Add query-ppc-cpu-cores command

Message ID 1453960195-15181-13-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bharata B Rao Jan. 28, 2016, 5:49 a.m. UTC
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 <bharata@linux.vnet.ibm.com>
---
 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

Comments

Eric Blake Jan. 28, 2016, 8:52 p.m. UTC | #1
On 01/27/2016 10:49 PM, Bharata B Rao wrote:
> Show the details of PPC CPU cores via a new QMP command.
> 
> TODO: update qmp-commands.hx with example

Is this a stale comment? [1]

> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---

>  #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)

Comment is a bit off - 'info ...' is an HMP command; this callback is
helping implement the QMP function query-ppc-cpu-cores.

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

Missing docs on 'id' and 'threads'.

> +##
> +
> +{ 'struct': 'PPCCPUCore',
> +  'data': { '*id': 'str',
> +            'hotplugged': 'bool',
> +            'hotpluggable': 'bool',
> +            'threads' : ['CpuInfo']
> +          }
> +}
> +
> +##
> +# @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'] }

Interface seems okay.

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

Hmm. Conditional compilation. Does the command show up in
'query-commands' and introspection output, even when the target is not
ppc64?  We may need to fix qapi introspection to support conditionals
better; maybe some of Marc-Andre's patches towards eliminating
qmp-commands.hx will come into play here.

> +
> +SQMP
> +@query-ppc-cpu-cores
> +--------------------
> +
> +Show PowerPC CPU core devices information.
> +
> +Example:
> +-> { "execute": "query-ppc-cpu-cores" }
> +<- {"return": [{"threads": [
> +                 {"arch": "ppc",

[1] looks like you provided an example after all.  Is it worth
documenting that this command is only conditionally available?


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

Hmm - will the stub even be used, since you used an #ifdef in the .hx file?
Bharata B Rao Jan. 29, 2016, 6:34 a.m. UTC | #2
On Thu, Jan 28, 2016 at 01:52:26PM -0700, Eric Blake wrote:
> On 01/27/2016 10:49 PM, Bharata B Rao wrote:
> > Show the details of PPC CPU cores via a new QMP command.
> > 
> > TODO: update qmp-commands.hx with example
> 
> Is this a stale comment? [1]

Yes, I missed removing it after I put the example in qmp-commands.hx.

> 
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> 
> >  #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)
> 
> Comment is a bit off - 'info ...' is an HMP command; this callback is
> helping implement the QMP function query-ppc-cpu-cores.

Ok, will make it "QMP: query-ppc-cpu-cores"

> 
> > +++ 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
> 
> Missing docs on 'id' and 'threads'.

Will add.

> 
> > +##
> > +
> > +{ 'struct': 'PPCCPUCore',
> > +  'data': { '*id': 'str',
> > +            'hotplugged': 'bool',
> > +            'hotpluggable': 'bool',
> > +            'threads' : ['CpuInfo']
> > +          }
> > +}
> > +
> > +##
> > +# @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'] }
> 
> Interface seems okay.
> 
> > +++ 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
> 
> Hmm. Conditional compilation. Does the command show up in
> 'query-commands' and introspection output, even when the target is not
> ppc64?  We may need to fix qapi introspection to support conditionals
> better; maybe some of Marc-Andre's patches towards eliminating
> qmp-commands.hx will come into play here.

For targets other than ppc64, query-commands doesn't list
query-ppc-cpu-cores.

> 
> > +
> > +SQMP
> > +@query-ppc-cpu-cores
> > +--------------------
> > +
> > +Show PowerPC CPU core devices information.
> > +
> > +Example:
> > +-> { "execute": "query-ppc-cpu-cores" }
> > +<- {"return": [{"threads": [
> > +                 {"arch": "ppc",
> 
> [1] looks like you provided an example after all.  Is it worth
> documenting that this command is only conditionally available?

Will add

# Note: This command is available only for PowerPC targets

> 
> 
> > +++ 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;
> > +}
> 
> Hmm - will the stub even be used, since you used an #ifdef in the .hx file?

Though I have put

.mhandler.cmd_new = qmp_marshal_query_ppc_cpu_cores,

under ifdef in .hx, qmp_marshal_query_ppc_cpu_cores() is getting defined in
qmp-marshal.c and hence we need this stub file so that qmp_query_ppc_cpu_cores()
gets resolved from qmp-marshal.c:qmp_marshal_query_ppc_cpu_cores().

Regards,
Bharata.
Igor Mammedov Jan. 29, 2016, 3:45 p.m. UTC | #3
On Thu, 28 Jan 2016 11:19:54 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> 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 <bharata@linux.vnet.ibm.com>
> ---
>  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
> +@query-ppc-cpu-cores
> +--------------------
> +
> +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;
> +}
Bharata B Rao Feb. 1, 2016, 8:43 a.m. UTC | #4
On Fri, Jan 29, 2016 at 04:45:06PM +0100, Igor Mammedov wrote:
> On Thu, 28 Jan 2016 11:19:54 +0530
> Bharata B Rao <bharata@linux.vnet.ibm.com> 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 <bharata@linux.vnet.ibm.com>
> > ---
> >  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?

Except that it is called PPCCPUCore, the fields are actually arch
neutral with 'threads' element defined as CpuInfo that gets interpreted
as arch-specific CpuInfo based on the target architecture.

In any case, this patchset adds PowerPC specific CPU core device that
sPAPR target implements. This is kept arch-specific in order to make it
more acceptable in short term in case arch-neutral, generic CPU hotplug
solutions take long time for reaching consensus.

> Perhaps it might make sense to replace 'threads'
> with qom-path so tools could inspect it in more detail
> if needed?

Hmm 'threads' is of CpuInfo type which already has qom-path inside it.
Did I get your question right ?

> 
> 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'
>           }
> }

This is PowerPC specific where only core granularity hotplug makes
sense, but if it helps libvirt or other tools, I could add those fields.

Regards,
Bharata.
Igor Mammedov Feb. 1, 2016, 9:56 a.m. UTC | #5
On Mon, 1 Feb 2016 14:13:58 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> On Fri, Jan 29, 2016 at 04:45:06PM +0100, Igor Mammedov wrote:
> > On Thu, 28 Jan 2016 11:19:54 +0530
> > Bharata B Rao <bharata@linux.vnet.ibm.com> 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 <bharata@linux.vnet.ibm.com>
> > > ---
> > >  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?  
> 
> Except that it is called PPCCPUCore, the fields are actually arch
> neutral with 'threads' element defined as CpuInfo that gets interpreted
> as arch-specific CpuInfo based on the target architecture.
> 
> In any case, this patchset adds PowerPC specific CPU core device that
> sPAPR target implements. This is kept arch-specific in order to make it
> more acceptable in short term in case arch-neutral, generic CPU hotplug
> solutions take long time for reaching consensus.
Yep it's not easy to agree on but this PPC specific you'd have to
keep around and support pretty much forever once it's released,
I hope it would be easier to agree on CPU hotplug specific qmp/hmp
interface.

Wouldn't 'struct' I've suggested below work for you?
It's more or less generic and would work for x86 as well
so less target specific stuff to do in libvirt would also
be a plus.

> 
> > Perhaps it might make sense to replace 'threads'
> > with qom-path so tools could inspect it in more detail
> > if needed?  
> 
> Hmm 'threads' is of CpuInfo type which already has qom-path inside it.
> Did I get your question right ?
> 
> > 
> > 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'
> >           }
> > }  
> 
> This is PowerPC specific where only core granularity hotplug makes
> sense, but if it helps libvirt or other tools, I could add those fields.
then PPC won't display/allow 'thread' parameter, while x86 will
since it allows thread granularity. For targets that allow only
sockets neither core/thread would be shown.

> 
> Regards,
> Bharata.
>
diff mbox

Patch

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']
+          }
+}
+
+##
+# @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
+@query-ppc-cpu-cores
+--------------------
+
+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;
+}