[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH RFC] target-ppc: tlbie should have global effect
From: |
Nikunj A Dadhania |
Subject: |
Re: [Qemu-ppc] [PATCH RFC] target-ppc: tlbie should have global effect |
Date: |
Fri, 09 Sep 2016 10:38:23 +0530 |
User-agent: |
Notmuch/0.21 (https://notmuchmail.org) Emacs/25.0.94.1 (x86_64-redhat-linux-gnu) |
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