From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Date: Thu, 21 Aug 2008 13:27:25 +0000 Subject: Re: [PATCH 2/6] kvmppc: magic page hypercall - host part Message-Id: <48AD6D3D.503@linux.vnet.ibm.com> List-Id: References: <1219142205-12062-3-git-send-email-ehrhardt@linux.vnet.ibm.com> In-Reply-To: <1219142205-12062-3-git-send-email-ehrhardt@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: kvm-ppc@vger.kernel.org Hollis Blanchard wrote: > On Tue, 2008-08-19 at 12:36 +0200, ehrhardt@linux.vnet.ibm.com wrote: > =20 >> diff --git a/arch/powerpc/kvm/booke_guest.c b/arch/powerpc/kvm/booke_gue= st.c >> --- a/arch/powerpc/kvm/booke_guest.c >> +++ b/arch/powerpc/kvm/booke_guest.c >> @@ -21,6 +21,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -43,6 +44,7 @@ >> { "itlb_v", VCPU_STAT(itlb_virt_miss_exits) }, >> { "dtlb_r", VCPU_STAT(dtlb_real_miss_exits) }, >> { "dtlb_v", VCPU_STAT(dtlb_virt_miss_exits) }, >> + { "dtlb_pv", VCPU_STAT(dtlb_pvmem_miss_exits) }, >> { "sysc", VCPU_STAT(syscall_exits) }, >> { "isi", VCPU_STAT(isi_exits) }, >> { "dsi", VCPU_STAT(dsi_exits) }, >> @@ -337,6 +339,16 @@ >> unsigned long eaddr =3D vcpu->arch.fault_dear; >> gfn_t gfn; >> >> + >> + if (vcpu->arch.pvmem && kvmppc_is_pvmem(vcpu, eaddr)) { >> + kvmppc_mmu_map(vcpu, eaddr, >> + vcpu->arch.pvmem_gpaddr >> KVM_PPCPV_MAGIC_PAGE_SHIFT, >> + 0, KVM_PPCPV_MAGIC_PAGE_FLAGS); >> + vcpu->stat.dtlb_pvmem_miss_exits++; >> + r =3D RESUME_GUEST; >> + break; >> + } >> + >> /* Check the guest TLB. */ >> gtlbe =3D kvmppc_44x_dtlb_search(vcpu, eaddr); >> if (!gtlbe) { >> =20 > > By the way, when this code is running, what's the rate of this new > counter? How does it compare to the reduction in instruction emulation? > > =20 >> @@ -488,6 +500,8 @@ >> >> vcpu->arch.shadow_pid =3D 1; >> >> + vcpu->arch.pvmem =3D NULL; >> =20 > > Isn't the whole structure initialized to 0? I don't think this is > needed. > > =20 Even if not necessary I would like to keep it. We might change arch=20 allocation or something else. This initialization doesn't hurt anyone and is not performance critical. But it's up to you - if you want me to remove it I'll do that. please=20 drop me a mail if I should do that. *Like win popup - "do you really want to delete this line?" - yes/no/maybe > [...] >> static void kvmppc_emul_rfi(struct kvm_vcpu *vcpu) >> @@ -207,8 +208,18 @@ >> static int kvmppc_do_hypercall(struct kvm_vcpu *vcpu) >> { >> int ret =3D 0; >> + struct page *pvmem_page; >> >> switch (vcpu->arch.gpr[0]) { >> + case KVM_HCALL_RESERVE_MAGICPAGE: >> + vcpu->arch.pvmem_gvaddr =3D vcpu->arch.gpr[3]; >> + vcpu->arch.pvmem_gpaddr =3D vcpu->arch.gpr[4]; >> + down_read(¤t->mm->mmap_sem); >> + pvmem_page =3D gfn_to_page(vcpu->kvm,=20 >> + vcpu->arch.pvmem_gpaddr >> KVM_PPCPV_MAGIC_PAGE_SHIFT); >> + up_read(¤t->mm->mmap_sem); >> + vcpu->arch.pvmem =3D kmap(pvmem_page); >> + break; >> default: >> printk(KERN_ERR "unknown hypercall %d\n", vcpu->arch.gpr[0]); >> kvmppc_dump_vcpu(vcpu); >> =20 > > Where is vcpu->arch.pvmem unmapped? > =20 atm nowhere - it is persistent once it is registered > What happens if the guest makes repeated KVM_HCALL_RESERVE_MAGICPAGE > hypercalls? Looks like a good way to leak host memory. > > Also, if we migrate a guest which has a page registered, the new host > won't have vcpu->arch.pvmem set because the guest doesn't re-invoke the > KVM_HCALL_RESERVE_MAGICPAGE hypercall. > =20 yeah, but I stored gvaddr/gpaddr. If the migration keeps those (and it=20 should or a lot other things break) we can fix that. This is bringing me back to our discussion that I wanted to have a=20 "migration starts to run on new host" hook. I need to look for that. > I think we need to put a little more thought into all the corner cases > here. > =20 yeah, lets do that in a brainstorm session next week if you have some time. It's better to clean corners interactively ;-) --=20 Gr=FCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization