On Mon, Oct 26, 2026 at 20:06:54 +0100, Denis V. Lunev wrote:
> 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;
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
