diff mbox series

[RFC,v2,12/19] i386: drop FEAT_HYPERV feature leaves

Message ID 20200930134027.1348021-13-vkuznets@redhat.com (mailing list archive)
State New, archived
Headers show
Series i386: KVM: expand Hyper-V features early | expand

Commit Message

Vitaly Kuznetsov Sept. 30, 2020, 1:40 p.m. UTC
Hyper-V feature leaves are weird. We have some of them in
feature_word_info[] array but we don't use feature_word_info
magic to enable them. Neither do we use feature_dependencies[]
mechanism to validate the configuration as it doesn't allign
well with Hyper-V's many-to-many dependency chains. Some of
the feature leaves hold not only feature bits, but also values.
E.g. FEAT_HV_NESTED_EAX contains both features and the supported
Enlightened VMCS range.

Hyper-V features are already represented in 'struct X86CPU' with
uint64_t hyperv_features so duplicating them in env->features adds
little (or zero) benefits. THe other half of Hyper-V emulation features
is also stored with values in hyperv_vendor_id[], hyperv_limits[],...
so env->features[] is already incomplete.

Remove Hyper-V feature leaves from env->features[] completely.
kvm_hyperv_properties[] is converted to using raw CPUID func/reg
pairs for features, this allows us to get rid of hv_cpuid_get_fw()
conversion.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/cpu.c |  90 +----------------------
 target/i386/cpu.h |   6 +-
 target/i386/kvm.c | 181 ++++++++++++++++++++++------------------------
 3 files changed, 90 insertions(+), 187 deletions(-)

Comments

Eduardo Habkost Sept. 30, 2020, 2:36 p.m. UTC | #1
On Wed, Sep 30, 2020 at 03:40:20PM +0200, Vitaly Kuznetsov wrote:
> Hyper-V feature leaves are weird. We have some of them in
> feature_word_info[] array but we don't use feature_word_info
> magic to enable them. Neither do we use feature_dependencies[]
> mechanism to validate the configuration as it doesn't allign
> well with Hyper-V's many-to-many dependency chains. Some of
> the feature leaves hold not only feature bits, but also values.
> E.g. FEAT_HV_NESTED_EAX contains both features and the supported
> Enlightened VMCS range.
> 
> Hyper-V features are already represented in 'struct X86CPU' with
> uint64_t hyperv_features so duplicating them in env->features adds
> little (or zero) benefits. THe other half of Hyper-V emulation features
> is also stored with values in hyperv_vendor_id[], hyperv_limits[],...
> so env->features[] is already incomplete.
> 
> Remove Hyper-V feature leaves from env->features[] completely.
> kvm_hyperv_properties[] is converted to using raw CPUID func/reg
> pairs for features, this allows us to get rid of hv_cpuid_get_fw()
> conversion.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

So, my first reaction to this was "we don't want to revert this,
having hyperv features in env->features is useful", but seeing
that there's only one line where env->features[] is actually used
in target/i386/cpu.c, I agree with this change.  We still have
the option to recreate those FeatureWord entries in the future,
if necessary.

I'm still reviewing the patch and it looks good so far, but I
have one question: have you considered doing this at the
beginning of the series?  There are
  env->features[...] = hv_cpuid_get_host(cs, ...)
lines you have added in the previous patches, just to remove them
in this patch.

> ---
>  target/i386/cpu.c |  90 +----------------------
>  target/i386/cpu.h |   6 +-
>  target/i386/kvm.c | 181 ++++++++++++++++++++++------------------------
>  3 files changed, 90 insertions(+), 187 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8ec0af0a6d48..a178db255641 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -828,94 +828,6 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>           */
>          .no_autoenable_flags = ~0U,
>      },
> -    /*
> -     * .feat_names are commented out for Hyper-V enlightenments because we
> -     * don't want to have two different ways for enabling them on QEMU command
> -     * line. Some features (e.g. "hyperv_time", "hyperv_vapic", ...) require
> -     * enabling several feature bits simultaneously, exposing these bits
> -     * individually may just confuse guests.
> -     */
> -    [FEAT_HYPERV_EAX] = {
> -        .type = CPUID_FEATURE_WORD,
> -        .feat_names = {
> -            NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
> -            NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
> -            NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */,
> -            NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
> -            NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
> -            NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */,
> -            NULL /* hv_msr_debug_access */, NULL /* hv_msr_reenlightenment_access */,
> -            NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -        },
> -        .cpuid = { .eax = 0x40000003, .reg = R_EAX, },
> -    },
> -    [FEAT_HYPERV_EBX] = {
> -        .type = CPUID_FEATURE_WORD,
> -        .feat_names = {
> -            NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
> -            NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
> -            NULL /* hv_post_messages */, NULL /* hv_signal_events */,
> -            NULL /* hv_create_port */, NULL /* hv_connect_port */,
> -            NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */,
> -            NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */,
> -            NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -        },
> -        .cpuid = { .eax = 0x40000003, .reg = R_EBX, },
> -    },
> -    [FEAT_HYPERV_EDX] = {
> -        .type = CPUID_FEATURE_WORD,
> -        .feat_names = {
> -            NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
> -            NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
> -            NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */,
> -            NULL, 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,
> -        },
> -        .cpuid = { .eax = 0x40000003, .reg = R_EDX, },
> -    },
> -    [FEAT_HV_RECOMM_EAX] = {
> -        .type = CPUID_FEATURE_WORD,
> -        .feat_names = {
> -            NULL /* hv_recommend_pv_as_switch */,
> -            NULL /* hv_recommend_pv_tlbflush_local */,
> -            NULL /* hv_recommend_pv_tlbflush_remote */,
> -            NULL /* hv_recommend_msr_apic_access */,
> -            NULL /* hv_recommend_msr_reset */,
> -            NULL /* hv_recommend_relaxed_timing */,
> -            NULL /* hv_recommend_dma_remapping */,
> -            NULL /* hv_recommend_int_remapping */,
> -            NULL /* hv_recommend_x2apic_msrs */,
> -            NULL /* hv_recommend_autoeoi_deprecation */,
> -            NULL /* hv_recommend_pv_ipi */,
> -            NULL /* hv_recommend_ex_hypercalls */,
> -            NULL /* hv_hypervisor_is_nested */,
> -            NULL /* hv_recommend_int_mbec */,
> -            NULL /* hv_recommend_evmcs */,
> -            NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -            NULL, NULL, NULL, NULL,
> -        },
> -        .cpuid = { .eax = 0x40000004, .reg = R_EAX, },
> -    },
> -    [FEAT_HV_NESTED_EAX] = {
> -        .type = CPUID_FEATURE_WORD,
> -        .cpuid = { .eax = 0x4000000A, .reg = R_EAX, },
> -    },
>      [FEAT_SVM] = {
>          .type = CPUID_FEATURE_WORD,
>          .feat_names = {
> @@ -6906,7 +6818,7 @@ static GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs)
>      CPUX86State *env = &cpu->env;
>      GuestPanicInformation *panic_info = NULL;
>  
> -    if (env->features[FEAT_HYPERV_EDX] & HV_GUEST_CRASH_MSR_AVAILABLE) {
> +    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_CRASH)) {
>          panic_info = g_malloc0(sizeof(GuestPanicInformation));
>  
>          panic_info->type = GUEST_PANIC_INFORMATION_TYPE_HYPER_V;
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 187e4a09e65d..b07325745bcb 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -519,11 +519,6 @@ typedef enum FeatureWord {
>      FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
>      FEAT_KVM,           /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
>      FEAT_KVM_HINTS,     /* CPUID[4000_0001].EDX */
> -    FEAT_HYPERV_EAX,    /* CPUID[4000_0003].EAX */
> -    FEAT_HYPERV_EBX,    /* CPUID[4000_0003].EBX */
> -    FEAT_HYPERV_EDX,    /* CPUID[4000_0003].EDX */
> -    FEAT_HV_RECOMM_EAX, /* CPUID[4000_0004].EAX */
> -    FEAT_HV_NESTED_EAX, /* CPUID[4000_000A].EAX */
>      FEAT_SVM,           /* CPUID[8000_000A].EDX */
>      FEAT_XSAVE,         /* CPUID[EAX=0xd,ECX=1].EAX */
>      FEAT_6_EAX,         /* CPUID[6].EAX */
> @@ -1663,6 +1658,7 @@ struct X86CPU {
>      uint32_t hyperv_interface_id[4];
>      uint32_t hyperv_version_id[4];
>      uint32_t hyperv_limits[3];
> +    uint32_t hyperv_nested[4];
>  
>      bool check_cpuid;
>      bool enforce_cpuid;
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 0e6eef6a52c2..86a5cfa1e12b 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -815,7 +815,8 @@ static bool tsc_is_stable_and_known(CPUX86State *env)
>  static struct {
>      const char *desc;
>      struct {
> -        uint32_t fw;
> +        uint32_t func;
> +        int reg;
>          uint32_t bits;
>      } flags[2];
>      uint64_t dependencies;
> @@ -823,25 +824,25 @@ static struct {
>      [HYPERV_FEAT_RELAXED] = {
>          .desc = "relaxed timing (hv-relaxed)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_HYPERCALL_AVAILABLE},
> -            {.fw = FEAT_HV_RECOMM_EAX,
> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>               .bits = HV_RELAXED_TIMING_RECOMMENDED}
>          }
>      },
>      [HYPERV_FEAT_VAPIC] = {
>          .desc = "virtual APIC (hv-vapic)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_HYPERCALL_AVAILABLE | HV_APIC_ACCESS_AVAILABLE},
> -            {.fw = FEAT_HV_RECOMM_EAX,
> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>               .bits = HV_APIC_ACCESS_RECOMMENDED}
>          }
>      },
>      [HYPERV_FEAT_TIME] = {
>          .desc = "clocksources (hv-time)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_HYPERCALL_AVAILABLE | HV_TIME_REF_COUNT_AVAILABLE |
>               HV_REFERENCE_TSC_AVAILABLE}
>          }
> @@ -849,42 +850,42 @@ static struct {
>      [HYPERV_FEAT_CRASH] = {
>          .desc = "crash MSRs (hv-crash)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EDX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
>               .bits = HV_GUEST_CRASH_MSR_AVAILABLE}
>          }
>      },
>      [HYPERV_FEAT_RESET] = {
>          .desc = "reset MSR (hv-reset)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_RESET_AVAILABLE}
>          }
>      },
>      [HYPERV_FEAT_VPINDEX] = {
>          .desc = "VP_INDEX MSR (hv-vpindex)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_VP_INDEX_AVAILABLE}
>          }
>      },
>      [HYPERV_FEAT_RUNTIME] = {
>          .desc = "VP_RUNTIME MSR (hv-runtime)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_VP_RUNTIME_AVAILABLE}
>          }
>      },
>      [HYPERV_FEAT_SYNIC] = {
>          .desc = "synthetic interrupt controller (hv-synic)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_SYNIC_AVAILABLE}
>          }
>      },
>      [HYPERV_FEAT_STIMER] = {
>          .desc = "synthetic timers (hv-stimer)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_SYNTIMERS_AVAILABLE}
>          },
>          .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
> @@ -892,23 +893,23 @@ static struct {
>      [HYPERV_FEAT_FREQUENCIES] = {
>          .desc = "frequency MSRs (hv-frequencies)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_ACCESS_FREQUENCY_MSRS},
> -            {.fw = FEAT_HYPERV_EDX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
>               .bits = HV_FREQUENCY_MSRS_AVAILABLE}
>          }
>      },
>      [HYPERV_FEAT_REENLIGHTENMENT] = {
>          .desc = "reenlightenment MSRs (hv-reenlightenment)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EAX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>               .bits = HV_ACCESS_REENLIGHTENMENTS_CONTROL}
>          }
>      },
>      [HYPERV_FEAT_TLBFLUSH] = {
>          .desc = "paravirtualized TLB flush (hv-tlbflush)",
>          .flags = {
> -            {.fw = FEAT_HV_RECOMM_EAX,
> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>               .bits = HV_REMOTE_TLB_FLUSH_RECOMMENDED |
>               HV_EX_PROCESSOR_MASKS_RECOMMENDED}
>          },
> @@ -917,7 +918,7 @@ static struct {
>      [HYPERV_FEAT_EVMCS] = {
>          .desc = "enlightened VMCS (hv-evmcs)",
>          .flags = {
> -            {.fw = FEAT_HV_RECOMM_EAX,
> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>               .bits = HV_ENLIGHTENED_VMCS_RECOMMENDED}
>          },
>          .dependencies = BIT(HYPERV_FEAT_VAPIC)
> @@ -925,7 +926,7 @@ static struct {
>      [HYPERV_FEAT_IPI] = {
>          .desc = "paravirtualized IPI (hv-ipi)",
>          .flags = {
> -            {.fw = FEAT_HV_RECOMM_EAX,
> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>               .bits = HV_CLUSTER_IPI_RECOMMENDED |
>               HV_EX_PROCESSOR_MASKS_RECOMMENDED}
>          },
> @@ -934,7 +935,7 @@ static struct {
>      [HYPERV_FEAT_STIMER_DIRECT] = {
>          .desc = "direct mode synthetic timers (hv-stimer-direct)",
>          .flags = {
> -            {.fw = FEAT_HYPERV_EDX,
> +            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
>               .bits = HV_STIMER_DIRECT_MODE_AVAILABLE}
>          },
>          .dependencies = BIT(HYPERV_FEAT_STIMER)
> @@ -1108,38 +1109,34 @@ static uint32_t hv_cpuid_get_host(CPUState *cs, uint32_t func, int reg)
>      return cpuid_entry_get_reg(entry, reg);
>  }
>  
> -static uint32_t hv_cpuid_get_fw(CPUState *cs, int fw)
> +static bool hyperv_feature_supported(CPUState *cs, int feature)
>  {
> -    uint32_t func;
> -    int reg;
> +    uint32_t func, bits;
> +    int i, reg;
>  
> -    switch (fw) {
> -    case FEAT_HYPERV_EAX:
> -        reg = R_EAX;
> -        func = HV_CPUID_FEATURES;
> -        break;
> -    case FEAT_HYPERV_EDX:
> -        reg = R_EDX;
> -        func = HV_CPUID_FEATURES;
> -        break;
> -    case FEAT_HV_RECOMM_EAX:
> -        reg = R_EAX;
> -        func = HV_CPUID_ENLIGHTMENT_INFO;
> -        break;
> -    default:
> -        return 0;
> +    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
> +
> +        func = kvm_hyperv_properties[feature].flags[i].func;
> +        reg = kvm_hyperv_properties[feature].flags[i].reg;
> +        bits = kvm_hyperv_properties[feature].flags[i].bits;
> +
> +        if (!func) {
> +            continue;
> +        }
> +
> +        if ((hv_cpuid_get_host(cs, func, reg) & bits) != bits) {
> +            return false;
> +        }
>      }
>  
> -    return hv_cpuid_get_host(cs, func, reg);
> +    return true;
>  }
>  
>  static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>  {
>      X86CPU *cpu = X86_CPU(cs);
> -    CPUX86State *env = &cpu->env;
> -    uint32_t r, fw, bits;
>      uint64_t deps;
> -    int i, dep_feat;
> +    int dep_feat;
>  
>      if (!hyperv_feat_enabled(cpu, feature) && !cpu->hyperv_passthrough) {
>          return 0;
> @@ -1158,27 +1155,15 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>          deps &= ~(1ull << dep_feat);
>      }
>  
> -    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
> -        fw = kvm_hyperv_properties[feature].flags[i].fw;
> -        bits = kvm_hyperv_properties[feature].flags[i].bits;
> -
> -        if (!fw) {
> -            continue;
> -        }
> -
> -        r = hv_cpuid_get_fw(cs, fw);
> -        if ((r & bits) != bits) {
> -            if (hyperv_feat_enabled(cpu, feature)) {
> -                fprintf(stderr,
> -                        "Hyper-V %s is not supported by kernel\n",
> -                        kvm_hyperv_properties[feature].desc);
> -                return 1;
> -            } else {
> -                return 0;
> -            }
> +    if (!hyperv_feature_supported(cs, feature)) {
> +        if (hyperv_feat_enabled(cpu, feature)) {
> +            fprintf(stderr,
> +                    "Hyper-V %s is not supported by kernel\n",
> +                    kvm_hyperv_properties[feature].desc);
> +            return 1;
> +        } else {
> +            return 0;
>          }
> -
> -        env->features[fw] |= bits;
>      }
>  
>      if (cpu->hyperv_passthrough) {
> @@ -1188,6 +1173,32 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>      return 0;
>  }
>  
> +static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    uint32_t r = 0;
> +    int i, j;
> +
> +    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties); i++) {
> +        if (!hyperv_feat_enabled(cpu, i)) {
> +            continue;
> +        }
> +
> +        for (j = 0; j < ARRAY_SIZE(kvm_hyperv_properties[i].flags); j++) {
> +            if (kvm_hyperv_properties[i].flags[j].func != func) {
> +                continue;
> +            }
> +            if (kvm_hyperv_properties[i].flags[j].reg != reg) {
> +                continue;
> +            }
> +
> +            r |= kvm_hyperv_properties[i].flags[j].bits;
> +        }
> +    }
> +
> +    return r;
> +}
> +
>  /*
>   * 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
> @@ -1197,7 +1208,6 @@ static int hyperv_handle_properties(CPUState *cs,
>                                      struct kvm_cpuid_entry2 *cpuid_ent)
>  {
>      X86CPU *cpu = X86_CPU(cs);
> -    CPUX86State *env = &cpu->env;
>      struct kvm_cpuid_entry2 *c;
>      uint32_t cpuid_i = 0;
>      int r;
> @@ -1219,9 +1229,7 @@ static int hyperv_handle_properties(CPUState *cs,
>          }
>  
>          if (!r) {
> -            env->features[FEAT_HV_RECOMM_EAX] |=
> -                HV_ENLIGHTENED_VMCS_RECOMMENDED;
> -            env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
> +            cpu->hyperv_nested[0] = evmcs_version;
>          }
>      }
>  
> @@ -1255,13 +1263,6 @@ static int hyperv_handle_properties(CPUState *cs,
>          cpu->hyperv_version_id[3] =
>              hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EDX);
>  
> -        env->features[FEAT_HYPERV_EAX] =
> -            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EAX);
> -        env->features[FEAT_HYPERV_EBX] =
> -            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EBX);
> -        env->features[FEAT_HYPERV_EDX] =
> -            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EDX);
> -
>          cpu->hv_max_vps = hv_cpuid_get_host(cs, HV_CPUID_IMPLEMENT_LIMITS,
>                                              R_EAX);
>          cpu->hyperv_limits[0] =
> @@ -1271,21 +1272,8 @@ static int hyperv_handle_properties(CPUState *cs,
>          cpu->hyperv_limits[2] =
>              hv_cpuid_get_host(cs, HV_CPUID_IMPLEMENT_LIMITS, R_EDX);
>  
> -        env->features[FEAT_HV_RECOMM_EAX] =
> -            hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
>          cpu->hyperv_spinlock_attempts =
>              hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EBX);
> -
> -        env->features[FEAT_HV_NESTED_EAX] =
> -            hv_cpuid_get_host(cs, HV_CPUID_NESTED_FEATURES, R_EAX);
> -    }
> -
> -    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
> -    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
> -        env->features[FEAT_HV_RECOMM_EAX] |=
> -            hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX) &
> -            HV_NO_NONARCH_CORESHARING;
>      }
>  
>      /* Features */
> @@ -1315,9 +1303,6 @@ static int hyperv_handle_properties(CPUState *cs,
>          r |= 1;
>      }
>  
> -    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
> -    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
> -
>      if (r) {
>          return -ENOSYS;
>      }
> @@ -1346,15 +1331,25 @@ static int hyperv_handle_properties(CPUState *cs,
>  
>      c = &cpuid_ent[cpuid_i++];
>      c->function = HV_CPUID_FEATURES;
> -    c->eax = env->features[FEAT_HYPERV_EAX];
> -    c->ebx = env->features[FEAT_HYPERV_EBX];
> -    c->edx = env->features[FEAT_HYPERV_EDX];
> +    c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EAX);
> +    c->ebx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EBX);
> +    c->edx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EDX);
> +
> +    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
> +    c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
>  
>      c = &cpuid_ent[cpuid_i++];
>      c->function = HV_CPUID_ENLIGHTMENT_INFO;
> -    c->eax = env->features[FEAT_HV_RECOMM_EAX];
> +    c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
>      c->ebx = cpu->hyperv_spinlock_attempts;
>  
> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
> +        c->eax |= HV_NO_NONARCH_CORESHARING;
> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
> +        c->eax |= hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX) &
> +            HV_NO_NONARCH_CORESHARING;
> +    }
> +
>      c = &cpuid_ent[cpuid_i++];
>      c->function = HV_CPUID_IMPLEMENT_LIMITS;
>      c->eax = cpu->hv_max_vps;
> @@ -1374,7 +1369,7 @@ static int hyperv_handle_properties(CPUState *cs,
>  
>          c = &cpuid_ent[cpuid_i++];
>          c->function = HV_CPUID_NESTED_FEATURES;
> -        c->eax = env->features[FEAT_HV_NESTED_EAX];
> +        c->eax = cpu->hyperv_nested[0];
>      }
>  
>      return cpuid_i;
> -- 
> 2.25.4
>
Vitaly Kuznetsov Sept. 30, 2020, 2:56 p.m. UTC | #2
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Wed, Sep 30, 2020 at 03:40:20PM +0200, Vitaly Kuznetsov wrote:
>> Hyper-V feature leaves are weird. We have some of them in
>> feature_word_info[] array but we don't use feature_word_info
>> magic to enable them. Neither do we use feature_dependencies[]
>> mechanism to validate the configuration as it doesn't allign
>> well with Hyper-V's many-to-many dependency chains. Some of
>> the feature leaves hold not only feature bits, but also values.
>> E.g. FEAT_HV_NESTED_EAX contains both features and the supported
>> Enlightened VMCS range.
>> 
>> Hyper-V features are already represented in 'struct X86CPU' with
>> uint64_t hyperv_features so duplicating them in env->features adds
>> little (or zero) benefits. THe other half of Hyper-V emulation features
>> is also stored with values in hyperv_vendor_id[], hyperv_limits[],...
>> so env->features[] is already incomplete.
>> 
>> Remove Hyper-V feature leaves from env->features[] completely.
>> kvm_hyperv_properties[] is converted to using raw CPUID func/reg
>> pairs for features, this allows us to get rid of hv_cpuid_get_fw()
>> conversion.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> So, my first reaction to this was "we don't want to revert this,
> having hyperv features in env->features is useful", but seeing
> that there's only one line where env->features[] is actually used
> in target/i386/cpu.c, I agree with this change.  We still have
> the option to recreate those FeatureWord entries in the future,
> if necessary.
>
> I'm still reviewing the patch and it looks good so far, but I
> have one question: have you considered doing this at the
> beginning of the series?  There are
>   env->features[...] = hv_cpuid_get_host(cs, ...)
> lines you have added in the previous patches, just to remove them
> in this patch.
>

In case the general idea (removing Hyper-V feature leaves from
env->features[]) is acceptable I can try to minimize code churn in
v1. Maybe separating hv_build_cpuid_leaf() into its own patch will make
the removal cleaner, I'll definitely give it a try.

>> ---
>>  target/i386/cpu.c |  90 +----------------------
>>  target/i386/cpu.h |   6 +-
>>  target/i386/kvm.c | 181 ++++++++++++++++++++++------------------------
>>  3 files changed, 90 insertions(+), 187 deletions(-)
>> 
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 8ec0af0a6d48..a178db255641 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -828,94 +828,6 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>>           */
>>          .no_autoenable_flags = ~0U,
>>      },
>> -    /*
>> -     * .feat_names are commented out for Hyper-V enlightenments because we
>> -     * don't want to have two different ways for enabling them on QEMU command
>> -     * line. Some features (e.g. "hyperv_time", "hyperv_vapic", ...) require
>> -     * enabling several feature bits simultaneously, exposing these bits
>> -     * individually may just confuse guests.
>> -     */
>> -    [FEAT_HYPERV_EAX] = {
>> -        .type = CPUID_FEATURE_WORD,
>> -        .feat_names = {
>> -            NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
>> -            NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
>> -            NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */,
>> -            NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
>> -            NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
>> -            NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */,
>> -            NULL /* hv_msr_debug_access */, NULL /* hv_msr_reenlightenment_access */,
>> -            NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -        },
>> -        .cpuid = { .eax = 0x40000003, .reg = R_EAX, },
>> -    },
>> -    [FEAT_HYPERV_EBX] = {
>> -        .type = CPUID_FEATURE_WORD,
>> -        .feat_names = {
>> -            NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
>> -            NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
>> -            NULL /* hv_post_messages */, NULL /* hv_signal_events */,
>> -            NULL /* hv_create_port */, NULL /* hv_connect_port */,
>> -            NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */,
>> -            NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */,
>> -            NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -        },
>> -        .cpuid = { .eax = 0x40000003, .reg = R_EBX, },
>> -    },
>> -    [FEAT_HYPERV_EDX] = {
>> -        .type = CPUID_FEATURE_WORD,
>> -        .feat_names = {
>> -            NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
>> -            NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
>> -            NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */,
>> -            NULL, 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,
>> -        },
>> -        .cpuid = { .eax = 0x40000003, .reg = R_EDX, },
>> -    },
>> -    [FEAT_HV_RECOMM_EAX] = {
>> -        .type = CPUID_FEATURE_WORD,
>> -        .feat_names = {
>> -            NULL /* hv_recommend_pv_as_switch */,
>> -            NULL /* hv_recommend_pv_tlbflush_local */,
>> -            NULL /* hv_recommend_pv_tlbflush_remote */,
>> -            NULL /* hv_recommend_msr_apic_access */,
>> -            NULL /* hv_recommend_msr_reset */,
>> -            NULL /* hv_recommend_relaxed_timing */,
>> -            NULL /* hv_recommend_dma_remapping */,
>> -            NULL /* hv_recommend_int_remapping */,
>> -            NULL /* hv_recommend_x2apic_msrs */,
>> -            NULL /* hv_recommend_autoeoi_deprecation */,
>> -            NULL /* hv_recommend_pv_ipi */,
>> -            NULL /* hv_recommend_ex_hypercalls */,
>> -            NULL /* hv_hypervisor_is_nested */,
>> -            NULL /* hv_recommend_int_mbec */,
>> -            NULL /* hv_recommend_evmcs */,
>> -            NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -            NULL, NULL, NULL, NULL,
>> -        },
>> -        .cpuid = { .eax = 0x40000004, .reg = R_EAX, },
>> -    },
>> -    [FEAT_HV_NESTED_EAX] = {
>> -        .type = CPUID_FEATURE_WORD,
>> -        .cpuid = { .eax = 0x4000000A, .reg = R_EAX, },
>> -    },
>>      [FEAT_SVM] = {
>>          .type = CPUID_FEATURE_WORD,
>>          .feat_names = {
>> @@ -6906,7 +6818,7 @@ static GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs)
>>      CPUX86State *env = &cpu->env;
>>      GuestPanicInformation *panic_info = NULL;
>>  
>> -    if (env->features[FEAT_HYPERV_EDX] & HV_GUEST_CRASH_MSR_AVAILABLE) {
>> +    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_CRASH)) {
>>          panic_info = g_malloc0(sizeof(GuestPanicInformation));
>>  
>>          panic_info->type = GUEST_PANIC_INFORMATION_TYPE_HYPER_V;
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index 187e4a09e65d..b07325745bcb 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -519,11 +519,6 @@ typedef enum FeatureWord {
>>      FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
>>      FEAT_KVM,           /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
>>      FEAT_KVM_HINTS,     /* CPUID[4000_0001].EDX */
>> -    FEAT_HYPERV_EAX,    /* CPUID[4000_0003].EAX */
>> -    FEAT_HYPERV_EBX,    /* CPUID[4000_0003].EBX */
>> -    FEAT_HYPERV_EDX,    /* CPUID[4000_0003].EDX */
>> -    FEAT_HV_RECOMM_EAX, /* CPUID[4000_0004].EAX */
>> -    FEAT_HV_NESTED_EAX, /* CPUID[4000_000A].EAX */
>>      FEAT_SVM,           /* CPUID[8000_000A].EDX */
>>      FEAT_XSAVE,         /* CPUID[EAX=0xd,ECX=1].EAX */
>>      FEAT_6_EAX,         /* CPUID[6].EAX */
>> @@ -1663,6 +1658,7 @@ struct X86CPU {
>>      uint32_t hyperv_interface_id[4];
>>      uint32_t hyperv_version_id[4];
>>      uint32_t hyperv_limits[3];
>> +    uint32_t hyperv_nested[4];
>>  
>>      bool check_cpuid;
>>      bool enforce_cpuid;
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 0e6eef6a52c2..86a5cfa1e12b 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -815,7 +815,8 @@ static bool tsc_is_stable_and_known(CPUX86State *env)
>>  static struct {
>>      const char *desc;
>>      struct {
>> -        uint32_t fw;
>> +        uint32_t func;
>> +        int reg;
>>          uint32_t bits;
>>      } flags[2];
>>      uint64_t dependencies;
>> @@ -823,25 +824,25 @@ static struct {
>>      [HYPERV_FEAT_RELAXED] = {
>>          .desc = "relaxed timing (hv-relaxed)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_HYPERCALL_AVAILABLE},
>> -            {.fw = FEAT_HV_RECOMM_EAX,
>> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>>               .bits = HV_RELAXED_TIMING_RECOMMENDED}
>>          }
>>      },
>>      [HYPERV_FEAT_VAPIC] = {
>>          .desc = "virtual APIC (hv-vapic)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_HYPERCALL_AVAILABLE | HV_APIC_ACCESS_AVAILABLE},
>> -            {.fw = FEAT_HV_RECOMM_EAX,
>> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>>               .bits = HV_APIC_ACCESS_RECOMMENDED}
>>          }
>>      },
>>      [HYPERV_FEAT_TIME] = {
>>          .desc = "clocksources (hv-time)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_HYPERCALL_AVAILABLE | HV_TIME_REF_COUNT_AVAILABLE |
>>               HV_REFERENCE_TSC_AVAILABLE}
>>          }
>> @@ -849,42 +850,42 @@ static struct {
>>      [HYPERV_FEAT_CRASH] = {
>>          .desc = "crash MSRs (hv-crash)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EDX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
>>               .bits = HV_GUEST_CRASH_MSR_AVAILABLE}
>>          }
>>      },
>>      [HYPERV_FEAT_RESET] = {
>>          .desc = "reset MSR (hv-reset)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_RESET_AVAILABLE}
>>          }
>>      },
>>      [HYPERV_FEAT_VPINDEX] = {
>>          .desc = "VP_INDEX MSR (hv-vpindex)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_VP_INDEX_AVAILABLE}
>>          }
>>      },
>>      [HYPERV_FEAT_RUNTIME] = {
>>          .desc = "VP_RUNTIME MSR (hv-runtime)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_VP_RUNTIME_AVAILABLE}
>>          }
>>      },
>>      [HYPERV_FEAT_SYNIC] = {
>>          .desc = "synthetic interrupt controller (hv-synic)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_SYNIC_AVAILABLE}
>>          }
>>      },
>>      [HYPERV_FEAT_STIMER] = {
>>          .desc = "synthetic timers (hv-stimer)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_SYNTIMERS_AVAILABLE}
>>          },
>>          .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
>> @@ -892,23 +893,23 @@ static struct {
>>      [HYPERV_FEAT_FREQUENCIES] = {
>>          .desc = "frequency MSRs (hv-frequencies)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_ACCESS_FREQUENCY_MSRS},
>> -            {.fw = FEAT_HYPERV_EDX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
>>               .bits = HV_FREQUENCY_MSRS_AVAILABLE}
>>          }
>>      },
>>      [HYPERV_FEAT_REENLIGHTENMENT] = {
>>          .desc = "reenlightenment MSRs (hv-reenlightenment)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EAX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>>               .bits = HV_ACCESS_REENLIGHTENMENTS_CONTROL}
>>          }
>>      },
>>      [HYPERV_FEAT_TLBFLUSH] = {
>>          .desc = "paravirtualized TLB flush (hv-tlbflush)",
>>          .flags = {
>> -            {.fw = FEAT_HV_RECOMM_EAX,
>> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>>               .bits = HV_REMOTE_TLB_FLUSH_RECOMMENDED |
>>               HV_EX_PROCESSOR_MASKS_RECOMMENDED}
>>          },
>> @@ -917,7 +918,7 @@ static struct {
>>      [HYPERV_FEAT_EVMCS] = {
>>          .desc = "enlightened VMCS (hv-evmcs)",
>>          .flags = {
>> -            {.fw = FEAT_HV_RECOMM_EAX,
>> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>>               .bits = HV_ENLIGHTENED_VMCS_RECOMMENDED}
>>          },
>>          .dependencies = BIT(HYPERV_FEAT_VAPIC)
>> @@ -925,7 +926,7 @@ static struct {
>>      [HYPERV_FEAT_IPI] = {
>>          .desc = "paravirtualized IPI (hv-ipi)",
>>          .flags = {
>> -            {.fw = FEAT_HV_RECOMM_EAX,
>> +            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
>>               .bits = HV_CLUSTER_IPI_RECOMMENDED |
>>               HV_EX_PROCESSOR_MASKS_RECOMMENDED}
>>          },
>> @@ -934,7 +935,7 @@ static struct {
>>      [HYPERV_FEAT_STIMER_DIRECT] = {
>>          .desc = "direct mode synthetic timers (hv-stimer-direct)",
>>          .flags = {
>> -            {.fw = FEAT_HYPERV_EDX,
>> +            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
>>               .bits = HV_STIMER_DIRECT_MODE_AVAILABLE}
>>          },
>>          .dependencies = BIT(HYPERV_FEAT_STIMER)
>> @@ -1108,38 +1109,34 @@ static uint32_t hv_cpuid_get_host(CPUState *cs, uint32_t func, int reg)
>>      return cpuid_entry_get_reg(entry, reg);
>>  }
>>  
>> -static uint32_t hv_cpuid_get_fw(CPUState *cs, int fw)
>> +static bool hyperv_feature_supported(CPUState *cs, int feature)
>>  {
>> -    uint32_t func;
>> -    int reg;
>> +    uint32_t func, bits;
>> +    int i, reg;
>>  
>> -    switch (fw) {
>> -    case FEAT_HYPERV_EAX:
>> -        reg = R_EAX;
>> -        func = HV_CPUID_FEATURES;
>> -        break;
>> -    case FEAT_HYPERV_EDX:
>> -        reg = R_EDX;
>> -        func = HV_CPUID_FEATURES;
>> -        break;
>> -    case FEAT_HV_RECOMM_EAX:
>> -        reg = R_EAX;
>> -        func = HV_CPUID_ENLIGHTMENT_INFO;
>> -        break;
>> -    default:
>> -        return 0;
>> +    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
>> +
>> +        func = kvm_hyperv_properties[feature].flags[i].func;
>> +        reg = kvm_hyperv_properties[feature].flags[i].reg;
>> +        bits = kvm_hyperv_properties[feature].flags[i].bits;
>> +
>> +        if (!func) {
>> +            continue;
>> +        }
>> +
>> +        if ((hv_cpuid_get_host(cs, func, reg) & bits) != bits) {
>> +            return false;
>> +        }
>>      }
>>  
>> -    return hv_cpuid_get_host(cs, func, reg);
>> +    return true;
>>  }
>>  
>>  static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>>  {
>>      X86CPU *cpu = X86_CPU(cs);
>> -    CPUX86State *env = &cpu->env;
>> -    uint32_t r, fw, bits;
>>      uint64_t deps;
>> -    int i, dep_feat;
>> +    int dep_feat;
>>  
>>      if (!hyperv_feat_enabled(cpu, feature) && !cpu->hyperv_passthrough) {
>>          return 0;
>> @@ -1158,27 +1155,15 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>>          deps &= ~(1ull << dep_feat);
>>      }
>>  
>> -    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
>> -        fw = kvm_hyperv_properties[feature].flags[i].fw;
>> -        bits = kvm_hyperv_properties[feature].flags[i].bits;
>> -
>> -        if (!fw) {
>> -            continue;
>> -        }
>> -
>> -        r = hv_cpuid_get_fw(cs, fw);
>> -        if ((r & bits) != bits) {
>> -            if (hyperv_feat_enabled(cpu, feature)) {
>> -                fprintf(stderr,
>> -                        "Hyper-V %s is not supported by kernel\n",
>> -                        kvm_hyperv_properties[feature].desc);
>> -                return 1;
>> -            } else {
>> -                return 0;
>> -            }
>> +    if (!hyperv_feature_supported(cs, feature)) {
>> +        if (hyperv_feat_enabled(cpu, feature)) {
>> +            fprintf(stderr,
>> +                    "Hyper-V %s is not supported by kernel\n",
>> +                    kvm_hyperv_properties[feature].desc);
>> +            return 1;
>> +        } else {
>> +            return 0;
>>          }
>> -
>> -        env->features[fw] |= bits;
>>      }
>>  
>>      if (cpu->hyperv_passthrough) {
>> @@ -1188,6 +1173,32 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>>      return 0;
>>  }
>>  
>> +static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg)
>> +{
>> +    X86CPU *cpu = X86_CPU(cs);
>> +    uint32_t r = 0;
>> +    int i, j;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties); i++) {
>> +        if (!hyperv_feat_enabled(cpu, i)) {
>> +            continue;
>> +        }
>> +
>> +        for (j = 0; j < ARRAY_SIZE(kvm_hyperv_properties[i].flags); j++) {
>> +            if (kvm_hyperv_properties[i].flags[j].func != func) {
>> +                continue;
>> +            }
>> +            if (kvm_hyperv_properties[i].flags[j].reg != reg) {
>> +                continue;
>> +            }
>> +
>> +            r |= kvm_hyperv_properties[i].flags[j].bits;
>> +        }
>> +    }
>> +
>> +    return r;
>> +}
>> +
>>  /*
>>   * 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
>> @@ -1197,7 +1208,6 @@ static int hyperv_handle_properties(CPUState *cs,
>>                                      struct kvm_cpuid_entry2 *cpuid_ent)
>>  {
>>      X86CPU *cpu = X86_CPU(cs);
>> -    CPUX86State *env = &cpu->env;
>>      struct kvm_cpuid_entry2 *c;
>>      uint32_t cpuid_i = 0;
>>      int r;
>> @@ -1219,9 +1229,7 @@ static int hyperv_handle_properties(CPUState *cs,
>>          }
>>  
>>          if (!r) {
>> -            env->features[FEAT_HV_RECOMM_EAX] |=
>> -                HV_ENLIGHTENED_VMCS_RECOMMENDED;
>> -            env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
>> +            cpu->hyperv_nested[0] = evmcs_version;
>>          }
>>      }
>>  
>> @@ -1255,13 +1263,6 @@ static int hyperv_handle_properties(CPUState *cs,
>>          cpu->hyperv_version_id[3] =
>>              hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EDX);
>>  
>> -        env->features[FEAT_HYPERV_EAX] =
>> -            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EAX);
>> -        env->features[FEAT_HYPERV_EBX] =
>> -            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EBX);
>> -        env->features[FEAT_HYPERV_EDX] =
>> -            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EDX);
>> -
>>          cpu->hv_max_vps = hv_cpuid_get_host(cs, HV_CPUID_IMPLEMENT_LIMITS,
>>                                              R_EAX);
>>          cpu->hyperv_limits[0] =
>> @@ -1271,21 +1272,8 @@ static int hyperv_handle_properties(CPUState *cs,
>>          cpu->hyperv_limits[2] =
>>              hv_cpuid_get_host(cs, HV_CPUID_IMPLEMENT_LIMITS, R_EDX);
>>  
>> -        env->features[FEAT_HV_RECOMM_EAX] =
>> -            hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
>>          cpu->hyperv_spinlock_attempts =
>>              hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EBX);
>> -
>> -        env->features[FEAT_HV_NESTED_EAX] =
>> -            hv_cpuid_get_host(cs, HV_CPUID_NESTED_FEATURES, R_EAX);
>> -    }
>> -
>> -    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
>> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
>> -    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
>> -        env->features[FEAT_HV_RECOMM_EAX] |=
>> -            hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX) &
>> -            HV_NO_NONARCH_CORESHARING;
>>      }
>>  
>>      /* Features */
>> @@ -1315,9 +1303,6 @@ static int hyperv_handle_properties(CPUState *cs,
>>          r |= 1;
>>      }
>>  
>> -    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
>> -    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
>> -
>>      if (r) {
>>          return -ENOSYS;
>>      }
>> @@ -1346,15 +1331,25 @@ static int hyperv_handle_properties(CPUState *cs,
>>  
>>      c = &cpuid_ent[cpuid_i++];
>>      c->function = HV_CPUID_FEATURES;
>> -    c->eax = env->features[FEAT_HYPERV_EAX];
>> -    c->ebx = env->features[FEAT_HYPERV_EBX];
>> -    c->edx = env->features[FEAT_HYPERV_EDX];
>> +    c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EAX);
>> +    c->ebx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EBX);
>> +    c->edx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EDX);
>> +
>> +    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
>> +    c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
>>  
>>      c = &cpuid_ent[cpuid_i++];
>>      c->function = HV_CPUID_ENLIGHTMENT_INFO;
>> -    c->eax = env->features[FEAT_HV_RECOMM_EAX];
>> +    c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
>>      c->ebx = cpu->hyperv_spinlock_attempts;
>>  
>> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
>> +        c->eax |= HV_NO_NONARCH_CORESHARING;
>> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
>> +        c->eax |= hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX) &
>> +            HV_NO_NONARCH_CORESHARING;
>> +    }
>> +
>>      c = &cpuid_ent[cpuid_i++];
>>      c->function = HV_CPUID_IMPLEMENT_LIMITS;
>>      c->eax = cpu->hv_max_vps;
>> @@ -1374,7 +1369,7 @@ static int hyperv_handle_properties(CPUState *cs,
>>  
>>          c = &cpuid_ent[cpuid_i++];
>>          c->function = HV_CPUID_NESTED_FEATURES;
>> -        c->eax = env->features[FEAT_HV_NESTED_EAX];
>> +        c->eax = cpu->hyperv_nested[0];
>>      }
>>  
>>      return cpuid_i;
>> -- 
>> 2.25.4
>>
diff mbox series

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8ec0af0a6d48..a178db255641 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -828,94 +828,6 @@  static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
          */
         .no_autoenable_flags = ~0U,
     },
-    /*
-     * .feat_names are commented out for Hyper-V enlightenments because we
-     * don't want to have two different ways for enabling them on QEMU command
-     * line. Some features (e.g. "hyperv_time", "hyperv_vapic", ...) require
-     * enabling several feature bits simultaneously, exposing these bits
-     * individually may just confuse guests.
-     */
-    [FEAT_HYPERV_EAX] = {
-        .type = CPUID_FEATURE_WORD,
-        .feat_names = {
-            NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
-            NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
-            NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */,
-            NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
-            NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
-            NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */,
-            NULL /* hv_msr_debug_access */, NULL /* hv_msr_reenlightenment_access */,
-            NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-        },
-        .cpuid = { .eax = 0x40000003, .reg = R_EAX, },
-    },
-    [FEAT_HYPERV_EBX] = {
-        .type = CPUID_FEATURE_WORD,
-        .feat_names = {
-            NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
-            NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
-            NULL /* hv_post_messages */, NULL /* hv_signal_events */,
-            NULL /* hv_create_port */, NULL /* hv_connect_port */,
-            NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */,
-            NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */,
-            NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-        },
-        .cpuid = { .eax = 0x40000003, .reg = R_EBX, },
-    },
-    [FEAT_HYPERV_EDX] = {
-        .type = CPUID_FEATURE_WORD,
-        .feat_names = {
-            NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
-            NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
-            NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */,
-            NULL, 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,
-        },
-        .cpuid = { .eax = 0x40000003, .reg = R_EDX, },
-    },
-    [FEAT_HV_RECOMM_EAX] = {
-        .type = CPUID_FEATURE_WORD,
-        .feat_names = {
-            NULL /* hv_recommend_pv_as_switch */,
-            NULL /* hv_recommend_pv_tlbflush_local */,
-            NULL /* hv_recommend_pv_tlbflush_remote */,
-            NULL /* hv_recommend_msr_apic_access */,
-            NULL /* hv_recommend_msr_reset */,
-            NULL /* hv_recommend_relaxed_timing */,
-            NULL /* hv_recommend_dma_remapping */,
-            NULL /* hv_recommend_int_remapping */,
-            NULL /* hv_recommend_x2apic_msrs */,
-            NULL /* hv_recommend_autoeoi_deprecation */,
-            NULL /* hv_recommend_pv_ipi */,
-            NULL /* hv_recommend_ex_hypercalls */,
-            NULL /* hv_hypervisor_is_nested */,
-            NULL /* hv_recommend_int_mbec */,
-            NULL /* hv_recommend_evmcs */,
-            NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-        },
-        .cpuid = { .eax = 0x40000004, .reg = R_EAX, },
-    },
-    [FEAT_HV_NESTED_EAX] = {
-        .type = CPUID_FEATURE_WORD,
-        .cpuid = { .eax = 0x4000000A, .reg = R_EAX, },
-    },
     [FEAT_SVM] = {
         .type = CPUID_FEATURE_WORD,
         .feat_names = {
@@ -6906,7 +6818,7 @@  static GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs)
     CPUX86State *env = &cpu->env;
     GuestPanicInformation *panic_info = NULL;
 
-    if (env->features[FEAT_HYPERV_EDX] & HV_GUEST_CRASH_MSR_AVAILABLE) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_CRASH)) {
         panic_info = g_malloc0(sizeof(GuestPanicInformation));
 
         panic_info->type = GUEST_PANIC_INFORMATION_TYPE_HYPER_V;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 187e4a09e65d..b07325745bcb 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -519,11 +519,6 @@  typedef enum FeatureWord {
     FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
     FEAT_KVM,           /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
     FEAT_KVM_HINTS,     /* CPUID[4000_0001].EDX */
-    FEAT_HYPERV_EAX,    /* CPUID[4000_0003].EAX */
-    FEAT_HYPERV_EBX,    /* CPUID[4000_0003].EBX */
-    FEAT_HYPERV_EDX,    /* CPUID[4000_0003].EDX */
-    FEAT_HV_RECOMM_EAX, /* CPUID[4000_0004].EAX */
-    FEAT_HV_NESTED_EAX, /* CPUID[4000_000A].EAX */
     FEAT_SVM,           /* CPUID[8000_000A].EDX */
     FEAT_XSAVE,         /* CPUID[EAX=0xd,ECX=1].EAX */
     FEAT_6_EAX,         /* CPUID[6].EAX */
@@ -1663,6 +1658,7 @@  struct X86CPU {
     uint32_t hyperv_interface_id[4];
     uint32_t hyperv_version_id[4];
     uint32_t hyperv_limits[3];
+    uint32_t hyperv_nested[4];
 
     bool check_cpuid;
     bool enforce_cpuid;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 0e6eef6a52c2..86a5cfa1e12b 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -815,7 +815,8 @@  static bool tsc_is_stable_and_known(CPUX86State *env)
 static struct {
     const char *desc;
     struct {
-        uint32_t fw;
+        uint32_t func;
+        int reg;
         uint32_t bits;
     } flags[2];
     uint64_t dependencies;
@@ -823,25 +824,25 @@  static struct {
     [HYPERV_FEAT_RELAXED] = {
         .desc = "relaxed timing (hv-relaxed)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_HYPERCALL_AVAILABLE},
-            {.fw = FEAT_HV_RECOMM_EAX,
+            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
              .bits = HV_RELAXED_TIMING_RECOMMENDED}
         }
     },
     [HYPERV_FEAT_VAPIC] = {
         .desc = "virtual APIC (hv-vapic)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_HYPERCALL_AVAILABLE | HV_APIC_ACCESS_AVAILABLE},
-            {.fw = FEAT_HV_RECOMM_EAX,
+            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
              .bits = HV_APIC_ACCESS_RECOMMENDED}
         }
     },
     [HYPERV_FEAT_TIME] = {
         .desc = "clocksources (hv-time)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_HYPERCALL_AVAILABLE | HV_TIME_REF_COUNT_AVAILABLE |
              HV_REFERENCE_TSC_AVAILABLE}
         }
@@ -849,42 +850,42 @@  static struct {
     [HYPERV_FEAT_CRASH] = {
         .desc = "crash MSRs (hv-crash)",
         .flags = {
-            {.fw = FEAT_HYPERV_EDX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
              .bits = HV_GUEST_CRASH_MSR_AVAILABLE}
         }
     },
     [HYPERV_FEAT_RESET] = {
         .desc = "reset MSR (hv-reset)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_RESET_AVAILABLE}
         }
     },
     [HYPERV_FEAT_VPINDEX] = {
         .desc = "VP_INDEX MSR (hv-vpindex)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_VP_INDEX_AVAILABLE}
         }
     },
     [HYPERV_FEAT_RUNTIME] = {
         .desc = "VP_RUNTIME MSR (hv-runtime)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_VP_RUNTIME_AVAILABLE}
         }
     },
     [HYPERV_FEAT_SYNIC] = {
         .desc = "synthetic interrupt controller (hv-synic)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_SYNIC_AVAILABLE}
         }
     },
     [HYPERV_FEAT_STIMER] = {
         .desc = "synthetic timers (hv-stimer)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_SYNTIMERS_AVAILABLE}
         },
         .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
@@ -892,23 +893,23 @@  static struct {
     [HYPERV_FEAT_FREQUENCIES] = {
         .desc = "frequency MSRs (hv-frequencies)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_ACCESS_FREQUENCY_MSRS},
-            {.fw = FEAT_HYPERV_EDX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
              .bits = HV_FREQUENCY_MSRS_AVAILABLE}
         }
     },
     [HYPERV_FEAT_REENLIGHTENMENT] = {
         .desc = "reenlightenment MSRs (hv-reenlightenment)",
         .flags = {
-            {.fw = FEAT_HYPERV_EAX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
              .bits = HV_ACCESS_REENLIGHTENMENTS_CONTROL}
         }
     },
     [HYPERV_FEAT_TLBFLUSH] = {
         .desc = "paravirtualized TLB flush (hv-tlbflush)",
         .flags = {
-            {.fw = FEAT_HV_RECOMM_EAX,
+            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
              .bits = HV_REMOTE_TLB_FLUSH_RECOMMENDED |
              HV_EX_PROCESSOR_MASKS_RECOMMENDED}
         },
@@ -917,7 +918,7 @@  static struct {
     [HYPERV_FEAT_EVMCS] = {
         .desc = "enlightened VMCS (hv-evmcs)",
         .flags = {
-            {.fw = FEAT_HV_RECOMM_EAX,
+            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
              .bits = HV_ENLIGHTENED_VMCS_RECOMMENDED}
         },
         .dependencies = BIT(HYPERV_FEAT_VAPIC)
@@ -925,7 +926,7 @@  static struct {
     [HYPERV_FEAT_IPI] = {
         .desc = "paravirtualized IPI (hv-ipi)",
         .flags = {
-            {.fw = FEAT_HV_RECOMM_EAX,
+            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
              .bits = HV_CLUSTER_IPI_RECOMMENDED |
              HV_EX_PROCESSOR_MASKS_RECOMMENDED}
         },
@@ -934,7 +935,7 @@  static struct {
     [HYPERV_FEAT_STIMER_DIRECT] = {
         .desc = "direct mode synthetic timers (hv-stimer-direct)",
         .flags = {
-            {.fw = FEAT_HYPERV_EDX,
+            {.func = HV_CPUID_FEATURES, .reg = R_EDX,
              .bits = HV_STIMER_DIRECT_MODE_AVAILABLE}
         },
         .dependencies = BIT(HYPERV_FEAT_STIMER)
@@ -1108,38 +1109,34 @@  static uint32_t hv_cpuid_get_host(CPUState *cs, uint32_t func, int reg)
     return cpuid_entry_get_reg(entry, reg);
 }
 
-static uint32_t hv_cpuid_get_fw(CPUState *cs, int fw)
+static bool hyperv_feature_supported(CPUState *cs, int feature)
 {
-    uint32_t func;
-    int reg;
+    uint32_t func, bits;
+    int i, reg;
 
-    switch (fw) {
-    case FEAT_HYPERV_EAX:
-        reg = R_EAX;
-        func = HV_CPUID_FEATURES;
-        break;
-    case FEAT_HYPERV_EDX:
-        reg = R_EDX;
-        func = HV_CPUID_FEATURES;
-        break;
-    case FEAT_HV_RECOMM_EAX:
-        reg = R_EAX;
-        func = HV_CPUID_ENLIGHTMENT_INFO;
-        break;
-    default:
-        return 0;
+    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
+
+        func = kvm_hyperv_properties[feature].flags[i].func;
+        reg = kvm_hyperv_properties[feature].flags[i].reg;
+        bits = kvm_hyperv_properties[feature].flags[i].bits;
+
+        if (!func) {
+            continue;
+        }
+
+        if ((hv_cpuid_get_host(cs, func, reg) & bits) != bits) {
+            return false;
+        }
     }
 
-    return hv_cpuid_get_host(cs, func, reg);
+    return true;
 }
 
 static int hv_cpuid_check_and_set(CPUState *cs, int feature)
 {
     X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
-    uint32_t r, fw, bits;
     uint64_t deps;
-    int i, dep_feat;
+    int dep_feat;
 
     if (!hyperv_feat_enabled(cpu, feature) && !cpu->hyperv_passthrough) {
         return 0;
@@ -1158,27 +1155,15 @@  static int hv_cpuid_check_and_set(CPUState *cs, int feature)
         deps &= ~(1ull << dep_feat);
     }
 
-    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
-        fw = kvm_hyperv_properties[feature].flags[i].fw;
-        bits = kvm_hyperv_properties[feature].flags[i].bits;
-
-        if (!fw) {
-            continue;
-        }
-
-        r = hv_cpuid_get_fw(cs, fw);
-        if ((r & bits) != bits) {
-            if (hyperv_feat_enabled(cpu, feature)) {
-                fprintf(stderr,
-                        "Hyper-V %s is not supported by kernel\n",
-                        kvm_hyperv_properties[feature].desc);
-                return 1;
-            } else {
-                return 0;
-            }
+    if (!hyperv_feature_supported(cs, feature)) {
+        if (hyperv_feat_enabled(cpu, feature)) {
+            fprintf(stderr,
+                    "Hyper-V %s is not supported by kernel\n",
+                    kvm_hyperv_properties[feature].desc);
+            return 1;
+        } else {
+            return 0;
         }
-
-        env->features[fw] |= bits;
     }
 
     if (cpu->hyperv_passthrough) {
@@ -1188,6 +1173,32 @@  static int hv_cpuid_check_and_set(CPUState *cs, int feature)
     return 0;
 }
 
+static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    uint32_t r = 0;
+    int i, j;
+
+    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties); i++) {
+        if (!hyperv_feat_enabled(cpu, i)) {
+            continue;
+        }
+
+        for (j = 0; j < ARRAY_SIZE(kvm_hyperv_properties[i].flags); j++) {
+            if (kvm_hyperv_properties[i].flags[j].func != func) {
+                continue;
+            }
+            if (kvm_hyperv_properties[i].flags[j].reg != reg) {
+                continue;
+            }
+
+            r |= kvm_hyperv_properties[i].flags[j].bits;
+        }
+    }
+
+    return r;
+}
+
 /*
  * 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
@@ -1197,7 +1208,6 @@  static int hyperv_handle_properties(CPUState *cs,
                                     struct kvm_cpuid_entry2 *cpuid_ent)
 {
     X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
     struct kvm_cpuid_entry2 *c;
     uint32_t cpuid_i = 0;
     int r;
@@ -1219,9 +1229,7 @@  static int hyperv_handle_properties(CPUState *cs,
         }
 
         if (!r) {
-            env->features[FEAT_HV_RECOMM_EAX] |=
-                HV_ENLIGHTENED_VMCS_RECOMMENDED;
-            env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
+            cpu->hyperv_nested[0] = evmcs_version;
         }
     }
 
@@ -1255,13 +1263,6 @@  static int hyperv_handle_properties(CPUState *cs,
         cpu->hyperv_version_id[3] =
             hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EDX);
 
-        env->features[FEAT_HYPERV_EAX] =
-            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EAX);
-        env->features[FEAT_HYPERV_EBX] =
-            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EBX);
-        env->features[FEAT_HYPERV_EDX] =
-            hv_cpuid_get_host(cs, HV_CPUID_FEATURES, R_EDX);
-
         cpu->hv_max_vps = hv_cpuid_get_host(cs, HV_CPUID_IMPLEMENT_LIMITS,
                                             R_EAX);
         cpu->hyperv_limits[0] =
@@ -1271,21 +1272,8 @@  static int hyperv_handle_properties(CPUState *cs,
         cpu->hyperv_limits[2] =
             hv_cpuid_get_host(cs, HV_CPUID_IMPLEMENT_LIMITS, R_EDX);
 
-        env->features[FEAT_HV_RECOMM_EAX] =
-            hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
         cpu->hyperv_spinlock_attempts =
             hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EBX);
-
-        env->features[FEAT_HV_NESTED_EAX] =
-            hv_cpuid_get_host(cs, HV_CPUID_NESTED_FEATURES, R_EAX);
-    }
-
-    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
-    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
-        env->features[FEAT_HV_RECOMM_EAX] |=
-            hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX) &
-            HV_NO_NONARCH_CORESHARING;
     }
 
     /* Features */
@@ -1315,9 +1303,6 @@  static int hyperv_handle_properties(CPUState *cs,
         r |= 1;
     }
 
-    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
-    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
-
     if (r) {
         return -ENOSYS;
     }
@@ -1346,15 +1331,25 @@  static int hyperv_handle_properties(CPUState *cs,
 
     c = &cpuid_ent[cpuid_i++];
     c->function = HV_CPUID_FEATURES;
-    c->eax = env->features[FEAT_HYPERV_EAX];
-    c->ebx = env->features[FEAT_HYPERV_EBX];
-    c->edx = env->features[FEAT_HYPERV_EDX];
+    c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EAX);
+    c->ebx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EBX);
+    c->edx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EDX);
+
+    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
+    c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
 
     c = &cpuid_ent[cpuid_i++];
     c->function = HV_CPUID_ENLIGHTMENT_INFO;
-    c->eax = env->features[FEAT_HV_RECOMM_EAX];
+    c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
     c->ebx = cpu->hyperv_spinlock_attempts;
 
+    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
+        c->eax |= HV_NO_NONARCH_CORESHARING;
+    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
+        c->eax |= hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX) &
+            HV_NO_NONARCH_CORESHARING;
+    }
+
     c = &cpuid_ent[cpuid_i++];
     c->function = HV_CPUID_IMPLEMENT_LIMITS;
     c->eax = cpu->hv_max_vps;
@@ -1374,7 +1369,7 @@  static int hyperv_handle_properties(CPUState *cs,
 
         c = &cpuid_ent[cpuid_i++];
         c->function = HV_CPUID_NESTED_FEATURES;
-        c->eax = env->features[FEAT_HV_NESTED_EAX];
+        c->eax = cpu->hyperv_nested[0];
     }
 
     return cpuid_i;