Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-27 Thread Eric Blake 
On 04/27/2018 01:53 AM, Markus Armbruster wrote:

> We could perhaps still declare *all* @arch values useless in v2.12.0,
> then fix them in v2.12.1.
> 

Well, it's still useful to know that "s390" means the extra field is
available even in 2.12.0.  But your plan:

> Or we deprecate @arch right when we introduce @target, and drop it later
> in accordance with our deprecation policy (qemu-doc.texi @appendix
> Deprecated features).  That way, the rather ridiculous code to compute
> it will be temporary.  I think that's cleaner.
> 
>@arch in query-cpus  query-cpus-fast
>before 2.6   nonexistent
>2.6 - 2.11   CpuInfoArch
>2.12 cmd deprecated  CpuInfoArch
>2.13 cmd deprecated  memb deprecated
>2.14 cmd gonememb deprecated
>2.15 cmd gonememb gone

works well for me.  I don't think we can accelerate the deprecation by
backporting that to 2.12.1, or if the deprecation belongs only in 2.13,
but the overall plan is sane (libvirt has the deprecation timeframe to
start accessing 'target' instead of 'arch' when worrying about whether
the extra s390x information is present - if it doesn't already just read
the information when present without worrying about the value of 'arch'
in the first place).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-27 Thread Markus Armbruster 
Laszlo Ersek  writes:

> On 04/26/18 17:51, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> On 04/26/2018 09:34 AM, Markus Armbruster wrote:
>
> Acerbic discovery of the day: @CpuInfoArch has "x86", while configure
> produces:
>
>   TARGET_NAME  TARGET_BASE_ARCH
>  i386  i386
>x86_64  i386
>
> Note how "i386" does not match "x86".

 Review fail.

 Just three weeks ago, we could still have fixed query-cpus-fast...
>>>
>>> Actually, I think we still can.  We already documented in the 2.12
>>> release notes that the "arch" field of query-cpus-fast is known to be
>>> broken for all but "s390x" (which is really the only arch field that
>>> MUST be correct, as that is the only time we send additional
>>> information).  And introspection can easily see both the enum contents
>>> (if we add something) as well as any other additions to the
>>> query-cpus-fast output union (although that is less likely), to use
>>> those as a witness for whether qemu is new enough to have fixed the
>>> bogus "arch" values.  I'd argue that if we change things right now, with
>>> intent to include the change in 2.12.1, before people start relying on
>>> the bogus "arch" of 2.12, then we should feel free to make
>>> query-cpus-fast output whatever we want for all architectures other than
>>> "s390x", even if it changes the current output of "x86".
>> 
>> In other words, we managed to screw up query-cpus-fast sufficiently to
>> let us fix it even now.  Cool, let's do it!
>> 
>
> Let me clarify a little.
>
> The @CpuInfoArch enum has the following constants now:
>
> ['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'riscv', 'other' ]
>
> This enum was originally introduced in 2.6 (according to the
> documentation), and only the @s390 and @riscv constants were added in 2.12.
>
> The enum constants show up in the following two places, *on the wire*:
>
> - in @CpuInfo.@arch, only produced by @query-cpus,
> - in @CpuInfoFast.@arch, only produced by @query-cpus-fast.
>
> The plan would be to rewrite *all* those enum constants of @CpuInfoArch
> to which the respective TARGET_BASE_ARCH values (from "configure") do
> not map *identically*. Here's the mapping:
>
>   TARGET_BASE_ARCH  CpuInfoArch  CpuInfoArch needs change
>     ---  
>   i386  x86  YES
>   sparc sparcno
>   ppc   ppc  no
>   mips  mips no
>   tricore   tricore  no
>   s390x s390 YES
>   riscv riscvno
>
> In other words, @CpuInfoArch would have to be changed to the following:
>
> ['i386', 'sparc', 'ppc', 'mips', 'tricore', 's390x', 'riscv', 'other' ]
>  ^^ ^^^
>
> This means that the @arch field, returned by @query-cpus and
> @query-cpus-fast, would change incompatibly for those QAPI clients that
> look specifically for "x86" or "s390".
>
> Is this a safe change?
>
> I would say, because of the 's390' -> 's390x' change, that it isn't.
>
> (Also, to confirm, the wiki section at
>  states,
>
>   * the query-cpus-fast QMP command reports bogus arch data for all
> architectures except x86 and s390; applications should be careful to
> not rely on the bogus information
>
> It (correctly) refers to "s390". That value would change.)

You're right, that's a compatibility break.

We could perhaps still declare *all* @arch values useless in v2.12.0,
then fix them in v2.12.1.

Or we deprecate @arch right when we introduce @target, and drop it later
in accordance with our deprecation policy (qemu-doc.texi @appendix
Deprecated features).  That way, the rather ridiculous code to compute
it will be temporary.  I think that's cleaner.

   @arch in query-cpus  query-cpus-fast
   before 2.6   nonexistent
   2.6 - 2.11   CpuInfoArch
   2.12 cmd deprecated  CpuInfoArch
   2.13 cmd deprecated  memb deprecated
   2.14 cmd gonememb deprecated
   2.15 cmd gonememb gone

Opinions?

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Laszlo Ersek 
On 04/26/18 17:51, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> On 04/26/2018 09:34 AM, Markus Armbruster wrote:

 Acerbic discovery of the day: @CpuInfoArch has "x86", while configure
 produces:

   TARGET_NAME  TARGET_BASE_ARCH
  i386  i386
x86_64  i386

 Note how "i386" does not match "x86".
>>>
>>> Review fail.
>>>
>>> Just three weeks ago, we could still have fixed query-cpus-fast...
>>
>> Actually, I think we still can.  We already documented in the 2.12
>> release notes that the "arch" field of query-cpus-fast is known to be
>> broken for all but "s390x" (which is really the only arch field that
>> MUST be correct, as that is the only time we send additional
>> information).  And introspection can easily see both the enum contents
>> (if we add something) as well as any other additions to the
>> query-cpus-fast output union (although that is less likely), to use
>> those as a witness for whether qemu is new enough to have fixed the
>> bogus "arch" values.  I'd argue that if we change things right now, with
>> intent to include the change in 2.12.1, before people start relying on
>> the bogus "arch" of 2.12, then we should feel free to make
>> query-cpus-fast output whatever we want for all architectures other than
>> "s390x", even if it changes the current output of "x86".
> 
> In other words, we managed to screw up query-cpus-fast sufficiently to
> let us fix it even now.  Cool, let's do it!
> 

Let me clarify a little.

The @CpuInfoArch enum has the following constants now:

['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'riscv', 'other' ]

This enum was originally introduced in 2.6 (according to the
documentation), and only the @s390 and @riscv constants were added in 2.12.

The enum constants show up in the following two places, *on the wire*:

- in @CpuInfo.@arch, only produced by @query-cpus,
- in @CpuInfoFast.@arch, only produced by @query-cpus-fast.

The plan would be to rewrite *all* those enum constants of @CpuInfoArch
to which the respective TARGET_BASE_ARCH values (from "configure") do
not map *identically*. Here's the mapping:

  TARGET_BASE_ARCH  CpuInfoArch  CpuInfoArch needs change
    ---  
  i386  x86  YES
  sparc sparcno
  ppc   ppc  no
  mips  mips no
  tricore   tricore  no
  s390x s390 YES
  riscv riscvno

In other words, @CpuInfoArch would have to be changed to the following:

['i386', 'sparc', 'ppc', 'mips', 'tricore', 's390x', 'riscv', 'other' ]
 ^^ ^^^

This means that the @arch field, returned by @query-cpus and
@query-cpus-fast, would change incompatibly for those QAPI clients that
look specifically for "x86" or "s390".

Is this a safe change?

I would say, because of the 's390' -> 's390x' change, that it isn't.

(Also, to confirm, the wiki section at
 states,

  * the query-cpus-fast QMP command reports bogus arch data for all
architectures except x86 and s390; applications should be careful to
not rely on the bogus information

It (correctly) refers to "s390". That value would change.)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Markus Armbruster 
Eric Blake  writes:

> On 04/26/2018 09:34 AM, Markus Armbruster wrote:
>>>
>>> Acerbic discovery of the day: @CpuInfoArch has "x86", while configure
>>> produces:
>>>
>>>   TARGET_NAME  TARGET_BASE_ARCH
>>>  i386  i386
>>>x86_64  i386
>>>
>>> Note how "i386" does not match "x86".
>> 
>> Review fail.
>> 
>> Just three weeks ago, we could still have fixed query-cpus-fast...
>
> Actually, I think we still can.  We already documented in the 2.12
> release notes that the "arch" field of query-cpus-fast is known to be
> broken for all but "s390x" (which is really the only arch field that
> MUST be correct, as that is the only time we send additional
> information).  And introspection can easily see both the enum contents
> (if we add something) as well as any other additions to the
> query-cpus-fast output union (although that is less likely), to use
> those as a witness for whether qemu is new enough to have fixed the
> bogus "arch" values.  I'd argue that if we change things right now, with
> intent to include the change in 2.12.1, before people start relying on
> the bogus "arch" of 2.12, then we should feel free to make
> query-cpus-fast output whatever we want for all architectures other than
> "s390x", even if it changes the current output of "x86".

In other words, we managed to screw up query-cpus-fast sufficiently to
let us fix it even now.  Cool, let's do it!

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Eric Blake 
On 04/26/2018 09:34 AM, Markus Armbruster wrote:
>>
>> Acerbic discovery of the day: @CpuInfoArch has "x86", while configure
>> produces:
>>
>>   TARGET_NAME  TARGET_BASE_ARCH
>>  i386  i386
>>x86_64  i386
>>
>> Note how "i386" does not match "x86".
> 
> Review fail.
> 
> Just three weeks ago, we could still have fixed query-cpus-fast...

Actually, I think we still can.  We already documented in the 2.12
release notes that the "arch" field of query-cpus-fast is known to be
broken for all but "s390x" (which is really the only arch field that
MUST be correct, as that is the only time we send additional
information).  And introspection can easily see both the enum contents
(if we add something) as well as any other additions to the
query-cpus-fast output union (although that is less likely), to use
those as a witness for whether qemu is new enough to have fixed the
bogus "arch" values.  I'd argue that if we change things right now, with
intent to include the change in 2.12.1, before people start relying on
the bogus "arch" of 2.12, then we should feel free to make
query-cpus-fast output whatever we want for all architectures other than
"s390x", even if it changes the current output of "x86".

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Markus Armbruster 
Laszlo Ersek  writes:

> On 04/26/18 11:18, Laszlo Ersek wrote:
>> On 04/26/18 08:26, Markus Armbruster wrote:
>>> Laszlo Ersek  writes:
>>>
>>> [...]
 In brief, I think extending configure / the build system would only help
 with the less painful part of this (the scalar mapping), and so it's not
 worth doing.
>>>
>>> We're going to leave deprecated query-cpus alone (see Eric's review of
>>> the previous patch).
>> 
>> Yes.
>> 
>>> Does that change matters?
>> 
>> It does.
>> 
>>> PS: Instead of configure changes, #TARGET_BASE_ARCH might do.
>> 
>> Does that already exist as a macro? Hmm... After grepping my build dir,
>> it doesn't seem to, but maybe I can change that. Because,
>> TARGET_BASE_ARCH is exactly what we need.
>
> Acerbic discovery of the day: @CpuInfoArch has "x86", while configure
> produces:
>
>   TARGET_NAME  TARGET_BASE_ARCH
>  i386  i386
>x86_64  i386
>
> Note how "i386" does not match "x86".

Review fail.

Just three weeks ago, we could still have fixed query-cpus-fast...

> I guess I'll have to keep the sysemu_target_to_cpuinfo_arch() function.

Assuming this is the only offender, you could also s/i386/x86/ before
you pass it to qapi_enum_parse().  Pick what you hate less.

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Laszlo Ersek 
On 04/26/18 11:18, Laszlo Ersek wrote:
> On 04/26/18 08:26, Markus Armbruster wrote:
>> Laszlo Ersek  writes:
>>
>> [...]
>>> In brief, I think extending configure / the build system would only help
>>> with the less painful part of this (the scalar mapping), and so it's not
>>> worth doing.
>>
>> We're going to leave deprecated query-cpus alone (see Eric's review of
>> the previous patch).
> 
> Yes.
> 
>> Does that change matters?
> 
> It does.
> 
>> PS: Instead of configure changes, #TARGET_BASE_ARCH might do.
> 
> Does that already exist as a macro? Hmm... After grepping my build dir,
> it doesn't seem to, but maybe I can change that. Because,
> TARGET_BASE_ARCH is exactly what we need.

Acerbic discovery of the day: @CpuInfoArch has "x86", while configure
produces:

  TARGET_NAME  TARGET_BASE_ARCH
 i386  i386
   x86_64  i386

Note how "i386" does not match "x86".

I guess I'll have to keep the sysemu_target_to_cpuinfo_arch() function.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Markus Armbruster 
Laszlo Ersek  writes:

> On 04/26/18 08:26, Markus Armbruster wrote:
>> Laszlo Ersek  writes:
>> 
>> [...]
>>> In brief, I think extending configure / the build system would only help
>>> with the less painful part of this (the scalar mapping), and so it's not
>>> worth doing.
>> 
>> We're going to leave deprecated query-cpus alone (see Eric's review of
>> the previous patch).
>
> Yes.
>
>> Does that change matters?
>
> It does.
>
>> PS: Instead of configure changes, #TARGET_BASE_ARCH might do.
>
> Does that already exist as a macro? Hmm... After grepping my build dir,
> it doesn't seem to, but maybe I can change that. Because,
> TARGET_BASE_ARCH is exactly what we need.

You're right, it's not in config-target.h, only in config-target.mak.
scripts/create_config would need a (trivial) patch.

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Laszlo Ersek 
On 04/26/18 08:26, Markus Armbruster wrote:
> Laszlo Ersek  writes:
> 
> [...]
>> In brief, I think extending configure / the build system would only help
>> with the less painful part of this (the scalar mapping), and so it's not
>> worth doing.
> 
> We're going to leave deprecated query-cpus alone (see Eric's review of
> the previous patch).

Yes.

> Does that change matters?

It does.

> PS: Instead of configure changes, #TARGET_BASE_ARCH might do.

Does that already exist as a macro? Hmm... After grepping my build dir,
it doesn't seem to, but maybe I can change that. Because,
TARGET_BASE_ARCH is exactly what we need.

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-26 Thread Markus Armbruster 
Laszlo Ersek  writes:

[...]
> In brief, I think extending configure / the build system would only help
> with the less painful part of this (the scalar mapping), and so it's not
> worth doing.

We're going to leave deprecated query-cpus alone (see Eric's review of
the previous patch).  Does that change matters?

PS: Instead of configure changes, #TARGET_BASE_ARCH might do.

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-25 Thread Laszlo Ersek 
On 04/25/18 09:33, Markus Armbruster wrote:
> Laszlo Ersek  writes:

[snip]

>> +static CpuInfoArch sysemu_target_to_cpuinfo_arch(SysEmuTarget target)
>> +{
>> +/*
>> + * The @SysEmuTarget -> @CpuInfoArch mapping below is based on the
>> + * TARGET_ARCH -> TARGET_BASE_ARCH mapping in the "configure" script.
>> + */
>> +switch (target) {
>> +case SYS_EMU_TARGET_I386:
>> +case SYS_EMU_TARGET_X86_64:
>> +return CPU_INFO_ARCH_X86;
>> +
>> +case SYS_EMU_TARGET_PPC:
>> +case SYS_EMU_TARGET_PPCEMB:
>> +case SYS_EMU_TARGET_PPC64:
>> +return CPU_INFO_ARCH_PPC;
>> +
>> +case SYS_EMU_TARGET_SPARC:
>> +case SYS_EMU_TARGET_SPARC64:
>> +return CPU_INFO_ARCH_SPARC;
>> +
>> +case SYS_EMU_TARGET_MIPS:
>> +case SYS_EMU_TARGET_MIPSEL:
>> +case SYS_EMU_TARGET_MIPS64:
>> +case SYS_EMU_TARGET_MIPS64EL:
>> +return CPU_INFO_ARCH_MIPS;
>> +
>> +case SYS_EMU_TARGET_TRICORE:
>> +return CPU_INFO_ARCH_TRICORE;
>> +
>> +case SYS_EMU_TARGET_S390X:
>> +return CPU_INFO_ARCH_S390;
>> +
>> +case SYS_EMU_TARGET_RISCV32:
>> +case SYS_EMU_TARGET_RISCV64:
>> +return CPU_INFO_ARCH_RISCV;
>> +
>> +default:
>> +return CPU_INFO_ARCH_OTHER;
>> +}
>> +}
> 
> Hmm.  Can we avoid duplicating configure's mapping here?  More on that
> below.
> 
>> +
>> +static void cpustate_to_cpuinfo_x86(CpuInfoX86 *info, const CPUState *cpu)
>> +{
>> +#ifdef TARGET_I386
>> +X86CPU *x86_cpu = X86_CPU(cpu);
>> +CPUX86State *env = _cpu->env;
>> +
>> +info->pc = env->eip + env->segs[R_CS].base;
>> +#else
>> +abort();
>> +#endif
>> +}
>> +
>> +static void cpustate_to_cpuinfo_ppc(CpuInfoPPC *info, const CPUState *cpu)
>> +{
>> +#ifdef TARGET_PPC
>> +PowerPCCPU *ppc_cpu = POWERPC_CPU(cpu);
>> +CPUPPCState *env = _cpu->env;
>> +
>> +info->nip = env->nip;
>> +#else
>> +abort();
>> +#endif
>> +}
>> +
>> +static void cpustate_to_cpuinfo_sparc(CpuInfoSPARC *info, const CPUState 
>> *cpu)
>> +{
>> +#ifdef TARGET_SPARC
>> +SPARCCPU *sparc_cpu = SPARC_CPU(cpu);
>> +CPUSPARCState *env = _cpu->env;
>> +
>> +info->pc = env->pc;
>> +info->npc = env->npc;
>> +#else
>> +abort();
>> +#endif
>> +}
>> +
>> +static void cpustate_to_cpuinfo_mips(CpuInfoMIPS *info, const CPUState *cpu)
>> +{
>> +#ifdef TARGET_MIPS
>> +MIPSCPU *mips_cpu = MIPS_CPU(cpu);
>> +CPUMIPSState *env = _cpu->env;
>> +
>> +info->PC = env->active_tc.PC;
>> +#else
>> +abort();
>> +#endif
>> +}
>> +
>> +static void cpustate_to_cpuinfo_tricore(CpuInfoTricore *info,
>> +const CPUState *cpu)
>> +{
>> +#ifdef TARGET_TRICORE
>> +TriCoreCPU *tricore_cpu = TRICORE_CPU(cpu);
>> +CPUTriCoreState *env = _cpu->env;
>> +
>> +info->PC = env->PC;
>> +#else
>> +abort();
>> +#endif
>> +}
>> +
>> +static void cpustate_to_cpuinfo_s390(CpuInfoS390 *info, const CPUState *cpu)
>> +{
>> +#ifdef TARGET_S390X
>> +S390CPU *s390_cpu = S390_CPU(cpu);
>> +CPUS390XState *env = _cpu->env;
>> +
>> +info->cpu_state = env->cpu_state;
>> +#else
>> +abort();
>> +#endif
>> +}
>> +
>> +static void cpustate_to_cpuinfo_riscv(CpuInfoRISCV *info, const CPUState 
>> *cpu)
>> +{
>> +#ifdef TARGET_RISCV
>> +RISCVCPU *riscv_cpu = RISCV_CPU(cpu);
>> +CPURISCVState *env = _cpu->env;
>> +
>> +info->pc = env->pc;
>> +#else
>> +abort();
>> +#endif
>> +}
>> +
> 
> To reduce #ifdeffery here, these helpers could be moved to suitable
> files in target/*/, plus stubs, but I doubt it's worth the bother.

Indeed. I did look for suitable recipient C files under target/*/, in
particular target/*/cpu.c, but my results weren't promising.

For example, "target/ppc/cpu.c" says "PowerPC CPU routines for qemu"
(and its actual contents agree), so quite inappropriate.

I wouldn't like to introduce new files for this, and if we're just
looking for a dumping ground for these functions, let's keep them here.

> 
>>  CpuInfoList *qmp_query_cpus(Error **errp)
>>  {
>>  MachineState *ms = MACHINE(qdev_get_machine());
>>  MachineClass *mc = MACHINE_GET_CLASS(ms);
>>  CpuInfoList *head = NULL, *cur_item = NULL;
>> +SysEmuTarget target = qapi_enum_parse(_lookup, TARGET_NAME,
>> +  -1, _abort);
> 
> Note how configure providing TARGET_NAME makes computing target easy.
> 
> Compare to how sysemu_target_to_cpuinfo_arch() computes arch.  Would it
> make sense to have configure provide TARGET_BASE_NAME, so we can compute
> arch the same way as target?

It would eliminate sysemu_target_to_cpuinfo_arch() entirely, yes.

However, the (quite more painful) mapping below would not be helped:

[snip]

>> +/*
>> + * The @SysEmuTarget -> @CpuInfo mapping below is based on the
>> + * TARGET_ARCH -> TARGET_BASE_ARCH mapping in the "configure" 
>> script.
>> + */
>> +switch (target) {
>> +case

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-25 Thread Markus Armbruster 
Laszlo Ersek  writes:

> Add a new field @target (of type @SysEmuTarget) to the outputs of the
> @query-cpus and @query-cpus-fast commands, which provides more information
> about the emulation target than the field @arch (of type @CpuInfoArch).
> Keep @arch for compatibility.
>
> Make @target the new discriminator for the @CpuInfo and @CpuInfoFast
> return structures. This lets us hoist @arch to @CpuInfoCommon, but it also
> requires some gymnastics in qmp_query_cpus() and qmp_query_cpus_fast().
>
> In particular, conditional compilation cannot be removed, because each
> pair of CPU base arch structures, such as X86CPU/CPUX86State,
> PowerPCCPU/CPUPPCState, SPARCCPU/CPUSPARCState, is only visible when
> building QEMU for a target that maps to that CPU base arch.
>
> Cc: Eric Blake 
> Cc: Markus Armbruster 
> Cc: Paolo Bonzini 
> Cc: Peter Crosthwaite 
> Cc: Richard Henderson 
> Signed-off-by: Laszlo Ersek 
> ---
>
> Notes:
> PATCHv1:
> 
> - new patch
>
>  qapi/misc.json | 118 ++---
>  cpus.c | 275 
> ++---
>  2 files changed, 291 insertions(+), 102 deletions(-)
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index d7b776a5af37..98c15880f9f0 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -361,77 +361,105 @@
>  # Collects fields common to @CpuInfoBase and @CpuInfoFastBase; that is,
>  # fields that are shared by @query-cpus and @query-cpus-fast, and not
>  # specific to the target architecture.
>  #
>  # @qom-path: path to the CPU object in the QOM tree (since 2.4)
>  #
>  # @thread-id: ID of the underlying host thread
>  #
>  # @props: properties describing which node/socket/core/thread the
>  # virtual CPU belongs to, if supported by the board (since 2.10)
>  #
> +# @arch: base architecture of the cpu (since 2.6)
> +#
>  # Since: 2.13
>  ##
>  { 'struct' : 'CpuInfoCommon',
>'data'   : { 'qom-path'  : 'str',
> 'thread-id' : 'int',
> -   '*props': 'CpuInstanceProperties' } }
> +   '*props': 'CpuInstanceProperties',
> +   'arch'  : 'CpuInfoArch' } }
>  
>  ##
>  # @CpuInfoBase:
>  #
>  # Extends @CpuInfoCommon with fields that are specific to the @query-cpus
>  # command, but not specific to the target architecture.
>  #
>  # @CPU: the index of the virtual CPU
>  #
>  # @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.
>  #
> -# @arch: architecture of the cpu, which determines which additional fields
> -#will be listed (since 2.6)
> +# @target: the QEMU system emulation target, which is more specific than
> +#  @arch and determines which additional fields will be listed
> +#
>  #
>  # Since: 2.13
>  #
>  # 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.
> -#Moreover, @arch cannot be moved up to @CpuInfoCommon because
> +#Moreover, @target cannot be moved up to @CpuInfoCommon because
>  #that would prevent its use as the discriminator in @CpuInfo.
>  ##
>  { 'struct' : 'CpuInfoBase',
>'base'   : 'CpuInfoCommon',
>'data'   : { 'CPU' : 'int',
> 'current' : 'bool',
> 'halted'  : 'bool',
> -   'arch': 'CpuInfoArch' } }
> +   'target'  : 'SysEmuTarget' } }
>  
>  ##
>  # @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',
> -'s390': 'CpuInfoS390',
> -'riscv': 'CpuInfoRISCV',
> -'other': 'CpuInfoOther' } }
> +{ 'union' : 'CpuInfo',
> +  'base'  : 'CpuInfoBase',
> +  'discriminator' : 'target',
> +  'data'  : { 'i386' : 'CpuInfoX86',
> +  'x86_64'   : 'CpuInfoX86',
> +  'sparc': 'CpuInfoSPARC',
> +  'sparc64'  : 'CpuInfoSPARC',
> +  'ppc'  : 'CpuInfoPPC',
> +  'ppcemb'   : 'CpuInfoPPC',
> +  'ppc64': 'CpuInfoPPC',
> +  'mips' : 'CpuInfoMIPS',
> +  'mipsel'   : 'CpuInfoMIPS',
> +  'mips64'   : 'CpuInfoMIPS',
> +  'mips64el' : 'CpuInfoMIPS',
> +  'tricore'  :

[Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch

2018-04-24 Thread Laszlo Ersek 
Add a new field @target (of type @SysEmuTarget) to the outputs of the
@query-cpus and @query-cpus-fast commands, which provides more information
about the emulation target than the field @arch (of type @CpuInfoArch).
Keep @arch for compatibility.

Make @target the new discriminator for the @CpuInfo and @CpuInfoFast
return structures. This lets us hoist @arch to @CpuInfoCommon, but it also
requires some gymnastics in qmp_query_cpus() and qmp_query_cpus_fast().

In particular, conditional compilation cannot be removed, because each
pair of CPU base arch structures, such as X86CPU/CPUX86State,
PowerPCCPU/CPUPPCState, SPARCCPU/CPUSPARCState, is only visible when
building QEMU for a target that maps to that CPU base arch.

Cc: Eric Blake 
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Peter Crosthwaite 
Cc: Richard Henderson 
Signed-off-by: Laszlo Ersek 
---

Notes:
PATCHv1:

- new patch

 qapi/misc.json | 118 ++---
 cpus.c | 275 ++---
 2 files changed, 291 insertions(+), 102 deletions(-)

diff --git a/qapi/misc.json b/qapi/misc.json
index d7b776a5af37..98c15880f9f0 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -361,77 +361,105 @@
 # Collects fields common to @CpuInfoBase and @CpuInfoFastBase; that is,
 # fields that are shared by @query-cpus and @query-cpus-fast, and not
 # specific to the target architecture.
 #
 # @qom-path: path to the CPU object in the QOM tree (since 2.4)
 #
 # @thread-id: ID of the underlying host thread
 #
 # @props: properties describing which node/socket/core/thread the
 # virtual CPU belongs to, if supported by the board (since 2.10)
 #
+# @arch: base architecture of the cpu (since 2.6)
+#
 # Since: 2.13
 ##
 { 'struct' : 'CpuInfoCommon',
   'data'   : { 'qom-path'  : 'str',
'thread-id' : 'int',
-   '*props': 'CpuInstanceProperties' } }
+   '*props': 'CpuInstanceProperties',
+   'arch'  : 'CpuInfoArch' } }
 
 ##
 # @CpuInfoBase:
 #
 # Extends @CpuInfoCommon with fields that are specific to the @query-cpus
 # command, but not specific to the target architecture.
 #
 # @CPU: the index of the virtual CPU
 #
 # @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.
 #
-# @arch: architecture of the cpu, which determines which additional fields
-#will be listed (since 2.6)
+# @target: the QEMU system emulation target, which is more specific than
+#  @arch and determines which additional fields will be listed
+#
 #
 # Since: 2.13
 #
 # 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.
-#Moreover, @arch cannot be moved up to @CpuInfoCommon because
+#Moreover, @target cannot be moved up to @CpuInfoCommon because
 #that would prevent its use as the discriminator in @CpuInfo.
 ##
 { 'struct' : 'CpuInfoBase',
   'base'   : 'CpuInfoCommon',
   'data'   : { 'CPU' : 'int',
'current' : 'bool',
'halted'  : 'bool',
-   'arch': 'CpuInfoArch' } }
+   'target'  : 'SysEmuTarget' } }
 
 ##
 # @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',
-'s390': 'CpuInfoS390',
-'riscv': 'CpuInfoRISCV',
-'other': 'CpuInfoOther' } }
+{ 'union' : 'CpuInfo',
+  'base'  : 'CpuInfoBase',
+  'discriminator' : 'target',
+  'data'  : { 'i386' : 'CpuInfoX86',
+  'x86_64'   : 'CpuInfoX86',
+  'sparc': 'CpuInfoSPARC',
+  'sparc64'  : 'CpuInfoSPARC',
+  'ppc'  : 'CpuInfoPPC',
+  'ppcemb'   : 'CpuInfoPPC',
+  'ppc64': 'CpuInfoPPC',
+  'mips' : 'CpuInfoMIPS',
+  'mipsel'   : 'CpuInfoMIPS',
+  'mips64'   : 'CpuInfoMIPS',
+  'mips64el' : 'CpuInfoMIPS',
+  'tricore'  : 'CpuInfoTricore',
+  's390x': 'CpuInfoS390',
+  'riscv32'  : 'CpuInfoRISCV',
+  'riscv64'  : 'CpuInfoRISCV',
+  'aarch64'  : 'CpuInfoOther',
+  'alpha': 'CpuInfoOther',
+