[PATCH 0/2] KVM: x86/mmu: Clean up hugepage split error handling

[PATCH 0/2] KVM: x86/mmu: Clean up hugepage split error handling

[Sean Christopherson]

"Fix" an impossible scenario where KVM would install a '0' SPTE instead of
using SHADOW_NONPRESENT_VALUE.  In quotes because (a) there's not truly anything
to fix (the code should never be hit), and (b) bugging the VM doesn't guarantee
KVM won't get confused (though it's still better than installing an empty SPTE).

The main motivation for this is to eliminate installing a '0' SPTE so that
future audits of the MMU don't complain about not using SHADOW_NONPRESENT_VALUE.

Sean Christopherson (2):
  KVM: x86/mmu: Bug the VM if KVM tries to split a !hugepage SPTE
  KVM: x86/mmu: Clean up make_huge_page_split_spte() definition and
    intro

 arch/x86/kvm/mmu/spte.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)


base-commit: 771df9ffadb8204e61d3e98f36c5067102aab78f
-- 
2.45.2.993.g49e7a77208-goog

[PATCH 1/2] KVM: x86/mmu: Bug the VM if KVM tries to split a !hugepage SPTE

[Sean Christopherson]

Bug the VM instead of simply warning if KVM tries to split a SPTE that is
non-present or not-huge.  KVM is guaranteed to end up in a broken state as
the callers fully expect a valid SPTE, e.g. the shadow MMU will add an
rmap entry, and all MMUs will account the expected small page.  Returning
'0' is also technically wrong now that SHADOW_NONPRESENT_VALUE exists,
i.e. would cause KVM to create a potential #VE SPTE.

While it would be possible to have the callers gracefully handle failure,
doing so would provide no practical value as the scenario really should be
impossible, while the error handling would add a non-trivial amount of
noise.

Fixes: a3fe5dbda0a4 ("KVM: x86/mmu: Split huge pages mapped by the TDP MMU when dirty logging is enabled")
Cc: David Matlack <dmatlack@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/spte.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
index c8fe13217ff7..bc55e3b26045 100644
--- a/arch/x86/kvm/mmu/spte.c
+++ b/arch/x86/kvm/mmu/spte.c
@@ -296,11 +296,7 @@ u64 make_huge_page_split_spte(struct kvm *kvm, u64 huge_spte, union kvm_mmu_page
 {
 	u64 child_spte;
 
-	if (WARN_ON_ONCE(!is_shadow_present_pte(huge_spte)))
-		return 0;
-
-	if (WARN_ON_ONCE(!is_large_pte(huge_spte)))
-		return 0;
+	KVM_BUG_ON(!is_shadow_present_pte(huge_spte) || !is_large_pte(huge_spte), kvm);
 
 	child_spte = huge_spte;
 
-- 
2.45.2.993.g49e7a77208-goog

[PATCH 2/2] KVM: x86/mmu: Clean up make_huge_page_split_spte() definition and intro

[Sean Christopherson]

Tweak the definition of make_huge_page_split_spte() to eliminate an
unnecessarily long line, and opportunistically initialize child_spte to
make it more obvious that the child is directly derived from the huge
parent.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/spte.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
index bc55e3b26045..10390eecd85d 100644
--- a/arch/x86/kvm/mmu/spte.c
+++ b/arch/x86/kvm/mmu/spte.c
@@ -291,15 +291,13 @@ static u64 make_spte_executable(u64 spte)
  * This is used during huge page splitting to build the SPTEs that make up the
  * new page table.
  */
-u64 make_huge_page_split_spte(struct kvm *kvm, u64 huge_spte, union kvm_mmu_page_role role,
-			      int index)
+u64 make_huge_page_split_spte(struct kvm *kvm, u64 huge_spte,
+			      union kvm_mmu_page_role role, int index)
 {
-	u64 child_spte;
+	u64 child_spte = huge_spte;
 
 	KVM_BUG_ON(!is_shadow_present_pte(huge_spte) || !is_large_pte(huge_spte), kvm);
 
-	child_spte = huge_spte;
-
 	/*
 	 * The child_spte already has the base address of the huge page being
 	 * split. So we just have to OR in the offset to the page at the next
-- 
2.45.2.993.g49e7a77208-goog

Re: [PATCH 0/2] KVM: x86/mmu: Clean up hugepage split error handling

[Paolo Bonzini]

Queued, thanks.

Paolo