On Thu, Dec 15, 2016 at 12:36:55PM +0000, 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 <alex.ben...@linaro.org> > Reviewed-by: Richard Henderson <r...@twiddle.net> > --- > qom/cpu.c | 10 ++++++++-- > 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, 66 insertions(+), 52 deletions(-) > > diff --git a/qom/cpu.c b/qom/cpu.c > index 03d9190f8c..61ee0cb88c 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -270,8 +270,14 @@ static void cpu_common_reset(CPUState *cpu) > cpu->exception_index = -1; > cpu->crash_occurred = false; > > - for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) { > - atomic_set(&cpu->tb_jmp_cache[i], NULL); > + if (tcg_enabled()) { > + for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) { > + atomic_set(&cpu->tb_jmp_cache[i], NULL); > + } > + > +#ifdef CONFIG_SOFTMMU > + tlb_flush(cpu, 0); > +#endif
The patch is changing 3 things at the same time: 1) Moving the tb_jmp_cache reset inside if (tcg_enabled()) 2) Moving the tlb_flush() call to cpu_common_reset() 3) Moving the tlb_flush() call inside if (tcg_enabled()) Are we 100% sure there's no non-TCG looking looking at tlb_* fields or calling tlb_*() functions anywhere? Isn't it better to add the tcg_enabled() check in a separate patch? > } > } > [...] > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index de1f30eeda..810893952a 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2815,8 +2815,6 @@ static void x86_cpu_reset(CPUState *s) > > memset(env, 0, offsetof(CPUX86State, end_reset_fields)); > > - tlb_flush(s, 1); > - > env->old_exception = -1; > > /* init to reset state */ > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index c605724022..95ed91d8d1 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -1119,10 +1119,12 @@ typedef struct CPUX86State { > uint8_t nmi_injected; > uint8_t nmi_pending; > > + /* Fields up to this point are cleared by a CPU reset */ > + struct {} end_reset_fields; > + > CPU_COMMON > > - /* Fields from here on are preserved across CPU reset. */ > - struct {} end_reset_fields; > + /* Fields after CPU_COMMON are preserved across CPU reset. */ > > /* processor features (e.g. for CPUID insn) */ > /* Minimum level/xlevel/xlevel2, based on CPU model + features */ Do you have plans to move the tlb_* fields from CPUArchState to CPUState and remove CPU_COMMON completely? > diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c > index 8d939a7779..2b8c36b6d0 100644 > --- a/target-lm32/cpu.c > +++ b/target-lm32/cpu.c > @@ -128,10 +128,9 @@ static void lm32_cpu_reset(CPUState *s) > lcc->parent_reset(s); > > /* reset cpu state */ > - memset(env, 0, offsetof(CPULM32State, eba)); > + memset(env, 0, offsetof(CPULM32State, end_reset_fields)); > > lm32_cpu_init_cfg_reg(cpu); > - tlb_flush(s, 1); > } > [...] -- Eduardo