On 01/18/2013 08:08:01 AM, Alexander Graf wrote:
On 18.01.2013, at 04:05, Scott Wood wrote:
> Invalidation paths that call kvmppc_e500_tlbil_all(), such as
MMUCSR0 and tlbivax, need a call to clear_tlb_refs() in order to get
the valid bits cleared.
Yeah, instead of kvmppc_e500_tlbil_all(). This might need a bit of
rework, since we end up flushing the full host TLB / cache even when
only evicting a single page at times.
I assume you're talking about the tlbivax single-page case. That could
certainly be improved, but I'd consider it a low priority until we have
a guest that actually uses it (Linux does tlbsx+tlbwe to locally
invalidate single pages on e500v2).
So one more question I have here is why kvmppc_core_flush_tlb() calls
clear_tlb1_bitmap().
That function looks like a better candidate for "flush all of the
pending host translation" than clear_tlb_refs() which to me sounds
more like an implementation detail of the host TLB implementation.
I don't quite follow. The implementation of kvmppc_core_flush_tlb() is
also an implementation detail of the host TLB implementation.
After clear_tlb_refs() there shouldn't be any pending bitmaps lying
around, right?
clear_tlb_refs() doesn't clear the bitmap. It does a bulk invalidation
rather than calling inval_gtlbe_on_host(). Perhaps we should move the
clear_tlb1_bitmap() call into clear_tlb_refs() and rename the latter to
kvmppc_core_flush_tlb(). The dirty_tlb ioctl calls clear_tlb_refs()
without clearing the bitmap, but that's probably a bug.
> In looking this up, I also saw that tlbilxlpid (T=0) seems to be
broken; it compares PID/TID as if it were T=1. Don't be fooled by
the "lpid" in the name; it's still relevant (and different from T=1)
in the absence of E.HV, and should be treated as "tlbilx all". Once
implemented, that will also presumably use kvmppc_e500_tlbil_all().
Yay :). That's a follow-up patch though. Let me put it on the todo
list.
Never mind, I wasn't reading the code closely enough. T=0 will use
inval_gtlbe_on_host() for all entries, though kvmppc_e500_tlbil_all()
would be more efficient.
-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
