From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [patch 3/3] KVM: move postcommit flush to x86, as mmio sptes are x86 specific Date: Mon, 27 Aug 2012 16:06:01 -0300 Message-ID: <20120827190601.GB27575@amt.cnet> References: <20120824185456.658814016@amt.cnet> <20120824185634.625676530@amt.cnet> <20120827234108.6f4417e2922e42ef461135c6@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Paul Mackerras , Avi Kivity , Xiao Guangrong , kvm@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:27606 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752558Ab2H0TNc (ORCPT ); Mon, 27 Aug 2012 15:13:32 -0400 Content-Disposition: inline In-Reply-To: <20120827234108.6f4417e2922e42ef461135c6@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Aug 27, 2012 at 11:41:08PM +0900, Takuya Yoshikawa wrote: > On Fri, 24 Aug 2012 15:54:59 -0300 > Marcelo Tosatti wrote: > > > Other arches do not need this. > > > > Signed-off-by: Marcelo Tosatti > > > > Index: kvm/arch/x86/kvm/x86.c > > =================================================================== > > --- kvm.orig/arch/x86/kvm/x86.c > > +++ kvm/arch/x86/kvm/x86.c > > @@ -6455,6 +6455,14 @@ void kvm_arch_commit_memory_region(struc > > kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages); > > kvm_mmu_slot_remove_write_access(kvm, mem->slot); > > spin_unlock(&kvm->mmu_lock); > > + /* > > + * If the new memory slot is created, we need to clear all > > + * mmio sptes. > > + */ > > + if (old.npages == 0 && npages) { > > + kvm_mmu_zap_all(kvm); > > + kvm_reload_remote_mmus(kvm); > > + } > > } > > Any explanation why (old.base_gfn != new.base_gfn) case can be > omitted? (old.base_gfn != new.base_gfn) check covers the cases 1. old.base_gfn = 0, new.base_gfn = !0 (slot creation) and x != 0, y != 0, x != y. 2. old.base_gfn = x, new.base_gfn = y (gpa base change) Patch 2 covers case 2, so its only necessary to cover case 1 here. Makes sense? > Takuya > > > > > void kvm_arch_flush_shadow_all(struct kvm *kvm) > > Index: kvm/virt/kvm/kvm_main.c > > =================================================================== > > --- kvm.orig/virt/kvm/kvm_main.c > > +++ kvm/virt/kvm/kvm_main.c > > @@ -849,13 +849,6 @@ int __kvm_set_memory_region(struct kvm * > > > > kvm_arch_commit_memory_region(kvm, mem, old, user_alloc); > > > > - /* > > - * If the new memory slot is created, we need to clear all > > - * mmio sptes. > > - */ > > - if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT) > > - kvm_arch_flush_shadow_all(kvm); > > - > > kvm_free_physmem_slot(&old, &new); > > kfree(old_memslots);