Patchwork [v2,26/27] KVM: x86/mmu: WARN if zapping a MMIO spte results in zapping children

mail settings
Submitter Christopherson, Sean J
Date Feb. 5, 2019, 9:01 p.m.
Message ID <>
Download mbox | patch
Permalink /patch/719023/
State New
Headers show


Christopherson, Sean J - Feb. 5, 2019, 9:01 p.m.
Paolo expressed a concern that kvm_mmu_zap_mmio_sptes() could have a
quadratic runtime[1], i.e. restarting the spte walk while zapping only
MMIO sptes could result in re-walking large portions of the list over
and over due to the non-MMIO sptes encountered before the restart not
being removed.

At the time, the concern was legitimate as the walk was restarted when
any spte was zapped.  But that is no longer the case as the walk is now
restarted iff one or more children have been zapped, which is necessary
because zapping children makes the active_mmu_pages list unstable.

Furthermore, it should be impossible for an MMIO spte to have children,
i.e. zapping an MMIO spte should never result in zapping children.  In
other words, kvm_mmu_zap_mmio_sptes() should never restart its walk, and
so should always execute in linear time.  WARN if this assertion fails.

Although it should never be needed, leave the restart logic in place.
In normal operation, the cost is at worst an extra CMP+Jcc, and if for
some reason the list does become unstable, not restarting would likely
crash KVM, or worse, the kernel.


Signed-off-by: Sean Christopherson <>
 arch/x86/kvm/mmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)


diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ba01cbaf4389..cda14580a086 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -5868,8 +5868,11 @@  static void kvm_mmu_zap_mmio_sptes(struct kvm *kvm)
 	list_for_each_entry_safe(sp, node, &kvm->arch.active_mmu_pages, link) {
 		if (!sp->mmio_cached)
-		if (__kvm_mmu_prepare_zap_page(kvm, sp, &invalid_list, &ign) ||
-		    cond_resched_lock(&kvm->mmu_lock))
+		if (__kvm_mmu_prepare_zap_page(kvm, sp, &invalid_list, &ign)) {
+			WARN_ON_ONCE(1);
+			goto restart;
+		}
+		if (cond_resched_lock(&kvm->mmu_lock))
 			goto restart;