From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Zyngier Subject: Re: [kvmarm] [PATCH 13/15] KVM: ARM: Handle guest faults in KVM Date: Thu, 27 Sep 2012 16:26:28 +0100 Message-ID: <50647024.2070307@arm.com> References: <20120915153359.21241.86002.stgit@ubuntu> <20120915153552.21241.8656.stgit@ubuntu> <000101cd9b0e$78934fe0$69b9efa0$@samsung.com> <000901cd9c5d$cc53fbc0$64fbf340$@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=KSC5601 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: 'Christoffer Dall' , =?EUC-KR?B?J7How6I=?= =?EUC-KR?B?yK8n?= , "linux-arm-kernel@lists.infradead.org" , "kvm@vger.kernel.org" , "kvmarm@lists.cs.columbia.edu" To: Min-gyu Kim Return-path: Received: from service87.mimecast.com ([91.220.42.44]:57156 "EHLO service87.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755201Ab2I0P0e convert rfc822-to-8bit (ORCPT ); Thu, 27 Sep 2012 11:26:34 -0400 In-Reply-To: <000901cd9c5d$cc53fbc0$64fbf340$@samsung.com> Sender: kvm-owner@vger.kernel.org List-ID: On 27/09/12 04:11, Min-gyu Kim wrote: >=20 >=20 >> -----Original Message----- >> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] O= n >> 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 >> >>>> + >>>> + /* >>>> + * 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? >>> >>> >> >> 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 mappi= ngs). >> 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 HP= =46AR >> which gives us the IPA is not populated on execute never faults, so = we >> would have to translate the PC's va to ipa using cp15 functionality = when >> this happens, which is then also racy with other CPUs. So the questi= on is >> really if this will even be an optimization, but it's definitely som= ething >> that requires further investigation. >=20 > OK. I understand your point. >=20 > But if guest maps a page for execution, guest will flush Icache > from __sync_icache_dcache. Then coherent_icache_guest_page doesn't se= em to > be > necessary again. One thing I'm not sure in this case is when guest ma= ps > 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= to > reduce=20 > the number of flush by limiting the address range for flush. I think you're missing the major point: When the guest maps a page for execution, it knows it has to synchroniz= e Icache and Dcache. But the guest never knows when we swap out a page because of memory pressure. When the guest eventually faults that page back in, chances are it will be a different physical page, and the cache content may be inconsistent= =2E We must then sync Icache/Dcache for this page. Now, as Christoffer mentioned, there's a number of schemes we could potentially use to mitigate this effect (using the XN bit in the Stage2 page tables), but it remains to be seen how effective it will be. M. --=20 Jazz is not dead. It just smells funny...