On 10/07/2015 7:58 pm, "Peter Maydell" <peter.mayd...@linaro.org> wrote:
>
> On 16 June 2015 at 02:51, Edgar E. Iglesias <edgar.igles...@gmail.com>
wrote:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> > ---
> > target-arm/cpu-qom.h | 1 +
> > target-arm/cpu.c | 2 ++
> > target-arm/cpu.h | 3 ++-
> > target-arm/helper.c | 68
++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 73 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index ed5a644..3aaa7b6 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -214,6 +214,7 @@ int arm_cpu_gdb_write_register(CPUState *cpu,
uint8_t *buf, int reg);
> > /* Callback functions for the generic timer's timers. */
> > void arm_gt_ptimer_cb(void *opaque);
> > void arm_gt_vtimer_cb(void *opaque);
> > +void arm_gt_htimer_cb(void *opaque);
> >
> > #ifdef TARGET_AARCH64
> > int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int
reg);
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 4a888ab..b631482 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -409,6 +409,8 @@ static void arm_cpu_initfn(Object *obj)
> > arm_gt_ptimer_cb, cpu);
> > cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL,
GTIMER_SCALE,
> > arm_gt_vtimer_cb, cpu);
> > + cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL,
GTIMER_SCALE,
> > + arm_gt_htimer_cb, cpu);
> > qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
> > ARRAY_SIZE(cpu->gt_timer_outputs));
> > #endif
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index f39c32b..dfa9d77 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -113,7 +113,8 @@ typedef struct ARMGenericTimer {
> >
> > #define GTIMER_PHYS 0
> > #define GTIMER_VIRT 1
> > -#define NUM_GTIMERS 2
> > +#define GTIMER_HYP 2
> > +#define NUM_GTIMERS 3
> >
> > typedef struct {
> > uint64_t raw_tcr;
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 92dbb28..32df2f5 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -1391,6 +1391,34 @@ static void gt_cntvoff_write(CPUARMState *env,
const ARMCPRegInfo *ri,
> > gt_recalc_timer(cpu, GTIMER_VIRT);
> > }
> >
> > +static void gt_hyp_cnt_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> > +{
> > + gt_cnt_reset(env, ri, GTIMER_HYP);
> > +}
> > + { .name = "CNTHP_TVAL_EL2", .state = ARM_CP_STATE_BOTH,
> > + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 0,
> > + .type = ARM_CP_IO, .access = PL2_RW,
> > + .resetfn = gt_hyp_cnt_reset,
> > + .readfn = gt_hyp_tval_read, .writefn = gt_hyp_tval_write },
>
> Something I just noticed while I was trying to add support
> for the secure physical timer on top of this series: the
> gt_*_cnt_reset functions are misnamed, because they're not
> resetting the counters, they're resetting the timers.
> (There are only two counters, physical and virtual, but there
> are four timers, physical, secure-physical, virtual and hyp.
> Since our reset function is deleting the underlying QEMU
> timer it's a timer reset, not a counter reset.)
> We should probably fix up the names and make sure they're
> associated with the correct registers (the phys and virt
> timer reset is currently hanging off a counter register).
>
> thanks
> -- PMM
Hi, yes that sounds good. Btw are you fixing this as you go or should I
send a new series fixing your comments? I've fixed the stuff you commented
on a few days ago in my tree...
Cheers,
Edgar
