Message ID | f43fc5580911080250t1f126ee6y63e5c07c78a30465@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Sun, Nov 08, 2009 at 12:50:21PM +0200, Blue Swirl wrote: > Both mmu_init() and mvp_init() allocate structures, so call cpu_mips_register > only when creating a CPU. > > In addition, maybe some of the some of the field initialization stuff > in cpu_mips_register, mmu_init, mvp_init, fpu_init etc. should be > moved to cpu_reset instead, in case the fields should be reset to > original values during CPU reset. Maximally only the env->mvp etc. > structure allocation would be left to cpu_mips_register. This is the > minimal version, but it may be incorrect. Comments? There is clearly a problem with some _init functions being called at each reset. However, your solution does not reset all the registers upon reset. osme of the registers are read-only (e.g. CP0_PRid) so it's not a problem, but some other are read-write (e.g. CP0_Config2). It looks like we need more code move to fix the problem. > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > --- > target-mips/translate.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index 58f483f..738efb7 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -8601,6 +8601,7 @@ CPUMIPSState *cpu_mips_init (const char *cpu_model) > > cpu_exec_init(env); > env->cpu_model_str = cpu_model; > + cpu_mips_register(env, def); > mips_tcg_init(); > cpu_reset(env); > qemu_init_vcpu(env); > @@ -8654,7 +8655,6 @@ void cpu_reset (CPUMIPSState *env) > env->hflags = MIPS_HFLAG_CP0; > #endif > env->exception_index = EXCP_NONE; > - cpu_mips_register(env, env->cpu_model); > } > > void gen_pc_load(CPUState *env, TranslationBlock *tb, > -- > 1.6.2.4 > > >
On Tue, Nov 10, 2009 at 04:30:59PM +0100, Aurelien Jarno wrote: > On Sun, Nov 08, 2009 at 12:50:21PM +0200, Blue Swirl wrote: > > Both mmu_init() and mvp_init() allocate structures, so call cpu_mips_register > > only when creating a CPU. > > > > In addition, maybe some of the some of the field initialization stuff > > in cpu_mips_register, mmu_init, mvp_init, fpu_init etc. should be > > moved to cpu_reset instead, in case the fields should be reset to > > original values during CPU reset. Maximally only the env->mvp etc. > > structure allocation would be left to cpu_mips_register. This is the > > minimal version, but it may be incorrect. Comments? > > There is clearly a problem with some _init functions being called at > each reset. However, your solution does not reset all the registers upon > reset. osme of the registers are read-only (e.g. CP0_PRid) so it's not a > problem, but some other are read-write (e.g. CP0_Config2). It looks like > we need more code move to fix the problem. > I have just pushed a patch that should fix the problem.
diff --git a/target-mips/translate.c b/target-mips/translate.c index 58f483f..738efb7 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -8601,6 +8601,7 @@ CPUMIPSState *cpu_mips_init (const char *cpu_model) cpu_exec_init(env); env->cpu_model_str = cpu_model; + cpu_mips_register(env, def); mips_tcg_init(); cpu_reset(env); qemu_init_vcpu(env); @@ -8654,7 +8655,6 @@ void cpu_reset (CPUMIPSState *env) env->hflags = MIPS_HFLAG_CP0; #endif env->exception_index = EXCP_NONE; - cpu_mips_register(env, env->cpu_model); } void gen_pc_load(CPUState *env, TranslationBlock *tb,
Both mmu_init() and mvp_init() allocate structures, so call cpu_mips_register only when creating a CPU. In addition, maybe some of the some of the field initialization stuff in cpu_mips_register, mmu_init, mvp_init, fpu_init etc. should be moved to cpu_reset instead, in case the fields should be reset to original values during CPU reset. Maximally only the env->mvp etc. structure allocation would be left to cpu_mips_register. This is the minimal version, but it may be incorrect. Comments? Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- target-mips/translate.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)