From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35897) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VnScY-0002bH-HA for qemu-devel@nongnu.org; Mon, 02 Dec 2013 07:29:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VnScU-0004fD-Et for qemu-devel@nongnu.org; Mon, 02 Dec 2013 07:29:02 -0500 Received: from cantor2.suse.de ([195.135.220.15]:40972 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VnScT-0004f9-Q2 for qemu-devel@nongnu.org; Mon, 02 Dec 2013 07:28:58 -0500 Message-ID: <529C7D03.4080109@suse.de> Date: Mon, 02 Dec 2013 13:28:51 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <4918e89476b8da916be2964ec41578b50d569a37.1385969450.git.peter.crosthwaite@xilinx.com> In-Reply-To: <4918e89476b8da916be2964ec41578b50d569a37.1385969450.git.peter.crosthwaite@xilinx.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH arm-devs v4 3/4] hw/timer: Introduce ARM A9 Global Timer. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite , qemu-devel@nongnu.org, peter.maydell@linaro.org Cc: Markus Armbruster Am 02.12.2013 08:36, schrieb Peter Crosthwaite: > The ARM A9 MPCore has a timer that is global to all CPUs in the mpcore. > The timer is shared but each CPU has a private independent comparator > and interrupt. >=20 > Based on version contributed by Francois LEGAL. >=20 > Signed-off-by: Fran=C3=A7ois LEGAL > [PC changes: > * New commit message > * Re-implemented as single timer model > * Fixed backwards counting issue in polled mode > * completed VMSD fields > * macroified magic numbers (and headerified reg definitions) > * split of as device-model-only patch "off" > * use bitops for 64 bit register access > * Fixed auto increment mode to check condition properly > * general cleanup (names/style etc). > ] > Signed-off-by: Peter Crosthwaite > --- > Changed sinve v3: > Add seperate CONFIG_A9_GTIMER (PMM review) > Changed since v2 (PMM - review): > Refactored for container embedding > Made frequency scaler consistent with mptimer > Fixed missing VMSD ref_counter field > Fixed missing gtb->inc reset > Changed Memory region names > Changed since v1: > Added /*< private >*/ to struct definition. > Pulled register definitions out into a header (AF review) > SOB Francois LEGAL with PC changes indicated. >=20 > default-configs/arm-softmmu.mak | 1 + > hw/timer/Makefile.objs | 1 + > hw/timer/a9gtimer.c | 369 ++++++++++++++++++++++++++++++++= ++++++++ > include/hw/timer/a9gtimer.h | 97 +++++++++++ > 4 files changed, 468 insertions(+) > create mode 100644 hw/timer/a9gtimer.c > create mode 100644 include/hw/timer/a9gtimer.h >=20 > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-soft= mmu.mak > index a555eef..e48f102 100644 > --- a/default-configs/arm-softmmu.mak > +++ b/default-configs/arm-softmmu.mak > @@ -41,6 +41,7 @@ CONFIG_ARM_GIC=3Dy > CONFIG_ARM_GIC_KVM=3D$(CONFIG_KVM) > CONFIG_ARM_TIMER=3Dy > CONFIG_ARM_MPTIMER=3Dy > +CONFIG_A9_GTIMER=3Dy > CONFIG_PL011=3Dy > CONFIG_PL022=3Dy > CONFIG_PL031=3Dy > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs > index eca5905..3ae091c 100644 > --- a/hw/timer/Makefile.objs > +++ b/hw/timer/Makefile.objs > @@ -1,5 +1,6 @@ > common-obj-$(CONFIG_ARM_TIMER) +=3D arm_timer.o > common-obj-$(CONFIG_ARM_MPTIMER) +=3D arm_mptimer.o > +common-obj-$(CONFIG_A9_GTIMER) +=3D a9gtimer.o > common-obj-$(CONFIG_CADENCE) +=3D cadence_ttc.o > common-obj-$(CONFIG_DS1338) +=3D ds1338.o > common-obj-$(CONFIG_HPET) +=3D hpet.o > diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c > new file mode 100644 > index 0000000..0f08c67 > --- /dev/null > +++ b/hw/timer/a9gtimer.c > @@ -0,0 +1,369 @@ > +/* > + * Global peripheral timer block for ARM A9MP > + * > + * (C) 2013 Xilinx Inc. > + * > + * Written by Fran=C3=A7ois LEGAL > + * Written by Peter Crosthwaite > + * > + * 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 a= long > + * with this program; if not, see . > + */ > + > +#include "hw/timer/a9gtimer.h" > +#include "qemu/timer.h" > +#include "qemu/bitops.h" > +#include "qemu/log.h" > + > +#ifndef A9_GTIMER_ERR_DEBUG > +#define A9_GTIMER_ERR_DEBUG 0 > +#endif > + > +#define DB_PRINT_L(level, ...) do { \ > + if (A9_GTIMER_ERR_DEBUG > (level)) { \ > + fprintf(stderr, ": %s: ", __func__); \ > + fprintf(stderr, ## __VA_ARGS__); \ > + } \ > +} while (0); > + > +#define DB_PRINT(...) DB_PRINT_L(0, ## __VA_ARGS__) > + > +static inline int a9_gtimer_get_current_cpu(A9GTimerState *s) > +{ > + if (current_cpu->cpu_index >=3D s->num_cpu) { > + hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n", Copy&paste? > + s->num_cpu, current_cpu->cpu_index); > + } > + return current_cpu->cpu_index; > +} > + > +static inline uint64_t a9_gtimer_get_conv(A9GTimerState *s) > +{ > + uint64_t prescale =3D extract32(s->control, R_CONTROL_PRESCALER_SH= IFT, > + R_CONTROL_PRESCALER_LEN); > + > + return (prescale + 1) * 10; > +} > + > +static A9GTimerUpdate a9_gtimer_get_update(A9GTimerState *s) > +{ > + A9GTimerUpdate ret; > + > + ret.now =3D qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + ret.new =3D s->ref_counter + > + (ret.now - s->cpu_ref_time) / a9_gtimer_get_conv(s); > + return ret; > +} > + > +static void a9_gtimer_update(A9GTimerState *s, bool sync) > +{ > + > + A9GTimerUpdate update =3D a9_gtimer_get_update(s); > + int i; > + int64_t next_cdiff =3D 0; > + > + for (i =3D 0; i < s->num_cpu; ++i) { > + A9GTimerPerCPU *gtb =3D &s->per_cpu[i]; > + int64_t cdiff =3D 0; > + > + if ((s->control & R_CONTROL_TIMER_ENABLE) && > + (gtb->control & R_CONTROL_COMP_ENABLE)) { > + /* R2p0+, where the compare function is >=3D */ > + while (gtb->compare < update.new) { > + DB_PRINT("Compare event happened for CPU %d\n", i); > + gtb->status =3D 1; > + if (gtb->control & R_CONTROL_AUTO_INCREMENT) { > + DB_PRINT("Auto incrementing timer compare by %" PR= Id32 "\n", > + gtb->inc); > + gtb->compare +=3D gtb->inc; > + } else { > + break; > + } > + } > + cdiff =3D (int64_t)gtb->compare - (int64_t)update.new + 1; > + if (cdiff > 0 && (cdiff < next_cdiff || !next_cdiff)) { > + next_cdiff =3D cdiff; > + } > + } > + > + qemu_set_irq(gtb->irq, > + gtb->status && (gtb->control & R_CONTROL_IRQ_ENAB= LE)); > + } > + > + timer_del(s->timer); > + if (next_cdiff) { > + DB_PRINT("scheduling qemu_timer to fire again in %" > + PRIx64 " cycles\n", next_cdiff); > + timer_mod(s->timer, update.now + next_cdiff * a9_gtimer_get_co= nv(s)); > + } > + > + if (s->control & R_CONTROL_TIMER_ENABLE) { > + s->counter =3D update.new; > + } > + > + if (sync) { > + s->cpu_ref_time =3D update.now; > + s->ref_counter =3D s->counter; > + } > +} > + > +static void a9_gtimer_update_no_sync(void *opaque) > +{ > + A9GTimerState *s =3D A9_GTIMER(opaque); > + > + return a9_gtimer_update(s, false); > +} > + > +static uint64_t a9_gtimer_read(void *opaque, hwaddr addr, unsigned siz= e) > +{ > + A9GTimerPerCPU *gtb =3D (A9GTimerPerCPU *)opaque; > + A9GTimerState *s =3D gtb->parent; > + A9GTimerUpdate update; > + uint64_t ret =3D 0; > + int shift =3D 0; > + > + switch (addr) { > + case R_COUNTER_HI: > + shift =3D 32; > + /* fallthrough */ > + case R_COUNTER_LO: > + update =3D a9_gtimer_get_update(s); > + ret =3D extract64(update.new, shift, 32); > + break; > + case R_CONTROL: > + ret =3D s->control | gtb->control; > + break; > + case R_INTERRUPT_STATUS: > + ret =3D gtb->status; > + break; > + case R_COMPARATOR_HI: > + shift =3D 32; > + /* fallthrough */ > + case R_COMPARATOR_LO: > + ret =3D extract64(gtb->compare, shift, 32); > + break; > + case R_AUTO_INCREMENT: > + ret =3D gtb->inc; > + break; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, "bad a9gtimer register: %x\n", > + (unsigned)addr); > + return 0; > + } > + > + DB_PRINT("addr:%#x data:%#08" PRIx64 "\n", (unsigned)addr, ret); > + return ret; > +} > + > +static void a9_gtimer_write(void *opaque, hwaddr addr, uint64_t value, > + unsigned size) > +{ > + A9GTimerPerCPU *gtb =3D (A9GTimerPerCPU *)opaque; > + A9GTimerState *s =3D gtb->parent; > + int shift =3D 0; > + > + DB_PRINT("addr:%#x data:%#08" PRIx64 "\n", (unsigned)addr, value); > + > + switch (addr) { > + case R_COUNTER_HI: > + shift =3D 32; > + /* fallthrough */ > + case R_COUNTER_LO: > + /* > + * Keep it simple - ARM docco explicitly says to disable timer= before > + * modding it, so dont bother trying to do all the difficult o= n the fly > + * timer modifications - (if they even work in real hardware??= ). > + */ > + if (s->control & R_CONTROL_TIMER_ENABLE) { > + qemu_log_mask(LOG_GUEST_ERROR, "Cannot mod running ARM gti= mer\n"); > + return; > + } > + s->counter =3D deposit64(s->counter, shift, 32, value); > + return; > + case R_CONTROL: > + a9_gtimer_update(s, (value ^ s->control) & R_CONTROL_NEEDS_SYN= C); > + gtb->control =3D value & R_CONTROL_BANKED; > + s->control =3D value & ~R_CONTROL_BANKED; > + break; > + case R_INTERRUPT_STATUS: > + a9_gtimer_update(s, false); > + gtb->status &=3D ~value; > + break; > + case R_COMPARATOR_HI: > + shift =3D 32; > + /* fallthrough */ > + case R_COMPARATOR_LO: > + a9_gtimer_update(s, false); > + gtb->compare =3D deposit64(gtb->compare, shift, 32, value); > + break; > + case R_AUTO_INCREMENT: > + gtb->inc =3D value; > + return; > + default: > + return; > + } > + > + a9_gtimer_update(s, false); > +} > + > +/* Wrapper functions to implement the "read global timer for > + * the current CPU" memory regions. > + */ > +static uint64_t a9_gtimer_this_read(void *opaque, hwaddr addr, > + unsigned size) > +{ > + A9GTimerState *s =3D A9_GTIMER(opaque); > + int id =3D a9_gtimer_get_current_cpu(s); > + > + /* no \n so concatenates with message from read fn */ > + DB_PRINT("CPU:%d:", id); > + > + return a9_gtimer_read(&s->per_cpu[id], addr, size); > +} > + > +static void a9_gtimer_this_write(void *opaque, hwaddr addr, > + uint64_t value, unsigned size) > +{ > + A9GTimerState *s =3D A9_GTIMER(opaque); > + int id =3D a9_gtimer_get_current_cpu(s); > + > + /* no \n so concatentates with message from write fn */ "concatenates" > + DB_PRINT("CPU:%d:", id); > + > + a9_gtimer_write(&s->per_cpu[id], addr, value, size); > +} > + > +static const MemoryRegionOps a9_gtimer_this_ops =3D { > + .read =3D a9_gtimer_this_read, > + .write =3D a9_gtimer_this_write, > + .valid =3D { > + .min_access_size =3D 4, > + .max_access_size =3D 4, > + }, > + .endianness =3D DEVICE_NATIVE_ENDIAN, > +}; > + > +static const MemoryRegionOps a9_gtimer_ops =3D { > + .read =3D a9_gtimer_read, > + .write =3D a9_gtimer_write, > + .valid =3D { > + .min_access_size =3D 4, > + .max_access_size =3D 4, > + }, > + .endianness =3D DEVICE_NATIVE_ENDIAN, > +}; > + > +static void a9_gtimer_reset(DeviceState *dev) > +{ > + A9GTimerState *s =3D A9_GTIMER(dev); > + int i; > + > + s->counter =3D 0; > + s->control =3D 0; > + > + for (i =3D 0; i < s->num_cpu; i++) { > + A9GTimerPerCPU *gtb =3D &s->per_cpu[i]; > + > + gtb->control =3D 0; > + gtb->status =3D 0; > + gtb->compare =3D 0; > + gtb->inc =3D 0; > + } > + a9_gtimer_update(s, false); > +} > + > +static void a9_gtimer_realize(DeviceState *dev, Error **errp) > +{ > + A9GTimerState *s =3D A9_GTIMER(dev); > + SysBusDevice *sbd =3D SYS_BUS_DEVICE(dev); > + int i; > + > + if (s->num_cpu < 1 || s->num_cpu > A9_GTIMER_MAX_CPUS) { > + error_setg(errp, "%s: num-cpu must be between 1 and %d\n", > + __func__, A9_GTIMER_MAX_CPUS); return; missing. > + } > + > + memory_region_init_io(&s->iomem, OBJECT(dev), &a9_gtimer_this_ops,= s, > + "a9gtimer shared", 0x20); > + sysbus_init_mmio(sbd, &s->iomem); > + s->timer =3D timer_new_ns(QEMU_CLOCK_VIRTUAL, a9_gtimer_update_no_= sync, s); > + > + for (i =3D 0; i < s->num_cpu; i++) { > + A9GTimerPerCPU *gtb =3D &s->per_cpu[i]; > + > + gtb->parent =3D s; > + sysbus_init_irq(sbd, >b->irq); > + memory_region_init_io(>b->iomem, OBJECT(dev), &a9_gtimer_ops= , gtb, > + "a9gtimer per cpu", 0x20); > + sysbus_init_mmio(sbd, >b->iomem); > + } > +} > + > +static const VMStateDescription vmstate_a9_gtimer_per_cpu =3D { > + .name =3D "arm.cortex-a9-global-timer.percpu", > + .version_id =3D 1, > + .minimum_version_id =3D 1, > + .fields =3D (VMStateField[]) { > + VMSTATE_UINT32(control, A9GTimerPerCPU), > + VMSTATE_UINT64(compare, A9GTimerPerCPU), > + VMSTATE_UINT32(status, A9GTimerPerCPU), > + VMSTATE_UINT32(inc, A9GTimerPerCPU), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_a9_gtimer =3D { > + .name =3D "arm.cortex-a9-global-timer", > + .version_id =3D 1, > + .minimum_version_id =3D 1, > + .fields =3D (VMStateField[]) { > + VMSTATE_TIMER(timer, A9GTimerState), > + VMSTATE_UINT64(counter, A9GTimerState), > + VMSTATE_UINT64(ref_counter, A9GTimerState), > + VMSTATE_UINT64(cpu_ref_time, A9GTimerState), > + VMSTATE_STRUCT_VARRAY_UINT32(per_cpu, A9GTimerState, num_cpu, > + 1, vmstate_a9_gtimer_per_cpu, > + A9GTimerPerCPU), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static Property a9_gtimer_properties[] =3D { > + DEFINE_PROP_UINT32("num-cpu", A9GTimerState, num_cpu, 0), > + DEFINE_PROP_END_OF_LIST() > +}; > + > +static void a9_gtimer_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(klass); > + > + dc->realize =3D a9_gtimer_realize; > + dc->vmsd =3D &vmstate_a9_gtimer; > + dc->reset =3D a9_gtimer_reset; > + dc->no_user =3D 1; There is a series from Markus waiting on my review that renames and investigates all uses of this field. Please explain why you are using it (in form of a comment) or drop it. There's only a numeric property above, so likely the latter. On a general note, please be aware that use of HWADDR_PRIx is preferred over the (unsigned) casts I'm seeing in this patch. Andreas > + dc->props =3D a9_gtimer_properties; > +} > + > +static const TypeInfo a9_gtimer_info =3D { > + .name =3D TYPE_A9_GTIMER, > + .parent =3D TYPE_SYS_BUS_DEVICE, > + .instance_size =3D sizeof(A9GTimerState), > + .class_init =3D a9_gtimer_class_init, > +}; > + > +static void a9_gtimer_register_types(void) > +{ > + type_register_static(&a9_gtimer_info); > +} > + > +type_init(a9_gtimer_register_types) > diff --git a/include/hw/timer/a9gtimer.h b/include/hw/timer/a9gtimer.h > new file mode 100644 > index 0000000..b88c02a > --- /dev/null > +++ b/include/hw/timer/a9gtimer.h > @@ -0,0 +1,97 @@ > +/* > + * Global peripheral timer block for ARM A9MP > + * > + * (C) 2013 Xilinx Inc. > + * > + * Written by Fran=C3=A7ois LEGAL > + * Written by Peter Crosthwaite > + * > + * 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 a= long > + * with this program; if not, see . > + */ > + > +#ifndef HW_TIMER_A9_GTIMER_H_H > +#define HW_TIMER_A9_GTIMER_H_H > + > +#include "hw/sysbus.h" > + > +#define A9_GTIMER_MAX_CPUS 4 > + > +#define TYPE_A9_GTIMER "arm.cortex-a9-global-timer" > +#define A9_GTIMER(obj) OBJECT_CHECK(A9GTimerState, (obj), TYPE_A9_GTIM= ER) > + > +#define R_COUNTER_LO 0x00 > +#define R_COUNTER_HI 0x04 > + > +#define R_CONTROL 0x08 > +#define R_CONTROL_TIMER_ENABLE (1 << 0) > +#define R_CONTROL_COMP_ENABLE (1 << 1) > +#define R_CONTROL_IRQ_ENABLE (1 << 2) > +#define R_CONTROL_AUTO_INCREMENT (1 << 2) > +#define R_CONTROL_PRESCALER_SHIFT 8 > +#define R_CONTROL_PRESCALER_LEN 8 > +#define R_CONTROL_PRESCALER_MASK (((1 << R_CONTROL_PRESCALER_LEN) -= 1) << \ > + R_CONTROL_PRESCALER_SHIFT) > + > +#define R_CONTROL_BANKED (R_CONTROL_COMP_ENABLE | \ > + R_CONTROL_IRQ_ENABLE | \ > + R_CONTROL_AUTO_INCREMENT) > +#define R_CONTROL_NEEDS_SYNC (R_CONTROL_TIMER_ENABLE | \ > + R_CONTROL_PRESCALER_MASK) > + > +#define R_INTERRUPT_STATUS 0x0C > +#define R_COMPARATOR_LO 0x10 > +#define R_COMPARATOR_HI 0x14 > +#define R_AUTO_INCREMENT 0x18 > + > +typedef struct A9GTimerPerCPU A9GTimerPerCPU; > +typedef struct A9GTimerState A9GTimerState; > + > +struct A9GTimerPerCPU { > + A9GTimerState *parent; > + > + uint32_t control; /* only per cpu banked bits valid */ > + uint64_t compare; > + uint32_t status; > + uint32_t inc; > + > + MemoryRegion iomem; > + qemu_irq irq; /* PPI interrupts */ > +}; > + > +struct A9GTimerState { > + /*< private >*/ > + SysBusDevice parent_obj; > + /*< public >*/ > + > + MemoryRegion iomem; > + /* static props */ > + uint32_t num_cpu; > + > + QEMUTimer *timer; > + > + uint64_t counter; /* current timer value */ > + > + uint64_t ref_counter; > + uint64_t cpu_ref_time; /* the cpu time as of last update of ref_co= unter */ > + uint32_t control; /* only non per cpu banked bits valid */ > + > + A9GTimerPerCPU per_cpu[A9_GTIMER_MAX_CPUS]; > +}; > + > +typedef struct A9GTimerUpdate { > + uint64_t now; > + uint64_t new; > +} A9GTimerUpdate; > + > +#endif /* #ifdef HW_TIMER_A9_GTIMER_H_H */ >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg