* Re: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
2025-12-30 20:56 [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active Sean Christopherson
@ 2026-01-02 18:00 ` Yosry Ahmed
2026-01-07 20:26 ` Sean Christopherson
2026-01-07 20:46 ` Yosry Ahmed
2026-01-12 17:38 ` Sean Christopherson
2 siblings, 1 reply; 8+ messages in thread
From: Yosry Ahmed @ 2026-01-02 18:00 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel, Kevin Cheng
On Tue, Dec 30, 2025 at 12:56:41PM -0800, Sean Christopherson wrote:
> 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>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
I can't speak to the policy change, but the code looks correct (with a
minor nit below):
Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
> arch/x86/kvm/cpuid.c | 19 +++++++++++--------
> arch/x86/kvm/mmu/mmu.c | 6 +-----
> arch/x86/kvm/pmu.c | 2 +-
> arch/x86/kvm/x86.c | 13 +++++++------
> arch/x86/kvm/x86.h | 4 ++--
> 5 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 88a5426674a1..f37331ad3ad8 100644
> --- 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 02c450686b4a..f17324546900 100644
> --- 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 487ad19a236e..ff20b4102173 100644
> --- 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 ff8812f3a129..211d8c24a4b1 100644
> --- 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.
> + * 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_vcpu_has_run(vcpu) && kvm_is_immutable_feature_msr(index) &&
> + 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 fdab0ad49098..9084e0dfa15c 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -172,9 +172,9 @@ 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)
> +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);
> }
To make this self-contained (e.g. for readers not coming from
kvm_set_cpuid()), should we add a comment here about is_guest_mode()
only possibly being true with last_vmentry_cpu == -1 if userspace does
the set CPUID, set nested state, set CPUID again dance?
>
> static inline void kvm_set_mp_state(struct kvm_vcpu *vcpu, int mp_state)
>
> base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
> --
> 2.52.0.351.gbe84eed79e-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
2026-01-02 18:00 ` Yosry Ahmed
@ 2026-01-07 20:26 ` Sean Christopherson
2026-01-08 7:59 ` Yosry Ahmed
0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-01-07 20:26 UTC (permalink / raw)
To: Yosry Ahmed; +Cc: Paolo Bonzini, kvm, linux-kernel, Kevin Cheng
On Fri, Jan 02, 2026, Yosry Ahmed wrote:
> On Tue, Dec 30, 2025 at 12:56:41PM -0800, Sean Christopherson wrote:
> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > index fdab0ad49098..9084e0dfa15c 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -172,9 +172,9 @@ 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)
> > +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);
> > }
>
> To make this self-contained (e.g. for readers not coming from
> kvm_set_cpuid()), should we add a comment here about is_guest_mode()
> only possibly being true with last_vmentry_cpu == -1 if userspace does
> the set CPUID, set nested state, set CPUID again dance?
Ya. If this looks good, I'll add it when applying.
/*
* 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 && !is_guest_mode(vcpu);
}
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
2026-01-07 20:26 ` Sean Christopherson
@ 2026-01-08 7:59 ` Yosry Ahmed
0 siblings, 0 replies; 8+ messages in thread
From: Yosry Ahmed @ 2026-01-08 7:59 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel, Kevin Cheng
On Wed, Jan 07, 2026 at 12:26:35PM -0800, Sean Christopherson wrote:
> On Fri, Jan 02, 2026, Yosry Ahmed wrote:
> > On Tue, Dec 30, 2025 at 12:56:41PM -0800, Sean Christopherson wrote:
>
> > > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > > index fdab0ad49098..9084e0dfa15c 100644
> > > --- a/arch/x86/kvm/x86.h
> > > +++ b/arch/x86/kvm/x86.h
> > > @@ -172,9 +172,9 @@ 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)
> > > +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);
> > > }
> >
> > To make this self-contained (e.g. for readers not coming from
> > kvm_set_cpuid()), should we add a comment here about is_guest_mode()
> > only possibly being true with last_vmentry_cpu == -1 if userspace does
> > the set CPUID, set nested state, set CPUID again dance?
>
> Ya. If this looks good, I'll add it when applying.
>
> /*
> * 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 && !is_guest_mode(vcpu);
> }
Looks good to me.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
2025-12-30 20:56 [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active Sean Christopherson
2026-01-02 18:00 ` Yosry Ahmed
@ 2026-01-07 20:46 ` Yosry Ahmed
2026-01-07 20:49 ` Yosry Ahmed
2026-01-12 17:38 ` Sean Christopherson
2 siblings, 1 reply; 8+ messages in thread
From: Yosry Ahmed @ 2026-01-07 20:46 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel, Kevin Cheng
On Tue, Dec 30, 2025 at 12:56:41PM -0800, Sean Christopherson wrote:
> 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.
Another reason why I think this may be needed, but I am not sure:
If kvm_vcpu_after_set_cpuid() is executed while L2 is active,
KVM_REQ_RECALC_INTERCEPTS will cause
svm_recalc_intercepts()->svm_recalc_instruction_intercepts() in the
context of L2. While the svm_clr_intercept() and svm_set_intercept()
calls explicitly modify vmcb01, we set and clear
VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK in svm->vmcb->control.virt_ext. So
this will set/clear the bit in vmcb02.
I think this is a bug, because we could end up setting
VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK when we shouldn't (e.g. L1 doesn't set
in vmcb12, or the X86_FEATURE_V_VMSAVE_VMLOAD is not exposed to L1).
Actually as I am typing this, I believe a separate fix for this is
needed. We should be probably setting/clearing
VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK on svm->vmcb01.control.
Did I miss something?
>
> 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>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/cpuid.c | 19 +++++++++++--------
> arch/x86/kvm/mmu/mmu.c | 6 +-----
> arch/x86/kvm/pmu.c | 2 +-
> arch/x86/kvm/x86.c | 13 +++++++------
> arch/x86/kvm/x86.h | 4 ++--
> 5 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 88a5426674a1..f37331ad3ad8 100644
> --- 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 02c450686b4a..f17324546900 100644
> --- 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 487ad19a236e..ff20b4102173 100644
> --- 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 ff8812f3a129..211d8c24a4b1 100644
> --- 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.
> + * 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_vcpu_has_run(vcpu) && kvm_is_immutable_feature_msr(index) &&
> + 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 fdab0ad49098..9084e0dfa15c 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -172,9 +172,9 @@ 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)
> +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)
>
> base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
> --
> 2.52.0.351.gbe84eed79e-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
2026-01-07 20:46 ` Yosry Ahmed
@ 2026-01-07 20:49 ` Yosry Ahmed
2026-01-07 20:59 ` Sean Christopherson
0 siblings, 1 reply; 8+ messages in thread
From: Yosry Ahmed @ 2026-01-07 20:49 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel, Kevin Cheng
On Wed, Jan 07, 2026 at 08:47:02PM +0000, Yosry Ahmed wrote:
> On Tue, Dec 30, 2025 at 12:56:41PM -0800, Sean Christopherson wrote:
> > 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.
>
> Another reason why I think this may be needed, but I am not sure:
>
> If kvm_vcpu_after_set_cpuid() is executed while L2 is active,
> KVM_REQ_RECALC_INTERCEPTS will cause
> svm_recalc_intercepts()->svm_recalc_instruction_intercepts() in the
> context of L2. While the svm_clr_intercept() and svm_set_intercept()
> calls explicitly modify vmcb01, we set and clear
> VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK in svm->vmcb->control.virt_ext. So
> this will set/clear the bit in vmcb02.
>
> I think this is a bug, because we could end up setting
> VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK when we shouldn't (e.g. L1 doesn't set
> in vmcb12, or the X86_FEATURE_V_VMSAVE_VMLOAD is not exposed to L1).
>
> Actually as I am typing this, I believe a separate fix for this is
> needed. We should be probably setting/clearing
> VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK on svm->vmcb01.control.
>
> Did I miss something?
If the analysis above is correct, then a separate fix is indeed required
because we can end up in the same situation from
kvm_vm_ioctl_set_msr_filter() -> KVM_REQ_RECALC_INTERCEPTS.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
2026-01-07 20:49 ` Yosry Ahmed
@ 2026-01-07 20:59 ` Sean Christopherson
0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2026-01-07 20:59 UTC (permalink / raw)
To: Yosry Ahmed; +Cc: Paolo Bonzini, kvm, linux-kernel, Kevin Cheng
On Wed, Jan 07, 2026, Yosry Ahmed wrote:
> On Wed, Jan 07, 2026 at 08:47:02PM +0000, Yosry Ahmed wrote:
> > On Tue, Dec 30, 2025 at 12:56:41PM -0800, Sean Christopherson wrote:
> > > 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.
> >
> > Another reason why I think this may be needed, but I am not sure:
> >
> > If kvm_vcpu_after_set_cpuid() is executed while L2 is active,
> > KVM_REQ_RECALC_INTERCEPTS will cause
> > svm_recalc_intercepts()->svm_recalc_instruction_intercepts() in the
> > context of L2. While the svm_clr_intercept() and svm_set_intercept()
> > calls explicitly modify vmcb01, we set and clear
> > VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK in svm->vmcb->control.virt_ext. So
> > this will set/clear the bit in vmcb02.
> >
> > I think this is a bug, because we could end up setting
> > VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK when we shouldn't (e.g. L1 doesn't set
> > in vmcb12, or the X86_FEATURE_V_VMSAVE_VMLOAD is not exposed to L1).
> >
> > Actually as I am typing this, I believe a separate fix for this is
> > needed. We should be probably setting/clearing
> > VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK on svm->vmcb01.control.
> >
> > Did I miss something?
>
> If the analysis above is correct, then a separate fix is indeed required
> because we can end up in the same situation from
> kvm_vm_ioctl_set_msr_filter() -> KVM_REQ_RECALC_INTERCEPTS.
Ouch. Yep, svm_recalc_instruction_intercepts() should always operate on vmcb01.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
2025-12-30 20:56 [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active Sean Christopherson
2026-01-02 18:00 ` Yosry Ahmed
2026-01-07 20:46 ` Yosry Ahmed
@ 2026-01-12 17:38 ` Sean Christopherson
2 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2026-01-12 17:38 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Yosry Ahmed, Kevin Cheng
On Tue, 30 Dec 2025 12:56:41 -0800, Sean Christopherson wrote:
> 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.
>
> [...]
Applied to kvm-x86 misc, thanks!
[1/1] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
https://github.com/kvm-x86/linux/commit/b47b93c15b12
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply [flat|nested] 8+ messages in thread