[PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[David Matlack]

Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
use the TDP MMU when TDP hardware support is enabled.  The rest of the
patches are related cleanups that follow (although the kvm_faultin_pfn()
cleanups at the end are only tangentially related at best).

The TDP MMU was introduced in 5.10 and has been enabled by default since
5.15. At this point there are no known functionality gaps between the
TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
better with the number of vCPUs. In other words, there is no good reason
to disable the TDP MMU.

Dropping the ability to disable the TDP MMU reduces the number of
possible configurations that need to be tested to validate KVM (i.e. no
need to test with tdp_mmu=N), and simplifies the code.

David Matlack (9):
  KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled
  KVM: x86/mmu: Drop kvm->arch.tdp_mmu_enabled
  KVM: x86/mmu: Consolidate mmu_seq calculations in kvm_faultin_pfn()
  KVM: x86/mmu: Rename __direct_map() to nonpaging_map()
  KVM: x86/mmu: Separate TDP and non-paging fault handling
  KVM: x86/mmu: Stop needlessly making MMU pages available for TDP MMU
    faults
  KVM: x86/mmu: Handle "error PFNs" in kvm_faultin_pfn()
  KVM: x86/mmu: Avoid memslot lookup during KVM_PFN_ERR_HWPOISON
    handling
  KVM: x86/mmu: Try to handle no-slot faults during kvm_faultin_pfn()

 .../admin-guide/kernel-parameters.txt         |   3 +-
 arch/x86/include/asm/kvm_host.h               |   9 -
 arch/x86/kvm/mmu.h                            |   8 +-
 arch/x86/kvm/mmu/mmu.c                        | 197 ++++++++++--------
 arch/x86/kvm/mmu/mmu_internal.h               |   1 +
 arch/x86/kvm/mmu/paging_tmpl.h                |  10 +-
 arch/x86/kvm/mmu/tdp_mmu.c                    |   9 +-
 7 files changed, 115 insertions(+), 122 deletions(-)


base-commit: 93472b79715378a2386598d6632c654a2223267b
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 1/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[David Matlack]

Delete the module parameter tdp_mmu and force KVM to always use the TDP
MMU when TDP hardware support is enabled.

The TDP MMU was introduced in 5.10 and has been enabled by default since
5.15. At this point there are no known functionality gaps between the
TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
better with the number of vCPUs. In other words, there is no good reason
to disable the TDP MMU.

Dropping the ability to disable the TDP MMU reduces the number of
possible configurations that need to be tested to validate KVM (i.e. no
need to test with tdp_mmu=N), and simplifies the code.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 ++-
 arch/x86/kvm/mmu/tdp_mmu.c                      | 5 +----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f7561cd494cb..e75d45a42b01 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2418,7 +2418,8 @@
 			the KVM_CLEAR_DIRTY ioctl, and only for the pages being
 			cleared.
 
-			Eager page splitting is only supported when kvm.tdp_mmu=Y.
+			Eager page splitting is only supported when TDP hardware
+			support is enabled.
 
 			Default is Y (on).
 
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index bf2ccf9debca..d6c30a648d8d 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -10,15 +10,12 @@
 #include <asm/cmpxchg.h>
 #include <trace/events/kvm.h>
 
-static bool __read_mostly tdp_mmu_enabled = true;
-module_param_named(tdp_mmu, tdp_mmu_enabled, bool, 0644);
-
 /* Initializes the TDP MMU for the VM, if enabled. */
 int kvm_mmu_init_tdp_mmu(struct kvm *kvm)
 {
 	struct workqueue_struct *wq;
 
-	if (!tdp_enabled || !READ_ONCE(tdp_mmu_enabled))
+	if (!tdp_enabled)
 		return 0;
 
 	wq = alloc_workqueue("kvm", WQ_UNBOUND|WQ_MEM_RECLAIM|WQ_CPU_INTENSIVE, 0);

base-commit: 93472b79715378a2386598d6632c654a2223267b
prerequisite-patch-id: 8c230105c8a2f1245dedb5b386327d98865d0bb2
prerequisite-patch-id: 9b4329037e2e880db19f3221e47d956b78acadc8
prerequisite-patch-id: 2e3661ba8856c29b769499bac525b6943d9284b8
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 2/9] KVM: x86/mmu: Drop kvm->arch.tdp_mmu_enabled

[David Matlack]

Drop kvm->arch.tdp_mmu_enabled and replace it with checks for
tdp_enabled. The TDP MMU is unconditionally enabled when TDP hardware
support is enabled, so these checks are identical.

No functional change intended.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/include/asm/kvm_host.h |  9 ---------
 arch/x86/kvm/mmu.h              |  8 +-------
 arch/x86/kvm/mmu/mmu.c          | 34 ++++++++++++++++-----------------
 arch/x86/kvm/mmu/tdp_mmu.c      |  4 +---
 4 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e8281d64a431..5243db32f954 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1262,15 +1262,6 @@ struct kvm_arch {
 	struct task_struct *nx_lpage_recovery_thread;
 
 #ifdef CONFIG_X86_64
-	/*
-	 * Whether the TDP MMU is enabled for this VM. This contains a
-	 * snapshot of the TDP MMU module parameter from when the VM was
-	 * created and remains unchanged for the life of the VM. If this is
-	 * true, TDP MMU handler functions will run for various MMU
-	 * operations.
-	 */
-	bool tdp_mmu_enabled;
-
 	/*
 	 * List of struct kvm_mmu_pages being used as roots.
 	 * All struct kvm_mmu_pages in the list should have
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index a99acec925eb..ee3102a424aa 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -227,15 +227,9 @@ static inline bool kvm_shadow_root_allocated(struct kvm *kvm)
 	return smp_load_acquire(&kvm->arch.shadow_root_allocated);
 }
 
-#ifdef CONFIG_X86_64
-static inline bool is_tdp_mmu_enabled(struct kvm *kvm) { return kvm->arch.tdp_mmu_enabled; }
-#else
-static inline bool is_tdp_mmu_enabled(struct kvm *kvm) { return false; }
-#endif
-
 static inline bool kvm_memslots_have_rmaps(struct kvm *kvm)
 {
-	return !is_tdp_mmu_enabled(kvm) || kvm_shadow_root_allocated(kvm);
+	return !tdp_enabled || kvm_shadow_root_allocated(kvm);
 }
 
 static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 3e1317325e1f..8c293a88d923 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1253,7 +1253,7 @@ static void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
 {
 	struct kvm_rmap_head *rmap_head;
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		kvm_tdp_mmu_clear_dirty_pt_masked(kvm, slot,
 				slot->base_gfn + gfn_offset, mask, true);
 
@@ -1286,7 +1286,7 @@ static void kvm_mmu_clear_dirty_pt_masked(struct kvm *kvm,
 {
 	struct kvm_rmap_head *rmap_head;
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		kvm_tdp_mmu_clear_dirty_pt_masked(kvm, slot,
 				slot->base_gfn + gfn_offset, mask, false);
 
@@ -1369,7 +1369,7 @@ bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
 		}
 	}
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		write_protected |=
 			kvm_tdp_mmu_write_protect_gfn(kvm, slot, gfn, min_level);
 
@@ -1532,7 +1532,7 @@ bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)
 	if (kvm_memslots_have_rmaps(kvm))
 		flush = kvm_handle_gfn_range(kvm, range, kvm_zap_rmap);
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		flush = kvm_tdp_mmu_unmap_gfn_range(kvm, range, flush);
 
 	return flush;
@@ -1545,7 +1545,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	if (kvm_memslots_have_rmaps(kvm))
 		flush = kvm_handle_gfn_range(kvm, range, kvm_set_pte_rmap);
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		flush |= kvm_tdp_mmu_set_spte_gfn(kvm, range);
 
 	return flush;
@@ -1618,7 +1618,7 @@ bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	if (kvm_memslots_have_rmaps(kvm))
 		young = kvm_handle_gfn_range(kvm, range, kvm_age_rmap);
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		young |= kvm_tdp_mmu_age_gfn_range(kvm, range);
 
 	return young;
@@ -1631,7 +1631,7 @@ bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	if (kvm_memslots_have_rmaps(kvm))
 		young = kvm_handle_gfn_range(kvm, range, kvm_test_age_rmap);
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		young |= kvm_tdp_mmu_test_age_gfn(kvm, range);
 
 	return young;
@@ -3543,7 +3543,7 @@ static int mmu_alloc_direct_roots(struct kvm_vcpu *vcpu)
 	if (r < 0)
 		goto out_unlock;
 
-	if (is_tdp_mmu_enabled(vcpu->kvm)) {
+	if (tdp_enabled) {
 		root = kvm_tdp_mmu_get_vcpu_root_hpa(vcpu);
 		mmu->root.hpa = root;
 	} else if (shadow_root_level >= PT64_ROOT_4LEVEL) {
@@ -5922,7 +5922,7 @@ static void kvm_mmu_zap_all_fast(struct kvm *kvm)
 	 * write and in the same critical section as making the reload request,
 	 * e.g. before kvm_zap_obsolete_pages() could drop mmu_lock and yield.
 	 */
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		kvm_tdp_mmu_invalidate_all_roots(kvm);
 
 	/*
@@ -5947,7 +5947,7 @@ static void kvm_mmu_zap_all_fast(struct kvm *kvm)
 	 * Deferring the zap until the final reference to the root is put would
 	 * lead to use-after-free.
 	 */
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		kvm_tdp_mmu_zap_invalidated_roots(kvm);
 }
 
@@ -6059,7 +6059,7 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end)
 
 	flush = kvm_rmap_zap_gfn_range(kvm, gfn_start, gfn_end);
 
-	if (is_tdp_mmu_enabled(kvm)) {
+	if (tdp_enabled) {
 		for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
 			flush = kvm_tdp_mmu_zap_leafs(kvm, i, gfn_start,
 						      gfn_end, true, flush);
@@ -6095,7 +6095,7 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
 		write_unlock(&kvm->mmu_lock);
 	}
 
-	if (is_tdp_mmu_enabled(kvm)) {
+	if (tdp_enabled) {
 		read_lock(&kvm->mmu_lock);
 		flush |= kvm_tdp_mmu_wrprot_slot(kvm, memslot, start_level);
 		read_unlock(&kvm->mmu_lock);
@@ -6364,7 +6364,7 @@ void kvm_mmu_try_split_huge_pages(struct kvm *kvm,
 				   u64 start, u64 end,
 				   int target_level)
 {
-	if (!is_tdp_mmu_enabled(kvm))
+	if (!tdp_enabled)
 		return;
 
 	if (kvm_memslots_have_rmaps(kvm))
@@ -6385,7 +6385,7 @@ void kvm_mmu_slot_try_split_huge_pages(struct kvm *kvm,
 	u64 start = memslot->base_gfn;
 	u64 end = start + memslot->npages;
 
-	if (!is_tdp_mmu_enabled(kvm))
+	if (!tdp_enabled)
 		return;
 
 	if (kvm_memslots_have_rmaps(kvm)) {
@@ -6470,7 +6470,7 @@ void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
 		write_unlock(&kvm->mmu_lock);
 	}
 
-	if (is_tdp_mmu_enabled(kvm)) {
+	if (tdp_enabled) {
 		read_lock(&kvm->mmu_lock);
 		kvm_tdp_mmu_zap_collapsible_sptes(kvm, slot);
 		read_unlock(&kvm->mmu_lock);
@@ -6507,7 +6507,7 @@ void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm,
 		write_unlock(&kvm->mmu_lock);
 	}
 
-	if (is_tdp_mmu_enabled(kvm)) {
+	if (tdp_enabled) {
 		read_lock(&kvm->mmu_lock);
 		flush |= kvm_tdp_mmu_clear_dirty_slot(kvm, memslot);
 		read_unlock(&kvm->mmu_lock);
@@ -6542,7 +6542,7 @@ void kvm_mmu_zap_all(struct kvm *kvm)
 
 	kvm_mmu_commit_zap_page(kvm, &invalid_list);
 
-	if (is_tdp_mmu_enabled(kvm))
+	if (tdp_enabled)
 		kvm_tdp_mmu_zap_all(kvm);
 
 	write_unlock(&kvm->mmu_lock);
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index d6c30a648d8d..383162989645 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -22,8 +22,6 @@ int kvm_mmu_init_tdp_mmu(struct kvm *kvm)
 	if (!wq)
 		return -ENOMEM;
 
-	/* This should not be changed for the lifetime of the VM. */
-	kvm->arch.tdp_mmu_enabled = true;
 	INIT_LIST_HEAD(&kvm->arch.tdp_mmu_roots);
 	spin_lock_init(&kvm->arch.tdp_mmu_pages_lock);
 	INIT_LIST_HEAD(&kvm->arch.tdp_mmu_pages);
@@ -45,7 +43,7 @@ static __always_inline bool kvm_lockdep_assert_mmu_lock_held(struct kvm *kvm,
 
 void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
 {
-	if (!kvm->arch.tdp_mmu_enabled)
+	if (!tdp_enabled)
 		return;
 
 	/* Also waits for any queued work items.  */
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 3/9] KVM: x86/mmu: Consolidate mmu_seq calculations in kvm_faultin_pfn()

[David Matlack]

Calculate mmu_seq during kvm_faultin_pfn() and stash it in struct
kvm_page_fault. The eliminates duplicate code and reduces the amount of
parameters needed for is_page_fault_stale().

Note, the smp_rmb() needs a comment but that is out of scope for this
commit which is pure code motion.

No functional change intended.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c          | 14 ++++++--------
 arch/x86/kvm/mmu/mmu_internal.h |  1 +
 arch/x86/kvm/mmu/paging_tmpl.h  |  6 +-----
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 8c293a88d923..af1b7e7fb4fb 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4127,6 +4127,9 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	struct kvm_memory_slot *slot = fault->slot;
 	bool async;
 
+	fault->mmu_seq = vcpu->kvm->mmu_notifier_seq;
+	smp_rmb();
+
 	/*
 	 * Retry the page fault if the gfn hit a memslot that is being deleted
 	 * or moved.  This ensures any existing SPTEs for the old memslot will
@@ -4183,7 +4186,7 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
  * root was invalidated by a memslot update or a relevant mmu_notifier fired.
  */
 static bool is_page_fault_stale(struct kvm_vcpu *vcpu,
-				struct kvm_page_fault *fault, int mmu_seq)
+				struct kvm_page_fault *fault)
 {
 	struct kvm_mmu_page *sp = to_shadow_page(vcpu->arch.mmu->root.hpa);
 
@@ -4203,14 +4206,12 @@ static bool is_page_fault_stale(struct kvm_vcpu *vcpu,
 		return true;
 
 	return fault->slot &&
-	       mmu_notifier_retry_hva(vcpu->kvm, mmu_seq, fault->hva);
+	       mmu_notifier_retry_hva(vcpu->kvm, fault->mmu_seq, fault->hva);
 }
 
 static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 {
 	bool is_tdp_mmu_fault = is_tdp_mmu(vcpu->arch.mmu);
-
-	unsigned long mmu_seq;
 	int r;
 
 	fault->gfn = fault->addr >> PAGE_SHIFT;
@@ -4227,9 +4228,6 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	if (r)
 		return r;
 
-	mmu_seq = vcpu->kvm->mmu_notifier_seq;
-	smp_rmb();
-
 	r = kvm_faultin_pfn(vcpu, fault);
 	if (r != RET_PF_CONTINUE)
 		return r;
@@ -4245,7 +4243,7 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	else
 		write_lock(&vcpu->kvm->mmu_lock);
 
-	if (is_page_fault_stale(vcpu, fault, mmu_seq))
+	if (is_page_fault_stale(vcpu, fault))
 		goto out_unlock;
 
 	r = make_mmu_pages_available(vcpu);
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 582def531d4d..1c0a1e7c796d 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -221,6 +221,7 @@ struct kvm_page_fault {
 	struct kvm_memory_slot *slot;
 
 	/* Outputs of kvm_faultin_pfn.  */
+	unsigned long mmu_seq;
 	kvm_pfn_t pfn;
 	hva_t hva;
 	bool map_writable;
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index f5958071220c..a199db4acecc 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -791,7 +791,6 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 {
 	struct guest_walker walker;
 	int r;
-	unsigned long mmu_seq;
 	bool is_self_change_mapping;
 
 	pgprintk("%s: addr %lx err %x\n", __func__, fault->addr, fault->error_code);
@@ -838,9 +837,6 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	else
 		fault->max_level = walker.level;
 
-	mmu_seq = vcpu->kvm->mmu_notifier_seq;
-	smp_rmb();
-
 	r = kvm_faultin_pfn(vcpu, fault);
 	if (r != RET_PF_CONTINUE)
 		return r;
@@ -871,7 +867,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	r = RET_PF_RETRY;
 	write_lock(&vcpu->kvm->mmu_lock);
 
-	if (is_page_fault_stale(vcpu, fault, mmu_seq))
+	if (is_page_fault_stale(vcpu, fault))
 		goto out_unlock;
 
 	r = make_mmu_pages_available(vcpu);
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 4/9] KVM: x86/mmu: Rename __direct_map() to nonpaging_map()

[David Matlack]

Rename __direct_map() to nonpaging_map() since it is only used to handle
faults for non-paging guests on TDP-disabled hosts.

Opportunistically make some trivial cleanups to comments that had to be
modified anyway since they mentioned __direct_map(). Specifically, use
"()" when referring to functions, and include kvm_tdp_mmu_map() among
the various callers of disallowed_hugepage_adjust().

No functional change intended.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c          | 14 +++++++-------
 arch/x86/kvm/mmu/mmu_internal.h |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index af1b7e7fb4fb..3e03407f1321 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3072,11 +3072,11 @@ void disallowed_hugepage_adjust(struct kvm_page_fault *fault, u64 spte, int cur_
 	    is_shadow_present_pte(spte) &&
 	    !is_large_pte(spte)) {
 		/*
-		 * A small SPTE exists for this pfn, but FNAME(fetch)
-		 * and __direct_map would like to create a large PTE
-		 * instead: just force them to go down another level,
-		 * patching back for them into pfn the next 9 bits of
-		 * the address.
+		 * A small SPTE exists for this pfn, but FNAME(fetch),
+		 * nonpaging_map(), and kvm_tdp_mmu_map() would like to create a
+		 * large PTE instead: just force them to go down another level,
+		 * patching back for them into pfn the next 9 bits of the
+		 * address.
 		 */
 		u64 page_mask = KVM_PAGES_PER_HPAGE(cur_level) -
 				KVM_PAGES_PER_HPAGE(cur_level - 1);
@@ -3085,7 +3085,7 @@ void disallowed_hugepage_adjust(struct kvm_page_fault *fault, u64 spte, int cur_
 	}
 }
 
-static int __direct_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
+static int nonpaging_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 {
 	struct kvm_shadow_walk_iterator it;
 	struct kvm_mmu_page *sp;
@@ -4253,7 +4253,7 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	if (is_tdp_mmu_fault)
 		r = kvm_tdp_mmu_map(vcpu, fault);
 	else
-		r = __direct_map(vcpu, fault);
+		r = nonpaging_map(vcpu, fault);
 
 out_unlock:
 	if (is_tdp_mmu_fault)
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 1c0a1e7c796d..f65892c2fdeb 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -198,7 +198,7 @@ struct kvm_page_fault {
 
 	/*
 	 * Maximum page size that can be created for this fault; input to
-	 * FNAME(fetch), __direct_map and kvm_tdp_mmu_map.
+	 * FNAME(fetch), nonpaging_map() and kvm_tdp_mmu_map().
 	 */
 	u8 max_level;
 
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 5/9] KVM: x86/mmu: Separate TDP and non-paging fault handling

[David Matlack]

Separate the page fault handling for TDP faults and non-paging faults.
This creates some duplicate code in the short term, but makes each
routine simpler to read by eliminating branches and enables future
cleanups by allowing the two paths to diverge.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 77 +++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 3e03407f1321..182f9f417e4e 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4209,11 +4209,15 @@ static bool is_page_fault_stale(struct kvm_vcpu *vcpu,
 	       mmu_notifier_retry_hva(vcpu->kvm, fault->mmu_seq, fault->hva);
 }
 
-static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
+static int nonpaging_page_fault(struct kvm_vcpu *vcpu,
+				struct kvm_page_fault *fault)
 {
-	bool is_tdp_mmu_fault = is_tdp_mmu(vcpu->arch.mmu);
 	int r;
 
+	pgprintk("%s: gva %lx error %x\n", __func__, fault->addr, fault->error_code);
+
+	/* This path builds a PAE pagetable, we can map 2mb pages at maximum. */
+	fault->max_level = PG_LEVEL_2M;
 	fault->gfn = fault->addr >> PAGE_SHIFT;
 	fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
 
@@ -4237,11 +4241,7 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 		return r;
 
 	r = RET_PF_RETRY;
-
-	if (is_tdp_mmu_fault)
-		read_lock(&vcpu->kvm->mmu_lock);
-	else
-		write_lock(&vcpu->kvm->mmu_lock);
+	write_lock(&vcpu->kvm->mmu_lock);
 
 	if (is_page_fault_stale(vcpu, fault))
 		goto out_unlock;
@@ -4250,30 +4250,14 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	if (r)
 		goto out_unlock;
 
-	if (is_tdp_mmu_fault)
-		r = kvm_tdp_mmu_map(vcpu, fault);
-	else
-		r = nonpaging_map(vcpu, fault);
+	r = nonpaging_map(vcpu, fault);
 
 out_unlock:
-	if (is_tdp_mmu_fault)
-		read_unlock(&vcpu->kvm->mmu_lock);
-	else
-		write_unlock(&vcpu->kvm->mmu_lock);
+	write_unlock(&vcpu->kvm->mmu_lock);
 	kvm_release_pfn_clean(fault->pfn);
 	return r;
 }
 
-static int nonpaging_page_fault(struct kvm_vcpu *vcpu,
-				struct kvm_page_fault *fault)
-{
-	pgprintk("%s: gva %lx error %x\n", __func__, fault->addr, fault->error_code);
-
-	/* This path builds a PAE pagetable, we can map 2mb pages at maximum. */
-	fault->max_level = PG_LEVEL_2M;
-	return direct_page_fault(vcpu, fault);
-}
-
 int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 				u64 fault_address, char *insn, int insn_len)
 {
@@ -4309,6 +4293,11 @@ EXPORT_SYMBOL_GPL(kvm_handle_page_fault);
 
 int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 {
+	int r;
+
+	fault->gfn = fault->addr >> PAGE_SHIFT;
+	fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
+
 	/*
 	 * If the guest's MTRRs may be used to compute the "real" memtype,
 	 * restrict the mapping level to ensure KVM uses a consistent memtype
@@ -4324,14 +4313,48 @@ int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	if (shadow_memtype_mask && kvm_arch_has_noncoherent_dma(vcpu->kvm)) {
 		for ( ; fault->max_level > PG_LEVEL_4K; --fault->max_level) {
 			int page_num = KVM_PAGES_PER_HPAGE(fault->max_level);
-			gfn_t base = (fault->addr >> PAGE_SHIFT) & ~(page_num - 1);
+			gfn_t base = fault->gfn & ~(page_num - 1);
 
 			if (kvm_mtrr_check_gfn_range_consistency(vcpu, base, page_num))
 				break;
 		}
 	}
 
-	return direct_page_fault(vcpu, fault);
+	if (page_fault_handle_page_track(vcpu, fault))
+		return RET_PF_EMULATE;
+
+	r = fast_page_fault(vcpu, fault);
+	if (r != RET_PF_INVALID)
+		return r;
+
+	r = mmu_topup_memory_caches(vcpu, false);
+	if (r)
+		return r;
+
+	r = kvm_faultin_pfn(vcpu, fault);
+	if (r != RET_PF_CONTINUE)
+		return r;
+
+	r = handle_abnormal_pfn(vcpu, fault, ACC_ALL);
+	if (r != RET_PF_CONTINUE)
+		return r;
+
+	r = RET_PF_RETRY;
+	read_lock(&vcpu->kvm->mmu_lock);
+
+	if (is_page_fault_stale(vcpu, fault))
+		goto out_unlock;
+
+	r = make_mmu_pages_available(vcpu);
+	if (r)
+		goto out_unlock;
+
+	r = kvm_tdp_mmu_map(vcpu, fault);
+
+out_unlock:
+	read_unlock(&vcpu->kvm->mmu_lock);
+	kvm_release_pfn_clean(fault->pfn);
+	return r;
 }
 
 static void nonpaging_init_context(struct kvm_mmu *context)
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 6/9] KVM: x86/mmu: Stop needlessly making MMU pages available for TDP MMU faults

[David Matlack]

Stop calling make_mmu_pages_available() when handling TDP MMU faults.
The TDP MMU does not participate in the "available MMU pages" tracking
and limiting so calling this function is unnecessary work when handling
TDP MMU faults.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 182f9f417e4e..6613ae387e1b 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4345,10 +4345,6 @@ int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	if (is_page_fault_stale(vcpu, fault))
 		goto out_unlock;
 
-	r = make_mmu_pages_available(vcpu);
-	if (r)
-		goto out_unlock;
-
 	r = kvm_tdp_mmu_map(vcpu, fault);
 
 out_unlock:
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 7/9] KVM: x86/mmu: Handle "error PFNs" in kvm_faultin_pfn()

[David Matlack]

Handle "error PFNs" directly in kvm_faultin_pfn() rather than relying on
the caller to invoke handle_abnormal_pfn() after kvm_faultin_pfn().
Opportunistically rename kvm_handle_bad_page() to kvm_handle_error_pfn()
to make it more consistent with e.g. is_error_pfn().

The reason for this change is to reduce the number of things being
handled in handle_abnormal_pfn(), which is currently a grab bag for edge
conditions (the other of which being updating the vCPU MMIO cache).

No functional change intended.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6613ae387e1b..36960ea0d4ef 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3134,7 +3134,7 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *
 	send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, PAGE_SHIFT, tsk);
 }
 
-static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
+static int kvm_handle_error_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 {
 	/*
 	 * Do not cache the mmio info caused by writing the readonly gfn
@@ -3155,10 +3155,6 @@ static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 static int handle_abnormal_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
 			       unsigned int access)
 {
-	/* The pfn is invalid, report the error! */
-	if (unlikely(is_error_pfn(fault->pfn)))
-		return kvm_handle_bad_page(vcpu, fault->gfn, fault->pfn);
-
 	if (unlikely(!fault->slot)) {
 		gva_t gva = fault->is_tdp ? 0 : fault->addr;
 
@@ -4144,7 +4140,7 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 			fault->slot = NULL;
 			fault->pfn = KVM_PFN_NOSLOT;
 			fault->map_writable = false;
-			return RET_PF_CONTINUE;
+			goto out;
 		}
 		/*
 		 * If the APIC access page exists but is disabled, go directly
@@ -4162,7 +4158,7 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 					  fault->write, &fault->map_writable,
 					  &fault->hva);
 	if (!async)
-		return RET_PF_CONTINUE; /* *pfn has correct page already */
+		goto out; /* *pfn has correct page already */
 
 	if (!fault->prefetch && kvm_can_do_async_pf(vcpu)) {
 		trace_kvm_try_async_get_page(fault->addr, fault->gfn);
@@ -4178,6 +4174,11 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	fault->pfn = __gfn_to_pfn_memslot(slot, fault->gfn, false, NULL,
 					  fault->write, &fault->map_writable,
 					  &fault->hva);
+
+out:
+	if (unlikely(is_error_pfn(fault->pfn)))
+		return kvm_handle_error_pfn(vcpu, fault->gfn, fault->pfn);
+
 	return RET_PF_CONTINUE;
 }
 
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 8/9] KVM: x86/mmu: Avoid memslot lookup during KVM_PFN_ERR_HWPOISON handling

[David Matlack]

Pass the kvm_page_fault struct down to kvm_handle_error_pfn() to avoid a
memslot lookup when handling KVM_PFN_ERR_HWPOISON. Opportunistically
move the gfn_to_hva_memslot() call and @current down into
kvm_send_hwpoison_signal() to cut down on line lengths.

No functional change intended.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 36960ea0d4ef..47f4d1e81db1 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3129,23 +3129,25 @@ static int nonpaging_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	return ret;
 }
 
-static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *tsk)
+static void kvm_send_hwpoison_signal(struct kvm_memory_slot *slot, gfn_t gfn)
 {
-	send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, PAGE_SHIFT, tsk);
+	unsigned long hva = gfn_to_hva_memslot(slot, gfn);
+
+	send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva, PAGE_SHIFT, current);
 }
 
-static int kvm_handle_error_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
+static int kvm_handle_error_pfn(struct kvm_page_fault *fault)
 {
 	/*
 	 * Do not cache the mmio info caused by writing the readonly gfn
 	 * into the spte otherwise read access on readonly gfn also can
 	 * caused mmio page fault and treat it as mmio access.
 	 */
-	if (pfn == KVM_PFN_ERR_RO_FAULT)
+	if (fault->pfn == KVM_PFN_ERR_RO_FAULT)
 		return RET_PF_EMULATE;
 
-	if (pfn == KVM_PFN_ERR_HWPOISON) {
-		kvm_send_hwpoison_signal(kvm_vcpu_gfn_to_hva(vcpu, gfn), current);
+	if (fault->pfn == KVM_PFN_ERR_HWPOISON) {
+		kvm_send_hwpoison_signal(fault->slot, fault->gfn);
 		return RET_PF_RETRY;
 	}
 
@@ -4177,7 +4179,7 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 
 out:
 	if (unlikely(is_error_pfn(fault->pfn)))
-		return kvm_handle_error_pfn(vcpu, fault->gfn, fault->pfn);
+		return kvm_handle_error_pfn(fault);
 
 	return RET_PF_CONTINUE;
 }
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 9/9] KVM: x86/mmu: Try to handle no-slot faults during kvm_faultin_pfn()

[David Matlack]

Try to handle faults on GFNs that do not have a backing memslot during
kvm_faultin_pfn(), rather than relying on the caller to call
handle_abnormal_pfn() right after kvm_faultin_pfn(). This reduces all of
the page fault paths by eliminating duplicate code.

Opportunistically tweak the comment about handling gfn > host.MAXPHYADDR
to reflect that the effect of returning RET_PF_EMULATE at that point is
to avoid creating an MMIO SPTE for such GFNs.

No functional change intended.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c         | 55 +++++++++++++++++-----------------
 arch/x86/kvm/mmu/paging_tmpl.h |  4 ---
 2 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 47f4d1e81db1..741b92b1f004 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3154,28 +3154,32 @@ static int kvm_handle_error_pfn(struct kvm_page_fault *fault)
 	return -EFAULT;
 }
 
-static int handle_abnormal_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
-			       unsigned int access)
+static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
+				   struct kvm_page_fault *fault,
+				   unsigned int access)
 {
-	if (unlikely(!fault->slot)) {
-		gva_t gva = fault->is_tdp ? 0 : fault->addr;
+	gva_t gva = fault->is_tdp ? 0 : fault->addr;
 
-		vcpu_cache_mmio_info(vcpu, gva, fault->gfn,
-				     access & shadow_mmio_access_mask);
-		/*
-		 * If MMIO caching is disabled, emulate immediately without
-		 * touching the shadow page tables as attempting to install an
-		 * MMIO SPTE will just be an expensive nop.  Do not cache MMIO
-		 * whose gfn is greater than host.MAXPHYADDR, any guest that
-		 * generates such gfns is running nested and is being tricked
-		 * by L0 userspace (you can observe gfn > L1.MAXPHYADDR if
-		 * and only if L1's MAXPHYADDR is inaccurate with respect to
-		 * the hardware's).
-		 */
-		if (unlikely(!enable_mmio_caching) ||
-		    unlikely(fault->gfn > kvm_mmu_max_gfn()))
-			return RET_PF_EMULATE;
-	}
+	vcpu_cache_mmio_info(vcpu, gva, fault->gfn,
+			     access & shadow_mmio_access_mask);
+
+	/*
+	 * If MMIO caching is disabled, emulate immediately without
+	 * touching the shadow page tables as attempting to install an
+	 * MMIO SPTE will just be an expensive nop.
+	 */
+	if (unlikely(!enable_mmio_caching))
+		return RET_PF_EMULATE;
+
+	/*
+	 * Do not create an MMIO SPTE for a gfn greater than host.MAXPHYADDR,
+	 * any guest that generates such gfns is running nested and is being
+	 * tricked by L0 userspace (you can observe gfn > L1.MAXPHYADDR if and
+	 * only if L1's MAXPHYADDR is inaccurate with respect to the
+	 * hardware's).
+	 */
+	if (unlikely(fault->gfn > kvm_mmu_max_gfn()))
+		return RET_PF_EMULATE;
 
 	return RET_PF_CONTINUE;
 }
@@ -4181,6 +4185,9 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	if (unlikely(is_error_pfn(fault->pfn)))
 		return kvm_handle_error_pfn(fault);
 
+	if (unlikely(!fault->slot))
+		return kvm_handle_noslot_fault(vcpu, fault, ACC_ALL);
+
 	return RET_PF_CONTINUE;
 }
 
@@ -4239,10 +4246,6 @@ static int nonpaging_page_fault(struct kvm_vcpu *vcpu,
 	if (r != RET_PF_CONTINUE)
 		return r;
 
-	r = handle_abnormal_pfn(vcpu, fault, ACC_ALL);
-	if (r != RET_PF_CONTINUE)
-		return r;
-
 	r = RET_PF_RETRY;
 	write_lock(&vcpu->kvm->mmu_lock);
 
@@ -4338,10 +4341,6 @@ int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	if (r != RET_PF_CONTINUE)
 		return r;
 
-	r = handle_abnormal_pfn(vcpu, fault, ACC_ALL);
-	if (r != RET_PF_CONTINUE)
-		return r;
-
 	r = RET_PF_RETRY;
 	read_lock(&vcpu->kvm->mmu_lock);
 
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index a199db4acecc..cf19227e842c 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -841,10 +841,6 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	if (r != RET_PF_CONTINUE)
 		return r;
 
-	r = handle_abnormal_pfn(vcpu, fault, walker.pte_access);
-	if (r != RET_PF_CONTINUE)
-		return r;
-
 	/*
 	 * Do not change pte_access if the pfn is a mmio page, otherwise
 	 * we will cache the incorrect access into mmio spte.
-- 
2.37.1.595.g718a3a8f04-goog

Re: [PATCH 9/9] KVM: x86/mmu: Try to handle no-slot faults during kvm_faultin_pfn()

[David Matlack]

On Mon, Aug 15, 2022 at 4:01 PM David Matlack <dmatlack@google.com> wrote:
>
> Try to handle faults on GFNs that do not have a backing memslot during
> kvm_faultin_pfn(), rather than relying on the caller to call
> handle_abnormal_pfn() right after kvm_faultin_pfn(). This reduces all of
> the page fault paths by eliminating duplicate code.
>
> Opportunistically tweak the comment about handling gfn > host.MAXPHYADDR
> to reflect that the effect of returning RET_PF_EMULATE at that point is
> to avoid creating an MMIO SPTE for such GFNs.
>
> No functional change intended.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  arch/x86/kvm/mmu/mmu.c         | 55 +++++++++++++++++-----------------
>  arch/x86/kvm/mmu/paging_tmpl.h |  4 ---
>  2 files changed, 27 insertions(+), 32 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
[...]
> @@ -4181,6 +4185,9 @@ static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>         if (unlikely(is_error_pfn(fault->pfn)))
>                 return kvm_handle_error_pfn(fault);
>
> +       if (unlikely(!fault->slot))
> +               return kvm_handle_noslot_fault(vcpu, fault, ACC_ALL);

This is broken. This needs to be pte_access for the shadow paging
case, not ACC_ALL. I remember now I had that in an earlier version but
it got lost at some point when I was rebasing locally.

Re: [PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[Peter Zijlstra]

On Mon, Aug 15, 2022 at 04:01:01PM -0700, David Matlack wrote:
> Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
> use the TDP MMU when TDP hardware support is enabled.  The rest of the
> patches are related cleanups that follow (although the kvm_faultin_pfn()
> cleanups at the end are only tangentially related at best).
> 
> The TDP MMU was introduced in 5.10 and has been enabled by default since
> 5.15. At this point there are no known functionality gaps between the
> TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> better with the number of vCPUs. In other words, there is no good reason
> to disable the TDP MMU.

Then how are you going to test the shadow mmu code -- which I assume is
still relevant for the platforms that don't have this hardware support
you speak of?

Re: [PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[David Matlack]

On Tue, Aug 16, 2022 at 1:17 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Aug 15, 2022 at 04:01:01PM -0700, David Matlack wrote:
> > Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
> > use the TDP MMU when TDP hardware support is enabled.  The rest of the
> > patches are related cleanups that follow (although the kvm_faultin_pfn()
> > cleanups at the end are only tangentially related at best).
> >
> > The TDP MMU was introduced in 5.10 and has been enabled by default since
> > 5.15. At this point there are no known functionality gaps between the
> > TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> > better with the number of vCPUs. In other words, there is no good reason
> > to disable the TDP MMU.
>
> Then how are you going to test the shadow mmu code -- which I assume is
> still relevant for the platforms that don't have this hardware support
> you speak of?

TDP hardware support can still be disabled with module parameters
(kvm_intel.ept=N and kvm_amd.npt=N).

The tdp_mmu module parameter only controls whether KVM uses the TDP
MMU or shadow MMU *when TDP hardware is enabled*.

Re: [PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[David Matlack]

On Mon, Aug 15, 2022 at 4:01 PM David Matlack <dmatlack@google.com> wrote:
>
> Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
> use the TDP MMU when TDP hardware support is enabled.  The rest of the
> patches are related cleanups that follow (although the kvm_faultin_pfn()
> cleanups at the end are only tangentially related at best).

Please feel free to ignore the kvm_faultin_pfn() cleanups, i.e.
patches 7-9. They conflict with Peter Xu's series [1], there is a bug
in patch 9, and they are only tangentially related.

[1] https://lore.kernel.org/kvm/20220721000318.93522-4-peterx@redhat.com/

Re: [PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[Peter Zijlstra]

On Tue, Aug 16, 2022 at 09:30:54AM -0700, David Matlack wrote:
> On Tue, Aug 16, 2022 at 1:17 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Mon, Aug 15, 2022 at 04:01:01PM -0700, David Matlack wrote:
> > > Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
> > > use the TDP MMU when TDP hardware support is enabled.  The rest of the
> > > patches are related cleanups that follow (although the kvm_faultin_pfn()
> > > cleanups at the end are only tangentially related at best).
> > >
> > > The TDP MMU was introduced in 5.10 and has been enabled by default since
> > > 5.15. At this point there are no known functionality gaps between the
> > > TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> > > better with the number of vCPUs. In other words, there is no good reason
> > > to disable the TDP MMU.
> >
> > Then how are you going to test the shadow mmu code -- which I assume is
> > still relevant for the platforms that don't have this hardware support
> > you speak of?
> 
> TDP hardware support can still be disabled with module parameters
> (kvm_intel.ept=N and kvm_amd.npt=N).
> 
> The tdp_mmu module parameter only controls whether KVM uses the TDP
> MMU or shadow MMU *when TDP hardware is enabled*.

Ah, fair enough. Carry on.

Re: [PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[Huang, Kai]

On Tue, 2022-08-16 at 09:30 -0700, David Matlack wrote:
> On Tue, Aug 16, 2022 at 1:17 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > On Mon, Aug 15, 2022 at 04:01:01PM -0700, David Matlack wrote:
> > > Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
> > > use the TDP MMU when TDP hardware support is enabled.  The rest of the
> > > patches are related cleanups that follow (although the kvm_faultin_pfn()
> > > cleanups at the end are only tangentially related at best).
> > > 
> > > The TDP MMU was introduced in 5.10 and has been enabled by default since
> > > 5.15. At this point there are no known functionality gaps between the
> > > TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> > > better with the number of vCPUs. In other words, there is no good reason
> > > to disable the TDP MMU.
> > 
> > Then how are you going to test the shadow mmu code -- which I assume is
> > still relevant for the platforms that don't have this hardware support
> > you speak of?
> 
> TDP hardware support can still be disabled with module parameters
> (kvm_intel.ept=N and kvm_amd.npt=N).
> 
> The tdp_mmu module parameter only controls whether KVM uses the TDP
> MMU or shadow MMU *when TDP hardware is enabled*.

With the tdp_mmu module parameter, when we develop some code, we can at least
easily test legacy MMU code (that it is still working) when *TDP hardware is
enabled* by turning the parameter off.  Or when there's some problem with TDP
MMU code, we can easily switch to use legacy MMU.

Do we want to lose those flexibilities?

-- 
Thanks,
-Kai

Re: [PATCH 1/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[Paolo Bonzini]

On 8/16/22 01:01, David Matlack wrote:
> Delete the module parameter tdp_mmu and force KVM to always use the TDP
> MMU when TDP hardware support is enabled.
> 
> The TDP MMU was introduced in 5.10 and has been enabled by default since
> 5.15. At this point there are no known functionality gaps between the
> TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> better with the number of vCPUs. In other words, there is no good reason
> to disable the TDP MMU.
> 
> Dropping the ability to disable the TDP MMU reduces the number of
> possible configurations that need to be tested to validate KVM (i.e. no
> need to test with tdp_mmu=N), and simplifies the code.

The snag is that the shadow MMU is only supported on 64-bit systems; 
testing tdp_mmu=0 is not a full replacement for booting up a 32-bit 
distro, but it's easier (I do 32-bit testing only with nested virt). 
Personally I'd have no problem restricting KVM to x86-64 but I know 
people would complain.

What about making the tdp_mmu module parameter read-only, so that at 
least kvm->arch.tdp_mmu_enabled can be replaced by a global variable?

Paolo

Re: [PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[David Matlack]

On Wed, Aug 17, 2022 at 3:01 AM Huang, Kai <kai.huang@intel.com> wrote:
>
> On Tue, 2022-08-16 at 09:30 -0700, David Matlack wrote:
> > On Tue, Aug 16, 2022 at 1:17 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Mon, Aug 15, 2022 at 04:01:01PM -0700, David Matlack wrote:
> > > > Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
> > > > use the TDP MMU when TDP hardware support is enabled.  The rest of the
> > > > patches are related cleanups that follow (although the kvm_faultin_pfn()
> > > > cleanups at the end are only tangentially related at best).
> > > >
> > > > The TDP MMU was introduced in 5.10 and has been enabled by default since
> > > > 5.15. At this point there are no known functionality gaps between the
> > > > TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> > > > better with the number of vCPUs. In other words, there is no good reason
> > > > to disable the TDP MMU.
> > >
> > > Then how are you going to test the shadow mmu code -- which I assume is
> > > still relevant for the platforms that don't have this hardware support
> > > you speak of?
> >
> > TDP hardware support can still be disabled with module parameters
> > (kvm_intel.ept=N and kvm_amd.npt=N).
> >
> > The tdp_mmu module parameter only controls whether KVM uses the TDP
> > MMU or shadow MMU *when TDP hardware is enabled*.
>
> With the tdp_mmu module parameter, when we develop some code, we can at least
> easily test legacy MMU code (that it is still working) when *TDP hardware is
> enabled* by turning the parameter off.

I am proposing that KVM stops supporting this use-case, so testing it
would no longer be necessary. However, based on Paolo's reply there
might be a snag with 32-bit systems.

> Or when there's some problem with TDP
> MMU code, we can easily switch to use legacy MMU.

Who is "we" in this context? For cloud providers, switching a customer
VM from the TDP MMU back to the shadow MMU to fix an issue is not
feasible. The shadow MMU uses more memory and has worse performance
for larger VMs, i.e. switching comes with significant downsides that
are unlikely to be lower risk than simply rolling out a fix for the
TDP MMU.

Also, over time, the TDP MMU will be less and less likely to have bugs
than the shadow MMU for TDP-enabled use-cases. e.g. The TDP MMU has
been default enabled since 5.15, so it has likely received
significantly more test cycles than the shadow MMU in the past 5
releases.


>
> Do we want to lose those flexibilities?

>
> --
> Thanks,
> -Kai
>
>

Re: [PATCH 1/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[David Matlack]

On Wed, Aug 17, 2022 at 3:05 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 8/16/22 01:01, David Matlack wrote:
> > Delete the module parameter tdp_mmu and force KVM to always use the TDP
> > MMU when TDP hardware support is enabled.
> >
> > The TDP MMU was introduced in 5.10 and has been enabled by default since
> > 5.15. At this point there are no known functionality gaps between the
> > TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> > better with the number of vCPUs. In other words, there is no good reason
> > to disable the TDP MMU.
> >
> > Dropping the ability to disable the TDP MMU reduces the number of
> > possible configurations that need to be tested to validate KVM (i.e. no
> > need to test with tdp_mmu=N), and simplifies the code.
>
> The snag is that the shadow MMU is only supported on 64-bit systems;
> testing tdp_mmu=0 is not a full replacement for booting up a 32-bit
> distro, but it's easier (I do 32-bit testing only with nested virt).

Ah, I did forget about 32-bit systems :(. Do Intel or AMD CPUs support
TDP in 32-bit mode?

> Personally I'd have no problem restricting KVM to x86-64 but I know
> people would complain.

As a middle-ground we could stop supporting TDP on 32-bit
systems. 32-bit KVM would continue working but just with shadow
paging.


>
> What about making the tdp_mmu module parameter read-only, so that at
> least kvm->arch.tdp_mmu_enabled can be replaced by a global variable?
>
> Paolo
>

Re: [PATCH 1/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[Paolo Bonzini]

On 8/17/22 18:49, David Matlack wrote:
> On Wed, Aug 17, 2022 at 3:05 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 8/16/22 01:01, David Matlack wrote:
>>> Delete the module parameter tdp_mmu and force KVM to always use the TDP
>>> MMU when TDP hardware support is enabled.
>>>
>>> The TDP MMU was introduced in 5.10 and has been enabled by default since
>>> 5.15. At this point there are no known functionality gaps between the
>>> TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
>>> better with the number of vCPUs. In other words, there is no good reason
>>> to disable the TDP MMU.
>>>
>>> Dropping the ability to disable the TDP MMU reduces the number of
>>> possible configurations that need to be tested to validate KVM (i.e. no
>>> need to test with tdp_mmu=N), and simplifies the code.
>>
>> The snag is that the shadow MMU is only supported on 64-bit systems;
>> testing tdp_mmu=0 is not a full replacement for booting up a 32-bit
>> distro, but it's easier (I do 32-bit testing only with nested virt).
> 
> Ah, I did forget about 32-bit systems :(. Do Intel or AMD CPUs support
> TDP in 32-bit mode?

Both do.  Intel theoretically on some 32-bit processors that were 
actually sold, too.

>> Personally I'd have no problem restricting KVM to x86-64 but I know
>> people would complain.
> 
> As a middle-ground we could stop supporting TDP on 32-bit
> systems. 32-bit KVM would continue working but just with shadow
> paging.

The main problem is, shadow paging is awfully slow due to Meltdown 
mitigations these days.  I would start with the read-only parameter and 
then see whether there's more low-hanging fruit (e.g. make fast page 
fault TDP MMU-only).

Paolo

>> What about making the tdp_mmu module parameter read-only, so that at
>> least kvm->arch.tdp_mmu_enabled can be replaced by a global variable?
>>
>> Paolo
>>
> 

Re: [PATCH 1/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[David Matlack]

On Wed, Aug 17, 2022 at 9:53 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 8/17/22 18:49, David Matlack wrote:
> > On Wed, Aug 17, 2022 at 3:05 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> On 8/16/22 01:01, David Matlack wrote:
> >>> Delete the module parameter tdp_mmu and force KVM to always use the TDP
> >>> MMU when TDP hardware support is enabled.
> >>>
> >>> The TDP MMU was introduced in 5.10 and has been enabled by default since
> >>> 5.15. At this point there are no known functionality gaps between the
> >>> TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> >>> better with the number of vCPUs. In other words, there is no good reason
> >>> to disable the TDP MMU.
> >>>
> >>> Dropping the ability to disable the TDP MMU reduces the number of
> >>> possible configurations that need to be tested to validate KVM (i.e. no
> >>> need to test with tdp_mmu=N), and simplifies the code.
> >>
> >> The snag is that the shadow MMU is only supported on 64-bit systems;
> >> testing tdp_mmu=0 is not a full replacement for booting up a 32-bit
> >> distro, but it's easier (I do 32-bit testing only with nested virt).
> >
> > Ah, I did forget about 32-bit systems :(. Do Intel or AMD CPUs support
> > TDP in 32-bit mode?
>
> Both do.  Intel theoretically on some 32-bit processors that were
> actually sold, too.
>
> >> Personally I'd have no problem restricting KVM to x86-64 but I know
> >> people would complain.
> >
> > As a middle-ground we could stop supporting TDP on 32-bit
> > systems. 32-bit KVM would continue working but just with shadow
> > paging.
>
> The main problem is, shadow paging is awfully slow due to Meltdown
> mitigations these days.  I would start with the read-only parameter and
> then see whether there's more low-hanging fruit (e.g. make fast page
> fault TDP MMU-only).

Will do, thanks for the feedback.

>
> Paolo
>
> >> What about making the tdp_mmu module parameter read-only, so that at
> >> least kvm->arch.tdp_mmu_enabled can be replaced by a global variable?
> >>
> >> Paolo
> >>
> >
>

Re: [PATCH 0/9] KVM: x86/mmu: Always enable the TDP MMU when TDP is enabled

[Huang, Kai]

On Wed, 2022-08-17 at 09:42 -0700, David Matlack wrote:
> On Wed, Aug 17, 2022 at 3:01 AM Huang, Kai <kai.huang@intel.com> wrote:
> > 
> > On Tue, 2022-08-16 at 09:30 -0700, David Matlack wrote:
> > > On Tue, Aug 16, 2022 at 1:17 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > > > 
> > > > On Mon, Aug 15, 2022 at 04:01:01PM -0700, David Matlack wrote:
> > > > > Patch 1 deletes the module parameter tdp_mmu and forces KVM to always
> > > > > use the TDP MMU when TDP hardware support is enabled.  The rest of the
> > > > > patches are related cleanups that follow (although the kvm_faultin_pfn()
> > > > > cleanups at the end are only tangentially related at best).
> > > > > 
> > > > > The TDP MMU was introduced in 5.10 and has been enabled by default since
> > > > > 5.15. At this point there are no known functionality gaps between the
> > > > > TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> > > > > better with the number of vCPUs. In other words, there is no good reason
> > > > > to disable the TDP MMU.
> > > > 
> > > > Then how are you going to test the shadow mmu code -- which I assume is
> > > > still relevant for the platforms that don't have this hardware support
> > > > you speak of?
> > > 
> > > TDP hardware support can still be disabled with module parameters
> > > (kvm_intel.ept=N and kvm_amd.npt=N).
> > > 
> > > The tdp_mmu module parameter only controls whether KVM uses the TDP
> > > MMU or shadow MMU *when TDP hardware is enabled*.
> > 
> > With the tdp_mmu module parameter, when we develop some code, we can at least
> > easily test legacy MMU code (that it is still working) when *TDP hardware is
> > enabled* by turning the parameter off.
> 
> I am proposing that KVM stops supporting this use-case, so testing it
> would no longer be necessary. However, based on Paolo's reply there
> might be a snag with 32-bit systems.
> 
> > Or when there's some problem with TDP
> > MMU code, we can easily switch to use legacy MMU.
> 
> Who is "we" in this context? For cloud providers, switching a customer
> VM from the TDP MMU back to the shadow MMU to fix an issue is not
> feasible. The shadow MMU uses more memory and has worse performance
> for larger VMs, i.e. switching comes with significant downsides that
> are unlikely to be lower risk than simply rolling out a fix for the
> TDP MMU.
> 
> Also, over time, the TDP MMU will be less and less likely to have bugs
> than the shadow MMU for TDP-enabled use-cases. e.g. The TDP MMU has
> been default enabled since 5.15, so it has likely received
> significantly more test cycles than the shadow MMU in the past 5
> releases.

"We" means normal KVM patch developers.  Yes I agree "switching to use legacy
MMU" isn't a good argument from cloud providers' point of view.


-- 
Thanks,
-Kai

Re: [PATCH 2/9] KVM: x86/mmu: Drop kvm->arch.tdp_mmu_enabled

[kernel test robot]

Hi David,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 93472b79715378a2386598d6632c654a2223267b]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Matlack/KVM-x86-mmu-Always-enable-the-TDP-MMU-when-TDP-is-enabled/20220816-135710
base:   93472b79715378a2386598d6632c654a2223267b
config: i386-debian-10.3-kselftests (https://download.01.org/0day-ci/archive/20220824/202208242248.quWrDOnO-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/05144150ed5b370369565be6b75d7504e64f3ba7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review David-Matlack/KVM-x86-mmu-Always-enable-the-TDP-MMU-when-TDP-is-enabled/20220816-135710
        git checkout 05144150ed5b370369565be6b75d7504e64f3ba7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "kvm_tdp_mmu_test_age_gfn" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_zap_all" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_clear_dirty_pt_masked" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_age_gfn_range" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_zap_leafs" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_unmap_gfn_range" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_invalidate_all_roots" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_get_vcpu_root_hpa" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_set_spte_gfn" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_wrprot_slot" [arch/x86/kvm/kvm.ko] undefined!
WARNING: modpost: suppressed 5 unresolved symbol warnings because there were too many)

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 5/9] KVM: x86/mmu: Separate TDP and non-paging fault handling

[kernel test robot]

Hi David,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 93472b79715378a2386598d6632c654a2223267b]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Matlack/KVM-x86-mmu-Always-enable-the-TDP-MMU-when-TDP-is-enabled/20220816-135710
base:   93472b79715378a2386598d6632c654a2223267b
config: i386-debian-10.3-kselftests (https://download.01.org/0day-ci/archive/20220825/202208250034.Vo6dL7C7-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/0cbb5e1684635e3f36d0283f5b3696b0ee0660e1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review David-Matlack/KVM-x86-mmu-Always-enable-the-TDP-MMU-when-TDP-is-enabled/20220816-135710
        git checkout 0cbb5e1684635e3f36d0283f5b3696b0ee0660e1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

ERROR: modpost: "kvm_tdp_mmu_test_age_gfn" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_zap_all" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_clear_dirty_pt_masked" [arch/x86/kvm/kvm.ko] undefined!
>> ERROR: modpost: "kvm_tdp_mmu_map" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_age_gfn_range" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_zap_leafs" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_unmap_gfn_range" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_invalidate_all_roots" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_get_vcpu_root_hpa" [arch/x86/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_tdp_mmu_set_spte_gfn" [arch/x86/kvm/kvm.ko] undefined!
WARNING: modpost: suppressed 6 unresolved symbol warnings because there were too many)

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp