On Thu, May 17, 2012 at 01:24:41PM +0300, Avi Kivity wrote: > This allows us to later move the actual flush out of protection of the > mmu spinlock, provided there are no additional dependencies. > > Constructs of the form > > if (pred) > kvm_flush_remote_tlbs(kvm) > > are converted to > > if (pred) > kvm_mark_tlb_dirty(kvm) > > kvm_cond_flush_remote_tlbs(kvm) > > so that if another thread caused pred to transition from true to false, > but has not yet flushed the tlb, we notice it and flush it before proceeding. > > Signed-off-by: Avi Kivity <avi@xxxxxxxxxx> > --- > arch/ia64/kvm/kvm-ia64.c | 6 ++++-- > arch/x86/kvm/mmu.c | 45 +++++++++++++++++++++++++++----------------- > arch/x86/kvm/paging_tmpl.h | 4 +++- > arch/x86/kvm/x86.c | 4 +++- > virt/kvm/kvm_main.c | 4 +++- > 5 files changed, 41 insertions(+), 22 deletions(-) > > diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c > index bd77cb5..a4a92d8 100644 > --- a/arch/ia64/kvm/kvm-ia64.c > +++ b/arch/ia64/kvm/kvm-ia64.c > @@ -1628,7 +1628,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, > > void kvm_arch_flush_shadow(struct kvm *kvm) > { > - kvm_flush_remote_tlbs(kvm); > + kvm_mark_tlb_dirty(kvm); > + kvm_cond_flush_remote_tlbs(kvm); > } > > long kvm_arch_dev_ioctl(struct file *filp, > @@ -1853,10 +1854,11 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, > > /* If nothing is dirty, don't bother messing with page tables. */ > if (is_dirty) { > - kvm_flush_remote_tlbs(kvm); > + kvm_mark_tlb_dirty(kvm); > n = kvm_dirty_bitmap_bytes(memslot); > memset(memslot->dirty_bitmap, 0, n); > } > + kvm_cond_flush_remote_tlbs(kvm); > r = 0; > out: > mutex_unlock(&kvm->slots_lock); > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 07424cf..03a9c80 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -1170,7 +1170,9 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp, > } > > if (need_flush) > - kvm_flush_remote_tlbs(kvm); > + kvm_mark_tlb_dirty(kvm); > + > + kvm_cond_flush_remote_tlbs(kvm); > > return 0; > } > @@ -1294,7 +1296,8 @@ static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn) > rmapp = gfn_to_rmap(vcpu->kvm, gfn, sp->role.level); > > kvm_unmap_rmapp(vcpu->kvm, rmapp, 0); > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_mark_tlb_dirty(vcpu->kvm); > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > } > > int kvm_age_hva(struct kvm *kvm, unsigned long hva) > @@ -1697,7 +1700,9 @@ static void mmu_sync_children(struct kvm_vcpu *vcpu, > protected |= rmap_write_protect(vcpu->kvm, sp->gfn); > > if (protected) > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_mark_tlb_dirty(vcpu->kvm); > + > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > > for_each_sp(pages, sp, parents, i) { > kvm_sync_page(vcpu, sp, &invalid_list); > @@ -1786,7 +1791,8 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, > &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)]); > if (!direct) { > if (rmap_write_protect(vcpu->kvm, gfn)) > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_mark_tlb_dirty(vcpu->kvm); > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > if (level > PT_PAGE_TABLE_LEVEL && need_sync) > kvm_sync_pages(vcpu, gfn); > > @@ -1861,7 +1867,8 @@ static void drop_large_spte(struct kvm_vcpu *vcpu, u64 *sptep) > if (is_large_pte(*sptep)) { > drop_spte(vcpu->kvm, sptep); > --vcpu->kvm->stat.lpages; > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_mark_tlb_dirty(vcpu->kvm); > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > } > } > > @@ -1883,7 +1890,8 @@ static void validate_direct_spte(struct kvm_vcpu *vcpu, u64 *sptep, > return; > > drop_parent_pte(child, sptep); > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_mark_tlb_dirty(vcpu->kvm); > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > } > } > > @@ -2021,7 +2029,8 @@ static void kvm_mmu_commit_zap_page(struct kvm *kvm, > if (list_empty(invalid_list)) > return; > > - kvm_flush_remote_tlbs(kvm); > + kvm_mark_tlb_dirty(kvm); > + kvm_cond_flush_remote_tlbs(kvm); > > if (atomic_read(&kvm->arch.reader_counter)) { > kvm_mmu_isolate_pages(invalid_list); > @@ -2344,7 +2353,9 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, > * might be cached on a CPU's TLB. > */ > if (is_writable_pte(entry) && !is_writable_pte(*sptep)) > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_mark_tlb_dirty(vcpu->kvm); > + > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > done: > return ret; > } > @@ -2376,15 +2387,15 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep, > > child = page_header(pte & PT64_BASE_ADDR_MASK); > drop_parent_pte(child, sptep); > - kvm_flush_remote_tlbs(vcpu->kvm); > } else if (pfn != spte_to_pfn(*sptep)) { > pgprintk("hfn old %llx new %llx\n", > spte_to_pfn(*sptep), pfn); > drop_spte(vcpu->kvm, sptep); > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_mark_tlb_dirty(vcpu->kvm); > } else > was_rmapped = 1; > } > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > > if (set_spte(vcpu, sptep, pte_access, user_fault, write_fault, > level, gfn, pfn, speculative, true, > @@ -3561,14 +3572,13 @@ static bool need_remote_flush(u64 old, u64 new) > } > > static void mmu_pte_write_flush_tlb(struct kvm_vcpu *vcpu, bool zap_page, > - bool remote_flush, bool local_flush) > + bool local_flush) > { > if (zap_page) > return; > > - if (remote_flush) > - kvm_flush_remote_tlbs(vcpu->kvm); > - else if (local_flush) > + kvm_cond_flush_remote_tlbs(vcpu->kvm); > + if (local_flush) > kvm_mmu_flush_tlb(vcpu); > } > > @@ -3742,11 +3752,11 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa, > & mask.word) && rmap_can_add(vcpu)) > mmu_pte_write_new_pte(vcpu, sp, spte, &gentry); > if (!remote_flush && need_remote_flush(entry, *spte)) > - remote_flush = true; > + kvm_mark_tlb_dirty(vcpu->kvm); A remote cpu can flush the TLBs, marking the TLB as synced. But "remote_flush" variable remains true, so for further spte updates, the TLB is not marked dirty. Better kill the remote_flush variable (still reviewing). -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html