Re: [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom

["Denis V. Lunev" <den@openvz.org>]

On 06/14/2016 11:54 PM, Eduardo Habkost wrote:
> On Tue, Jun 14, 2016 at 11:45:08PM +0300, Denis V. Lunev wrote:
>> On 06/14/2016 10:59 PM, Eduardo Habkost wrote:
>>> On Tue, Jun 14, 2016 at 01:28:40PM +0300, Denis V. Lunev wrote:
>>>> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>>>>
>>>> This change adds hyperv feature words report through qom rpc.
>>>>
>>>> When VM is configured with hyperv features enabled libvirt will check that
>>>> required featured words are set in cpuid leaf 40000003 through qom
>>>> request.
>>>>
>>>> Currently qemu does not report hyperv feature words which prevents windows
>>>> guests from starting with libvirt.
>>>>
>>>> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> CC: Paolo Bonzini <pbonzini@redhat.com>
>>>> CC: Richard Henderson <rth@twiddle.net>
>>>> CC: Eduardo Habkost <ehabkost@redhat.com>
>>>> CC: Marcelo Tosatti <mtosatti@redhat.com>
>>> Which QEMU version did you use to test this? Some of those properties already
>>> exist. See:
>>>
>>>     static Property x86_cpu_properties[] = {
>>>         [...]
>>>         { .name  = "hv-spinlocks", .info  = &qdev_prop_spinlocks },
>>>         DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
>>>         DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
>>>         DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
>>>         DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
>>>         DEFINE_PROP_BOOL("hv-reset", X86CPU, hyperv_reset, false),
>>>         DEFINE_PROP_BOOL("hv-vpindex", X86CPU, hyperv_vpindex, false),
>>>         DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
>>>         DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
>>>         DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
>>>         [...]
>>>         DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
>>>         DEFINE_PROP_END_OF_LIST()
>>>     };
>>>
>>> QEMU will crash if you try to register the properties twice:
>>>
>>>     $ ./x86_64-softmmu/qemu-system-x86_64
>>>     qemu-system-x86_64: /home/ehabkost/rh/proj/virt/qemu/target-i386/cpu.c:3094: x86_cpu_register_bit_prop: Assertion `fp->ptr == field' failed.
>>>     Aborted (core dumped)
>>>
>>> I like the idea of moving hyperv feature information inside the features array,
>>> though.
>> no, idea is a bit different.
>>
>> The user selects properties in the command line to enable
>> different HyperV enlightenments. This is how we do that
>> and this is how the QEMU is expected to work.
>>
>> After that libvirt starts to check that these properties do
>> work. In order to do that it executes qom-get and expects
>> to find enabled HyperV enlightenments in the guest CPUID.
>>
>> This is the idea of this patch.
> And why exactly moving hyperv feature information inside the
> CPUX86State::features array wouldn't do exactly what you say
> above?
>
property remains ;)

OK, may be I have missed you point. I fear word "move"
in your letter. I think we "add" it to features.

Anyway, I got your point that features comes first,
CPUID is filled in the base of features. No prob.

Den