* [PATCH 1/7] KVM: nSVM: Check validity of vmcs01 save area copied from SMRAM on RSM
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
@ 2026-11-02 23:14 ` Paolo Bonzini
2026-11-02 23:14 ` [PATCH 2/7] KVM: x86/mmu: Conditionally resched when retrying gPTE walk on A/D update failure Paolo Bonzini
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2026-11-02 23:14 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Sean Christopherson, stable, Bill Wendling, Stefan Teodorescu
From: Sean Christopherson <seanjc@google.com>
When doing RSM to L2, validate L1's state after copying it back into vmcb01
from SMRAM, as SMRAM is carved out of guest memory, i.e. is writable by the
guest. Failure to validate L1's state is especially problematic for inputs
to MMU context, as KVM can end up with impossible CR4 vs. EFER state. E.g.
without the recent hardening FNAME(walk_addr_generic), a guest can trigger:
------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in arch/x86/kvm/mmu/paging_tmpl.h:435:15
index 3 is out of range for type 'u32 [2]'
CPU: 3 UID: 0 PID: 92 Comm: CPU 1/KVM Not tainted 7.3.0-rc1 #2 PREEMPT(lazy)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x5f/0x90
dump_stack+0x10/0x18
ubsan_epilogue+0x9/0x39
__ubsan_handle_out_of_bounds.cold+0x50/0x55
paging32_walk_addr_generic+0x889/0x970
paging32_page_fault+0x79/0xa60
kvm_mmu_do_page_fault+0x125/0x270
kvm_mmu_page_fault+0x88/0x7b0
npf_interception+0xc8/0x220 [kvm_amd]
svm_invoke_exit_handler+0x16d/0x1d0 [kvm_amd]
svm_handle_exit+0xb7/0x160 [kvm_amd]
vcpu_enter_guest+0x428/0x16a0
kvm_arch_vcpu_ioctl_run+0x479/0x920
kvm_vcpu_ioctl+0x316/0xb00
__x64_sys_ioctl+0xa3/0x100
x64_sys_call+0x281/0x23d0
do_syscall_64+0xba/0x550
entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x79bb30deb04b
</TASK>
---[ end trace ]---
Fixes: 37be407b2ce8 ("KVM: nSVM: Fix L1 state corruption upon return from SMM")
Cc: stable@vger.kernel.org
Reported-by: Bill Wendling <morbo@google.com>
Reported-by: Stefan Teodorescu <fane@google.com>
Tested-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20261022021046.295001-1-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/svm/nested.c | 23 ++++++++++++++++-------
arch/x86/kvm/svm/svm.c | 2 ++
arch/x86/kvm/svm/svm.h | 2 ++
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index af0ef678433d..07554089ec05 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1889,6 +1889,21 @@ static void nested_copy_vmcb_cache_to_control(struct vmcb_control_area *dst,
/* 'clean' and 'hv_enlightenments' are not changed by KVM */
}
+bool nested_svm_check_vmcb01_save(struct kvm_vcpu *vcpu,
+ struct vmcb_save_area *save)
+{
+ struct vmcb_save_area_cached save_cached;
+
+ if (!(save->cr0 & X86_CR0_PG) ||
+ !(save->cr0 & X86_CR0_PE) ||
+ (save->rflags & X86_EFLAGS_VM) ||
+ ((save->efer & EFER_LMA) && !(save->efer & EFER_LME)))
+ return false;
+
+ __nested_copy_vmcb_save_to_cache(&save_cached, save);
+ return nested_vmcb_check_save(vcpu, &save_cached, false);
+}
+
static int svm_get_nested_state(struct kvm_vcpu *vcpu,
struct kvm_nested_state __user *user_kvm_nested_state,
u32 user_data_size)
@@ -1968,7 +1983,6 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
&user_kvm_nested_state->data.svm[0];
struct vmcb_control_area *ctl;
struct vmcb_save_area *save;
- struct vmcb_save_area_cached save_cached;
struct vmcb_ctrl_area_cached ctl_cached;
bool use_separate_l2_pat;
unsigned long cr0;
@@ -2040,12 +2054,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
* KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated
* #VMEXIT.
*/
- __nested_copy_vmcb_save_to_cache(&save_cached, save);
- if (!(save->cr0 & X86_CR0_PG) ||
- !(save->cr0 & X86_CR0_PE) ||
- (save->rflags & X86_EFLAGS_VM) ||
- ((save->efer & EFER_LMA) && !(save->efer & EFER_LME)) ||
- !nested_vmcb_check_save(vcpu, &save_cached, false))
+ if (!nested_svm_check_vmcb01_save(vcpu, save))
goto out_free;
/*
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 487897ef11dd..581bc3a23c95 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5219,6 +5219,8 @@ static int svm_leave_smm(struct kvm_vcpu *vcpu, const union kvm_smram *smram)
*/
svm_copy_vmrun_state(vcpu, &svm->vmcb01.ptr->save, m_save.map.hva + 0x400);
+ if (!nested_svm_check_vmcb01_save(vcpu, &svm->vmcb01.ptr->save))
+ return 1;
/*
* Enter the nested guest now
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 4b0b2ee13a3a..b9f7786bf17c 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -911,6 +911,8 @@ void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
struct vmcb_save_area *save);
void nested_sync_control_from_vmcb02(struct vcpu_svm *svm);
+bool nested_svm_check_vmcb01_save(struct kvm_vcpu *vcpu,
+ struct vmcb_save_area *save);
void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb);
--
2.52.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 2/7] KVM: x86/mmu: Conditionally resched when retrying gPTE walk on A/D update failure
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
2026-11-02 23:14 ` [PATCH 1/7] KVM: nSVM: Check validity of vmcs01 save area copied from SMRAM on RSM Paolo Bonzini
@ 2026-11-02 23:14 ` Paolo Bonzini
2026-11-02 23:14 ` [PATCH 3/7] KVM: x86: Conditionally resched when restarting REP string emulation Paolo Bonzini
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2026-11-02 23:14 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Sean Christopherson, stable, Stefan Teodorescu
From: Sean Christopherson <seanjc@google.com>
Conditionally reschedule when retrying a guest PTE walk because the CMPXCHG
to update A/D bits failed, e.g. because a different task/vCPU modified the
relevant gPTEs. On kernels without involuntary preemption of some kind, a
misbehaving guest can "capture" a pCPU effectively indefinitely and trigger
soft lockups by continuously modifying its gPTEs such that the result of
the walk is remains valid.
Fixes: b3e4e63fd9ee ("KVM: MMU: Use cmpxchg for pte updates on walk_addr()")
Cc: stable@vger.kernel.org
Reported-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20261022015903.282261-1-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/mmu/paging_tmpl.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 8e350095508c..18f7eb633127 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -340,6 +340,13 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
gpa_t real_gpa;
gfn_t gfn;
+ /*
+ * KVM might sleep/resched when retrying the walk if the updating A/D
+ * fails because the gPTE changed between reading the gPTE and trying
+ * the CMPXHG to set A/D bits.
+ */
+ might_sleep();
+
trace_kvm_mmu_pagetable_walk(addr, access);
retry_walk:
walker->level = w->cpu_role.base.level;
@@ -482,8 +489,10 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
addr, write_fault);
if (unlikely(ret < 0))
goto error;
- else if (ret)
+ else if (ret) {
+ cond_resched();
goto retry_walk;
+ }
}
return 1;
--
2.52.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 3/7] KVM: x86: Conditionally resched when restarting REP string emulation
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
2026-11-02 23:14 ` [PATCH 1/7] KVM: nSVM: Check validity of vmcs01 save area copied from SMRAM on RSM Paolo Bonzini
2026-11-02 23:14 ` [PATCH 2/7] KVM: x86/mmu: Conditionally resched when retrying gPTE walk on A/D update failure Paolo Bonzini
@ 2026-11-02 23:14 ` Paolo Bonzini
2026-11-02 23:14 ` [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry Paolo Bonzini
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2026-11-02 23:14 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Sean Christopherson, stable, David Woodhouse, Stefan Teodorescu
From: Sean Christopherson <seanjc@google.com>
When restarting emulation, e.g. of a REP-prefixed string instruction,
conditionally reschedule as emulating guest writes can be deceptively slow
and expensive, e.g. when shadow paging is in use and each write can trigger
write tracking.
In a perfect world, restarting emulation (or not) would be less arbitrary,
e.g. would honor not only host events (NEED_RESCHED, signals, etc.) but
also guest events (e.g. by checking kvm_vcpu_has_events()). Unfortunately,
as pointed out by Sashiko, pio_in_emulated()'s read-ahead logic is baked
into the restart logic, as KVM relies on restarting emulation to drain the
cache and avoid duplicate reads from the I/O port. I.e. re-entering the
guest on kvm_vcpu_has_events() or __xfer_to_guest_mode_work_pending() would
introduce architecturally-incorrect, guest-visible changes.
Fixes: 5cd21917da24 ("KVM: x86 emulator: restart string instruction without going back to a guest.")
Cc: stable@vger.kernel.org
Cc: David Woodhouse <dwmw2@infradead.org>
Reported-by: Stefan Teodorescu <fane@google.com>
Suggested-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20261028173638.1765329-1-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 32563a91fba9..c4a06214cde6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6558,10 +6558,12 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
} else if (vcpu->arch.complete_userspace_io) {
writeback = false;
r = 0;
- } else if (r == EMULATION_RESTART)
+ } else if (r == EMULATION_RESTART) {
+ cond_resched();
goto restart;
- else
+ } else {
r = 1;
+ }
writeback:
if (writeback) {
--
2.52.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
` (2 preceding siblings ...)
2026-11-02 23:14 ` [PATCH 3/7] KVM: x86: Conditionally resched when restarting REP string emulation Paolo Bonzini
@ 2026-11-02 23:14 ` Paolo Bonzini
2026-11-02 23:39 ` Jim Mattson
2026-11-02 23:14 ` [PATCH 5/7] KVM: x86/mmu: Cap number of parent PTEs per SP at 1000 (the arbitrary rmap limit) Paolo Bonzini
` (2 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2026-11-02 23:14 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Sean Christopherson, stable, David Woodhouse, Stefan Teodorescu
From: Sean Christopherson <seanjc@google.com>
Add a missing "break" in the inner for-loop of the open coded mess that
unwinds MSRs back to the pre-VM-Enter state when VMLAUNCH/VMRESUME VM-Fails
due to a consistency check that is caught by hardware but missed by KVM.
I.e. move on to the next MSR in the VM-Entry load list after performing a
write using a value found in the VM-Exit load list. Processing *every*
matching entry in the VM-Exit load list makes the processing quadratic,
which is problematic if the load lists are full of MSRs that are expensive
to emulate. E.g. writing APIC_BASE triggers kvm_recalculate_apic_map() and
a whole pile of unnecessary work.
Functionally, arbitrarily restoring the value of the first matching entry
is "fine", because none of this is architecturally correct: the load lists
are never supposed to have been used in the first place. In practice, it's
likely a moot point as no well-behaved VMM will have multiple, diverging
entries in the load lists.
Fixes: bd18bffca353 ("KVM: nVMX: restore host state in nested_vmx_vmexit for VMFail")
Cc: stable@vger.kernel.org
Cc: David Woodhouse <dwmw2@infradead.org>
Reported-by: Stefan Teodorescu <fane@google.com>
Suggested-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20261027185917.1478906-1-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/vmx/nested.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 9b0bfa2f854c..f0d75f5a968c 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5057,6 +5057,7 @@ static void nested_vmx_restore_host_state(struct kvm_vcpu *vcpu)
__func__, j, h.index, h.value);
goto vmabort;
}
+ break;
}
}
--
2.52.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry
2026-11-02 23:14 ` [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry Paolo Bonzini
@ 2026-11-02 23:39 ` Jim Mattson
2026-11-03 0:10 ` Sean Christopherson
0 siblings, 1 reply; 11+ messages in thread
From: Jim Mattson @ 2026-11-02 23:39 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-kernel, kvm, Sean Christopherson, stable, David Woodhouse,
Stefan Teodorescu
On Mon, Nov 2, 2026 at 3:19 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Add a missing "break" in the inner for-loop of the open coded mess that
> unwinds MSRs back to the pre-VM-Enter state when VMLAUNCH/VMRESUME VM-Fails
> due to a consistency check that is caught by hardware but missed by KVM.
That sentence does not accurately describe what the code does.
> I.e. move on to the next MSR in the VM-Entry load list after performing a
> write using a value found in the VM-Exit load list. Processing *every*
> matching entry in the VM-Exit load list makes the processing quadratic,
> which is problematic if the load lists are full of MSRs that are expensive
> to emulate. E.g. writing APIC_BASE triggers kvm_recalculate_apic_map() and
> a whole pile of unnecessary work.
>
> Functionally, arbitrarily restoring the value of the first matching entry
> is "fine", because none of this is architecturally correct: the load lists
> are never supposed to have been used in the first place. In practice, it's
> likely a moot point as no well-behaved VMM will have multiple, diverging
> entries in the load lists.
This isn't "fine." This is disgusting.
The assumption that the VM-exit MSR-load list will "undo" every
assignment made by the VM-entry MSR-load list is misguided and wrong.
Xen, for instance, puts SPEC_CTRL on the VM-entry MSR-load list, but
it does not put it on the VM-exit MSR-load list. Instead, after a
normal VM-exit, it restores the host value in
vmx_asm_vmexit_handler():
/*
* Restore Xen's MSR_SPEC_CTRL setting. The guest's value resides in
* the MSR load/save list. For Legacy IBRS, this flushes/inhibits
* indirect predictions and does not flush the RSB. For eIBRS, this
* prevents CALLs/JMPs using predictions learnt at a lower predictor
* mode, and it flushes the RSB. On eIBRS parts that also suffer from
* PBRSB, the prior RSB stuffing suffices to make the RSB safe.
*/
.macro restore_spec_ctrl
mov $MSR_SPEC_CTRL, %ecx
mov CPUINFO_xen_spec_ctrl(%rsp), %eax
xor %edx, %edx
wrmsr
.endm
ALTERNATIVE "", restore_spec_ctrl, X86_FEATURE_SC_MSR_HVM
After VM-entry failure, Xen executes this code:
.Lvmx_vmentry_fail:
sti
PUSH_AND_CLEAR_GPRS
/*
* SPEC_CTRL_ENTRY notes
*
* If we end up here, no guest code has executed. The MSR lists have
* not been processed, so we still have Xen's choice of MSR_SPEC_CTRL
* in context, and the RSB is unchanged.
*/
call vmx_vmentry_failure
KVM needs to do better.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry
2026-11-02 23:39 ` Jim Mattson
@ 2026-11-03 0:10 ` Sean Christopherson
2026-11-03 0:34 ` Jim Mattson
0 siblings, 1 reply; 11+ messages in thread
From: Sean Christopherson @ 2026-11-03 0:10 UTC (permalink / raw)
To: Jim Mattson
Cc: Paolo Bonzini, linux-kernel, kvm, stable, David Woodhouse,
Stefan Teodorescu
On Mon, Nov 02, 2026, Jim Mattson wrote:
> On Mon, Nov 2, 2026 at 3:19 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > From: Sean Christopherson <seanjc@google.com>
> >
> > Add a missing "break" in the inner for-loop of the open coded mess that
> > unwinds MSRs back to the pre-VM-Enter state when VMLAUNCH/VMRESUME VM-Fails
> > due to a consistency check that is caught by hardware but missed by KVM.
>
> That sentence does not accurately describe what the code does.
"attempts to unwind MSRs back to the pre-VM-Enter state"
> > I.e. move on to the next MSR in the VM-Entry load list after performing a
> > write using a value found in the VM-Exit load list. Processing *every*
> > matching entry in the VM-Exit load list makes the processing quadratic,
> > which is problematic if the load lists are full of MSRs that are expensive
> > to emulate. E.g. writing APIC_BASE triggers kvm_recalculate_apic_map() and
> > a whole pile of unnecessary work.
> >
> > Functionally, arbitrarily restoring the value of the first matching entry
> > is "fine", because none of this is architecturally correct: the load lists
> > are never supposed to have been used in the first place. In practice, it's
> > likely a moot point as no well-behaved VMM will have multiple, diverging
> > entries in the load lists.
>
> This isn't "fine." This is disgusting.
The "fine" here refers to what is being changed in *this* patch, which is to
load each MSR at most once, and to use the first value encountered, e.g. versus
the last value. IMO, that claim stands: in the context of the existing code,
this change is "fine".
> The assumption that the VM-exit MSR-load list will "undo" every
> assignment made by the VM-entry MSR-load list is misguided and wrong.
No one is assuming that. The comment even says *exactly* what you're saying:
/*
* This nasty bit of open coding is a compromise between blindly
* loading L1's MSRs using the exit load lists (incorrect emulation
* of VMFail), leaving the nested VM's MSRs in the software model
* (incorrect behavior) and snapshotting the modified MSRs (too
* expensive since the lists are unbound by hardware). For each
* MSR that was (prematurely) loaded from the nested VMEntry load
* list, reload it from the exit load list if it exists and differs
* from the guest value. The intent is to stuff host state as
* silently as possible, not to fully process the exit load list.
*/
> Xen, for instance, puts SPEC_CTRL on the VM-entry MSR-load list, but
> it does not put it on the VM-exit MSR-load list. Instead, after a
> normal VM-exit, it restores the host value in
> vmx_asm_vmexit_handler():
...
> KVM needs to do better.
Meh, KVM is doing pretty darn good, all things considered. Yeah, this code sucks.
But I'm not convinced the alternatives suck less. We've gotten the set of missed
consistency checks down to effectively zero, module the TPR mess that I do test
by running warn_on_missed_cc=true.
E.g. for Xen to run afoul of a misbehaving guest, Xen would have botch its
handling of vTPR.
I'm not opposed to trying to figure out a not-horrible way to snapshot a limited,
hardcoded number of MSRs to make KVM less wrong. But I sure as hell ain't going
to spend cycles on it at this point, because potentially clobbering L1 MSRs when
L1 itself has already screwed up is firmly in "don't care" territory right now.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry
2026-11-03 0:10 ` Sean Christopherson
@ 2026-11-03 0:34 ` Jim Mattson
0 siblings, 0 replies; 11+ messages in thread
From: Jim Mattson @ 2026-11-03 0:34 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, linux-kernel, kvm, stable, David Woodhouse,
Stefan Teodorescu
On Mon, Nov 2, 2026 at 4:10 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Nov 02, 2026, Jim Mattson wrote:
> > On Mon, Nov 2, 2026 at 3:19 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >
> > > From: Sean Christopherson <seanjc@google.com>
> > >
> > > Add a missing "break" in the inner for-loop of the open coded mess that
> > > unwinds MSRs back to the pre-VM-Enter state when VMLAUNCH/VMRESUME VM-Fails
> > > due to a consistency check that is caught by hardware but missed by KVM.
> >
> > That sentence does not accurately describe what the code does.
>
> "attempts to unwind MSRs back to the pre-VM-Enter state"
KVM makes no attempt to restore MSR values to the pre-VM-Enter state.
MSRs are forwarded to a post-VM-exit state, as if VM-entry had not
failed.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/7] KVM: x86/mmu: Cap number of parent PTEs per SP at 1000 (the arbitrary rmap limit)
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
` (3 preceding siblings ...)
2026-11-02 23:14 ` [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry Paolo Bonzini
@ 2026-11-02 23:14 ` Paolo Bonzini
2026-11-02 23:14 ` [PATCH 6/7] KVM: x86: Unconditionally reset MMU context when emulating INIT Paolo Bonzini
2026-11-02 23:14 ` [PATCH 7/7] KVM: x86: Flush guest TLB when emulating a RESET to strictly honor the spec Paolo Bonzini
6 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2026-11-02 23:14 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Sean Christopherson, Jim Mattson, James Houghton, Yosry Ahmed,
David Woodhouse, stable, Stefan Teodorescu
From: Sean Christopherson <seanjc@google.com>
When potentially reusing a shadow page, zap the shadow page if its number
of parent PTEs exceeds KVM's (mostly) arbitrary rmap recycling threshold.
Letting the guest create a (mostly) unbounded number of parent PTEs allows
the guest to coerce KVM into creating lists containing millions of PTEs,
effectively bounded only by the maximum number of shadow pages.
Because PTE lists are simple singly linked lists, removal is O(n). Even
worse, if a list is constructed just so, teardown can be O(n^2), e.g. if
SPTEs are built such that zapping child SPTEs always zaps the last entry
in the list. To cap things off, all paths that walk PTE lists do so
while holding mmu_lock, without conditionally rescheduling.
Using a doubly linked list would not address the issue, as removal would
remain O(n) since KVM still wouldn't know where in the list to remove from.
Removal could be made O(1) by storing a per-SPTE pointer, but that would
largely defeat the purpose of reusing SPs, and would penalize well-behaved
guests, especially for nested TDP where aliasing page tables is uncommon.
So, cap the maximum number of parent PTEs at the rmap limit. In practice,
well-behaved guests will likely never run afoul of the limit. E.g. when
running Linux as L1 with TDP, the number of parent PTEs for a single SP
tops out at under 200 (though undoubtedly obscure workloads can push it
higher). Deliberately use a "greater than or equal" as opposed to rmap's
strictly "greater than", as the rmap code checks the threshold immediately
*after* inserting into the list, i.e. the limit is 1000 for rmaps, KVM just
very briefly blows past it.
Apply the check to both indirect and direct SPs, because while a fully
direct MMU naturally limits the number of parent PTEs to '1', direct SPs in
indirect MMUs have no such constraint. E.g. if the guest maps a huge PTE
into many locations in its page tables, and KVM is backing the hugepage
with a direct SP, i.e. with a series of smaller pages, then KVM could end
up with a direct SP with many parents.
Note, shadow_mmu_try_split_huge_pages() always restarts the rmap walk or
bails entirely if it gets to the point where it tries to grab a SP, i.e.
there is *no* pre-existing bug that would be tripped by zapping a SP and
thus rmaps when getting a SP.
Cc: Jim Mattson <jmattson@google.com>
Cc: James Houghton <jthoughton@google.com>
Cc: Yosry Ahmed <yosry@kernel.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: stable@vger.kernel.org
Reported-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20261027171518.1432557-1-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/mmu/mmu.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 5a2cc6a17a9b..f4b12d6aef55 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2355,6 +2355,16 @@ static struct kvm_mmu_page *kvm_mmu_find_shadow_page(struct kvm *kvm,
continue;
}
+ /*
+ * Note! Unlike fully direct MMUs, direct SPs in indirect MMUs
+ * can have an arbitrary number of parents, e.g if the guest is
+ * mapping a huge PTE at many points in its page tables.
+ */
+ if (pte_list_count(&sp->parent_ptes) >= RMAP_RECYCLE_THRESHOLD) {
+ kvm_mmu_prepare_zap_page(kvm, sp, &invalid_list);
+ break;
+ }
+
/* unsync and write-flooding only apply to indirect SPs. */
if (sp->role.direct)
goto out;
--
2.52.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 6/7] KVM: x86: Unconditionally reset MMU context when emulating INIT
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
` (4 preceding siblings ...)
2026-11-02 23:14 ` [PATCH 5/7] KVM: x86/mmu: Cap number of parent PTEs per SP at 1000 (the arbitrary rmap limit) Paolo Bonzini
@ 2026-11-02 23:14 ` Paolo Bonzini
2026-11-02 23:14 ` [PATCH 7/7] KVM: x86: Flush guest TLB when emulating a RESET to strictly honor the spec Paolo Bonzini
6 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2026-11-02 23:14 UTC (permalink / raw)
To: linux-kernel, kvm
Cc: Sean Christopherson, Stefan Teodorescu, Oren Yomtov, Chris Mason,
stable, Yosry Ahmed
From: Sean Christopherson <seanjc@google.com>
Force an MMU reset when emulating an INIT even if paging was already
disabled, as the MMU page role includes guest_mode (L2), i.e. an INIT while
L2 is running with CR0.PG=0 will result in KVM running L1 with a stale MMU
context.
Alternatively, the reset could be scoped strictly to paging being enabled
or L2 being active, but SMM is also encoded in the page role. Even though
it _should_ be impossible for the vCPU to be INIT'd while SMM is active,
there's zero value in not handling SMM. At that point, the only scenario
where KVM _doesn't_ need to reset the MMU context is on an INIT with the
vCPU running in Real Mode or Unpaged Protect Mode, and while INIT in those
modes is somewhat common, it's far from a hot path and re-initializing the
MMU is not expensive, especially when the role isn't changing (KVM elides
most of the re-configuration in that case).
Note, like the Fixes commit, this bug only affects nested SVM as INIT is an
unconditional intercept on VMX, i.e. it's impossible to emulate INIT while
L2 is active.
Temporarily keep the guest TLB flush when CR0.PG is being cleared even
though it is fully redundant with the flush that is emitted on all INIT
events (KVM itself only does RESET when a vCPU is being created, i.e. when
CR0.PG must be '0').
Fixes: ed129ec9057f ("KVM: x86: forcibly leave nested mode on vCPU reset")
Reported-by: Stefan Teodorescu <fane@google.com>
Reported-by: Oren Yomtov <thenameisoren@gmail.com>
Reported-by: Chris Mason <mason@kernel.org>
Cc: stable@vger.kernel.org
Cc: Yosry Ahmed <yosry@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20261022015407.279870-2-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c4a06214cde6..09c6df00ebb6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9731,18 +9731,23 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
kvm_x86_call(set_efer)(vcpu, 0);
kvm_x86_call(update_exception_bitmap)(vcpu);
- /*
- * On the standard CR0/CR4/EFER modification paths, there are several
- * complex conditions determining whether the MMU has to be reset and/or
- * which PCIDs have to be flushed. However, CR0.WP and the paging-related
- * bits in CR4 and EFER are irrelevant if CR0.PG was '0'; and a reset+flush
- * is needed anyway if CR0.PG was '1' (which can only happen for INIT, as
- * CR0 will be '0' prior to RESET). So we only need to check CR0.PG here.
- */
- if (old_cr0 & X86_CR0_PG) {
+ if (old_cr0 & X86_CR0_PG)
kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
- kvm_mmu_reset_context(vcpu);
- }
+
+ /*
+ * Unconditionally reset the MMU, even though super strictly speaking,
+ * a MMU reset is only necessary if and only one of the page role bits
+ * is changing. All of the CR0/CR4/EFER bits effectively get covered
+ * by checking CR0.PG=1, as CR0.WP and the paging-related bits in CR4
+ * and EFER are irrelevant if CR0.PG was '0'. Subtly, if the vCPU was
+ * in guest mode (L2) or SMM (although emulating INIT in SMM _should_
+ * be impossible), an MMU reset is also necessary as "guest mode" and
+ * "smm" are both encoded in the shadow page role. As a result, the
+ * only scenarios where an MMU reset is *not* needed is if the vCPU is
+ * in Real Mode or Unpaged Protected Mode, and micro-optimizing those
+ * paths are not worth the risk of running with a stale MMU context.
+ */
+ kvm_mmu_reset_context(vcpu);
/*
* Intel's SDM states that all TLB entries are flushed on INIT. AMD's
--
2.52.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 7/7] KVM: x86: Flush guest TLB when emulating a RESET to strictly honor the spec
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
` (5 preceding siblings ...)
2026-11-02 23:14 ` [PATCH 6/7] KVM: x86: Unconditionally reset MMU context when emulating INIT Paolo Bonzini
@ 2026-11-02 23:14 ` Paolo Bonzini
6 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2026-11-02 23:14 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Sean Christopherson
From: Sean Christopherson <seanjc@google.com>
Flush the guest TLB when emulating a RESET even though such a flush should
be completely unnecessary since KVM only performs a RESET when creating a
vCPU, and so it should be impossible for the vCPU to have any accrued TLB
state. Making the flush unconditional aligns KVM's behavior with both the
SDM and APM (ignoring that both specifications contradict themselves), and
avoids having to worry about hypotheticals like "what if KVM did provide a
way to emulate RESET after a vCPU is created?".
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20261022015407.279870-3-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 09c6df00ebb6..26e7beaff8a0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9731,9 +9731,6 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
kvm_x86_call(set_efer)(vcpu, 0);
kvm_x86_call(update_exception_bitmap)(vcpu);
- if (old_cr0 & X86_CR0_PG)
- kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
-
/*
* Unconditionally reset the MMU, even though super strictly speaking,
* a MMU reset is only necessary if and only one of the page role bits
@@ -9750,16 +9747,16 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
kvm_mmu_reset_context(vcpu);
/*
- * Intel's SDM states that all TLB entries are flushed on INIT. AMD's
- * APM states the TLBs are untouched by INIT, but it also states that
- * the TLBs are flushed on "External initialization of the processor."
- * Flush the guest TLB regardless of vendor, there is no meaningful
- * benefit in relying on the guest to flush the TLB immediately after
- * INIT. A spurious TLB flush is benign and likely negligible from a
- * performance perspective.
+ * Intel's SDM states that all TLB entries are flushed on INIT "as with
+ * a hardware reset", and AMD's APM states the TLBs are flushed on
+ * "External initialization of the processor". Flush the guest TLB for
+ * both RESET and INIT, regardless of vendor, even though both the SDM
+ * and APM contradict the above statements in other tables. There is
+ * no meaningful benefit in relying on the guest to flush the TLB, and
+ * a spurious TLB flush is benign and likely negligible from a
+ * performance perspective (especially for RESET).
*/
- if (init_event)
- kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
+ kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
}
EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_vcpu_reset);
--
2.52.0
^ permalink raw reply [flat|nested] 11+ messages in thread