[PATCH v4 0/6] KVM: x86/mmu: Fix wrong usages of range-based tlb flushing

[PATCH v4 0/6] KVM: x86/mmu: Fix wrong usages of range-based tlb flushing

[Hou Wenlong]

Commit c3134ce240eed ("KVM: Replace old tlb flush function with new one
to flush a specified range.") replaces old tlb flush function with
kvm_flush_remote_tlbs_with_address() to do tlb flushing. However, the
gfn range of tlb flushing is wrong in some cases. E.g., when a spte is
dropped, the start gfn of tlb flushing should be the gfn of spte not the
base gfn of SP which contains the spte. Although, as Paolo said, Hyper-V
may treat a 1-page flush the same if the address points to a huge page,
and no fixes are reported so far. So it seems that it works well for
Hyper-V. But it would be better to use the correct size for huge page.
So this patchset would fix them and introduce some helper functions as
David suggested to make the code clear.

Changed from v3:
- Move patch 1 after kvm_flush_remote_tlbs_sptep() is introduced,
  Drop kvm_flush_remote_tlbs_direct_sp() helper and use
  kvm_flush_remote_tlbs_sptep() instead.
- Wrap changelogs at ~75 chars.

Changed from v2:
- Introduce kvm_flush_remote_tlbs_gfn() in Patch 1 early.
- Move round_gfn_for_level() in tdp_iter.c into mmu_internal.h for
  common usage and cleanup the call sites of rounding down the GFN.
- Drop Patch 6.

Changed from v1:
- Align down gfn in kvm_set_pte_rmapp() instead of change iterator->gfn
  in rmap_walk_init_level() in Patch 2.
- Introduce some helper functions for common operations as David
  suggested.

v3: https://lore.kernel.org/kvm/cover.1663929851.git.houwenlong.hwl@antgroup.com

Hou Wenlong (6):
  KVM: x86/mmu: Move round_gfn_for_level() helper into mmu_internal.h
  KVM: x86/mmu: Fix wrong gfn range of tlb flushing in
    kvm_set_pte_rmapp()
  KVM: x86/mmu: Reduce gfn range of tlb flushing in
    tdp_mmu_map_handle_target_level()
  KVM: x86/mmu: Fix wrong start gfn of tlb flushing with range
  KVM: x86/mmu: Fix wrong gfn range of tlb flushing in
    validate_direct_spte()
  KVM: x86/mmu: Cleanup range-based flushing for given page

 arch/x86/kvm/mmu/mmu.c          | 36 +++++++++++++++++++++------------
 arch/x86/kvm/mmu/mmu_internal.h | 15 ++++++++++++++
 arch/x86/kvm/mmu/paging_tmpl.h  |  5 ++---
 arch/x86/kvm/mmu/tdp_iter.c     | 11 +++-------
 arch/x86/kvm/mmu/tdp_mmu.c      |  6 ++----
 5 files changed, 45 insertions(+), 28 deletions(-)

--
2.31.1

[PATCH v4 1/6] KVM: x86/mmu: Move round_gfn_for_level() helper into mmu_internal.h

[Hou Wenlong]

Rounding down the GFN to a huge page size is a common pattern throughout
KVM, so move round_gfn_for_level() helper in tdp_iter.c to
mmu_internal.h for common usage. Also rename it as gfn_round_for_level()
to use gfn_* prefix and clean up the other call sites.

Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 arch/x86/kvm/mmu/mmu.c          |  5 +++--
 arch/x86/kvm/mmu/mmu_internal.h |  5 +++++
 arch/x86/kvm/mmu/paging_tmpl.h  |  2 +-
 arch/x86/kvm/mmu/tdp_iter.c     | 11 +++--------
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6f81539061d6..7de3579d5a27 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3117,7 +3117,7 @@ static int __direct_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 		if (fault->nx_huge_page_workaround_enabled)
 			disallowed_hugepage_adjust(fault, *it.sptep, it.level);
 
-		base_gfn = fault->gfn & ~(KVM_PAGES_PER_HPAGE(it.level) - 1);
+		base_gfn = gfn_round_for_level(fault->gfn, it.level);
 		if (it.level == fault->goal_level)
 			break;
 
@@ -4340,7 +4340,8 @@ 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 = gfn_round_for_level(fault->addr >> PAGE_SHIFT,
+							 fault->max_level);
 
 			if (kvm_mtrr_check_gfn_range_consistency(vcpu, base, page_num))
 				break;
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 582def531d4d..17488d70f7da 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -155,6 +155,11 @@ static inline bool kvm_mmu_page_ad_need_write_protect(struct kvm_mmu_page *sp)
 	return kvm_x86_ops.cpu_dirty_log_size && sp->role.guest_mode;
 }
 
+static inline gfn_t gfn_round_for_level(gfn_t gfn, int level)
+{
+	return gfn & -KVM_PAGES_PER_HPAGE(level);
+}
+
 int mmu_try_to_unsync_pages(struct kvm *kvm, const struct kvm_memory_slot *slot,
 			    gfn_t gfn, bool can_unsync, bool prefetch);
 
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 5ab5f94dcb6f..d23295c1d1ac 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -701,7 +701,7 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
 		if (fault->nx_huge_page_workaround_enabled)
 			disallowed_hugepage_adjust(fault, *it.sptep, it.level);
 
-		base_gfn = fault->gfn & ~(KVM_PAGES_PER_HPAGE(it.level) - 1);
+		base_gfn = gfn_round_for_level(fault->gfn, it.level);
 		if (it.level == fault->goal_level)
 			break;
 
diff --git a/arch/x86/kvm/mmu/tdp_iter.c b/arch/x86/kvm/mmu/tdp_iter.c
index 39b48e7d7d1a..c4bb4abed6d0 100644
--- a/arch/x86/kvm/mmu/tdp_iter.c
+++ b/arch/x86/kvm/mmu/tdp_iter.c
@@ -15,11 +15,6 @@ static void tdp_iter_refresh_sptep(struct tdp_iter *iter)
 	iter->old_spte = kvm_tdp_mmu_read_spte(iter->sptep);
 }
 
-static gfn_t round_gfn_for_level(gfn_t gfn, int level)
-{
-	return gfn & -KVM_PAGES_PER_HPAGE(level);
-}
-
 /*
  * Return the TDP iterator to the root PT and allow it to continue its
  * traversal over the paging structure from there.
@@ -30,7 +25,7 @@ void tdp_iter_restart(struct tdp_iter *iter)
 	iter->yielded_gfn = iter->next_last_level_gfn;
 	iter->level = iter->root_level;
 
-	iter->gfn = round_gfn_for_level(iter->next_last_level_gfn, iter->level);
+	iter->gfn = gfn_round_for_level(iter->next_last_level_gfn, iter->level);
 	tdp_iter_refresh_sptep(iter);
 
 	iter->valid = true;
@@ -97,7 +92,7 @@ static bool try_step_down(struct tdp_iter *iter)
 
 	iter->level--;
 	iter->pt_path[iter->level - 1] = child_pt;
-	iter->gfn = round_gfn_for_level(iter->next_last_level_gfn, iter->level);
+	iter->gfn = gfn_round_for_level(iter->next_last_level_gfn, iter->level);
 	tdp_iter_refresh_sptep(iter);
 
 	return true;
@@ -139,7 +134,7 @@ static bool try_step_up(struct tdp_iter *iter)
 		return false;
 
 	iter->level++;
-	iter->gfn = round_gfn_for_level(iter->gfn, iter->level);
+	iter->gfn = gfn_round_for_level(iter->gfn, iter->level);
 	tdp_iter_refresh_sptep(iter);
 
 	return true;
-- 
2.31.1

[PATCH v4 2/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in kvm_set_pte_rmapp()

[Hou Wenlong]

When the spte of hupe page is dropped in kvm_set_pte_rmapp(), the whole
gfn range covered by the spte should be flushed. However,
rmap_walk_init_level() doesn't align down the gfn for new level like tdp
iterator does, then the gfn used in kvm_set_pte_rmapp() is not the base
gfn of huge page. And the size of gfn range is wrong too for huge page.
Use the base gfn of huge page and the size of huge page for flushing
tlbs for huge page. Also introduce a helper function to flush the given
page (huge or not) of guest memory, which would help prevent future
buggy use of kvm_flush_remote_tlbs_with_address() in such case.

Fixes: c3134ce240eed ("KVM: Replace old tlb flush function with new one to flush a specified range.")
Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 arch/x86/kvm/mmu/mmu.c          |  4 +++-
 arch/x86/kvm/mmu/mmu_internal.h | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 7de3579d5a27..4874c603ed1c 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1430,7 +1430,9 @@ static bool kvm_set_pte_rmap(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
 	}
 
 	if (need_flush && kvm_available_flush_tlb_with_range()) {
-		kvm_flush_remote_tlbs_with_address(kvm, gfn, 1);
+		gfn_t base = gfn_round_for_level(gfn, level);
+
+		kvm_flush_remote_tlbs_gfn(kvm, base, level);
 		return false;
 	}
 
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 17488d70f7da..249bfcd502b4 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -168,8 +168,18 @@ void kvm_mmu_gfn_allow_lpage(const struct kvm_memory_slot *slot, gfn_t gfn);
 bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
 				    struct kvm_memory_slot *slot, u64 gfn,
 				    int min_level);
+
 void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
 					u64 start_gfn, u64 pages);
+
+/* Flush the given page (huge or not) of guest memory. */
+static inline void kvm_flush_remote_tlbs_gfn(struct kvm *kvm, gfn_t gfn, int level)
+{
+	u64 pages = KVM_PAGES_PER_HPAGE(level);
+
+	kvm_flush_remote_tlbs_with_address(kvm, gfn, pages);
+}
+
 unsigned int pte_list_count(struct kvm_rmap_head *rmap_head);
 
 extern int nx_huge_pages;
-- 
2.31.1

[PATCH v4 3/6] KVM: x86/mmu: Reduce gfn range of tlb flushing in tdp_mmu_map_handle_target_level()

[Hou Wenlong]

Since the children SP is zapped, the gfn range of tlb flushing should be
the range covered by children SP not parent SP. Replace sp->gfn which is
the base gfn of parent SP with iter->gfn and use the correct size of gfn
range for children SP to reduce tlb flushing range.

Fixes: bb95dfb9e2df ("KVM: x86/mmu: Defer TLB flush to caller when freeing TDP MMU shadow pages")
Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
Reviewed-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 672f0432d777..88e67dc5ca3f 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1082,8 +1082,7 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
 		return RET_PF_RETRY;
 	else if (is_shadow_present_pte(iter->old_spte) &&
 		 !is_last_spte(iter->old_spte, iter->level))
-		kvm_flush_remote_tlbs_with_address(vcpu->kvm, sp->gfn,
-						   KVM_PAGES_PER_HPAGE(iter->level + 1));
+		kvm_flush_remote_tlbs_gfn(vcpu->kvm, iter->gfn, iter->level);
 
 	/*
 	 * If the page fault was caused by a write but the page is write
-- 
2.31.1

[PATCH v4 4/6] KVM: x86/mmu: Fix wrong start gfn of tlb flushing with range

[Hou Wenlong]

When a spte is dropped, the start gfn of tlb flushing should be the gfn
of spte not the base gfn of SP which contains the spte. Also introduce a
helper function to do range-based flushing when a spte is dropped, which
would help prevent future buggy use of
kvm_flush_remote_tlbs_with_address() in such case.

Fixes: c3134ce240eed ("KVM: Replace old tlb flush function with new one to flush a specified range.")
Suggested-by: David Matlack <dmatlack@google.com>
Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 arch/x86/kvm/mmu/mmu.c         | 20 ++++++++++++++------
 arch/x86/kvm/mmu/paging_tmpl.h |  3 +--
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 4874c603ed1c..1564cb2d4297 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -260,6 +260,17 @@ void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
 	kvm_flush_remote_tlbs_with_range(kvm, &range);
 }
 
+static gfn_t kvm_mmu_page_get_gfn(struct kvm_mmu_page *sp, int index);
+
+/* Flush the range of guest memory mapped by the given SPTE. */
+static void kvm_flush_remote_tlbs_sptep(struct kvm *kvm, u64 *sptep)
+{
+	struct kvm_mmu_page *sp = sptep_to_sp(sptep);
+	gfn_t gfn = kvm_mmu_page_get_gfn(sp, spte_index(sptep));
+
+	kvm_flush_remote_tlbs_gfn(kvm, gfn, sp->role.level);
+}
+
 static void mark_mmio_spte(struct kvm_vcpu *vcpu, u64 *sptep, u64 gfn,
 			   unsigned int access)
 {
@@ -1148,8 +1159,7 @@ static void drop_large_spte(struct kvm *kvm, u64 *sptep, bool flush)
 	drop_spte(kvm, sptep);
 
 	if (flush)
-		kvm_flush_remote_tlbs_with_address(kvm, sp->gfn,
-			KVM_PAGES_PER_HPAGE(sp->role.level));
+		kvm_flush_remote_tlbs_sptep(kvm, sptep);
 }
 
 /*
@@ -1602,8 +1612,7 @@ static void __rmap_add(struct kvm *kvm,
 		kvm->stat.max_mmu_rmap_size = rmap_count;
 	if (rmap_count > RMAP_RECYCLE_THRESHOLD) {
 		kvm_zap_all_rmap_sptes(kvm, rmap_head);
-		kvm_flush_remote_tlbs_with_address(
-				kvm, sp->gfn, KVM_PAGES_PER_HPAGE(sp->role.level));
+		kvm_flush_remote_tlbs_gfn(kvm, gfn, sp->role.level);
 	}
 }
 
@@ -6410,8 +6419,7 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 			kvm_zap_one_rmap_spte(kvm, rmap_head, sptep);
 
 			if (kvm_available_flush_tlb_with_range())
-				kvm_flush_remote_tlbs_with_address(kvm, sp->gfn,
-					KVM_PAGES_PER_HPAGE(sp->role.level));
+				kvm_flush_remote_tlbs_sptep(kvm, sptep);
 			else
 				need_tlb_flush = 1;
 
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index d23295c1d1ac..a7736992199e 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -937,8 +937,7 @@ static void FNAME(invlpg)(struct kvm_vcpu *vcpu, gva_t gva, hpa_t root_hpa)
 
 			mmu_page_zap_pte(vcpu->kvm, sp, sptep, NULL);
 			if (is_shadow_present_pte(old_spte))
-				kvm_flush_remote_tlbs_with_address(vcpu->kvm,
-					sp->gfn, KVM_PAGES_PER_HPAGE(sp->role.level));
+				kvm_flush_remote_tlbs_sptep(vcpu->kvm, sptep);
 
 			if (!rmap_can_add(vcpu))
 				break;
-- 
2.31.1

[PATCH v4 5/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in validate_direct_spte()

[Hou Wenlong]

The spte pointing to the children SP is dropped, so the whole gfn range
covered by the children SP should be flushed. Although, Hyper-V may
treat a 1-page flush the same if the address points to a huge page, it
still would be better to use the correct size of huge page.

Fixes: c3134ce240eed ("KVM: Replace old tlb flush function with new one to flush a specified range.")
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 arch/x86/kvm/mmu/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 1564cb2d4297..6757c921f412 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2366,7 +2366,7 @@ static void validate_direct_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 			return;
 
 		drop_parent_pte(child, sptep);
-		kvm_flush_remote_tlbs_with_address(vcpu->kvm, child->gfn, 1);
+		kvm_flush_remote_tlbs_sptep(vcpu->kvm, sptep);
 	}
 }
 
-- 
2.31.1

[PATCH v4 6/6] KVM: x86/mmu: Cleanup range-based flushing for given page

[Hou Wenlong]

Use the new kvm_flush_remote_tlbs_gfn() helper to cleanup the call sites
of range-based flushing for given page, which makes the code clear.

Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 arch/x86/kvm/mmu/mmu.c     | 5 ++---
 arch/x86/kvm/mmu/tdp_mmu.c | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6757c921f412..f8feb2f41293 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -810,7 +810,7 @@ static void account_shadowed(struct kvm *kvm, struct kvm_mmu_page *sp)
 	kvm_mmu_gfn_disallow_lpage(slot, gfn);
 
 	if (kvm_mmu_slot_gfn_write_protect(kvm, slot, gfn, PG_LEVEL_4K))
-		kvm_flush_remote_tlbs_with_address(kvm, gfn, 1);
+		kvm_flush_remote_tlbs_gfn(kvm, gfn, PG_LEVEL_4K);
 }
 
 void account_huge_nx_page(struct kvm *kvm, struct kvm_mmu_page *sp)
@@ -2849,8 +2849,7 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, struct kvm_memory_slot *slot,
 	}
 
 	if (flush)
-		kvm_flush_remote_tlbs_with_address(vcpu->kvm, gfn,
-				KVM_PAGES_PER_HPAGE(level));
+		kvm_flush_remote_tlbs_gfn(vcpu->kvm, gfn, level);
 
 	pgprintk("%s: setting spte %llx\n", __func__, *sptep);
 
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 88e67dc5ca3f..724202fc7fde 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -684,8 +684,7 @@ static inline int tdp_mmu_zap_spte_atomic(struct kvm *kvm,
 	if (ret)
 		return ret;
 
-	kvm_flush_remote_tlbs_with_address(kvm, iter->gfn,
-					   KVM_PAGES_PER_HPAGE(iter->level));
+	kvm_flush_remote_tlbs_gfn(kvm, iter->gfn, iter->level);
 
 	/*
 	 * No other thread can overwrite the removed SPTE as they must either
-- 
2.31.1

Re: [PATCH v4 2/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in kvm_set_pte_rmapp()

[Sean Christopherson]

On Mon, Oct 10, 2022, Hou Wenlong wrote:
> When the spte of hupe page is dropped in kvm_set_pte_rmapp(), the whole
> gfn range covered by the spte should be flushed. However,
> rmap_walk_init_level() doesn't align down the gfn for new level like tdp
> iterator does, then the gfn used in kvm_set_pte_rmapp() is not the base
> gfn of huge page. And the size of gfn range is wrong too for huge page.
> Use the base gfn of huge page and the size of huge page for flushing
> tlbs for huge page. Also introduce a helper function to flush the given
> page (huge or not) of guest memory, which would help prevent future
> buggy use of kvm_flush_remote_tlbs_with_address() in such case.
> 
> Fixes: c3134ce240eed ("KVM: Replace old tlb flush function with new one to flush a specified range.")
> Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> ---
>  arch/x86/kvm/mmu/mmu.c          |  4 +++-
>  arch/x86/kvm/mmu/mmu_internal.h | 10 ++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 7de3579d5a27..4874c603ed1c 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1430,7 +1430,9 @@ static bool kvm_set_pte_rmap(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
>  	}
>  
>  	if (need_flush && kvm_available_flush_tlb_with_range()) {
> -		kvm_flush_remote_tlbs_with_address(kvm, gfn, 1);
> +		gfn_t base = gfn_round_for_level(gfn, level);
> +
> +		kvm_flush_remote_tlbs_gfn(kvm, base, level);
>  		return false;
>  	}
>  
> diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> index 17488d70f7da..249bfcd502b4 100644
> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -168,8 +168,18 @@ void kvm_mmu_gfn_allow_lpage(const struct kvm_memory_slot *slot, gfn_t gfn);
>  bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
>  				    struct kvm_memory_slot *slot, u64 gfn,
>  				    int min_level);
> +
>  void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
>  					u64 start_gfn, u64 pages);
> +
> +/* Flush the given page (huge or not) of guest memory. */
> +static inline void kvm_flush_remote_tlbs_gfn(struct kvm *kvm, gfn_t gfn, int level)
> +{
> +	u64 pages = KVM_PAGES_PER_HPAGE(level);
> +

Rather than require the caller to align gfn, what about doing gfn_round_for_level()
in this helper?  It's a little odd that the caller needs to align gfn but doesn't
have to compute the size.

I'm 99% certain kvm_set_pte_rmap() is the only path that doesn't already align the
gfn, but it's nice to not have to worry about getting this right, e.g. alternatively
this helper could WARN if the gfn is misaligned, but that's _more work.

	kvm_flush_remote_tlbs_with_address(kvm, gfn_round_for_level(gfn, level),
					   KVM_PAGES_PER_HPAGE(level);

If no one objects, this can be done when the series is applied, i.e. no need to
send v5 just for this.


> +	kvm_flush_remote_tlbs_with_address(kvm, gfn, pages);
> +}
> +
>  unsigned int pte_list_count(struct kvm_rmap_head *rmap_head);
>  
>  extern int nx_huge_pages;
> -- 
> 2.31.1
> 

Re: [PATCH v4 2/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in kvm_set_pte_rmapp()

[Lai Jiangshan]

On Thu, Oct 13, 2022 at 1:00 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Oct 10, 2022, Hou Wenlong wrote:
> > When the spte of hupe page is dropped in kvm_set_pte_rmapp(), the whole
> > gfn range covered by the spte should be flushed. However,
> > rmap_walk_init_level() doesn't align down the gfn for new level like tdp
> > iterator does, then the gfn used in kvm_set_pte_rmapp() is not the base
> > gfn of huge page. And the size of gfn range is wrong too for huge page.
> > Use the base gfn of huge page and the size of huge page for flushing
> > tlbs for huge page. Also introduce a helper function to flush the given
> > page (huge or not) of guest memory, which would help prevent future
> > buggy use of kvm_flush_remote_tlbs_with_address() in such case.
> >
> > Fixes: c3134ce240eed ("KVM: Replace old tlb flush function with new one to flush a specified range.")
> > Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> > ---
> >  arch/x86/kvm/mmu/mmu.c          |  4 +++-
> >  arch/x86/kvm/mmu/mmu_internal.h | 10 ++++++++++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 7de3579d5a27..4874c603ed1c 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -1430,7 +1430,9 @@ static bool kvm_set_pte_rmap(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
> >       }
> >
> >       if (need_flush && kvm_available_flush_tlb_with_range()) {
> > -             kvm_flush_remote_tlbs_with_address(kvm, gfn, 1);
> > +             gfn_t base = gfn_round_for_level(gfn, level);
> > +
> > +             kvm_flush_remote_tlbs_gfn(kvm, base, level);
> >               return false;
> >       }
> >
> > diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> > index 17488d70f7da..249bfcd502b4 100644
> > --- a/arch/x86/kvm/mmu/mmu_internal.h
> > +++ b/arch/x86/kvm/mmu/mmu_internal.h
> > @@ -168,8 +168,18 @@ void kvm_mmu_gfn_allow_lpage(const struct kvm_memory_slot *slot, gfn_t gfn);
> >  bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
> >                                   struct kvm_memory_slot *slot, u64 gfn,
> >                                   int min_level);
> > +
> >  void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
> >                                       u64 start_gfn, u64 pages);
> > +
> > +/* Flush the given page (huge or not) of guest memory. */
> > +static inline void kvm_flush_remote_tlbs_gfn(struct kvm *kvm, gfn_t gfn, int level)
> > +{
> > +     u64 pages = KVM_PAGES_PER_HPAGE(level);
> > +
>
> Rather than require the caller to align gfn, what about doing gfn_round_for_level()
> in this helper?  It's a little odd that the caller needs to align gfn but doesn't
> have to compute the size.
>
> I'm 99% certain kvm_set_pte_rmap() is the only path that doesn't already align the
> gfn, but it's nice to not have to worry about getting this right, e.g. alternatively
> this helper could WARN if the gfn is misaligned, but that's _more work.
>
>         kvm_flush_remote_tlbs_with_address(kvm, gfn_round_for_level(gfn, level),
>                                            KVM_PAGES_PER_HPAGE(level);
>
> If no one objects, this can be done when the series is applied, i.e. no need to
> send v5 just for this.
>

Hello Paolo, Sean, Hou,

It seems the patchset has not been queued.  I believe it does
fix bugs.

Thanks
Lai

Re: [PATCH v4 2/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in kvm_set_pte_rmapp()

[Sean Christopherson]

On Wed, Dec 14, 2022, Lai Jiangshan wrote:
> On Thu, Oct 13, 2022 at 1:00 AM Sean Christopherson <seanjc@google.com> wrote:
> > > +/* Flush the given page (huge or not) of guest memory. */
> > > +static inline void kvm_flush_remote_tlbs_gfn(struct kvm *kvm, gfn_t gfn, int level)
> > > +{
> > > +     u64 pages = KVM_PAGES_PER_HPAGE(level);
> > > +
> >
> > Rather than require the caller to align gfn, what about doing gfn_round_for_level()
> > in this helper?  It's a little odd that the caller needs to align gfn but doesn't
> > have to compute the size.
> >
> > I'm 99% certain kvm_set_pte_rmap() is the only path that doesn't already align the
> > gfn, but it's nice to not have to worry about getting this right, e.g. alternatively
> > this helper could WARN if the gfn is misaligned, but that's _more work.
> >
> >         kvm_flush_remote_tlbs_with_address(kvm, gfn_round_for_level(gfn, level),
> >                                            KVM_PAGES_PER_HPAGE(level);
> >
> > If no one objects, this can be done when the series is applied, i.e. no need to
> > send v5 just for this.
> >
> 
> Hello Paolo, Sean, Hou,
> 
> It seems the patchset has not been queued.  I believe it does
> fix bugs.

It's on my list of things to get merged for 6.3.  I haven't been more agressive
in getting it queued because I assume there are very few KVM-on-HyperV users that
are likely to be affected.

Re: [PATCH v4 0/6] KVM: x86/mmu: Fix wrong usages of range-based tlb flushing

[Sean Christopherson]

On Mon, 10 Oct 2022 20:19:11 +0800, Hou Wenlong wrote:
> Commit c3134ce240eed ("KVM: Replace old tlb flush function with new one
> to flush a specified range.") replaces old tlb flush function with
> kvm_flush_remote_tlbs_with_address() to do tlb flushing. However, the
> gfn range of tlb flushing is wrong in some cases. E.g., when a spte is
> dropped, the start gfn of tlb flushing should be the gfn of spte not the
> base gfn of SP which contains the spte. Although, as Paolo said, Hyper-V
> may treat a 1-page flush the same if the address points to a huge page,
> and no fixes are reported so far. So it seems that it works well for
> Hyper-V. But it would be better to use the correct size for huge page.
> So this patchset would fix them and introduce some helper functions as
> David suggested to make the code clear.
> 
> [...]

David and/or Hou, it's probably a good idea to double check my results, there
were a few minor conflicts and I doubt anything would fail if I messed up.

Applied to kvm-x86 mmu, thanks!

[1/6] KVM: x86/mmu: Move round_gfn_for_level() helper into mmu_internal.h
      https://github.com/kvm-x86/linux/commit/bb05964f0a3c
[2/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in kvm_set_pte_rmapp()
      https://github.com/kvm-x86/linux/commit/564246ae7da2
[3/6] KVM: x86/mmu: Reduce gfn range of tlb flushing in tdp_mmu_map_handle_target_level()
      https://github.com/kvm-x86/linux/commit/c6753e20e09d
[4/6] KVM: x86/mmu: Fix wrong start gfn of tlb flushing with range
      https://github.com/kvm-x86/linux/commit/4fa7e22ed6ed
[5/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in validate_direct_spte()
      https://github.com/kvm-x86/linux/commit/976d07c25056
[6/6] KVM: x86/mmu: Cleanup range-based flushing for given page
      https://github.com/kvm-x86/linux/commit/f9309825c4b1

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes

Re: [PATCH v4 0/6] KVM: x86/mmu: Fix wrong usages of range-based tlb flushing

[Sean Christopherson]

On Thu, Jan 19, 2023, Sean Christopherson wrote:
> On Mon, 10 Oct 2022 20:19:11 +0800, Hou Wenlong wrote:
> > Commit c3134ce240eed ("KVM: Replace old tlb flush function with new one
> > to flush a specified range.") replaces old tlb flush function with
> > kvm_flush_remote_tlbs_with_address() to do tlb flushing. However, the
> > gfn range of tlb flushing is wrong in some cases. E.g., when a spte is
> > dropped, the start gfn of tlb flushing should be the gfn of spte not the
> > base gfn of SP which contains the spte. Although, as Paolo said, Hyper-V
> > may treat a 1-page flush the same if the address points to a huge page,
> > and no fixes are reported so far. So it seems that it works well for
> > Hyper-V. But it would be better to use the correct size for huge page.
> > So this patchset would fix them and introduce some helper functions as
> > David suggested to make the code clear.
> > 
> > [...]
> 
> David and/or Hou, it's probably a good idea to double check my results, there
> were a few minor conflicts and I doubt anything would fail if I messed up.

Gah, doesn't even compile because I missed a paranthesis.  Messed up my scripts
and didn't pull 'mmu' into 'next.

Force pushed, new hashes are below.  Testing now...

[1/6] KVM: x86/mmu: Move round_gfn_for_level() helper into mmu_internal.h
      https://github.com/kvm-x86/linux/commit/bb05964f0a3c
[2/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in kvm_set_pte_rmapp()
      https://github.com/kvm-x86/linux/commit/c61baeaa2a14
[3/6] KVM: x86/mmu: Reduce gfn range of tlb flushing in tdp_mmu_map_handle_target_level()
      https://github.com/kvm-x86/linux/commit/24c17bc3def7
[4/6] KVM: x86/mmu: Fix wrong start gfn of tlb flushing with range
      https://github.com/kvm-x86/linux/commit/873f68d8dac3
[5/6] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in validate_direct_spte()
      https://github.com/kvm-x86/linux/commit/22f34c933198
[6/6] KVM: x86/mmu: Cleanup range-based flushing for given page
      https://github.com/kvm-x86/linux/commit/e7b406974086