Skip to content

Commit

Permalink
KVM: x86/mmu: fix KVM_X86_QUIRK_SLOT_ZAP_ALL for shadow MMU
Browse files Browse the repository at this point in the history
As was tried in commit 4e10313 ("KVM: x86/mmu: Zap only the relevant
pages when removing a memslot"), all shadow pages, i.e. non-leaf SPTEs,
need to be zapped.  All of the accounting for a shadow page is tied to the
memslot, i.e. the shadow page holds a reference to the memslot, for all
intents and purposes.  Deleting the memslot without removing all relevant
shadow pages, as is done when KVM_X86_QUIRK_SLOT_ZAP_ALL is disabled,
results in NULL pointer derefs when tearing down the VM.

Reintroduce from that commit the code that walks the whole memslot when
there are active shadow MMU pages.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
Paolo Bonzini committed Oct 3, 2024
1 parent 76f972c commit fcd1ec9
Showing 1 changed file with 46 additions and 14 deletions.
60 changes: 46 additions & 14 deletions arch/x86/kvm/mmu/mmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -1884,10 +1884,14 @@ static bool sp_has_gptes(struct kvm_mmu_page *sp)
if (is_obsolete_sp((_kvm), (_sp))) { \
} else

#define for_each_gfn_valid_sp_with_gptes(_kvm, _sp, _gfn) \
#define for_each_gfn_valid_sp(_kvm, _sp, _gfn) \
for_each_valid_sp(_kvm, _sp, \
&(_kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(_gfn)]) \
if ((_sp)->gfn != (_gfn) || !sp_has_gptes(_sp)) {} else
if ((_sp)->gfn != (_gfn)) {} else

#define for_each_gfn_valid_sp_with_gptes(_kvm, _sp, _gfn) \
for_each_gfn_valid_sp(_kvm, _sp, _gfn) \
if (!sp_has_gptes(_sp)) {} else

static bool kvm_sync_page_check(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
{
Expand Down Expand Up @@ -7049,26 +7053,54 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
kvm_mmu_zap_all(kvm);
}

/*
* Zapping leaf SPTEs with memslot range when a memslot is moved/deleted.
*
* Zapping non-leaf SPTEs, a.k.a. not-last SPTEs, isn't required, worst
* case scenario we'll have unused shadow pages lying around until they
* are recycled due to age or when the VM is destroyed.
*/
static void kvm_mmu_zap_memslot_leafs(struct kvm *kvm, struct kvm_memory_slot *slot)
static void kvm_mmu_zap_memslot_pages_and_flush(struct kvm *kvm,
struct kvm_memory_slot *slot,
bool flush)
{
LIST_HEAD(invalid_list);
unsigned long i;

if (list_empty(&kvm->arch.active_mmu_pages))
goto out_flush;

/*
* Since accounting information is stored in struct kvm_arch_memory_slot,
* shadow pages deletion (e.g. unaccount_shadowed()) requires that all
* gfns with a shadow page have a corresponding memslot. Do so before
* the memslot goes away.
*/
for (i = 0; i < slot->npages; i++) {
struct kvm_mmu_page *sp;
gfn_t gfn = slot->base_gfn + i;

for_each_gfn_valid_sp(kvm, sp, gfn)
kvm_mmu_prepare_zap_page(kvm, sp, &invalid_list);

if (need_resched() || rwlock_needbreak(&kvm->mmu_lock)) {
kvm_mmu_remote_flush_or_zap(kvm, &invalid_list, flush);
flush = false;
cond_resched_rwlock_write(&kvm->mmu_lock);
}
}

out_flush:
kvm_mmu_remote_flush_or_zap(kvm, &invalid_list, flush);
}

static void kvm_mmu_zap_memslot(struct kvm *kvm,
struct kvm_memory_slot *slot)
{
struct kvm_gfn_range range = {
.slot = slot,
.start = slot->base_gfn,
.end = slot->base_gfn + slot->npages,
.may_block = true,
};
bool flush;

write_lock(&kvm->mmu_lock);
if (kvm_unmap_gfn_range(kvm, &range))
kvm_flush_remote_tlbs_memslot(kvm, slot);

flush = kvm_unmap_gfn_range(kvm, &range);
kvm_mmu_zap_memslot_pages_and_flush(kvm, slot, flush);
write_unlock(&kvm->mmu_lock);
}

Expand All @@ -7084,7 +7116,7 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
if (kvm_memslot_flush_zap_all(kvm))
kvm_mmu_zap_all_fast(kvm);
else
kvm_mmu_zap_memslot_leafs(kvm, slot);
kvm_mmu_zap_memslot(kvm, slot);
}

void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, u64 gen)
Expand Down

0 comments on commit fcd1ec9

Please sign in to comment.