* [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom
@ 2016-06-14 10:28 Denis V. Lunev
2016-06-14 19:59 ` Eduardo Habkost
0 siblings, 1 reply; 6+ messages in thread
From: Denis V. Lunev @ 2016-06-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: den, Evgeny Yakovlev, Paolo Bonzini, Richard Henderson,
Eduardo Habkost, Marcelo Tosatti
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>
---
target-i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
target-i386/cpu.h | 3 +++
target-i386/kvm.c | 3 +++
3 files changed, 53 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 895a386..ea01f06 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -244,6 +244,41 @@ static const char *kvm_feature_name[] = {
NULL, NULL, NULL, NULL,
};
+static const char *hyperv_priv_feature_name[] = {
+ "hv_runtime", "hv_refcount", "hv_synic", "hv_stimer",
+ "hv_apic", "hv_hypercall", "hv_vpindex", "hv_reset",
+ "hv_stats", "hv_reftsc", "hv_idle", "hv_frequency",
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+};
+
+static const char *hyperv_ident_feature_name[] = {
+ "hv_create_partitions", "hv_access_partition_id",
+ "hv_access_memory_pool", "hv_adjust_message_buffers",
+ "hv_post_messages", "hv_signal_events",
+ "hv_create_port", "hv_connect_port",
+ "hv_access_stats", NULL, NULL, "hv_debugging",
+ "hv_cpu_power_management", "hv_configure_profiler", NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+};
+
+static const char *hyperv_misc_feature_name[] = {
+ "hv_mwait", "hv_guest_debugging", "hv_perf_monitor", "hv_cpu_dynamic_part",
+ "hv_hypercall_params_xmm", "hv_guest_idle_state", NULL, NULL,
+ NULL, NULL, "hv_guest_crash_msr", NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+};
+
static const char *svm_feature_name[] = {
"npt", "lbrv", "svm_lock", "nrip_save",
"tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists",
@@ -410,6 +445,18 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
.tcg_features = TCG_KVM_FEATURES,
},
+ [FEAT_HYPERV_EAX] = {
+ .feat_names = hyperv_priv_feature_name,
+ .cpuid_eax = 0x40000003, .cpuid_reg = R_EAX,
+ },
+ [FEAT_HYPERV_EBX] = {
+ .feat_names = hyperv_ident_feature_name,
+ .cpuid_eax = 0x40000003, .cpuid_reg = R_EBX,
+ },
+ [FEAT_HYPERV_EDX] = {
+ .feat_names = hyperv_misc_feature_name,
+ .cpuid_eax = 0x40000003, .cpuid_reg = R_EDX,
+ },
[FEAT_SVM] = {
.feat_names = svm_feature_name,
.cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX,
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 0426459..d3b7ad4 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -440,6 +440,9 @@ typedef enum FeatureWord {
FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */
FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
+ FEAT_HYPERV_EAX, /* CPUID[4000_0003].EAX */
+ FEAT_HYPERV_EBX, /* CPUID[4000_0003].EBX */
+ FEAT_HYPERV_EDX, /* CPUID[4000_0003].EDX */
FEAT_SVM, /* CPUID[8000_000A].EDX */
FEAT_XSAVE, /* CPUID[EAX=0xd,ECX=1].EAX */
FEAT_6_EAX, /* CPUID[6].EAX */
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index abf50e6..f4ca8c4 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -679,6 +679,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
c->eax |= HV_X64_MSR_SYNTIMER_AVAILABLE;
}
+ env->features[FEAT_HYPERV_EAX] = c->eax;
+ env->features[FEAT_HYPERV_EBX] = c->ebx;
+ env->features[FEAT_HYPERV_EDX] = c->edx;
c = &cpuid_data.entries[cpuid_i++];
c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
if (cpu->hyperv_relaxed_timing) {
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom
2016-06-14 10:28 [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom Denis V. Lunev
@ 2016-06-14 19:59 ` Eduardo Habkost
2016-06-14 20:45 ` Denis V. Lunev
0 siblings, 1 reply; 6+ messages in thread
From: Eduardo Habkost @ 2016-06-14 19:59 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, Evgeny Yakovlev, Marcelo Tosatti, Paolo Bonzini,
Richard Henderson
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.
See additional comments below:
> ---
> target-i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> target-i386/cpu.h | 3 +++
> target-i386/kvm.c | 3 +++
> 3 files changed, 53 insertions(+)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 895a386..ea01f06 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -244,6 +244,41 @@ static const char *kvm_feature_name[] = {
> NULL, NULL, NULL, NULL,
> };
>
> +static const char *hyperv_priv_feature_name[] = {
> + "hv_runtime", "hv_refcount", "hv_synic", "hv_stimer",
> + "hv_apic", "hv_hypercall", "hv_vpindex", "hv_reset",
> + "hv_stats", "hv_reftsc", "hv_idle", "hv_frequency",
I sent a patch some time ago to change all feature names to use "-" instead of
"_". But this can be fixed when applying the patch, if necessary.
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> +};
> +
> +static const char *hyperv_ident_feature_name[] = {
> + "hv_create_partitions", "hv_access_partition_id",
> + "hv_access_memory_pool", "hv_adjust_message_buffers",
> + "hv_post_messages", "hv_signal_events",
> + "hv_create_port", "hv_connect_port",
> + "hv_access_stats", NULL, NULL, "hv_debugging",
> + "hv_cpu_power_management", "hv_configure_profiler", NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> +};
> +
> +static const char *hyperv_misc_feature_name[] = {
> + "hv_mwait", "hv_guest_debugging", "hv_perf_monitor", "hv_cpu_dynamic_part",
> + "hv_hypercall_params_xmm", "hv_guest_idle_state", NULL, NULL,
> + NULL, NULL, "hv_guest_crash_msr", NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> +};
> +
> static const char *svm_feature_name[] = {
> "npt", "lbrv", "svm_lock", "nrip_save",
> "tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists",
> @@ -410,6 +445,18 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
> .tcg_features = TCG_KVM_FEATURES,
> },
> + [FEAT_HYPERV_EAX] = {
> + .feat_names = hyperv_priv_feature_name,
> + .cpuid_eax = 0x40000003, .cpuid_reg = R_EAX,
> + },
> + [FEAT_HYPERV_EBX] = {
> + .feat_names = hyperv_ident_feature_name,
> + .cpuid_eax = 0x40000003, .cpuid_reg = R_EBX,
> + },
> + [FEAT_HYPERV_EDX] = {
> + .feat_names = hyperv_misc_feature_name,
> + .cpuid_eax = 0x40000003, .cpuid_reg = R_EDX,
> + },
> [FEAT_SVM] = {
> .feat_names = svm_feature_name,
> .cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX,
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 0426459..d3b7ad4 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -440,6 +440,9 @@ typedef enum FeatureWord {
> FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */
> FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
> FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
> + FEAT_HYPERV_EAX, /* CPUID[4000_0003].EAX */
> + FEAT_HYPERV_EBX, /* CPUID[4000_0003].EBX */
> + FEAT_HYPERV_EDX, /* CPUID[4000_0003].EDX */
> FEAT_SVM, /* CPUID[8000_000A].EDX */
> FEAT_XSAVE, /* CPUID[EAX=0xd,ECX=1].EAX */
> FEAT_6_EAX, /* CPUID[6].EAX */
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index abf50e6..f4ca8c4 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -679,6 +679,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
> c->eax |= HV_X64_MSR_SYNTIMER_AVAILABLE;
> }
> + env->features[FEAT_HYPERV_EAX] = c->eax;
> + env->features[FEAT_HYPERV_EBX] = c->ebx;
> + env->features[FEAT_HYPERV_EDX] = c->edx;
We normally store the configured features inside env->features
(using the properties), and then CPUID data is filled using
env->features. Not the other way around.
In other words, the new env->features[FEAT_HYPERV_*] bits can simply replace
the existing X86CPU::hyperv_* booleans.
--
Eduardo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom
2016-06-14 19:59 ` Eduardo Habkost
@ 2016-06-14 20:45 ` Denis V. Lunev
2016-06-14 20:54 ` Eduardo Habkost
0 siblings, 1 reply; 6+ messages in thread
From: Denis V. Lunev @ 2016-06-14 20:45 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, Evgeny Yakovlev, Marcelo Tosatti, Paolo Bonzini,
Richard Henderson
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom
2016-06-14 20:45 ` Denis V. Lunev
@ 2016-06-14 20:54 ` Eduardo Habkost
2016-06-14 20:59 ` Denis V. Lunev
0 siblings, 1 reply; 6+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:54 UTC (permalink / raw)
To: Denis V. Lunev
Cc: Paolo Bonzini, Marcelo Tosatti, Evgeny Yakovlev, qemu-devel,
Richard Henderson
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?
--
Eduardo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom
2016-06-14 20:54 ` Eduardo Habkost
@ 2016-06-14 20:59 ` Denis V. Lunev
2016-06-14 21:16 ` Eduardo Habkost
0 siblings, 1 reply; 6+ messages in thread
From: Denis V. Lunev @ 2016-06-14 20:59 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Marcelo Tosatti, Evgeny Yakovlev, qemu-devel,
Richard Henderson
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom
2016-06-14 20:59 ` Denis V. Lunev
@ 2016-06-14 21:16 ` Eduardo Habkost
0 siblings, 0 replies; 6+ messages in thread
From: Eduardo Habkost @ 2016-06-14 21:16 UTC (permalink / raw)
To: Denis V. Lunev
Cc: Paolo Bonzini, Marcelo Tosatti, Evgeny Yakovlev, qemu-devel,
Richard Henderson
On Tue, Jun 14, 2016 at 11:59:18PM +0300, Denis V. Lunev wrote:
> 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.
I mean we move data to a different struct field (from the
hyperv_* booleans to the features array), and let the existing
feature property system handle it.
Your patch does that partially, but it conflicts with the
existing properties registered at x86_cpu_properties.
But we also need to make kvm_arch_get_supported_cpuid() handle
the hyperv CPUID leaves (to replace the existing
kvm_check_extensions() and has_msr_* checks in
kvm_arch_init_vcpu()).
After we do that, some things should work automatically:
* New hv-* properties available in QOM and "-cpu";
* hyperv support for the "check" and "enforce" options;
* hyperv support in "-cpu host'; and
* hyperv information returned in the "feature-words" and
"filtered-features" QOM properties.
See how FEAT_KVM is handled in target-i386/cpu.c and
kvm_arch_init_vcpu(), for reference.
--
Eduardo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-14 21:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-14 10:28 [Qemu-devel] [PATCH 1/1] cpu: report hyperv feature words through qom Denis V. Lunev
2016-06-14 19:59 ` Eduardo Habkost
2016-06-14 20:45 ` Denis V. Lunev
2016-06-14 20:54 ` Eduardo Habkost
2016-06-14 20:59 ` Denis V. Lunev
2016-06-14 21:16 ` Eduardo Habkost
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).