KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active

Extend KVM's restriction on CPUID and feature MSR changes to disallow
updates while L2 is active in addition to rejecting updates after the vCPU
has run at least once.  Like post-run vCPU model updates, attempting to
react to model changes while L2 is active is practically infeasible, e.g.
KVM would need to do _something_ in response to impossible situations where
userspace has a removed a feature that was consumed as parted of nested
VM-Enter.

In practice, disallowing vCPU model changes while L2 is active is largely
uninteresting, as the only way for L2 to be active without the vCPU having
run at least once is if userspace stuffed state via KVM_SET_NESTED_STATE.
And because KVM_SET_NESTED_STATE can't put the vCPU into L2 without
userspace first defining the vCPU model, e.g. to enable SVM/VMX, modifying
the vCPU model while L2 is active would require deliberately setting the
vCPU model, then loading nested state, and then changing the model.  I.e.
no sane VMM should run afoul of the new restriction, and any VMM that does
encounter problems has likely been running a broken setup for a long time.

Cc: Yosry Ahmed <yosry.ahmed@linux.dev>
Cc: Kevin Cheng <chengkev@google.com>
Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>
Link: https://patch.msgid.link/20251230205641.4092235-1-seanjc@google.com
Signed-off-by: Sean Christopherson <seanjc@google.com>

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 88a5426674a10bd689c1788b0829f3587e3ca1c5..f37331ad3ad859ae0b160ab0bc327ef757be5c7f 100644 (file)
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -534,17 +534,20 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,
        BUILD_BUG_ON(sizeof(vcpu_caps) != sizeof(vcpu->arch.cpu_caps));
 
        /*
-        * KVM does not correctly handle changing guest CPUID after KVM_RUN, as
-        * MAXPHYADDR, GBPAGES support, AMD reserved bit behavior, etc.. aren't
-        * tracked in kvm_mmu_page_role.  As a result, KVM may miss guest page
-        * faults due to reusing SPs/SPTEs. In practice no sane VMM mucks with
-        * the core vCPU model on the fly. It would've been better to forbid any
-        * KVM_SET_CPUID{,2} calls after KVM_RUN altogether but unfortunately
-        * some VMMs (e.g. QEMU) reuse vCPU fds for CPU hotplug/unplug and do
+        * KVM does not correctly handle changing guest CPUID after KVM_RUN or
+        * while L2 is active, as MAXPHYADDR, GBPAGES support, AMD reserved bit
+        * behavior, etc. aren't tracked in kvm_mmu_page_role, and L2 state
+        * can't be adjusted (without breaking L2 in some way).  As a result,
+        * KVM may reuse SPs/SPTEs and/or run L2 with bad/misconfigured state.
+        *
+        * In practice, no sane VMM mucks with the core vCPU model on the fly.
+        * It would've been better to forbid any KVM_SET_CPUID{,2} calls after
+        * KVM_RUN or KVM_SET_NESTED_STATE altogether, but unfortunately some
+        * VMMs (e.g. QEMU) reuse vCPU fds for CPU hotplug/unplug and do
         * KVM_SET_CPUID{,2} again. To support this legacy behavior, check
         * whether the supplied CPUID data is equal to what's already set.
         */
-       if (kvm_vcpu_has_run(vcpu)) {
+       if (!kvm_can_set_cpuid_and_feature_msrs(vcpu)) {
                r = kvm_cpuid_check_equal(vcpu, e2, nent);
                if (r)
                        goto err;

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 02c450686b4a8a6a1e2878fec6ab7ecb936baa62..f173245469005808c460be4064cc8276e730f8d8 100644 (file)
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6031,11 +6031,7 @@ void kvm_mmu_after_set_cpuid(struct kvm_vcpu *vcpu)
        vcpu->arch.nested_mmu.cpu_role.ext.valid = 0;
        kvm_mmu_reset_context(vcpu);
 
-       /*
-        * Changing guest CPUID after KVM_RUN is forbidden, see the comment in
-        * kvm_arch_vcpu_ioctl().
-        */
-       KVM_BUG_ON(kvm_vcpu_has_run(vcpu), vcpu->kvm);
+       KVM_BUG_ON(!kvm_can_set_cpuid_and_feature_msrs(vcpu), vcpu->kvm);
 }
 
 void kvm_mmu_reset_context(struct kvm_vcpu *vcpu)

diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 487ad19a236e30fe2a8cb61c067036ce6f9f892b..ff20b4102173ae511865ada2f0e7fbb8e52603d7 100644 (file)
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -853,7 +853,7 @@ void kvm_pmu_refresh(struct kvm_vcpu *vcpu)
 {
        struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
 
-       if (KVM_BUG_ON(kvm_vcpu_has_run(vcpu), vcpu->kvm))
+       if (KVM_BUG_ON(!kvm_can_set_cpuid_and_feature_msrs(vcpu), vcpu->kvm))
                return;
 
        /*

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff8812f3a1293e6e4f0d39a82ee32bfe086b55e6..211d8c24a4b11453a0f1e4e23b4173d5d160f1c9 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2314,13 +2314,14 @@ static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
        u64 val;
 
        /*
-        * Disallow writes to immutable feature MSRs after KVM_RUN.  KVM does
-        * not support modifying the guest vCPU model on the fly, e.g. changing
-        * the nVMX capabilities while L2 is running is nonsensical.  Allow
-        * writes of the same value, e.g. to allow userspace to blindly stuff
-        * all MSRs when emulating RESET.
-        */
-       if (kvm_vcpu_has_run(vcpu) && kvm_is_immutable_feature_msr(index) &&
+        * Reject writes to immutable feature MSRs if the vCPU model is frozen,
+        * as KVM doesn't support modifying the guest vCPU model on the fly,
+        * e.g. changing the VMX capabilities MSRs while L2 is active is
+        * nonsensical.  Allow writes of the same value, e.g. so that userspace
+        * can blindly stuff all MSRs when emulating RESET.
+        */
+       if (!kvm_can_set_cpuid_and_feature_msrs(vcpu) &&
+           kvm_is_immutable_feature_msr(index) &&
            (do_get_msr(vcpu, index, &val) || *data != val))
                return -EINVAL;
 

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index fdab0ad490988e297a370c85bf4fa54b3dd5ee68..5cee34f6a0e064c0e11d77201a892452d7f9ee78 100644 (file)
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -172,9 +172,20 @@ static inline void kvm_nested_vmexit_handle_ibrs(struct kvm_vcpu *vcpu)
                indirect_branch_prediction_barrier();
 }
 
-static inline bool kvm_vcpu_has_run(struct kvm_vcpu *vcpu)
+/*
+ * Disallow modifying CPUID and feature MSRs, which affect the core virtual CPU
+ * model exposed to the guest and virtualized by KVM, if the vCPU has already
+ * run or is in guest mode (L2).  In both cases, KVM has already consumed the
+ * current virtual CPU model, and doesn't support "unwinding" to react to the
+ * new model.
+ *
+ * Note, the only way is_guest_mode() can be true with 'last_vmentry_cpu == -1'
+ * is if userspace sets CPUID and feature MSRs (to enable VMX/SVM), then sets
+ * nested state, and then attempts to set CPUID and/or feature MSRs *again*.
+ */
+static inline bool kvm_can_set_cpuid_and_feature_msrs(struct kvm_vcpu *vcpu)
 {
-       return vcpu->arch.last_vmentry_cpu != -1;
+       return vcpu->arch.last_vmentry_cpu == -1 && !is_guest_mode(vcpu);
 }
 
 static inline void kvm_set_mp_state(struct kvm_vcpu *vcpu, int mp_state)