From: Denis V. Lunev <[email protected]> Since Linux kernel commit de572491a975 ("KVM: x86: AMD's IBPB is not equivalent to Intel's IBPB") KVM offers spec-ctrl on AMD hosts only when the CPU enumerates IBPB_RET, which Zen 1-3 do not. A host-model guest started on such a CPU under an older kernel has spec-ctrl in its live definition. On a destination with the newer kernel QEMU filters the feature out and the incoming migration fails:
guest CPU doesn't match specification: missing features: spec-ctrl Restoring a save image or reverting a snapshot fails the same way, so the guest cannot be moved to the new kernel without a restart. Dropping the bit in libvirt costs the guest nothing beyond what KVM already withholds. KVM grants PRED_CMD for spec-ctrl or ibpb and SPEC_CTRL for spec-ctrl or any of ibrs, amd-stibp and amd-ssbd (kernel commit 39485ed95d6b), and checks the values written to SPEC_CTRL against the host, not the guest CPUID (kernel commit 841c2be09fe4). Both predate de572491a975. Since kernel commit e592ec657d84 KVM also ignores guest CPUID bits it does not support, so a host without IBPB_RET treats the guest as lacking spec-ctrl even if the bit stayed. QEMU drops the bit either way; libvirt only decides whether the guest survives. When an existing domain is started (incoming migration, restore, snapshot revert) and the hypervisor dropped a required spec-ctrl while providing ibpb and one of ibrs, amd-stibp and amd-ssbd, disable spec-ctrl in the live definition and log a warning. spec-ctrl is also removed from the features QEMU reports as filtered, because check='full' fails on any filtered feature whatever the definition says. Fresh starts and any other missing or extra feature still fail with check='full'. Signed-off-by: Denis V. Lunev <[email protected]> --- RHEL-149512 reports the same failure. Linux commit: https://git.kernel.org/torvalds/c/de572491a975 KVM thread: https://lore.kernel.org/all/[email protected]/ RHEL bug: https://redhat.atlassian.net/browse/RHEL-149512 src/cpu/cpu_x86.c | 55 ++++++++++++ src/cpu/cpu_x86.h | 4 + src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 15 +++- tests/cputest.c | 87 +++++++++++++++++++ ...PYC-7713-64-Core-no-spec-ctrl-disabled.xml | 4 + ...EPYC-7713-64-Core-no-spec-ctrl-enabled.xml | 11 +++ ...3-64-Core-no-spec-ctrl-no-ibpb-enabled.xml | 11 +++ 8 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-disabled.xml create mode 100644 tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-enabled.xml create mode 100644 tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-no-ibpb-enabled.xml diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 18c5756ef9..b986720224 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -3269,6 +3269,61 @@ virCPUx86UpdateLive(virCPUDef *cpu, } +/* + * Since kernel commit de572491a975 KVM no longer offers spec-ctrl on AMD + * hosts without IBPB_RET, while a guest started before still requires it. + * + * Returns 1 if spec-ctrl was disabled in @cpu, 0 if not, -1 on error. + */ +int +virCPUx86UpdateLiveSpecCtrl(virCPUDef *cpu, + const virCPUData *dataEnabled, + virCPUData *dataDisabled) +{ + const char *specCtrlMSR[] = { "ibrs", "amd-stibp", "amd-ssbd" }; + virCPUx86Map *map; + virCPUx86Feature *specCtrl; + virCPUFeatureDef *feature; + const virCPUx86Data *enabled; + size_t i; + int rc; + + if (!dataEnabled) + return 0; + + if (!(feature = virCPUDefFindFeature(cpu, "spec-ctrl")) || + feature->policy != VIR_CPU_FEATURE_REQUIRE) + return 0; + + if (!(map = virCPUx86GetMap())) + return -1; + + enabled = &dataEnabled->data.x86; + + if ((rc = x86FeatureInData("spec-ctrl", enabled, map)) != 0) + return rc < 0 ? -1 : 0; + + if ((rc = x86FeatureInData("ibpb", enabled, map)) <= 0) + return rc; + + for (i = 0; i < G_N_ELEMENTS(specCtrlMSR); i++) { + if ((rc = x86FeatureInData(specCtrlMSR[i], enabled, map)) < 0) + return -1; + if (rc == 1) + break; + } + + if (i == G_N_ELEMENTS(specCtrlMSR)) + return 0; + + virCPUDefUpdateFeature(cpu, "spec-ctrl", VIR_CPU_FEATURE_DISABLE); + if (dataDisabled && (specCtrl = x86FeatureFind(map, "spec-ctrl"))) + x86DataSubtract(&dataDisabled->data.x86, &specCtrl->data); + + return 1; +} + + static int virCPUx86GetModels(char ***models) { diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h index bbc2a16447..0b05e5c695 100644 --- a/src/cpu/cpu_x86.h +++ b/src/cpu/cpu_x86.h @@ -55,3 +55,7 @@ bool virCPUx86FeatureFilterDropMSR(const char *name, int virCPUx86GetAddedFeatures(const char *modelName, const char * const **features); + +int virCPUx86UpdateLiveSpecCtrl(virCPUDef *cpu, + const virCPUData *dataEnabled, + virCPUData *dataDisabled); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 798d2d28c8..6b2eb3eef4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1608,6 +1608,7 @@ virCPUx86DataSetVendor; virCPUx86FeatureFilterDropMSR; virCPUx86FeatureFilterSelectMSR; virCPUx86GetAddedFeatures; +virCPUx86UpdateLiveSpecCtrl; # datatypes.h virConnectClass; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3f1960f314..9eee738334 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4799,10 +4799,12 @@ qemuProcessUpdateLiveGuestCPU(virDomainObj *vm, static int qemuProcessUpdateAndVerifyCPU(virDomainObj *vm, - virDomainAsyncJob asyncJob) + virDomainAsyncJob asyncJob, + unsigned int flags) { g_autoptr(virCPUData) cpu = NULL; g_autoptr(virCPUData) disabled = NULL; + int rc; if (qemuProcessFetchGuestCPU(vm, asyncJob, &cpu, &disabled) < 0) return -1; @@ -4810,6 +4812,15 @@ qemuProcessUpdateAndVerifyCPU(virDomainObj *vm, if (qemuProcessVerifyCPU(vm, cpu) < 0) return -1; + if (!(flags & VIR_QEMU_PROCESS_START_NEW) && + ARCH_IS_X86(vm->def->os.arch) && vm->def->cpu) { + if ((rc = virCPUx86UpdateLiveSpecCtrl(vm->def->cpu, cpu, disabled)) < 0) + return -1; + if (rc > 0) + VIR_WARN("Hypervisor no longer provides spec-ctrl, disabling it in the live CPU of domain %s", + vm->def->name); + } + if (qemuProcessUpdateLiveGuestCPU(vm, cpu, disabled) < 0) return -1; @@ -8639,7 +8650,7 @@ qemuProcessLaunch(virConnectPtr conn, } VIR_DEBUG("Verifying and updating provided guest CPU"); - if (qemuProcessUpdateAndVerifyCPU(vm, asyncJob) < 0) + if (qemuProcessUpdateAndVerifyCPU(vm, asyncJob, flags) < 0) goto cleanup; VIR_DEBUG("Detecting IOThread PIDs"); diff --git a/tests/cputest.c b/tests/cputest.c index e29c81cb8e..38ed45cfdb 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -905,6 +905,91 @@ cpuTestUpdateLive(const void *arg) } +static virCPUData * +cpuTestLoadCPUData(const struct data *data, + const char *suffix) +{ + g_autofree char *file = NULL; + g_autofree char *xml = NULL; + + file = g_strdup_printf("%s/cputestdata/%s-cpuid-%s-%s.xml", + abs_srcdir, virArchToString(data->arch), + data->host, suffix); + virTestCaseMarkUsed(data->existingTestCases, file); + + if (virTestLoadFile(file, &xml) < 0) + return NULL; + + return virCPUDataParse(xml); +} + + +static int +cpuTestUpdateLiveSpecCtrl(const void *arg) +{ + const struct data *data = arg; + g_autofree char *cpuFile = NULL; + g_autoptr(virCPUDef) cpu = NULL; + g_autoptr(virCPUDef) fresh = NULL; + g_autoptr(virCPUData) srcEnabled = NULL; + g_autoptr(virCPUData) srcDisabled = NULL; + g_autoptr(virCPUData) enabled = NULL; + g_autoptr(virCPUData) disabled = NULL; + g_autoptr(virCPUData) noIbpb = NULL; + virCPUFeatureDef *feature; + + cpuFile = g_strdup_printf("cpuid-%s-json", data->host); + if (!(cpu = cpuTestLoadXML(data->arch, cpuFile, data->existingTestCases))) + return -1; + + if (!(srcEnabled = cpuTestLoadCPUData(data, "enabled")) || + !(srcDisabled = cpuTestLoadCPUData(data, "disabled")) || + !(enabled = cpuTestLoadCPUData(data, "no-spec-ctrl-enabled")) || + !(disabled = cpuTestLoadCPUData(data, "no-spec-ctrl-disabled")) || + !(noIbpb = cpuTestLoadCPUData(data, "no-spec-ctrl-no-ibpb-enabled"))) + return -1; + + /* the live CPU of a guest started where KVM still offered spec-ctrl */ + if (virCPUUpdateLive(data->arch, cpu, srcEnabled, srcDisabled) < 0) + return -1; + cpu->check = VIR_CPU_CHECK_FULL; + + fresh = virCPUDefCopy(cpu); + + if (virCPUx86UpdateLiveSpecCtrl(fresh, srcEnabled, srcDisabled) != 0) { + VIR_TEST_VERBOSE("spec-ctrl touched although the hypervisor provides it"); + return -1; + } + + if (virCPUx86UpdateLiveSpecCtrl(fresh, noIbpb, disabled) != 0) { + VIR_TEST_VERBOSE("spec-ctrl disabled although ibpb is missing"); + return -1; + } + + if (virCPUUpdateLive(data->arch, fresh, enabled, disabled) == 0) { + VIR_TEST_VERBOSE("missing spec-ctrl passed check='full'"); + return -1; + } + virResetLastError(); + + if (virCPUx86UpdateLiveSpecCtrl(cpu, enabled, disabled) != 1) { + VIR_TEST_VERBOSE("spec-ctrl was not disabled"); + return -1; + } + + if (virCPUUpdateLive(data->arch, cpu, enabled, disabled) < 0) + return -1; + + if (!(feature = virCPUDefFindFeature(cpu, "spec-ctrl")) || + feature->policy != VIR_CPU_FEATURE_DISABLE) { + VIR_TEST_VERBOSE("spec-ctrl is not disabled in the live CPU"); + return -1; + } + + return 0; +} + + #if WITH_QEMU static int cpuTestJSONCPUID(const void *arg) @@ -1270,6 +1355,8 @@ mymain(void) DO_TEST_CPUID(VIR_ARCH_X86_64, "EPYC-7601-32-Core-ibpb", JSON_MODELS_REQUIRED); DO_TEST_CPUID(VIR_ARCH_X86_64, "EPYC-7502-32-Core", JSON_MODELS); DO_TEST_CPUID(VIR_ARCH_X86_64, "EPYC-7713-64-Core", JSON_MODELS); + DO_TEST(VIR_ARCH_X86_64, cpuTestUpdateLiveSpecCtrl, "EPYC-7713-64-Core", + "EPYC-7713-64-Core", NULL, NULL, 0, NULL, 0, 0); DO_TEST_CPUID(VIR_ARCH_X86_64, "EPYC-9334-32-Core", JSON_MODELS); DO_TEST_CPUID(VIR_ARCH_X86_64, "FX-8150", JSON_NONE); DO_TEST_CPUID(VIR_ARCH_X86_64, "Opteron-1352", JSON_NONE); diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-disabled.xml b/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-disabled.xml new file mode 100644 index 0000000000..d29d62bcad --- /dev/null +++ b/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-disabled.xml @@ -0,0 +1,4 @@ +<!-- Features disabled by QEMU --> +<cpudata arch='x86'> + <cpuid eax_in='0x00000007' ecx_in='0x00' eax='0x00000000' ebx='0x00000000' ecx='0x00000000' edx='0x04000000'/> +</cpudata> diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-enabled.xml b/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-enabled.xml new file mode 100644 index 0000000000..6c7b14cd6f --- /dev/null +++ b/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-enabled.xml @@ -0,0 +1,11 @@ +<!-- Features enabled by QEMU --> +<cpudata arch='x86'> + <cpuid eax_in='0x00000001' ecx_in='0x00' eax='0x00000000' ebx='0x00000000' ecx='0xf7fa3203' edx='0x078bfbff'/> + <cpuid eax_in='0x00000006' ecx_in='0x00' eax='0x00000004' ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/> + <cpuid eax_in='0x00000007' ecx_in='0x00' eax='0x00000000' ebx='0x219c07ab' ecx='0x0040060c' edx='0xa8000010'/> + <cpuid eax_in='0x0000000d' ecx_in='0x01' eax='0x0000000f' ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/> + <cpuid eax_in='0x80000001' ecx_in='0x00' eax='0x00000000' ebx='0x00000000' ecx='0x008003f7' edx='0x2e500800'/> + <cpuid eax_in='0x80000008' ecx_in='0x00' eax='0x00000000' ebx='0x03009205' ecx='0x00000000' edx='0x00000000'/> + <cpuid eax_in='0x8000000a' ecx_in='0x00' eax='0x00000000' ebx='0x00000000' ecx='0x00000000' edx='0x00000009'/> + <msr index='0x10a' edx='0x00000000' eax='0x00000069'/> +</cpudata> diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-no-ibpb-enabled.xml b/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-no-ibpb-enabled.xml new file mode 100644 index 0000000000..0662433a08 --- /dev/null +++ b/tests/cputestdata/x86_64-cpuid-EPYC-7713-64-Core-no-spec-ctrl-no-ibpb-enabled.xml @@ -0,0 +1,11 @@ +<!-- Features enabled by QEMU --> +<cpudata arch='x86'> + <cpuid eax_in='0x00000001' ecx_in='0x00' eax='0x00000000' ebx='0x00000000' ecx='0xf7fa3203' edx='0x078bfbff'/> + <cpuid eax_in='0x00000006' ecx_in='0x00' eax='0x00000004' ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/> + <cpuid eax_in='0x00000007' ecx_in='0x00' eax='0x00000000' ebx='0x219c07ab' ecx='0x0040060c' edx='0xa8000010'/> + <cpuid eax_in='0x0000000d' ecx_in='0x01' eax='0x0000000f' ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/> + <cpuid eax_in='0x80000001' ecx_in='0x00' eax='0x00000000' ebx='0x00000000' ecx='0x008003f7' edx='0x2e500800'/> + <cpuid eax_in='0x80000008' ecx_in='0x00' eax='0x00000000' ebx='0x03008205' ecx='0x00000000' edx='0x00000000'/> + <cpuid eax_in='0x8000000a' ecx_in='0x00' eax='0x00000000' ebx='0x00000000' ecx='0x00000000' edx='0x00000009'/> + <msr index='0x10a' edx='0x00000000' eax='0x00000069'/> +</cpudata> base-commit: 742c1555b7d439ca8edca6297093c74e2c20a913 -- 2.53.0
