From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH 4/7] KVM: PPC: Add book3s_32 tlbie flush acceleration Date: Sun, 1 Aug 2010 22:20:37 +0200 Message-ID: <9F2B6C37-4D0E-46F4-9D15-CC785C2DDBD4@suse.de> References: <1280408662-10328-1-git-send-email-agraf@suse.de> <1280408662-10328-5-git-send-email-agraf@suse.de> <4C557FF1.4000400@redhat.com> Mime-Version: 1.0 (Apple Message framework v1081) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8BIT Cc: kvm-ppc@vger.kernel.org, KVM list , linuxppc-dev To: Avi Kivity Return-path: Received: from cantor2.suse.de ([195.135.220.15]:44505 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751310Ab0HAUUk convert rfc822-to-8bit (ORCPT ); Sun, 1 Aug 2010 16:20:40 -0400 In-Reply-To: <4C557FF1.4000400@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01.08.2010, at 16:08, Avi Kivity wrote: > On 07/29/2010 04:04 PM, Alexander Graf wrote: >> On Book3s_32 the tlbie instruction flushed effective addresses by the mask >> 0x0ffff000. This is pretty hard to reflect with a hash that hashes ~0xfff, so >> to speed up that target we should also keep a special hash around for it. >> >> >> static inline u64 kvmppc_mmu_hash_vpte(u64 vpage) >> { >> return hash_64(vpage& 0xfffffffffULL, HPTEG_HASH_BITS_VPTE); >> @@ -66,6 +72,11 @@ void kvmppc_mmu_hpte_cache_map(struct kvm_vcpu *vcpu, struct hpte_cache *pte) >> index = kvmppc_mmu_hash_pte(pte->pte.eaddr); >> hlist_add_head_rcu(&pte->list_pte,&vcpu->arch.hpte_hash_pte[index]); >> >> + /* Add to ePTE_long list */ >> + index = kvmppc_mmu_hash_pte_long(pte->pte.eaddr); >> + hlist_add_head_rcu(&pte->list_pte_long, >> + &vcpu->arch.hpte_hash_pte_long[index]); >> + > > Isn't it better to make operations on this list conditional on Book3s_32? Hashes are expensive since they usually cost cache misses. Yes, the same for vpte_long and vpte - book3s_32 guests don't need them except for the all flush. The tough part is that this is not host but guest dependent, so I need to have different structs for book3s_32 and book3s_64 guests. This isn't a big issue, but complicates the code. > Can of course be done later as an optimization. Yes, that was the plan. Great to see you got the same feeling there though :). To be honest, I even started a book3s_32 host optimization patch and threw it away because it made the code less readable. So yes, this is on my radar. Alex From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by ozlabs.org (Postfix) with ESMTP id 737D9B70D5 for ; Mon, 2 Aug 2010 06:20:42 +1000 (EST) Subject: Re: [PATCH 4/7] KVM: PPC: Add book3s_32 tlbie flush acceleration Mime-Version: 1.0 (Apple Message framework v1081) Content-Type: text/plain; charset=us-ascii From: Alexander Graf In-Reply-To: <4C557FF1.4000400@redhat.com> Date: Sun, 1 Aug 2010 22:20:37 +0200 Message-Id: <9F2B6C37-4D0E-46F4-9D15-CC785C2DDBD4@suse.de> References: <1280408662-10328-1-git-send-email-agraf@suse.de> <1280408662-10328-5-git-send-email-agraf@suse.de> <4C557FF1.4000400@redhat.com> To: Avi Kivity Cc: linuxppc-dev , KVM list , kvm-ppc@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 01.08.2010, at 16:08, Avi Kivity wrote: > On 07/29/2010 04:04 PM, Alexander Graf wrote: >> On Book3s_32 the tlbie instruction flushed effective addresses by the = mask >> 0x0ffff000. This is pretty hard to reflect with a hash that hashes = ~0xfff, so >> to speed up that target we should also keep a special hash around for = it. >>=20 >>=20 >> static inline u64 kvmppc_mmu_hash_vpte(u64 vpage) >> { >> return hash_64(vpage& 0xfffffffffULL, HPTEG_HASH_BITS_VPTE); >> @@ -66,6 +72,11 @@ void kvmppc_mmu_hpte_cache_map(struct kvm_vcpu = *vcpu, struct hpte_cache *pte) >> index =3D kvmppc_mmu_hash_pte(pte->pte.eaddr); >> = hlist_add_head_rcu(&pte->list_pte,&vcpu->arch.hpte_hash_pte[index]); >>=20 >> + /* Add to ePTE_long list */ >> + index =3D kvmppc_mmu_hash_pte_long(pte->pte.eaddr); >> + hlist_add_head_rcu(&pte->list_pte_long, >> + &vcpu->arch.hpte_hash_pte_long[index]); >> + >=20 > Isn't it better to make operations on this list conditional on = Book3s_32? Hashes are expensive since they usually cost cache misses. Yes, the same for vpte_long and vpte - book3s_32 guests don't need them = except for the all flush. The tough part is that this is not host but = guest dependent, so I need to have different structs for book3s_32 and = book3s_64 guests. This isn't a big issue, but complicates the code. > Can of course be done later as an optimization. Yes, that was the plan. Great to see you got the same feeling there = though :). To be honest, I even started a book3s_32 host optimization = patch and threw it away because it made the code less readable. So yes, = this is on my radar. Alex From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Sun, 01 Aug 2010 20:20:37 +0000 Subject: Re: [PATCH 4/7] KVM: PPC: Add book3s_32 tlbie flush acceleration Message-Id: <9F2B6C37-4D0E-46F4-9D15-CC785C2DDBD4@suse.de> List-Id: References: <1280408662-10328-1-git-send-email-agraf@suse.de> <1280408662-10328-5-git-send-email-agraf@suse.de> <4C557FF1.4000400@redhat.com> In-Reply-To: <4C557FF1.4000400@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Avi Kivity Cc: kvm-ppc@vger.kernel.org, KVM list , linuxppc-dev On 01.08.2010, at 16:08, Avi Kivity wrote: > On 07/29/2010 04:04 PM, Alexander Graf wrote: >> On Book3s_32 the tlbie instruction flushed effective addresses by the mask >> 0x0ffff000. This is pretty hard to reflect with a hash that hashes ~0xfff, so >> to speed up that target we should also keep a special hash around for it. >> >> >> static inline u64 kvmppc_mmu_hash_vpte(u64 vpage) >> { >> return hash_64(vpage& 0xfffffffffULL, HPTEG_HASH_BITS_VPTE); >> @@ -66,6 +72,11 @@ void kvmppc_mmu_hpte_cache_map(struct kvm_vcpu *vcpu, struct hpte_cache *pte) >> index = kvmppc_mmu_hash_pte(pte->pte.eaddr); >> hlist_add_head_rcu(&pte->list_pte,&vcpu->arch.hpte_hash_pte[index]); >> >> + /* Add to ePTE_long list */ >> + index = kvmppc_mmu_hash_pte_long(pte->pte.eaddr); >> + hlist_add_head_rcu(&pte->list_pte_long, >> + &vcpu->arch.hpte_hash_pte_long[index]); >> + > > Isn't it better to make operations on this list conditional on Book3s_32? Hashes are expensive since they usually cost cache misses. Yes, the same for vpte_long and vpte - book3s_32 guests don't need them except for the all flush. The tough part is that this is not host but guest dependent, so I need to have different structs for book3s_32 and book3s_64 guests. This isn't a big issue, but complicates the code. > Can of course be done later as an optimization. Yes, that was the plan. Great to see you got the same feeling there though :). To be honest, I even started a book3s_32 host optimization patch and threw it away because it made the code less readable. So yes, this is on my radar. Alex