| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ac774797-f82c-4717-9c40-8602e799e966@linux.intel.com>
Date: Wed, 3 Sep 2025 14:57:07 +0800
From: Binbin Wu <binbin.wu@...ux.intel.com>
To: Yan Zhao <yan.y.zhao@...el.com>
Cc: pbonzini@...hat.com, seanjc@...gle.com, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, x86@...nel.org, rick.p.edgecombe@...el.com,
dave.hansen@...el.com, kas@...nel.org, tabba@...gle.com,
ackerleytng@...gle.com, quic_eberman@...cinc.com, michael.roth@....com,
david@...hat.com, vannapurve@...gle.com, vbabka@...e.cz,
thomas.lendacky@....com, pgonda@...gle.com, zhiquan1.li@...el.com,
fan.du@...el.com, jun.miao@...el.com, ira.weiny@...el.com,
isaku.yamahata@...el.com, xiaoyao.li@...el.com, chao.p.peng@...el.com
Subject: Re: [RFC PATCH v2 12/23] KVM: x86/mmu: Introduce
kvm_split_cross_boundary_leafs()
On 8/7/2025 5:43 PM, Yan Zhao wrote:
> Introduce kvm_split_cross_boundary_leafs() to split huge leaf entries that
> cross the boundary of a specified range.
>
> Splitting huge leaf entries that cross the boundary is essential before
> zapping the range in the mirror root. This ensures that the subsequent zap
> operation does not affect any GFNs outside the specified range. This is
> crucial for the mirror root, as the private page table requires the guest's
> ACCEPT operation after a GFN faults back.
>
> The core of kvm_split_cross_boundary_leafs() leverages the main logic from
> tdp_mmu_split_huge_pages_root(). It traverses the specified root and splits
> huge leaf entries if they cross the range boundary. When splitting is
> necessary, kvm->mmu_lock is temporarily released for memory allocation,
> which means returning -ENOMEM is possible.
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@...el.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@...el.com>
> ---
> RFC v2:
> - Rename the API to kvm_split_cross_boundary_leafs().
> - Make the API to be usable for direct roots or under shared mmu_lock.
> - Leverage the main logic from tdp_mmu_split_huge_pages_root(). (Rick)
>
> RFC v1:
> - Split patch.
> - introduced API kvm_split_boundary_leafs(), refined the logic and
> simplified the code.
> ---
> arch/x86/kvm/mmu/mmu.c | 27 +++++++++++++++
> arch/x86/kvm/mmu/tdp_mmu.c | 68 ++++++++++++++++++++++++++++++++++++--
> arch/x86/kvm/mmu/tdp_mmu.h | 3 ++
> include/linux/kvm_host.h | 2 ++
> 4 files changed, 97 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 9182192daa3a..13910ae05f76 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1647,6 +1647,33 @@ static bool __kvm_rmap_zap_gfn_range(struct kvm *kvm,
> start, end - 1, can_yield, true, flush);
> }
>
> +/*
> + * Split large leafs crossing the boundary of the specified range
> + *
> + * Return value:
> + * 0 : success, no flush is required;
> + * 1 : success, flush is required;
> + * <0: failure.
> + */
> +int kvm_split_cross_boundary_leafs(struct kvm *kvm, struct kvm_gfn_range *range,
> + bool shared)
> +{
> + bool ret = 0;
> +
> + lockdep_assert_once(kvm->mmu_invalidate_in_progress ||
> + lockdep_is_held(&kvm->slots_lock) ||
> + srcu_read_lock_held(&kvm->srcu));
> +
> + if (!range->may_block)
> + return -EOPNOTSUPP;
> +
> + if (tdp_mmu_enabled)
> + ret = kvm_tdp_mmu_gfn_range_split_cross_boundary_leafs(kvm, range, shared);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(kvm_split_cross_boundary_leafs);
> +
> bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)
> {
> bool flush = false;
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index ce49cc850ed5..62a09a9655c3 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1574,10 +1574,17 @@ static int tdp_mmu_split_huge_page(struct kvm *kvm, struct tdp_iter *iter,
> return ret;
> }
>
> +static bool iter_cross_boundary(struct tdp_iter *iter, gfn_t start, gfn_t end)
> +{
> + return !(iter->gfn >= start &&
> + (iter->gfn + KVM_PAGES_PER_HPAGE(iter->level)) <= end);
> +}
> +
> static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> struct kvm_mmu_page *root,
> gfn_t start, gfn_t end,
> - int target_level, bool shared)
> + int target_level, bool shared,
> + bool only_cross_bounday, bool *flush)
s/only_cross_bounday/only_cross_boundary
> {
> struct kvm_mmu_page *sp = NULL;
> struct tdp_iter iter;
> @@ -1589,6 +1596,13 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> * level into one lower level. For example, if we encounter a 1GB page
> * we split it into 512 2MB pages.
> *
> + * When only_cross_bounday is true, just split huge pages above the
> + * target level into one lower level if the huge pages cross the start
> + * or end boundary.
> + *
> + * No need to update @flush for !only_cross_bounday cases, which rely
> + * on the callers to do the TLB flush in the end.
I think API wise, it's a bit confusing, although it's a local API.
If just look at the API without digging into the function implementation, my
initial thought is *flush will tell whether TLB flush is needed or not.
Just update *flush unconditionally? Or move the comment as the description for
the function to call it out?
I have thought another option to combine the two inputs, i.e., if *flush is a
valid pointer, it means it's for only_cross_boundary. Otherwise, just passing
NULL. But then I felt it was a bit risky to reply on the pointer to indicate the
scenario.
> + *
> * Since the TDP iterator uses a pre-order traversal, we are guaranteed
> * to visit an SPTE before ever visiting its children, which means we
> * will correctly recursively split huge pages that are more than one
> @@ -1597,12 +1611,19 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> */
> for_each_tdp_pte_min_level(iter, kvm, root, target_level + 1, start, end) {
> retry:
> - if (tdp_mmu_iter_cond_resched(kvm, &iter, false, shared))
> + if (tdp_mmu_iter_cond_resched(kvm, &iter, *flush, shared)) {
> + if (only_cross_bounday)
> + *flush = false;
> continue;
> + }
>
> if (!is_shadow_present_pte(iter.old_spte) || !is_large_pte(iter.old_spte))
> continue;
>
> + if (only_cross_bounday &&
> + !iter_cross_boundary(&iter, start, end))
> + continue;
> +
> if (!sp) {
> rcu_read_unlock();
>
> @@ -1637,6 +1658,8 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> goto retry;
>
> sp = NULL;
> + if (only_cross_bounday)
> + *flush = true;
> }
>
> rcu_read_unlock();
[...]
Powered by blists - more mailing lists