Patchwork [V2] ARM Cortex A9 Global Timer

login
register
mail settings
Submitter François Legal
Date April 16, 2013, 12:50 p.m.
Message ID <37e5da1bbc6d72dac52559deac7ef181@thom.fr.eu.org>
Download mbox | patch
Permalink /patch/236950/
State New
Headers show

Comments

François Legal - April 16, 2013, 12:50 p.m.
Le 16-04-2013 14:19, Peter Maydell a écrit :

> On 16 April 2013 13:09, François Legal <devel@thom.fr.eu.org> wrote:
> Ugh. Your mail client has completely mangled things (it's
> run all the lines of code into each other and it's still
> posting as multipart text+HTML). Please could you look at
> fixing its configuration -- it makes it hard to reply in
> response to individual things you've said.
> 

Sorry !

> Anyway, you may be right about needing multiple qemutimers,
> I think I misunderstood that part. We set up one timer
> for each comparator, right? (ie it fires when the comparator
> would fire). In that case I think that is correct.
> 

At least, that's how I understood the stuff.

> You might like to try having each gTimerBlock have an
> ARMMPGTimerState* field, so you get at the non-banked
> parts of the control register with 'gtb->gtimer.gtimer_control'
> rather than via an anonymous uint32_t*. I think that would
> be clearer.
> 

Ok.

> Regarding gdb access to memory mapped registers causing a crash
> due to NULL cpu_single_env -- I think this is a general issue with
> the gdb support code. I suggest you drop those parts of your
> patch for now; we should look at fixing it in a coherent way
> separately and later (eg by having gdb memory accesses always look
> as if they are from CPU 0, or something).
> 

Alright. I'll drop that from the patch.

> PS: I didn't mention it, but the struct names and so on
> should also change in line with my suggested new device
> name/filename.
> 

Done.
> thanks
> -- PMM


New patch follows.

---

+type_init(a9mp_globaltimer_register_types)
diff -urN qemu-master.old/hw/timer/Makefile.objs 
qemu-master/hw/timer/Makefile.objs
--- qemu-master.old/hw/timer/Makefile.objs	2013-04-08 20:12:33.000000000 
+0200
+++ qemu-master/hw/timer/Makefile.objs	2013-04-16 13:53:09.000000000 +0200
@@ -24,5 +24,5 @@
  obj-$(CONFIG_SH4) += sh_timer.o
  obj-$(CONFIG_TUSB6010) += tusb6010.o

-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
+obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o a9gtimer.o
  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o

Signed-off-by: François LEGAL <devel@thom.fr.eu.org>
Peter Crosthwaite - April 16, 2013, 1:23 p.m.
Hi Francois,

On Tue, Apr 16, 2013 at 10:50 PM, François Legal
<francois.legal@thom.fr.eu.org> wrote:
> Le 16-04-2013 14:19, Peter Maydell a écrit :
>
>
>> On 16 April 2013 13:09, François Legal <devel@thom.fr.eu.org> wrote:
>> Ugh. Your mail client has completely mangled things (it's
>> run all the lines of code into each other and it's still
>> posting as multipart text+HTML). Please could you look at
>> fixing its configuration -- it makes it hard to reply in
>> response to individual things you've said.
>>
>
> Sorry !
>
>
>> Anyway, you may be right about needing multiple qemutimers,
>> I think I misunderstood that part. We set up one timer
>> for each comparator, right? (ie it fires when the comparator
>> would fire). In that case I think that is correct.
>>
>

I had this problem with timer/cadence_ttc.c, which is a single timer
with multiple comparators. Went the other way implementation wise
though, using a single QEMUTimer and each trap of the timer it
computes which event is going to occur next and qemu_mod_timer with
the MIN of the comparators.

> At least, that's how I understood the stuff.
>
>
>> You might like to try having each gTimerBlock have an
>> ARMMPGTimerState* field, so you get at the non-banked
>> parts of the control register with 'gtb->gtimer.gtimer_control'
>> rather than via an anonymous uint32_t*. I think that would
>> be clearer.
>>
>
> Ok.
>
>
>> Regarding gdb access to memory mapped registers causing a crash
>> due to NULL cpu_single_env -- I think this is a general issue with
>> the gdb support code. I suggest you drop those parts of your
>> patch for now; we should look at fixing it in a coherent way
>> separately and later (eg by having gdb memory accesses always look
>> as if they are from CPU 0, or something).
>>
>
> Alright. I'll drop that from the patch.
>
>
>> PS: I didn't mention it, but the struct names and so on
>> should also change in line with my suggested new device
>> name/filename.
>>
>
> Done.
>>
>> thanks
>> -- PMM
>
>
>
> New patch follows.
>

Use git to create and send patches. Check out the commands:

git format-patch
git send-email

This will send you patch as plain text as well.

Regards,
Peter

> ---
>
> diff -urN qemu-master.old/hw/cpu/a9mpcore.c qemu-master/hw/cpu/a9mpcore.c
> --- qemu-master.old/hw/cpu/a9mpcore.c   2013-04-08 20:12:33.000000000 +0200
> +++ qemu-master/hw/cpu/a9mpcore.c       2013-04-16 13:18:39.000000000 +0200
>
> @@ -15,6 +15,7 @@
>      uint32_t num_cpu;
>      MemoryRegion container;
>      DeviceState *mptimer;
> +    DeviceState *mpgtimer;
>      DeviceState *wdt;
>      DeviceState *gic;
>      DeviceState *scu;
> @@ -31,6 +32,7 @@
>  {
>      A9MPPrivState *s = FROM_SYSBUS(A9MPPrivState, dev);
>      SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
> +    SysBusDevice *gtimerbusdev;
>      int i;
>
>      s->gic = qdev_create(NULL, "arm_gic");
> @@ -50,6 +52,11 @@
>      qdev_init_nofail(s->scu);
>      scubusdev = SYS_BUS_DEVICE(s->scu);
>
> +    s->mpgtimer = qdev_create(NULL, "a9_globaltimer");
>
> +    qdev_prop_set_uint32(s->mpgtimer, "num-cpu", s->num_cpu);
> +    qdev_init_nofail(s->mpgtimer);
> +    gtimerbusdev = SYS_BUS_DEVICE(s->mpgtimer);
> +
>      s->mptimer = qdev_create(NULL, "arm_mptimer");
>      qdev_prop_set_uint32(s->mptimer, "num-cpu", s->num_cpu);
>      qdev_init_nofail(s->mptimer);
> @@ -68,8 +75,6 @@
>       *  0x0600-0x06ff -- private timers and watchdogs
>       *  0x0700-0x0fff -- nothing
>       *  0x1000-0x1fff -- GIC Distributor
> -     *
> -     * We should implement the global timer but don't currently do so.
>       */
>      memory_region_init(&s->container, "a9mp-priv-container", 0x2000);
>      memory_region_add_subregion(&s->container, 0,
> @@ -80,6 +85,8 @@
>      /* Note that the A9 exposes only the "timer/watchdog for this core"
>       * memory region, not the "timer/watchdog for core X" ones 11MPcore
> has.
>       */
> +    memory_region_add_subregion(&s->container, 0x200,
> +                                sysbus_mmio_get_region(gtimerbusdev, 0));
>      memory_region_add_subregion(&s->container, 0x600,
>                                  sysbus_mmio_get_region(timerbusdev, 0));
>      memory_region_add_subregion(&s->container, 0x620,
> @@ -90,10 +97,13 @@
>      sysbus_init_mmio(dev, &s->container);
>
>      /* Wire up the interrupt from each watchdog and timer.
> -     * For each core the timer is PPI 29 and the watchdog PPI 30.
> +     * For each core the global timer is PPI 27, the private
> +     * timer is PPI 29 and the watchdog PPI 30.
>       */
>      for (i = 0; i < s->num_cpu; i++) {
>          int ppibase = (s->num_irq - 32) + i * 32;
> +        sysbus_connect_irq(gtimerbusdev, i,
> +                           qdev_get_gpio_in(s->gic, ppibase + 27));
>          sysbus_connect_irq(timerbusdev, i,
>                             qdev_get_gpio_in(s->gic, ppibase + 29));
>          sysbus_connect_irq(wdtbusdev, i,
> diff -urN qemu-master.old/hw/timer/a9gtimer.c
> qemu-master/hw/timer/a9gtimer.c
> --- qemu-master.old/hw/timer/a9gtimer.c 1970-01-01 01:00:00.000000000 +0100
> +++ qemu-master/hw/timer/a9gtimer.c     2013-04-16 14:35:48.000000000 +0200
> @@ -0,0 +1,348 @@
> +/*
> + * Global peripheral timer block for ARM A9MP
>
> + *
> + * Written by François LEGAL
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/sysbus.h"
> +#include "qemu/timer.h"
> +
> +/* This device implements the per-cpu private timer and watchdog block
> + * which is used in the Cortex-A9MP.
>
> + */
> +
> +#define MAX_CPUS 4
> +#define TYPE_GTIMER "a9_globaltimer"
> +#define GTIMER(obj) OBJECT_CHECK(a9globaltimerState, (obj), TYPE_GTIMER)
>
> +
> +/* State of a single gtimer or block */
> +typedef struct {
> +    uint32_t control;
> +    uint64_t compare;
> +    uint32_t inc;
> +    uint32_t status;
> +    int64_t  tick;
> +
> +    int64_t    delta;
> +
> +    struct a9globaltimerState *gtimer_state;
>
> +    QEMUTimer *timer;
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +} gTimerBlock;
> +
> +typedef struct a9globaltimerState {
>
> +    SysBusDevice busdev;
> +    uint32_t num_cpu;
> +    uint64_t gtimer_counter;
> +    uint32_t gtimer_control;
> +    gTimerBlock gtimer[MAX_CPUS];
> +    MemoryRegion iomem;
> +} a9globaltimerState;
> +
> +static inline int get_current_cpu(a9globaltimerState *s)
> +{
> +    CPUState *cpu_single_cpu = ENV_GET_CPU(cpu_single_env);
>
> +
> +    if (cpu_single_cpu->cpu_index >= s->num_cpu) {
> +        hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n",
> +                 s->num_cpu, cpu_single_cpu->cpu_index);
> +    }
> +    return cpu_single_cpu->cpu_index;
> +}
> +
> +static inline uint32_t gtimerblock_get_control_reg(gTimerBlock *gtb)
> +{
> +    return gtb->control | gtb->gtimer_state->gtimer_control;
> +}
>
> +
> +static inline void gtimerblock_update_irq(gTimerBlock *gtb)
> +{
> +    qemu_set_irq(gtb->irq, gtb->status);
> +}
> +
> +/* Return conversion factor from mpcore timer ticks to qemu timer ticks.
> */
> +static inline uint32_t gtimerblock_scale(gTimerBlock *gtb)
> +{
> +    return (((gtb->gtimer_state->gtimer_control >> 8) & 0xff) + 1) * 10;
>
> +}
> +
> +static void gtimerblock_reload(gTimerBlock *gtb, int restart)
> +{
> +    if (restart) {
> +        gtb->tick = qemu_get_clock_ns(vm_clock);
> +        gtb->tick += (int64_t)(((gtb->compare -
> +                                 gtb->gtimer_state->gtimer_counter) +
>
> +                                 gtb->delta) * gtimerblock_scale(gtb));
> +    } else {
> +        gtb->tick += (int64_t)(gtb->inc * gtimerblock_scale(gtb));
> +    }
> +    qemu_mod_timer(gtb->timer, gtb->tick);
> +}
> +
> +static void gtimerblock_tick(void *opaque)
> +{
> +    gTimerBlock *gtb = (gTimerBlock *)opaque;
> +    gtb->gtimer_state->gtimer_counter = gtb->compare;
> +    if (gtimerblock_get_control_reg(gtb) & 0x9) {
>
> +        gtb->compare += gtb->inc;
> +        gtimerblock_reload(gtb, 0);
> +    }
> +    if ((gtimerblock_get_control_reg(gtb) & 0x7) &&
>
> +        ((gtb->status & 1) == 0)) {
> +        gtb->status = 1;
> +        gtimerblock_update_irq(gtb);
> +    }
> +}
> +
> +static uint64_t gtimer_read(void *opaque, hwaddr addr,
> +                                unsigned size)
> +{
> +    gTimerBlock *gtb = (gTimerBlock *)opaque;
> +    int64_t val = 0;
> +    switch (addr) {
> +    case 0: /* Counter LSB */
> +        if (gtimerblock_get_control_reg(gtb) & 1)  {
>
> +            val = gtb->tick - qemu_get_clock_ns(vm_clock);
> +            val /= gtimerblock_scale(gtb);
> +        }
> +        return val < 0 ? val : 0 & 0xFFFFFFFF;
> +    case 4: /* Counter MSB.  */
> +        if (gtimerblock_get_control_reg(gtb) & 1)  {
>
> +            val = gtb->tick - qemu_get_clock_ns(vm_clock);
> +            val /= gtimerblock_scale(gtb);
> +        }
> +        return val < 0 ? (val >> 32) : 0;
> +    case 8: /* Control.  */
> +        return gtimerblock_get_control_reg(gtb) & 0x0000FF0F;
>
> +    case 12: /* Interrupt status.  */
> +        return gtb->status;
> +    case 16: /* Compare LSB */
> +        return gtb->compare & 0xFFFFFFFF;
> +    case 20: /* Counter MSB  */
> +        return gtb->compare >> 32;
> +    case 24: /* Autoincrement */
> +        return gtb->inc;
> +    default:
> +        return 0;
> +    }
> +}
> +
> +static void gtimer_write(void *opaque, hwaddr addr,
> +                             uint64_t value, unsigned size)
> +{
> +    gTimerBlock *gtb = (gTimerBlock *)opaque;
> +    int64_t old;
> +    switch (addr) {
> +    case 0: /* Counter LSB */
> +        old = gtb->gtimer_state->gtimer_counter;
>
> +        old &= 0xFFFFFFFF;
> +        gtb->delta = old - (value & 0xFFFFFFFF);
> +        old |= value & 0xFFFFFFFF;
> +        gtb->gtimer_state->gtimer_counter = old;
>
> +        /* Cancel the previous timer.  */
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 4: /* Counter MSB.  */
> +        old = gtb->gtimer_state->gtimer_counter;
>
> +        old &= 0xFFFFFFFF00000000;
> +        gtb->delta = old - (value << 32);
> +        old |= value << 32;
> +        gtb->gtimer_state->gtimer_counter = old;
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 8: /* Control.  */
> +        old = gtb->gtimer_state->gtimer_control;
>
> +        gtb->control = value & 0x0000000E;
> +        gtb->gtimer_state->gtimer_control = value & 0x0000FF01;
>
> +        if (((old & 1) == 0) && ((value & 7) == 7)) {
> +            gtb->delta = 0;
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 12: /* Interrupt status.  */
> +        gtb->status &= ~ value;
>
> +        gtimerblock_update_irq(gtb);
> +        break;
> +    case 16: /* Compare LSB */
> +        old = gtb->compare;
> +        old &= 0xFFFFFFFF;
> +        gtb->delta = (value & 0xFFFFFFFF) - old;
> +        old |= value & 0xFFFFFFFF;
> +        gtb->compare = old;
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 20: /* Compare MSB.  */
> +        old = gtb->compare;
> +        old &= 0xFFFFFFFF00000000;
> +        gtb->delta = (value << 32) - old;
> +        old |= value << 32;
> +        gtb->compare = old;
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 24: /* Autoincrement */
> +        gtb->inc = value;
> +        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +/* Wrapper functions to implement the "read global timer for
>
> + * the current CPU" memory regions.
> + */
> +static uint64_t arm_thisgtimer_read(void *opaque, hwaddr addr,
> +                                    unsigned size)
> +{
> +    a9globaltimerState *s = (a9globaltimerState *)opaque;
>
> +    int id = get_current_cpu(s);
> +    return gtimer_read(&s->gtimer[id], addr, size);
> +}
> +
> +static void arm_thisgtimer_write(void *opaque, hwaddr addr,
> +                                 uint64_t value, unsigned size)
> +{
> +    a9globaltimerState *s = (a9globaltimerState *)opaque;
>
> +    int id = get_current_cpu(s);
> +    gtimer_write(&s->gtimer[id], addr, value, size);
> +}
> +
> +static const MemoryRegionOps arm_thisgtimer_ops = {
> +    .read = arm_thisgtimer_read,
> +    .write = arm_thisgtimer_write,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const MemoryRegionOps gtimerblock_ops = {
> +    .read = gtimer_read,
> +    .write = gtimer_write,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void gtimer_reset(gTimerBlock *gtb)
> +{
> +    if (gtb->timer) {
> +        qemu_del_timer(gtb->timer);
>
> +    }
> +    gtb->control = 0;
> +    gtb->status = 0;
> +    gtb->compare = 0;
> +    gtb->inc = 0;
> +    gtb->tick = 0;
> +}
> +
> +static void a9mp_globaltimer_reset(DeviceState *dev)
> +{
> +    a9globaltimerState *s = GTIMER(dev);
>
> +    int i;
> +    for (i = 0; i < ARRAY_SIZE(s->gtimer); i++) {
> +        gtimer_reset(&s->gtimer[i]);
> +    }
> +}
> +
> +static void a9mp_globaltimer_realize(DeviceState *dev, Error **errp)
> +{
> +    a9globaltimerState *s = GTIMER(dev);
> +    int i;
> +
>
> +    if (s->num_cpu < 1 || s->num_cpu > MAX_CPUS) {
> +        error_setg(errp, "%s: num-cpu must be between 1 and %d\n",
> +                   __func__, MAX_CPUS);
> +    }
> +
>
> +    memory_region_init_io(&s->iomem, &arm_thisgtimer_ops, s,
> +                          "arm_mptimer_gtimer", 0x20);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>
> +    for (i = 0; i < s->num_cpu; i++) {
> +        gTimerBlock *gtb = &s->gtimer[i];
> +        gtb->gtimer_state = s;
>
> +        gtb->timer = qemu_new_timer_ns(vm_clock, gtimerblock_tick, gtb);
> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &gtb->irq);
>
> +        memory_region_init_io(&gtb->iomem, &gtimerblock_ops, gtb,
> +                              "arm_mptimer_gtimerblock", 0x20);
> +        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &gtb->iomem);
> +    }
> +}
> +
>
> +static const VMStateDescription vmstate_gtimerblock = {
> +    .name = "a9mp_gtimerblock",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
>
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(control, gTimerBlock),
> +        VMSTATE_UINT32(status, gTimerBlock),
> +        VMSTATE_UINT64(compare, gTimerBlock),
> +        VMSTATE_INT64(tick, gTimerBlock),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_a9mp_globaltimer = {
> +    .name = "a9mp_globaltimer",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
>
> +    .fields = (VMStateField[]) {
> +    VMSTATE_STRUCT_VARRAY_UINT32(gtimer, a9globaltimerState, num_cpu,
>
> +                                 1, vmstate_gtimerblock, gTimerBlock),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property a9mp_globaltimer_properties[] = {
> +    DEFINE_PROP_UINT32("num-cpu", a9globaltimerState, num_cpu, 0),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void a9mp_globaltimer_class_init(ObjectClass *klass, void *data)
>
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = a9mp_globaltimer_realize;
> +    dc->vmsd = &vmstate_a9mp_globaltimer;
> +    dc->reset = a9mp_globaltimer_reset;
>
> +    dc->no_user = 1;
> +    dc->props = a9mp_globaltimer_properties;
> +}
> +
> +static const TypeInfo a9mp_globaltimer_info = {
> +    .name          = "a9_globaltimer",
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(a9globaltimerState),
> +    .class_init    = a9mp_globaltimer_class_init,
> +};
> +
> +static void a9mp_globaltimer_register_types(void)
> +{
> +    type_register_static(&a9mp_globaltimer_info);
> +}
> +
> +type_init(a9mp_globaltimer_register_types)
> diff -urN qemu-master.old/hw/timer/Makefile.objs
> qemu-master/hw/timer/Makefile.objs
> --- qemu-master.old/hw/timer/Makefile.objs      2013-04-08
> 20:12:33.000000000 +0200
> +++ qemu-master/hw/timer/Makefile.objs  2013-04-16 13:53:09.000000000 +0200
>
> @@ -24,5 +24,5 @@
>  obj-$(CONFIG_SH4) += sh_timer.o
>  obj-$(CONFIG_TUSB6010) += tusb6010.o
>
> -obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
> +obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o a9gtimer.o
>
>  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
>
> Signed-off-by: François LEGAL <devel@thom.fr.eu.org>
>
>
François Legal - April 16, 2013, 2:06 p.m.
Hi Peter,

Le 16-04-2013 15:23, Peter Crosthwaite a écrit :

> Hi Francois,
> 
> On Tue, Apr 16, 2013 at 10:50 PM, François Legal
> <francois.legal@thom.fr.eu.org> wrote:
> 
>> Le 16-04-2013 14:19, Peter Maydell a écrit :
>> 
>>> On 16 April 2013 13:09, François Legal <devel@thom.fr.eu.org> wrote:
>>> Ugh. Your mail client has completely mangled things (it's run all the
>>> lines of code into each other and it's still posting as multipart
>>> text+HTML). Please could you look at fixing its configuration -- it
>>> makes it hard to reply in response to individual things you've said.
>> Sorry !
>> 
>>> Anyway, you may be right about needing multiple qemutimers, I think I
>>> misunderstood that part. We set up one timer for each comparator,
>>> right? (ie it fires when the comparator would fire). In that case I
>>> think that is correct.
> 
> I had this problem with timer/cadence_ttc.c, which is a single timer
> with multiple comparators. Went the other way implementation wise
> though, using a single QEMUTimer and each trap of the timer it
> computes which event is going to occur next and qemu_mod_timer with
> the MIN of the comparators.
> 

That's another solution, but I found out that, given my poor knowledge on 
Qemu, the other way was easier ;-)

>> At least, that's how I understood the stuff.
>> 
>>> You might like to try having each gTimerBlock have an
>>> ARMMPGTimerState* field, so you get at the non-banked parts of the
>>> control register with 'gtb->gtimer.gtimer_control' rather than via an
>>> anonymous uint32_t*. I think that would be clearer.
>> Ok.
>> 
>>> Regarding gdb access to memory mapped registers causing a crash due
>>> to NULL cpu_single_env -- I think this is a general issue with the
>>> gdb support code. I suggest you drop those parts of your patch for
>>> now; we should look at fixing it in a coherent way separately and
>>> later (eg by having gdb memory accesses always look as if they are
>>> from CPU 0, or something).
>> Alright. I'll drop that from the p
>> 
>>> quote>PS: I didn't mention it, but the struct names and so on should
>>> also change in line with my suggested new device name/> te> Done.
>> ks -- PMM New patch follows.
> 
> Use git to create and send patches. Check out the commands:
> 
> git format-patch
> git send-email
> 
> This will send you patch as plain text as well.

I wish I could, unfortunately, my corporate VPN/proxy blocks GIT.

> 
> Regards,
> Peter



Links:
------
[1] http://www.gnu.org/licenses/
Peter Crosthwaite - April 16, 2013, 3:17 p.m.
Hi Francois,

On Wed, Apr 17, 2013 at 12:06 AM, François Legal
<francois.legal@thom.fr.eu.org> wrote:
> Hi Peter,
>
> Le 16-04-2013 15:23, Peter Crosthwaite a écrit :
>
>
>> Hi Francois,
>>
>> On Tue, Apr 16, 2013 at 10:50 PM, François Legal
>> <francois.legal@thom.fr.eu.org> wrote:
>>
>>> Le 16-04-2013 14:19, Peter Maydell a écrit :
>>>
>>>> On 16 April 2013 13:09, François Legal <devel@thom.fr.eu.org> wrote:
>>>> Ugh. Your mail client has completely mangled things (it's run all the
>>>> lines of code into each other and it's still posting as multipart
>>>> text+HTML). Please could you look at fixing its configuration -- it
>>>> makes it hard to reply in response to individual things you've said.
>>>
>>> Sorry !
>>>
>>>> Anyway, you may be right about needing multiple qemutimers, I think I
>>>> misunderstood that part. We set up one timer for each comparator,
>>>> right? (ie it fires when the comparator would fire). In that case I
>>>> think that is correct.
>>
>>
>> I had this problem with timer/cadence_ttc.c, which is a single timer
>> with multiple comparators. Went the other way implementation wise
>> though, using a single QEMUTimer and each trap of the timer it
>> computes which event is going to occur next and qemu_mod_timer with
>> the MIN of the comparators.
>>
>
> That's another solution, but I found out that, given my poor knowledge on
> Qemu, the other way was easier ;-)
>
>>> At least, that's how I understood the stuff.
>>>
>>>> You might like to try having each gTimerBlock have an
>>>> ARMMPGTimerState* field, so you get at the non-banked parts of the
>>>> control register with 'gtb->gtimer.gtimer_control' rather than via an
>>>> anonymous uint32_t*. I think that would be clearer.
>>>
>>> Ok.
>>>
>>>> Regarding gdb access to memory mapped registers causing a crash due
>>>> to NULL cpu_single_env -- I think this is a general issue with the
>>>> gdb support code. I suggest you drop those parts of your patch for
>>>> now; we should look at fixing it in a coherent way separately and
>>>> later (eg by having gdb memory accesses always look as if they are
>>>> from CPU 0, or something).
>>>
>>> Alright. I'll drop that from the p
>>>
>>>> quote>PS: I didn't mention it, but the struct names and so on should
>>>> also change in line with my suggested new device name/> te> Done.
>>>
>>> ks -- PMM New patch follows.
>>
>>
>> Use git to create and send patches. Check out the commands:
>>
>> git format-patch
>> git send-email
>>
>> This will send you patch as plain text as well.
>
>
> I wish I could, unfortunately, my corporate VPN/proxy blocks GIT.
>

It is very difficult to work on this project without git.

But git remotes also work over http, so that could be usable to do
clones and pulls. If QEMU still has an up-to-date github mirror, you
should be able to clone from there.

So once you have your http clone you can use git locally to create and
format your patches. You are most of the way there, you just need a
way to send email. Just need a smtp server and config git send-email
to talk to it.

Ultimately, none of this requires the git network protocol - so git
can be done even with a proxy/firewall that blocks git.

Regards,
Peter


>>
>> Regards,
>> Peter
>
>
>
>
> Links:
> ------
> [1] http://www.gnu.org/licenses/
>
Peter Crosthwaite - June 24, 2013, 5:40 a.m.
I've respun this, but still testing and debugging a few issues.

Some notes FTR.

On Tue, Apr 16, 2013 at 10:50 PM, François Legal
<francois.legal@thom.fr.eu.org> wrote:
> Le 16-04-2013 14:19, Peter Maydell a écrit :
[Snip]
>
>
> New patch follows.
>
> ---
>
> diff -urN qemu-master.old/hw/cpu/a9mpcore.c qemu-master/hw/cpu/a9mpcore.c
> --- qemu-master.old/hw/cpu/a9mpcore.c   2013-04-08 20:12:33.000000000 +0200
> +++ qemu-master/hw/cpu/a9mpcore.c       2013-04-16 13:18:39.000000000 +0200
>
> @@ -15,6 +15,7 @@
>      uint32_t num_cpu;
>      MemoryRegion container;
>      DeviceState *mptimer;
> +    DeviceState *mpgtimer;
>      DeviceState *wdt;
>      DeviceState *gic;
>      DeviceState *scu;
> @@ -31,6 +32,7 @@
>  {
>      A9MPPrivState *s = FROM_SYSBUS(A9MPPrivState, dev);
>      SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
> +    SysBusDevice *gtimerbusdev;
>      int i;
>
>      s->gic = qdev_create(NULL, "arm_gic");
> @@ -50,6 +52,11 @@
>      qdev_init_nofail(s->scu);
>      scubusdev = SYS_BUS_DEVICE(s->scu);
>
> +    s->mpgtimer = qdev_create(NULL, "a9_globaltimer");
>
> +    qdev_prop_set_uint32(s->mpgtimer, "num-cpu", s->num_cpu);
> +    qdev_init_nofail(s->mpgtimer);
> +    gtimerbusdev = SYS_BUS_DEVICE(s->mpgtimer);
> +
>      s->mptimer = qdev_create(NULL, "arm_mptimer");
>      qdev_prop_set_uint32(s->mptimer, "num-cpu", s->num_cpu);
>      qdev_init_nofail(s->mptimer);
> @@ -68,8 +75,6 @@
>       *  0x0600-0x06ff -- private timers and watchdogs
>       *  0x0700-0x0fff -- nothing
>       *  0x1000-0x1fff -- GIC Distributor
> -     *
> -     * We should implement the global timer but don't currently do so.
>       */
>      memory_region_init(&s->container, "a9mp-priv-container", 0x2000);
>      memory_region_add_subregion(&s->container, 0,
> @@ -80,6 +85,8 @@
>      /* Note that the A9 exposes only the "timer/watchdog for this core"
>       * memory region, not the "timer/watchdog for core X" ones 11MPcore
> has.
>       */
> +    memory_region_add_subregion(&s->container, 0x200,
> +                                sysbus_mmio_get_region(gtimerbusdev, 0));
>      memory_region_add_subregion(&s->container, 0x600,
>                                  sysbus_mmio_get_region(timerbusdev, 0));
>      memory_region_add_subregion(&s->container, 0x620,
> @@ -90,10 +97,13 @@
>      sysbus_init_mmio(dev, &s->container);
>
>      /* Wire up the interrupt from each watchdog and timer.
> -     * For each core the timer is PPI 29 and the watchdog PPI 30.
> +     * For each core the global timer is PPI 27, the private
> +     * timer is PPI 29 and the watchdog PPI 30.
>       */
>      for (i = 0; i < s->num_cpu; i++) {
>          int ppibase = (s->num_irq - 32) + i * 32;
> +        sysbus_connect_irq(gtimerbusdev, i,
> +                           qdev_get_gpio_in(s->gic, ppibase + 27));
>          sysbus_connect_irq(timerbusdev, i,
>                             qdev_get_gpio_in(s->gic, ppibase + 29));
>          sysbus_connect_irq(wdtbusdev, i,
> diff -urN qemu-master.old/hw/timer/a9gtimer.c
> qemu-master/hw/timer/a9gtimer.c
> --- qemu-master.old/hw/timer/a9gtimer.c 1970-01-01 01:00:00.000000000 +0100
> +++ qemu-master/hw/timer/a9gtimer.c     2013-04-16 14:35:48.000000000 +0200
> @@ -0,0 +1,348 @@
> +/*
> + * Global peripheral timer block for ARM A9MP
>
> + *
> + * Written by François LEGAL
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/sysbus.h"
> +#include "qemu/timer.h"
> +
> +/* This device implements the per-cpu private timer and watchdog block
> + * which is used in the Cortex-A9MP.
>
> + */
> +
> +#define MAX_CPUS 4
> +#define TYPE_GTIMER "a9_globaltimer"
> +#define GTIMER(obj) OBJECT_CHECK(a9globaltimerState, (obj), TYPE_GTIMER)
>
> +
> +/* State of a single gtimer or block */
> +typedef struct {
> +    uint32_t control;
> +    uint64_t compare;
> +    uint32_t inc;
> +    uint32_t status;
> +    int64_t  tick;
> +
> +    int64_t    delta;
> +
> +    struct a9globaltimerState *gtimer_state;
>
> +    QEMUTimer *timer;

Set it up so there's only one timer which will trigger on the
next-to-occur comparator. Stops rounding error from potentially
occuring events out of order.

> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +} gTimerBlock;
> +
> +typedef struct a9globaltimerState {
>
> +    SysBusDevice busdev;
> +    uint32_t num_cpu;
> +    uint64_t gtimer_counter;
> +    uint32_t gtimer_control;
> +    gTimerBlock gtimer[MAX_CPUS];
> +    MemoryRegion iomem;
> +} a9globaltimerState;

Struct name fixed.

> +
> +static inline int get_current_cpu(a9globaltimerState *s)
> +{
> +    CPUState *cpu_single_cpu = ENV_GET_CPU(cpu_single_env);
>
> +
> +    if (cpu_single_cpu->cpu_index >= s->num_cpu) {
> +        hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n",
> +                 s->num_cpu, cpu_single_cpu->cpu_index);
> +    }
> +    return cpu_single_cpu->cpu_index;
> +}
> +
> +static inline uint32_t gtimerblock_get_control_reg(gTimerBlock *gtb)
> +{
> +    return gtb->control | gtb->gtimer_state->gtimer_control;
> +}
>
> +
> +static inline void gtimerblock_update_irq(gTimerBlock *gtb)
> +{
> +    qemu_set_irq(gtb->irq, gtb->status);
> +}
> +
> +/* Return conversion factor from mpcore timer ticks to qemu timer ticks.
> */
> +static inline uint32_t gtimerblock_scale(gTimerBlock *gtb)
> +{
> +    return (((gtb->gtimer_state->gtimer_control >> 8) & 0xff) + 1) * 10;
>
> +}
> +
> +static void gtimerblock_reload(gTimerBlock *gtb, int restart)
> +{
> +    if (restart) {
> +        gtb->tick = qemu_get_clock_ns(vm_clock);
> +        gtb->tick += (int64_t)(((gtb->compare -
> +                                 gtb->gtimer_state->gtimer_counter) +
>
> +                                 gtb->delta) * gtimerblock_scale(gtb));
> +    } else {
> +        gtb->tick += (int64_t)(gtb->inc * gtimerblock_scale(gtb));
> +    }
> +    qemu_mod_timer(gtb->timer, gtb->tick);
> +}
> +
> +static void gtimerblock_tick(void *opaque)
> +{
> +    gTimerBlock *gtb = (gTimerBlock *)opaque;
> +    gtb->gtimer_state->gtimer_counter = gtb->compare;
> +    if (gtimerblock_get_control_reg(gtb) & 0x9) {

Bad condition - auto will always happen when timer enabled.

>
> +        gtb->compare += gtb->inc;

> +        gtimerblock_reload(gtb, 0);
> +    }
> +    if ((gtimerblock_get_control_reg(gtb) & 0x7) &&
>

This reads that if any of the three LSB of control are set you get an interrupt.

> +        ((gtb->status & 1) == 0)) {
> +        gtb->status = 1;
> +        gtimerblock_update_irq(gtb);
> +    }
> +}
> +
> +static uint64_t gtimer_read(void *opaque, hwaddr addr,
> +                                unsigned size)
> +{
> +    gTimerBlock *gtb = (gTimerBlock *)opaque;
> +    int64_t val = 0;
> +    switch (addr) {
> +    case 0: /* Counter LSB */
> +        if (gtimerblock_get_control_reg(gtb) & 1)  {
>
> +            val = gtb->tick - qemu_get_clock_ns(vm_clock);

Strange - the further forward you are in emulation time, the lower the
counter value? I had some strangness here when using the timer in
polled mode - it ran backwards on me and I think its this, and I think
the operation of the timer is dep. on interrupts. I've reworked this
significantly to get polling working.

> +            val /= gtimerblock_scale(gtb);
> +        }
> +        return val < 0 ? val : 0 & 0xFFFFFFFF;
> +    case 4: /* Counter MSB.  */
> +        if (gtimerblock_get_control_reg(gtb) & 1)  {
>
> +            val = gtb->tick - qemu_get_clock_ns(vm_clock);
> +            val /= gtimerblock_scale(gtb);
> +        }
> +        return val < 0 ? (val >> 32) : 0;
> +    case 8: /* Control.  */
> +        return gtimerblock_get_control_reg(gtb) & 0x0000FF0F;
>

factored out replicated code. used extract64 rather than mask/shift.

> +    case 12: /* Interrupt status.  */
> +        return gtb->status;
> +    case 16: /* Compare LSB */
> +        return gtb->compare & 0xFFFFFFFF;
> +    case 20: /* Counter MSB  */
> +        return gtb->compare >> 32;
> +    case 24: /* Autoincrement */
> +        return gtb->inc;
> +    default:
> +        return 0;
> +    }
> +}
> +
> +static void gtimer_write(void *opaque, hwaddr addr,
> +                             uint64_t value, unsigned size)
> +{
> +    gTimerBlock *gtb = (gTimerBlock *)opaque;
> +    int64_t old;
> +    switch (addr) {
> +    case 0: /* Counter LSB */
> +        old = gtb->gtimer_state->gtimer_counter;
>
> +        old &= 0xFFFFFFFF;
> +        gtb->delta = old - (value & 0xFFFFFFFF);
> +        old |= value & 0xFFFFFFFF;
> +        gtb->gtimer_state->gtimer_counter = old;
>
> +        /* Cancel the previous timer.  */
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 4: /* Counter MSB.  */
> +        old = gtb->gtimer_state->gtimer_counter;
>
> +        old &= 0xFFFFFFFF00000000;
> +        gtb->delta = old - (value << 32);
> +        old |= value << 32;
> +        gtb->gtimer_state->gtimer_counter = old;
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 8: /* Control.  */
> +        old = gtb->gtimer_state->gtimer_control;
>
> +        gtb->control = value & 0x0000000E;
> +        gtb->gtimer_state->gtimer_control = value & 0x0000FF01;
>
> +        if (((old & 1) == 0) && ((value & 7) == 7)) {
> +            gtb->delta = 0;
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 12: /* Interrupt status.  */
> +        gtb->status &= ~ value;
>
> +        gtimerblock_update_irq(gtb);
> +        break;
> +    case 16: /* Compare LSB */
> +        old = gtb->compare;
> +        old &= 0xFFFFFFFF;
> +        gtb->delta = (value & 0xFFFFFFFF) - old;
> +        old |= value & 0xFFFFFFFF;
> +        gtb->compare = old;
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 20: /* Compare MSB.  */
> +        old = gtb->compare;
> +        old &= 0xFFFFFFFF00000000;
> +        gtb->delta = (value << 32) - old;
> +        old |= value << 32;
> +        gtb->compare = old;
> +        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
>
> +            gtimerblock_reload(gtb, 1);
> +        }
> +        break;
> +    case 24: /* Autoincrement */
> +        gtb->inc = value;
> +        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +/* Wrapper functions to implement the "read global timer for
>
> + * the current CPU" memory regions.
> + */
> +static uint64_t arm_thisgtimer_read(void *opaque, hwaddr addr,
> +                                    unsigned size)
> +{
> +    a9globaltimerState *s = (a9globaltimerState *)opaque;
>
> +    int id = get_current_cpu(s);
> +    return gtimer_read(&s->gtimer[id], addr, size);
> +}
> +
> +static void arm_thisgtimer_write(void *opaque, hwaddr addr,
> +                                 uint64_t value, unsigned size)
> +{
> +    a9globaltimerState *s = (a9globaltimerState *)opaque;
>
> +    int id = get_current_cpu(s);
> +    gtimer_write(&s->gtimer[id], addr, value, size);
> +}
> +
> +static const MemoryRegionOps arm_thisgtimer_ops = {
> +    .read = arm_thisgtimer_read,
> +    .write = arm_thisgtimer_write,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const MemoryRegionOps gtimerblock_ops = {
> +    .read = gtimer_read,
> +    .write = gtimer_write,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void gtimer_reset(gTimerBlock *gtb)
> +{
> +    if (gtb->timer) {
> +        qemu_del_timer(gtb->timer);
>
> +    }
> +    gtb->control = 0;
> +    gtb->status = 0;
> +    gtb->compare = 0;
> +    gtb->inc = 0;
> +    gtb->tick = 0;
> +}
> +
> +static void a9mp_globaltimer_reset(DeviceState *dev)
> +{
> +    a9globaltimerState *s = GTIMER(dev);
>
> +    int i;
> +    for (i = 0; i < ARRAY_SIZE(s->gtimer); i++) {
> +        gtimer_reset(&s->gtimer[i]);
> +    }
> +}
> +
> +static void a9mp_globaltimer_realize(DeviceState *dev, Error **errp)
> +{
> +    a9globaltimerState *s = GTIMER(dev);
> +    int i;
> +
>
> +    if (s->num_cpu < 1 || s->num_cpu > MAX_CPUS) {
> +        error_setg(errp, "%s: num-cpu must be between 1 and %d\n",
> +                   __func__, MAX_CPUS);
> +    }
> +
>
> +    memory_region_init_io(&s->iomem, &arm_thisgtimer_ops, s,
> +                          "arm_mptimer_gtimer", 0x20);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>
> +    for (i = 0; i < s->num_cpu; i++) {
> +        gTimerBlock *gtb = &s->gtimer[i];
> +        gtb->gtimer_state = s;
>
> +        gtb->timer = qemu_new_timer_ns(vm_clock, gtimerblock_tick, gtb);
> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &gtb->irq);
>
> +        memory_region_init_io(&gtb->iomem, &gtimerblock_ops, gtb,
> +                              "arm_mptimer_gtimerblock", 0x20);
> +        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &gtb->iomem);
> +    }
> +}
> +
>
> +static const VMStateDescription vmstate_gtimerblock = {
> +    .name = "a9mp_gtimerblock",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
>
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(control, gTimerBlock),
> +        VMSTATE_UINT32(status, gTimerBlock),
> +        VMSTATE_UINT64(compare, gTimerBlock),
> +        VMSTATE_INT64(tick, gTimerBlock),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_a9mp_globaltimer = {
> +    .name = "a9mp_globaltimer",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
>
> +    .fields = (VMStateField[]) {
> +    VMSTATE_STRUCT_VARRAY_UINT32(gtimer, a9globaltimerState, num_cpu,
>
> +                                 1, vmstate_gtimerblock, gTimerBlock),
> +        VMSTATE_END_OF_LIST()

Dont think theres enough state here. The counter value should be here.

> +    }
> +};
> +
> +static Property a9mp_globaltimer_properties[] = {
> +    DEFINE_PROP_UINT32("num-cpu", a9globaltimerState, num_cpu, 0),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void a9mp_globaltimer_class_init(ObjectClass *klass, void *data)
>
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = a9mp_globaltimer_realize;
> +    dc->vmsd = &vmstate_a9mp_globaltimer;
> +    dc->reset = a9mp_globaltimer_reset;
>
> +    dc->no_user = 1;
> +    dc->props = a9mp_globaltimer_properties;
> +}
> +
> +static const TypeInfo a9mp_globaltimer_info = {
> +    .name          = "a9_globaltimer",

Macroified.

Regards,
Peter

> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(a9globaltimerState),
> +    .class_init    = a9mp_globaltimer_class_init,
> +};
> +
> +static void a9mp_globaltimer_register_types(void)
> +{
> +    type_register_static(&a9mp_globaltimer_info);
> +}
> +
> +type_init(a9mp_globaltimer_register_types)
> diff -urN qemu-master.old/hw/timer/Makefile.objs
> qemu-master/hw/timer/Makefile.objs
> --- qemu-master.old/hw/timer/Makefile.objs      2013-04-08
> 20:12:33.000000000 +0200
> +++ qemu-master/hw/timer/Makefile.objs  2013-04-16 13:53:09.000000000 +0200
>
> @@ -24,5 +24,5 @@
>  obj-$(CONFIG_SH4) += sh_timer.o
>  obj-$(CONFIG_TUSB6010) += tusb6010.o
>
> -obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
> +obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o a9gtimer.o
>
>  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
>
> Signed-off-by: François LEGAL <devel@thom.fr.eu.org>
>
>

Patch

diff -urN qemu-master.old/hw/cpu/a9mpcore.c qemu-master/hw/cpu/a9mpcore.c
--- qemu-master.old/hw/cpu/a9mpcore.c	2013-04-08 20:12:33.000000000 +0200
+++ qemu-master/hw/cpu/a9mpcore.c	2013-04-16 13:18:39.000000000 +0200
@@ -15,6 +15,7 @@ 
      uint32_t num_cpu;
      MemoryRegion container;
      DeviceState *mptimer;
+    DeviceState *mpgtimer;
      DeviceState *wdt;
      DeviceState *gic;
      DeviceState *scu;
@@ -31,6 +32,7 @@ 
  {
      A9MPPrivState *s = FROM_SYSBUS(A9MPPrivState, dev);
      SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
+    SysBusDevice *gtimerbusdev;
      int i;

      s->gic = qdev_create(NULL, "arm_gic");
@@ -50,6 +52,11 @@ 
      qdev_init_nofail(s->scu);
      scubusdev = SYS_BUS_DEVICE(s->scu);

+    s->mpgtimer = qdev_create(NULL, "a9_globaltimer");
+    qdev_prop_set_uint32(s->mpgtimer, "num-cpu", s->num_cpu);
+    qdev_init_nofail(s->mpgtimer);
+    gtimerbusdev = SYS_BUS_DEVICE(s->mpgtimer);
+
      s->mptimer = qdev_create(NULL, "arm_mptimer");
      qdev_prop_set_uint32(s->mptimer, "num-cpu", s->num_cpu);
      qdev_init_nofail(s->mptimer);
@@ -68,8 +75,6 @@ 
       *  0x0600-0x06ff -- private timers and watchdogs
       *  0x0700-0x0fff -- nothing
       *  0x1000-0x1fff -- GIC Distributor
-     *
-     * We should implement the global timer but don't currently do so.
       */
      memory_region_init(&s->container, "a9mp-priv-container", 0x2000);
      memory_region_add_subregion(&s->container, 0,
@@ -80,6 +85,8 @@ 
      /* Note that the A9 exposes only the "timer/watchdog for this core"
       * memory region, not the "timer/watchdog for core X" ones 11MPcore 
has.
       */
+    memory_region_add_subregion(&s->container, 0x200,
+                                sysbus_mmio_get_region(gtimerbusdev, 0));
      memory_region_add_subregion(&s->container, 0x600,
                                  sysbus_mmio_get_region(timerbusdev, 0));
      memory_region_add_subregion(&s->container, 0x620,
@@ -90,10 +97,13 @@ 
      sysbus_init_mmio(dev, &s->container);

      /* Wire up the interrupt from each watchdog and timer.
-     * For each core the timer is PPI 29 and the watchdog PPI 30.
+     * For each core the global timer is PPI 27, the private
+     * timer is PPI 29 and the watchdog PPI 30.
       */
      for (i = 0; i < s->num_cpu; i++) {
          int ppibase = (s->num_irq - 32) + i * 32;
+        sysbus_connect_irq(gtimerbusdev, i,
+                           qdev_get_gpio_in(s->gic, ppibase + 27));
          sysbus_connect_irq(timerbusdev, i,
                             qdev_get_gpio_in(s->gic, ppibase + 29));
          sysbus_connect_irq(wdtbusdev, i,
diff -urN qemu-master.old/hw/timer/a9gtimer.c 
qemu-master/hw/timer/a9gtimer.c
--- qemu-master.old/hw/timer/a9gtimer.c	1970-01-01 01:00:00.000000000 +0100
+++ qemu-master/hw/timer/a9gtimer.c	2013-04-16 14:35:48.000000000 +0200
@@ -0,0 +1,348 @@ 
+/*
+ * Global peripheral timer block for ARM A9MP
+ *
+ * Written by François LEGAL
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/sysbus.h"
+#include "qemu/timer.h"
+
+/* This device implements the per-cpu private timer and watchdog block
+ * which is used in the Cortex-A9MP.
+ */
+
+#define MAX_CPUS 4
+#define TYPE_GTIMER "a9_globaltimer"
+#define GTIMER(obj) OBJECT_CHECK(a9globaltimerState, (obj), TYPE_GTIMER)
+
+/* State of a single gtimer or block */
+typedef struct {
+    uint32_t control;
+    uint64_t compare;
+    uint32_t inc;
+    uint32_t status;
+    int64_t  tick;
+
+    int64_t    delta;
+
+    struct a9globaltimerState *gtimer_state;
+    QEMUTimer *timer;
+    MemoryRegion iomem;
+    qemu_irq irq;
+} gTimerBlock;
+
+typedef struct a9globaltimerState {
+    SysBusDevice busdev;
+    uint32_t num_cpu;
+    uint64_t gtimer_counter;
+    uint32_t gtimer_control;
+    gTimerBlock gtimer[MAX_CPUS];
+    MemoryRegion iomem;
+} a9globaltimerState;
+
+static inline int get_current_cpu(a9globaltimerState *s)
+{
+    CPUState *cpu_single_cpu = ENV_GET_CPU(cpu_single_env);
+
+    if (cpu_single_cpu->cpu_index >= s->num_cpu) {
+        hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n",
+                 s->num_cpu, cpu_single_cpu->cpu_index);
+    }
+    return cpu_single_cpu->cpu_index;
+}
+
+static inline uint32_t gtimerblock_get_control_reg(gTimerBlock *gtb)
+{
+    return gtb->control | gtb->gtimer_state->gtimer_control;
+}
+
+static inline void gtimerblock_update_irq(gTimerBlock *gtb)
+{
+    qemu_set_irq(gtb->irq, gtb->status);
+}
+
+/* Return conversion factor from mpcore timer ticks to qemu timer ticks.  
*/
+static inline uint32_t gtimerblock_scale(gTimerBlock *gtb)
+{
+    return (((gtb->gtimer_state->gtimer_control >> 8) & 0xff) + 1) * 10;
+}
+
+static void gtimerblock_reload(gTimerBlock *gtb, int restart)
+{
+    if (restart) {
+        gtb->tick = qemu_get_clock_ns(vm_clock);
+        gtb->tick += (int64_t)(((gtb->compare -
+                                 gtb->gtimer_state->gtimer_counter) +
+                                 gtb->delta) * gtimerblock_scale(gtb));
+    } else {
+        gtb->tick += (int64_t)(gtb->inc * gtimerblock_scale(gtb));
+    }
+    qemu_mod_timer(gtb->timer, gtb->tick);
+}
+
+static void gtimerblock_tick(void *opaque)
+{
+    gTimerBlock *gtb = (gTimerBlock *)opaque;
+    gtb->gtimer_state->gtimer_counter = gtb->compare;
+    if (gtimerblock_get_control_reg(gtb) & 0x9) {
+        gtb->compare += gtb->inc;
+        gtimerblock_reload(gtb, 0);
+    }
+    if ((gtimerblock_get_control_reg(gtb) & 0x7) &&
+        ((gtb->status & 1) == 0)) {
+        gtb->status = 1;
+        gtimerblock_update_irq(gtb);
+    }
+}
+
+static uint64_t gtimer_read(void *opaque, hwaddr addr,
+                                unsigned size)
+{
+    gTimerBlock *gtb = (gTimerBlock *)opaque;
+    int64_t val = 0;
+    switch (addr) {
+    case 0: /* Counter LSB */
+        if (gtimerblock_get_control_reg(gtb) & 1)  {
+            val = gtb->tick - qemu_get_clock_ns(vm_clock);
+            val /= gtimerblock_scale(gtb);
+        }
+        return val < 0 ? val : 0 & 0xFFFFFFFF;
+    case 4: /* Counter MSB.  */
+        if (gtimerblock_get_control_reg(gtb) & 1)  {
+            val = gtb->tick - qemu_get_clock_ns(vm_clock);
+            val /= gtimerblock_scale(gtb);
+        }
+        return val < 0 ? (val >> 32) : 0;
+    case 8: /* Control.  */
+        return gtimerblock_get_control_reg(gtb) & 0x0000FF0F;
+    case 12: /* Interrupt status.  */
+        return gtb->status;
+    case 16: /* Compare LSB */
+        return gtb->compare & 0xFFFFFFFF;
+    case 20: /* Counter MSB  */
+        return gtb->compare >> 32;
+    case 24: /* Autoincrement */
+        return gtb->inc;
+    default:
+        return 0;
+    }
+}
+
+static void gtimer_write(void *opaque, hwaddr addr,
+                             uint64_t value, unsigned size)
+{
+    gTimerBlock *gtb = (gTimerBlock *)opaque;
+    int64_t old;
+    switch (addr) {
+    case 0: /* Counter LSB */
+        old = gtb->gtimer_state->gtimer_counter;
+        old &= 0xFFFFFFFF;
+        gtb->delta = old - (value & 0xFFFFFFFF);
+        old |= value & 0xFFFFFFFF;
+        gtb->gtimer_state->gtimer_counter = old;
+        /* Cancel the previous timer.  */
+        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
+            gtimerblock_reload(gtb, 1);
+        }
+        break;
+    case 4: /* Counter MSB.  */
+        old = gtb->gtimer_state->gtimer_counter;
+        old &= 0xFFFFFFFF00000000;
+        gtb->delta = old - (value << 32);
+        old |= value << 32;
+        gtb->gtimer_state->gtimer_counter = old;
+        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
+            gtimerblock_reload(gtb, 1);
+        }
+        break;
+    case 8: /* Control.  */
+        old = gtb->gtimer_state->gtimer_control;
+        gtb->control = value & 0x0000000E;
+        gtb->gtimer_state->gtimer_control = value & 0x0000FF01;
+        if (((old & 1) == 0) && ((value & 7) == 7)) {
+            gtb->delta = 0;
+            gtimerblock_reload(gtb, 1);
+        }
+        break;
+    case 12: /* Interrupt status.  */
+        gtb->status &= ~ value;
+        gtimerblock_update_irq(gtb);
+        break;
+    case 16: /* Compare LSB */
+        old = gtb->compare;
+        old &= 0xFFFFFFFF;
+        gtb->delta = (value & 0xFFFFFFFF) - old;
+        old |= value & 0xFFFFFFFF;
+        gtb->compare = old;
+        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
+            gtimerblock_reload(gtb, 1);
+        }
+        break;
+    case 20: /* Compare MSB.  */
+        old = gtb->compare;
+        old &= 0xFFFFFFFF00000000;
+        gtb->delta = (value << 32) - old;
+        old |= value << 32;
+        gtb->compare = old;
+        if ((gtimerblock_get_control_reg(gtb) & 7) == 7) {
+            gtimerblock_reload(gtb, 1);
+        }
+        break;
+    case 24: /* Autoincrement */
+        gtb->inc = value;
+        break;
+    default:
+        break;
+    }
+}
+
+/* Wrapper functions to implement the "read global timer for
+ * the current CPU" memory regions.
+ */
+static uint64_t arm_thisgtimer_read(void *opaque, hwaddr addr,
+                                    unsigned size)
+{
+    a9globaltimerState *s = (a9globaltimerState *)opaque;
+    int id = get_current_cpu(s);
+    return gtimer_read(&s->gtimer[id], addr, size);
+}
+
+static void arm_thisgtimer_write(void *opaque, hwaddr addr,
+                                 uint64_t value, unsigned size)
+{
+    a9globaltimerState *s = (a9globaltimerState *)opaque;
+    int id = get_current_cpu(s);
+    gtimer_write(&s->gtimer[id], addr, value, size);
+}
+
+static const MemoryRegionOps arm_thisgtimer_ops = {
+    .read = arm_thisgtimer_read,
+    .write = arm_thisgtimer_write,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const MemoryRegionOps gtimerblock_ops = {
+    .read = gtimer_read,
+    .write = gtimer_write,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void gtimer_reset(gTimerBlock *gtb)
+{
+    if (gtb->timer) {
+        qemu_del_timer(gtb->timer);
+    }
+    gtb->control = 0;
+    gtb->status = 0;
+    gtb->compare = 0;
+    gtb->inc = 0;
+    gtb->tick = 0;
+}
+
+static void a9mp_globaltimer_reset(DeviceState *dev)
+{
+    a9globaltimerState *s = GTIMER(dev);
+    int i;
+    for (i = 0; i < ARRAY_SIZE(s->gtimer); i++) {
+        gtimer_reset(&s->gtimer[i]);
+    }
+}
+
+static void a9mp_globaltimer_realize(DeviceState *dev, Error **errp)
+{
+    a9globaltimerState *s = GTIMER(dev);
+    int i;
+
+    if (s->num_cpu < 1 || s->num_cpu > MAX_CPUS) {
+        error_setg(errp, "%s: num-cpu must be between 1 and %d\n",
+                   __func__, MAX_CPUS);
+    }
+
+    memory_region_init_io(&s->iomem, &arm_thisgtimer_ops, s,
+                          "arm_mptimer_gtimer", 0x20);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
+    for (i = 0; i < s->num_cpu; i++) {
+        gTimerBlock *gtb = &s->gtimer[i];
+        gtb->gtimer_state = s;
+        gtb->timer = qemu_new_timer_ns(vm_clock, gtimerblock_tick, gtb);
+        sysbus_init_irq(SYS_BUS_DEVICE(dev), &gtb->irq);
+        memory_region_init_io(&gtb->iomem, &gtimerblock_ops, gtb,
+                              "arm_mptimer_gtimerblock", 0x20);
+        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &gtb->iomem);
+    }
+}
+
+static const VMStateDescription vmstate_gtimerblock = {
+    .name = "a9mp_gtimerblock",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(control, gTimerBlock),
+        VMSTATE_UINT32(status, gTimerBlock),
+        VMSTATE_UINT64(compare, gTimerBlock),
+        VMSTATE_INT64(tick, gTimerBlock),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_a9mp_globaltimer = {
+    .name = "a9mp_globaltimer",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+    VMSTATE_STRUCT_VARRAY_UINT32(gtimer, a9globaltimerState, num_cpu,
+                                 1, vmstate_gtimerblock, gTimerBlock),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property a9mp_globaltimer_properties[] = {
+    DEFINE_PROP_UINT32("num-cpu", a9globaltimerState, num_cpu, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void a9mp_globaltimer_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = a9mp_globaltimer_realize;
+    dc->vmsd = &vmstate_a9mp_globaltimer;
+    dc->reset = a9mp_globaltimer_reset;
+    dc->no_user = 1;
+    dc->props = a9mp_globaltimer_properties;
+}
+
+static const TypeInfo a9mp_globaltimer_info = {
+    .name          = "a9_globaltimer",
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(a9globaltimerState),
+    .class_init    = a9mp_globaltimer_class_init,
+};
+
+static void a9mp_globaltimer_register_types(void)
+{
+    type_register_static(&a9mp_globaltimer_info);
+}
+