[PATCH v4 0/6] KVM: arm64: nv: Implement nested stage-2 reverse map

[PATCH v4 0/6] KVM: arm64: nv: Implement nested stage-2 reverse map

[Wei-Lin Chang]

Hi,

This is v4 of optimizing the shadow s2 mmu unmapping during MMU
notifiers. The general theme stays the same, but various
improvements/fixes are added. Thanks for all the feedback. Please see
below for the changes.

* Changes from v3 [1]:

  - Rebased on to the latest kvmarm/fixes.

  - Removed commit "KVM: arm64: Move shadow_pt_debugfs_dentry to reduce
    holes in kvm_s2_mmu" because the current shadow ptdump code is found
    to be buggy and the proposed fix removes shadow_pt_debugfs_dentry [2].

  - Rename some variables to improve readability e.g. ipa ->
    canonical_ipa, fault_ipa -> nested_ipa.

  - Skip reverse map optimization when !mmu->nested_stage2_enabled,
    and just unmap the canonical IPA range during MMU notifiers.

  - Change comment above the code that checks whether an identical
    reverse map already exists in kvm_record_nested_revmap() from saying
    it could be a permission update to saying another parallel stage-2
    fault could have inserted an identical reverse map. This is because
    permission update won't trigger kvm_record_nested_revmap().

  - Add helpers for checking maple tree entry flags VALID_ENTRY and
    UNKNOWN_IPA, also warn if both are set at the same time.

  - Test canonical IPA alignment with the mapping size during reverse
    map insertion.

  - Move clean up of kvm_unmap_gfn_range() to its own patch.

  - Avoid naming variables with "*dir*", as it can be misunderstood to
    be related to directories.

  - Use mas_find() instead of mas_find_range() as mas_find_range()
    returns every range including empty/NULL ranges, for our use case
    we only want to iterate over non-NULL ranges.

  - Added a trace point to track when the reverse map maple tree becomes
    "broken" i.e. maple tree insertion failure.

Thanks!

[1]: https://lore.kernel.org/kvmarm/20260510145338.322962-1-weilin.chang@arm.com/
[2]: https://lore.kernel.org/kvmarm/20260630121005.1130996-1-weilin.chang@arm.com/

Wei-Lin Chang (6):
  KVM: arm64: Use a variable for the canonical GPA in kvm_s2_fault_map()
  KVM: arm64: nv: Avoid full shadow s2 unmap
  KVM: arm64: nv: Add nested revmap broken tracepoint
  KVM: arm64: Refactor kvm_unmap_gfn_range() with common variables
  KVM: arm64: nv: Remove reverse map entries during TLBI handling
  KVM: arm64: nv: Create nested IPA direct map to speed up reverse map
    removal

 arch/arm64/include/asm/kvm_host.h   |   9 +
 arch/arm64/include/asm/kvm_nested.h |   6 +
 arch/arm64/kvm/mmu.c                |  42 +++--
 arch/arm64/kvm/nested.c             | 280 +++++++++++++++++++++++++++-
 arch/arm64/kvm/sys_regs.c           |   3 +
 arch/arm64/kvm/trace_arm.h          |  27 +++
 6 files changed, 349 insertions(+), 18 deletions(-)

-- 
2.43.0

[PATCH v4 1/6] KVM: arm64: Use a variable for the canonical GPA in kvm_s2_fault_map()

[Wei-Lin Chang]

Create a variable to store the canonical GPA, instead of calculating it
when needed. This will be useful when we need to use the canonical GPA
for the nested reverse map later.

Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
 arch/arm64/kvm/mmu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 8811ad60cf72..54395a608fd3 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2021,6 +2021,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 	long mapping_size;
 	kvm_pfn_t pfn;
 	gfn_t gfn;
+	phys_addr_t canonical_gpa;
 	int ret;
 
 	kvm_fault_lock(kvm);
@@ -2034,6 +2035,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 	mapping_size = s2vi->vma_pagesize;
 	pfn = s2vi->pfn;
 	gfn = s2vi->gfn;
+	canonical_gpa = gfn_to_gpa(get_canonical_gfn(s2fd, s2vi));
 
 	/*
 	 * If we are not forced to use page mapping, check if we are
@@ -2052,6 +2054,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 				goto out_unlock;
 			}
 		}
+		canonical_gpa = ALIGN_DOWN(canonical_gpa, mapping_size);
 	}
 
 	if (!perm_fault_granule && !s2vi->map_non_cacheable && kvm_has_mte(kvm))
@@ -2085,11 +2088,9 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 	 * making sure we adjust the canonical IPA if the mapping size has
 	 * been updated (via a THP upgrade, for example).
 	 */
-	if (writable && !ret) {
-		phys_addr_t ipa = gfn_to_gpa(get_canonical_gfn(s2fd, s2vi));
-		ipa &= ~(mapping_size - 1);
-		mark_page_dirty_in_slot(kvm, s2fd->memslot, gpa_to_gfn(ipa));
-	}
+	if (writable && !ret)
+		mark_page_dirty_in_slot(kvm, s2fd->memslot,
+					gpa_to_gfn(canonical_gpa));
 
 	if (ret != -EAGAIN)
 		return ret;
-- 
2.43.0

[PATCH v4 2/6] KVM: arm64: nv: Avoid full shadow s2 unmap

[Wei-Lin Chang]

Currently we are forced to fully unmap all shadow stage-2 for a VM when
unmapping a page from the canonical stage-2, for example during an MMU
notifier call. This is because we are not tracking which canonical IPAs
are mapped in the shadow stage-2 page tables hence there is no way to
know what to unmap.

Create a per kvm_s2_mmu maple tree to track canonical IPA range ->
nested IPA range, so that it is possible to partially unmap shadow
stage-2 when a canonical IPA range is unmapped. The algorithm is simple
and conservative:

At each shadow stage-2 map, insert the nested IPA range into the maple
tree, with the canonical IPA range as the key. If the canonical IPA
range doesn't overlap with existing ranges in the tree, insert as is,
and a reverse mapping for this range is established. But if the
canonical IPA range overlaps with any existing ranges in the tree,
create a new range that spans all the overlapping ranges including the
input range and replace those existing ranges. In the meantime, mark
this new spanning canonical IPA range with an "UNKNOWN_IPA" bit,
indicating we give up tracking the nested IPA ranges that map to this
canonical IPA range.

The maple tree's 64-bit entry is enough to store the nested IPA and
the UNKNOWN_IPA status, therefore besides maple tree's internal
operation, memory allocation is avoided.

Example:
|||| means existing range, ---- means empty range

input:            $$$$$$$$$$$$$$$$$$$$$$$$$$
tree:  --||||-----|||||||---------||||||||||-----------

insert spanning range and replace overlapping ones:
       --||||-----||||||||||||||||||||||||||-----------
                  ^^^^marked UNKNOWN_IPA^^^^

With the reverse map created, when a canonical IPA range gets unmapped,
look into each s2 mmu's maple tree and look for canonical IPA ranges
affected, and based on their UNKNOWN_IPA status:

UNKNOWN_IPA     -> fall back and fully unmap the current shadow
                   stage-2, also clear the tree

not UNKNOWN_IPA -> unmap the nested IPA range, and remove the reverse
                   map entry

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
 arch/arm64/include/asm/kvm_host.h   |   4 +
 arch/arm64/include/asm/kvm_nested.h |   4 +
 arch/arm64/kvm/mmu.c                |  21 +++-
 arch/arm64/kvm/nested.c             | 167 +++++++++++++++++++++++++++-
 4 files changed, 190 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 2faa60df847d..42ab121a04f8 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -227,6 +227,10 @@ struct kvm_s2_mmu {
 	 */
 	bool	pending_unmap;
 
+	bool	nested_revmap_broken;
+	/* canonical IPA to nested IPA range lookup */
+	struct maple_tree nested_revmap_mt;
+
 	/*
 	 *  0: Nobody is currently using this, check vttbr for validity
 	 * >0: Somebody is actively using this.
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index cbdaaa2a2903..79de224f3438 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -77,6 +77,8 @@ extern void kvm_s2_mmu_iterate_by_vmid(struct kvm *kvm, u16 vmid,
 				       const union tlbi_info *info,
 				       void (*)(struct kvm_s2_mmu *,
 						const union tlbi_info *));
+extern void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
+				     gpa_t nested_ipa, size_t map_size);
 extern void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu);
 extern void kvm_vcpu_put_hw_mmu(struct kvm_vcpu *vcpu);
 
@@ -165,6 +167,8 @@ extern int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu,
 				    struct kvm_s2_trans *trans);
 extern int kvm_inject_s2_fault(struct kvm_vcpu *vcpu, u64 esr_el2);
 extern void kvm_nested_s2_wp(struct kvm *kvm);
+extern void kvm_unmap_cipa_range_nested(struct kvm *kvm, gpa_t canonical_ipa, size_t size,
+				       bool may_block);
 extern void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block);
 extern void kvm_nested_s2_flush(struct kvm *kvm);
 
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 54395a608fd3..73629aa1ade6 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/maple_tree.h>
 #include <linux/mman.h>
 #include <linux/kvm_host.h>
 #include <linux/io.h>
@@ -1126,6 +1127,7 @@ void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)
 {
 	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
 	struct kvm_pgtable *pgt = NULL;
+	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
 
 	write_lock(&kvm->mmu_lock);
 	pgt = mmu->pgt;
@@ -1135,8 +1137,11 @@ void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)
 		free_percpu(mmu->last_vcpu_ran);
 	}
 
-	if (kvm_is_nested_s2_mmu(kvm, mmu))
+	if (kvm_is_nested_s2_mmu(kvm, mmu)) {
+		if (!mtree_empty(revmap_mt))
+			mtree_destroy(revmap_mt);
 		kvm_init_nested_s2_mmu(mmu);
+	}
 
 	write_unlock(&kvm->mmu_lock);
 
@@ -1670,6 +1675,9 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_relax_perms)(pgt, s2fd->fault_ipa,
 								 prot, flags);
 	} else {
+		if (s2fd->nested)
+			kvm_record_nested_revmap(gfn << PAGE_SHIFT, pgt->mmu,
+						 s2fd->fault_ipa, PAGE_SIZE);
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_map)(pgt, s2fd->fault_ipa, PAGE_SIZE,
 							 __pfn_to_phys(pfn), prot,
 							 memcache, flags);
@@ -2074,6 +2082,10 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_relax_perms)(pgt, gfn_to_gpa(gfn),
 								 prot, flags);
 	} else {
+		if (s2fd->nested)
+			kvm_record_nested_revmap(canonical_gpa, pgt->mmu,
+						 gfn_to_gpa(gfn), mapping_size);
+
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_map)(pgt, gfn_to_gpa(gfn), mapping_size,
 							 __pfn_to_phys(pfn), prot,
 							 memcache, flags);
@@ -2435,8 +2447,9 @@ bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)
 	__unmap_stage2_range(&kvm->arch.mmu, range->start << PAGE_SHIFT,
 			     (range->end - range->start) << PAGE_SHIFT,
 			     range->may_block);
-
-	kvm_nested_s2_unmap(kvm, range->may_block);
+	kvm_unmap_cipa_range_nested(kvm, range->start << PAGE_SHIFT,
+				    (range->end - range->start) << PAGE_SHIFT,
+				    range->may_block);
 	return false;
 }
 
@@ -2714,7 +2727,7 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
 
 	write_lock(&kvm->mmu_lock);
 	kvm_stage2_unmap_range(&kvm->arch.mmu, gpa, size, true);
-	kvm_nested_s2_unmap(kvm, true);
+	kvm_unmap_cipa_range_nested(kvm, gpa, size, true);
 	write_unlock(&kvm->mmu_lock);
 }
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 94df26de6990..065823e415ce 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -7,6 +7,7 @@
 #include <linux/bitfield.h>
 #include <linux/kvm.h>
 #include <linux/kvm_host.h>
+#include <linux/maple_tree.h>
 
 #include <asm/fixmap.h>
 #include <asm/kvm_arm.h>
@@ -44,6 +45,32 @@ struct vncr_tlb {
  */
 #define S2_MMU_PER_VCPU		2
 
+/*
+ * Per shadow S2 reverse map (canonical IPA -> nested IPA range) maple tree
+ * payload layout:
+ *
+ * bit  62:     valid, prevents the case where the nested IPA is 0 and turning
+ *              the whole value to 0 (NULL for maple tree)
+ * bits 55-12:  nested IPA bits 55-12
+ * bit  0:      UNKNOWN_IPA bit, 1 indicates we give up on tracking what nested
+ *              IPA maps to this canonical IPA in the shadow stage-2
+ */
+#define VALID_ENTRY		BIT(62)
+#define ADDR_MASK		GENMASK_ULL(55, 12)
+#define UNKNOWN_IPA		BIT(0)
+
+static bool valid_entry(unsigned long entry)
+{
+	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
+	return entry & VALID_ENTRY;
+}
+
+static bool unknown_ipa_entry(unsigned long entry)
+{
+	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
+	return entry & UNKNOWN_IPA;
+}
+
 void kvm_init_nested(struct kvm *kvm)
 {
 	kvm->arch.nested_mmus = NULL;
@@ -844,12 +871,65 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
 	return s2_mmu;
 }
 
+void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
+			      gpa_t nested_ipa, size_t map_size)
+{
+	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
+	gpa_t canonical_ipa_end;
+	u64 entry, new_entry = 0;
+
+	lockdep_assert_held_read(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
+
+	if (mmu->nested_revmap_broken)
+		return;
+
+	if (WARN_ON(!IS_ALIGNED(canonical_ipa, map_size)))
+		canonical_ipa = ALIGN_DOWN(canonical_ipa, map_size);
+
+	canonical_ipa_end = canonical_ipa + map_size - 1;
+	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
+
+	mtree_lock(revmap_mt);
+	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
+
+	if (entry) {
+		/* parallel faults can be adding the same mapping */
+		if (valid_entry(entry) &&
+		    mas_rmap.index == canonical_ipa &&
+		    mas_rmap.last == canonical_ipa_end &&
+		    nested_ipa == (entry & ADDR_MASK))
+			goto unlock;
+		/*
+		 * Create a "UNKNOWN_IPA" range that spans all the overlapping
+		 * ranges and store it.
+		 */
+		while (entry && mas_rmap.index <= canonical_ipa_end) {
+			canonical_ipa = min(mas_rmap.index, canonical_ipa);
+			canonical_ipa_end = max(mas_rmap.last, canonical_ipa_end);
+			entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
+		}
+		new_entry |= UNKNOWN_IPA;
+	} else {
+		new_entry |= nested_ipa;
+		new_entry |= VALID_ENTRY;
+	}
+
+	mas_set_range(&mas_rmap, canonical_ipa, canonical_ipa_end);
+	if (mas_store_gfp(&mas_rmap, xa_mk_value(new_entry),
+			  GFP_NOWAIT | __GFP_ACCOUNT))
+		mmu->nested_revmap_broken = true;
+unlock:
+	mtree_unlock(revmap_mt);
+}
+
 void kvm_init_nested_s2_mmu(struct kvm_s2_mmu *mmu)
 {
 	/* CnP being set denotes an invalid entry */
 	mmu->tlb_vttbr = VTTBR_CNP_BIT;
 	mmu->nested_stage2_enabled = false;
 	atomic_set(&mmu->refcnt, 0);
+	mt_init(&mmu->nested_revmap_mt);
+	mmu->nested_revmap_broken = false;
 }
 
 void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
@@ -1225,6 +1305,89 @@ void kvm_nested_s2_wp(struct kvm *kvm)
 	kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
 }
 
+static void reset_revmap_and_unmap(struct kvm_s2_mmu *mmu, bool may_block)
+{
+	mtree_destroy(&mmu->nested_revmap_mt);
+	mmu->nested_revmap_broken = false;
+	kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
+}
+
+static void unmap_mmu_cipa_range(struct kvm_s2_mmu *mmu, gpa_t canonical_ipa,
+				  size_t unmap_size, bool may_block)
+{
+	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
+	gpa_t canonical_ipa_end = canonical_ipa + unmap_size - 1;
+	size_t entry_size;
+	gpa_t next_addr;
+	u64 entry;
+	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
+
+	lockdep_assert_held_write(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
+
+	if (mmu->nested_revmap_broken) {
+		reset_revmap_and_unmap(mmu, may_block);
+		return;
+	}
+
+	if (!mmu->nested_stage2_enabled) {
+		kvm_stage2_unmap_range(mmu, canonical_ipa, unmap_size, may_block);
+		return;
+	}
+
+	mtree_lock(revmap_mt);
+	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
+
+	while (entry && mas_rmap.index <= canonical_ipa_end) {
+		entry_size = mas_rmap.last - mas_rmap.index + 1;
+		next_addr = mas_rmap.index + entry_size;
+		/*
+		 * Give up and invalidate this s2 mmu if the unmap range
+		 * touches any UNKNOWN_IPA range.
+		 */
+		if (unknown_ipa_entry(entry)) {
+			mtree_unlock(revmap_mt);
+			reset_revmap_and_unmap(mmu, may_block);
+			return;
+		}
+
+		/*
+		 * Ignore result, it is okay if a reverse mapping erase
+		 * fails.
+		 */
+		mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
+
+		mtree_unlock(revmap_mt);
+		kvm_stage2_unmap_range(mmu, entry & ADDR_MASK, entry_size,
+				       may_block);
+		mtree_lock(revmap_mt);
+		/*
+		 * Other maple tree operations during preemption could render
+		 * this ma_state invalid, so reset it.
+		 */
+		mas_set_range(&mas_rmap, next_addr, canonical_ipa_end);
+		entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
+	}
+	mtree_unlock(revmap_mt);
+}
+
+void kvm_unmap_cipa_range_nested(struct kvm *kvm, gpa_t canonical_ipa, size_t size,
+				bool may_block)
+{
+	int i;
+
+	if (!kvm->arch.nested_mmus_size)
+		return;
+
+	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
+		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+
+		if (kvm_s2_mmu_valid(mmu))
+			unmap_mmu_cipa_range(mmu, canonical_ipa, size, may_block);
+	}
+
+	kvm_invalidate_vncr_ipa(kvm, canonical_ipa, canonical_ipa + size);
+}
+
 void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
 {
 	int i;
@@ -1238,7 +1401,7 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
 		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
 
 		if (kvm_s2_mmu_valid(mmu))
-			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
+			reset_revmap_and_unmap(mmu, may_block);
 	}
 
 	kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
@@ -1924,7 +2087,7 @@ void check_nested_vcpu_requests(struct kvm_vcpu *vcpu)
 
 		write_lock(&vcpu->kvm->mmu_lock);
 		if (mmu->pending_unmap) {
-			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), true);
+			reset_revmap_and_unmap(mmu, true);
 			mmu->pending_unmap = false;
 		}
 		write_unlock(&vcpu->kvm->mmu_lock);
-- 
2.43.0

Re: [PATCH v4 2/6] KVM: arm64: nv: Avoid full shadow s2 unmap

[Oliver Upton]

Hey,

On Tue, Jul 14, 2026 at 12:59:21PM +0100, Wei-Lin Chang wrote:
> +static bool valid_entry(unsigned long entry)
> +{
> +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> +	return entry & VALID_ENTRY;
> +}
> +
> +static bool unknown_ipa_entry(unsigned long entry)
> +{
> +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> +	return entry & UNKNOWN_IPA;
> +}

These should be WARN_ON_ONCE(), but I find the condition you're
asserting to be a bit confusing. An aliased reverse map entry is still a
valid entry, would you not set the valid bit?

> +void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
> +			      gpa_t nested_ipa, size_t map_size)
> +{
> +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> +	gpa_t canonical_ipa_end;
> +	u64 entry, new_entry = 0;
> +
> +	lockdep_assert_held_read(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> +
> +	if (mmu->nested_revmap_broken)
> +		return;
> +
> +	if (WARN_ON(!IS_ALIGNED(canonical_ipa, map_size)))
> +		canonical_ipa = ALIGN_DOWN(canonical_ipa, map_size);
> +
> +	canonical_ipa_end = canonical_ipa + map_size - 1;
> +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> +
> +	mtree_lock(revmap_mt);
> +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> +
> +	if (entry) {
> +		/* parallel faults can be adding the same mapping */
> +		if (valid_entry(entry) &&
> +		    mas_rmap.index == canonical_ipa &&
> +		    mas_rmap.last == canonical_ipa_end &&
> +		    nested_ipa == (entry & ADDR_MASK))
> +			goto unlock;
> +		/*
> +		 * Create a "UNKNOWN_IPA" range that spans all the overlapping
> +		 * ranges and store it.
> +		 */
> +		while (entry && mas_rmap.index <= canonical_ipa_end) {
> +			canonical_ipa = min(mas_rmap.index, canonical_ipa);
> +			canonical_ipa_end = max(mas_rmap.last, canonical_ipa_end);
> +			entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> +		}
> +		new_entry |= UNKNOWN_IPA;
> +	} else {
> +		new_entry |= nested_ipa;
> +		new_entry |= VALID_ENTRY;
> +	}
> +
> +	mas_set_range(&mas_rmap, canonical_ipa, canonical_ipa_end);
> +	if (mas_store_gfp(&mas_rmap, xa_mk_value(new_entry),
> +			  GFP_NOWAIT | __GFP_ACCOUNT))

The general pattern for handling this situation would be to preallocate
nodes prior to acquiring the lock. If we can preallocate then we can
just outright refuse to create a shadow stage-2 mapping if the update to
the reverse map fails.

> +		mmu->nested_revmap_broken = true;
> +unlock:
> +	mtree_unlock(revmap_mt);
> +}
> +
>  void kvm_init_nested_s2_mmu(struct kvm_s2_mmu *mmu)
>  {
>  	/* CnP being set denotes an invalid entry */
>  	mmu->tlb_vttbr = VTTBR_CNP_BIT;
>  	mmu->nested_stage2_enabled = false;
>  	atomic_set(&mmu->refcnt, 0);
> +	mt_init(&mmu->nested_revmap_mt);
> +	mmu->nested_revmap_broken = false;
>  }
>  
>  void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
> @@ -1225,6 +1305,89 @@ void kvm_nested_s2_wp(struct kvm *kvm)
>  	kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
>  }
>  
> +static void reset_revmap_and_unmap(struct kvm_s2_mmu *mmu, bool may_block)
> +{
> +	mtree_destroy(&mmu->nested_revmap_mt);
> +	mmu->nested_revmap_broken = false;
> +	kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
> +}
> +
> +static void unmap_mmu_cipa_range(struct kvm_s2_mmu *mmu, gpa_t canonical_ipa,
> +				  size_t unmap_size, bool may_block)
> +{
> +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> +	gpa_t canonical_ipa_end = canonical_ipa + unmap_size - 1;
> +	size_t entry_size;
> +	gpa_t next_addr;
> +	u64 entry;
> +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> +
> +	lockdep_assert_held_write(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> +
> +	if (mmu->nested_revmap_broken) {
> +		reset_revmap_and_unmap(mmu, may_block);
> +		return;
> +	}
> +
> +	if (!mmu->nested_stage2_enabled) {
> +		kvm_stage2_unmap_range(mmu, canonical_ipa, unmap_size, may_block);
> +		return;
> +	}
> +
> +	mtree_lock(revmap_mt);
> +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> +
> +	while (entry && mas_rmap.index <= canonical_ipa_end) {
> +		entry_size = mas_rmap.last - mas_rmap.index + 1;
> +		next_addr = mas_rmap.index + entry_size;
> +		/*
> +		 * Give up and invalidate this s2 mmu if the unmap range
> +		 * touches any UNKNOWN_IPA range.
> +		 */
> +		if (unknown_ipa_entry(entry)) {
> +			mtree_unlock(revmap_mt);
> +			reset_revmap_and_unmap(mmu, may_block);
> +			return;
> +		}
> +
> +		/*
> +		 * Ignore result, it is okay if a reverse mapping erase
> +		 * fails.
> +		 */
> +		mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);

I wonder if we can handle this a bit more gracefully.

Ideally we'd be able to place a search mark on the entry flagging it as
stale which could be used to avoid attempting to overinvalidate the
shadow stage-2.

Interestingly enough, Liam recently gave the suggestion [*] of storing
XA_ZERO_ENTRY for a similar use case where erasure happens in an atomic
context. We should be able to do the exact same thing here and push the
cleanup to the next insertion.

[*] https://lore.kernel.org/all/iv7sxqezzz42xieytbf3lskutricz7ltkr3wii3w4jbnbzto7f@2tz5hwa43l37/

Thanks,
Oliver

Re: [PATCH v4 2/6] KVM: arm64: nv: Avoid full shadow s2 unmap

[Wei-Lin Chang]

On Thu, Jul 16, 2026 at 12:05:29AM -0700, Oliver Upton wrote:
> Hey,
> 
> On Tue, Jul 14, 2026 at 12:59:21PM +0100, Wei-Lin Chang wrote:
> > +static bool valid_entry(unsigned long entry)
> > +{
> > +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> > +	return entry & VALID_ENTRY;
> > +}
> > +
> > +static bool unknown_ipa_entry(unsigned long entry)
> > +{
> > +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> > +	return entry & UNKNOWN_IPA;
> > +}
> 
> These should be WARN_ON_ONCE(), but I find the condition you're
> asserting to be a bit confusing. An aliased reverse map entry is still a
> valid entry, would you not set the valid bit?

Sorry, VALID_ENTRY might not be the best name for this flag. I used it
to mean entries in the maple tree, that are not UNKNOWN_IPA, meaning the
canonical IPA <-> nested IPA relation is one-to-one.

So by definition UNKNOWN_IPA and "VALID_ENTRY" should not be set at the
same time. Unfortunately they can't be a single flag, as the stored
address can be 0, this can make the whole value 0 when being stored if
the single flag is also 0, effectively not doing the store.

Maybe it's better with just "KNOWN_IPA"?

> 
> > +void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
> > +			      gpa_t nested_ipa, size_t map_size)
> > +{
> > +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> > +	gpa_t canonical_ipa_end;
> > +	u64 entry, new_entry = 0;
> > +
> > +	lockdep_assert_held_read(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> > +
> > +	if (mmu->nested_revmap_broken)
> > +		return;
> > +
> > +	if (WARN_ON(!IS_ALIGNED(canonical_ipa, map_size)))
> > +		canonical_ipa = ALIGN_DOWN(canonical_ipa, map_size);
> > +
> > +	canonical_ipa_end = canonical_ipa + map_size - 1;
> > +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> > +
> > +	mtree_lock(revmap_mt);
> > +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > +
> > +	if (entry) {
> > +		/* parallel faults can be adding the same mapping */
> > +		if (valid_entry(entry) &&
> > +		    mas_rmap.index == canonical_ipa &&
> > +		    mas_rmap.last == canonical_ipa_end &&
> > +		    nested_ipa == (entry & ADDR_MASK))
> > +			goto unlock;
> > +		/*
> > +		 * Create a "UNKNOWN_IPA" range that spans all the overlapping
> > +		 * ranges and store it.
> > +		 */
> > +		while (entry && mas_rmap.index <= canonical_ipa_end) {
> > +			canonical_ipa = min(mas_rmap.index, canonical_ipa);
> > +			canonical_ipa_end = max(mas_rmap.last, canonical_ipa_end);
> > +			entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > +		}
> > +		new_entry |= UNKNOWN_IPA;
> > +	} else {
> > +		new_entry |= nested_ipa;
> > +		new_entry |= VALID_ENTRY;
> > +	}
> > +
> > +	mas_set_range(&mas_rmap, canonical_ipa, canonical_ipa_end);
> > +	if (mas_store_gfp(&mas_rmap, xa_mk_value(new_entry),
> > +			  GFP_NOWAIT | __GFP_ACCOUNT))
> 
> The general pattern for handling this situation would be to preallocate
> nodes prior to acquiring the lock. If we can preallocate then we can
> just outright refuse to create a shadow stage-2 mapping if the update to
> the reverse map fails.

Preallocation was thought about, but it was found to be impractical in
this case [*], mainly because we must hold the lock in order to know
what range to store.

However, we can make this function return an error when storing to the
maple tree fails, and just not do the shadow mapping. This ties the
wellness of the maple tree to guest progress. For repeated failure I
think of these possibilities:

 - The maple tree code is somehow broken that stores never succeed.
 - We have so little available memory that maple tree stores don't
   succeed.

These cause the guest fault repeatedly, but do we still care about the
guest when these happen? Probably not for the OOM case, and it solves
itself when the kernel gets back memory. For the first case I'm not so
sure, and I might have missed some other important possibilities.

[*]: https://lore.kernel.org/kvmarm/867bq72n7l.wl-maz@kernel.org/

> 
> > +		mmu->nested_revmap_broken = true;
> > +unlock:
> > +	mtree_unlock(revmap_mt);
> > +}
> > +
> >  void kvm_init_nested_s2_mmu(struct kvm_s2_mmu *mmu)
> >  {
> >  	/* CnP being set denotes an invalid entry */
> >  	mmu->tlb_vttbr = VTTBR_CNP_BIT;
> >  	mmu->nested_stage2_enabled = false;
> >  	atomic_set(&mmu->refcnt, 0);
> > +	mt_init(&mmu->nested_revmap_mt);
> > +	mmu->nested_revmap_broken = false;
> >  }
> >  
> >  void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
> > @@ -1225,6 +1305,89 @@ void kvm_nested_s2_wp(struct kvm *kvm)
> >  	kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
> >  }
> >  
> > +static void reset_revmap_and_unmap(struct kvm_s2_mmu *mmu, bool may_block)
> > +{
> > +	mtree_destroy(&mmu->nested_revmap_mt);
> > +	mmu->nested_revmap_broken = false;
> > +	kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
> > +}
> > +
> > +static void unmap_mmu_cipa_range(struct kvm_s2_mmu *mmu, gpa_t canonical_ipa,
> > +				  size_t unmap_size, bool may_block)
> > +{
> > +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> > +	gpa_t canonical_ipa_end = canonical_ipa + unmap_size - 1;
> > +	size_t entry_size;
> > +	gpa_t next_addr;
> > +	u64 entry;
> > +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> > +
> > +	lockdep_assert_held_write(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> > +
> > +	if (mmu->nested_revmap_broken) {
> > +		reset_revmap_and_unmap(mmu, may_block);
> > +		return;
> > +	}
> > +
> > +	if (!mmu->nested_stage2_enabled) {
> > +		kvm_stage2_unmap_range(mmu, canonical_ipa, unmap_size, may_block);
> > +		return;
> > +	}
> > +
> > +	mtree_lock(revmap_mt);
> > +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > +
> > +	while (entry && mas_rmap.index <= canonical_ipa_end) {
> > +		entry_size = mas_rmap.last - mas_rmap.index + 1;
> > +		next_addr = mas_rmap.index + entry_size;
> > +		/*
> > +		 * Give up and invalidate this s2 mmu if the unmap range
> > +		 * touches any UNKNOWN_IPA range.
> > +		 */
> > +		if (unknown_ipa_entry(entry)) {
> > +			mtree_unlock(revmap_mt);
> > +			reset_revmap_and_unmap(mmu, may_block);
> > +			return;
> > +		}
> > +
> > +		/*
> > +		 * Ignore result, it is okay if a reverse mapping erase
> > +		 * fails.
> > +		 */
> > +		mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
> 
> I wonder if we can handle this a bit more gracefully.
> 
> Ideally we'd be able to place a search mark on the entry flagging it as
> stale which could be used to avoid attempting to overinvalidate the
> shadow stage-2.
> 
> Interestingly enough, Liam recently gave the suggestion [*] of storing
> XA_ZERO_ENTRY for a similar use case where erasure happens in an atomic
> context. We should be able to do the exact same thing here and push the
> cleanup to the next insertion.

I see. Not saying I have a better idea, but this is relying on the fact
that replacing an existing range with another value does not require
memory allocation. I would say this is an obvious assumption and I don't
see it becoming false in the future, but still it's an implementation
detail and relying on it as a user feels strange...

What do you think?

Thanks,
Wei-Lin Chang

> 
> [*] https://lore.kernel.org/all/iv7sxqezzz42xieytbf3lskutricz7ltkr3wii3w4jbnbzto7f@2tz5hwa43l37/
> 
> Thanks,
> Oliver

Re: [PATCH v4 2/6] KVM: arm64: nv: Avoid full shadow s2 unmap

[Oliver Upton]

On Thu, Jul 16, 2026 at 05:14:46PM +0100, Wei-Lin Chang wrote:
> On Thu, Jul 16, 2026 at 12:05:29AM -0700, Oliver Upton wrote:
> > Hey,
> > 
> > On Tue, Jul 14, 2026 at 12:59:21PM +0100, Wei-Lin Chang wrote:
> > > +static bool valid_entry(unsigned long entry)
> > > +{
> > > +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> > > +	return entry & VALID_ENTRY;
> > > +}
> > > +
> > > +static bool unknown_ipa_entry(unsigned long entry)
> > > +{
> > > +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> > > +	return entry & UNKNOWN_IPA;
> > > +}
> > 
> > These should be WARN_ON_ONCE(), but I find the condition you're
> > asserting to be a bit confusing. An aliased reverse map entry is still a
> > valid entry, would you not set the valid bit?
> 
> Sorry, VALID_ENTRY might not be the best name for this flag. I used it
> to mean entries in the maple tree, that are not UNKNOWN_IPA, meaning the
> canonical IPA <-> nested IPA relation is one-to-one.
> 
> So by definition UNKNOWN_IPA and "VALID_ENTRY" should not be set at the
> same time. Unfortunately they can't be a single flag, as the stored
> address can be 0, this can make the whole value 0 when being stored if
> the single flag is also 0, effectively not doing the store.
> 
> Maybe it's better with just "KNOWN_IPA"?

Does anything break with:

state			{VALID, UNKNOWN}

invalid entry (NULL)	{0, 0}
valid IPA		{1, 0}
aliased IPA		{1, 1}

It's just the assertion that the bits are mutually exclusive that I find
confusing, agree that we need to have two bits of state.

> > 
> > > +void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
> > > +			      gpa_t nested_ipa, size_t map_size)
> > > +{
> > > +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> > > +	gpa_t canonical_ipa_end;
> > > +	u64 entry, new_entry = 0;
> > > +
> > > +	lockdep_assert_held_read(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> > > +
> > > +	if (mmu->nested_revmap_broken)
> > > +		return;
> > > +
> > > +	if (WARN_ON(!IS_ALIGNED(canonical_ipa, map_size)))
> > > +		canonical_ipa = ALIGN_DOWN(canonical_ipa, map_size);
> > > +
> > > +	canonical_ipa_end = canonical_ipa + map_size - 1;
> > > +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> > > +
> > > +	mtree_lock(revmap_mt);
> > > +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > > +
> > > +	if (entry) {
> > > +		/* parallel faults can be adding the same mapping */
> > > +		if (valid_entry(entry) &&
> > > +		    mas_rmap.index == canonical_ipa &&
> > > +		    mas_rmap.last == canonical_ipa_end &&
> > > +		    nested_ipa == (entry & ADDR_MASK))
> > > +			goto unlock;
> > > +		/*
> > > +		 * Create a "UNKNOWN_IPA" range that spans all the overlapping
> > > +		 * ranges and store it.
> > > +		 */
> > > +		while (entry && mas_rmap.index <= canonical_ipa_end) {
> > > +			canonical_ipa = min(mas_rmap.index, canonical_ipa);
> > > +			canonical_ipa_end = max(mas_rmap.last, canonical_ipa_end);
> > > +			entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > > +		}
> > > +		new_entry |= UNKNOWN_IPA;
> > > +	} else {
> > > +		new_entry |= nested_ipa;
> > > +		new_entry |= VALID_ENTRY;
> > > +	}
> > > +
> > > +	mas_set_range(&mas_rmap, canonical_ipa, canonical_ipa_end);
> > > +	if (mas_store_gfp(&mas_rmap, xa_mk_value(new_entry),
> > > +			  GFP_NOWAIT | __GFP_ACCOUNT))
> > 
> > The general pattern for handling this situation would be to preallocate
> > nodes prior to acquiring the lock. If we can preallocate then we can
> > just outright refuse to create a shadow stage-2 mapping if the update to
> > the reverse map fails.
> 
> Preallocation was thought about, but it was found to be impractical in
> this case [*], mainly because we must hold the lock in order to know
> what range to store.

Could we perform an RCU-protected walk to predict the range we intend to
store and preallocate based off of that? And if we're not able to
preallocate from a sleepable context then bailing to userspace seems
appropriate.

> However, we can make this function return an error when storing to the
> maple tree fails, and just not do the shadow mapping. This ties the
> wellness of the maple tree to guest progress. For repeated failure I
> think of these possibilities:
> 
>  - The maple tree code is somehow broken that stores never succeed.
>  - We have so little available memory that maple tree stores don't
>    succeed.
> 
> These cause the guest fault repeatedly, but do we still care about the
> guest when these happen? Probably not for the OOM case, and it solves
> itself when the kernel gets back memory. For the first case I'm not so
> sure, and I might have missed some other important possibilities.

So there's two failure modes that we need to handle.

The most common case would be a race to update the maple tree, where the
preallocation we did outside of the MMU lock is insufficient for the actual
operation we perform. Typically in the event of a race we restart the faulting
instruction and re-attempt the fault. So long as you pass gfp=0 to the
store, you can reinterpret -ENOMEM as -EAGAIN and refault.

At that point, any non ENOMEM failure to store in the maple tree can be
treated as fatal for the VM, KVM_BUG_ON() and out to userspace.

> > > +static void unmap_mmu_cipa_range(struct kvm_s2_mmu *mmu, gpa_t canonical_ipa,
> > > +				  size_t unmap_size, bool may_block)
> > > +{
> > > +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> > > +	gpa_t canonical_ipa_end = canonical_ipa + unmap_size - 1;
> > > +	size_t entry_size;
> > > +	gpa_t next_addr;
> > > +	u64 entry;
> > > +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> > > +
> > > +	lockdep_assert_held_write(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> > > +
> > > +	if (mmu->nested_revmap_broken) {
> > > +		reset_revmap_and_unmap(mmu, may_block);
> > > +		return;
> > > +	}
> > > +
> > > +	if (!mmu->nested_stage2_enabled) {
> > > +		kvm_stage2_unmap_range(mmu, canonical_ipa, unmap_size, may_block);
> > > +		return;
> > > +	}
> > > +
> > > +	mtree_lock(revmap_mt);
> > > +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > > +
> > > +	while (entry && mas_rmap.index <= canonical_ipa_end) {
> > > +		entry_size = mas_rmap.last - mas_rmap.index + 1;
> > > +		next_addr = mas_rmap.index + entry_size;
> > > +		/*
> > > +		 * Give up and invalidate this s2 mmu if the unmap range
> > > +		 * touches any UNKNOWN_IPA range.
> > > +		 */
> > > +		if (unknown_ipa_entry(entry)) {
> > > +			mtree_unlock(revmap_mt);
> > > +			reset_revmap_and_unmap(mmu, may_block);
> > > +			return;
> > > +		}
> > > +
> > > +		/*
> > > +		 * Ignore result, it is okay if a reverse mapping erase
> > > +		 * fails.
> > > +		 */
> > > +		mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
> > 
> > I wonder if we can handle this a bit more gracefully.
> > 
> > Ideally we'd be able to place a search mark on the entry flagging it as
> > stale which could be used to avoid attempting to overinvalidate the
> > shadow stage-2.
> > 
> > Interestingly enough, Liam recently gave the suggestion [*] of storing
> > XA_ZERO_ENTRY for a similar use case where erasure happens in an atomic
> > context. We should be able to do the exact same thing here and push the
> > cleanup to the next insertion.
> 
> I see. Not saying I have a better idea, but this is relying on the fact
> that replacing an existing range with another value does not require
> memory allocation. I would say this is an obvious assumption and I don't
> see it becoming false in the future, but still it's an implementation
> detail and relying on it as a user feels strange...
> 
> What do you think?

I believe there's value in such an approach because it reduces the
likelihood of a false positive. Since we could be updating the maple
tree as part of reclaim I think it's safe to assume the system is under
some amount of memory pressure (and GFP_NOWAIT could fail). Although I
agree, it'd be good to document this behavior explicitly.

Thanks,
Oliver

Re: [PATCH v4 2/6] KVM: arm64: nv: Avoid full shadow s2 unmap

[Wei-Lin Chang]

On Thu, Jul 16, 2026 at 10:16:51AM -0700, Oliver Upton wrote:
> On Thu, Jul 16, 2026 at 05:14:46PM +0100, Wei-Lin Chang wrote:
> > On Thu, Jul 16, 2026 at 12:05:29AM -0700, Oliver Upton wrote:
> > > Hey,
> > > 
> > > On Tue, Jul 14, 2026 at 12:59:21PM +0100, Wei-Lin Chang wrote:
> > > > +static bool valid_entry(unsigned long entry)
> > > > +{
> > > > +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> > > > +	return entry & VALID_ENTRY;
> > > > +}
> > > > +
> > > > +static bool unknown_ipa_entry(unsigned long entry)
> > > > +{
> > > > +	WARN_ON(entry & VALID_ENTRY && entry & UNKNOWN_IPA);
> > > > +	return entry & UNKNOWN_IPA;
> > > > +}
> > > 
> > > These should be WARN_ON_ONCE(), but I find the condition you're
> > > asserting to be a bit confusing. An aliased reverse map entry is still a
> > > valid entry, would you not set the valid bit?
> > 
> > Sorry, VALID_ENTRY might not be the best name for this flag. I used it
> > to mean entries in the maple tree, that are not UNKNOWN_IPA, meaning the
> > canonical IPA <-> nested IPA relation is one-to-one.
> > 
> > So by definition UNKNOWN_IPA and "VALID_ENTRY" should not be set at the
> > same time. Unfortunately they can't be a single flag, as the stored
> > address can be 0, this can make the whole value 0 when being stored if
> > the single flag is also 0, effectively not doing the store.
> > 
> > Maybe it's better with just "KNOWN_IPA"?
> 
> Does anything break with:
> 
> state			{VALID, UNKNOWN}
> 
> invalid entry (NULL)	{0, 0}
> valid IPA		{1, 0}
> aliased IPA		{1, 1}
> 
> It's just the assertion that the bits are mutually exclusive that I find
> confusing, agree that we need to have two bits of state.

This looks like a better way of representing state, thanks! I think
"valid" now makes more sense.

> 
> > > 
> > > > +void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
> > > > +			      gpa_t nested_ipa, size_t map_size)
> > > > +{
> > > > +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> > > > +	gpa_t canonical_ipa_end;
> > > > +	u64 entry, new_entry = 0;
> > > > +
> > > > +	lockdep_assert_held_read(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> > > > +
> > > > +	if (mmu->nested_revmap_broken)
> > > > +		return;
> > > > +
> > > > +	if (WARN_ON(!IS_ALIGNED(canonical_ipa, map_size)))
> > > > +		canonical_ipa = ALIGN_DOWN(canonical_ipa, map_size);
> > > > +
> > > > +	canonical_ipa_end = canonical_ipa + map_size - 1;
> > > > +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> > > > +
> > > > +	mtree_lock(revmap_mt);
> > > > +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > > > +
> > > > +	if (entry) {
> > > > +		/* parallel faults can be adding the same mapping */
> > > > +		if (valid_entry(entry) &&
> > > > +		    mas_rmap.index == canonical_ipa &&
> > > > +		    mas_rmap.last == canonical_ipa_end &&
> > > > +		    nested_ipa == (entry & ADDR_MASK))
> > > > +			goto unlock;
> > > > +		/*
> > > > +		 * Create a "UNKNOWN_IPA" range that spans all the overlapping
> > > > +		 * ranges and store it.
> > > > +		 */
> > > > +		while (entry && mas_rmap.index <= canonical_ipa_end) {
> > > > +			canonical_ipa = min(mas_rmap.index, canonical_ipa);
> > > > +			canonical_ipa_end = max(mas_rmap.last, canonical_ipa_end);
> > > > +			entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > > > +		}
> > > > +		new_entry |= UNKNOWN_IPA;
> > > > +	} else {
> > > > +		new_entry |= nested_ipa;
> > > > +		new_entry |= VALID_ENTRY;
> > > > +	}
> > > > +
> > > > +	mas_set_range(&mas_rmap, canonical_ipa, canonical_ipa_end);
> > > > +	if (mas_store_gfp(&mas_rmap, xa_mk_value(new_entry),
> > > > +			  GFP_NOWAIT | __GFP_ACCOUNT))
> > > 
> > > The general pattern for handling this situation would be to preallocate
> > > nodes prior to acquiring the lock. If we can preallocate then we can
> > > just outright refuse to create a shadow stage-2 mapping if the update to
> > > the reverse map fails.
> > 
> > Preallocation was thought about, but it was found to be impractical in
> > this case [*], mainly because we must hold the lock in order to know
> > what range to store.
> 
> Could we perform an RCU-protected walk to predict the range we intend to
> store and preallocate based off of that? And if we're not able to
> preallocate from a sleepable context then bailing to userspace seems
> appropriate.

I agree it's much better if we can allocate from a sleepable context,
however I see a few concerns in this case:

1. It needs to happen before kvm_fault_lock(), where some of the fault
   information is still unknown, mostly the THP adjustment is not done
   yet.

2. The nested kvm_s2_mmu array, which contain the maple trees, could be
   freed without holding mmu_lock (kvm_vcpu_init_nested()).

3. If we use RCU, we still can't preallocate with a sleepable flag
   because we would be holding rcu_read_lock().

4. Caching maple tree state while holding rcu_read_lock() and try
   preallocating after dropping the lock would also not work, because
   mas_preallocate() walks the tree therefore it must be protected from
   tree writers. And also (2.).

> 
> > However, we can make this function return an error when storing to the
> > maple tree fails, and just not do the shadow mapping. This ties the
> > wellness of the maple tree to guest progress. For repeated failure I
> > think of these possibilities:
> > 
> >  - The maple tree code is somehow broken that stores never succeed.
> >  - We have so little available memory that maple tree stores don't
> >    succeed.
> > 
> > These cause the guest fault repeatedly, but do we still care about the
> > guest when these happen? Probably not for the OOM case, and it solves
> > itself when the kernel gets back memory. For the first case I'm not so
> > sure, and I might have missed some other important possibilities.
> 
> So there's two failure modes that we need to handle.
> 
> The most common case would be a race to update the maple tree, where the
> preallocation we did outside of the MMU lock is insufficient for the actual
> operation we perform. Typically in the event of a race we restart the faulting
> instruction and re-attempt the fault. So long as you pass gfp=0 to the
> store, you can reinterpret -ENOMEM as -EAGAIN and refault.
> At that point, any non ENOMEM failure to store in the maple tree can be
> treated as fatal for the VM, KVM_BUG_ON() and out to userspace.

From the above, I don't think sleepable preallocation is feasible, at
least without invasive changes. Even if we somehow solve problem 2.
above, we'll probably need a new stateless preallocation API from the
maple tree (to avoid walking the tree without any locks), or start using
an external sleepable lock for the maple tree.

With this information, would the current no preallocation approach be
preferrable, or do we want to pursue sleepable preallocation anyway?

> 
> > > > +static void unmap_mmu_cipa_range(struct kvm_s2_mmu *mmu, gpa_t canonical_ipa,
> > > > +				  size_t unmap_size, bool may_block)
> > > > +{
> > > > +	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
> > > > +	gpa_t canonical_ipa_end = canonical_ipa + unmap_size - 1;
> > > > +	size_t entry_size;
> > > > +	gpa_t next_addr;
> > > > +	u64 entry;
> > > > +	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
> > > > +
> > > > +	lockdep_assert_held_write(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
> > > > +
> > > > +	if (mmu->nested_revmap_broken) {
> > > > +		reset_revmap_and_unmap(mmu, may_block);
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	if (!mmu->nested_stage2_enabled) {
> > > > +		kvm_stage2_unmap_range(mmu, canonical_ipa, unmap_size, may_block);
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	mtree_lock(revmap_mt);
> > > > +	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
> > > > +
> > > > +	while (entry && mas_rmap.index <= canonical_ipa_end) {
> > > > +		entry_size = mas_rmap.last - mas_rmap.index + 1;
> > > > +		next_addr = mas_rmap.index + entry_size;
> > > > +		/*
> > > > +		 * Give up and invalidate this s2 mmu if the unmap range
> > > > +		 * touches any UNKNOWN_IPA range.
> > > > +		 */
> > > > +		if (unknown_ipa_entry(entry)) {
> > > > +			mtree_unlock(revmap_mt);
> > > > +			reset_revmap_and_unmap(mmu, may_block);
> > > > +			return;
> > > > +		}
> > > > +
> > > > +		/*
> > > > +		 * Ignore result, it is okay if a reverse mapping erase
> > > > +		 * fails.
> > > > +		 */
> > > > +		mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
> > > 
> > > I wonder if we can handle this a bit more gracefully.
> > > 
> > > Ideally we'd be able to place a search mark on the entry flagging it as
> > > stale which could be used to avoid attempting to overinvalidate the
> > > shadow stage-2.
> > > 
> > > Interestingly enough, Liam recently gave the suggestion [*] of storing
> > > XA_ZERO_ENTRY for a similar use case where erasure happens in an atomic
> > > context. We should be able to do the exact same thing here and push the
> > > cleanup to the next insertion.
> > 
> > I see. Not saying I have a better idea, but this is relying on the fact
> > that replacing an existing range with another value does not require
> > memory allocation. I would say this is an obvious assumption and I don't
> > see it becoming false in the future, but still it's an implementation
> > detail and relying on it as a user feels strange...
> > 
> > What do you think?
> 
> I believe there's value in such an approach because it reduces the
> likelihood of a false positive. Since we could be updating the maple
> tree as part of reclaim I think it's safe to assume the system is under
> some amount of memory pressure (and GFP_NOWAIT could fail). Although I
> agree, it'd be good to document this behavior explicitly.

Sure, this is doable.

Thanks,
Wei-Lin Chang

> 
> Thanks,
> Oliver

[PATCH v4 3/6] KVM: arm64: nv: Add nested revmap broken tracepoint

[Wei-Lin Chang]

Add a tracepoint to track when a nested reverse map becomes broken. This
makes it easier to detect NV performance degradation caused by full
shadow stage-2 unmaps.

Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
 arch/arm64/kvm/nested.c    |  7 ++++++-
 arch/arm64/kvm/trace_arm.h | 27 +++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 065823e415ce..22f8a1daca93 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -17,6 +17,7 @@
 #include <asm/sysreg.h>
 
 #include "sys_regs.h"
+#include "trace.h"
 
 struct vncr_tlb {
 	/* The guest's VNCR_EL2 */
@@ -916,8 +917,12 @@ void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
 
 	mas_set_range(&mas_rmap, canonical_ipa, canonical_ipa_end);
 	if (mas_store_gfp(&mas_rmap, xa_mk_value(new_entry),
-			  GFP_NOWAIT | __GFP_ACCOUNT))
+			  GFP_NOWAIT | __GFP_ACCOUNT)) {
+		trace_kvm_nested_revmap_broken(mmu - mmu->arch->nested_mmus,
+					       canonical_ipa, canonical_ipa_end,
+					       nested_ipa, new_entry);
 		mmu->nested_revmap_broken = true;
+	}
 unlock:
 	mtree_unlock(revmap_mt);
 }
diff --git a/arch/arm64/kvm/trace_arm.h b/arch/arm64/kvm/trace_arm.h
index 9c60f6465c78..0db90068ff63 100644
--- a/arch/arm64/kvm/trace_arm.h
+++ b/arch/arm64/kvm/trace_arm.h
@@ -415,6 +415,33 @@ TRACE_EVENT(kvm_forward_sysreg_trap,
 		      sys_reg_Op2(__entry->sysreg))
 );
 
+TRACE_EVENT(kvm_nested_revmap_broken,
+	    TP_PROTO(int mmu_idx, unsigned long canonical_ipa,
+		     unsigned long canonical_ipa_end, unsigned long nested_ipa,
+		     unsigned long revmap_entry),
+	    TP_ARGS(mmu_idx, canonical_ipa, canonical_ipa_end, nested_ipa, revmap_entry),
+
+	    TP_STRUCT__entry(
+		__field(int,			mmu_idx)
+		__field(unsigned long,		canonical_ipa)
+		__field(unsigned long,		nested_ipa)
+		__field(size_t,			size)
+		__field(unsigned long,		revmap_entry)
+	    ),
+
+	    TP_fast_assign(
+		__entry->mmu_idx = mmu_idx;
+		__entry->canonical_ipa = canonical_ipa;
+		__entry->nested_ipa = nested_ipa;
+		__entry->size = canonical_ipa_end - canonical_ipa + 1;
+		__entry->revmap_entry = revmap_entry;
+	    ),
+
+	    TP_printk("mmu idx: %d, cipa: 0x%016lx, nipa: 0x%016lx, size: 0x%016lx, entry: %016lx",
+		      __entry->mmu_idx, __entry->canonical_ipa, __entry->nested_ipa,
+		      __entry->size, __entry->revmap_entry)
+);
+
 #endif /* _TRACE_ARM_ARM64_KVM_H */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.43.0

Re: [PATCH v4 3/6] KVM: arm64: nv: Add nested revmap broken tracepoint

[Oliver Upton]

Hi Wei-Lin,

On Tue, Jul 14, 2026 at 12:59:22PM +0100, Wei-Lin Chang wrote:
> Add a tracepoint to track when a nested reverse map becomes broken. This
> makes it easier to detect NV performance degradation caused by full
> shadow stage-2 unmaps.
> 
> Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>

I'm not too excited about making a tracepoint out of this, especially
since a 'broken' reverse map is more of a KVM implementation detail
(that could change) than anything else.

BPF is quite popular at this point, do you think it's reasonable to
instead expect the curious KVM developer to attach a kprobe and inspect
the state of the nested MMU?

Thanks,
Oliver

> ---
>  arch/arm64/kvm/nested.c    |  7 ++++++-
>  arch/arm64/kvm/trace_arm.h | 27 +++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 065823e415ce..22f8a1daca93 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -17,6 +17,7 @@
>  #include <asm/sysreg.h>
>  
>  #include "sys_regs.h"
> +#include "trace.h"
>  
>  struct vncr_tlb {
>  	/* The guest's VNCR_EL2 */
> @@ -916,8 +917,12 @@ void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
>  
>  	mas_set_range(&mas_rmap, canonical_ipa, canonical_ipa_end);
>  	if (mas_store_gfp(&mas_rmap, xa_mk_value(new_entry),
> -			  GFP_NOWAIT | __GFP_ACCOUNT))
> +			  GFP_NOWAIT | __GFP_ACCOUNT)) {
> +		trace_kvm_nested_revmap_broken(mmu - mmu->arch->nested_mmus,
> +					       canonical_ipa, canonical_ipa_end,
> +					       nested_ipa, new_entry);
>  		mmu->nested_revmap_broken = true;
> +	}
>  unlock:
>  	mtree_unlock(revmap_mt);
>  }
> diff --git a/arch/arm64/kvm/trace_arm.h b/arch/arm64/kvm/trace_arm.h
> index 9c60f6465c78..0db90068ff63 100644
> --- a/arch/arm64/kvm/trace_arm.h
> +++ b/arch/arm64/kvm/trace_arm.h
> @@ -415,6 +415,33 @@ TRACE_EVENT(kvm_forward_sysreg_trap,
>  		      sys_reg_Op2(__entry->sysreg))
>  );
>  
> +TRACE_EVENT(kvm_nested_revmap_broken,
> +	    TP_PROTO(int mmu_idx, unsigned long canonical_ipa,
> +		     unsigned long canonical_ipa_end, unsigned long nested_ipa,
> +		     unsigned long revmap_entry),
> +	    TP_ARGS(mmu_idx, canonical_ipa, canonical_ipa_end, nested_ipa, revmap_entry),
> +
> +	    TP_STRUCT__entry(
> +		__field(int,			mmu_idx)
> +		__field(unsigned long,		canonical_ipa)
> +		__field(unsigned long,		nested_ipa)
> +		__field(size_t,			size)
> +		__field(unsigned long,		revmap_entry)
> +	    ),
> +
> +	    TP_fast_assign(
> +		__entry->mmu_idx = mmu_idx;
> +		__entry->canonical_ipa = canonical_ipa;
> +		__entry->nested_ipa = nested_ipa;
> +		__entry->size = canonical_ipa_end - canonical_ipa + 1;
> +		__entry->revmap_entry = revmap_entry;
> +	    ),
> +
> +	    TP_printk("mmu idx: %d, cipa: 0x%016lx, nipa: 0x%016lx, size: 0x%016lx, entry: %016lx",
> +		      __entry->mmu_idx, __entry->canonical_ipa, __entry->nested_ipa,
> +		      __entry->size, __entry->revmap_entry)
> +);
> +
>  #endif /* _TRACE_ARM_ARM64_KVM_H */
>  
>  #undef TRACE_INCLUDE_PATH
> -- 
> 2.43.0
> 

Re: [PATCH v4 3/6] KVM: arm64: nv: Add nested revmap broken tracepoint

[Marc Zyngier]

On Thu, 16 Jul 2026 07:45:05 +0100,
Oliver Upton <oupton@kernel.org> wrote:
> 
> Hi Wei-Lin,
> 
> On Tue, Jul 14, 2026 at 12:59:22PM +0100, Wei-Lin Chang wrote:
> > Add a tracepoint to track when a nested reverse map becomes broken. This
> > makes it easier to detect NV performance degradation caused by full
> > shadow stage-2 unmaps.
> > 
> > Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
> 
> I'm not too excited about making a tracepoint out of this, especially
> since a 'broken' reverse map is more of a KVM implementation detail
> (that could change) than anything else.
> 
> BPF is quite popular at this point, do you think it's reasonable to
> instead expect the curious KVM developer to attach a kprobe and inspect
> the state of the nested MMU?

I was the one asking for a way to report this stuff. I'd be completely
fine with BPF as long as we give the recipe for this so that people
can readily look at this.

Even better, find a way not to have a broken rmap the first place ;-).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v4 3/6] KVM: arm64: nv: Add nested revmap broken tracepoint

[Wei-Lin Chang]

On Thu, Jul 16, 2026 at 09:56:33AM +0100, Marc Zyngier wrote:
> On Thu, 16 Jul 2026 07:45:05 +0100,
> Oliver Upton <oupton@kernel.org> wrote:
> > 
> > Hi Wei-Lin,
> > 
> > On Tue, Jul 14, 2026 at 12:59:22PM +0100, Wei-Lin Chang wrote:
> > > Add a tracepoint to track when a nested reverse map becomes broken. This
> > > makes it easier to detect NV performance degradation caused by full
> > > shadow stage-2 unmaps.
> > > 
> > > Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
> > 
> > I'm not too excited about making a tracepoint out of this, especially
> > since a 'broken' reverse map is more of a KVM implementation detail
> > (that could change) than anything else.
> > 
> > BPF is quite popular at this point, do you think it's reasonable to
> > instead expect the curious KVM developer to attach a kprobe and inspect
> > the state of the nested MMU?
> 
> I was the one asking for a way to report this stuff. I'd be completely
> fine with BPF as long as we give the recipe for this so that people
> can readily look at this.

Cool, I'll drop the tracepoint in the next iteration.

> 
> Even better, find a way not to have a broken rmap the first place ;-).

One possible way is to not do the shadow stage-2 map if maple tree store
fails, please see the response for patch 2 for details!

Thanks,
Wei-Lin Chang

> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

[PATCH v4 4/6] KVM: arm64: Refactor kvm_unmap_gfn_range() with common variables

[Wei-Lin Chang]

__unmap_stage2_range() and kvm_unmap_cipa_range_nested() are using the
same arguments, clean this up by using local variables.

Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
 arch/arm64/kvm/mmu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 73629aa1ade6..6bfc1fbc5387 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2441,15 +2441,16 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
 
 bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)
 {
+	gpa_t gpa = range->start << PAGE_SHIFT;
+	size_t size = (range->end - range->start) << PAGE_SHIFT;
+	bool may_block = range->may_block;
+
 	if (!kvm->arch.mmu.pgt || kvm_vm_is_protected(kvm))
 		return false;
 
-	__unmap_stage2_range(&kvm->arch.mmu, range->start << PAGE_SHIFT,
-			     (range->end - range->start) << PAGE_SHIFT,
-			     range->may_block);
-	kvm_unmap_cipa_range_nested(kvm, range->start << PAGE_SHIFT,
-				    (range->end - range->start) << PAGE_SHIFT,
-				    range->may_block);
+	__unmap_stage2_range(&kvm->arch.mmu, gpa, size, may_block);
+	kvm_unmap_cipa_range_nested(kvm, gpa, size, may_block);
+
 	return false;
 }
 
-- 
2.43.0

[PATCH v4 5/6] KVM: arm64: nv: Remove reverse map entries during TLBI handling

[Wei-Lin Chang]

When a guest hypervisor issues a TLBI for a specific IPA range, KVM
unmaps that range from all the affected shadow stage-2s. During this we
get the opportunity to remove the reverse map, and lower the probability
of creating UNKNOWN_IPA reverse map ranges at subsequent stage-2 faults.

However, the TLBI ranges are specified in nested IPA, so in order to
locate the affected ranges in the reverse map maple tree, which is a
mapping from canonical IPA to nested IPA, we can only iterate through
the entire tree and check each entry.

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
 arch/arm64/include/asm/kvm_nested.h |  2 ++
 arch/arm64/kvm/nested.c             | 37 +++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c           |  3 +++
 3 files changed, 42 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 79de224f3438..664d789d17b1 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -77,6 +77,8 @@ extern void kvm_s2_mmu_iterate_by_vmid(struct kvm *kvm, u16 vmid,
 				       const union tlbi_info *info,
 				       void (*)(struct kvm_s2_mmu *,
 						const union tlbi_info *));
+extern void kvm_remove_nested_revmap(struct kvm_s2_mmu *mmu, u64 nested_ipa,
+				     size_t size);
 extern void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
 				     gpa_t nested_ipa, size_t map_size);
 extern void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 22f8a1daca93..cd93793fe89d 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -872,6 +872,43 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
 	return s2_mmu;
 }
 
+void kvm_remove_nested_revmap(struct kvm_s2_mmu *mmu, u64 nested_ipa, size_t size)
+{
+	/*
+	 * Iterate through the mt of this mmu, remove all canonical ipa ranges
+	 * with !UNKNOWN_IPA that maps to ranges that are strictly within
+	 * [addr, addr + size).
+	 */
+	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
+	u64 entry_val, nested_ipa_end = nested_ipa + size;
+	u64 this_nested_ipa, this_nested_ipa_end;
+	size_t revmap_size;
+	void *entry;
+
+	MA_STATE(mas_rmap, revmap_mt, 0, ULONG_MAX);
+
+	mtree_lock(revmap_mt);
+	mas_for_each(&mas_rmap, entry, ULONG_MAX) {
+		entry_val = xa_to_value(entry);
+		if (unknown_ipa_entry(entry_val))
+			continue;
+
+		revmap_size = mas_rmap.last - mas_rmap.index + 1;
+		this_nested_ipa = entry_val & ADDR_MASK;
+		this_nested_ipa_end = this_nested_ipa + revmap_size;
+
+		if (this_nested_ipa >= nested_ipa &&
+		    this_nested_ipa_end <= nested_ipa_end) {
+			/*
+			 * Ignore result, failure to remove reverse mapping will
+			 * only cause extra unmaps.
+			 */
+			mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
+		}
+	}
+	mtree_unlock(revmap_mt);
+}
+
 void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
 			      gpa_t nested_ipa, size_t map_size)
 {
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 33c921df19b5..e4ce70508af6 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -4006,6 +4006,7 @@ union tlbi_info {
 static void s2_mmu_unmap_range(struct kvm_s2_mmu *mmu,
 			       const union tlbi_info *info)
 {
+	kvm_remove_nested_revmap(mmu, info->range.start, info->range.size);
 	/*
 	 * The unmap operation is allowed to drop the MMU lock and block, which
 	 * means that @mmu could be used for a different context than the one
@@ -4104,6 +4105,8 @@ static void s2_mmu_unmap_ipa(struct kvm_s2_mmu *mmu,
 	max_size = compute_tlb_inval_range(mmu, info->ipa.addr);
 	base_addr &= ~(max_size - 1);
 
+	kvm_remove_nested_revmap(mmu, base_addr, max_size);
+
 	/*
 	 * See comment in s2_mmu_unmap_range() for why this is allowed to
 	 * reschedule.
-- 
2.43.0

[PATCH v4 6/6] KVM: arm64: nv: Create nested IPA direct map to speed up reverse map removal

[Wei-Lin Chang]

Iterating through the whole reverse map to find which entries to remove
when handling guest hypervisor TLBIs is not efficient. Create a direct
map that goes from nested IPA to canonical IPA so that the canonical
IPA range affected by the TLBI can be quickly determined, then remove
the entries in the reverse map accordingly.

Since insertion and deletion to the maple tree aren't guaranteed to
succeed, the direct map is implemented best-effort, and we always check
the direct mapping and the reverse mapping are symmetric (they store the
same mapping in reverse) before removing the reverse mapping.

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
---
 arch/arm64/include/asm/kvm_host.h |   5 ++
 arch/arm64/kvm/mmu.c              |   9 +-
 arch/arm64/kvm/nested.c           | 131 ++++++++++++++++++++++--------
 3 files changed, 108 insertions(+), 37 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 42ab121a04f8..9b385897c352 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -230,6 +230,11 @@ struct kvm_s2_mmu {
 	bool	nested_revmap_broken;
 	/* canonical IPA to nested IPA range lookup */
 	struct maple_tree nested_revmap_mt;
+	/*
+	 * Nested IPA to canonical IPA range lookup, essentially a cache of
+	 * the guest's stage-2.
+	 */
+	struct maple_tree nested_direct_mt;
 
 	/*
 	 *  0: Nobody is currently using this, check vttbr for validity
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 6bfc1fbc5387..b0cca48040c4 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1128,6 +1128,7 @@ void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)
 	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
 	struct kvm_pgtable *pgt = NULL;
 	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
+	struct maple_tree *direct_mt = &mmu->nested_direct_mt;
 
 	write_lock(&kvm->mmu_lock);
 	pgt = mmu->pgt;
@@ -1138,8 +1139,12 @@ void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)
 	}
 
 	if (kvm_is_nested_s2_mmu(kvm, mmu)) {
-		if (!mtree_empty(revmap_mt))
-			mtree_destroy(revmap_mt);
+		if (!mtree_empty(revmap_mt) || !mtree_empty(direct_mt)) {
+			mtree_lock(revmap_mt);
+			__mt_destroy(revmap_mt);
+			__mt_destroy(direct_mt);
+			mtree_unlock(revmap_mt);
+		}
 		kvm_init_nested_s2_mmu(mmu);
 	}
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index cd93793fe89d..975a1b38d545 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -47,14 +47,14 @@ struct vncr_tlb {
 #define S2_MMU_PER_VCPU		2
 
 /*
- * Per shadow S2 reverse map (canonical IPA -> nested IPA range) maple tree
- * payload layout:
+ * Per shadow S2 reverse & direct map maple tree payload layout:
  *
- * bit  62:     valid, prevents the case where the nested IPA is 0 and turning
+ * bit  62:     valid, prevents the case where the address is 0 and turning
  *              the whole value to 0 (NULL for maple tree)
- * bits 55-12:  nested IPA bits 55-12
+ * bits 55-12:  {nested, canonical} IPA bits 55-12
  * bit  0:      UNKNOWN_IPA bit, 1 indicates we give up on tracking what nested
- *              IPA maps to this canonical IPA in the shadow stage-2
+ *              IPA maps to this canonical IPA in the shadow stage-2, only used
+ *              in reverse map
  */
 #define VALID_ENTRY		BIT(62)
 #define ADDR_MASK		GENMASK_ULL(55, 12)
@@ -127,12 +127,18 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
 	swap(kvm->arch.nested_mmus, tmp);
 
 	/*
-	 * If we went through a realocation, adjust the MMU back-pointers in
-	 * the previously initialised kvm_pgtable structures.
+	 * If we went through a reallocation, adjust pointers in the
+	 * previously initialised MMUs.
 	 */
-	if (kvm->arch.nested_mmus != tmp)
-		for (int i = 0; i < kvm->arch.nested_mmus_size; i++)
-			kvm->arch.nested_mmus[i].pgt->mmu = &kvm->arch.nested_mmus[i];
+	if (kvm->arch.nested_mmus != tmp) {
+		for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
+			struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+
+			mmu->pgt->mmu = &kvm->arch.nested_mmus[i];
+			mt_set_external_lock(&mmu->nested_direct_mt,
+					     &mmu->nested_revmap_mt.ma_lock);
+		}
+	}
 
 	for (int i = kvm->arch.nested_mmus_size; !ret && i < num_mmus; i++)
 		ret = init_nested_s2_mmu(kvm, &kvm->arch.nested_mmus[i]);
@@ -875,36 +881,61 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
 void kvm_remove_nested_revmap(struct kvm_s2_mmu *mmu, u64 nested_ipa, size_t size)
 {
 	/*
-	 * Iterate through the mt of this mmu, remove all canonical ipa ranges
-	 * with !UNKNOWN_IPA that maps to ranges that are strictly within
-	 * [addr, addr + size).
+	 * Remove the reverse map and its corresponding direct map together,
+	 * when these conditions are met:
+	 *
+	 * 1. The reverse map is valid (not UNKNOWN_IPA).
+	 * 2. The reverse map is completely covered by the TLBI range.
+	 * 3. The reverse map and the direct map are symmetric i.e. they map to
+	 *    each other, with the same size.
+	 *
+	 * The symmetry check ensures we don't remove wrong reverse mappings.
 	 */
 	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
-	u64 entry_val, nested_ipa_end = nested_ipa + size;
-	u64 this_nested_ipa, this_nested_ipa_end;
-	size_t revmap_size;
-	void *entry;
-
-	MA_STATE(mas_rmap, revmap_mt, 0, ULONG_MAX);
-
+	struct maple_tree *direct_mt = &mmu->nested_direct_mt;
+	gpa_t nested_ipa_end = nested_ipa + size - 1;
+	u64 entry_dmap;
+	struct mapping {
+		u64 from;
+		u64 to;
+		size_t size;
+	};
+
+	MA_STATE(mas_dmap, direct_mt, nested_ipa, nested_ipa_end);
 	mtree_lock(revmap_mt);
-	mas_for_each(&mas_rmap, entry, ULONG_MAX) {
-		entry_val = xa_to_value(entry);
-		if (unknown_ipa_entry(entry_val))
-			continue;
+	entry_dmap = xa_to_value(mas_find(&mas_dmap, nested_ipa_end));
 
-		revmap_size = mas_rmap.last - mas_rmap.index + 1;
-		this_nested_ipa = entry_val & ADDR_MASK;
-		this_nested_ipa_end = this_nested_ipa + revmap_size;
+	while (entry_dmap && mas_dmap.index <= nested_ipa_end) {
+		struct mapping dmap, rmap;
+		u64 entry_rmap;
 
-		if (this_nested_ipa >= nested_ipa &&
-		    this_nested_ipa_end <= nested_ipa_end) {
-			/*
-			 * Ignore result, failure to remove reverse mapping will
-			 * only cause extra unmaps.
-			 */
+		dmap.from = mas_dmap.index;
+		dmap.to   = entry_dmap & ADDR_MASK;
+		dmap.size = mas_dmap.last - mas_dmap.index + 1;
+
+		/*
+		 * Use the canonical ipa range to find the corresponding entry
+		 * in revmap.
+		 */
+		MA_STATE(mas_rmap, revmap_mt, dmap.to, dmap.to + dmap.size - 1);
+		entry_rmap = xa_to_value(mas_find(&mas_rmap,
+						       dmap.to + dmap.size - 1));
+
+		rmap.from = mas_rmap.index;
+		rmap.to   = entry_rmap & ADDR_MASK;
+		rmap.size = mas_rmap.last - mas_rmap.index + 1;
+
+		/* The three conditions outlined above. */
+		if (entry_rmap && valid_entry(entry_rmap) &&
+		    dmap.from >= nested_ipa &&
+		    dmap.from + dmap.size - 1 <= nested_ipa_end &&
+		    dmap.from == rmap.to &&
+		    rmap.from == dmap.to &&
+		    dmap.size == rmap.size) {
+			mas_store_gfp(&mas_dmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
 			mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
 		}
+		entry_dmap = xa_to_value(mas_find(&mas_dmap, nested_ipa_end));
 	}
 	mtree_unlock(revmap_mt);
 }
@@ -913,7 +944,8 @@ void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
 			      gpa_t nested_ipa, size_t map_size)
 {
 	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
-	gpa_t canonical_ipa_end;
+	struct maple_tree *direct_mt = &mmu->nested_direct_mt;
+	gpa_t canonical_ipa_end, nested_ipa_end;
 	u64 entry, new_entry = 0;
 
 	lockdep_assert_held_read(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
@@ -925,7 +957,9 @@ void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
 		canonical_ipa = ALIGN_DOWN(canonical_ipa, map_size);
 
 	canonical_ipa_end = canonical_ipa + map_size - 1;
+	nested_ipa_end = nested_ipa + map_size - 1;
 	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
+	MA_STATE(mas_dmap, direct_mt, nested_ipa, nested_ipa_end);
 
 	mtree_lock(revmap_mt);
 	entry = xa_to_value(mas_find(&mas_rmap, canonical_ipa_end));
@@ -960,6 +994,15 @@ void kvm_record_nested_revmap(gpa_t canonical_ipa, struct kvm_s2_mmu *mmu,
 					       nested_ipa, new_entry);
 		mmu->nested_revmap_broken = true;
 	}
+
+	/*
+	 * Add direct map but ignore the result, missing a direct map does not
+	 * affect correctness.
+	 */
+	if (valid_entry(new_entry) && !mmu->nested_revmap_broken)
+		mas_store_gfp(&mas_dmap, xa_mk_value(canonical_ipa | VALID_ENTRY),
+			      GFP_NOWAIT | __GFP_ACCOUNT);
+
 unlock:
 	mtree_unlock(revmap_mt);
 }
@@ -971,6 +1014,8 @@ void kvm_init_nested_s2_mmu(struct kvm_s2_mmu *mmu)
 	mmu->nested_stage2_enabled = false;
 	atomic_set(&mmu->refcnt, 0);
 	mt_init(&mmu->nested_revmap_mt);
+	mt_init_flags(&mmu->nested_direct_mt, MT_FLAGS_LOCK_EXTERN);
+	mt_set_external_lock(&mmu->nested_direct_mt, &mmu->nested_revmap_mt.ma_lock);
 	mmu->nested_revmap_broken = false;
 }
 
@@ -1349,7 +1394,10 @@ void kvm_nested_s2_wp(struct kvm *kvm)
 
 static void reset_revmap_and_unmap(struct kvm_s2_mmu *mmu, bool may_block)
 {
-	mtree_destroy(&mmu->nested_revmap_mt);
+	mtree_lock(&mmu->nested_revmap_mt);
+	__mt_destroy(&mmu->nested_revmap_mt);
+	__mt_destroy(&mmu->nested_direct_mt);
+	mtree_unlock(&mmu->nested_revmap_mt);
 	mmu->nested_revmap_broken = false;
 	kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
 }
@@ -1357,12 +1405,15 @@ static void reset_revmap_and_unmap(struct kvm_s2_mmu *mmu, bool may_block)
 static void unmap_mmu_cipa_range(struct kvm_s2_mmu *mmu, gpa_t canonical_ipa,
 				  size_t unmap_size, bool may_block)
 {
+	struct maple_tree *direct_mt = &mmu->nested_direct_mt;
 	struct maple_tree *revmap_mt = &mmu->nested_revmap_mt;
 	gpa_t canonical_ipa_end = canonical_ipa + unmap_size - 1;
+	gpa_t nested_ipa, nested_ipa_end;
 	size_t entry_size;
 	gpa_t next_addr;
 	u64 entry;
 	MA_STATE(mas_rmap, revmap_mt, canonical_ipa, canonical_ipa_end);
+	MA_STATE(mas_dmap, direct_mt, 0, ULONG_MAX);
 
 	lockdep_assert_held_write(&kvm_s2_mmu_to_kvm(mmu)->mmu_lock);
 
@@ -1398,6 +1449,16 @@ static void unmap_mmu_cipa_range(struct kvm_s2_mmu *mmu, gpa_t canonical_ipa,
 		 */
 		mas_store_gfp(&mas_rmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
 
+		/*
+		 * Try to also remove the direct map, it is okay if this fails,
+		 * as we check for direct map consistency in
+		 * kvm_remove_nested_revmap().
+		 */
+		nested_ipa = entry & ADDR_MASK;
+		nested_ipa_end = nested_ipa + entry_size - 1;
+		mas_set_range(&mas_dmap, nested_ipa, nested_ipa_end);
+		mas_store_gfp(&mas_dmap, NULL, GFP_NOWAIT | __GFP_ACCOUNT);
+
 		mtree_unlock(revmap_mt);
 		kvm_stage2_unmap_range(mmu, entry & ADDR_MASK, entry_size,
 				       may_block);
-- 
2.43.0

Re: [PATCH v4 0/6] KVM: arm64: nv: Implement nested stage-2 reverse map

[Itaru Kitayama]

On Tue, Jul 14, 2026 at 12:59:19PM +0100, Wei-Lin Chang wrote:
> Hi,
> 
> This is v4 of optimizing the shadow s2 mmu unmapping during MMU
> notifiers. The general theme stays the same, but various
> improvements/fixes are added. Thanks for all the feedback. Please see
> below for the changes.
> 
> * Changes from v3 [1]:
> 
>   - Rebased on to the latest kvmarm/fixes.
> 
>   - Removed commit "KVM: arm64: Move shadow_pt_debugfs_dentry to reduce
>     holes in kvm_s2_mmu" because the current shadow ptdump code is found
>     to be buggy and the proposed fix removes shadow_pt_debugfs_dentry [2].
> 
>   - Rename some variables to improve readability e.g. ipa ->
>     canonical_ipa, fault_ipa -> nested_ipa.
> 
>   - Skip reverse map optimization when !mmu->nested_stage2_enabled,
>     and just unmap the canonical IPA range during MMU notifiers.
> 
>   - Change comment above the code that checks whether an identical
>     reverse map already exists in kvm_record_nested_revmap() from saying
>     it could be a permission update to saying another parallel stage-2
>     fault could have inserted an identical reverse map. This is because
>     permission update won't trigger kvm_record_nested_revmap().
> 
>   - Add helpers for checking maple tree entry flags VALID_ENTRY and
>     UNKNOWN_IPA, also warn if both are set at the same time.
> 
>   - Test canonical IPA alignment with the mapping size during reverse
>     map insertion.
> 
>   - Move clean up of kvm_unmap_gfn_range() to its own patch.
> 
>   - Avoid naming variables with "*dir*", as it can be misunderstood to
>     be related to directories.
> 
>   - Use mas_find() instead of mas_find_range() as mas_find_range()
>     returns every range including empty/NULL ranges, for our use case
>     we only want to iterate over non-NULL ranges.
> 
>   - Added a trace point to track when the reverse map maple tree becomes
>     "broken" i.e. maple tree insertion failure.
> 
> Thanks!
> 
> [1]: https://lore.kernel.org/kvmarm/20260510145338.322962-1-weilin.chang@arm.com/
> [2]: https://lore.kernel.org/kvmarm/20260630121005.1130996-1-weilin.chang@arm.com/

Thanks for the update. I know this series is sitll being discussed, but I
went ahead testing the kernel with your proposed hello_nested seltest on a Grace
CPU. 

Before this series applied:
real    0m2.869s
user    0m0.003s
sys     0m2.862s

After this series applied:
real    0m0.027s
user    0m0.001s
sys     0m0.005s

It reduces the time needed for the VM tear-down paths. I will test this series again 
when v5 comes out, since I've git this a try;

Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com>

Thanks,
Itaru.

> 
> Wei-Lin Chang (6):
>   KVM: arm64: Use a variable for the canonical GPA in kvm_s2_fault_map()
>   KVM: arm64: nv: Avoid full shadow s2 unmap
>   KVM: arm64: nv: Add nested revmap broken tracepoint
>   KVM: arm64: Refactor kvm_unmap_gfn_range() with common variables
>   KVM: arm64: nv: Remove reverse map entries during TLBI handling
>   KVM: arm64: nv: Create nested IPA direct map to speed up reverse map
>     removal
> 
>  arch/arm64/include/asm/kvm_host.h   |   9 +
>  arch/arm64/include/asm/kvm_nested.h |   6 +
>  arch/arm64/kvm/mmu.c                |  42 +++--
>  arch/arm64/kvm/nested.c             | 280 +++++++++++++++++++++++++++-
>  arch/arm64/kvm/sys_regs.c           |   3 +
>  arch/arm64/kvm/trace_arm.h          |  27 +++
>  6 files changed, 349 insertions(+), 18 deletions(-)
> 
> -- 
> 2.43.0
>