diff mbox

[RFC,v1,3/3] target-ppc: tlbie should have global effect

Message ID 1473417926-14263-3-git-send-email-nikunj@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nikunj A. Dadhania Sept. 9, 2016, 10:45 a.m. UTC 
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     |  3 +++
 target-ppc/cpu.h         |  1 +
 target-ppc/helper_regs.h | 23 ++++++++++++++++++++++-
 target-ppc/mmu-hash64.c  |  2 +-
 target-ppc/mmu_helper.c  | 10 +++++++---
 target-ppc/translate.c   |  6 ++++++
 6 files changed, 40 insertions(+), 5 deletions(-)

Comments

Benjamin Herrenschmidt Sept. 9, 2016, 11:35 a.m. UTC | #1 
On Fri, 2016-09-09 at 16:15 +0530, Nikunj A Dadhania wrote:
> 

> +    env->tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;

> check_tlb_flush(env th, 1);

> 


Hrm... how did that work bore ? IE. check_tlb_flush won't do anything
if tlb_need_flush is 0, isn't it already set elsewhere ? If not, that's a
bug in the existing bug that I'd suggest you fix separately. If it is, then
this only needs to OR-in TLB_NEED_GLOBAL_FLUSH and only if the hash entry
was found, it, at the point where tlb_need_flush was originally set

>      return r

> @@ -319,6 +320,8 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,

>      ppc_hash64_store_hpte(cpu, pte_index,

>                            (v & ~HPTE64_V_VALID) | H.PTE64_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 fe20870..f18b2ff 100644

> --- a/target-ppc/helper_regs.h

> +++ b/target-ppc/helper_regs.h

> @@ -154,13 +154,34 @@ 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));

> +    CPUState *other_cs;

> +

>      if ((env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) == TLB_NEED_LOCAL_FLUSH) {

> -        env->tlb_need_flush = 0;

>          tlb_flush(cs, 1);

>      }

> +

> +    if (global &&

> +        (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH) == TLB_NEED_GLOBAL_FLUSH)

> +    {

> +        CPU_FOREACH(other_cs) {

> +            if (other_cs != cs) {

> +                tlb_clear_flag(other_cs);

> +                tlb_flush(other_cs, 1);

> +            }

> +        }

> +    }

> +    env->tlb_need_flush = 0;

>  }

>  #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 4c7ceef..f582efe 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 59dea8f..aee272f 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 5f12c41..d3cfbc8 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) */

>  }

>
Nikunj A. Dadhania Sept. 9, 2016, 11:57 a.m. UTC | #2 
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Fri, 2016-09-09 at 16:15 +0530, Nikunj A Dadhania wrote:
>> 
>> +    env->tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;
>> check_tlb_flush(env th, 1);
>> 
>
> Hrm... how did that work bore ? IE. check_tlb_flush won't do anything
> if tlb_need_flush is 0, isn't it already set elsewhere ?

Error of my judgement, it is set in remove_hpte, so the above condition
is not required. Good catch.


Regards
Nikunj
diff mbox

Patch

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index ef12ea0..19fbae6 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -282,6 +282,7 @@  static target_ulong h_bulk_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         }
     }
  exit:
+    env->tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;
     check_tlb_flush(env, 1);
 
     return rc;
@@ -319,6 +320,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 fe20870..f18b2ff 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -154,13 +154,34 @@  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));
+    CPUState *other_cs;
+
     if ((env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) == TLB_NEED_LOCAL_FLUSH) {
-        env->tlb_need_flush = 0;
         tlb_flush(cs, 1);
     }
+
+    if (global &&
+        (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH) == TLB_NEED_GLOBAL_FLUSH)
+    {
+        CPU_FOREACH(other_cs) {
+            if (other_cs != cs) {
+                tlb_clear_flag(other_cs);
+                tlb_flush(other_cs, 1);
+            }
+        }
+    }
+    env->tlb_need_flush = 0;
 }
 #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 4c7ceef..f582efe 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 59dea8f..aee272f 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 5f12c41..d3cfbc8 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) */
 }