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