As far as I can tell tlb_flush does not need to be called
this early. tlb_flush is eventually called after the CPU
has been realized.
This change paves the way to the introduction of tlb_init,
which will be called from cpu_exec_realizefn.
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
target/alpha/cpu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index b08078e7fc..a953897fcc 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -201,7 +201,6 @@ static void alpha_cpu_initfn(Object *obj)
CPUAlphaState *env = &cpu->env;
cs->env_ptr = env;
- tlb_flush(cs);
env->lock_addr = -1;
#if defined(CONFIG_USER_ONLY)
--
2.17.1
On 9 October 2018 at 18:45, Emilio G. Cota <cota@braap.org> wrote: > As far as I can tell tlb_flush does not need to be called > this early. tlb_flush is eventually called after the CPU > has been realized. > > This change paves the way to the introduction of tlb_init, > which will be called from cpu_exec_realizefn. > > Signed-off-by: Emilio G. Cota <cota@braap.org> > --- > target/alpha/cpu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c > index b08078e7fc..a953897fcc 100644 > --- a/target/alpha/cpu.c > +++ b/target/alpha/cpu.c > @@ -201,7 +201,6 @@ static void alpha_cpu_initfn(Object *obj) > CPUAlphaState *env = &cpu->env; > > cs->env_ptr = env; > - tlb_flush(cs); > > env->lock_addr = -1; > #if defined(CONFIG_USER_ONLY) > -- > 2.17.1 Definitely agreed that we don't want to tlb_flush in the target cpu initfn. What's the codepath by which tlb_flush gets called on cpu reset? I had a quick look but couldn't find it... (The other dubious-looking bit of flushing in the target/alpha code is the code that generates calls to tb_flush()... we have very few calls to tb_flush outside the 'core' code and I suspect they could all be avoided.) thanks -- PMM
On Tue, Oct 09, 2018 at 18:55:30 +0100, Peter Maydell wrote:
> On 9 October 2018 at 18:45, Emilio G. Cota <cota@braap.org> wrote:
(snip)
> > @@ -201,7 +201,6 @@ static void alpha_cpu_initfn(Object *obj)
> > CPUAlphaState *env = &cpu->env;
> >
> > cs->env_ptr = env;
> > - tlb_flush(cs);
> >
> > env->lock_addr = -1;
> > #if defined(CONFIG_USER_ONLY)
> > --
> > 2.17.1
>
> Definitely agreed that we don't want to tlb_flush in the
> target cpu initfn.
>
>
> What's the codepath by which tlb_flush gets called on
> cpu reset? I had a quick look but couldn't find it...
From cpu.c:
static void cpu_common_reset(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
...
if (tcg_enabled()) {
cpu_tb_jmp_cache_clear(cpu);
tcg_flush_softmmu_tlb(cpu);
}
}
tcg_flush_softmmu_tlb is defined in translate-all.c:
/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
void tcg_flush_softmmu_tlb(CPUState *cs)
{
#ifdef CONFIG_SOFTMMU
tlb_flush(cs);
#endif
}
> (The other dubious-looking bit of flushing in the
> target/alpha code is the code that generates calls
> to tb_flush()... we have very few calls to tb_flush
> outside the 'core' code and I suspect they could all
> be avoided.)
If the comment below is accurate, seems fair enough.
tb_flush is only called from target/alpha through a helper,
generated by:
/* PALBR */
tcg_gen_st_i64(vb, cpu_env, offsetof(CPUAlphaState, palbr));
/* Changing the PAL base register implies un-chaining all of the TBs
that ended with a CALL_PAL. Since the base register usually only
changes during boot, flushing everything works well. */
gen_helper_tb_flush(cpu_env);
return DISAS_PC_STALE;
Thanks,
Emilio
On 9 October 2018 at 19:20, Emilio G. Cota <cota@braap.org> wrote:
> On Tue, Oct 09, 2018 at 18:55:30 +0100, Peter Maydell wrote:
>>
>> What's the codepath by which tlb_flush gets called on
>> cpu reset? I had a quick look but couldn't find it...
>
> From cpu.c:
>
> static void cpu_common_reset(CPUState *cpu)
> {
> CPUClass *cc = CPU_GET_CLASS(cpu);
> ...
> if (tcg_enabled()) {
> cpu_tb_jmp_cache_clear(cpu);
>
> tcg_flush_softmmu_tlb(cpu);
> }
> }
>
> tcg_flush_softmmu_tlb is defined in translate-all.c:
>
> /* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
> void tcg_flush_softmmu_tlb(CPUState *cs)
> {
> #ifdef CONFIG_SOFTMMU
> tlb_flush(cs);
> #endif
> }
Ah, thank you. I missed this because of the indirection
via tcg_flush_softmmu_tlb().
>> (The other dubious-looking bit of flushing in the
>> target/alpha code is the code that generates calls
>> to tb_flush()... we have very few calls to tb_flush
>> outside the 'core' code and I suspect they could all
>> be avoided.)
>
> If the comment below is accurate, seems fair enough.
> tb_flush is only called from target/alpha through a helper,
> generated by:
>
> /* PALBR */
> tcg_gen_st_i64(vb, cpu_env, offsetof(CPUAlphaState, palbr));
> /* Changing the PAL base register implies un-chaining all of the TBs
> that ended with a CALL_PAL. Since the base register usually only
> changes during boot, flushing everything works well. */
> gen_helper_tb_flush(cpu_env);
> return DISAS_PC_STALE;
Mmm, it works (though would it work if the CPU wasn't the
only one in the system?). I just have a reflexive dislike
of design approaches used by only one thing -- I tend to
like to smooth them out so there's more consistency...
thanks
-- PMM
On 10/9/18 11:25 AM, Peter Maydell wrote: >> /* PALBR */ >> tcg_gen_st_i64(vb, cpu_env, offsetof(CPUAlphaState, palbr)); >> /* Changing the PAL base register implies un-chaining all of the TBs >> that ended with a CALL_PAL. Since the base register usually only >> changes during boot, flushing everything works well. */ >> gen_helper_tb_flush(cpu_env); >> return DISAS_PC_STALE; > > Mmm, it works (though would it work if the CPU wasn't the > only one in the system?). I just have a reflexive dislike > of design approaches used by only one thing -- I tend to > like to smooth them out so there's more consistency... It could be less important now that we have goto_ptr. If you really think I should get rid of it, I will. r~
Emilio G. Cota <cota@braap.org> writes: > As far as I can tell tlb_flush does not need to be called > this early. tlb_flush is eventually called after the CPU > has been realized. > > This change paves the way to the introduction of tlb_init, > which will be called from cpu_exec_realizefn. > > Signed-off-by: Emilio G. Cota <cota@braap.org> > --- > target/alpha/cpu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c > index b08078e7fc..a953897fcc 100644 > --- a/target/alpha/cpu.c > +++ b/target/alpha/cpu.c > @@ -201,7 +201,6 @@ static void alpha_cpu_initfn(Object *obj) > CPUAlphaState *env = &cpu->env; > > cs->env_ptr = env; > - tlb_flush(cs); > > env->lock_addr = -1; > #if defined(CONFIG_USER_ONLY) Reviewed-by: Alex Bennée <alex.bennee@linaro.org> -- Alex Bennée
On 10/9/18 10:45 AM, Emilio G. Cota wrote: > As far as I can tell tlb_flush does not need to be called > this early. tlb_flush is eventually called after the CPU > has been realized. > > This change paves the way to the introduction of tlb_init, > which will be called from cpu_exec_realizefn. > > Signed-off-by: Emilio G. Cota <cota@braap.org> > --- > target/alpha/cpu.c | 1 - > 1 file changed, 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
© 2016 - 2026 Red Hat, Inc.