Re: [PATCH v9] qapi: introduce 'query-kvm-cpuid' action

[Claudio Fontana <cfontana@suse.de>]

On 6/17/21 9:49 AM, Valeriy Vdovin wrote:
> On Thu, Jun 17, 2021 at 07:22:36AM +0200, Markus Armbruster wrote:
>> Valeriy Vdovin <valeriy.vdovin@virtuozzo.com> writes:
>>
>>> Introducing new qapi method 'query-kvm-cpuid'. This method can be used to
>>
>> It's actually a QMP command.  There are no "qapi methods".
>>
>>> get virtualized cpu model info generated by QEMU during VM initialization in
>>> the form of cpuid representation.
>>>
>>> Diving into more details about virtual cpu generation: QEMU first parses '-cpu'
>>
>> virtual CPU
>>
>>> command line option. From there it takes the name of the model as the basis for
>>> feature set of the new virtual cpu. After that it uses trailing '-cpu' options,
>>> that state if additional cpu features should be present on the virtual cpu or
>>> excluded from it (tokens '+'/'-' or '=on'/'=off').
>>> After that QEMU checks if the host's cpu can actually support the derived
>>> feature set and applies host limitations to it.
>>> After this initialization procedure, virtual cpu has it's model and
>>> vendor names, and a working feature set and is ready for identification
>>> instructions such as CPUID.
>>>
>>> Currently full output for this method is only supported for x86 cpus.
>>
>> Not sure about "currently": the interface looks quite x86-specific to me.
>>
> Yes, at some point I was thinking this interface could become generic,
> but does not seem possible, so I'll remove this note.


Why is it impossible? What is the use case, is it something useful for example for ARM?


> 
>> The commit message doesn't mention KVM except in the command name.  The
>> schema provides the command only if defined(CONFIG_KVM).
>>
>> Can you explain why you need the restriction to CONFIG_KVM?
>>
> This CONFIG_KVM is used as a solution to a broken build if --disable-kvm
> flag is set. I was choosing between this and writing empty implementation into
> kvm-stub.c
>>> To learn exactly how virtual cpu is presented to the guest machine via CPUID
>>> instruction, new qapi method can be used. By calling 'query-kvm-cpuid'
>>
>> QMP command again.
>>
>>> method, one can get a full listing of all CPUID leafs with subleafs which are
>>> supported by the initialized virtual cpu.
>>>
>>> Other than debug, the method is useful in cases when we would like to
>>> utilize QEMU's virtual cpu initialization routines and put the retrieved
>>> values into kernel CPUID overriding mechanics for more precise control
>>> over how various processes perceive its underlying hardware with
>>> container processes as a good example.
>>>
>>> Output format:
>>> The output is a plain list of leaf/subleaf agrument combinations, that
>>
>> Typo: argument
>>
>>> return 4 words in registers EAX, EBX, ECX, EDX.
>>>
>>> Use example:
>>> qmp_request: {
>>>   "execute": "query-kvm-cpuid"
>>> }
>>>
>>> qmp_response: {
>>>   "return": [
>>>     {
>>>       "eax": 1073741825,
>>>       "edx": 77,
>>>       "in-eax": 1073741824,
>>>       "ecx": 1447775574,
>>>       "ebx": 1263359563
>>>     },
>>>     {
>>>       "eax": 16777339,
>>>       "edx": 0,
>>>       "in-eax": 1073741825,
>>>       "ecx": 0,
>>>       "ebx": 0
>>>     },
>>>     {
>>>       "eax": 13,
>>>       "edx": 1231384169,
>>>       "in-eax": 0,
>>>       "ecx": 1818588270,
>>>       "ebx": 1970169159
>>>     },
>>>     {
>>>       "eax": 198354,
>>>       "edx": 126614527,
>>>       "in-eax": 1,
>>>       "ecx": 2176328193,
>>>       "ebx": 2048
>>>     },
>>>     ....
>>>     {
>>>       "eax": 12328,
>>>       "edx": 0,
>>>       "in-eax": 2147483656,
>>>       "ecx": 0,
>>>       "ebx": 0
>>>     }
>>>   ]
>>> }
>>>
>>> Signed-off-by: Valeriy Vdovin <valeriy.vdovin@virtuozzo.com>
>>> ---
>>> v2: - Removed leaf/subleaf iterators.
>>>     - Modified cpu_x86_cpuid to return false in cases when count is
>>>       greater than supported subleaves.
>>> v3: - Fixed structure name coding style.
>>>     - Added more comments
>>>     - Ensured buildability for non-x86 targets.
>>> v4: - Fixed cpu_x86_cpuid return value logic and handling of 0xA leaf.
>>>     - Fixed comments.
>>>     - Removed target check in qmp_query_cpu_model_cpuid.
>>> v5: - Added error handling code in qmp_query_cpu_model_cpuid
>>> v6: - Fixed error handling code. Added method to query_error_class
>>> v7: - Changed implementation in favor of cached cpuid_data for
>>>       KVM_SET_CPUID2
>>> v8: - Renamed qmp method to query-kvm-cpuid and some fields in response.
>>>     - Modified documentation to qmp method
>>>     - Removed helper struct declaration
>>> v9: - Renamed 'in_eax' / 'in_ecx' fields to 'in-eax' / 'in-ecx'
>>>     - Pasted more complete response to commit message.
>>>
>>>  qapi/machine-target.json   | 43 ++++++++++++++++++++++++++++++++++++++
>>>  target/i386/kvm/kvm.c      | 37 ++++++++++++++++++++++++++++++++
>>>  tests/qtest/qmp-cmd-test.c |  1 +
>>>  3 files changed, 81 insertions(+)
>>>
>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>> index e7811654b7..1e591ba481 100644
>>> --- a/qapi/machine-target.json
>>> +++ b/qapi/machine-target.json
>>> @@ -329,3 +329,46 @@
>>>  ##
>>>  { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>>>    'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
>>> +
>>> +##
>>> +# @CpuidEntry:
>>> +#
>>> +# A single entry of a CPUID response.
>>> +#
>>> +# One entry holds full set of information (leaf) returned to the guest in response
>>> +# to it calling a CPUID instruction with eax, ecx used as the agruments to that
>>
>> Typi: agruments
>>
>>> +# instruction. ecx is an optional argument as not all of the leaves support it.
>>
>> Please wrap doc comment lines around column 70.
>>
>>> +#
>>> +# @in-eax: CPUID argument in eax
>>> +# @in-ecx: CPUID argument in ecx
>>> +# @eax: eax
>>> +# @ebx: ebx
>>> +# @ecx: ecx
>>> +# @edx: edx
>>
>> Suggest
>>
>>    # @eax: CPUID result in eax
>>
>> and so forth.
>>
>>> +#
>>> +# Since: 6.1
>>> +##
>>> +{ 'struct': 'CpuidEntry',
>>> +  'data': { 'in-eax' : 'uint32',
>>> +            '*in-ecx' : 'uint32',
>>> +            'eax' : 'uint32',
>>> +            'ebx' : 'uint32',
>>> +            'ecx' : 'uint32',
>>> +            'edx' : 'uint32'
>>> +          },
>>> +  'if': 'defined(TARGET_I386) && defined(CONFIG_KVM)' }
>>> +
>>> +##
>>> +# @query-kvm-cpuid:
>>> +#
>>> +# Returns raw data from the KVM CPUID table for the first VCPU.
>>> +# The KVM CPUID table defines the response to the CPUID
>>> +# instruction when executed by the guest operating system.
>>> +#
>>> +# Returns: a list of CpuidEntry
>>> +#
>>> +# Since: 6.1
>>> +##
>>> +{ 'command': 'query-kvm-cpuid',
>>> +  'returns': ['CpuidEntry'],
>>> +  'if': 'defined(TARGET_I386) && defined(CONFIG_KVM)' }
>>
>> Is this intended to be a stable interface?  Interfaces intended just for
>> debugging usually aren't.
>>
> It is intented to be used as a stable interface.
>> [...]
>>
>