[PATCH] qemu: let incoming AMD guests drop spec-ctrl instead of failing
From: Denis V. Lunev <den@openvz.org> 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 <den@openvz.org> --- RHEL-149512 reports the same failure. Linux commit: https://git.kernel.org/torvalds/c/de572491a975 KVM thread: https://lore.kernel.org/all/20241011214353.1625057-5-jmattson@google.com/ 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
On Mon, Oct 26, 2026 at 20:06:54 +0100, Denis V. Lunev wrote:
From: Denis V. Lunev <den@openvz.org>
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 <den@openvz.org> --- RHEL-149512 reports the same failure.
Linux commit: https://git.kernel.org/torvalds/c/de572491a975 KVM thread: https://lore.kernel.org/all/20241011214353.1625057-5-jmattson@google.com/ RHEL bug: https://redhat.atlassian.net/browse/RHEL-149512
So in other words.. The spec-ctrl feature (bit 26 of the main sub leaf of CPUID leaf 0x7) says whether the CPU supports both IA32_PRED_CMD and IA32_SPEC_CTRL MSRs. So when migrating to a host which does not report spec-ctrl, we must make sure the two MSRs are still available by checking other features. The following is the KVM code for deciding when these two MSRs are available static inline bool guest_has_spec_ctrl_msr(struct kvm_vcpu *vcpu) { return (guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_STIBP) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD)); } static inline bool guest_has_pred_cmd_msr(struct kvm_vcpu *vcpu) { return (guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB)); } Which confirms your claim that the CPU must provide ibpb and either ibrs, amd-stibp, or amd-ssbd to support both MSRs. In which case we can safely drop the requirement for spec-ctrl as a guest using these MSRs based on previous spec-ctrl check will keep working. That said I agree with the idea of this patch. I'll look at the actual code with a fresh mind next week :-) Jirka
On 10/30/26 17:06, Jiri Denemark wrote:
From: Denis V. Lunev <den@openvz.org>
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 <den@openvz.org> --- RHEL-149512 reports the same failure.
Linux commit: https://git.kernel.org/torvalds/c/de572491a975 KVM thread: https://lore.kernel.org/all/20241011214353.1625057-5-jmattson@google.com/ RHEL bug: https://redhat.atlassian.net/browse/RHEL-149512 So in other words.. The spec-ctrl feature (bit 26 of the main sub leaf of CPUID leaf 0x7) says whether the CPU supports both IA32_PRED_CMD and IA32_SPEC_CTRL MSRs. So when migrating to a host which does not report spec-ctrl, we must make sure the two MSRs are still available by checking other features. The following is the KVM code for deciding when
On Mon, Oct 26, 2026 at 20:06:54 +0100, Denis V. Lunev wrote: these two MSRs are available
static inline bool guest_has_spec_ctrl_msr(struct kvm_vcpu *vcpu) { return (guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_STIBP) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD)); }
static inline bool guest_has_pred_cmd_msr(struct kvm_vcpu *vcpu) { return (guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) || guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB)); }
Which confirms your claim that the CPU must provide ibpb and either ibrs, amd-stibp, or amd-ssbd to support both MSRs. In which case we can safely drop the requirement for spec-ctrl as a guest using these MSRs based on previous spec-ctrl check will keep working.
That said I agree with the idea of this patch. I'll look at the actual code with a fresh mind next week :-)
Jirka Actually there are more issues on the same transition.
bus-lock-detect on never Zen and v-vmsave-vmload which are also safe to ignore but let us come to the decision first here and decide on followup stuff later on. Den
On Mon, Oct 26, 2026 at 20:06:54 +0100, Denis V. Lunev wrote:
From: Denis V. Lunev <den@openvz.org>
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 <den@openvz.org> --- RHEL-149512 reports the same failure.
Linux commit: https://git.kernel.org/torvalds/c/de572491a975 KVM thread: https://lore.kernel.org/all/20241011214353.1625057-5-jmattson@google.com/ 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;
This will become redundant if the function is called from a different place (see below).
+ + 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;
I think it would be a bit more readable without the ternary operator, e.g.: if ((rc = ...) < 0) return -1; if (rc == 1) return 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;
Instead of breaking and than checking whether the for loop did ended early we could just update the CPU here and return.
+ } + + if (i == G_N_ELEMENTS(specCtrlMSR)) + return 0; +
This code is making the changes so it should log a message about it rather than leaving it up to the caller
+ 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) &&
The more I think about this the more I'm leaning towards doing this update even for freshly started domains. Imagine you have defined domains which require spec-ctrl and then update the host to the version that drops spec-ctrl. You'd have to update all affected domain XMLs to be able to start them for no real reason.
+ 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);
This VIR_WARN should be moved inside virCPUx86UpdateLiveSpecCtrl. And since warnings are not really visible to end users, I think VIR_INFO would be good enough.
+ } +
Anyway, the call to virCPUx86UpdateLiveSpecCtrl should be moved inside virCPUUpdateLive.
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
I believe that calling virCPUx86UpdateLiveSpecCtrl from virCPUUpdateLive would limit the required cputest modifications to test cases. Jirka
participants (3)
-
Denis V. Lunev -
Denis V. Lunev -
Jiri Denemark