[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [Qemu-devel] [PATCH 2/6] qapi: handle the riscv CpuInf
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-stable] [Qemu-devel] [PATCH 2/6] qapi: handle the riscv CpuInfoArch in query-cpus-fast |
Date: |
Wed, 25 Apr 2018 14:43:36 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 04/25/18 09:48, Cornelia Huck wrote:
> On Wed, 25 Apr 2018 08:44:15 +0200
> Markus Armbruster <address@hidden> wrote:
>
>> Laszlo Ersek <address@hidden> 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 <address@hidden>
>>> Cc: Eric Blake <address@hidden>
>>> Cc: Laurent Vivier <address@hidden>
>>> Cc: Markus Armbruster <address@hidden>
>>> Cc: Michael Clark <address@hidden>
>>> Cc: Palmer Dabbelt <address@hidden>
>>> Cc: Paolo Bonzini <address@hidden>
>>> Cc: Peter Crosthwaite <address@hidden>
>>> Cc: Richard Henderson <address@hidden>
>>> Cc: Riku Voipio <address@hidden>
>>> Cc: Sagar Karandikar <address@hidden>
>>> Cc: address@hidden
>>> Fixes: 25fa194b7b11901561532e435beb83d046899f7a
>>> Signed-off-by: Laszlo Ersek <address@hidden>
>>> ---
>>>
>>> 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