From mboxrd@z Thu Jan 1 00:00:00 1970 From: Min-gyu Kim Subject: RE: [PATCH 13/15] KVM: ARM: Handle guest faults in KVM Date: Thu, 27 Sep 2012 12:11:33 +0900 Message-ID: <000901cd9c5d$cc53fbc0$64fbf340$@samsung.com> References: <20120915153359.21241.86002.stgit@ubuntu> <20120915153552.21241.8656.stgit@ubuntu> <000101cd9b0e$78934fe0$69b9efa0$@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ks_c_5601-1987 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, =?ks_c_5601-1987?B?J7How6LIryc=?= To: 'Christoffer Dall' Return-path: Received: from mailout1.samsung.com ([203.254.224.24]:44146 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751461Ab2I0DLk convert rfc822-to-8bit (ORCPT ); Wed, 26 Sep 2012 23:11:40 -0400 Received: from epcpsbgm1.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MAZ00H3FNJDX3K0@mailout1.samsung.com> for kvm@vger.kernel.org; Thu, 27 Sep 2012 12:11:37 +0900 (KST) Received: from MingyuPC ([182.198.1.3]) by mmp1.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MAZ00AMANJDMC30@mmp1.samsung.com> for kvm@vger.kernel.org; Thu, 27 Sep 2012 12:11:37 +0900 (KST) In-reply-to: Content-language: ko Sender: kvm-owner@vger.kernel.org List-ID: > -----Original Message----- > From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On > Behalf Of Christoffer Dall > Sent: Tuesday, September 25, 2012 9:39 PM > To: Min-gyu Kim > Cc: kvm@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > kvmarm@lists.cs.columbia.edu; =B1=E8=C3=A2=C8=AF > Subject: Re: [PATCH 13/15] KVM: ARM: Handle guest faults in KVM >=20 > >> + > >> + /* > >> + * If this is a write fault (think COW) we need to make sure= the > >> + * existing page, which other CPUs might still read, doesn't= go > >> away > >> + * from under us, by calling gfn_to_pfn_prot(write_fault=3Dt= rue). > >> + * Therefore, we call gfn_to_pfn_prot(write_fault=3Dfalse), = which > >> will > >> + * pin the existing page, then we get a new page for the use= r > space > >> + * pte and map this in the stage-2 table where we also make = sure > to > >> + * flush the TLB for the VM, if there was an existing entry > >> + (the > >> entry > >> + * was updated setting the write flag to the potentially new page). > >> + */ > >> + if (fault_status =3D=3D FSC_PERM) { > >> + pfn_existing =3D gfn_to_pfn_prot(vcpu->kvm, gfn, fal= se, NULL); > >> + if (is_error_pfn(pfn_existing)) > >> + return -EFAULT; > >> + } > >> + > >> + pfn =3D gfn_to_pfn_prot(vcpu->kvm, gfn, write_fault, &writab= le); > >> + if (is_error_pfn(pfn)) { > >> + ret =3D -EFAULT; > >> + goto out_put_existing; > >> + } > >> + > >> + /* We need minimum second+third level pages */ > >> + ret =3D mmu_topup_memory_cache(memcache, 2, KVM_NR_MEM_OBJS)= ; > >> + if (ret) > >> + goto out; > >> + new_pte =3D pfn_pte(pfn, PAGE_KVM_GUEST); > >> + if (writable) > >> + pte_val(new_pte) |=3D L_PTE2_WRITE; > >> + coherent_icache_guest_page(vcpu->kvm, gfn); > > > > why don't you flush icache only when guest has mapped executable pa= ge > > as __sync_icache_dcache function does currently? > > > > >=20 > because we don't know if the guest will map the page executable. The = guest > may read the page through a normal load, which causes the fault, and > subsequently execute it (even possible through different guest mappin= gs). > The only way to see this happening would be to mark all pages as non- > executable and catch the fault when it occurs - unfortunately the HPF= AR > which gives us the IPA is not populated on execute never faults, so w= e > would have to translate the PC's va to ipa using cp15 functionality w= hen > this happens, which is then also racy with other CPUs. So the questio= n is > really if this will even be an optimization, but it's definitely some= thing > that requires further investigation. OK. I understand your point. But if guest maps a page for execution, guest will flush Icache from __sync_icache_dcache. Then coherent_icache_guest_page doesn't seem= to be necessary again. One thing I'm not sure in this case is when guest maps for kernel executable page(module loading) and it reuses the kernel executable page from host(module unloading). But in that case, I think it is possible t= o reduce=20 the number of flush by limiting the address range for flush. >=20 > -Christoffer > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in the= body > of a message to majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html