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

[Claudio Fontana]

On 6/17/21 1:09 PM, Markus Armbruster wrote:
> Valeriy Vdovin <valeriy.vdovin@virtuozzo.com> writes:
> 
>> 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.
>>
>>> 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
> 
> If the command only makes sense for KVM, then it's named correctly, but
> the commit message lacks a (brief!) explanation why it only makes for
> KVM.


Is it meaningful for HVF?


> 
> If it just isn't implemented for anything but KVM, then putting "kvm"
> into the command name is a bad idea.  Also, the commit message should
> briefly note the restriction to KVM.
> 
> Pick one :)
> 
> [...]
> 
>