From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 1/3] KVM: PPC: e500: Call kvmppc_mmu_map for initial mapping Date: Thu, 17 Jan 2013 18:11:17 -0600 Message-ID: <1358467877.13978.18@snotra> References: <1358463041-25922-2-git-send-email-agraf@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: , To: Alexander Graf Return-path: In-Reply-To: <1358463041-25922-2-git-send-email-agraf@suse.de> (from agraf@suse.de on Thu Jan 17 16:50:39 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 01/17/2013 04:50:39 PM, Alexander Graf wrote: > @@ -1024,9 +1001,11 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 > eaddr, gpa_t gpaddr, > { > struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > struct tlbe_priv *priv; > - struct kvm_book3e_206_tlb_entry *gtlbe, stlbe; > + struct kvm_book3e_206_tlb_entry *gtlbe, stlbe = {}; Is there a code path in which stlbe gets used but not fully filled in without this? > int tlbsel = tlbsel_of(index); > int esel = esel_of(index); > + /* Needed for initial map, where we can't use the cached value > */ > + int force_map = index & KVM_E500_INDEX_FORCE_MAP; > int stlbsel, sesel; > > gtlbe = get_entry(vcpu_e500, tlbsel, esel); > @@ -1038,7 +1017,7 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 > eaddr, gpa_t gpaddr, > priv = &vcpu_e500->gtlb_priv[tlbsel][esel]; > > /* Only triggers after clear_tlb_refs */ > - if (unlikely(!(priv->ref.flags & E500_TLB_VALID))) > + if (force_map || unlikely(!(priv->ref.flags & > E500_TLB_VALID))) > kvmppc_e500_tlb0_map(vcpu_e500, esel, &stlbe); > else > kvmppc_e500_setup_stlbe(vcpu, gtlbe, > BOOK3E_PAGESZ_4K, It seems a bit odd to overload index rather than adding a flags parameter... It also seems like it would be cleaner to just invalidate the old entry in tlbwe, and then this function doesn't need to change at all. I am a bit confused by how invalidation is currently operating -- why is E500_TLB_VALID not cleared on invalidations (except for MMU API stuff and MMU notifiers)? -Scott