On Fri, Mar 29, 2019 at 03:18:28PM +0100, Vitaly Kuznetsov wrote:
> In many case we just want to give Windows guests all currently supported
> Hyper-V enlightenments and that's where this new mode may come handy. We
> pass through what was returned by KVM_GET_SUPPORTED_HV_CPUID.
The only one out of those "many cases" I can think of is when you've
developed a new hyperv feature in the kernel and you want to test it
with a version of QEMU that's not aware of it. Are there any others?
>
> hv_cpuid_check_and_set() is modified to also set cpu->hyperv_* flags as
> we may want to check them later (and we actually do for hv_runtime,
> hv_synic,...).
>
> 'hv-all' is a development only feature, a migration blocker is added to
> prevent issues while migrating between hosts with different feature sets.
>
> Signed-off-by: Vitaly Kuznetsov <vkuzn...@redhat.com>
> ---
> docs/hyperv.txt | 10 ++++
> target/i386/cpu.c | 1 +
> target/i386/cpu.h | 1 +
> target/i386/kvm.c | 148 +++++++++++++++++++++++++++++++++++++---------
> 4 files changed, 132 insertions(+), 28 deletions(-)
>
> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
> index 397f2517b8..d1299aba81 100644
> --- a/docs/hyperv.txt
> +++ b/docs/hyperv.txt
> @@ -174,6 +174,16 @@ without the feature to find out if enabling it is
> beneficial.
> Requires: hv-vapic
>
>
> +4. Development features
> +========================
> +In some cases (e.g. during development) it may make sense to use QEMU in
> +'pass-through' mode and give Windows guests all enlightenments currently
> +supported by KVM. This pass-through mode is enabled by "hv-all" CPU flag.
> +Note: enabling this flag effectively prevents migration as supported features
> +may differ between target and destination.
I find 'hv-passthrough' a more adequate name for this.
> +Note: "hv-all" doesn't include 'hv-evmcs', it needs to be enabled explicitly.
This is extremely confusing, when some features are more equal than
others. I think it'd make more sense instead to support filtering out
some features, like in "hv-passthrough,hv-evmcs=off".
Thanks,
Roman.
> +
> +
> 4. Useful links
> ================
> Hyper-V Top Level Functional specification and other information:
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d6bb57d210..4e01ad076e 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5785,6 +5785,7 @@ static Property x86_cpu_properties[] = {
> DEFINE_PROP_BOOL("hv-tlbflush", X86CPU, hyperv_tlbflush, false),
> DEFINE_PROP_BOOL("hv-evmcs", X86CPU, hyperv_evmcs, false),
> DEFINE_PROP_BOOL("hv-ipi", X86CPU, hyperv_ipi, false),
> + DEFINE_PROP_BOOL("hv-all", X86CPU, hyperv_all, false),
> DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
> DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
> DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 83fb522554..9cd3a8bc2f 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1396,6 +1396,7 @@ struct X86CPU {
> bool hyperv_tlbflush;
> bool hyperv_evmcs;
> bool hyperv_ipi;
> + bool hyperv_all;
> bool check_cpuid;
> bool enforce_cpuid;
> bool expose_kvm;
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 63031358ae..af45241adb 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -656,7 +656,8 @@ static bool hyperv_enabled(X86CPU *cpu)
> cpu->hyperv_stimer ||
> cpu->hyperv_reenlightenment ||
> cpu->hyperv_tlbflush ||
> - cpu->hyperv_ipi);
> + cpu->hyperv_ipi ||
> + cpu->hyperv_all);
> }
>
> static int kvm_arch_set_tsc_khz(CPUState *cs)
> @@ -1004,14 +1005,15 @@ static int hv_cpuid_get_fw(struct kvm_cpuid2 *cpuid,
> int fw, uint32_t *r)
> }
>
> static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
> - const char *name, bool flag)
> + const char *name, bool *flag)
> {
> X86CPU *cpu = X86_CPU(cs);
> CPUX86State *env = &cpu->env;
> uint32_t r, fw, bits;;
> int i, j;
> + bool present;
>
> - if (!flag) {
> + if (!*flag && !cpu->hyperv_all) {
> return 0;
> }
>
> @@ -1020,6 +1022,7 @@ static int hv_cpuid_check_and_set(CPUState *cs, struct
> kvm_cpuid2 *cpuid,
> continue;
> }
>
> + present = true;
> for (j = 0; j < ARRAY_SIZE(kvm_hyperv_properties[i].flags); j++) {
> fw = kvm_hyperv_properties[i].flags[j].fw;
> bits = kvm_hyperv_properties[i].flags[j].bits;
> @@ -1029,17 +1032,26 @@ static int hv_cpuid_check_and_set(CPUState *cs,
> struct kvm_cpuid2 *cpuid,
> }
>
> if (hv_cpuid_get_fw(cpuid, fw, &r) || (r & bits) != bits) {
> - fprintf(stderr,
> - "Hyper-V %s (requested by '%s' cpu flag) "
> - "is not supported by kernel\n",
> - kvm_hyperv_properties[i].desc,
> - kvm_hyperv_properties[i].name);
> - return 1;
> + if (*flag) {
> + fprintf(stderr,
> + "Hyper-V %s (requested by '%s' cpu flag) "
> + "is not supported by kernel\n",
> + kvm_hyperv_properties[i].desc,
> + kvm_hyperv_properties[i].name);
> + return 1;
> + } else {
> + present = false;
> + break;
> + }
> }
>
> env->features[fw] |= bits;
> }
>
> + if (cpu->hyperv_all && present) {
> + *flag = true;
> + }
> +
> return 0;
> }
>
> @@ -1047,6 +1059,43 @@ static int hv_cpuid_check_and_set(CPUState *cs, struct
> kvm_cpuid2 *cpuid,
> return 1;
> }
>
> +static int hv_report_missing_dep(X86CPU *cpu, const char *name,
> + const char *dep_name)
> +{
> + int i, j, nprops = sizeof(kvm_hyperv_properties);
> +
> + for (i = 0; i < nprops; i++) {
> + if (!strcmp(kvm_hyperv_properties[i].name, name)) {
> + break;
> + }
> + }
> + for (j = 0; j < nprops; j++) {
> + if (!strcmp(kvm_hyperv_properties[j].name, dep_name)) {
> + break;
> + }
> + }
> +
> + /*
> + * Internal error: either feature or its dependency is not in
> + * kvm_hyperv_properties!
> + */
> + if (i == nprops || j == nprops) {
> + return 1;
> + }
> +
> + if (cpu->hyperv_all) {
> + fprintf(stderr, "Hyper-V %s (requested by 'hv-all' cpu flag) "
> + "requires %s (is not supported by kernel)\n",
> + kvm_hyperv_properties[i].desc,
> kvm_hyperv_properties[j].desc);
> + } else {
> + fprintf(stderr, "Hyper-V %s (requested by '%s' cpu flag) "
> + "requires %s ('%s')\n", kvm_hyperv_properties[i].desc,
> + name, kvm_hyperv_properties[j].desc, dep_name);
> + }
> +
> + return 1;
> +}
> +
> /*
> * Fill in Hyper-V CPUIDs. Returns the number of entries filled in cpuid_ent
> in
> * case of success, errno < 0 in case of failure and 0 when no Hyper-V
> @@ -1086,32 +1135,54 @@ static int hyperv_handle_properties(CPUState *cs,
> cpuid = get_supported_hv_cpuid_legacy(cs);
> }
>
> + if (cpu->hyperv_all) {
> + memcpy(cpuid_ent, &cpuid->entries[0],
> + cpuid->nent * sizeof(cpuid->entries[0]));
> +
> + c = cpuid_find_entry(cpuid, HV_CPUID_FEATURES, 0);
> + if (c) {
> + env->features[FEAT_HYPERV_EAX] = c->eax;
> + env->features[FEAT_HYPERV_EBX] = c->ebx;
> + env->features[FEAT_HYPERV_EDX] = c->eax;
> + }
> + c = cpuid_find_entry(cpuid, HV_CPUID_ENLIGHTMENT_INFO, 0);
> + if (c) {
> + env->features[FEAT_HV_RECOMM_EAX] = c->eax;
> +
> + /* hv-spinlocks may have been overriden */
> + if (cpu->hyperv_spinlock_attempts !=
> HYPERV_SPINLOCK_NEVER_RETRY) {
> + c->ebx = cpu->hyperv_spinlock_attempts;
> + }
> + }
> + c = cpuid_find_entry(cpuid, HV_CPUID_NESTED_FEATURES, 0);
> + if (c) {
> + env->features[FEAT_HV_NESTED_EAX] = c->eax;
> + }
> + }
> +
> /* Features */
> r |= hv_cpuid_check_and_set(cs, cpuid, "hv-relaxed",
> - cpu->hyperv_relaxed_timing);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-vapic", cpu->hyperv_vapic);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-time", cpu->hyperv_time);
> + &cpu->hyperv_relaxed_timing);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-vapic", &cpu->hyperv_vapic);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-time", &cpu->hyperv_time);
> r |= hv_cpuid_check_and_set(cs, cpuid, "hv-frequencies",
> - cpu->hyperv_frequencies);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-crash", cpu->hyperv_crash);
> + &cpu->hyperv_frequencies);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-crash", &cpu->hyperv_crash);
> r |= hv_cpuid_check_and_set(cs, cpuid, "hv-reenlightenment",
> - cpu->hyperv_reenlightenment);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-reset", cpu->hyperv_reset);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-vpindex",
> cpu->hyperv_vpindex);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-runtime",
> cpu->hyperv_runtime);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-synic", cpu->hyperv_synic);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-stimer", cpu->hyperv_stimer);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-tlbflush",
> cpu->hyperv_tlbflush);
> - r |= hv_cpuid_check_and_set(cs, cpuid, "hv-ipi", cpu->hyperv_ipi);
> + &cpu->hyperv_reenlightenment);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-reset", &cpu->hyperv_reset);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-vpindex",
> &cpu->hyperv_vpindex);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-runtime",
> &cpu->hyperv_runtime);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-synic", &cpu->hyperv_synic);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-stimer", &cpu->hyperv_stimer);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-tlbflush",
> + &cpu->hyperv_tlbflush);
> + r |= hv_cpuid_check_and_set(cs, cpuid, "hv-ipi", &cpu->hyperv_ipi);
>
> /* Dependencies */
> if (cpu->hyperv_synic && !cpu->hyperv_synic_kvm_only &&
> - !cpu->hyperv_vpindex) {
> - fprintf(stderr, "Hyper-V SynIC "
> - "(requested by 'hv-synic' cpu flag) "
> - "requires Hyper-V VP_INDEX ('hv-vpindex')\n");
> - r |= 1;
> - }
> + !cpu->hyperv_vpindex)
> + r |= hv_report_missing_dep(cpu, "hv-synic", "hv-vpindex");
>
> /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
> env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
> @@ -1121,6 +1192,12 @@ static int hyperv_handle_properties(CPUState *cs,
> goto free;
> }
>
> + if (cpu->hyperv_all) {
> + /* We already copied all feature words from KVM as is */
> + r = cpuid->nent;
> + goto free;
> + }
> +
> c = &cpuid_ent[cpuid_i++];
> c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
> if (!cpu->hyperv_vendor_id) {
> @@ -1192,11 +1269,26 @@ free:
> return r;
> }
>
> +static Error *hv_all_mig_blocker;
> +
> static int hyperv_init_vcpu(X86CPU *cpu)
> {
> CPUState *cs = CPU(cpu);
> + Error *local_err = NULL;
> int ret;
>
> + if (cpu->hyperv_all && hv_all_mig_blocker == NULL) {
> + error_setg(&hv_all_mig_blocker,
> + "'hv-all' CPU flag prevents migration, use explicit set
> of "
> + "hv-* flags instead");
> + ret = migrate_add_blocker(hv_all_mig_blocker, &local_err);
> + if (local_err) {
> + error_report_err(local_err);
> + error_free(hv_all_mig_blocker);
> + return ret;
> + }
> + }
> +
> if (cpu->hyperv_vpindex && !hv_vpindex_settable) {
> /*
> * the kernel doesn't support setting vp_index; assert that its value
