Re: [PATCH v2 2/5] KVM: MMU: Convert remote flushes to kvm_mark_tlb_dirty() and a conditional flush
On Thu, 17 May 2012 13:24:41 +0300 Avi Kivity wrote: > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 2256f51..a2149d8 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3130,7 +3130,9 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct > kvm_dirty_log *log) > kvm_mmu_write_protect_pt_masked(kvm, memslot, offset, mask); > } > if (is_dirty) > - kvm_flush_remote_tlbs(kvm); > + kvm_mark_tlb_dirty(kvm); > + > + kvm_cond_flush_remote_tlbs(kvm); > > spin_unlock(&kvm->mmu_lock); Any reason not to move this flush outside of the mmu_lock in this patch series? Unlike other rmap write protections, this one seems to be simple. Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/5] KVM: MMU: Convert remote flushes to kvm_mark_tlb_dirty() and a conditional flush
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
> ---
> 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))
> -
[PATCH v2 2/5] KVM: MMU: Convert remote flushes to kvm_mark_tlb_dirty() and a conditional flush
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
---
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(s
