[Qemu-devel] [PATCH v2 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag

[Qemu-devel] [PATCH v2 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/cpu.h         | 1 +
 target-ppc/helper_regs.h | 4 ++--
 target-ppc/mmu-hash64.c  | 4 ++--
 target-ppc/mmu_helper.c  | 6 +++---
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 1e808c8..71111dc 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1009,6 +1009,7 @@ struct CPUPPCState {
     bool tlb_dirty;   /* Set to non-zero when modifying TLB                  */
     bool kvm_sw_tlb;  /* non-zero if KVM SW TLB API is active                */
     uint32_t tlb_need_flush; /* Delayed flush needed */
+#define TLB_NEED_LOCAL_FLUSH   0x1
 #endif
 
     /* Other registers */
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 3d279f1..69204a5 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -157,9 +157,9 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
 static inline void check_tlb_flush(CPUPPCState *env)
 {
     CPUState *cs = CPU(ppc_env_get_cpu(env));
-    if (env->tlb_need_flush) {
-        env->tlb_need_flush = 0;
+    if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
         tlb_flush(cs, 1);
+        env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
     }
 }
 #else
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 8118143..1f52b64 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -110,7 +110,7 @@ void helper_slbia(CPUPPCState *env)
              *      and we still don't have a tlb_flush_mask(env, n, mask)
              *      in QEMU, we just invalidate all TLBs
              */
-            env->tlb_need_flush = 1;
+            env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
         }
     }
 }
@@ -132,7 +132,7 @@ void helper_slbie(CPUPPCState *env, target_ulong addr)
          *      and we still don't have a tlb_flush_mask(env, n, mask)
          *      in QEMU, we just invalidate all TLBs
          */
-        env->tlb_need_flush = 1;
+        env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
     }
 }
 
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 696bb03..d59d2f8 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -1965,7 +1965,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
          * we just mark the TLB to be flushed later (context synchronizing
          * event or sync instruction on 32-bit).
          */
-        env->tlb_need_flush = 1;
+        env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
         break;
 #if defined(TARGET_PPC64)
     case POWERPC_MMU_64B:
@@ -1979,7 +1979,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
          *      and we still don't have a tlb_flush_mask(env, n, mask) in QEMU,
          *      we just invalidate all TLBs
          */
-        env->tlb_need_flush = 1;
+        env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
         break;
 #endif /* defined(TARGET_PPC64) */
     default:
@@ -2065,7 +2065,7 @@ void helper_store_sr(CPUPPCState *env, target_ulong srnum, target_ulong value)
             }
         }
 #else
-        env->tlb_need_flush = 1;
+        env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
 #endif
     }
 }
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/3] target-ppc: add flag in chech_tlb_flush()

[Nikunj A Dadhania]

The flag will be used to indicate whether broadcast tlb flush is needed
or not.

Moreover, BookS does both ptesync and tlbsync, so make that a nop for
the server and tlbsync would generate a check flush for BookE

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/ppc/spapr_hcall.c     |  4 ++--
 target-ppc/excp_helper.c |  4 ++--
 target-ppc/helper.h      |  2 +-
 target-ppc/helper_regs.h |  4 ++--
 target-ppc/mmu_helper.c  |  4 ++--
 target-ppc/translate.c   | 20 ++++++++++----------
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 73af112..ef12ea0 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -201,7 +201,7 @@ static target_ulong h_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
     switch (ret) {
     case REMOVE_SUCCESS:
-        check_tlb_flush(env);
+        check_tlb_flush(env, 1);
         return H_SUCCESS;
 
     case REMOVE_NOT_FOUND:
@@ -282,7 +282,7 @@ static target_ulong h_bulk_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         }
     }
  exit:
-    check_tlb_flush(env);
+    check_tlb_flush(env, 1);
 
     return rc;
 }
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 04ed4da..3b78126 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -711,7 +711,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
     /* Any interrupt is context synchronizing, check if TCG TLB
      * needs a delayed flush on ppc64
      */
-    check_tlb_flush(env);
+    check_tlb_flush(env, 0);
 }
 
 void ppc_cpu_do_interrupt(CPUState *cs)
@@ -973,7 +973,7 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
 
     /* Context synchronizing: check if TCG TLB needs flush */
-    check_tlb_flush(env);
+    check_tlb_flush(env, 0);
 }
 
 void helper_rfi(CPUPPCState *env)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index dcf3f95..a86e184 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -18,7 +18,7 @@ DEF_HELPER_1(rfid, void, env)
 DEF_HELPER_1(hrfid, void, env)
 DEF_HELPER_2(store_lpcr, void, env, tl)
 #endif
-DEF_HELPER_1(check_tlb_flush, void, env)
+DEF_HELPER_2(check_tlb_flush, void, env, i32)
 #endif
 
 DEF_HELPER_3(lmw, void, env, tl, i32)
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 69204a5..bcf65ce 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -154,7 +154,7 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
 }
 
 #if !defined(CONFIG_USER_ONLY)
-static inline void check_tlb_flush(CPUPPCState *env)
+static inline void check_tlb_flush(CPUPPCState *env, uint32_t global)
 {
     CPUState *cs = CPU(ppc_env_get_cpu(env));
     if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
@@ -163,7 +163,7 @@ static inline void check_tlb_flush(CPUPPCState *env)
     }
 }
 #else
-static inline void check_tlb_flush(CPUPPCState *env) { }
+static inline void check_tlb_flush(CPUPPCState *env, uint32_t global) { }
 #endif
 
 #endif /* HELPER_REGS_H */
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index d59d2f8..bf9f329 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2867,9 +2867,9 @@ void helper_booke206_tlbflush(CPUPPCState *env, target_ulong type)
 }
 
 
-void helper_check_tlb_flush(CPUPPCState *env)
+void helper_check_tlb_flush(CPUPPCState *env, unsigned int global)
 {
-    check_tlb_flush(env);
+    check_tlb_flush(env, global);
 }
 
 /*****************************************************************************/
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 618334a..77bb312 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3064,7 +3064,7 @@ static void gen_eieio(DisasContext *ctx)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-static inline void gen_check_tlb_flush(DisasContext *ctx)
+static inline void gen_check_tlb_flush(DisasContext *ctx, uint32_t global)
 {
     TCGv_i32 t;
     TCGLabel *l;
@@ -3076,12 +3076,13 @@ static inline void gen_check_tlb_flush(DisasContext *ctx)
     t = tcg_temp_new_i32();
     tcg_gen_ld_i32(t, cpu_env, offsetof(CPUPPCState, tlb_need_flush));
     tcg_gen_brcondi_i32(TCG_COND_EQ, t, 0, l);
-    gen_helper_check_tlb_flush(cpu_env);
+    tcg_gen_movi_i32(t, global);
+    gen_helper_check_tlb_flush(cpu_env, t);
     gen_set_label(l);
     tcg_temp_free_i32(t);
 }
 #else
-static inline void gen_check_tlb_flush(DisasContext *ctx) { }
+static inline void gen_check_tlb_flush(DisasContext *ctx, uint32_t global) { }
 #endif
 
 /* isync */
@@ -3092,7 +3093,7 @@ static void gen_isync(DisasContext *ctx)
      * kernel mode however so check MSR_PR
      */
     if (!ctx->pr) {
-        gen_check_tlb_flush(ctx);
+        gen_check_tlb_flush(ctx, 0);
     }
     gen_stop_exception(ctx);
 }
@@ -3257,7 +3258,7 @@ static void gen_sync(DisasContext *ctx)
      * check MSR_PR as well.
      */
     if (((l == 2) || !(ctx->insns_flags & PPC_64B)) && !ctx->pr) {
-        gen_check_tlb_flush(ctx);
+        gen_check_tlb_flush(ctx, 1);
     }
 }
 
@@ -4463,11 +4464,10 @@ static void gen_tlbsync(DisasContext *ctx)
 #else
     CHK_HV;
 
-    /* tlbsync is a nop for server, ptesync handles delayed tlb flush,
-     * embedded however needs to deal with tlbsync. We don't try to be
-     * fancy and swallow the overhead of checking for both.
-     */
-    gen_check_tlb_flush(ctx);
+    /* BookS does both ptesync and tlbsync make tlbsync a nop for server */
+    if (ctx->insns_flags & PPC_BOOKE) {
+        gen_check_tlb_flush(ctx, 1);
+    }
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
-- 
2.7.4

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

[Nikunj A Dadhania]

tlbie (H_REMOVE, H_PROTECT and H_BULK_REMOVE for pseries) should have a
global effect.

Introduces TLB_NEED_GLOBAL_FLUSH flag. During delayed flush, once taking
care of local flush, check broadcast flush(ptesync, tlbsync, etc) is
needed. Depending on the bitmask state of the tlb_need_flush, tlb is
flushed from other cpus if needed and the flags are cleared.

Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/ppc/spapr_hcall.c     |  2 ++
 target-ppc/cpu.h         |  1 +
 target-ppc/helper_regs.h | 19 +++++++++++++++++++
 target-ppc/mmu-hash64.c  |  2 +-
 target-ppc/mmu_helper.c  | 10 +++++++---
 target-ppc/translate.c   |  6 ++++++
 6 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index ef12ea0..6144e17 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -319,6 +319,8 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     ppc_hash64_store_hpte(cpu, pte_index,
                           (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
     ppc_hash64_tlb_flush_hpte(cpu, pte_index, v, r);
+    /* Flush the tlb */
+    check_tlb_flush(env, 1);
     /* Don't need a memory barrier, due to qemu's global lock */
     ppc_hash64_store_hpte(cpu, pte_index, v | HPTE64_V_HPTE_DIRTY, r);
     return H_SUCCESS;
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 71111dc..50fe0f5 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1010,6 +1010,7 @@ struct CPUPPCState {
     bool kvm_sw_tlb;  /* non-zero if KVM SW TLB API is active                */
     uint32_t tlb_need_flush; /* Delayed flush needed */
 #define TLB_NEED_LOCAL_FLUSH   0x1
+#define TLB_NEED_GLOBAL_FLUSH  0x2
 #endif
 
     /* Other registers */
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index bcf65ce..24872ca 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -154,6 +154,14 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
 }
 
 #if !defined(CONFIG_USER_ONLY)
+static inline void tlb_clear_flag(CPUState *cs)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+    env->tlb_need_flush = 0;
+}
+
 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
 static inline void check_tlb_flush(CPUPPCState *env, uint32_t global) { }
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 1f52b64..fdb7a78 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);
+    cpu->env.tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;
 }
 
 void ppc_hash64_update_rmls(CPUPPCState *env)
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index bf9f329..1dd057a 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2757,7 +2757,7 @@ static inline void booke206_invalidate_ea_tlb(CPUPPCState *env, int tlbn,
 
 void helper_booke206_tlbivax(CPUPPCState *env, target_ulong address)
 {
-    PowerPCCPU *cpu = ppc_env_get_cpu(env);
+    CPUState *cs;
 
     if (address & 0x4) {
         /* flush all entries */
@@ -2774,11 +2774,15 @@ void helper_booke206_tlbivax(CPUPPCState *env, target_ulong address)
     if (address & 0x8) {
         /* flush TLB1 entries */
         booke206_invalidate_ea_tlb(env, 1, address);
-        tlb_flush(CPU(cpu), 1);
+        CPU_FOREACH(cs) {
+            tlb_flush(cs, 1);
+        }
     } else {
         /* flush TLB0 entries */
         booke206_invalidate_ea_tlb(env, 0, address);
-        tlb_flush_page(CPU(cpu), address & MAS2_EPN_MASK);
+        CPU_FOREACH(cs) {
+            tlb_flush_page(cs, address & MAS2_EPN_MASK);
+        }
     }
 }
 
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 77bb312..2aae43b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4443,6 +4443,7 @@ static void gen_tlbie(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     GEN_PRIV;
 #else
+    TCGv_i32 t1;
     CHK_HV;
 
     if (NARROW_MODE(ctx)) {
@@ -4453,6 +4454,11 @@ static void gen_tlbie(DisasContext *ctx)
     } else {
         gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
     }
+    t1 = tcg_temp_new_i32();
+    tcg_gen_ld_i32(t1, cpu_env, offsetof(CPUPPCState, tlb_need_flush));
+    tcg_gen_ori_i32(t1, t1, TLB_NEED_GLOBAL_FLUSH);
+    tcg_gen_st_i32(t1, cpu_env, offsetof(CPUPPCState, tlb_need_flush));
+    tcg_temp_free_i32(t1);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
-- 
2.7.4

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

[Benjamin Herrenschmidt]

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

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

[David Gibson]

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

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

[Nikunj A Dadhania]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

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

When I wrote, i thought this would be used from various places. But dont
think its a requirement. Will inline it.

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

Right, will only clear the local flag.

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

Regards,
Nikunj