On Mon, Jun 20, 2016 at 06:29:40PM +0300, Denis V. Lunev wrote:
From: Evgeny Yakovlev <address@hidden>
This change adds hyperv feature words report through qom rpc.
When VM is configured with hyperv features enabled
libvirt will check that required feature 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 <address@hidden>
Signed-off-by: Denis V. Lunev <address@hidden>
CC: Paolo Bonzini <address@hidden>
CC: Richard Henderson <address@hidden>
CC: Eduardo Habkost <address@hidden>
CC: Marcelo Tosatti <address@hidden>
---
Changes from v1:
- renamed hyperv features so they don't conflict with hyperv properties
- refactored kvm_arch_init_vcpu to process hyperv props into feature words
target-i386/cpu.c | 50 ++++++++++++++++++++++++
target-i386/cpu.h | 3 ++
target-i386/kvm.c | 114 +++++++++++++++++++++++++++++++-----------------------
3 files changed, 119 insertions(+), 48 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3665fec..c79b4e3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -245,6 +245,44 @@ static const char *kvm_feature_name[] = {
NULL, NULL, NULL, NULL,
};
+static const char *hyperv_priv_feature_name[] = {
+ "hv_msr_vp_runtime_access", "hv_msr_time_refcount_access",
+ "hv_msr_synic_access", "hv_msr_stimer_access",
+ "hv_msr_apic_access", "hv_msr_hypercall_access",
+ "hv_vpindex_access", "hv_msr_reset_access",
+ "hv_msr_stats_access", "hv_reftsc_access",
+ "hv_msr_idle_access", "hv_msr_frequency_access",
+ 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,
+};
Adding these feature bit names will let individual bits to be
configured from the command-line, not just reported using QOM.
I am not sure this is intentional, as now we have conflicting
ways to configure some hyperv features. For example: now
HV_X64_MSR_VP_RUNTIME_AVAILABLE can be set using "+hv-runtime" or
"+hv_msr_vp_runtime_access". And the difference between both
methods is not clear for users.
Also, as kvm_arch_get_supported_cpuid() won't return anything
about those feature flags, QEMU will get confused if you try to
use "+hv_msr_vp_runtime_access" in the command-line.
I believe this can be addressed by doing the work in three steps:
1) Add hyperv CPUID leaves to FeatureWord/feature_word_info
without any name arrays, make the changes you made below to
change env->features inside kvm_arch_init_vcpu().
* In other words: this patch, but without the feature_name
arrays.
* This wil make QEMU report all the hyperv feature bits in the
"feature-words" QOM property (read-only)
* This won't change any command-line interface.
* This shouldn't confuse QEMU due to
lack of kvm_arch_get_supported_cpuid() support, because
env->features is being set up after
x86_cpu_filter_features() was already called.
If all you want is to report low-level CPUID data through QMP,
step (1) is enough: it will already include the low-level hyperv
CPUID data in the "feature-words" property.
2) Replace the hyperv_* boolean fields with env->feature bits.
* See below how this could be done for each specific case.
* This requires making kvm_arch_get_supported_cpuid() report
them (after making the appropriate capability checks).
* This makes the check/enforce flags support hyperv
capabilities.
3) (optional) Add the remaining feature names (that are not
configurable yet) to the feature_names arrays.
* This won't let them be configured in the command-line by
now, if kvm_arch_get_supported_cpuid() doesn't report them
as supported.
* I am not sure we really want that. What would be the point
of adding feature names that we don't even support yet?
+
static const char *svm_feature_name[] = {
"npt", "lbrv", "svm_lock", "nrip_save",
"tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists",
[...]
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index ff92b1d..5a3f14d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -574,6 +574,66 @@ static int kvm_arch_set_tsc_khz(CPUState *cs)
return 0;
}
+static int hyperv_handle_properties(CPUState *cs)
+{
+ X86CPU *cpu = X86_CPU(cs);
+ CPUX86State *env = &cpu->env;
+
+ if (cpu->hyperv_relaxed_timing) {
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
+ }
HYPERV_CPUID_ENLIGHTMENT_INFO is not in FeatureWordInfo yet, so
this looks OK by now.
+ if (cpu->hyperv_vapic) {
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_APIC_ACCESS_AVAILABLE;
+ has_msr_hv_vapic = true;
+ }
Now we can control HV_X64_MSR_APIC_ACCESS_AVAILABLE using
"+hv-vapic" and "+hv_msr_apic_access", and the difference between
both is unclear.
I suggest the following:
1) Remove the "hv-vapic" static property from
x86_cpu_properties, and the hyperv_vapic field
2) Change "hv_msr_apic_access" to "hv-vapic"
in hyperv_priv_feature_name.
2) Replace code using cpu->hyperv_vapic with
(env->features[FEAT_HYPERV_EAX] & HV_X64_MSR_APIC_ACCESS_AVAILABLE)
3) Change the setup code to:
if (env->features[FEAT_HYPERV_EAX] & HV_X64_MSR_APIC_ACCESS_AVAILABLE) {
env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
has_msr_hv_vapic = true;
}
+ if (cpu->hyperv_time &&
+ kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) {
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
+ env->features[FEAT_HYPERV_EAX] |= 0x200;
+ has_msr_hv_tsc = true;
+ }
This is similar to hyperv_vapic, but with the
kvm_check_extension() check that needs to be moved to
kvm_arch_get_supported_cpuid().
I suggest:
1) Remove the "hv-time" static property from
x86_cpu_properties, and the hyperv_time field
2) Change "hv_msr_time_refcount_access" to "hv-time"
in hyperv_priv_feature_name.
2) Replace code using cpu->hyperv_time field with
(env->features[FEAT_HYPERV_EAX] & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
3) Change the setup code to:
if (env->features[FEAT_HYPERV_EAX] & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE) {
env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
/*TODO: replace magic number with macro */
env->features[FEAT_HYPERV_EAX] |= 0x200;
has_msr_hv_tsc = true;
}
4) Check KVM_CAP_HYPERV_TIME inside
kvm_arch_get_supported_cpuid()
This will add support for the check/enforce flags for hv-time
automatically.
+ if (cpu->hyperv_crash && has_msr_hv_crash) {
+ env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
+ }
This is similar to hv-time, if the has_msr_hv_crash check is
moved to kvm_arch_get_supported_cpuid().
+ if (cpu->hyperv_reset && has_msr_hv_reset) {
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_RESET_AVAILABLE;
+ }
This is similar to hv-time/hv-crash above.
+ if (cpu->hyperv_vpindex && has_msr_hv_vpindex) {
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_VP_INDEX_AVAILABLE;
+ }
This is similar to hv-time/hv-crash above.
+ if (cpu->hyperv_runtime && has_msr_hv_runtime) {
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_VP_RUNTIME_AVAILABLE;
+ }
Similar to hv-time/hv-crash.
+ if (cpu->hyperv_synic) {
+ int sint;
+
+ if (!has_msr_hv_synic ||
+ kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_SYNIC, 0)) {
+ fprintf(stderr, "Hyper-V SynIC is not supported by kernel\n");
+ return -ENOSYS;
+ }
+
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_SYNIC_AVAILABLE;
+ env->msr_hv_synic_version = HV_SYNIC_VERSION_1;
+ for (sint = 0; sint < ARRAY_SIZE(env->msr_hv_synic_sint); sint++) {
+ env->msr_hv_synic_sint[sint] = HV_SYNIC_SINT_MASKED;
+ }
+ }
This is a bit more complex, but the general idea is the same: add
"hv-synic" to the feature name array, replace cpu->hyperv_synic
with (env->features[FEAT_HYPERV_EAX] & HV_X64_MSR_SYNIC_AVAILABLE),
move the capability check to kvm_arch_get_supported_cpuid(), keep
the remaining setup code (for msr_hv_synic_*) here.
+ if (cpu->hyperv_stimer) {
+ if (!has_msr_hv_stimer) {
+ fprintf(stderr, "Hyper-V timers aren't supported by kernel\n");
+ return -ENOSYS;
+ }
+ env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_SYNTIMER_AVAILABLE;
+ }
Similar to hv-time/hv-crash.
Interestingly, the last two features are the only ones that don't
get silently disabled by the setup code if unsupported by KVM.
Does anybody know why?
+ if (MACHINE_GET_CLASS(current_machine)->hot_add_cpu != NULL) {
+ env->features[FEAT_HYPERV_EDX] |=
HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
+ }
This probably can be left as-is.
+ return 0;
+}
+
static Error *invtsc_mig_blocker;
#define KVM_MAX_CPUID_ENTRIES 100
@@ -633,56 +693,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
[...]
c = &cpuid_data.entries[cpuid_i++];
c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
if (cpu->hyperv_relaxed_timing) {
Do you plan to add HYPERV_CPUID_ENLIGHTMENT_INFO to
FeatureWord/feature_word_info later?