Re: [Qemu-devel] [PATCH v3 1/3] qom/cpu: move tlb_flush to cpu_common_reset
On Thu, Jan 12, 2017 at 03:47:29PM +, Alex Bennée wrote: > It is a common thing amongst the various cpu reset functions want to > flush the SoftMMU's TLB entries. This is done either by calling > tlb_flush directly or by way of a general memset of the CPU > structure (sometimes both). > > This moves the tlb_flush call to the common reset function and > additionally ensures it is only done for the CONFIG_SOFTMMU case and > when tcg is enabled. > > In some target cases we add an empty end_of_reset_fields structure to the > target vCPU structure so have a clear end point for any memset which > is resetting value in the structure before CPU_COMMON (where the TLB > structures are). > > While this is a nice clean-up in general it is also a precursor for > changes coming to cputlb for MTTCG where the clearing of entries > can't be done arbitrarily across vCPUs. Currently the cpu_reset > function is usually called from the context of another vCPU as the > architectural power up sequence is run. By using the cputlb API > functions we can ensure the right behaviour in the future. > > Signed-off-by: Alex Bennée> Reviewed-by: Richard Henderson For ppc: Reviewed-by: David Gibson > > --- > v3: > - split tcg_enabled() into a separate patch > --- > qom/cpu.c | 4 > target/arm/cpu.c| 5 ++--- > target/arm/cpu.h| 5 - > target/cris/cpu.c | 3 +-- > target/cris/cpu.h | 9 ++--- > target/i386/cpu.c | 2 -- > target/i386/cpu.h | 6 -- > target/lm32/cpu.c | 3 +-- > target/lm32/cpu.h | 3 +++ > target/m68k/cpu.c | 3 +-- > target/m68k/cpu.h | 3 +++ > target/microblaze/cpu.c | 3 +-- > target/microblaze/cpu.h | 3 +++ > target/mips/cpu.c | 3 +-- > target/mips/cpu.h | 3 +++ > target/moxie/cpu.c | 4 +--- > target/moxie/cpu.h | 3 +++ > target/openrisc/cpu.c | 9 + > target/openrisc/cpu.h | 3 +++ > target/ppc/translate_init.c | 3 --- > target/s390x/cpu.c | 7 ++- > target/s390x/cpu.h | 5 +++-- > target/sh4/cpu.c| 3 +-- > target/sh4/cpu.h| 3 +++ > target/sparc/cpu.c | 3 +-- > target/sparc/cpu.h | 3 +++ > target/tilegx/cpu.c | 3 +-- > target/tilegx/cpu.h | 3 +++ > target/tricore/cpu.c| 2 -- > 29 files changed, 62 insertions(+), 50 deletions(-) > > diff --git a/qom/cpu.c b/qom/cpu.c > index 03d9190f8c..cc51de2a8c 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -273,6 +273,10 @@ static void cpu_common_reset(CPUState *cpu) > for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) { > atomic_set(>tb_jmp_cache[i], NULL); > } > + > +#ifdef CONFIG_SOFTMMU > +tlb_flush(cpu, 0); > +#endif > } > > static bool cpu_common_has_work(CPUState *cs) > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index f5cb30af6c..91046111d9 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -122,7 +122,8 @@ static void arm_cpu_reset(CPUState *s) > > acc->parent_reset(s); > > -memset(env, 0, offsetof(CPUARMState, features)); > +memset(env, 0, offsetof(CPUARMState, end_reset_fields)); > + > g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu); > g_hash_table_foreach(cpu->cp_regs, cp_reg_check_reset, cpu); > > @@ -226,8 +227,6 @@ static void arm_cpu_reset(CPUState *s) >>vfp.fp_status); > set_float_detect_tininess(float_tininess_before_rounding, >>vfp.standard_fp_status); > -tlb_flush(s, 1); > - > #ifndef CONFIG_USER_ONLY > if (kvm_enabled()) { > kvm_arm_reset_vcpu(cpu); > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index ab119e62ab..7bd16eec18 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -491,9 +491,12 @@ typedef struct CPUARMState { > struct CPUBreakpoint *cpu_breakpoint[16]; > struct CPUWatchpoint *cpu_watchpoint[16]; > > +/* Fields up to this point are cleared by a CPU reset */ > +struct {} end_reset_fields; > + > CPU_COMMON > > -/* These fields after the common ones so they are preserved on reset. */ > +/* Fields after CPU_COMMON are preserved across CPU reset. */ > > /* Internal CPU feature flags. */ > uint64_t features; > diff --git a/target/cris/cpu.c b/target/cris/cpu.c > index 2e9ab9700e..5f766f09d6 100644 > --- a/target/cris/cpu.c > +++ b/target/cris/cpu.c > @@ -52,9 +52,8 @@ static void cris_cpu_reset(CPUState *s) > ccc->parent_reset(s); > > vr = env->pregs[PR_VR]; > -memset(env, 0, offsetof(CPUCRISState, load_info)); > +memset(env, 0, offsetof(CPUCRISState, end_reset_fields)); > env->pregs[PR_VR] = vr; > -tlb_flush(s, 1); > > #if defined(CONFIG_USER_ONLY) > /* start in user mode with interrupts enabled. */ > diff
Re: [Qemu-devel] [PATCH v3 1/3] qom/cpu: move tlb_flush to cpu_common_reset
On Thu, Jan 12, 2017 at 03:47:29PM +, Alex Bennée wrote: > It is a common thing amongst the various cpu reset functions want to > flush the SoftMMU's TLB entries. This is done either by calling > tlb_flush directly or by way of a general memset of the CPU > structure (sometimes both). > > This moves the tlb_flush call to the common reset function and > additionally ensures it is only done for the CONFIG_SOFTMMU case and > when tcg is enabled. > > In some target cases we add an empty end_of_reset_fields structure to the > target vCPU structure so have a clear end point for any memset which > is resetting value in the structure before CPU_COMMON (where the TLB > structures are). > > While this is a nice clean-up in general it is also a precursor for > changes coming to cputlb for MTTCG where the clearing of entries > can't be done arbitrarily across vCPUs. Currently the cpu_reset > function is usually called from the context of another vCPU as the > architectural power up sequence is run. By using the cputlb API > functions we can ensure the right behaviour in the future. > > Signed-off-by: Alex Bennée> Reviewed-by: Richard Henderson For i386: Reviewed-by: Eduardo Habkost -- Eduardo
[Qemu-devel] [PATCH v3 1/3] qom/cpu: move tlb_flush to cpu_common_reset
It is a common thing amongst the various cpu reset functions want to flush the SoftMMU's TLB entries. This is done either by calling tlb_flush directly or by way of a general memset of the CPU structure (sometimes both). This moves the tlb_flush call to the common reset function and additionally ensures it is only done for the CONFIG_SOFTMMU case and when tcg is enabled. In some target cases we add an empty end_of_reset_fields structure to the target vCPU structure so have a clear end point for any memset which is resetting value in the structure before CPU_COMMON (where the TLB structures are). While this is a nice clean-up in general it is also a precursor for changes coming to cputlb for MTTCG where the clearing of entries can't be done arbitrarily across vCPUs. Currently the cpu_reset function is usually called from the context of another vCPU as the architectural power up sequence is run. By using the cputlb API functions we can ensure the right behaviour in the future. Signed-off-by: Alex BennéeReviewed-by: Richard Henderson --- v3: - split tcg_enabled() into a separate patch --- qom/cpu.c | 4 target/arm/cpu.c| 5 ++--- target/arm/cpu.h| 5 - target/cris/cpu.c | 3 +-- target/cris/cpu.h | 9 ++--- target/i386/cpu.c | 2 -- target/i386/cpu.h | 6 -- target/lm32/cpu.c | 3 +-- target/lm32/cpu.h | 3 +++ target/m68k/cpu.c | 3 +-- target/m68k/cpu.h | 3 +++ target/microblaze/cpu.c | 3 +-- target/microblaze/cpu.h | 3 +++ target/mips/cpu.c | 3 +-- target/mips/cpu.h | 3 +++ target/moxie/cpu.c | 4 +--- target/moxie/cpu.h | 3 +++ target/openrisc/cpu.c | 9 + target/openrisc/cpu.h | 3 +++ target/ppc/translate_init.c | 3 --- target/s390x/cpu.c | 7 ++- target/s390x/cpu.h | 5 +++-- target/sh4/cpu.c| 3 +-- target/sh4/cpu.h| 3 +++ target/sparc/cpu.c | 3 +-- target/sparc/cpu.h | 3 +++ target/tilegx/cpu.c | 3 +-- target/tilegx/cpu.h | 3 +++ target/tricore/cpu.c| 2 -- 29 files changed, 62 insertions(+), 50 deletions(-) diff --git a/qom/cpu.c b/qom/cpu.c index 03d9190f8c..cc51de2a8c 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -273,6 +273,10 @@ static void cpu_common_reset(CPUState *cpu) for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) { atomic_set(>tb_jmp_cache[i], NULL); } + +#ifdef CONFIG_SOFTMMU +tlb_flush(cpu, 0); +#endif } static bool cpu_common_has_work(CPUState *cs) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index f5cb30af6c..91046111d9 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -122,7 +122,8 @@ static void arm_cpu_reset(CPUState *s) acc->parent_reset(s); -memset(env, 0, offsetof(CPUARMState, features)); +memset(env, 0, offsetof(CPUARMState, end_reset_fields)); + g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu); g_hash_table_foreach(cpu->cp_regs, cp_reg_check_reset, cpu); @@ -226,8 +227,6 @@ static void arm_cpu_reset(CPUState *s) >vfp.fp_status); set_float_detect_tininess(float_tininess_before_rounding, >vfp.standard_fp_status); -tlb_flush(s, 1); - #ifndef CONFIG_USER_ONLY if (kvm_enabled()) { kvm_arm_reset_vcpu(cpu); diff --git a/target/arm/cpu.h b/target/arm/cpu.h index ab119e62ab..7bd16eec18 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -491,9 +491,12 @@ typedef struct CPUARMState { struct CPUBreakpoint *cpu_breakpoint[16]; struct CPUWatchpoint *cpu_watchpoint[16]; +/* Fields up to this point are cleared by a CPU reset */ +struct {} end_reset_fields; + CPU_COMMON -/* These fields after the common ones so they are preserved on reset. */ +/* Fields after CPU_COMMON are preserved across CPU reset. */ /* Internal CPU feature flags. */ uint64_t features; diff --git a/target/cris/cpu.c b/target/cris/cpu.c index 2e9ab9700e..5f766f09d6 100644 --- a/target/cris/cpu.c +++ b/target/cris/cpu.c @@ -52,9 +52,8 @@ static void cris_cpu_reset(CPUState *s) ccc->parent_reset(s); vr = env->pregs[PR_VR]; -memset(env, 0, offsetof(CPUCRISState, load_info)); +memset(env, 0, offsetof(CPUCRISState, end_reset_fields)); env->pregs[PR_VR] = vr; -tlb_flush(s, 1); #if defined(CONFIG_USER_ONLY) /* start in user mode with interrupts enabled. */ diff --git a/target/cris/cpu.h b/target/cris/cpu.h index 43d5f9d1da..920e1c33ba 100644 --- a/target/cris/cpu.h +++ b/target/cris/cpu.h @@ -167,10 +167,13 @@ typedef struct CPUCRISState { */ TLBSet tlbsets[2][4][16]; - CPU_COMMON +/* Fields up to this point are cleared by a CPU reset */ +struct {} end_reset_fields; -/* Members from load_info on are