Messages in this thread | | | Subject | Re: [PATCH 09/11] KVM: MMU: simplify mmu_need_write_protect | From | Kai Huang <> | Date | Tue, 15 Dec 2015 16:43:13 +0800 |
| |
On 12/01/2015 02:26 AM, Xiao Guangrong wrote: > Now, all non-leaf shadow page are page tracked, if gfn is not tracked > there is no non-leaf shadow page of gfn is existed, we can directly > make the shadow page of gfn to unsync > > Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > --- > arch/x86/kvm/mmu.c | 26 ++++++++------------------ > 1 file changed, 8 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 5a2ca73..f89e77f 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -2461,41 +2461,31 @@ static void __kvm_unsync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp) > kvm_mmu_mark_parents_unsync(sp); > } > > -static void kvm_unsync_pages(struct kvm_vcpu *vcpu, gfn_t gfn) > +static bool kvm_unsync_pages(struct kvm_vcpu *vcpu, gfn_t gfn, > + bool can_unsync) > { > struct kvm_mmu_page *s; > > for_each_gfn_indirect_valid_sp(vcpu->kvm, s, gfn) { > + if (!can_unsync) > + return true; How about moving this right before for_each_gfn_indirect_valid_sp? As can_unsync is passed as parameter, so there's no point checking it several times.
A further thinking is can we move it to mmu_need_write_protect? Passing can_unsync as parameter to kvm_unsync_pages sounds a little bit odd.
> + > if (s->unsync) > continue; > WARN_ON(s->role.level != PT_PAGE_TABLE_LEVEL); How about large page mapping? Such as if guest uses 2M mapping and its shadow is indirect, does above WARN_ON still meet? As you removed the PT level check in mmu_need_write_protect.
Thanks, -Kai > __kvm_unsync_page(vcpu, s); > } > + > + return false; > }
> > static bool mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn, > bool can_unsync) > { > - struct kvm_mmu_page *s; > - bool need_unsync = false; > - > if (kvm_page_track_check_mode(vcpu, gfn, KVM_PAGE_TRACK_WRITE)) > return true; > > - for_each_gfn_indirect_valid_sp(vcpu->kvm, s, gfn) { > - if (!can_unsync) > - return true; > - > - if (s->role.level != PT_PAGE_TABLE_LEVEL) > - return true; > - > - if (!s->unsync) > - need_unsync = true; > - } > - if (need_unsync) > - kvm_unsync_pages(vcpu, gfn); > - > - return false; > + return kvm_unsync_pages(vcpu, gfn, can_unsync); > } > > static bool kvm_is_mmio_pfn(pfn_t pfn)
| |