On Fri, Mar 11, 2022 at 12:25:08AM +0000, David Matlack wrote: > Passing the memslot to kvm_mmu_new_shadow_page() avoids the need for the > vCPU pointer when write-protecting indirect 4k shadow pages. This moves > us closer to being able to create new shadow pages during VM ioctls for > eager page splitting, where there is not vCPU pointer. > > This change does not negatively impact "Populate memory time" for ept=Y > or ept=N configurations since kvm_vcpu_gfn_to_memslot() caches the last > use slot. So even though we now look up the slot more often, it is a > very cheap check. > > Opportunistically move the code to write-protect GFNs shadowed by > PG_LEVEL_4K shadow pages into account_shadowed() to reduce indentation > and consolidate the code. This also eliminates a memslot lookup. > > No functional change intended. > > Signed-off-by: David Matlack <[email protected]> > --- > arch/x86/kvm/mmu/mmu.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index b6fb50e32291..519910938478 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -793,16 +793,14 @@ void kvm_mmu_gfn_allow_lpage(const struct > kvm_memory_slot *slot, gfn_t gfn) > update_gfn_disallow_lpage_count(slot, gfn, -1); > } > > -static void account_shadowed(struct kvm *kvm, struct kvm_mmu_page *sp) > +static void account_shadowed(struct kvm *kvm, > + struct kvm_memory_slot *slot, > + struct kvm_mmu_page *sp) > { > - struct kvm_memslots *slots; > - struct kvm_memory_slot *slot; > gfn_t gfn; > > kvm->arch.indirect_shadow_pages++; > gfn = sp->gfn; > - slots = kvm_memslots_for_spte_role(kvm, sp->role); > - slot = __gfn_to_memslot(slots, gfn); > > /* the non-leaf shadow pages are keeping readonly. */ > if (sp->role.level > PG_LEVEL_4K) > @@ -810,6 +808,9 @@ static void account_shadowed(struct kvm *kvm, struct > kvm_mmu_page *sp) > KVM_PAGE_TRACK_WRITE); > > kvm_mmu_gfn_disallow_lpage(slot, gfn); > + > + if (kvm_mmu_slot_gfn_write_protect(kvm, slot, gfn, PG_LEVEL_4K)) > + kvm_flush_remote_tlbs_with_address(kvm, gfn, 1);
It's not immediately obvious in this diff, but when looking at the code yeah it looks right to just drop the 4K check.. I also never understood why we only write-track the >1 levels but only wr-protect the last level. It'll be great if there's quick answer from anyone.. even though it's probably unrelated to the patch. The change looks all correct: Reviewed-by: Peter Xu <[email protected]> Thanks, -- Peter Xu _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
