From 00c1b316edb33f2efb0775c2983bc5348c86529d Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 8 Jun 2021 14:08:10 +0200 Subject: [PATCH 01/11] i386: clarify 'hv-passthrough' behavior Clarify the fact that 'hv-passthrough' only enables features which are already known to QEMU and that it overrides all other 'hv-*' settings. Reviewed-by: Eduardo Habkost Signed-off-by: Vitaly Kuznetsov Message-Id: <20210608120817.1325125-3-vkuznets@redhat.com> Signed-off-by: Eduardo Habkost --- docs/hyperv.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/hyperv.txt b/docs/hyperv.txt index e53c581f45..a51953daa8 100644 --- a/docs/hyperv.txt +++ b/docs/hyperv.txt @@ -209,8 +209,11 @@ 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-passthrough" CPU flag. -Note: enabling this flag effectively prevents migration as supported features -may differ between target and destination. +Note: "hv-passthrough" flag only enables enlightenments which are known to QEMU +(have corresponding "hv-*" flag) and copies "hv-spinlocks="/"hv-vendor-id=" +values from KVM to QEMU. "hv-passthrough" overrides all other "hv-*" settings on +the command line. Also, enabling this flag effectively prevents migration as the +list of enabled enlightenments may differ between target and destination hosts. 4. Useful links From 07454e2ea84ee9be298c96d9730dc82abfb1488a Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 8 Jun 2021 14:08:11 +0200 Subject: [PATCH 02/11] i386: hardcode supported eVMCS version to '1' Currently, the only eVMCS version, supported by KVM (and described in TLFS) is '1'. When Enlightened VMCS feature is enabled, QEMU takes the supported eVMCS version range (from KVM_CAP_HYPERV_ENLIGHTENED_VMCS enablement) and puts it to guest visible CPUIDs. When (and if) eVMCS ver.2 appears a problem on migration is expected: it doesn't seem to be possible to migrate from a host supporting eVMCS ver.2 to a host, which only support eVMCS ver.1. Hardcode eVMCS ver.1 as the result of 'hv-evmcs' enablement for now. Newer eVMCS versions will have to have their own enablement options (e.g. 'hv-evmcs=2'). Signed-off-by: Vitaly Kuznetsov Reviewed-by: Eduardo Habkost Message-Id: <20210608120817.1325125-4-vkuznets@redhat.com> Signed-off-by: Eduardo Habkost --- docs/hyperv.txt | 2 +- target/i386/kvm/kvm.c | 39 +++++++++++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/docs/hyperv.txt b/docs/hyperv.txt index a51953daa8..000638a2fd 100644 --- a/docs/hyperv.txt +++ b/docs/hyperv.txt @@ -170,7 +170,7 @@ Recommended: hv-frequencies 3.16. hv-evmcs =============== The enlightenment is nested specific, it targets Hyper-V on KVM guests. When -enabled, it provides Enlightened VMCS feature to the guest. The feature +enabled, it provides Enlightened VMCS version 1 feature to the guest. The feature implements paravirtualized protocol between L0 (KVM) and L1 (Hyper-V) hypervisors making L2 exits to the hypervisor faster. The feature is Intel-only. Note: some virtualization features (e.g. Posted Interrupts) are disabled when diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index a85035492f..02216b7031 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -1409,6 +1409,21 @@ static int hyperv_fill_cpuids(CPUState *cs, static Error *hv_passthrough_mig_blocker; static Error *hv_no_nonarch_cs_mig_blocker; +/* Checks that the exposed eVMCS version range is supported by KVM */ +static bool evmcs_version_supported(uint16_t evmcs_version, + uint16_t supported_evmcs_version) +{ + uint8_t min_version = evmcs_version & 0xff; + uint8_t max_version = evmcs_version >> 8; + uint8_t min_supported_version = supported_evmcs_version & 0xff; + uint8_t max_supported_version = supported_evmcs_version >> 8; + + return (min_version >= min_supported_version) && + (max_version <= max_supported_version); +} + +#define DEFAULT_EVMCS_VERSION ((1 << 8) | 1) + static int hyperv_init_vcpu(X86CPU *cpu) { CPUState *cs = CPU(cpu); @@ -1488,17 +1503,33 @@ static int hyperv_init_vcpu(X86CPU *cpu) } if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) { - uint16_t evmcs_version; + uint16_t evmcs_version = DEFAULT_EVMCS_VERSION; + uint16_t supported_evmcs_version; ret = kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0, - (uintptr_t)&evmcs_version); + (uintptr_t)&supported_evmcs_version); + /* + * KVM is required to support EVMCS ver.1. as that's what 'hv-evmcs' + * option sets. Note: we hardcode the maximum supported eVMCS version + * to '1' as well so 'hv-evmcs' feature is migratable even when (and if) + * ver.2 is implemented. A new option (e.g. 'hv-evmcs=2') will then have + * to be added. + */ if (ret < 0) { - fprintf(stderr, "Hyper-V %s is not supported by kernel\n", - kvm_hyperv_properties[HYPERV_FEAT_EVMCS].desc); + error_report("Hyper-V %s is not supported by kernel", + kvm_hyperv_properties[HYPERV_FEAT_EVMCS].desc); return ret; } + if (!evmcs_version_supported(evmcs_version, supported_evmcs_version)) { + error_report("eVMCS version range [%d..%d] is not supported by " + "kernel (supported: [%d..%d])", evmcs_version & 0xff, + evmcs_version >> 8, supported_evmcs_version & 0xff, + supported_evmcs_version >> 8); + return -ENOTSUP; + } + cpu->hyperv_nested[0] = evmcs_version; } From d7652b772f302346c8f1043aa850a28c445e80d7 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 8 Jun 2021 14:08:12 +0200 Subject: [PATCH 03/11] i386: make hyperv_expand_features() return bool Return 'false' when hyperv_expand_features() sets an error. No functional change intended. Reviewed-by: Eduardo Habkost Signed-off-by: Vitaly Kuznetsov Message-Id: <20210608120817.1325125-5-vkuznets@redhat.com> Signed-off-by: Eduardo Habkost --- target/i386/kvm/kvm.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 02216b7031..ef127762bc 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -1220,12 +1220,12 @@ static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg) * of 'hv_passthrough' mode and fills the environment with all supported * Hyper-V features. */ -static void hyperv_expand_features(CPUState *cs, Error **errp) +static bool hyperv_expand_features(CPUState *cs, Error **errp) { X86CPU *cpu = X86_CPU(cs); if (!hyperv_enabled(cpu)) - return; + return true; if (cpu->hyperv_passthrough) { cpu->hyperv_vendor_id[0] = @@ -1273,49 +1273,49 @@ static void hyperv_expand_features(CPUState *cs, Error **errp) /* Features */ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI, errp)) { - return; + return false; } if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT, errp)) { - return; + return false; } /* Additional dependencies not covered by kvm_hyperv_properties[] */ @@ -1325,7 +1325,10 @@ static void hyperv_expand_features(CPUState *cs, Error **errp) error_setg(errp, "Hyper-V %s requires Hyper-V %s", kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc, kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc); + return false; } + + return true; } /* @@ -1591,8 +1594,7 @@ int kvm_arch_init_vcpu(CPUState *cs) env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY; /* Paravirtualization CPUIDs */ - hyperv_expand_features(cs, &local_err); - if (local_err) { + if (!hyperv_expand_features(cs, &local_err)) { error_report_err(local_err); return -ENOSYS; } From 071ce4b03becf9e2df6b758fde9609be8ddf56f1 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 8 Jun 2021 14:08:13 +0200 Subject: [PATCH 04/11] i386: expand Hyper-V features during CPU feature expansion time To make Hyper-V features appear in e.g. QMP query-cpu-model-expansion we need to expand and set the corresponding CPUID leaves early. Modify x86_cpu_get_supported_feature_word() to call newly intoduced Hyper-V specific kvm_hv_get_supported_cpuid() instead of kvm_arch_get_supported_cpuid(). We can't use kvm_arch_get_supported_cpuid() as Hyper-V specific CPUID leaves intersect with KVM's. Note, early expansion will only happen when KVM supports system wide KVM_GET_SUPPORTED_HV_CPUID ioctl (KVM_CAP_SYS_HYPERV_CPUID). Reviewed-by: Eduardo Habkost Signed-off-by: Vitaly Kuznetsov Message-Id: <20210608120817.1325125-6-vkuznets@redhat.com> Signed-off-by: Eduardo Habkost --- target/i386/cpu.c | 4 ++++ target/i386/kvm/kvm-stub.c | 5 +++++ target/i386/kvm/kvm.c | 24 ++++++++++++++++++++---- target/i386/kvm/kvm_i386.h | 1 + 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 5f595a0d7e..46befde387 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5974,6 +5974,10 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp) if (env->cpuid_xlevel2 == UINT32_MAX) { env->cpuid_xlevel2 = env->cpuid_min_xlevel2; } + + if (kvm_enabled()) { + kvm_hyperv_expand_features(cpu, errp); + } } /* diff --git a/target/i386/kvm/kvm-stub.c b/target/i386/kvm/kvm-stub.c index 92f49121b8..f6e7e4466e 100644 --- a/target/i386/kvm/kvm-stub.c +++ b/target/i386/kvm/kvm-stub.c @@ -39,3 +39,8 @@ bool kvm_hv_vpindex_settable(void) { return false; } + +bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp) +{ + abort(); +} diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index ef127762bc..556815db13 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -1220,13 +1220,22 @@ static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg) * of 'hv_passthrough' mode and fills the environment with all supported * Hyper-V features. */ -static bool hyperv_expand_features(CPUState *cs, Error **errp) +bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp) { - X86CPU *cpu = X86_CPU(cs); + CPUState *cs = CPU(cpu); if (!hyperv_enabled(cpu)) return true; + /* + * When kvm_hyperv_expand_features is called at CPU feature expansion + * time per-CPU kvm_state is not available yet so we can only proceed + * when KVM_CAP_SYS_HYPERV_CPUID is supported. + */ + if (!cs->kvm_state && + !kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID)) + return true; + if (cpu->hyperv_passthrough) { cpu->hyperv_vendor_id[0] = hv_cpuid_get_host(cs, HV_CPUID_VENDOR_AND_MAX_FUNCTIONS, R_EBX); @@ -1593,8 +1602,15 @@ int kvm_arch_init_vcpu(CPUState *cs) env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY; - /* Paravirtualization CPUIDs */ - if (!hyperv_expand_features(cs, &local_err)) { + /* + * kvm_hyperv_expand_features() is called here for the second time in case + * KVM_CAP_SYS_HYPERV_CPUID is not supported. While we can't possibly handle + * 'query-cpu-model-expansion' in this case as we don't have a KVM vCPU to + * check which Hyper-V enlightenments are supported and which are not, we + * can still proceed and check/expand Hyper-V enlightenments here so legacy + * behavior is preserved. + */ + if (!kvm_hyperv_expand_features(cpu, &local_err)) { error_report_err(local_err); return -ENOSYS; } diff --git a/target/i386/kvm/kvm_i386.h b/target/i386/kvm/kvm_i386.h index dc72508389..54667b35f0 100644 --- a/target/i386/kvm/kvm_i386.h +++ b/target/i386/kvm/kvm_i386.h @@ -47,6 +47,7 @@ bool kvm_has_x2apic_api(void); bool kvm_has_waitpkg(void); bool kvm_hv_vpindex_settable(void); +bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp); uint64_t kvm_swizzle_msi_ext_dest_id(uint64_t address); From 5ce48fa354f2270731e20f81dbb7ff191630c321 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 8 Jun 2021 14:08:14 +0200 Subject: [PATCH 05/11] i386: kill off hv_cpuid_check_and_set() hv_cpuid_check_and_set() does too much: - Checks if the feature is supported by KVM; - Checks if all dependencies are enabled; - Sets the feature bit in cpu->hyperv_features for 'passthrough' mode. To reduce the complexity, move all the logic except for dependencies check out of it. Also, in 'passthrough' mode we don't really need to check dependencies because KVM is supposed to provide a consistent set anyway. Reviewed-by: Eduardo Habkost Signed-off-by: Vitaly Kuznetsov Message-Id: <20210608120817.1325125-7-vkuznets@redhat.com> Signed-off-by: Eduardo Habkost --- target/i386/kvm/kvm.c | 104 +++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 68 deletions(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 556815db13..945d24300c 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -1148,16 +1148,12 @@ static bool hyperv_feature_supported(CPUState *cs, int feature) return true; } -static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp) +/* Checks that all feature dependencies are enabled */ +static bool hv_feature_check_deps(X86CPU *cpu, int feature, Error **errp) { - X86CPU *cpu = X86_CPU(cs); uint64_t deps; int dep_feat; - if (!hyperv_feat_enabled(cpu, feature) && !cpu->hyperv_passthrough) { - return 0; - } - deps = kvm_hyperv_properties[feature].dependencies; while (deps) { dep_feat = ctz64(deps); @@ -1165,26 +1161,12 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp) error_setg(errp, "Hyper-V %s requires Hyper-V %s", kvm_hyperv_properties[feature].desc, kvm_hyperv_properties[dep_feat].desc); - return 1; + return false; } deps &= ~(1ull << dep_feat); } - if (!hyperv_feature_supported(cs, feature)) { - if (hyperv_feat_enabled(cpu, feature)) { - error_setg(errp, "Hyper-V %s is not supported by kernel", - kvm_hyperv_properties[feature].desc); - return 1; - } else { - return 0; - } - } - - if (cpu->hyperv_passthrough) { - cpu->hyperv_features |= BIT(feature); - } - - return 0; + return true; } static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg) @@ -1223,6 +1205,8 @@ static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg) bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp) { CPUState *cs = CPU(cpu); + Error *local_err = NULL; + int feat; if (!hyperv_enabled(cpu)) return true; @@ -1278,53 +1262,37 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp) cpu->hyperv_spinlock_attempts = hv_cpuid_get_host(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EBX); - } - /* Features */ - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI, errp)) { - return false; - } - if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT, errp)) { - return false; + /* + * Mark feature as enabled in 'cpu->hyperv_features' as + * hv_build_cpuid_leaf() uses this info to build guest CPUIDs. + */ + for (feat = 0; feat < ARRAY_SIZE(kvm_hyperv_properties); feat++) { + if (hyperv_feature_supported(cs, feat)) { + cpu->hyperv_features |= BIT(feat); + } + } + } else { + /* Check features availability and dependencies */ + for (feat = 0; feat < ARRAY_SIZE(kvm_hyperv_properties); feat++) { + /* If the feature was not requested skip it. */ + if (!hyperv_feat_enabled(cpu, feat)) { + continue; + } + + /* Check if the feature is supported by KVM */ + if (!hyperv_feature_supported(cs, feat)) { + error_setg(errp, "Hyper-V %s is not supported by kernel", + kvm_hyperv_properties[feat].desc); + return false; + } + + /* Check dependencies */ + if (!hv_feature_check_deps(cpu, feat, &local_err)) { + error_propagate(errp, local_err); + return false; + } + } } /* Additional dependencies not covered by kvm_hyperv_properties[] */ From b26f68c36bf6edaaa224f8a6ab285394af4d0f8c Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 8 Jun 2021 14:08:15 +0200 Subject: [PATCH 06/11] i386: HV_HYPERCALL_AVAILABLE privilege bit is always needed According to TLFS, Hyper-V guest is supposed to check HV_HYPERCALL_AVAILABLE privilege bit before accessing HV_X64_MSR_GUEST_OS_ID/HV_X64_MSR_HYPERCALL MSRs but at least some Windows versions ignore that. As KVM is very permissive and allows accessing these MSRs unconditionally, no issue is observed. We may, however, want to tighten the checks eventually. Conforming to the spec is probably also a good idea. Enable HV_HYPERCALL_AVAILABLE bit unconditionally. Reviewed-by: Eduardo Habkost Signed-off-by: Vitaly Kuznetsov Message-Id: <20210608120817.1325125-8-vkuznets@redhat.com> Signed-off-by: Eduardo Habkost --- target/i386/kvm/kvm.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 945d24300c..eee1a6b46e 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -813,8 +813,6 @@ static struct { [HYPERV_FEAT_RELAXED] = { .desc = "relaxed timing (hv-relaxed)", .flags = { - {.func = HV_CPUID_FEATURES, .reg = R_EAX, - .bits = HV_HYPERCALL_AVAILABLE}, {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX, .bits = HV_RELAXED_TIMING_RECOMMENDED} } @@ -823,7 +821,7 @@ static struct { .desc = "virtual APIC (hv-vapic)", .flags = { {.func = HV_CPUID_FEATURES, .reg = R_EAX, - .bits = HV_HYPERCALL_AVAILABLE | HV_APIC_ACCESS_AVAILABLE}, + .bits = HV_APIC_ACCESS_AVAILABLE}, {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX, .bits = HV_APIC_ACCESS_RECOMMENDED} } @@ -832,8 +830,7 @@ static struct { .desc = "clocksources (hv-time)", .flags = { {.func = HV_CPUID_FEATURES, .reg = R_EAX, - .bits = HV_HYPERCALL_AVAILABLE | HV_TIME_REF_COUNT_AVAILABLE | - HV_REFERENCE_TSC_AVAILABLE} + .bits = HV_TIME_REF_COUNT_AVAILABLE | HV_REFERENCE_TSC_AVAILABLE} } }, [HYPERV_FEAT_CRASH] = { @@ -1346,6 +1343,9 @@ static int hyperv_fill_cpuids(CPUState *cs, c->ebx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EBX); c->edx = hv_build_cpuid_leaf(cs, HV_CPUID_FEATURES, R_EDX); + /* Unconditionally required with any Hyper-V enlightenment */ + c->eax |= HV_HYPERCALL_AVAILABLE; + /* Not exposed by KVM but needed to make CPU hotplug in Windows work */ c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE; From cce087f628c651e905f5e2097d9bb9f678689669 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 8 Jun 2021 14:08:16 +0200 Subject: [PATCH 07/11] i386: Hyper-V SynIC requires POST_MESSAGES/SIGNAL_EVENTS privileges When Hyper-V SynIC is enabled, we may need to allow Windows guests to make hypercalls (POST_MESSAGES/SIGNAL_EVENTS). No issue is currently observed because KVM is very permissive, allowing these hypercalls regarding of guest visible CPUid bits. Reviewed-by: Eduardo Habkost Signed-off-by: Vitaly Kuznetsov Message-Id: <20210608120817.1325125-9-vkuznets@redhat.com> Signed-off-by: Eduardo Habkost --- target/i386/kvm/hyperv-proto.h | 6 ++++++ target/i386/kvm/kvm.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h index e30d64b4ad..5fbb385cc1 100644 --- a/target/i386/kvm/hyperv-proto.h +++ b/target/i386/kvm/hyperv-proto.h @@ -38,6 +38,12 @@ #define HV_ACCESS_FREQUENCY_MSRS (1u << 11) #define HV_ACCESS_REENLIGHTENMENTS_CONTROL (1u << 13) +/* + * HV_CPUID_FEATURES.EBX bits + */ +#define HV_POST_MESSAGES (1u << 4) +#define HV_SIGNAL_EVENTS (1u << 5) + /* * HV_CPUID_FEATURES.EDX bits */ diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index eee1a6b46e..59ed8327ac 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -1346,6 +1346,12 @@ static int hyperv_fill_cpuids(CPUState *cs, /* Unconditionally required with any Hyper-V enlightenment */ c->eax |= HV_HYPERCALL_AVAILABLE; + /* SynIC and Vmbus devices require messages/signals hypercalls */ + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) && + !cpu->hyperv_synic_kvm_only) { + c->ebx |= HV_POST_MESSAGES | HV_SIGNAL_EVENTS; + } + /* Not exposed by KVM but needed to make CPU hotplug in Windows work */ c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE; From a7a0da844d299971bdbf99665bd63398668dde83 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Wed, 7 Jul 2021 19:36:23 -0500 Subject: [PATCH 08/11] target/i386: suppress CPUID leaves not defined by the CPU vendor Currently all built-in CPUs report cache information via CPUID leaves 2 and 4, but these have never been defined for AMD. In the case of SEV-SNP this can cause issues with CPUID enforcement. Address this by allowing CPU types to suppress these via a new "x-vendor-cpuid-only" CPU property, which is true by default, but switched off for older machine types to maintain compatibility. Cc: "Dr. David Alan Gilbert" Cc: Eduardo Habkost Cc: Richard Henderson Cc: Igor Mammedov Cc: zhenwei pi Suggested-by: Eduardo Habkost Signed-off-by: Michael Roth Message-Id: <20210708003623.18665-1-michael.roth@amd.com> Signed-off-by: Eduardo Habkost --- hw/i386/pc.c | 1 + target/i386/cpu.c | 6 ++++++ target/i386/cpu.h | 3 +++ 3 files changed, 10 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 8e1220db72..aa79c5e0e6 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = { { "qemu64" "-" TYPE_X86_CPU, "family", "6" }, { "qemu64" "-" TYPE_X86_CPU, "model", "6" }, { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" }, + { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" }, }; const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0); diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 46befde387..6b7043e425 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5155,6 +5155,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (cpu->cache_info_passthrough) { host_cpuid(index, 0, eax, ebx, ecx, edx); break; + } else if (cpu->vendor_cpuid_only && IS_AMD_CPU(env)) { + *eax = *ebx = *ecx = *edx = 0; + break; } *eax = 1; /* Number of CPUID[EAX=2] calls required */ *ebx = 0; @@ -5176,6 +5179,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if ((*eax & 31) && cs->nr_cores > 1) { *eax |= (cs->nr_cores - 1) << 26; } + } else if (cpu->vendor_cpuid_only && IS_AMD_CPU(env)) { + *eax = *ebx = *ecx = *edx = 0; } else { *eax = 0; switch (count) { @@ -6651,6 +6656,7 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level, true), DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor), DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true), + DEFINE_PROP_BOOL("x-vendor-cpuid-only", X86CPU, vendor_cpuid_only, true), DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false), DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true), DEFINE_PROP_BOOL("kvm-no-smi-migration", X86CPU, kvm_no_smi_migration, diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 8f3747dd28..950a991a71 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1748,6 +1748,9 @@ struct X86CPU { /* Enable auto level-increase for all CPUID leaves */ bool full_cpuid_auto_level; + /* Only advertise CPUID leaves defined by the vendor */ + bool vendor_cpuid_only; + /* Enable auto level-increase for Intel Processor Trace leave */ bool intel_pt_auto_level; From 760746ac533dd770a56340b1376e7b3fb1693562 Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Thu, 8 Jul 2021 12:06:41 -0500 Subject: [PATCH 09/11] target/i386: Fix cpuid level for AMD A AMD server typically has cpuid level 0x10(test on Rome/Milan), it should not be changed to 0x1f in multi-dies case. * to maintain compatibility with older machine types, only implement this change when the CPU's "x-vendor-cpuid-only" property is false Cc: "Dr. David Alan Gilbert" Cc: Eduardo Habkost Cc: Richard Henderson Cc: Igor Mammedov Cc: zhenwei pi Fixes: a94e1428991 (target/i386: Add CPUID.1F generation support for multi-dies PCMachine) Signed-off-by: zhenwei pi Signed-off-by: Michael Roth Message-Id: <20210708170641.49410-1-michael.roth@amd.com> Signed-off-by: Eduardo Habkost --- target/i386/cpu.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 6b7043e425..48b55ebd0a 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5950,8 +5950,15 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp) } } - /* CPU topology with multi-dies support requires CPUID[0x1F] */ - if (env->nr_dies > 1) { + /* + * Intel CPU topology with multi-dies support requires CPUID[0x1F]. + * For AMD Rome/Milan, cpuid level is 0x10, and guest OS should detect + * extended toplogy by leaf 0xB. Only adjust it for Intel CPU, unless + * cpu->vendor_cpuid_only has been unset for compatibility with older + * machine types. + */ + if ((env->nr_dies > 1) && + (IS_INTEL_CPU(env) || !cpu->vendor_cpuid_only)) { x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F); } From f74d339c86d4460f1d7a644e965170c03518a737 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 7 Jul 2021 15:40:29 +0200 Subject: [PATCH 10/11] numa: Report expected initiator When setting up NUMA with HMAT enabled there's a check performed in machine_set_cpu_numa_node() that reports an error when a NUMA node has a CPU but the node's initiator is not itself. The error message reported contains only the expected value and not the actual value (which is different because an error is being reported). Report both values in the error message. Signed-off-by: Michal Privoznik Reviewed-by: Igor Mammedov Reviewed-by: Pankaj Gupta Message-Id: Signed-off-by: Eduardo Habkost --- hw/core/machine.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 57c18f909a..6f59fb0b7f 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -728,7 +728,8 @@ void machine_set_cpu_numa_node(MachineState *machine, if ((numa_info[props->node_id].initiator < MAX_NODES) && (props->node_id != numa_info[props->node_id].initiator)) { error_setg(errp, "The initiator of CPU NUMA node %" PRId64 - " should be itself", props->node_id); + " should be itself (got %" PRIu16 ")", + props->node_id, numa_info[props->node_id].initiator); return; } numa_info[props->node_id].has_cpu = true; From 294aa0437b7f6a3e94653ef661310ef621859c87 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 7 Jul 2021 15:40:30 +0200 Subject: [PATCH 11/11] numa: Parse initiator= attribute before cpus= attribute When parsing cpus= attribute of -numa object couple of checks is performed, such as correct initiator setting (see the if() statement at the end of for() loop in machine_set_cpu_numa_node()). However, with the current code cpus= attribute is parsed before initiator= attribute and thus the check may fail even though it is not obvious why. But since parsing the initiator= attribute does not depend on the cpus= attribute we can swap the order of the two. It's fairly easy to reproduce with the following command line (snippet of an actual cmd line): -smp 4,sockets=4,cores=1,threads=1 \ -object '{"qom-type":"memory-backend-ram","id":"ram-node0","size":2147483648}' \ -numa node,nodeid=0,cpus=0-1,initiator=0,memdev=ram-node0 \ -object '{"qom-type":"memory-backend-ram","id":"ram-node1","size":2147483648}' \ -numa node,nodeid=1,cpus=2-3,initiator=1,memdev=ram-node1 \ -numa hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-latency,latency=5 \ -numa hmat-lb,initiator=0,target=0,hierarchy=first-level,data-type=access-latency,latency=10 \ -numa hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-latency,latency=5 \ -numa hmat-lb,initiator=1,target=1,hierarchy=first-level,data-type=access-latency,latency=10 \ -numa hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=204800K \ -numa hmat-lb,initiator=0,target=0,hierarchy=first-level,data-type=access-bandwidth,bandwidth=208896K \ -numa hmat-lb,initiator=1,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=204800K \ -numa hmat-lb,initiator=1,target=1,hierarchy=first-level,data-type=access-bandwidth,bandwidth=208896K \ -numa hmat-cache,node-id=0,size=10K,level=1,associativity=direct,policy=write-back,line=8 \ -numa hmat-cache,node-id=1,size=10K,level=1,associativity=direct,policy=write-back,line=8 \ Signed-off-by: Michal Privoznik Reviewed-by: Igor Mammedov Message-Id: Signed-off-by: Eduardo Habkost --- hw/core/numa.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/hw/core/numa.c b/hw/core/numa.c index 1058d3697b..510d096a88 100644 --- a/hw/core/numa.c +++ b/hw/core/numa.c @@ -88,6 +88,29 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, return; } + /* + * If not set the initiator, set it to MAX_NODES. And if + * HMAT is enabled and this node has no cpus, QEMU will raise error. + */ + numa_info[nodenr].initiator = MAX_NODES; + if (node->has_initiator) { + if (!ms->numa_state->hmat_enabled) { + error_setg(errp, "ACPI Heterogeneous Memory Attribute Table " + "(HMAT) is disabled, enable it with -machine hmat=on " + "before using any of hmat specific options"); + return; + } + + if (node->initiator >= MAX_NODES) { + error_report("The initiator id %" PRIu16 " expects an integer " + "between 0 and %d", node->initiator, + MAX_NODES - 1); + return; + } + + numa_info[nodenr].initiator = node->initiator; + } + for (cpus = node->cpus; cpus; cpus = cpus->next) { CpuInstanceProperties props; if (cpus->value >= max_cpus) { @@ -142,28 +165,6 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, numa_info[nodenr].node_memdev = MEMORY_BACKEND(o); } - /* - * If not set the initiator, set it to MAX_NODES. And if - * HMAT is enabled and this node has no cpus, QEMU will raise error. - */ - numa_info[nodenr].initiator = MAX_NODES; - if (node->has_initiator) { - if (!ms->numa_state->hmat_enabled) { - error_setg(errp, "ACPI Heterogeneous Memory Attribute Table " - "(HMAT) is disabled, enable it with -machine hmat=on " - "before using any of hmat specific options"); - return; - } - - if (node->initiator >= MAX_NODES) { - error_report("The initiator id %" PRIu16 " expects an integer " - "between 0 and %d", node->initiator, - MAX_NODES - 1); - return; - } - - numa_info[nodenr].initiator = node->initiator; - } numa_info[nodenr].present = true; max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1); ms->numa_state->num_nodes++;