* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.