diff mbox

[2/6] qapi: handle the riscv CpuInfoArch in query-cpus-fast

Message ID 20180424214550.32549-3-lersek@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laszlo Ersek April 24, 2018, 9:45 p.m. UTC
Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
and @query-cpus-fast commands, respectively), and assigned, in both return
structures, the @CpuInfoRISCV sub-structure to the new enum value.

However, qmp_query_cpus_fast() would not populate either the @arch field
or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
qmp_query_cpus() would.

In theory, there are two ways to fix this:

(a) Fill in both the @arch field and the @CpuInfoRISCV sub-structure in
    qmp_query_cpus_fast(), by copying the logic from qmp_query_cpus().

(b) Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
    populate only the @arch field in qmp_query_cpus_fast().

Approach (b) seems more robust, because:

- clearly there has never been an attempt to get actual RISV CPU state
  from qmp_query_cpus_fast(), so its lack of RISCV support is not actually
  a problem,

- getting CPU state without interrupting KVM looks like an exceptional
  thing to do (only S390X does it currently).

Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Eric Blake <eblake@redhat.com>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Michael Clark <mjc@sifive.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Riku Voipio <riku.voipio@iki.fi>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: qemu-stable@nongnu.org
Fixes: 25fa194b7b11901561532e435beb83d046899f7a
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    PATCHv1:
    
    - new patch

 qapi/misc.json | 2 +-
 cpus.c         | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Eric Blake April 24, 2018, 10:32 p.m. UTC | #1
On 04/24/2018 04:45 PM, Laszlo Ersek wrote:
> Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
> in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
> and @query-cpus-fast commands, respectively), and assigned, in both return
> structures, the @CpuInfoRISCV sub-structure to the new enum value.
> 
> However, qmp_query_cpus_fast() would not populate either the @arch field
> or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
> qmp_query_cpus() would.
> 
> In theory, there are two ways to fix this:
> 
> (a) Fill in both the @arch field and the @CpuInfoRISCV sub-structure in
>     qmp_query_cpus_fast(), by copying the logic from qmp_query_cpus().
> 
> (b) Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
>     populate only the @arch field in qmp_query_cpus_fast().
> 
> Approach (b) seems more robust, because:
> 
> - clearly there has never been an attempt to get actual RISV CPU state
>   from qmp_query_cpus_fast(), so its lack of RISCV support is not actually
>   a problem,
> 
> - getting CPU state without interrupting KVM looks like an exceptional
>   thing to do (only S390X does it currently).
> 
...
> Cc: qemu-stable@nongnu.org
> Fixes: 25fa194b7b11901561532e435beb83d046899f7a
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster April 25, 2018, 6:44 a.m. UTC | #2
Laszlo Ersek <lersek@redhat.com> writes:

> Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
> in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
> and @query-cpus-fast commands, respectively), and assigned, in both return
> structures, the @CpuInfoRISCV sub-structure to the new enum value.
>
> However, qmp_query_cpus_fast() would not populate either the @arch field
> or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
> qmp_query_cpus() would.
>
> In theory, there are two ways to fix this:
>
> (a) Fill in both the @arch field and the @CpuInfoRISCV sub-structure in
>     qmp_query_cpus_fast(), by copying the logic from qmp_query_cpus().
>
> (b) Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
>     populate only the @arch field in qmp_query_cpus_fast().
>
> Approach (b) seems more robust, because:
>
> - clearly there has never been an attempt to get actual RISV CPU state
>   from qmp_query_cpus_fast(), so its lack of RISCV support is not actually
>   a problem,
>
> - getting CPU state without interrupting KVM looks like an exceptional
>   thing to do (only S390X does it currently).
>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Michael Clark <mjc@sifive.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Riku Voipio <riku.voipio@iki.fi>
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: qemu-stable@nongnu.org
> Fixes: 25fa194b7b11901561532e435beb83d046899f7a
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
>     PATCHv1:
>     
>     - new patch
>
>  qapi/misc.json | 2 +-
>  cpus.c         | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 5636f4a14997..104d013adba6 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -565,23 +565,23 @@
>  { 'union': 'CpuInfoFast',
>    'base': {'cpu-index': 'int', 'qom-path': 'str',
>             'thread-id': 'int', '*props': 'CpuInstanceProperties',
>             'arch': 'CpuInfoArch' },
>    'discriminator': 'arch',
>    'data': { 'x86': 'CpuInfoOther',
>              'sparc': 'CpuInfoOther',
>              'ppc': 'CpuInfoOther',
>              'mips': 'CpuInfoOther',
>              'tricore': 'CpuInfoOther',
>              's390': 'CpuInfoS390',
> -            'riscv': 'CpuInfoRISCV',
> +            'riscv': 'CpuInfoOther',
>              'other': 'CpuInfoOther' } }

Why do CpuInfoFast's variants match CpuInfo's for s390, but not the
others?  Your commit message has an educated guess: "looks like an
exceptional thing to do (only S390X does it currently)".  But why guess
when we can ask authors of commit ce74ee3dea6?  Luiz and Victor, please
advise.

>  ##
>  # @query-cpus-fast:
>  #
>  # Returns information about all virtual CPUs. This command does not
>  # incur a performance penalty and should be used in production
>  # instead of query-cpus.
>  #
>  # Returns: list of @CpuInfoFast
>  #
> diff --git a/cpus.c b/cpus.c
> index 1a9a2edee1f2..60563a6d54ec 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -2225,22 +2225,24 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
>  #elif defined(TARGET_SPARC)
>          info->value->arch = CPU_INFO_ARCH_SPARC;
>  #elif defined(TARGET_MIPS)
>          info->value->arch = CPU_INFO_ARCH_MIPS;
>  #elif defined(TARGET_TRICORE)
>          info->value->arch = CPU_INFO_ARCH_TRICORE;
>  #elif defined(TARGET_S390X)
>          s390_cpu = S390_CPU(cpu);
>          env = &s390_cpu->env;
>          info->value->arch = CPU_INFO_ARCH_S390;
>          info->value->u.s390.cpu_state = env->cpu_state;
> +#elif defined(TARGET_RISCV)
> +        info->value->arch = CPU_INFO_ARCH_RISCV;
>  #else
>          info->value->arch = CPU_INFO_ARCH_OTHER;
>  #endif
>          if (!cur_item) {
>              head = cur_item = info;
>          } else {
>              cur_item->next = info;
>              cur_item = info;
>          }
>      }
Cornelia Huck April 25, 2018, 7:48 a.m. UTC | #3
On Wed, 25 Apr 2018 08:44:15 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Laszlo Ersek <lersek@redhat.com> writes:
> 
> > Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
> > in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
> > and @query-cpus-fast commands, respectively), and assigned, in both return
> > structures, the @CpuInfoRISCV sub-structure to the new enum value.
> >
> > However, qmp_query_cpus_fast() would not populate either the @arch field
> > or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
> > qmp_query_cpus() would.
> >
> > In theory, there are two ways to fix this:
> >
> > (a) Fill in both the @arch field and the @CpuInfoRISCV sub-structure in
> >     qmp_query_cpus_fast(), by copying the logic from qmp_query_cpus().
> >
> > (b) Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
> >     populate only the @arch field in qmp_query_cpus_fast().
> >
> > Approach (b) seems more robust, because:
> >
> > - clearly there has never been an attempt to get actual RISV CPU state
> >   from qmp_query_cpus_fast(), so its lack of RISCV support is not actually
> >   a problem,
> >
> > - getting CPU state without interrupting KVM looks like an exceptional
> >   thing to do (only S390X does it currently).
> >
> > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> > Cc: Eric Blake <eblake@redhat.com>
> > Cc: Laurent Vivier <laurent@vivier.eu>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Cc: Michael Clark <mjc@sifive.com>
> > Cc: Palmer Dabbelt <palmer@sifive.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Riku Voipio <riku.voipio@iki.fi>
> > Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> > Cc: qemu-stable@nongnu.org
> > Fixes: 25fa194b7b11901561532e435beb83d046899f7a
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> >
> > Notes:
> >     PATCHv1:
> >     
> >     - new patch
> >
> >  qapi/misc.json | 2 +-
> >  cpus.c         | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi/misc.json b/qapi/misc.json
> > index 5636f4a14997..104d013adba6 100644
> > --- a/qapi/misc.json
> > +++ b/qapi/misc.json
> > @@ -565,23 +565,23 @@
> >  { 'union': 'CpuInfoFast',
> >    'base': {'cpu-index': 'int', 'qom-path': 'str',
> >             'thread-id': 'int', '*props': 'CpuInstanceProperties',
> >             'arch': 'CpuInfoArch' },
> >    'discriminator': 'arch',
> >    'data': { 'x86': 'CpuInfoOther',
> >              'sparc': 'CpuInfoOther',
> >              'ppc': 'CpuInfoOther',
> >              'mips': 'CpuInfoOther',
> >              'tricore': 'CpuInfoOther',
> >              's390': 'CpuInfoS390',
> > -            'riscv': 'CpuInfoRISCV',
> > +            'riscv': 'CpuInfoOther',
> >              'other': 'CpuInfoOther' } }  
> 
> Why do CpuInfoFast's variants match CpuInfo's for s390, but not the
> others?  Your commit message has an educated guess: "looks like an
> exceptional thing to do (only S390X does it currently)".  But why guess
> when we can ask authors of commit ce74ee3dea6?  Luiz and Victor, please
> advise.

I'm neither Luiz nor Viktor, but Laszlo's educated guess is correct. See
https://www.redhat.com/archives/libvir-list/2018-February/msg00121.html
for some background. So yes, s390x is exceptional in that it has state
in QEMU that is actually interesting for upper layers and can be
retrieved without performance penalty.

Might make sense to refer to the above.

> 
> >  ##
> >  # @query-cpus-fast:
> >  #
> >  # Returns information about all virtual CPUs. This command does not
> >  # incur a performance penalty and should be used in production
> >  # instead of query-cpus.
> >  #
> >  # Returns: list of @CpuInfoFast
> >  #
> > diff --git a/cpus.c b/cpus.c
> > index 1a9a2edee1f2..60563a6d54ec 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -2225,22 +2225,24 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
> >  #elif defined(TARGET_SPARC)
> >          info->value->arch = CPU_INFO_ARCH_SPARC;
> >  #elif defined(TARGET_MIPS)
> >          info->value->arch = CPU_INFO_ARCH_MIPS;
> >  #elif defined(TARGET_TRICORE)
> >          info->value->arch = CPU_INFO_ARCH_TRICORE;
> >  #elif defined(TARGET_S390X)
> >          s390_cpu = S390_CPU(cpu);
> >          env = &s390_cpu->env;
> >          info->value->arch = CPU_INFO_ARCH_S390;
> >          info->value->u.s390.cpu_state = env->cpu_state;
> > +#elif defined(TARGET_RISCV)
> > +        info->value->arch = CPU_INFO_ARCH_RISCV;
> >  #else
> >          info->value->arch = CPU_INFO_ARCH_OTHER;
> >  #endif
> >          if (!cur_item) {
> >              head = cur_item = info;
> >          } else {
> >              cur_item->next = info;
> >              cur_item = info;
> >          }
> >      }  
>
Laszlo Ersek April 25, 2018, 12:32 p.m. UTC | #4
On 04/25/18 00:32, Eric Blake wrote:
> On 04/24/2018 04:45 PM, Laszlo Ersek wrote:
>> Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
>> in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
>> and @query-cpus-fast commands, respectively), and assigned, in both return
>> structures, the @CpuInfoRISCV sub-structure to the new enum value.
>>
>> However, qmp_query_cpus_fast() would not populate either the @arch field
>> or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
>> qmp_query_cpus() would.
>>
>> In theory, there are two ways to fix this:
>>
>> (a) Fill in both the @arch field and the @CpuInfoRISCV sub-structure in
>>     qmp_query_cpus_fast(), by copying the logic from qmp_query_cpus().
>>
>> (b) Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
>>     populate only the @arch field in qmp_query_cpus_fast().
>>
>> Approach (b) seems more robust, because:
>>
>> - clearly there has never been an attempt to get actual RISV CPU state
>>   from qmp_query_cpus_fast(), so its lack of RISCV support is not actually
>>   a problem,
>>
>> - getting CPU state without interrupting KVM looks like an exceptional
>>   thing to do (only S390X does it currently).
>>
> ...
>> Cc: qemu-stable@nongnu.org
>> Fixes: 25fa194b7b11901561532e435beb83d046899f7a
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Thanks Eric, I'll fuse your R-b's from #1 and #2 into one R-b for the
squashed variant. Hope that's OK. (Please do check the upcoming commit
message for more garblage.)

Thanks!
Laszlo
Viktor Mihajlovski April 25, 2018, 12:38 p.m. UTC | #5
On 25.04.2018 09:48, Cornelia Huck wrote:
> On Wed, 25 Apr 2018 08:44:15 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
> 
>> Laszlo Ersek <lersek@redhat.com> writes:
>>
>>> Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
>>> in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
>>> and @query-cpus-fast commands, respectively), and assigned, in both return
>>> structures, the @CpuInfoRISCV sub-structure to the new enum value.
>>>
>>> However, qmp_query_cpus_fast() would not populate either the @arch field
>>> or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
>>> qmp_query_cpus() would.
>>>
>>> In theory, there are two ways to fix this:
>>>
>>> (a) Fill in both the @arch field and the @CpuInfoRISCV sub-structure in
>>>     qmp_query_cpus_fast(), by copying the logic from qmp_query_cpus().
>>>
>>> (b) Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
>>>     populate only the @arch field in qmp_query_cpus_fast().
>>>
>>> Approach (b) seems more robust, because:
>>>
>>> - clearly there has never been an attempt to get actual RISV CPU state
>>>   from qmp_query_cpus_fast(), so its lack of RISCV support is not actually
>>>   a problem,
>>>
>>> - getting CPU state without interrupting KVM looks like an exceptional
>>>   thing to do (only S390X does it currently).
>>>
>>> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Laurent Vivier <laurent@vivier.eu>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Michael Clark <mjc@sifive.com>
>>> Cc: Palmer Dabbelt <palmer@sifive.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>> Cc: Richard Henderson <rth@twiddle.net>
>>> Cc: Riku Voipio <riku.voipio@iki.fi>
>>> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
>>> Cc: qemu-stable@nongnu.org
>>> Fixes: 25fa194b7b11901561532e435beb83d046899f7a
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>>
>>> Notes:
>>>     PATCHv1:
>>>     
>>>     - new patch
>>>
>>>  qapi/misc.json | 2 +-
>>>  cpus.c         | 2 ++
>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/misc.json b/qapi/misc.json
>>> index 5636f4a14997..104d013adba6 100644
>>> --- a/qapi/misc.json
>>> +++ b/qapi/misc.json
>>> @@ -565,23 +565,23 @@
>>>  { 'union': 'CpuInfoFast',
>>>    'base': {'cpu-index': 'int', 'qom-path': 'str',
>>>             'thread-id': 'int', '*props': 'CpuInstanceProperties',
>>>             'arch': 'CpuInfoArch' },
>>>    'discriminator': 'arch',
>>>    'data': { 'x86': 'CpuInfoOther',
>>>              'sparc': 'CpuInfoOther',
>>>              'ppc': 'CpuInfoOther',
>>>              'mips': 'CpuInfoOther',
>>>              'tricore': 'CpuInfoOther',
>>>              's390': 'CpuInfoS390',
>>> -            'riscv': 'CpuInfoRISCV',
>>> +            'riscv': 'CpuInfoOther',
>>>              'other': 'CpuInfoOther' } }  
>>
>> Why do CpuInfoFast's variants match CpuInfo's for s390, but not the
>> others?  Your commit message has an educated guess: "looks like an
>> exceptional thing to do (only S390X does it currently)".  But why guess
>> when we can ask authors of commit ce74ee3dea6?  Luiz and Victor, please
>> advise.
> 
> I'm neither Luiz nor Viktor, but Laszlo's educated guess is correct. See
> https://www.redhat.com/archives/libvir-list/2018-February/msg00121.html
> for some background. So yes, s390x is exceptional in that it has state
> in QEMU that is actually interesting for upper layers and can be
> retrieved without performance penalty.
Right, the only 'upper layer' using this I'm aware of is libvirt and the
arch evaluation is only done for s390. Not pretty, but no harm done so far.
> 
> Might make sense to refer to the above.
> 
[...]
Laszlo Ersek April 25, 2018, 12:43 p.m. UTC | #6
On 04/25/18 09:48, Cornelia Huck wrote:
> On Wed, 25 Apr 2018 08:44:15 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
> 
>> Laszlo Ersek <lersek@redhat.com> writes:
>>
>>> Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
>>> in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
>>> and @query-cpus-fast commands, respectively), and assigned, in both return
>>> structures, the @CpuInfoRISCV sub-structure to the new enum value.
>>>
>>> However, qmp_query_cpus_fast() would not populate either the @arch field
>>> or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
>>> qmp_query_cpus() would.
>>>
>>> In theory, there are two ways to fix this:
>>>
>>> (a) Fill in both the @arch field and the @CpuInfoRISCV sub-structure in
>>>     qmp_query_cpus_fast(), by copying the logic from qmp_query_cpus().
>>>
>>> (b) Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
>>>     populate only the @arch field in qmp_query_cpus_fast().
>>>
>>> Approach (b) seems more robust, because:
>>>
>>> - clearly there has never been an attempt to get actual RISV CPU state
>>>   from qmp_query_cpus_fast(), so its lack of RISCV support is not actually
>>>   a problem,
>>>
>>> - getting CPU state without interrupting KVM looks like an exceptional
>>>   thing to do (only S390X does it currently).
>>>
>>> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Laurent Vivier <laurent@vivier.eu>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Michael Clark <mjc@sifive.com>
>>> Cc: Palmer Dabbelt <palmer@sifive.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>> Cc: Richard Henderson <rth@twiddle.net>
>>> Cc: Riku Voipio <riku.voipio@iki.fi>
>>> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
>>> Cc: qemu-stable@nongnu.org
>>> Fixes: 25fa194b7b11901561532e435beb83d046899f7a
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>>
>>> Notes:
>>>     PATCHv1:
>>>     
>>>     - new patch
>>>
>>>  qapi/misc.json | 2 +-
>>>  cpus.c         | 2 ++
>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/misc.json b/qapi/misc.json
>>> index 5636f4a14997..104d013adba6 100644
>>> --- a/qapi/misc.json
>>> +++ b/qapi/misc.json
>>> @@ -565,23 +565,23 @@
>>>  { 'union': 'CpuInfoFast',
>>>    'base': {'cpu-index': 'int', 'qom-path': 'str',
>>>             'thread-id': 'int', '*props': 'CpuInstanceProperties',
>>>             'arch': 'CpuInfoArch' },
>>>    'discriminator': 'arch',
>>>    'data': { 'x86': 'CpuInfoOther',
>>>              'sparc': 'CpuInfoOther',
>>>              'ppc': 'CpuInfoOther',
>>>              'mips': 'CpuInfoOther',
>>>              'tricore': 'CpuInfoOther',
>>>              's390': 'CpuInfoS390',
>>> -            'riscv': 'CpuInfoRISCV',
>>> +            'riscv': 'CpuInfoOther',
>>>              'other': 'CpuInfoOther' } }  
>>
>> Why do CpuInfoFast's variants match CpuInfo's for s390, but not the
>> others?  Your commit message has an educated guess: "looks like an
>> exceptional thing to do (only S390X does it currently)".  But why guess
>> when we can ask authors of commit ce74ee3dea6?  Luiz and Victor, please
>> advise.
> 
> I'm neither Luiz nor Viktor, but Laszlo's educated guess is correct. See
> https://www.redhat.com/archives/libvir-list/2018-February/msg00121.html
> for some background. So yes, s390x is exceptional in that it has state
> in QEMU that is actually interesting for upper layers and can be
> retrieved without performance penalty.
> 
> Might make sense to refer to the above.

I'll plagiarize the living daylights [1] out of Cornelia's summary, and
insert the mailing list archive link too. (Viktor's detailed explanation
is likely best not corrupted by yours truly...)

[1] this expression itself was stolen :)

Thanks!
Laszlo
diff mbox

Patch

diff --git a/qapi/misc.json b/qapi/misc.json
index 5636f4a14997..104d013adba6 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -565,23 +565,23 @@ 
 { 'union': 'CpuInfoFast',
   'base': {'cpu-index': 'int', 'qom-path': 'str',
            'thread-id': 'int', '*props': 'CpuInstanceProperties',
            'arch': 'CpuInfoArch' },
   'discriminator': 'arch',
   'data': { 'x86': 'CpuInfoOther',
             'sparc': 'CpuInfoOther',
             'ppc': 'CpuInfoOther',
             'mips': 'CpuInfoOther',
             'tricore': 'CpuInfoOther',
             's390': 'CpuInfoS390',
-            'riscv': 'CpuInfoRISCV',
+            'riscv': 'CpuInfoOther',
             'other': 'CpuInfoOther' } }
 
 ##
 # @query-cpus-fast:
 #
 # Returns information about all virtual CPUs. This command does not
 # incur a performance penalty and should be used in production
 # instead of query-cpus.
 #
 # Returns: list of @CpuInfoFast
 #
diff --git a/cpus.c b/cpus.c
index 1a9a2edee1f2..60563a6d54ec 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2225,22 +2225,24 @@  CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
 #elif defined(TARGET_SPARC)
         info->value->arch = CPU_INFO_ARCH_SPARC;
 #elif defined(TARGET_MIPS)
         info->value->arch = CPU_INFO_ARCH_MIPS;
 #elif defined(TARGET_TRICORE)
         info->value->arch = CPU_INFO_ARCH_TRICORE;
 #elif defined(TARGET_S390X)
         s390_cpu = S390_CPU(cpu);
         env = &s390_cpu->env;
         info->value->arch = CPU_INFO_ARCH_S390;
         info->value->u.s390.cpu_state = env->cpu_state;
+#elif defined(TARGET_RISCV)
+        info->value->arch = CPU_INFO_ARCH_RISCV;
 #else
         info->value->arch = CPU_INFO_ARCH_OTHER;
 #endif
         if (!cur_item) {
             head = cur_item = info;
         } else {
             cur_item->next = info;
             cur_item = info;
         }
     }