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

[Claudio Fontana]

On 6/17/21 5:39 PM, Valeriy Vdovin wrote:
> On Thu, Jun 17, 2021 at 04:14:17PM +0200, Markus Armbruster wrote:
>> Claudio Fontana <cfontana@suse.de> writes:
>>
>>> 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?
>>
>> I can't see why it couldn't be.
> Should I also make some note about that in the commit message?
>>
>> Different tack: if KVM is compiled out entirely, the command isn't
>> there, and trying to use it fails like
>>
>>     {"error": {"class": "CommandNotFound", "desc": "The command 
>> query-kvm-cpuid has not been found"}}
>>
>> If KVM is compiled in, but disabled, e.g. with -machine accel=tcg, then
>> the command fails like
>>
>>     {"error": {"class": "GenericError", "desc": "VCPU was not initialized 
>> yet"}}
>>
>> This is misleading.  The VCPU is actually running, it's just the wrong
>> kind of VCPU.
>>
>>>> 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.
>>
>> Perhaps this one is closer to reality.
>>
> I agree.
> What command name do you suggest?

query-exposed-cpuid?


>>>> Pick one :)
>>>>
>>>> [...]
>>