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

[Valeriy Vdovin]

On Thu, Jun 17, 2021 at 01:58:09PM +0200, Claudio Fontana wrote:
> 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?

CPUID is an x86 instruction that KVM virtualizes and which responses we would 
like
to retrieve(the usecase) here via this new command.

In ARM there is no single CPUID instruction as such, so to get equivalent 
information
one would read a list of system registers with say an MRS instruction.

At first I was thinking that it would be easy to fit this into some
sophisticated abstraction of a key-value type with some conditionals,
but then I understood some things:
 - the data types are conceptually different.
 - By the comments in the first versions I've figured out complex code for 
nothing is
   not welcome here, and this generalization will bring additional complexity 
and
   confusion 100%.

I said it's impossible to generalize them, well of course actully it is 
possible to have
one command and one data structure that can represent both, but it would get 
uglier with
each new architecture added and each new architecture will bring the risk of 
expanding
the existing data structure potentially breaking old tools.
So what for?

> 
> 
> > 
> >> 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.
> >> [...]
> >>
> > 
>