From: David Matlack <dmatlack@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>,
Ben Gardon <bgardon@google.com>,
kvm@vger.kernel.org, David Matlack <dmatlack@google.com>
Subject: [PATCH 2/2] KVM: x86/mmu: Improve comment about TLB flush semantics for write-protection
Date: Wed, 12 Jan 2022 21:58:01 +0000 [thread overview]
Message-ID: <20220112215801.3502286-3-dmatlack@google.com> (raw)
In-Reply-To: <20220112215801.3502286-1-dmatlack@google.com>
Rewrite the comment in kvm_mmu_slot_remove_write_access() that explains
why it is safe to flush TLBs outside of the MMU lock after
write-protecting SPTEs for dirty logging. The current comment is a long
run-on sentance that was difficult to undertsand. In addition it was
specific to the shadow MMU (mentioning mmu_spte_update()) when the TDP
MMU has to handle this as well.
Signed-off-by: David Matlack <dmatlack@google.com>
---
arch/x86/kvm/mmu/mmu.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 1d275e9d76b5..33f550b3be8f 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5825,15 +5825,26 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
}
/*
- * We can flush all the TLBs out of the mmu lock without TLB
- * corruption since we just change the spte from writable to
- * readonly so that we only need to care the case of changing
- * spte from present to present (changing the spte from present
- * to nonpresent will flush all the TLBs immediately), in other
- * words, the only case we care is mmu_spte_update() where we
- * have checked Host-writable | MMU-writable instead of
- * PT_WRITABLE_MASK, that means it does not depend on PT_WRITABLE_MASK
- * anymore.
+ * It is safe to flush TLBs outside of the MMU lock since SPTEs are only
+ * being changed from writable to read-only (i.e. the mapping to host
+ * PFNs is not changing). All we care about is that CPUs start using the
+ * read-only mappings from this point forward to ensure the dirty bitmap
+ * gets updated, but that does not need to run under the MMU lock.
+ *
+ * Note that there are other reasons why SPTEs can be write-protected
+ * besides dirty logging: (1) to intercept guest page table
+ * modifications when doing shadow paging and (2) to protecting guest
+ * memory that is not host-writable. Both of these usecases require
+ * flushing the TLB under the MMU lock to ensure CPUs are not running
+ * with writable SPTEs in their TLB. The tricky part is knowing when it
+ * is safe to skip a TLB flush if an SPTE is already write-protected,
+ * since it could have been write-protected for dirty-logging which does
+ * not flush under the lock.
+ *
+ * To handle this each SPTE has an MMU-writable bit and a Host-writable
+ * bit (KVM-specific bits that are not used by hardware). These bits
+ * allow KVM to deduce *why* a given SPTE is currently write-protected,
+ * so that it knows when it needs to flush TLBs under the MMU lock.
*/
if (flush)
kvm_arch_flush_remote_tlbs_memslot(kvm, memslot);
--
2.34.1.703.g22d0c6ccf7-goog
next prev parent reply other threads:[~2022-01-12 21:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-12 21:57 [PATCH 0/2] KVM: x86/mmu: Fix write-protection bug in the TDP MMU David Matlack
2022-01-12 21:58 ` [PATCH 1/2] KVM: x86/mmu: Fix write-protection of PTs mapped by " David Matlack
2022-01-12 23:14 ` Sean Christopherson
2022-01-12 23:57 ` David Matlack
2022-01-13 0:28 ` Sean Christopherson
2022-01-13 17:04 ` David Matlack
2022-01-13 18:28 ` David Matlack
2022-01-13 19:29 ` Sean Christopherson
2022-01-12 21:58 ` David Matlack [this message]
2022-01-13 0:46 ` [PATCH 2/2] KVM: x86/mmu: Improve comment about TLB flush semantics for write-protection Sean Christopherson
2022-01-13 17:10 ` David Matlack
2022-01-13 22:40 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220112215801.3502286-3-dmatlack@google.com \
--to=dmatlack@google.com \
--cc=bgardon@google.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.