On 14 September 2012 18:26, Blue Swirl <blauwir...@gmail.com> wrote: > On Wed, Sep 12, 2012 at 11:49 AM, Daniel Forsgren >> +++ qemu-linaro-1.1.50-2012.07-modified/target-arm/helper.c 2012-09-12 >> 13:15:45.544424842 +0200 >> @@ -1012,9 +1012,11 @@ void register_cp_regs_for_features(ARMCP >> if (arm_feature(env, ARM_FEATURE_THUMB2EE)) { >> define_arm_cp_regs(cpu, t2ee_cp_reginfo); >> } >> + /* > > Why would this be needed? > >> if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) { >> define_arm_cp_regs(cpu, generic_timer_cp_reginfo); >> } >> + */
At the moment helper.c defines a set of registers which RAZ/WI the entire crn=14 space where the generic timer lives (I forget why, probably because Linux probes for it); this bit of code is installing those dummy registers. The correct way to add the feature this patch is adding is to replace that dummy definition with the proper one, not merely to comment out this bit of code which installs the dummies. How exactly we should model this kind of device which is really an integral part of the CPU core (ie it is a set of cp15 registers, not memory mapped IO) but still has interrupt lines that connect into the GIC) is an interesting question. "This is a sysbus device" is even more dubious than for most of our sysbus devices :-) -- PMM