From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38508) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZDV4s-0005Wq-4g for qemu-devel@nongnu.org; Fri, 10 Jul 2015 05:58:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZDV4o-00081M-Lf for qemu-devel@nongnu.org; Fri, 10 Jul 2015 05:58:41 -0400 Received: from mail-vn0-f41.google.com ([209.85.216.41]:34607) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZDV4o-00081E-IH for qemu-devel@nongnu.org; Fri, 10 Jul 2015 05:58:38 -0400 Received: by vnbf7 with SMTP id f7so31163976vnb.1 for ; Fri, 10 Jul 2015 02:58:37 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1434419515-3572-5-git-send-email-edgar.iglesias@gmail.com> References: <1434419515-3572-1-git-send-email-edgar.iglesias@gmail.com> <1434419515-3572-5-git-send-email-edgar.iglesias@gmail.com> From: Peter Maydell Date: Fri, 10 Jul 2015 10:58:18 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v5 4/6] target-arm: Add the Hypervisor timer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Edgar E. Iglesias" Cc: Edgar Iglesias , Sergey Fedorov , =?UTF-8?B?QWxleCBCZW5uw6ll?= , QEMU Developers , Alexander Graf On 16 June 2015 at 02:51, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" > > Signed-off-by: Edgar E. Iglesias > --- > 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