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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]

On Sat, Sep 10, 2016 at 09:03:56AM +1000, Benjamin Herrenschmidt wrote:
> 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.

I'll wait for a respin addressing these comments.

Please also add a cover letter on the next version.

> 
> >  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
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]