Eric Blake <ebl...@redhat.com> writes:
> The CpuInfo struct is used only by the 'query-cpus' output
> command, so we are free to modify it by adding fields (clients
> are already supposed to ignore unknown output fields), or by
> changing optional members to mandatory, while still keeping
> QMP wire compatibility with older versions of qemu.
>
> When qapi type CpuInfo was originally created for 0.14, we had
> no notion of a flat union, and instead just listed a bunch of
> optional fields with documentation about the mutually-exclusive
> choice of which instruction pointer field(s) would be provided
> for a given architecture. But now that we have flat unions and
> introspection, it is better to segregate off which fields will
> be provided according to the actual architecture. With this in
> place, we no longer need the fields to be optional, because the
> choice of the new 'arch' discriminator serves that role.
>
> This has an additional benefit: the old all-in-one struct was
> the only place in the code base that had a case-sensitive
> naming of members 'pc' vs. 'PC'. Separating these spellings
> into different branches of the flat union will allow us to add
> restrictions against future case-insensitive collisions, since
> that is generally a poor interface practice.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v12: s/2.5/2.6/, typo fix
> v11: also fix qmp-commands.hx, improve commit message. If this
> misses 2.5 (likely), it will need updates to call out 2.6
> v10: new patch
> ---
> cpus.c | 31 ++++++++------
> hmp.c | 30 +++++++++-----
> qapi-schema.json | 120
> ++++++++++++++++++++++++++++++++++++++++++++++---------
> qmp-commands.hx | 4 ++
> 4 files changed, 143 insertions(+), 42 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 877bd70..77ba15b 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1556,22 +1556,29 @@ CpuInfoList *qmp_query_cpus(Error **errp)
> info->value->qom_path = object_get_canonical_path(OBJECT(cpu));
> info->value->thread_id = cpu->thread_id;
> #if defined(TARGET_I386)
> - info->value->has_pc = true;
> - info->value->pc = env->eip + env->segs[R_CS].base;
> + info->value->arch = CPU_INFO_ARCH_X86;
> + info->value->u.x86 = g_new0(CpuInfoX86, 1);
> + info->value->u.x86->pc = env->eip + env->segs[R_CS].base;
> #elif defined(TARGET_PPC)
> - info->value->has_nip = true;
> - info->value->nip = env->nip;
> + info->value->arch = CPU_INFO_ARCH_PPC;
> + info->value->u.ppc = g_new0(CpuInfoPpc, 1);
> + info->value->u.ppc->nip = env->nip;
> #elif defined(TARGET_SPARC)
> - info->value->has_pc = true;
> - info->value->pc = env->pc;
> - info->value->has_npc = true;
> - info->value->npc = env->npc;
> + info->value->arch = CPU_INFO_ARCH_SPARC;
> + info->value->u.sparc = g_new0(CpuInfoSPARC, 1);
CpuInfoSparc.
> + info->value->u.sparc->pc = env->pc;
> + info->value->u.sparc->npc = env->npc;
> #elif defined(TARGET_MIPS)
> - info->value->has_PC = true;
> - info->value->PC = env->active_tc.PC;
> + info->value->arch = CPU_INFO_ARCH_MIPS;
> + info->value->u.mips = g_new0(CpuInfoMips, 1);
> + info->value->u.mips->PC = env->active_tc.PC;
> #elif defined(TARGET_TRICORE)
> - info->value->has_PC = true;
> - info->value->PC = env->PC;
> + info->value->arch = CPU_INFO_ARCH_TRICORE;
> + info->value->u.tricore = g_new0(CpuInfoTricore, 1);
> + info->value->u.tricore->PC = env->PC;
> +#else
> + info->value->arch = CPU_INFO_ARCH_OTHER;
> + info->value->u.other = g_new0(CpuInfoOther, 1);
> #endif
>
> /* XXX: waiting for the qapi to support GSList */
> diff --git a/hmp.c b/hmp.c
> index 1b402c4..c2b2c16 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -311,17 +311,25 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
>
> monitor_printf(mon, "%c CPU #%" PRId64 ":", active, cpu->value->CPU);
>
> - if (cpu->value->has_pc) {
> - monitor_printf(mon, " pc=0x%016" PRIx64, cpu->value->pc);
> - }
> - if (cpu->value->has_nip) {
> - monitor_printf(mon, " nip=0x%016" PRIx64, cpu->value->nip);
> - }
> - if (cpu->value->has_npc) {
> - monitor_printf(mon, " npc=0x%016" PRIx64, cpu->value->npc);
> - }
> - if (cpu->value->has_PC) {
> - monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->PC);
> + switch (cpu->value->arch) {
> + case CPU_INFO_ARCH_X86:
> + monitor_printf(mon, " pc=0x%016" PRIx64, cpu->value->u.x86->pc);
> + break;
> + case CPU_INFO_ARCH_PPC:
> + monitor_printf(mon, " nip=0x%016" PRIx64,
> cpu->value->u.ppc->nip);
> + break;
> + case CPU_INFO_ARCH_SPARC:
> + monitor_printf(mon, " pc=0x%016" PRIx64,
> cpu->value->u.sparc->pc);
> + monitor_printf(mon, " npc=0x%016" PRIx64,
> cpu->value->u.sparc->npc);
> + break;
> + case CPU_INFO_ARCH_MIPS:
> + monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.mips->PC);
> + break;
> + case CPU_INFO_ARCH_TRICORE:
> + monitor_printf(mon, " PC=0x%016" PRIx64,
> cpu->value->u.tricore->PC);
> + break;
> + default:
> + break;
> }
>
> if (cpu->value->halted) {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8b1a423..67e80a5 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -744,43 +744,125 @@
> { 'command': 'query-mice', 'returns': ['MouseInfo'] }
>
> ##
> -# @CpuInfo:
> +# @CpuInfoArch:
> #
> -# Information about a virtual CPU
> +# An enumeration of cpu types that enable additional information during
> +# @query-cpus.
> +#
> +# Since: 2.6
> +##
> +{ 'enum': 'CpuInfoArch',
> + 'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 'other' ] }
> +
> +##
> +# @CpuInfoBase:
> +#
> +# Common information about a virtual CPU
> #
> # @CPU: the index of the virtual CPU
> #
> -# @current: this only exists for backwards compatible and should be ignored
> +# @current: this only exists for backwards compatibility and should be
> ignored
> #
> # @halted: true if the virtual CPU is in the halt state. Halt usually refers
> # to a processor specific low power mode.
> #
> # @qom_path: path to the CPU object in the QOM tree (since 2.4)
> #
> -# @pc: #optional If the target is i386 or x86_64, this is the 64-bit
> instruction
> -# pointer.
> -# If the target is Sparc, this is the PC component of the
> -# instruction pointer.
> -#
> -# @nip: #optional If the target is PPC, the instruction pointer
> -#
> -# @npc: #optional If the target is Sparc, the NPC component of the
> instruction
> -# pointer
> -#
> -# @PC: #optional If the target is MIPS, the instruction pointer
> -#
> # @thread_id: ID of the underlying host thread
> #
> +# @arch: architecture of the cpu, which determines which additional fields
> +# will be listed (since 2.6)
> +#
> # Since: 0.14.0
> #
> # Notes: @halted is a transient state that changes frequently. By the time
> the
> # data is sent to the client, the guest may no longer be halted.
> ##
> -{ 'struct': 'CpuInfo',
> +{ 'struct': 'CpuInfoBase',
> 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool',
> - 'qom_path': 'str',
> - '*pc': 'int', '*nip': 'int', '*npc': 'int', '*PC': 'int',
> - 'thread_id': 'int'} }
> + 'qom_path': 'str', 'thread_id': 'int', 'arch': 'CpuInfoArch' } }
> +
> +##
> +# @CpuInfo:
> +#
> +# Information about a virtual CPU
> +#
> +# Since: 0.14.0
> +##
> +{ 'union': 'CpuInfo', 'base': 'CpuInfoBase', 'discriminator': 'arch',
> + 'data': { 'x86': 'CpuInfoX86',
> + 'sparc': 'CpuInfoSparc',
> + 'ppc': 'CpuInfoPpc',
> + 'mips': 'CpuInfoMips',
> + 'tricore': 'CpuInfoTricore',
> + 'other': 'CpuInfoOther' } }
> +
> +##
> +# @CpuInfoX86:
> +#
> +# Additional information about a virtual i386 or x86_64 CPU
> +#
> +# @pc: the 64-bit instruction pointer
> +#
> +# Since 2.6
> +##
> +{ 'struct': 'CpuInfoX86', 'data': { 'pc': 'int' } }
> +
> +##
> +# @CpuInfoSparc:
CpuInfoSPARC would be slightly prettier, actually.
> +#
> +# Additional information about a virtual Sparc CPU
> +#
> +# @pc: the PC component of the instruction pointer
> +#
> +# @npc: the NPC component of the instruction pointer
> +#
> +# Since 2.6
> +##
> +{ 'struct': 'CpuInfoSparc', 'data': { 'pc': 'int', 'npc': 'int' } }
> +
> +##
> +# @CpuInfoPpc:
CpuInfoPPC would definitely be prettier.
> +#
> +# Additional information about a virtual PPC CPU
> +#
> +# @nip: the instruction pointer
> +#
> +# Since 2.6
> +##
> +{ 'struct': 'CpuInfoPpc', 'data': { 'nip': 'int' } }
> +
> +##
> +# @CpuInfoMips:
CpuInfoMIPS?
> +#
> +# Additional information about a virtual MIPS CPU
> +#
> +# @PC: the instruction pointer
> +#
> +# Since 2.6
> +##
> +{ 'struct': 'CpuInfoMips', 'data': { 'PC': 'int' } }
> +
> +##
> +# @CpuInfoTricore:
> +#
> +# Additional information about a virtual Tricore CPU
> +#
> +# @PC: the instruction pointer
> +#
> +# Since 2.6
> +##
> +{ 'struct': 'CpuInfoTricore', 'data': { 'PC': 'int' } }
> +
> +##
> +# @CpuInfoOther:
> +#
> +# No additional information is available about the virtual CPU
> +#
> +# Since 2.6
> +#
> +##
> +{ 'struct': 'CpuInfoOther', 'data': { } }
>
> ##
> # @query-cpus:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 9d8b42f..1b5ecb3 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2765,6 +2765,8 @@ Return a json-array. Each CPU is represented by a
> json-object, which contains:
> - "current": true if this is the current CPU, false otherwise (json-bool)
> - "halted": true if the cpu is halted, false otherwise (json-bool)
> - "qom_path": path to the CPU object in the QOM tree (json-str)
> +- "arch": architecture of the cpu, which determines what additional
> + keys will be present (json-str)
> - Current program counter. The key's name depends on the architecture:
> "pc": i386/x86_64 (json-int)
> "nip": PPC (json-int)
> @@ -2782,6 +2784,7 @@ Example:
> "current":true,
> "halted":false,
> "qom_path":"/machine/unattached/device[0]",
> + "arch":"x86",
> "pc":3227107138,
> "thread_id":3134
> },
> @@ -2790,6 +2793,7 @@ Example:
> "current":false,
> "halted":true,
> "qom_path":"/machine/unattached/device[2]",
> + "arch":"x86",
> "pc":7108165,
> "thread_id":3135
> }
