Re: [PATCH v6 13/16] KVM: x86/tdp_mmu: Try to merge pages into a large page
From: Binbin Wu
Date: Wed Nov 22 2023 - 02:24:45 EST
On 11/7/2023 11:00 PM, isaku.yamahata@xxxxxxxxx wrote:
From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
When a large page is passed to the KVM page fault handler and some of sub
pages are already populated, try to merge sub pages into a large page.
This situation can happen when the guest converts small pages into shared
and convert it back into private.
When a large page is passed to KVM mmu page fault handler and the spte
corresponding to the page is non-leaf (one or more of sub pages are already
populated at lower page level), the current kvm mmu zaps non-leaf spte at a
large page level, and populate a leaf spte at that level. Thus small pages
are converted into a large page. However, it doesn't work for TDX because
zapping and re-populating results in zeroing page content. Instead,
populate all small pages and merge them into a large page.
Merging pages into a large page can fail when some sub pages are accepted
and some are not. In such case, with the assumption that guest tries to
accept at large page size for performance when possible, don't try to be
smart to identify which page is still pending, map all pages at lower page
level, and let vcpu re-execute.
Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
---
arch/x86/include/asm/kvm-x86-ops.h | 2 +
arch/x86/include/asm/kvm_host.h | 4 +
arch/x86/kvm/mmu/tdp_iter.c | 37 +++++--
arch/x86/kvm/mmu/tdp_iter.h | 2 +
arch/x86/kvm/mmu/tdp_mmu.c | 172 ++++++++++++++++++++++++++++-
5 files changed, 207 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 3deb6ab4f291..9a7d4db304c7 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -104,9 +104,11 @@ KVM_X86_OP(load_mmu_pgd)
KVM_X86_OP_OPTIONAL(link_private_spt)
KVM_X86_OP_OPTIONAL(free_private_spt)
KVM_X86_OP_OPTIONAL(split_private_spt)
+KVM_X86_OP_OPTIONAL(merge_private_spt)
KVM_X86_OP_OPTIONAL(set_private_spte)
KVM_X86_OP_OPTIONAL(remove_private_spte)
KVM_X86_OP_OPTIONAL(zap_private_spte)
+KVM_X86_OP_OPTIONAL(unzap_private_spte)
KVM_X86_OP(has_wbinvd_exit)
KVM_X86_OP(get_l2_tsc_offset)
KVM_X86_OP(get_l2_tsc_multiplier)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e75a461bdea7..0254c382f4f0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -146,6 +146,7 @@
#define KVM_MAX_HUGEPAGE_LEVEL PG_LEVEL_1G
#define KVM_NR_PAGE_SIZES (KVM_MAX_HUGEPAGE_LEVEL - PG_LEVEL_4K + 1)
#define KVM_HPAGE_GFN_SHIFT(x) (((x) - 1) * 9)
+#define KVM_HPAGE_GFN_MASK(x) (~((1UL << KVM_HPAGE_GFN_SHIFT(x)) - 1))
#define KVM_HPAGE_SHIFT(x) (PAGE_SHIFT + KVM_HPAGE_GFN_SHIFT(x))
#define KVM_HPAGE_SIZE(x) (1UL << KVM_HPAGE_SHIFT(x))
#define KVM_HPAGE_MASK(x) (~(KVM_HPAGE_SIZE(x) - 1))
@@ -1755,11 +1756,14 @@ struct kvm_x86_ops {
void *private_spt);
int (*split_private_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
void *private_spt);
+ int (*merge_private_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
+ void *private_spt);
int (*set_private_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
kvm_pfn_t pfn);
int (*remove_private_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
kvm_pfn_t pfn);
int (*zap_private_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level);
+ int (*unzap_private_spte)(struct kvm *kvm, gfn_t gfn, enum pg_level level);
bool (*has_wbinvd_exit)(void);
diff --git a/arch/x86/kvm/mmu/tdp_iter.c b/arch/x86/kvm/mmu/tdp_iter.c
index bd30ebfb2f2c..f33226fcd62a 100644
--- a/arch/x86/kvm/mmu/tdp_iter.c
+++ b/arch/x86/kvm/mmu/tdp_iter.c
@@ -71,6 +71,14 @@ tdp_ptep_t spte_to_child_pt(u64 spte, int level)
return (tdp_ptep_t)__va(spte_to_pfn(spte) << PAGE_SHIFT);
}
+static void step_down(struct tdp_iter *iter, tdp_ptep_t child_pt)
+{
+ iter->level--;
+ iter->pt_path[iter->level - 1] = child_pt;
+ iter->gfn = gfn_round_for_level(iter->next_last_level_gfn, iter->level);
+ tdp_iter_refresh_sptep(iter);
+}
+
/*
* Steps down one level in the paging structure towards the goal GFN. Returns
* true if the iterator was able to step down a level, false otherwise.
@@ -92,14 +100,28 @@ static bool try_step_down(struct tdp_iter *iter)
if (!child_pt)
return false;
- iter->level--;
- iter->pt_path[iter->level - 1] = child_pt;
- iter->gfn = gfn_round_for_level(iter->next_last_level_gfn, iter->level);
- tdp_iter_refresh_sptep(iter);
-
+ step_down(iter, child_pt);
return true;
}
+/* Steps down for freezed spte. Don't re-read sptep because it was freezed. */
+void tdp_iter_step_down(struct tdp_iter *iter, tdp_ptep_t child_pt)
+{
+ WARN_ON_ONCE(!child_pt);
+ WARN_ON_ONCE(iter->yielded);
+ WARN_ON_ONCE(iter->level == iter->min_level);
+
+ step_down(iter, child_pt);
+}
+
+void tdp_iter_step_side(struct tdp_iter *iter)
+{
+ iter->gfn += KVM_PAGES_PER_HPAGE(iter->level);
+ iter->next_last_level_gfn = iter->gfn;
+ iter->sptep++;
+ iter->old_spte = kvm_tdp_mmu_read_spte(iter->sptep);
+}
+
/*
* Steps to the next entry in the current page table, at the current page table
* level. The next entry could point to a page backing guest memory or another
@@ -117,10 +139,7 @@ static bool try_step_side(struct tdp_iter *iter)
(SPTE_ENT_PER_PAGE - 1))
return false;
- iter->gfn += KVM_PAGES_PER_HPAGE(iter->level);
- iter->next_last_level_gfn = iter->gfn;
- iter->sptep++;
- iter->old_spte = kvm_tdp_mmu_read_spte(iter->sptep);
+ tdp_iter_step_side(iter);
return true;
}
diff --git a/arch/x86/kvm/mmu/tdp_iter.h b/arch/x86/kvm/mmu/tdp_iter.h
index a9c9cd0db20a..ca00db799a50 100644
--- a/arch/x86/kvm/mmu/tdp_iter.h
+++ b/arch/x86/kvm/mmu/tdp_iter.h
@@ -134,6 +134,8 @@ void tdp_iter_start(struct tdp_iter *iter, struct kvm_mmu_page *root,
int min_level, gfn_t next_last_level_gfn);
void tdp_iter_next(struct tdp_iter *iter);
void tdp_iter_restart(struct tdp_iter *iter);
+void tdp_iter_step_side(struct tdp_iter *iter);
+void tdp_iter_step_down(struct tdp_iter *iter, tdp_ptep_t child_pt);
static inline union kvm_mmu_page_role tdp_iter_child_role(struct tdp_iter *iter)
{
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 734ee822b43c..c8a4bd052c71 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1224,6 +1224,176 @@ void kvm_tdp_mmu_invalidate_all_roots(struct kvm *kvm, bool skip_private)
}
}
+static int tdp_mmu_iter_step_side(int i, struct tdp_iter *iter)
+{
+ i++;
+
+ /*
+ * if i = SPTE_ENT_PER_PAGE, tdp_iter_step_side() results
+ * in reading the entry beyond the last entry.
+ */
+ if (i < SPTE_ENT_PER_PAGE)
+ tdp_iter_step_side(iter);
+
+ return i;
+}
+
+static int tdp_mmu_merge_private_spt(struct kvm_vcpu *vcpu,
+ struct kvm_page_fault *fault,
+ struct tdp_iter *iter, u64 new_spte)
+{
+ u64 *sptep = rcu_dereference(iter->sptep);
+ u64 old_spte = iter->old_spte;
+ struct kvm_mmu_page *child_sp;
+ struct kvm *kvm = vcpu->kvm;
+ struct tdp_iter child_iter;
+ int level = iter->level;
+ gfn_t gfn = iter->gfn;
+ tdp_ptep_t child_pt;
+ u64 child_spte;
+ int ret = 0;
+ int i;
+
+ /*
+ * TDX KVM supports only 2MB large page. It's not supported to merge
+ * 2MB pages into 1GB page at the moment.
+ */
+ WARN_ON_ONCE(fault->goal_level != PG_LEVEL_2M);
+ WARN_ON_ONCE(iter->level != PG_LEVEL_2M);
+ WARN_ON_ONCE(!is_large_pte(new_spte));
+
+ /* Freeze the spte to prevent other threads from working spte. */
+ if (!try_cmpxchg64(sptep, &iter->old_spte, REMOVED_SPTE))
+ return -EBUSY;
+
+ /*
+ * Step down to the child spte. Because tdp_iter_next() assumes the
+ * parent spte isn't freezed, do it manually.
s/freezed/frozen
+ */
+ child_pt = spte_to_child_pt(iter->old_spte, iter->level);
+ child_sp = sptep_to_sp(child_pt);
+ WARN_ON_ONCE(child_sp->role.level != PG_LEVEL_4K);
+ WARN_ON_ONCE(!kvm_mmu_page_role_is_private(child_sp->role));
+
+ /* Don't modify iter as the caller will use iter after this function. */
+ child_iter = *iter;
+ /* Adjust the target gfn to the head gfn of the large page. */
+ child_iter.next_last_level_gfn &= -KVM_PAGES_PER_HPAGE(level);
+ tdp_iter_step_down(&child_iter, child_pt);
+
+ /*
+ * All child pages are required to be populated for merging them into a
+ * large page. Populate all child spte.
+ */
+ for (i = 0; i < SPTE_ENT_PER_PAGE; i = tdp_mmu_iter_step_side(i, &child_iter)) {
+ int tmp;
+
+ WARN_ON_ONCE(child_iter.level != PG_LEVEL_4K);
+
+ if (is_shadow_present_pte(child_iter.old_spte)) {
+ /* TODO: relocate page for huge page. */
+ if (WARN_ON_ONCE(spte_to_pfn(child_iter.old_spte) !=
+ spte_to_pfn(new_spte) + i)) {
+ if (!ret)
+ ret = -EAGAIN;
+ continue;
+ }
+ /*
+ * When SEPT_VE_DISABLE=true and the page state is
+ * pending, this case can happen. Just resume the vcpu
+ * again with the expectation for other vcpu to accept
+ * this page.
+ */
+ if (child_iter.gfn == fault->gfn) {
+ if (!ret)
+ ret = -EAGAIN;
+ }
+ continue;
+ }
+
+ child_spte = make_huge_page_split_spte(kvm, new_spte, child_sp->role, i);
+ /*
+ * Because other thread may have started to operate on this spte
+ * before freezing the parent spte, Use atomic version to
+ * prevent race.
+ */
+ tmp = tdp_mmu_set_spte_atomic(vcpu->kvm, &child_iter, child_spte);
+ if (tmp == -EBUSY || tmp == -EAGAIN) {
+ /*
+ * There was a race condition. Populate remaining 4K
+ * spte to resolve fault->gfn to guarantee the forward
+ * progress.
+ */
+ if (!ret)
+ ret = tmp;
+ } else if (tmp) {
+ ret = tmp;
+ goto out;
+ }
+ }
+ if (ret)
+ goto out;
+
+ /* Prevent the Secure-EPT entry from being used. */
+ ret = static_call(kvm_x86_zap_private_spte)(kvm, gfn, level);
+ if (ret)
+ goto out;
+ kvm_flush_remote_tlbs_range(kvm, gfn & KVM_HPAGE_GFN_MASK(level),
+ KVM_PAGES_PER_HPAGE(level));
+
+ /* Merge pages into a large page. */
+ ret = static_call(kvm_x86_merge_private_spt)(kvm, gfn, level,
+ kvm_mmu_private_spt(child_sp));
+ /*
+ * Failed to merge pages because some pages are accepted and some are
+ * pending. Since the child page was mapped above, let vcpu run.
+ */
+ if (ret) {
+ if (static_call(kvm_x86_unzap_private_spte)(kvm, gfn, level))
+ old_spte = SHADOW_NONPRESENT_VALUE |
+ (spte_to_pfn(old_spte) << PAGE_SHIFT) |
+ PT_PAGE_SIZE_MASK;
+ goto out;
+ }
+
+ /* Unfreeze spte. */
+ iter->old_spte = new_spte;
+ __kvm_tdp_mmu_write_spte(sptep, new_spte);
+
+ /*
+ * Free unused child sp. Secure-EPT page was already freed at TDX level
+ * by kvm_x86_merge_private_spt().
+ */
+ tdp_unaccount_mmu_page(kvm, child_sp);
+ tdp_mmu_free_sp(child_sp);
+ return -EAGAIN;
It successfully promoted the page, why still return -EAGAIN?
+
+out:
+ iter->old_spte = old_spte;
+ __kvm_tdp_mmu_write_spte(sptep, old_spte);
+ return ret;
+}
+
+static int __tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
+ struct kvm_page_fault *fault,
+ struct tdp_iter *iter, u64 new_spte)
+{
+ /*
+ * The private page has smaller-size pages. For example, the child
+ * pages was converted from shared to page, and now it can be mapped as
+ * a large page. Try to merge small pages into a large page.
+ */
+ if (fault->slot &&
+ kvm_gfn_shared_mask(vcpu->kvm) &&
+ iter->level > PG_LEVEL_4K &&
+ kvm_is_private_gpa(vcpu->kvm, fault->addr) &&
+ is_shadow_present_pte(iter->old_spte) &&
+ !is_large_pte(iter->old_spte))
+ return tdp_mmu_merge_private_spt(vcpu, fault, iter, new_spte);
+
+ return tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte);
+}
+
/*
* Installs a last-level SPTE to handle a TDP page fault.
* (NPT/EPT violation/misconfiguration)
@@ -1265,7 +1435,7 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
if (new_spte == iter->old_spte)
ret = RET_PF_SPURIOUS;
- else if (tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte))
+ else if (__tdp_mmu_map_handle_target_level(vcpu, fault, iter, new_spte))
return RET_PF_RETRY;
else if (is_shadow_present_pte(iter->old_spte) &&
!is_last_spte(iter->old_spte, iter->level))