[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2] ARM Cortex A9 Global Timer
|
From: |
François Legal |
|
Subject: |
Re: [Qemu-devel] [PATCH V2] ARM Cortex A9 Global Timer |
|
Date: |
Tue, 16 Apr 2013 14:50:41 +0200 |
|
User-agent: |
Roundcube Webmail/0.8.5 |
Le 16-04-2013 14:19, Peter Maydell a écrit :
On 16 April 2013 13:09, François Legal <address@hidden> 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.
---
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), >b->irq);
+ memory_region_init_io(>b->iomem, >imerblock_ops, gtb,
+ "arm_mptimer_gtimerblock", 0x20);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), >b->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 <address@hidden>
Re: [Qemu-devel] [PATCH] ARM Cortex A9 Global Timer, Peter Crosthwaite, 2013/04/16