Re: [Qemu-devel] [PATCH v3 1/3] arm_generic_timer: Add the ARM Generic Timer

[Alistair Francis]

On Tue, Jan 10, 2017 at 1:42 AM, Peter Maydell  wrote:
> On 10 January 2017 at 01:41, Alistair Francis
>  wrote:
>> On Fri, Jan 6, 2017 at 3:57 AM, Peter Maydell  
>> wrote:
>>> On 20 December 2016 at 22:42, Alistair Francis
>>>  wrote:
>
 +},{ .name = "CNTCV_LOWER",
 +.addr = A_CNTCV_LOWER,
 +.post_read = counter_low_value_postr,
 +},{ .name = "CNTCV_UPPER",
 +.addr = A_CNTCV_UPPER,
 +.post_read = counter_high_value_postr,
>>>
>>> Spec says that you should also be able to access the whole
>>> 64-bit counter value with a 64-bit access -- is this reginfo
>>> sufficient to make that work?
>>
>> How do you know, all I see if that it is a 64-bit register. All the
>> documentation about accesses specifies only 32-bit accesses.
>
> v8 ARM ARM DDI0487A.k, I3.5.3 (CNTCV register description):
> "In an implementation that supports 64-bit atomic memory accesses,
> this register must be accessible using a 64-bit atomic access."

Ok, I don't even have that section. I'm going to try to find an updated ARM ARM.

>
>
>>> This means you can't use this device in a system which
>>> doesn't implement TrustZone. I think this would be better
>>> handled by just having the board map the memory region
>>> in to only the Secure address space if it is a TZ-aware board.
>>
>> How can I map something into the secure address space? Is there an
>> example of this? The only think I can find is in the arm/virt.c
>> machine with the secure_sysmem MemoryRegion.
>
> Yeah, virt.c is the place to look -- for instance we put one
> of the UARTs in the secure address space only.
> Basically to support separate address spaces:
>  * create a MemoryRegion to be the secure view of the world
>  * add the NS MemoryRegion to it as a subregion with low
>priority (so S sees all the NS devices -- this isn't
>required but that's usually how h/w is set up)
>  * use object_property_set_link() to connect the S memory
>region to each CPU's secure-memory property
>  * add any S-only devices to the S MemoryRegion

Ok, sounds easy enough.

>
 +REG32(CNTCR, 0x00)
 +FIELD(CNTCR, EN, 0, 1)
 +FIELD(CNTCR, HDBG, 1, 1)
 +REG32(CNTSR, 0x04)
 +FIELD(CNTSR, DBGH, 1, 1)
 +REG32(CNTCV_LOWER, 0x08)
 +REG32(CNTCV_UPPER, 0x0C)
 +REG32(CNTFID0, 0x20)
 +/* We don't model CNTFIDn */
 +/* We don't model the CounterID registers either */
>>>
>>> Does the Xilinx h/w not implement them at all?
>>> (ie do we need a property for "device has the ID regs" if we add them
>>> later?)
>>
>> There is nothing in the register spec describing registers being here.
>>
>> The last register I see is called: (IOU_SCNTRS)
>> base_frequency_ID_register at address 0xFF260020.
>
> I suspect there's an implicit reads-as-zero after that.
> (The frequency table has to have at least one real entry
> plus the zero terminator -- see I3.5.5 (CNTFID0).)
>
> QEMU's implementation will only support one frequency so
> we only need one table entry too (for the moment, anyway).
>
>>> The spec says it's mandatory to have at least the entry for
>>> the counter base frequency plus end-of-table marker.
>>> I would expect EL3 firmware to want to read this table in order
>>> to write the correct values to CNTFRQ_EL0 for each CPU.
>>
>> I don't see anything like that in section I3.5.21 in the ARM ARM,
>> where are you looking?
>
> There's no I3.5.21 in my copy of the spec (rev A.k), I suspect
> you have an older rev. I think the generic timer docs got
> revamped at some point, so you might want to grab the latest rev.
>
> Anyway, it looks like you have got CNTFIDn (that's CNTFID0)
> and an implicit undocumented zero-terminator.
>
> The spec says CounterIDn are IMPDEF, which I guess is why
> your implementation doesn't bother to implement them.

I do have an out of date version. I'll find a newer one and update the
registers to follow that.

Thanks,

Alistair

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v3 1/3] arm_generic_timer: Add the ARM Generic Timer

[Peter Maydell]

On 10 January 2017 at 01:41, Alistair Francis
 wrote:
> On Fri, Jan 6, 2017 at 3:57 AM, Peter Maydell  
> wrote:
>> On 20 December 2016 at 22:42, Alistair Francis
>>  wrote:

>>> +},{ .name = "CNTCV_LOWER",
>>> +.addr = A_CNTCV_LOWER,
>>> +.post_read = counter_low_value_postr,
>>> +},{ .name = "CNTCV_UPPER",
>>> +.addr = A_CNTCV_UPPER,
>>> +.post_read = counter_high_value_postr,
>>
>> Spec says that you should also be able to access the whole
>> 64-bit counter value with a 64-bit access -- is this reginfo
>> sufficient to make that work?
>
> How do you know, all I see if that it is a 64-bit register. All the
> documentation about accesses specifies only 32-bit accesses.

v8 ARM ARM DDI0487A.k, I3.5.3 (CNTCV register description):
"In an implementation that supports 64-bit atomic memory accesses,
this register must be accessible using a 64-bit atomic access."


>> This means you can't use this device in a system which
>> doesn't implement TrustZone. I think this would be better
>> handled by just having the board map the memory region
>> in to only the Secure address space if it is a TZ-aware board.
>
> How can I map something into the secure address space? Is there an
> example of this? The only think I can find is in the arm/virt.c
> machine with the secure_sysmem MemoryRegion.

Yeah, virt.c is the place to look -- for instance we put one
of the UARTs in the secure address space only.
Basically to support separate address spaces:
 * create a MemoryRegion to be the secure view of the world
 * add the NS MemoryRegion to it as a subregion with low
   priority (so S sees all the NS devices -- this isn't
   required but that's usually how h/w is set up)
 * use object_property_set_link() to connect the S memory
   region to each CPU's secure-memory property
 * add any S-only devices to the S MemoryRegion

>>> +REG32(CNTCR, 0x00)
>>> +FIELD(CNTCR, EN, 0, 1)
>>> +FIELD(CNTCR, HDBG, 1, 1)
>>> +REG32(CNTSR, 0x04)
>>> +FIELD(CNTSR, DBGH, 1, 1)
>>> +REG32(CNTCV_LOWER, 0x08)
>>> +REG32(CNTCV_UPPER, 0x0C)
>>> +REG32(CNTFID0, 0x20)
>>> +/* We don't model CNTFIDn */
>>> +/* We don't model the CounterID registers either */
>>
>> Does the Xilinx h/w not implement them at all?
>> (ie do we need a property for "device has the ID regs" if we add them
>> later?)
>
> There is nothing in the register spec describing registers being here.
>
> The last register I see is called: (IOU_SCNTRS)
> base_frequency_ID_register at address 0xFF260020.

I suspect there's an implicit reads-as-zero after that.
(The frequency table has to have at least one real entry
plus the zero terminator -- see I3.5.5 (CNTFID0).)

QEMU's implementation will only support one frequency so
we only need one table entry too (for the moment, anyway).

>> The spec says it's mandatory to have at least the entry for
>> the counter base frequency plus end-of-table marker.
>> I would expect EL3 firmware to want to read this table in order
>> to write the correct values to CNTFRQ_EL0 for each CPU.
>
> I don't see anything like that in section I3.5.21 in the ARM ARM,
> where are you looking?

There's no I3.5.21 in my copy of the spec (rev A.k), I suspect
you have an older rev. I think the generic timer docs got
revamped at some point, so you might want to grab the latest rev.

Anyway, it looks like you have got CNTFIDn (that's CNTFID0)
and an implicit undocumented zero-terminator.

The spec says CounterIDn are IMPDEF, which I guess is why
your implementation doesn't bother to implement them.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 1/3] arm_generic_timer: Add the ARM Generic Timer

[Alistair Francis]

On Fri, Jan 6, 2017 at 3:57 AM, Peter Maydell  wrote:
> On 20 December 2016 at 22:42, Alistair Francis
>  wrote:
>> Add the ARM generic timer. This allows the guest to poll the timer for
>> values and also supports secure writes only.
>>
>> Signed-off-by: Alistair Francis 
>> ---
>> V3:
>>  - Use ARM ARM names
>>  - Indicate that we don't support all of the registers
>>  - Fixup the Makefile CONFIG
>> V2:
>>  - Fix couter/counter typo
>>
>>  hw/timer/Makefile.objs   |   1 +
>>  hw/timer/arm_generic_timer.c | 205 
>> +++
>>  include/hw/timer/arm_generic_timer.h |  62 +++
>>  3 files changed, 268 insertions(+)
>>  create mode 100644 hw/timer/arm_generic_timer.c
>>  create mode 100644 include/hw/timer/arm_generic_timer.h
>>
>> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
>> index 7ba8c23..bb203e2 100644
>> --- a/hw/timer/Makefile.objs
>> +++ b/hw/timer/Makefile.objs
>> @@ -17,6 +17,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
>>  common-obj-$(CONFIG_IMX) += imx_gpt.o
>>  common-obj-$(CONFIG_LM32) += lm32_timer.o
>>  common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
>> +common-obj-$(CONFIG_ARM_TIMER) += arm_generic_timer.o
>>
>>  obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
>>  obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
>> diff --git a/hw/timer/arm_generic_timer.c b/hw/timer/arm_generic_timer.c
>> new file mode 100644
>> index 000..da434a7
>> --- /dev/null
>> +++ b/hw/timer/arm_generic_timer.c
>> @@ -0,0 +1,205 @@
>> +/*
>> + * QEMU model of the ARM Generic Timer
>> + *
>> + * Copyright (c) 2016 Xilinx Inc.
>> + * Written by Alistair Francis 
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/timer/arm_generic_timer.h"
>> +#include "qemu/timer.h"
>> +#include "qemu/log.h"
>> +
>> +#ifndef ARM_GEN_TIMER_ERR_DEBUG
>> +#define ARM_GEN_TIMER_ERR_DEBUG 0
>> +#endif
>> +
>> +static void counter_control_postw(RegisterInfo *reg, uint64_t val64)
>> +{
>> +ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
>> +bool new_status = extract32(s->regs[R_CNTCR],
>> +R_CNTCR_EN_SHIFT,
>> +R_CNTCR_EN_LENGTH);
>> +uint64_t current_ticks;
>> +
>> +current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
>> + NANOSECONDS_PER_SECOND, 100);
>> +
>> +if ((s->enabled && !new_status) ||
>> +(!s->enabled && new_status)) {
>
> Since s->enabled and new_status are both bool, you can
> write this as "if (s->enabled != new_status)".
> (If they were ints you could use xor.)

I should have realised that, thanks.

>
>> +/* The timer is being disabled or enabled */
>> +s->tick_offset = current_ticks - s->tick_offset;
>> +}
>> +
>> +s->enabled = new_status;
>> +}
>> +
>> +static uint64_t counter_value_postr(RegisterInfo *reg)
>> +{
>> +ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
>> +uint64_t current_ticks, total_ticks;
>> +
>> +if (s->enabled) {
>> +current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
>> + NANOSECONDS_PER_SECOND, 100);
>> +total_ticks = current_ticks - s->tick_offset;
>> +} else {
>> +/* Timer is disabled, return the time when it was disabled */
>> +total_ticks = s->tick_offset;
>> +}
>> +
>> +return total_ticks;
>> +}
>> +
>> +static uint64_t counter_low_value_postr(RegisterInfo *reg, uint64_t val64)
>> +{
>> +return (uint32_t) counter_value_postr(reg);
>> +}
>> +
>> +static uint64_t counter_high_value_postr(RegisterInfo *reg, uint64_t val64)
>> +{
>> +return (uint32_t) (counter_value_postr(reg) >> 32);
>> +}
>
> The spec says 

Re: [Qemu-devel] [PATCH v3 1/3] arm_generic_timer: Add the ARM Generic Timer

[Peter Maydell]

On 20 December 2016 at 22:42, Alistair Francis
 wrote:
> Add the ARM generic timer. This allows the guest to poll the timer for
> values and also supports secure writes only.
>
> Signed-off-by: Alistair Francis 
> ---
> V3:
>  - Use ARM ARM names
>  - Indicate that we don't support all of the registers
>  - Fixup the Makefile CONFIG
> V2:
>  - Fix couter/counter typo
>
>  hw/timer/Makefile.objs   |   1 +
>  hw/timer/arm_generic_timer.c | 205 
> +++
>  include/hw/timer/arm_generic_timer.h |  62 +++
>  3 files changed, 268 insertions(+)
>  create mode 100644 hw/timer/arm_generic_timer.c
>  create mode 100644 include/hw/timer/arm_generic_timer.h
>
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 7ba8c23..bb203e2 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -17,6 +17,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
>  common-obj-$(CONFIG_IMX) += imx_gpt.o
>  common-obj-$(CONFIG_LM32) += lm32_timer.o
>  common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
> +common-obj-$(CONFIG_ARM_TIMER) += arm_generic_timer.o
>
>  obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
>  obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
> diff --git a/hw/timer/arm_generic_timer.c b/hw/timer/arm_generic_timer.c
> new file mode 100644
> index 000..da434a7
> --- /dev/null
> +++ b/hw/timer/arm_generic_timer.c
> @@ -0,0 +1,205 @@
> +/*
> + * QEMU model of the ARM Generic Timer
> + *
> + * Copyright (c) 2016 Xilinx Inc.
> + * Written by Alistair Francis 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/timer/arm_generic_timer.h"
> +#include "qemu/timer.h"
> +#include "qemu/log.h"
> +
> +#ifndef ARM_GEN_TIMER_ERR_DEBUG
> +#define ARM_GEN_TIMER_ERR_DEBUG 0
> +#endif
> +
> +static void counter_control_postw(RegisterInfo *reg, uint64_t val64)
> +{
> +ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
> +bool new_status = extract32(s->regs[R_CNTCR],
> +R_CNTCR_EN_SHIFT,
> +R_CNTCR_EN_LENGTH);
> +uint64_t current_ticks;
> +
> +current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
> + NANOSECONDS_PER_SECOND, 100);
> +
> +if ((s->enabled && !new_status) ||
> +(!s->enabled && new_status)) {

Since s->enabled and new_status are both bool, you can
write this as "if (s->enabled != new_status)".
(If they were ints you could use xor.)

> +/* The timer is being disabled or enabled */
> +s->tick_offset = current_ticks - s->tick_offset;
> +}
> +
> +s->enabled = new_status;
> +}
> +
> +static uint64_t counter_value_postr(RegisterInfo *reg)
> +{
> +ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
> +uint64_t current_ticks, total_ticks;
> +
> +if (s->enabled) {
> +current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
> + NANOSECONDS_PER_SECOND, 100);
> +total_ticks = current_ticks - s->tick_offset;
> +} else {
> +/* Timer is disabled, return the time when it was disabled */
> +total_ticks = s->tick_offset;
> +}
> +
> +return total_ticks;
> +}
> +
> +static uint64_t counter_low_value_postr(RegisterInfo *reg, uint64_t val64)
> +{
> +return (uint32_t) counter_value_postr(reg);
> +}
> +
> +static uint64_t counter_high_value_postr(RegisterInfo *reg, uint64_t val64)
> +{
> +return (uint32_t) (counter_value_postr(reg) >> 32);
> +}

The spec says that a write to the CNTCV should cause changes
to each CPU's CNTPCT/CNTPCT_EL0 register -- how do we plan to implement this?

(v8 ARM ARM section D6.1 is the clearest description of the
relationship between the system counter and the per-CPU timers.)

> +
> +static 

[Qemu-devel] [PATCH v3 1/3] arm_generic_timer: Add the ARM Generic Timer

[Alistair Francis]

Add the ARM generic timer. This allows the guest to poll the timer for
values and also supports secure writes only.

Signed-off-by: Alistair Francis 
---
V3:
 - Use ARM ARM names
 - Indicate that we don't support all of the registers
 - Fixup the Makefile CONFIG
V2:
 - Fix couter/counter typo

 hw/timer/Makefile.objs   |   1 +
 hw/timer/arm_generic_timer.c | 205 +++
 include/hw/timer/arm_generic_timer.h |  62 +++
 3 files changed, 268 insertions(+)
 create mode 100644 hw/timer/arm_generic_timer.c
 create mode 100644 include/hw/timer/arm_generic_timer.h

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 7ba8c23..bb203e2 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -17,6 +17,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
 common-obj-$(CONFIG_IMX) += imx_gpt.o
 common-obj-$(CONFIG_LM32) += lm32_timer.o
 common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
+common-obj-$(CONFIG_ARM_TIMER) += arm_generic_timer.o
 
 obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
diff --git a/hw/timer/arm_generic_timer.c b/hw/timer/arm_generic_timer.c
new file mode 100644
index 000..da434a7
--- /dev/null
+++ b/hw/timer/arm_generic_timer.c
@@ -0,0 +1,205 @@
+/*
+ * QEMU model of the ARM Generic Timer
+ *
+ * Copyright (c) 2016 Xilinx Inc.
+ * Written by Alistair Francis 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/timer/arm_generic_timer.h"
+#include "qemu/timer.h"
+#include "qemu/log.h"
+
+#ifndef ARM_GEN_TIMER_ERR_DEBUG
+#define ARM_GEN_TIMER_ERR_DEBUG 0
+#endif
+
+static void counter_control_postw(RegisterInfo *reg, uint64_t val64)
+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+bool new_status = extract32(s->regs[R_CNTCR],
+R_CNTCR_EN_SHIFT,
+R_CNTCR_EN_LENGTH);
+uint64_t current_ticks;
+
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+
+if ((s->enabled && !new_status) ||
+(!s->enabled && new_status)) {
+/* The timer is being disabled or enabled */
+s->tick_offset = current_ticks - s->tick_offset;
+}
+
+s->enabled = new_status;
+}
+
+static uint64_t counter_value_postr(RegisterInfo *reg)
+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+uint64_t current_ticks, total_ticks;
+
+if (s->enabled) {
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+total_ticks = current_ticks - s->tick_offset;
+} else {
+/* Timer is disabled, return the time when it was disabled */
+total_ticks = s->tick_offset;
+}
+
+return total_ticks;
+}
+
+static uint64_t counter_low_value_postr(RegisterInfo *reg, uint64_t val64)
+{
+return (uint32_t) counter_value_postr(reg);
+}
+
+static uint64_t counter_high_value_postr(RegisterInfo *reg, uint64_t val64)
+{
+return (uint32_t) (counter_value_postr(reg) >> 32);
+}
+
+static RegisterAccessInfo arm_gen_timer_regs_info[] = {
+{   .name = "CNTCR",
+.addr = A_CNTCR,
+.rsvd = 0xfffc,
+.post_write = counter_control_postw,
+},{ .name = "CNTSR",
+.addr = A_CNTSR,
+.rsvd = 0xfffd, .ro = 0x2,
+},{ .name = "CNTCV_LOWER",
+.addr = A_CNTCV_LOWER,
+.post_read = counter_low_value_postr,
+},{ .name = "CNTCV_UPPER",
+.addr = A_CNTCV_UPPER,
+.post_read = counter_high_value_postr,
+},{ .name = "CNTFID0",
+.addr = A_CNTFID0,
+}
+/* We don't model CNTFIDn */
+/* We don't model the CounterID registers either */
+};
+
+static void arm_gen_timer_reset(DeviceState *dev)
+{
+ARMGenTimer *s =