Skip to content

Commit a955cad

Browse files
sean-jcbonzini
authored andcommitted
KVM: x86/mmu: Retry page fault if root is invalidated by memslot update
Bail from the page fault handler if the root shadow page was obsoleted by a memslot update. Do the check _after_ acuiring mmu_lock, as the TDP MMU doesn't rely on the memslot/MMU generation, and instead relies on the root being explicit marked invalid by kvm_mmu_zap_all_fast(), which takes mmu_lock for write. For the TDP MMU, inserting a SPTE into an obsolete root can leak a SP if kvm_tdp_mmu_zap_invalidated_roots() has already zapped the SP, i.e. has moved past the gfn associated with the SP. For other MMUs, the resulting behavior is far more convoluted, though unlikely to be truly problematic. Installing SPs/SPTEs into the obsolete root isn't directly problematic, as the obsolete root will be unloaded and dropped before the vCPU re-enters the guest. But because the legacy MMU tracks shadow pages by their role, any SP created by the fault can can be reused in the new post-reload root. Again, that _shouldn't_ be problematic as any leaf child SPTEs will be created for the current/valid memslot generation, and kvm_mmu_get_page() will not reuse child SPs from the old generation as they will be flagged as obsolete. But, given that continuing with the fault is pointess (the root will be unloaded), apply the check to all MMUs. Fixes: b7cccd3 ("KVM: x86/mmu: Fast invalidation for TDP MMU") Cc: [email protected] Cc: Ben Gardon <[email protected]> Signed-off-by: Sean Christopherson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent bfbb307 commit a955cad

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

arch/x86/kvm/mmu/mmu.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,7 +1936,11 @@ static void mmu_audit_disable(void) { }
19361936

19371937
static bool is_obsolete_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
19381938
{
1939-
return sp->role.invalid ||
1939+
if (sp->role.invalid)
1940+
return true;
1941+
1942+
/* TDP MMU pages due not use the MMU generation. */
1943+
return !sp->tdp_mmu_page &&
19401944
unlikely(sp->mmu_valid_gen != kvm->arch.mmu_valid_gen);
19411945
}
19421946

@@ -3976,6 +3980,20 @@ static bool kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
39763980
return true;
39773981
}
39783982

3983+
/*
3984+
* Returns true if the page fault is stale and needs to be retried, i.e. if the
3985+
* root was invalidated by a memslot update or a relevant mmu_notifier fired.
3986+
*/
3987+
static bool is_page_fault_stale(struct kvm_vcpu *vcpu,
3988+
struct kvm_page_fault *fault, int mmu_seq)
3989+
{
3990+
if (is_obsolete_sp(vcpu->kvm, to_shadow_page(vcpu->arch.mmu->root_hpa)))
3991+
return true;
3992+
3993+
return fault->slot &&
3994+
mmu_notifier_retry_hva(vcpu->kvm, mmu_seq, fault->hva);
3995+
}
3996+
39793997
static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
39803998
{
39813999
bool is_tdp_mmu_fault = is_tdp_mmu(vcpu->arch.mmu);
@@ -4013,8 +4031,9 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
40134031
else
40144032
write_lock(&vcpu->kvm->mmu_lock);
40154033

4016-
if (fault->slot && mmu_notifier_retry_hva(vcpu->kvm, mmu_seq, fault->hva))
4034+
if (is_page_fault_stale(vcpu, fault, mmu_seq))
40174035
goto out_unlock;
4036+
40184037
r = make_mmu_pages_available(vcpu);
40194038
if (r)
40204039
goto out_unlock;

arch/x86/kvm/mmu/paging_tmpl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,8 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
911911

912912
r = RET_PF_RETRY;
913913
write_lock(&vcpu->kvm->mmu_lock);
914-
if (fault->slot && mmu_notifier_retry_hva(vcpu->kvm, mmu_seq, fault->hva))
914+
915+
if (is_page_fault_stale(vcpu, fault, mmu_seq))
915916
goto out_unlock;
916917

917918
kvm_mmu_audit(vcpu, AUDIT_PRE_PAGE_FAULT);

0 commit comments

Comments
 (0)