* [Qemu-devel] [PATCH RFC v1 2/3] target-ppc: add flag in chech_tlb_flush()
2016-09-09 10:45 [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania
@ 2016-09-09 10:45 ` Nikunj A Dadhania
2016-09-09 11:33 ` Benjamin Herrenschmidt
2016-09-09 10:45 ` [Qemu-devel] [PATCH RFC v1 3/3] target-ppc: tlbie should have global effect Nikunj A Dadhania
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Nikunj A Dadhania @ 2016-09-09 10:45 UTC (permalink / raw)
To: qemu-ppc, david, benh; +Cc: alex.bennee, qemu-devel, rth, nikunj
The flag will be used to indicate whether broadcast tlb flush is needed
or not.
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 | 13 +++++++------
6 files changed, 16 insertions(+), 15 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..09947e4 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, 1);
}
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, 1);
}
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 4457a30..fe20870 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) == 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 2498888..59dea8f 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..5f12c41 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);
}
}
@@ -4467,7 +4468,7 @@ static void gen_tlbsync(DisasContext *ctx)
* 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);
+ gen_check_tlb_flush(ctx, 1);
#endif /* defined(CONFIG_USER_ONLY) */
}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH RFC v1 2/3] target-ppc: add flag in chech_tlb_flush()
2016-09-09 10:45 ` [Qemu-devel] [PATCH RFC v1 2/3] target-ppc: add flag in chech_tlb_flush() Nikunj A Dadhania
@ 2016-09-09 11:33 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2016-09-09 11:33 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-ppc, david; +Cc: alex.bennee, qemu-devel, rth
On Fri, 2016-09-09 at 16:15 +0530, Nikunj A Dadhania wrote:
> The flag will be used to indicate whether broadcast tlb flush is
> needed
> or not.
>
> 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 | 13 +++++++------
> 6 files changed, 16 insertions(+), 15 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..09947e4 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, 1);
No that one is local
> }
>
> 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, 1);
> }
Same
> 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 4457a30..fe20870 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) ==
> 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 2498888..59dea8f 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..5f12c41 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);
> }
> }
>
> @@ -4467,7 +4468,7 @@ static void gen_tlbsync(DisasContext *ctx)
> * 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);
> + gen_check_tlb_flush(ctx, 1);
> #endif /* defined(CONFIG_USER_ONLY) */
> }
You may want to make that one a nop on BookS since it will do both
tlbsync and ptesync. BookE only does tlbsync.
Ben.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH RFC v1 3/3] target-ppc: tlbie should have global effect
2016-09-09 10:45 [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania
2016-09-09 10:45 ` [Qemu-devel] [PATCH RFC v1 2/3] target-ppc: add flag in chech_tlb_flush() Nikunj A Dadhania
@ 2016-09-09 10:45 ` Nikunj A Dadhania
2016-09-09 11:35 ` Benjamin Herrenschmidt
2016-09-09 11:30 ` [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Benjamin Herrenschmidt
2016-09-09 14:07 ` Alex Bennée
3 siblings, 1 reply; 10+ messages in thread
From: Nikunj A Dadhania @ 2016-09-09 10:45 UTC (permalink / raw)
To: qemu-ppc, david, benh; +Cc: alex.bennee, qemu-devel, rth, nikunj
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(-)
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) */
}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH RFC v1 3/3] target-ppc: tlbie should have global effect
2016-09-09 10:45 ` [Qemu-devel] [PATCH RFC v1 3/3] target-ppc: tlbie should have global effect Nikunj A Dadhania
@ 2016-09-09 11:35 ` Benjamin Herrenschmidt
2016-09-09 11:57 ` Nikunj A Dadhania
0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2016-09-09 11:35 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-ppc, david; +Cc: alex.bennee, qemu-devel, rth
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) */
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH RFC v1 3/3] target-ppc: tlbie should have global effect
2016-09-09 11:35 ` Benjamin Herrenschmidt
@ 2016-09-09 11:57 ` Nikunj A Dadhania
0 siblings, 0 replies; 10+ messages in thread
From: Nikunj A Dadhania @ 2016-09-09 11:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, qemu-ppc, david; +Cc: alex.bennee, qemu-devel, rth
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag
2016-09-09 10:45 [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania
2016-09-09 10:45 ` [Qemu-devel] [PATCH RFC v1 2/3] target-ppc: add flag in chech_tlb_flush() Nikunj A Dadhania
2016-09-09 10:45 ` [Qemu-devel] [PATCH RFC v1 3/3] target-ppc: tlbie should have global effect Nikunj A Dadhania
@ 2016-09-09 11:30 ` Benjamin Herrenschmidt
2016-09-09 14:07 ` Alex Bennée
3 siblings, 0 replies; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2016-09-09 11:30 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-ppc, david; +Cc: alex.bennee, qemu-devel, rth
On Fri, 2016-09-09 at 16:15 +0530, Nikunj A Dadhania wrote:
> > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
> target-ppc/cpu.h | 1 +
> target-ppc/helper_regs.h | 2 +-
> target-ppc/mmu-hash64.c | 4 ++--
> target-ppc/mmu_helper.c | 6 +++---
> 4 files changed, 7 insertions(+), 6 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..4457a30 100644
> --- a/target-ppc/helper_regs.h
> +++ b/target-ppc/helper_regs.h
> @@ -157,7 +157,7 @@ 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) {
> + if ((env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) == TLB_NEED_LOCAL_FLUSH) {
> env->tlb_need_flush = 0;
> tlb_flush(cs, 1);
> }
No. This should be
if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
tlb_flush(cs, 1);
env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
}
> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> index 8118143..4c7ceef 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;
> }
> }
> }
Should be
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;
> }
> }
ditto.
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index 696bb03..2498888 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;
again.
> #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;
again.
> #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
and one more.
> }
> }
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag
2016-09-09 10:45 [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania
` (2 preceding siblings ...)
2016-09-09 11:30 ` [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Benjamin Herrenschmidt
@ 2016-09-09 14:07 ` Alex Bennée
2016-09-09 15:13 ` Nikunj A Dadhania
2016-09-09 23:06 ` Benjamin Herrenschmidt
3 siblings, 2 replies; 10+ messages in thread
From: Alex Bennée @ 2016-09-09 14:07 UTC (permalink / raw)
To: Nikunj A Dadhania; +Cc: qemu-ppc, david, benh, qemu-devel, rth
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:
I think we need a little more detail here. In fact when you post the
next version of the series could you please include a cover letter to
cover what the series is trying to achieve?
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
> target-ppc/cpu.h | 1 +
> target-ppc/helper_regs.h | 2 +-
> target-ppc/mmu-hash64.c | 4 ++--
> target-ppc/mmu_helper.c | 6 +++---
> 4 files changed, 7 insertions(+), 6 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..4457a30 100644
> --- a/target-ppc/helper_regs.h
> +++ b/target-ppc/helper_regs.h
> @@ -157,7 +157,7 @@ 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) {
> + if ((env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) == TLB_NEED_LOCAL_FLUSH) {
> env->tlb_need_flush = 0;
> tlb_flush(cs, 1);
> }
> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> index 8118143..4c7ceef 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;
I'm not sure what we gain here versus just using a straight bool for the flag.
> }
> }
> }
> @@ -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..2498888 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
> }
> }
--
Alex Bennée
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag
2016-09-09 14:07 ` Alex Bennée
@ 2016-09-09 15:13 ` Nikunj A Dadhania
2016-09-09 23:06 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 10+ messages in thread
From: Nikunj A Dadhania @ 2016-09-09 15:13 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-ppc, david, benh, qemu-devel, rth
Alex Bennée <alex.bennee@linaro.org> writes:
> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:
>
> I think we need a little more detail here. In fact when you post the
> next version of the series could you please include a cover letter to
> cover what the series is trying to achieve?
Sure will do that.
>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>> target-ppc/cpu.h | 1 +
>> target-ppc/helper_regs.h | 2 +-
>> target-ppc/mmu-hash64.c | 4 ++--
>> target-ppc/mmu_helper.c | 6 +++---
>> 4 files changed, 7 insertions(+), 6 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..4457a30 100644
>> --- a/target-ppc/helper_regs.h
>> +++ b/target-ppc/helper_regs.h
>> @@ -157,7 +157,7 @@ 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) {
>> + if ((env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) == TLB_NEED_LOCAL_FLUSH) {
>> env->tlb_need_flush = 0;
>> tlb_flush(cs, 1);
>> }
>> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
>> index 8118143..4c7ceef 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;
>
> I'm not sure what we gain here versus just using a straight bool for the flag.
In the next patches I am adding TLB_NEED_GLOBAL_FLUSH, that is for
broadcast flush for other cpus.
TLB_NEED_LOCAL_FLUSH = 0x1
TLB_NEED_GLOBAL_FLUSH = 0x2
Regards
Nikunj
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag
2016-09-09 14:07 ` Alex Bennée
2016-09-09 15:13 ` Nikunj A Dadhania
@ 2016-09-09 23:06 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2016-09-09 23:06 UTC (permalink / raw)
To: Alex Bennée, Nikunj A Dadhania; +Cc: qemu-ppc, david, qemu-devel, rth
On Fri, 2016-09-09 at 15:07 +0100, Alex Bennée wrote:
> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:
>
> I think we need a little more detail here. In fact when you post the
> next version of the series could you please include a cover letter to
> cover what the series is trying to achieve?
In the meantime, for the readers, this is about fixing a problem
on TCG today (without MT-TCG) where we fail to properly propagate
TLB invalidations to other CPUs when we should (when the guest uses
boradcast TLB invalidation instructions).
The implementation also provides some ground work to make it easier to
plumb in the necessary MT-TCG additions.
>
> >
> > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> > ---
> > target-ppc/cpu.h | 1 +
> > target-ppc/helper_regs.h | 2 +-
> > target-ppc/mmu-hash64.c | 4 ++--
> > target-ppc/mmu_helper.c | 6 +++---
> > 4 files changed, 7 insertions(+), 6 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..4457a30 100644
> > --- a/target-ppc/helper_regs.h
> > +++ b/target-ppc/helper_regs.h
> > @@ -157,7 +157,7 @@ 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) {
> > + if ((env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) ==
> > TLB_NEED_LOCAL_FLUSH) {
> > env->tlb_need_flush = 0;
> > tlb_flush(cs, 1);
> > }
> > diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> > index 8118143..4c7ceef 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;
>
> I'm not sure what we gain here versus just using a straight bool for
> the flag.
>
> >
> > }
> > }
> > }
> > @@ -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..2498888 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
> > }
> > }
>
>
> --
> Alex Bennée
^ permalink raw reply [flat|nested] 10+ messages in thread