Re: [Qemu-devel] [PATCH v2 3/3] target-ppc: tlbie should have global effect

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Fri, 2016-09-09 at 18:44 +0530, Nikunj A Dadhania wrote:
> +static inline void tlb_clear_flag(CPUState *cs)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +    env->tlb_need_flush = 0;
> +}

What is the point of making this a separate function ?

Also I'm not 100% certain about the correctness of clearing
TLB_NEED_GLOBAL_FLUSH on the "other" guy.

We could have the situation where:

	cpu 1:					cpu 2:
	sets both				..
	isync (clears local flush)		..
	<insert new translation>
	..					set both
	..					..
	..					..
	ptesync (clears global flush)		.. (both gets cleared)

Now here, you can see that cpu2 never does a global flush and so the
new translation inserted by cpu 1 is not cleared while architecturally
it should be.

That being said, I doubt the above scenario can happen in practice,
but I think it's safer if you only clear the local bit on the "other"
CPUs.

>  static inline void check_tlb_flush(CPUPPCState *env, uint32_t
> global)
>  {
>      CPUState *cs = CPU(ppc_env_get_cpu(env));
> @@ -161,6 +169,17 @@ static inline void check_tlb_flush(CPUPPCState
> *env, uint32_t global)
>          tlb_flush(cs, 1);
>          env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
>      }
> +
> +    if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
> +        CPUState *other_cs;
> +        CPU_FOREACH(other_cs) {
> +            if (other_cs != cs) {
> +                tlb_clear_flag(other_cs);
> +                tlb_flush(other_cs, 1);
> +            }
> +        }
> +        env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
> +    }
>  }
>  #else