Re: [PATCH 1/2] KVM: PPC: Book3S HV: Remove shared-TLB optimisation from vCPU TLB coherency logic

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Paul Mackerras's message of February 10, 2021 10:39 am:
> On Tue, Feb 09, 2021 at 06:44:53PM +1000, Nicholas Piggin wrote:
>> Excerpts from Paul Mackerras's message of February 9, 2021 5:19 pm:
>> > On Mon, Jan 18, 2021 at 10:26:08PM +1000, Nicholas Piggin wrote:
>> >> Processors that implement ISA v3.0 or later don't necessarily have
>> >> threads in a core sharing all translations, and/or TLBIEL does not
>> >> necessarily invalidate translations on all other threads (the
>> >> architecture talks only about the effect on translations for "the thread
>> >> executing the tlbiel instruction".
>> > 
>> > It seems to me that to have an implementation where TLB entries
>> > created on one thread (say T0) are visible to and usable by another
>> > thread (T1), but a tlbiel on thread T0 does not result in the entry
>> > being removed from visibility/usability on T1, is a pretty insane
>> > implementation.  I'm not sure that the architecture envisaged allowing
>> > this kind of implementation, though perhaps the language doesn't
>> > absolutely prohibit it.
>> > 
>> > This kind of implementation is what you are allowing for in this
>> > patch, isn't it?
>> 
>> Not intentionally, and patch 2 removes the possibility.
>> 
>> The main thing it allows is an implementation where TLB entries created 
>> by T1 which are visble only to T1 do not get removed by TLBIEL on T0.
> 
> I could understand this patch as trying to accommodate both those
> implementations where TLB entries are private to each thread, and
> those implementations where TLB entries are shared, without needing to
> distinguish between them, at the expense of doing unnecessary
> invalidations on both kinds of implementation.

That's exactly what it is. Existing code accommodates shared TLBs, this 
patch additionally allows for private.

>> I also have some concern with ordering of in-flight operations (ptesync,
>> memory ordering, etc) which are mostly avoided with this.
>> 
>> > The sane implementations would be ones where either (a) TLB entries
>> > are private to each thread and tlbiel only works on the local thread,
>> > or (b) TLB entries can be shared and tlbiel works across all threads.
>> > I think this is the conclusion we collectively came to when working on
>> > that bug we worked on towards the end of last year.
>> 
>> I think an implementation could have both types. So it's hard to get 
>> away from flushing all threads.
> 
> Having both private and shared TLB entries in the same implementation
> would seem very odd to me.  What would determine whether a given entry
> is shared or private?

Example: an ERAT or L1 TLB per-thread, and a shared L2 TLB behind that.
The L1 may not be PID/LPID tagged so you don't want to cross-invalidate
other threads every time, say.

Thanks,
Nick