* [Qemu-devel] [PATCH v4 0/3] ppc: handle broadcast tlb flush
@ 2016-09-14 5:53 Nikunj A Dadhania
2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Nikunj A Dadhania @ 2016-09-14 5:53 UTC (permalink / raw)
To: qemu-ppc, david, benh; +Cc: alex.bennee, qemu-devel, rth, nikunj
PowerPC failed to handle broadcast TLB flush operations. Executing
instructions that are defined architecturally as synchronizing global TLB
should have a global effect.
* tlbie on BookS
* tlbivax on BookE
* H_CALLs (H_REMOVE, H_BULK_REMOVE and H_PROTECT) in case of pseries,
since they contain a tlbie on a real hypervisor.
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.
Changelog
v3:
* Updated commit messages and cover letter(benh)
Nikunj A Dadhania (3):
target-ppc: add TLB_NEED_LOCAL_FLUSH flag
target-ppc: add flag in chech_tlb_flush()
target-ppc: tlbie/tlbivax 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
^ permalink raw reply [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v4 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag 2016-09-14 5:53 [Qemu-devel] [PATCH v4 0/3] ppc: handle broadcast tlb flush Nikunj A Dadhania @ 2016-09-14 5:54 ` Nikunj A Dadhania 2016-09-15 0:15 ` David Gibson 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() Nikunj A Dadhania 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect Nikunj A Dadhania 2 siblings, 1 reply; 13+ messages in thread From: Nikunj A Dadhania @ 2016-09-14 5:54 UTC (permalink / raw) To: qemu-ppc, david, benh; +Cc: alex.bennee, qemu-devel, rth, nikunj 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania @ 2016-09-15 0:15 ` David Gibson 0 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2016-09-15 0:15 UTC (permalink / raw) To: Nikunj A Dadhania; +Cc: qemu-ppc, benh, alex.bennee, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 3929 bytes --] On Wed, Sep 14, 2016 at 11:24:00AM +0530, Nikunj A Dadhania wrote: You need some sort of commit message here. I'd ignore and apply anyway, except that there are some other things in later patches that will need a respin. > 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 > } > } -- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() 2016-09-14 5:53 [Qemu-devel] [PATCH v4 0/3] ppc: handle broadcast tlb flush Nikunj A Dadhania 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania @ 2016-09-14 5:54 ` Nikunj A Dadhania 2016-09-15 0:20 ` David Gibson 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect Nikunj A Dadhania 2 siblings, 1 reply; 13+ messages in thread From: Nikunj A Dadhania @ 2016-09-14 5:54 UTC (permalink / raw) To: qemu-ppc, david, benh; +Cc: alex.bennee, qemu-devel, rth, nikunj 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. 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() Nikunj A Dadhania @ 2016-09-15 0:20 ` David Gibson 2016-09-15 6:02 ` Nikunj A Dadhania 0 siblings, 1 reply; 13+ messages in thread From: David Gibson @ 2016-09-15 0:20 UTC (permalink / raw) To: Nikunj A Dadhania; +Cc: qemu-ppc, benh, alex.bennee, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 7438 bytes --] On Wed, Sep 14, 2016 at 11:24:01AM +0530, Nikunj A Dadhania wrote: > 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. Much better description, thank you. > > 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) You're using an unsigned int for the flag here, but uint32_t for check_tlb_flush(), which is a needless inconsistency. You might as well make them both bools, since that's how it's actually being used. As a general rule don't use fixed width types unless you actually *need* the fixed width - the type choices are part of the interface documentation and using a fixed width type when you don't need it sends a misleading message. > { > - 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() 2016-09-15 0:20 ` David Gibson @ 2016-09-15 6:02 ` Nikunj A Dadhania 2016-09-15 6:16 ` David Gibson 0 siblings, 1 reply; 13+ messages in thread From: Nikunj A Dadhania @ 2016-09-15 6:02 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, benh, alex.bennee, qemu-devel, rth David Gibson <david@gibson.dropbear.id.au> writes: > [ Unknown signature status ] > On Wed, Sep 14, 2016 at 11:24:01AM +0530, Nikunj A Dadhania wrote: >> 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. > > Much better description, thank you. > >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> --- >> 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) Not sure if I can use bool here, maybe I can use target_ulong. >> -void helper_check_tlb_flush(CPUPPCState *env) >> +void helper_check_tlb_flush(CPUPPCState *env, unsigned int global) > > You're using an unsigned int for the flag here, but uint32_t for > check_tlb_flush(), which is a needless inconsistency. I can make this as uint32_t for consistency. > You might as well make them both bools, since that's how it's actually > being used. > > As a general rule don't use fixed width types unless you > actually *need* the fixed width - the type choices are part of the > interface documentation and using a fixed width type when you don't > need it sends a misleading message. I optimized it because to avoid a new variable, and re-used "t": -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); } Regards Nikunj ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() 2016-09-15 6:02 ` Nikunj A Dadhania @ 2016-09-15 6:16 ` David Gibson 2016-09-15 6:22 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 13+ messages in thread From: David Gibson @ 2016-09-15 6:16 UTC (permalink / raw) To: Nikunj A Dadhania; +Cc: qemu-ppc, benh, alex.bennee, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 3503 bytes --] On Thu, Sep 15, 2016 at 11:32:39AM +0530, Nikunj A Dadhania wrote: > David Gibson <david@gibson.dropbear.id.au> writes: > > [ Unknown signature status ] > > On Wed, Sep 14, 2016 at 11:24:01AM +0530, Nikunj A Dadhania wrote: > >> 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. > > > > Much better description, thank you. > > > >> > >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > >> --- > > >> 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) > > Not sure if I can use bool here, maybe I can use target_ulong. I think target_ulong would make more sense. > >> -void helper_check_tlb_flush(CPUPPCState *env) > >> +void helper_check_tlb_flush(CPUPPCState *env, unsigned int global) > > > > You're using an unsigned int for the flag here, but uint32_t for > > check_tlb_flush(), which is a needless inconsistency. > > I can make this as uint32_t for consistency. As below, I'd prefer not. Actually I hadn't thought through the TCG helper constraints, so I think having it target_ulong in the helper and bool in the direct function makes sense. > > You might as well make them both bools, since that's how it's actually > > being used. > > > > As a general rule don't use fixed width types unless you > > actually *need* the fixed width - the type choices are part of the > > interface documentation and using a fixed width type when you don't > > need it sends a misleading message. > > I optimized it because to avoid a new variable, and re-used "t": Oh, I see. Hmm. I don't know if that will make a real difference in TCG or not. > -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); > } > > > Regards > Nikunj > -- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() 2016-09-15 6:16 ` David Gibson @ 2016-09-15 6:22 ` Benjamin Herrenschmidt 2016-09-15 6:25 ` David Gibson 0 siblings, 1 reply; 13+ messages in thread From: Benjamin Herrenschmidt @ 2016-09-15 6:22 UTC (permalink / raw) To: David Gibson, Nikunj A Dadhania; +Cc: qemu-ppc, alex.bennee, qemu-devel, rth On Thu, 2016-09-15 at 16:16 +1000, David Gibson wrote: > Oh, I see. Hmm. I don't know if that will make a real difference in > TCG or not. It will on 32-bit hosts. Cheers, Ben. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() 2016-09-15 6:22 ` Benjamin Herrenschmidt @ 2016-09-15 6:25 ` David Gibson 0 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2016-09-15 6:25 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Nikunj A Dadhania, qemu-ppc, alex.bennee, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 566 bytes --] On Thu, Sep 15, 2016 at 04:22:31PM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2016-09-15 at 16:16 +1000, David Gibson wrote: > > Oh, I see. Hmm. I don't know if that will make a real difference in > > TCG or not. > > It will on 32-bit hosts. Hm, yes, I guess it will. Ok, leave it has u32. I'd still prefer to see a bool in the direct-called version. -- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect 2016-09-14 5:53 [Qemu-devel] [PATCH v4 0/3] ppc: handle broadcast tlb flush Nikunj A Dadhania 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() Nikunj A Dadhania @ 2016-09-14 5:54 ` Nikunj A Dadhania 2016-09-15 0:25 ` David Gibson 2 siblings, 1 reply; 13+ messages in thread From: Nikunj A Dadhania @ 2016-09-14 5:54 UTC (permalink / raw) To: qemu-ppc, david, benh; +Cc: alex.bennee, qemu-devel, rth, nikunj tlbie (BookS) and tlbivax (BookE) plus the H_CALLs(pseries) should have a global effect. Introduces TLB_NEED_GLOBAL_FLUSH flag. During lazy tlb flush, after taking care of pending local flushes, check broadcast flush(at context synchronizing event 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect Nikunj A Dadhania @ 2016-09-15 0:25 ` David Gibson 2016-09-15 1:41 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 13+ messages in thread From: David Gibson @ 2016-09-15 0:25 UTC (permalink / raw) To: Nikunj A Dadhania; +Cc: qemu-ppc, benh, alex.bennee, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 5972 bytes --] On Wed, Sep 14, 2016 at 11:24:02AM +0530, Nikunj A Dadhania wrote: > tlbie (BookS) and tlbivax (BookE) plus the H_CALLs(pseries) should have > a global effect. > > Introduces TLB_NEED_GLOBAL_FLUSH flag. During lazy tlb flush, after > taking care of pending local flushes, check broadcast flush(at context > synchronizing event 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); > + } Why are these explicit CPU_FOREACH()s instead of using the flags you've just built? > } > } > > 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) */ > } > -- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect 2016-09-15 0:25 ` David Gibson @ 2016-09-15 1:41 ` Benjamin Herrenschmidt 2016-09-15 1:48 ` David Gibson 0 siblings, 1 reply; 13+ messages in thread From: Benjamin Herrenschmidt @ 2016-09-15 1:41 UTC (permalink / raw) To: David Gibson, Nikunj A Dadhania; +Cc: qemu-ppc, alex.bennee, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 1212 bytes --] On Thu, 2016-09-15 at 10:25 +1000, David Gibson wrote: > > 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); > > + } > > Why are these explicit CPU_FOREACH()s instead of using the flags > you've just bui Because we haven't converted BookE to lazy TLB flushing yet... Cheers, Ben. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 855 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect 2016-09-15 1:41 ` Benjamin Herrenschmidt @ 2016-09-15 1:48 ` David Gibson 0 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2016-09-15 1:48 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Nikunj A Dadhania, qemu-ppc, alex.bennee, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 1437 bytes --] On Thu, Sep 15, 2016 at 11:41:01AM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2016-09-15 at 10:25 +1000, David Gibson wrote: > > > 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); > > > + } > > > > Why are these explicit CPU_FOREACH()s instead of using the flags > > you've just bui > > Because we haven't converted BookE to lazy TLB flushing yet... Ah, right. -- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-09-15 6:28 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-14 5:53 [Qemu-devel] [PATCH v4 0/3] ppc: handle broadcast tlb flush Nikunj A Dadhania 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag Nikunj A Dadhania 2016-09-15 0:15 ` David Gibson 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 2/3] target-ppc: add flag in chech_tlb_flush() Nikunj A Dadhania 2016-09-15 0:20 ` David Gibson 2016-09-15 6:02 ` Nikunj A Dadhania 2016-09-15 6:16 ` David Gibson 2016-09-15 6:22 ` Benjamin Herrenschmidt 2016-09-15 6:25 ` David Gibson 2016-09-14 5:54 ` [Qemu-devel] [PATCH v4 3/3] target-ppc: tlbie/tlbivax should have global effect Nikunj A Dadhania 2016-09-15 0:25 ` David Gibson 2016-09-15 1:41 ` Benjamin Herrenschmidt 2016-09-15 1:48 ` David Gibson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.