[PATCH v2 0/5] Small changes related to prefetch and spurious faults

[PATCH v2 0/5] Small changes related to prefetch and spurious faults

[Yan Zhao]

Hi

This is v2 of the series for some small changes related to
prefetch/prefault and spurious faults.

Patch 1: Checks if a shadow-present old SPTE is leaf to determine a
         prefetch fault is spurious.

Patch 2: Merges the checks for prefetch and is_access_allowed() for
         spurious faults into a common path.

Patch 3: Adds a warning when the PFN changes on a spurious fault in the TDP
         MMU

Patch 4: Adds a warning when the PFN changes on a shadow-present SPTE in
         the shadow MMU. This implementation differs from the v1
         discussion. Upon reconsideration, I realized that WARN_ON_ONCE()
         in mmu_spte_update() cannot warn when the PFN changes on a
         shadow-present SPTE. Add the warning in mmu_set_spte() and have 
         the prefetch fault to leverage the warning.

Patch 5: Checks req and frees obsolete roots in each MMU reload.

With below scenario
1. add a memslot with size 4K
2. prefault GPA A in the memslot
3. delete the memslot
4. re-add the memslot with size 2M
5. prefault GPA A again.

Patch 1 is required if zap all quirk is disabled in step 3.
Patch 5 is required if zap all is performed in step 3 and if step 2/5 are
        executed before any vcpu_run().

Change log:
v2:
- Check both fault->prefetch and is_access_allowed() in patch 2. (Sean)
- Split patch 3 in v1 into patches 3 and 4.
- Only warn on PFN changes in case of spurious fault in TDP MMU in patch 3.
  (Sean).
- Add patch 4 to warn on PFN changes on shadow-present SPTE in shadow MMU.
- Move kvm_mmu_free_obsolete_roots() from kvm_arch_vcpu_pre_fault_memory()
  to kvm_mmu_reload() in patch 5. (Sean)

Thanks
Yan

v1: https://lore.kernel.org/all/20250207030640.1585-1-yan.y.zhao@intel.com

Yan Zhao (5):
  KVM: x86/mmu: Further check old SPTE is leaf for spurious prefetch
    fault
  KVM: x86/tdp_mmu: Merge prefetch and access checks for spurious faults
  KVM: x86/tdp_mmu: WARN if PFN changes for spurious faults
  KVM: x86/mmu: Warn if PFN changes on shadow-present SPTE in shadow MMU
  KVM: x86/mmu: Check and free obsolete roots in kvm_mmu_reload()

 arch/x86/kvm/mmu.h         | 3 +++
 arch/x86/kvm/mmu/mmu.c     | 6 ++++--
 arch/x86/kvm/mmu/tdp_mmu.c | 9 ++++-----
 3 files changed, 11 insertions(+), 7 deletions(-)

base-commit: c9ea48bb6ee6b28bbc956c1e8af98044618fed5e
-- 
2.43.2

[PATCH v2 1/5] KVM: x86/mmu: Further check old SPTE is leaf for spurious prefetch fault

[Yan Zhao]

Instead of simply treating a prefetch fault as spurious when there's a
shadow-present old SPTE, further check if the old SPTE is leaf to determine
if a prefetch fault is spurious.

It's not reasonable to treat a prefetch fault as spurious when there's a
shadow-present non-leaf SPTE without a corresponding shadow-present leaf
SPTE. e.g., in the following sequence, a prefetch fault should not be
considered spurious:
1. add a memslot with size 4K
2. prefault GPA A in the memslot
3. delete the memslot (zap all disabled)
4. re-add the memslot with size 2M
5. prefault GPA A again.
In step 5, the prefetch fault attempts to install a 2M huge entry.
Since step 3 zaps the leaf SPTE for GPA A while keeping the non-leaf SPTE,
the leaf entry will remain empty after step 5 if the fetch fault is
regarded as spurious due to a shadow-present non-leaf SPTE.

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/kvm/mmu/mmu.c     | 2 +-
 arch/x86/kvm/mmu/tdp_mmu.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 8160870398b9..94c677f8cc05 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2846,7 +2846,7 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, struct kvm_memory_slot *slot,
 	}
 
 	if (is_shadow_present_pte(*sptep)) {
-		if (prefetch)
+		if (prefetch && is_last_spte(*sptep, level))
 			return RET_PF_SPURIOUS;
 
 		/*
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 046b6ba31197..ab65fd915ef2 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1137,7 +1137,8 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
 	if (WARN_ON_ONCE(sp->role.level != fault->goal_level))
 		return RET_PF_RETRY;
 
-	if (fault->prefetch && is_shadow_present_pte(iter->old_spte))
+	if (fault->prefetch && is_shadow_present_pte(iter->old_spte) &&
+	    is_last_spte(iter->old_spte, iter->level))
 		return RET_PF_SPURIOUS;
 
 	if (is_shadow_present_pte(iter->old_spte) &&
-- 
2.43.2

[PATCH v2 2/5] KVM: x86/tdp_mmu: Merge prefetch and access checks for spurious faults

[Yan Zhao]

Combine prefetch and is_access_allowed() checks into a unified path to
detect spurious faults, since both cases now share identical logic.

No functional changes.

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index ab65fd915ef2..6365eb6c1390 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1137,12 +1137,8 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
 	if (WARN_ON_ONCE(sp->role.level != fault->goal_level))
 		return RET_PF_RETRY;
 
-	if (fault->prefetch && is_shadow_present_pte(iter->old_spte) &&
-	    is_last_spte(iter->old_spte, iter->level))
-		return RET_PF_SPURIOUS;
-
 	if (is_shadow_present_pte(iter->old_spte) &&
-	    is_access_allowed(fault, iter->old_spte) &&
+	    (fault->prefetch || is_access_allowed(fault, iter->old_spte)) &&
 	    is_last_spte(iter->old_spte, iter->level))
 		return RET_PF_SPURIOUS;
 
-- 
2.43.2

[PATCH v2 3/5] KVM: x86/tdp_mmu: WARN if PFN changes for spurious faults

[Yan Zhao]

Add a WARN() to assert that KVM does _not_ change the PFN of a
shadow-present SPTE during spurious fault handling.

KVM should _never_ change the PFN of a shadow-present SPTE and TDP MMU
already BUG()s on this. However, spurious faults just return early before
the existing BUG() could be hit.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 6365eb6c1390..d219ecd4ac5b 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1139,8 +1139,10 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
 
 	if (is_shadow_present_pte(iter->old_spte) &&
 	    (fault->prefetch || is_access_allowed(fault, iter->old_spte)) &&
-	    is_last_spte(iter->old_spte, iter->level))
+	    is_last_spte(iter->old_spte, iter->level)) {
+		WARN_ON_ONCE(fault->pfn != spte_to_pfn(iter->old_spte));
 		return RET_PF_SPURIOUS;
+	}
 
 	if (unlikely(!fault->slot))
 		new_spte = make_mmio_spte(vcpu, iter->gfn, ACC_ALL);
-- 
2.43.2

[PATCH v2 4/5] KVM: x86/mmu: Warn if PFN changes on shadow-present SPTE in shadow MMU

[Yan Zhao]

Warn if PFN changes on shadow-present SPTE in mmu_set_spte().

KVM should _never_ change the PFN of a shadow-present SPTE. In
mmu_set_spte(), there is a WARN_ON_ONCE() on pfn changes on shadow-present
SPTE in mmu_spte_update() to detect this condition. However, that
WARN_ON_ONCE() is not hittable since mmu_set_spte() invokes drop_spte()
earlier before mmu_spte_update(), which clears SPTE to a !shadow-present
state. So, before invoking drop_spte(), add a WARN_ON_ONCE() in
mmu_set_spte() to warn PFN change of a shadow-present SPTE.

For the spurious prefetch fault, only return RET_PF_SPURIOUS directly when
PFN is not changed. When PFN changes, fall through to follow the sequence
of drop_spte(), warn of PFN change, make_spte(), flush tlb, rmap_add().

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/kvm/mmu/mmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 94c677f8cc05..607cbb19ea96 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2846,7 +2846,8 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, struct kvm_memory_slot *slot,
 	}
 
 	if (is_shadow_present_pte(*sptep)) {
-		if (prefetch && is_last_spte(*sptep, level))
+		if (prefetch && is_last_spte(*sptep, level) &&
+		    pfn == spte_to_pfn(*sptep))
 			return RET_PF_SPURIOUS;
 
 		/*
@@ -2860,7 +2861,7 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, struct kvm_memory_slot *slot,
 			child = spte_to_child_sp(pte);
 			drop_parent_pte(vcpu->kvm, child, sptep);
 			flush = true;
-		} else if (pfn != spte_to_pfn(*sptep)) {
+		} else if (WARN_ON_ONCE(pfn != spte_to_pfn(*sptep))) {
 			drop_spte(vcpu->kvm, sptep);
 			flush = true;
 		} else
-- 
2.43.2

[PATCH v2 5/5] KVM: x86/mmu: Check and free obsolete roots in kvm_mmu_reload()

[Yan Zhao]

Check request KVM_REQ_MMU_FREE_OBSOLETE_ROOTS to free obsolete roots in
kvm_mmu_reload() to prevent kvm_mmu_reload() from seeing a stale obsolete
root.

Since kvm_mmu_reload() can be called outside the
vcpu_enter_guest() path (e.g., kvm_arch_vcpu_pre_fault_memory()), it may be
invoked after a root has been marked obsolete and before vcpu_enter_guest()
is invoked to process KVM_REQ_MMU_FREE_OBSOLETE_ROOTS and set root.hpa to
invalid. This causes kvm_mmu_reload() to fail to load a new root, which
can lead to kvm_arch_vcpu_pre_fault_memory() being stuck in the while
loop in kvm_tdp_map_page() since RET_PF_RETRY is always returned due to
is_page_fault_stale().

Keep the existing check of KVM_REQ_MMU_FREE_OBSOLETE_ROOTS in
vcpu_enter_guest() since the cost of kvm_check_request() is negligible,
especially a check that's guarded by kvm_request_pending().

Export symbol of kvm_mmu_free_obsolete_roots() as kvm_mmu_reload() is
inline and may be called outside of kvm.ko.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/kvm/mmu.h     | 3 +++
 arch/x86/kvm/mmu/mmu.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 050a0e229a4d..f2b36d32ef40 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -104,6 +104,9 @@ void kvm_mmu_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
 
 static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
 {
+	if (kvm_check_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu))
+		kvm_mmu_free_obsolete_roots(vcpu);
+
 	/*
 	 * Checking root.hpa is sufficient even when KVM has mirror root.
 	 * We can have either:
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 607cbb19ea96..15fd4838e4f2 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5802,6 +5802,7 @@ void kvm_mmu_free_obsolete_roots(struct kvm_vcpu *vcpu)
 	__kvm_mmu_free_obsolete_roots(vcpu->kvm, &vcpu->arch.root_mmu);
 	__kvm_mmu_free_obsolete_roots(vcpu->kvm, &vcpu->arch.guest_mmu);
 }
+EXPORT_SYMBOL_GPL(kvm_mmu_free_obsolete_roots);
 
 static u64 mmu_pte_write_fetch_gpte(struct kvm_vcpu *vcpu, gpa_t *gpa,
 				    int *bytes)
-- 
2.43.2

Re: [PATCH v2 0/5] Small changes related to prefetch and spurious faults

[Sean Christopherson]

On Tue, 18 Mar 2025 09:30:37 +0800, Yan Zhao wrote:
> This is v2 of the series for some small changes related to
> prefetch/prefault and spurious faults.
> 
> Patch 1: Checks if a shadow-present old SPTE is leaf to determine a
>          prefetch fault is spurious.
> 
> Patch 2: Merges the checks for prefetch and is_access_allowed() for
>          spurious faults into a common path.
> 
> [...]

Applied 1-4 to kvm-x86 mmu, and patch 5 to fixes.  Thanks!

[1/5] KVM: x86/mmu: Further check old SPTE is leaf for spurious prefetch fault
      https://github.com/kvm-x86/linux/commit/ea9fcdf76d3d
[2/5] KVM: x86/tdp_mmu: Merge prefetch and access checks for spurious faults
      https://github.com/kvm-x86/linux/commit/d17cc13cc484
[3/5] KVM: x86/tdp_mmu: WARN if PFN changes for spurious faults
      https://github.com/kvm-x86/linux/commit/988da7820206
[4/5] KVM: x86/mmu: Warn if PFN changes on shadow-present SPTE in shadow MMU
      https://github.com/kvm-x86/linux/commit/11d45175111d
[5/5] KVM: x86/mmu: Check and free obsolete roots in kvm_mmu_reload()
      https://github.com/kvm-x86/linux/commit/20a6cff3b283
--
https://github.com/kvm-x86/linux/tree/next