Reply-To: Sean Christopherson <seanjc@google.com>
Date:   Sat, 20 Nov 2021 04:50:35 +0000
In-Reply-To: <20211120045046.3940942-1-seanjc@google.com>
Message-Id: <20211120045046.3940942-18-seanjc@google.com>
Mime-Version: 1.0
References: <20211120045046.3940942-1-seanjc@google.com>
Subject: [PATCH 17/28] KVM: x86/mmu: Terminate yield-friendly walk if invalid
 root observed
From:   Sean Christopherson <seanjc@google.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Hou Wenlong <houwenlong93@linux.alibaba.com>,
        Ben Gardon <bgardon@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Stop walking TDP MMU roots if the previous root was invalidated while the
caller yielded (dropped mmu_lock).  Any effect that the caller wishes to
be recognized by a root must be made visible before walking the list of
roots.  Because all roots are invalided when any root is invalidated, if
the previous root was invalidated, then all roots on the list when the
caller first started the walk also were invalidated, and any valid roots
on the list must have been added after the invalidation event.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index d9524b387221..cc8d021a1ba5 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -140,16 +140,21 @@ static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm,
 	lockdep_assert_held(&kvm->mmu_lock);
 
 	/*
-	 * Restart the walk if the previous root was invalidated, which can
-	 * happen if the caller drops mmu_lock when yielding.  Restarting the
-	 * walke is necessary because invalidating a root also removes it from
-	 * tdp_mmu_roots.  Restarting is safe and correct because invalidating
-	 * a root is done if and only if _all_ roots are invalidated, i.e. any
-	 * root on tdp_mmu_roots was added _after_ the invalidation event.
+	 * Terminate the walk if the previous root was invalidated, which can
+	 * happen if the caller yielded and dropped mmu_lock.  Because invalid
+	 * roots are removed from tdp_mmu_roots with mmu_lock held for write,
+	 * if the previous root was invalidated, then the invalidation occurred
+	 * after this walk started.  And because _all_ roots are invalidated
+	 * during an invalidation event, any root on tdp_mmu_roots was created
+	 * after the invalidation.  Lastly, any state change being made by the
+	 * caller must be effected before updating SPTEs, otherwise vCPUs could
+	 * simply create new SPTEs with the old state.  Thus, if the previous
+	 * root was invalidated, all valid roots are guaranteed to have been
+	 * created after the desired state change and don't need to be updated.
 	 */
 	if (prev_root && prev_root->role.invalid) {
 		kvm_tdp_mmu_put_root(kvm, prev_root, shared);
-		prev_root = NULL;
+		return NULL;
 	}
 
 	/*
-- 
2.34.0.rc2.393.gf8c9666880-goog