From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from va3outboundpool.messaging.microsoft.com (va3ehsobe010.messaging.microsoft.com [216.32.180.30]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (Client CN "mail.global.frontbridge.com", Issuer "MSIT Machine Auth CA 2" (not verified)) by ozlabs.org (Postfix) with ESMTPS id E5E6D2C014F for ; Sat, 21 Sep 2013 04:50:26 +1000 (EST) Message-ID: <1379703009.16231.32.camel@aoeu.buserror.net> Subject: Re: [PATCH 5/6 v6] kvm: booke: clear host tlb reference flag on guest tlb invalidation From: Scott Wood To: Bharat Bhushan Date: Fri, 20 Sep 2013 13:50:09 -0500 In-Reply-To: <1379651122-26078-1-git-send-email-Bharat.Bhushan@freescale.com> References: <1379651122-26078-1-git-send-email-Bharat.Bhushan@freescale.com> Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Cc: kvm@vger.kernel.org, agraf@suse.de, kvm-ppc@vger.kernel.org, Bharat Bhushan , paulus@samba.org, linuxppc-dev@lists.ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, 2013-09-20 at 09:55 +0530, Bharat Bhushan wrote: > On booke, "struct tlbe_ref" contains host tlb mapping information > (pfn: for guest-pfn to pfn, flags: attribute associated with this mapping) > for a guest tlb entry. So when a guest creates a TLB entry then > "struct tlbe_ref" is set to point to valid "pfn" and set attributes in > "flags" field of the above said structure. When a guest TLB entry is > invalidated then flags field of corresponding "struct tlbe_ref" is > updated to point that this is no more valid, also we selectively clear > some other attribute bits, example: if E500_TLB_BITMAP was set then we clear > E500_TLB_BITMAP, if E500_TLB_TLB0 is set then we clear this. > > Ideally we should clear complete "flags" as this entry is invalid and does not > have anything to re-used. The other part of the problem is that when we use > the same entry again then also we do not clear (started doing or-ing etc). > > So far it was working because the selectively clearing mentioned above > actually clears "flags" what was set during TLB mapping. But the problem > starts coming when we add more attributes to this then we need to selectively > clear them and which is not needed. > > This patch we do both > - Clear "flags" when invalidating; > - Clear "flags" when reusing same entry later > > Signed-off-by: Bharat Bhushan > --- > v5->v6 > - Fix flag clearing comment The changes between v5 and v6 are not just about comments... > > arch/powerpc/kvm/e500_mmu_host.c | 16 ++++++++-------- > 1 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c > index 1c6a9d7..7370e1c 100644 > --- a/arch/powerpc/kvm/e500_mmu_host.c > +++ b/arch/powerpc/kvm/e500_mmu_host.c > @@ -230,15 +230,15 @@ void inval_gtlbe_on_host(struct kvmppc_vcpu_e500 *vcpu_e500, int tlbsel, > ref->flags &= ~(E500_TLB_TLB0 | E500_TLB_VALID); > } > > - /* Already invalidated in between */ > - if (!(ref->flags & E500_TLB_VALID)) > - return; > - > - /* Guest tlbe is backed by at most one host tlbe per shadow pid. */ > - kvmppc_e500_tlbil_one(vcpu_e500, gtlbe); > + /* > + * Check whether TLB entry is already invalidated in between > + * Guest tlbe is backed by at most one host tlbe per shadow pid. > + */ > + if (ref->flags & E500_TLB_VALID) > + kvmppc_e500_tlbil_one(vcpu_e500, gtlbe); I'd phrase this combined comment as "If it's still valid, it's a TLB0 entry, and thus backed by at most one host tlbe per shadow pid". Otherwise looks good. -Scott