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 <bharat.bhushan@xxxxxxxxxxxxx> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
