Re: [PATCH v5 03/26] x86/hyperv: Update 'struct hv_enlightened_vmcs' definition
From: Sean Christopherson <seanjc@google.com>
Date: 2022-08-23 17:27:05
Also in:
kvm, lkml
On Tue, Aug 23, 2022, Vitaly Kuznetsov wrote:
Sean Christopherson [off-list ref] writes:quoted
On Mon, Aug 22, 2022, Vitaly Kuznetsov wrote:quoted
QEMU's migration depends on the assumption that identical QEMU's command lines create identical (from guest PoV) configurations. Assume we have (simplified) "-cpu CascadeLake-Sever,hv-evmcs" on both source and destination but source host is newer, i.e. its KVM knows about TSC Scaling in eVMCS and destination host has no idea about it. If we just apply filtering upon vCPU creation, guest visible MSR values are going to be different, right? Ok, assuming QEMU also migrates VMX feature MSRs (TODO: check if that's true), we will be able to fail mirgration late (which is already much worse than not being able to create the desired configuration on destination, 'fail early') if we use in-KVM filtering to throw an error to userspace. But if we blindly filter control MSRs on the destination, 'TscScaling' will just disapper undreneath the guest. This is unlikely to work.But all of that holds true irrespetive of eVMCS. If QEMU attempts to migrate a nested guest from a KVM that supports TSC_SCALING to a KVM that doesn't support TSC_SCALING, then TSC_SCALING is going to disappear and VM-Entry on the dest will fail. I.e. QEMU _must_ be able to detect the incompatibility and not attempt the migration. And with that code in place, QEMU doesn't need to do anything new for eVMCS, it Just Works.I'm obviously missing something. "-cpu CascadeLake-Sever" presumes cetain features, including VMX features (e.g. TSC_SCALING), an attempt to create such vCPU on a CPU which doesn't support it will lead to immediate failure. So two VMs created on different hosts with -cpu CascadeLake-Sever are guaranteed to look exactly the same from guest PoV. This is not true for '-cpu CascadeLake-Sever,hv-evmcs' (if we do it the way you suggest) as 'hv-evmcs' will be a *different* filter on each host (which is going to depend on KVM version, not even on the host's hardware).
We're talking about nested VMX, i.e. exposing TSC_SCALING to L1. QEMU's CLX
definition doesn't include TSC_SCALING. In fact, none of QEMU's predefined CPU
models supports TSC_SCALING, precisely because KVM didn't support exposing the
feature to L1 until relatively recently.
$ git grep VMX_SECONDARY_EXEC_TSC_SCALING
target/i386/cpu.h:#define VMX_SECONDARY_EXEC_TSC_SCALING 0x02000000
target/i386/kvm/kvm.c: if (f[FEAT_VMX_SECONDARY_CTLS] & VMX_SECONDARY_EXEC_TSC_SCALING) {
quoted
quoted
In any case, what we need, is an option for VMM (read: QEMU) to create the configuration with 'TscScaling' filtered out even KVM supports the bit in eVMCS. This way the guest will be able to migrate backwards to an older KVM which doesn't support it, i.e. '-cpu CascadeLake-Sever,hv-evmcs' creates the 'origin' eVMCS configuration, no TscScaling '-cpu CascadeLake-Sever,hv-evmcs,hv-evmcs-2022' creates the updated one.
Ah, I see what you're worried about. Your concern is that QEMU will add a VMX
feature to a predefined CPU model, but only later gain eVMCS support, and so
"CascadeLake-Server,hv-evmcs" will do different things depending on the KVM
version.
But again, that's already reality. Run "-cpu CascadeLake-Server" against a KVM
from before commits:
28c1c9fabf48 ("KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES")
1eaafe91a0df ("kvm: x86: IA32_ARCH_CAPABILITIES is always supported")
and it will fail. There are undoubtedly many other features that are similarly
affected, just go back far enough in KVM time.
Or simply run "-cpu CascadeLake-Server" on pre-CLX hardware. Anything that KVM
doesn't fully emulate will not be present.
quoted
Again, this conundrum exists irrespective of eVMCS. Properly solve the problem for regular nVMX and eVMCS should naturally work.I don't think we have this problem for VMX features as named CPU models in QEMU encode all of them explicitly, they *must* be present whenever such vCPU is created.
Yes, and if KVM doesn't support features that CascadeLake-Server requires, spawning the VM will fail on the destination, as it should. My point is that this behavior is not unique to eVMCS. QEMU/Libvirt must also be prepared for rejection, because it is flat out impossible to ensure that KVM+hardware supports a specific feature.
quoted
quoted
KVM_CAP_HYPERV_ENLIGHTENED_VMCS is bad as it only takes 'eVMCS' version as a parameter (as we assumed it will always change when new fields are added, but that turned out to be false). That's why I suggested KVM_CAP_HYPERV_ENLIGHTENED_VMCS2.Enumerating features via versions is such a bad API though, e.g. if there's a bug with nested TSC_SCALING, userspace can't disable just nested TSC_SCALING without everything else under the inscrutable "hv-evmcs-2022" being collateral damage.Why? Something like "-cpu CascadeLake-Sever,hv-evmcs,hv-evmcs-2022,-vmx-tsc-scaling" should work well, no? 'hv-evmcs*' are just filters, if the VMX feature is not there -- it's not there.
Because it's completely unnecessary, adds non-trivial maintenance burden to KVM, and requires explicit documentation to explain to userspace what "hv-evmcs-2022" means. It's unnecessary because if the user is concerned about eVMCS features showing up in the future, then they should do: -cpu CascadeLake-Server,hv-evmcs,-vmx-tsc-scaling,-<any other VMX features not eVMCS-friendly> If QEMU wants to make that more user friendly, then define CascadeLake-Server-eVMCS or whatever so that the features that are unlikely be supported for eVMCS are off by default. This is no different than QEMU not including nested TSC_SCALING in any of the predefined models; the developers _know_ KVM doesn't widely support TSC_SCALING, so it was omitted, even though a real CLX CPU is guaranteed to support TSC_SCALING. It's non-trivial maintenance for KVM because it would require defining new versions every time an eVMCS field is added, allowing userspace to specify and restrict features based on arbitrary versions, and do all of that without conflicting with whatever PV enumeration Microsoft adds.