[Qemu-devel] [PATCH v3 0/3] ppc: Broadcast tlb flush should have global effect

[Qemu-devel] [PATCH v3 0/3] ppc: Broadcast tlb flush should have global effect

[Nikunj A Dadhania]

PowerPC targets should do tlb invalidation on other cpus on 
instructions that expect a global effect.

* ptesync for BookS
* tlbsync primarily for BookE
  (for BookS make it a nop, as it always come along with ptesync)
* tlbivax for other ppc tragets
* H_REMOVE, H_BULK_REMOVE and H_PROTECT hcalls in case of pseries

The implementation provides a single point that can be used in MTTCG for 
doing async-flushes.

The patchset introduces bit-flags in CPUPPCState::tlb_need_flush:

  TLB_NEED_LOCAL_FLUSH (0x1) - Flush local tlb
  TLB_NEED_GLOBAL_FLUSH (0x2) - Flush tlb on other cpus.

Nikunj A Dadhania (3):
  target-ppc: add TLB_NEED_LOCAL_FLUSH flag
  target-ppc: add flag in chech_tlb_flush()
  target-ppc: tlbie should have global effect

 hw/ppc/spapr_hcall.c     |  6 ++++--
 target-ppc/cpu.h         |  2 ++
 target-ppc/excp_helper.c |  4 ++--
 target-ppc/helper.h      |  2 +-
 target-ppc/helper_regs.h | 25 +++++++++++++++++++++----
 target-ppc/mmu-hash64.c  |  6 +++---
 target-ppc/mmu_helper.c  | 20 ++++++++++++--------
 target-ppc/translate.c   | 26 ++++++++++++++++----------
 8 files changed, 61 insertions(+), 30 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v3 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 v3 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 e75d070..5ececf1 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 a27f455..5026804 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3066,7 +3066,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;
@@ -3078,12 +3078,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 */
@@ -3094,7 +3095,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);
 }
@@ -3259,7 +3260,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);
     }
 }
 
@@ -4468,11 +4469,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 v3 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 | 17 +++++++++++++++++
 target-ppc/mmu-hash64.c  |  2 +-
 target-ppc/mmu_helper.c  | 10 +++++++---
 target-ppc/translate.c   |  6 ++++++
 6 files changed, 34 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..fd2c961 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -161,6 +161,23 @@ static inline void check_tlb_flush(CPUPPCState *env, uint32_t global)
         tlb_flush(cs, 1);
         env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
     }
+
+    /* Propagate TLB invalidations to other CPUs when the guest uses broadcast
+     * TLB invalidation instructions.
+     */
+    if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
+        CPUState *other_cs;
+        CPU_FOREACH(other_cs) {
+            if (other_cs != cs) {
+                PowerPCCPU *cpu = POWERPC_CPU(other_cs);
+                CPUPPCState *other_env = &cpu->env;
+
+                other_env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
+                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 5026804..d96ff66 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4448,6 +4448,7 @@ static void gen_tlbie(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     GEN_PRIV;
 #else
+    TCGv_i32 t1;
     CHK_HV;
 
     if (NARROW_MODE(ctx)) {
@@ -4458,6 +4459,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 v3 0/3] ppc: Broadcast tlb flush should have global effect

[Benjamin Herrenschmidt]

On Mon, 2016-09-12 at 11:18 +0530, Nikunj A Dadhania wrote:
> PowerPC targets should do tlb invalidation on other cpus on 
> instructions that expect a global effect.
> 
> * ptesync for BookS
> * tlbsync primarily for BookE
>   (for BookS make it a nop, as it always come along with ptesync)
> * tlbivax for other ppc tragets
> * H_REMOVE, H_BULK_REMOVE and H_PROTECT hcalls in case of pseries

The above is confusing.

The instructions that have global effects are tlbie (BookS) and tlbivax
(BookE) plus the H_CALLs since they contain a tlbie on a real
hypervisor.

The ptesync and tlbsync instructions are barriers use to synchronize
with the global invalidations. That means that we don't need to ensure
the global invalidations have completed until we hit those barriers.

We typically use them as a way to "batch" the invalidations in TCG.

> The implementation provides a single point that can be used in MTTCG
> for 
> doing async-flushes.
> 
> The patchset introduces bit-flags in CPUPPCState::tlb_need_flush:
> 
>   TLB_NEED_LOCAL_FLUSH (0x1) - Flush local tlb
>   TLB_NEED_GLOBAL_FLUSH (0x2) - Flush tlb on other cpus.
> 
> Nikunj A Dadhania (3):
>   target-ppc: add TLB_NEED_LOCAL_FLUSH flag
>   target-ppc: add flag in chech_tlb_flush()
>   target-ppc: tlbie should have global effect
> 
>  hw/ppc/spapr_hcall.c     |  6 ++++--
>  target-ppc/cpu.h         |  2 ++
>  target-ppc/excp_helper.c |  4 ++--
>  target-ppc/helper.h      |  2 +-
>  target-ppc/helper_regs.h | 25 +++++++++++++++++++++----
>  target-ppc/mmu-hash64.c  |  6 +++---
>  target-ppc/mmu_helper.c  | 20 ++++++++++++--------
>  target-ppc/translate.c   | 26 ++++++++++++++++----------
>  8 files changed, 61 insertions(+), 30 deletions(-)
> 

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

[Benjamin Herrenschmidt]

On Mon, 2016-09-12 at 11:18 +0530, Nikunj A Dadhania wrote:
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 5026804..d96ff66 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -4448,6 +4448,7 @@ static void gen_tlbie(DisasContext *ctx)
>  #if defined(CONFIG_USER_ONLY)
>      GEN_PRIV;
>  #else
> +    TCGv_i32 t1;
>      CHK_HV;
>  
>      if (NARROW_MODE(ctx)) {
> @@ -4458,6 +4459,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) */

Why not do this in the helper ? 

Cheers,
Ben.

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

[Nikunj A Dadhania]

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

> On Mon, 2016-09-12 at 11:18 +0530, Nikunj A Dadhania wrote:
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 5026804..d96ff66 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -4448,6 +4448,7 @@ static void gen_tlbie(DisasContext *ctx)
>>  #if defined(CONFIG_USER_ONLY)
>>      GEN_PRIV;
>>  #else
>> +    TCGv_i32 t1;
>>      CHK_HV;
>>  
>>      if (NARROW_MODE(ctx)) {
>> @@ -4458,6 +4459,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) */
>
> Why not do this in the helper ? 

No particular reason though, I can do it there as well.

Regards
Nikunj

Re: [Qemu-devel] [PATCH v3 0/3] ppc: Broadcast tlb flush should have global effect

[Nikunj A Dadhania]

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

> On Mon, 2016-09-12 at 11:18 +0530, Nikunj A Dadhania wrote:
>> PowerPC targets should do tlb invalidation on other cpus on 
>> instructions that expect a global effect.
>> 
>> * ptesync for BookS
>> * tlbsync primarily for BookE
>>   (for BookS make it a nop, as it always come along with ptesync)
>> * tlbivax for other ppc tragets
>> * H_REMOVE, H_BULK_REMOVE and H_PROTECT hcalls in case of pseries
>
> The above is confusing.
>
> The instructions that have global effects are tlbie (BookS) and tlbivax
> (BookE) plus the H_CALLs since they contain a tlbie on a real
> hypervisor.
>
> The ptesync and tlbsync instructions are barriers use to synchronize
> with the global invalidations. That means that we don't need to ensure
> the global invalidations have completed until we hit those barriers.
>
> We typically use them as a way to "batch" the invalidations in TCG.

Sure, will updated. I mixed up the delayed flush with instructions.

Regards
Nikunj

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

[Nikunj A Dadhania]

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

> On Mon, 2016-09-12 at 11:18 +0530, Nikunj A Dadhania wrote:
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 5026804..d96ff66 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -4448,6 +4448,7 @@ static void gen_tlbie(DisasContext *ctx)
>>  #if defined(CONFIG_USER_ONLY)
>>      GEN_PRIV;
>>  #else
>> +    TCGv_i32 t1;
>>      CHK_HV;
>>  
>>      if (NARROW_MODE(ctx)) {
>> @@ -4458,6 +4459,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) */
>
> Why not do this in the helper ? 

Ah ok, now I recall: If I need to do this in helper:

* I will need to split tlbie and tlbiel
  OR
* Add a new argument in helper_tlbie

So, I thought of doing this in tcg.

Regards
Nikunj

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

[David Gibson]

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

On Mon, Sep 12, 2016 at 11:18:33AM +0530, Nikunj A Dadhania wrote:
> 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

This commit message needs a bunch more detail.  The flag indicates
whether a broadcast tlb flush is neede where exactly?  How does the
information in the parameter differ from the information in the
existing flags which check_tlb_flush() checks?

> 
> 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 e75d070..5ececf1 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 a27f455..5026804 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3066,7 +3066,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;
> @@ -3078,12 +3078,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 */
> @@ -3094,7 +3095,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);
>  }
> @@ -3259,7 +3260,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);
>      }
>  }
>  
> @@ -4468,11 +4469,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) */
>  }
>  

-- 
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 v3 2/3] target-ppc: add flag in chech_tlb_flush()

[Benjamin Herrenschmidt]

On Wed, 2016-09-14 at 13:09 +1000, David Gibson wrote:
> On Mon, Sep 12, 2016 at 11:18:33AM +0530, Nikunj A Dadhania wrote:
> > 
> > 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
> 
> This commit message needs a bunch more detail.  The flag indicates
> whether a broadcast tlb flush is neede where exactly?  How does the
> information in the parameter differ from the information in the
> existing flags which check_tlb_flush() checks?

The flag to check_tlb_flush() indicates that any pending broadcast
needs to be performed/completed. IE. it's set to 1 as a result of
an instruction (or an H_CALL) defined to synchronize broadcast tlbie's

> > 
> > 
> > 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 e75d070..5ececf1 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 a27f455..5026804 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -3066,7 +3066,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;
> > @@ -3078,12 +3078,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 */
> > @@ -3094,7 +3095,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);
> >  }
> > @@ -3259,7 +3260,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);
> >      }
> >  }
> >  
> > @@ -4468,11 +4469,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) */
> >  }
> >  
> 

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

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Mon, Sep 12, 2016 at 11:18:33AM +0530, Nikunj A Dadhania wrote:
>> 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
>
> This commit message needs a bunch more detail.  The flag indicates
> whether a broadcast tlb flush is neede where exactly?  How does the
> information in the parameter differ from the information in the
> existing flags which check_tlb_flush() checks?

How about the below:

Due to lazy tlb flushes, propagation of the tlb flush is delayed.
Moreover, certain operations need to do broadcast flush, this too can be
delayed until we hit the operation that warrant a broadcast.

Currently, the code only checks if local flush is necessary and does the
local flush. This global flag would inform check_tlb_flush that this
operation needs a broadcast flush. The patch just enables the routine
with the flag, succeeding patch will use this information along with
global_flush flag(set by tlbie/h_remove instruction) to broadcast the
tlb flushes.

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

Regards,
Nikunj

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

[Benjamin Herrenschmidt]

On Wed, 2016-09-14 at 09:23 +0530, Nikunj A Dadhania wrote:

Hr... this is confusing, let me rephrase ;-)

> Due to lazy tlb flushes, propagation of the tlb flush is delayed.
Moreover, certain operations need to do broadcast flush, this too can
be
> delayed until we hit the operation that warrant a broadcast.

Instead:

We flush the qemu TLB lazily. check_tlb_flush is called whenever we
hit a context synchronizing event or instruction that requires a pending
flush to be performed.

However, we fail to handle broadcast TLB flush operations. In order
to fix that efficiently, we want to differenciate whether check_tlb_flush()
needs to only apply pending local flushes (isync instructions,
interrupts, ...) or also global pending flush operations. The latter
is only needed when executing instructions that are defined architecturally
as synchronizing global TLB flush operations. This in our case is ptesync
on BookS and tlbsync on BookE along with the paravirtualized hypervisor
calls.

Cheers,
Ben.

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

[Nikunj A Dadhania]

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

> On Wed, 2016-09-14 at 09:23 +0530, Nikunj A Dadhania wrote:
>
> Hr... this is confusing, let me rephrase ;-)
>
>> Due to lazy tlb flushes, propagation of the tlb flush is delayed.
> Moreover, certain operations need to do broadcast flush, this too can
> be
>> delayed until we hit the operation that warrant a broadcast.
>
> Instead:
>
> We flush the qemu TLB lazily. check_tlb_flush is called whenever we
> hit a context synchronizing event or instruction that requires a pending
> flush to be performed.
>
> However, we fail to handle broadcast TLB flush operations. In order
> to fix that efficiently, we want to differenciate whether check_tlb_flush()
> needs to only apply pending local flushes (isync instructions,
> interrupts, ...) or also global pending flush operations. The latter
> is only needed when executing instructions that are defined architecturally
> as synchronizing global TLB flush operations. This in our case is ptesync
> on BookS and tlbsync on BookE along with the paravirtualized hypervisor
> calls.

Nice ;-)

Regards
Nikunj

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

[David Gibson]

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

On Wed, Sep 14, 2016 at 02:37:03PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2016-09-14 at 09:23 +0530, Nikunj A Dadhania wrote:
> 
> Hr... this is confusing, let me rephrase ;-)
> 
> > Due to lazy tlb flushes, propagation of the tlb flush is delayed.
> Moreover, certain operations need to do broadcast flush, this too can
> be
> > delayed until we hit the operation that warrant a broadcast.
> 
> Instead:
> 
> We flush the qemu TLB lazily. check_tlb_flush is called whenever we
> hit a context synchronizing event or instruction that requires a pending
> flush to be performed.
> 
> However, we fail to handle broadcast TLB flush operations. In order
> to fix that efficiently, we want to differenciate whether check_tlb_flush()
> needs to only apply pending local flushes (isync instructions,
> interrupts, ...) or also global pending flush operations. The latter
> is only needed when executing instructions that are defined architecturally
> as synchronizing global TLB flush operations. This in our case is ptesync
> on BookS and tlbsync on BookE along with the paravirtualized hypervisor
> calls.

Nice.

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