Re: [Qemu-devel] [PATCH RFC 4/4] target-ppc: flush tlb from all the cpu

["Alex Bennée" <alex.bennee@linaro.org>]

Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:

> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
>
>> On Fri, 2016-09-02 at 12:02 +0530, Nikunj A Dadhania wrote:
>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>> ---
>>> cputlb.c| 15 +++++++++++++++
>>> include/exec/exec-all.h |2 ++
>>> target-ppc/mmu-hash64.c |2 +-
>>> 3 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cputlb.c b/cputlb.c
>>> index 64faf47..17ff58e 100644
>>> --- a/cputlb.c
>>> +++ b/cputlb.c
>>> @@ -123,6 +123,21 @@ void tlb_flush(CPUState *cpu, int flush_global)
>>> }
>>> }
>>>
>>> +static void tlb_flush_all_async_work(CPUState *cpu, void *opaque)
>>> +{
>>> +tlb_flush_nocheck(cpu, GPOINTER_TO_INT(opaque));
>>> +}
>>> +
>>> +void tlb_flush_all(CPUState *cpu, int flush_global)
>>> +{
>>> +CPUState *c;
>>> +
>>> +CPU_FOREACH(c) {
>>> +async_run_on_cpu(c, tlb_flush_all_async_work,
>>> +GUINT_TO_POINTER(flush_global));
>>> +}
>>> +}
>>
>> Hrm... this is asynchronous?
>
> Yes.
>
>> It probably needs to be synchronous...
>
> I see run_on_cpu() which seems suitable.

I'm not so happy with run_on_cpu as it involves busy waiting for the
other CPU to finish.

>> We must provide a guarantee that no other processor can see the old
>> translation when the tlb invalidation sequence completes. With the
>> current lazy TLB flush, we already delay the invalidation until
>> we hit that synchronization point so we need to be synchronous.

When is the synchronisation point? On ARM we end the basic block on
system instructions that mess with the cache. As a result the flush is
done as soon as we exit the run loop on the next instruction.

>
>
>>> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
>>> index 8118143..d852c21 100644
>>> --- a/target-ppc/mmu-hash64.c
>>> +++ b/target-ppc/mmu-hash64.c
>>> @@ -912,7 +912,7 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
>>> * invalidate, and we still don't have a tlb_flush_mask(env, n,
>>> * mask) in QEMU, we just invalidate all TLBs
>>> */
>>> -tlb_flush(CPU(cpu), 1);
>>> +tlb_flush_all(CPU(cpu), 1);
>>> }
>>>
>>> void ppc_hash64_update_rmls(CPUPPCState *env)
>
> Regards,
> Nikunj


--
Alex Bennée