On 30 April 2012 17:21, Paul Brook <p...@codesourcery.com> wrote: >> Move feature register value setup to per-CPU init functions. > >> + env->cp15.c0_c1[0] = cpu->id_pfr0; >> + env->cp15.c0_c1[1] = cpu->id_pfr1; >> + env->cp15.c0_c1[2] = cpu->id_dfr0; >> + env->cp15.c0_c1[3] = cpu->id_afr0; >> + env->cp15.c0_c1[4] = cpu->id_mmfr0; >> + env->cp15.c0_c1[5] = cpu->id_mmfr1; >> + env->cp15.c0_c1[6] = cpu->id_mmfr2; >> + env->cp15.c0_c1[7] = cpu->id_mmfr3; >> + env->cp15.c0_c2[0] = cpu->id_isar0; >> + env->cp15.c0_c2[1] = cpu->id_isar1; >> + env->cp15.c0_c2[2] = cpu->id_isar2; >> + env->cp15.c0_c2[3] = cpu->id_isar3; >> + env->cp15.c0_c2[4] = cpu->id_isar4; >> + env->cp15.c0_c2[5] = cpu->id_isar5; > > Why are we copying these values? All these registers are readonly, so the > duplication seems wrong. Shouldn't we should be using cpu->whatever > everywhere? > > I feel like I've asked this before, but don't remember seeing an answer.
My proposed cp15 rework deletes this copying code; instead the registers are defined with values set directly from the cpu->whatever fields: http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01811.html > Also, I'd prefer that id_isr5 were explicitly initialized, rather than relying > on it being implicitly zero. Bugs in an earlier patch series show how easy it > is to accidentally miss a register. IMO it's worth distinguishing a defined > register that happens to be zero from a register this core doesn't have. Yes, I think that's probably a good idea. -- PMM