Re: [Qemu-ppc] [PATCH RFC] target-ppc: tlbie should have global effect

[Nikunj A Dadhania]

Benjamin Herrenschmidt <address@hidden> writes:

> On Fri, 2016-09-09 at 10:15 +0530, Nikunj A Dadhania wrote:
>> > Benjamin Herrenschmidt <address@hidden> writes:
>> 
>> > 
>> > On Fri, 2016-09-09 at 09:53 +0530, Nikunj A Dadhania wrote:
>> > > 
>> > > tlbie (and H_REMOVE for pseries) should have a global effect. This is
>> > > achieved by iterating and setting tlb_need_flush in all the CPUs.
>> > > 
>> > > > > > Suggested-by: Benjamin Herrenschmidt <address@hidden>
>> > > > > > Signed-off-by: Nikunj A Dadhania <address@hidden>
>> > > 
>> > > --
>> > > 
>> > > Note: Haven't changed following POWERPC_MMU_32B and POWERPC_MMU_601
>> > > yet.
>> > > As I am not sure about it.
>> > 
>> > 604 and 7400 can do SMP.
>> 
>> Sure, will add similar logic there.
>> 
>> > 
>> > That said, I think the approach in your patch is going to be a bit big
>> > of a hammer.
>> 
>> > 
>> > We should have a separate flag indicating that we need to
>> > broadcast a flush and only set it on tlbie (non-l) and tlbivax (on
>> > BookE) so that we don't end up doing expensive broadcasts on things
>> > like context switches.
>> 
>> I had implemented that initially, and was checking that in
>> check_tlb_flush, the logic was getting complicated, so thought about
>> this way.
>> 
>> Moreover, I was thinking about it, that needs to be a global tcg flag
>> and not part the CPUPPCState structure, I was then worried about how to
>> synchronise a global tcg variable.
>
> No it doesn't.
>
> When a "broadcast TLB" op happens, such as tlbie, you set both flags.
> The existing one which just means the current CPU needs flushing, that
> logic doesnt' change at all.
>
> The other one means that *this* CPU needs to broadcast a TLB inval,
> thus it's also a local flag !
>
> Then you change gen_check_tlb_flush() to take an argument (an immediate
> value) which you set to 1 in ptesync on BookS and tlbsync on BookE and
> leave 0 on all the others.

One more question, when in gen_check_tlb_flush, don't I need to see if
other CPU has global flag set ?

Regards,
Nikunj