[Qemu-devel] Timers

[Qemu-devel] Timers

[Paul Brook]

I get fed up of having to re-implement a simple countdown timer for every new 
board, so I've added a simple periodic timer implementation to cvs 
(ptimer.c). Currently only the Arm PrimeCell based boards use this, but I've 
a few other uses in the pipeline.

Paul

Re: [Qemu-devel] Timers

[George G. Davis]

On Wed, May 23, 2007 at 01:06:59AM +0100, Paul Brook wrote:
> I get fed up of having to re-implement a simple countdown timer for every new 
> board, so I've added a simple periodic timer implementation to cvs 
> (ptimer.c). Currently only the Arm PrimeCell based boards use this, but I've 
> a few other uses in the pipeline.

.../qemu/hw/arm_timer.c:25: syntax error before "ptimer_state"
.../qemu/hw/arm_timer.c:25: warning: no semicolon at end of struct or union

Current CVS is missing definition for ptimer_state.


-- 
Regards,
George

    _____   __o   George G. Davis - Software Engineer           (o>
-------    -\<,   MontaVista Sofware - Platform to Innovate     //\
 ------ ( )/ ( )  www.mvista.com                                V_/_
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: [Qemu-devel] Timers

[Paul Brook]

> .../qemu/hw/arm_timer.c:25: syntax error before "ptimer_state"
> .../qemu/hw/arm_timer.c:25: warning: no semicolon at end of struct or union
>
> Current CVS is missing definition for ptimer_state.

Oops, sorry. Should be fixed now.

Paul

Re: [Qemu-devel] Timers

[George G. Davis]

On Wed, May 23, 2007 at 02:14:02AM +0100, Paul Brook wrote:
> > .../qemu/hw/arm_timer.c:25: syntax error before "ptimer_state"
> > .../qemu/hw/arm_timer.c:25: warning: no semicolon at end of struct or union
> >
> > Current CVS is missing definition for ptimer_state.
> 
> Oops, sorry. Should be fixed now.

Works for me now, thanks!

-- 
Regards,
George

Re: [Qemu-devel] Timers

[Blue Swirl]

On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> I get fed up of having to re-implement a simple countdown timer for every new
> board, so I've added a simple periodic timer implementation to cvs
> (ptimer.c). Currently only the Arm PrimeCell based boards use this, but I've
> a few other uses in the pipeline.

Nice idea! On Sparc the timer can be configured to work in 64-bit
mode, so could the ptimer_get/set_count be changed to use 64-bit
values?

Re: [Qemu-devel] Timers

[Paul Brook]

On Wednesday 23 May 2007, Blue Swirl wrote:
> On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> > I get fed up of having to re-implement a simple countdown timer for every
> > new board, so I've added a simple periodic timer implementation to cvs
> > (ptimer.c). Currently only the Arm PrimeCell based boards use this, but
> > I've a few other uses in the pipeline.
>
> Nice idea! On Sparc the timer can be configured to work in 64-bit
> mode, so could the ptimer_get/set_count be changed to use 64-bit
> values?

In principle yes, though you may have to be careful to avoid overflows.

The current API supports specifying either frequency (better for fast, large 
count timers) and period (better for slow, small count timers). We want to 
avoid breaking either extreme when adding 64-bit counters.

Paul

Re: [Qemu-devel] Timers

[George G. Davis]

On Wed, May 23, 2007 at 08:00:24PM +0300, Blue Swirl wrote:
> On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> >I get fed up of having to re-implement a simple countdown timer for every 
> >new
> >board, so I've added a simple periodic timer implementation to cvs
> >(ptimer.c). Currently only the Arm PrimeCell based boards use this, but 
> >I've
> >a few other uses in the pipeline.
> 
> Nice idea! On Sparc the timer can be configured to work in 64-bit
> mode, so could the ptimer_get/set_count be changed to use 64-bit
> values?

Perhaps the width could be made runtime configurable, e.g. 16, 24, 32,
64-bits as required for a given system, since there are other targets
which may need differring widths (mind I haven't looked all too closely
at the code to see if it already handles this : ).  Likewise, perhaps
up/down count can also be made runtime configurable, since, again, some
targets implement up counters.  The logic is all the same in that
case, with just a difference in sign...
> 

-- 
Regards,
George

    _____   __o   George G. Davis - Software Engineer           (o>
-------    -\<,   MontaVista Sofware - Platform to Innovate     //\
 ------ ( )/ ( )  www.mvista.com                                V_/_
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: [Qemu-devel] Timers

[Paul Brook]

> > Nice idea! On Sparc the timer can be configured to work in 64-bit
> > mode, so could the ptimer_get/set_count be changed to use 64-bit
> > values?
>
> Perhaps the width could be made runtime configurable, e.g. 16, 24, 32,
> 64-bits as required for a given system, since there are other targets
> which may need differring widths (mind I haven't looked all too closely
> at the code to see if it already handles this : ).  Likewise, perhaps
> up/down count can also be made runtime configurable, since, again, some
> targets implement up counters.  The logic is all the same in that
> case, with just a difference in sign...

I don't think there's any point. A 64-bit countdown timer is sufficient to 
implement everything, and I'd expect the overhead is going to minimal. 
Converting a count-down timer into a count-up timer is trivial. The ARM 
timers are actually count-up timers.

Paul

Re: [Qemu-devel] Timers

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> On Wednesday 23 May 2007, Blue Swirl wrote:
> > On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> > > I get fed up of having to re-implement a simple countdown timer for every
> > > new board, so I've added a simple periodic timer implementation to cvs
> > > (ptimer.c). Currently only the Arm PrimeCell based boards use this, but
> > > I've a few other uses in the pipeline.
> >
> > Nice idea! On Sparc the timer can be configured to work in 64-bit
> > mode, so could the ptimer_get/set_count be changed to use 64-bit
> > values?
>
> In principle yes, though you may have to be careful to avoid overflows.
>
> The current API supports specifying either frequency (better for fast, large
> count timers) and period (better for slow, small count timers). We want to
> avoid breaking either extreme when adding 64-bit counters.

I made the API change and converted Sparc timers. Looks like it works
(guest clock runs normally), though there are the following messages
on startup:
FIXME: ptimer_set_limit with running timer

Comments? Did I break something?

[-- Attachment #2: generic_timers.diff --]
[-- Type: text/x-diff, Size: 8800 bytes --]

Index: qemu/hw/slavio_timer.c
===================================================================
--- qemu.orig/hw/slavio_timer.c	2007-05-23 18:44:20.000000000 +0000
+++ qemu/hw/slavio_timer.c	2007-05-23 18:46:18.000000000 +0000
@@ -48,61 +48,26 @@
  */
 
 typedef struct SLAVIO_TIMERState {
-    uint32_t limit, count, counthigh;
-    int64_t count_load_time;
-    int64_t expire_time;
-    int64_t stop_time, tick_offset;
+    ptimer_state *timer;
+    uint32_t limit, count, counthigh, reached;
     QEMUTimer *irq_timer;
     int irq;
-    int reached, stopped;
+    int stopped;
     int mode; // 0 = processor, 1 = user, 2 = system
     unsigned int cpu;
     void *intctl;
 } SLAVIO_TIMERState;
 
 #define TIMER_MAXADDR 0x1f
-#define CNT_FREQ 2000000
 
 // Update count, set irq, update expire_time
 static void slavio_timer_get_out(SLAVIO_TIMERState *s)
 {
-    int out;
-    int64_t diff, ticks, count;
-    uint32_t limit;
-
-    // There are three clock tick units: CPU ticks, register units
-    // (nanoseconds), and counter ticks (500 ns).
-    if (s->mode == 1 && s->stopped)
-	ticks = s->stop_time;
-    else
-	ticks = qemu_get_clock(vm_clock) - s->tick_offset;
-
-    out = (ticks > s->expire_time);
-    if (out)
-	s->reached = 0x80000000;
-    // Convert register units to counter ticks
-    limit = s->limit >> 9;
-
-    if (!limit)
-	limit = 0x7fffffff >> 9;
-
-    // Convert cpu ticks to counter ticks
-    diff = muldiv64(ticks - s->count_load_time, CNT_FREQ, ticks_per_sec);
-
-    // Calculate what the counter should be, convert to register
-    // units
-    count = diff % limit;
-    s->count = count << 9;
-    s->counthigh = count >> 22;
-
-    // Expire time: CPU ticks left to next interrupt
-    // Convert remaining counter ticks to CPU ticks
-    s->expire_time = ticks + muldiv64(limit - count, ticks_per_sec, CNT_FREQ);
+    uint64_t count;
 
-    DPRINTF("irq %d limit %d reached %d d %" PRId64 " count %d s->c %x diff %" PRId64 " stopped %d mode %d\n", s->irq, limit, s->reached?1:0, (ticks-s->count_load_time), count, s->count, s->expire_time - ticks, s->stopped, s->mode);
-
-    if (s->mode != 1)
-	pic_set_irq_cpu(s->intctl, s->irq, out, s->cpu);
+    count = ptimer_get_count(s->timer);
+    s->count = count & 0xffffffff;
+    s->counthigh = count >> 32;
 }
 
 // timer callback
@@ -110,11 +75,11 @@
 {
     SLAVIO_TIMERState *s = opaque;
 
-    if (!s->irq_timer)
-        return;
     slavio_timer_get_out(s);
+    DPRINTF("callback: count %x%08x\n", s->counthigh, s->count);
+    s->reached = 0x80000000;
     if (s->mode != 1)
-	qemu_mod_timer(s->irq_timer, s->expire_time);
+	pic_set_irq_cpu(s->intctl, s->irq, 1, s->cpu);
 }
 
 static uint32_t slavio_timer_mem_readl(void *opaque, target_phys_addr_t addr)
@@ -122,6 +87,7 @@
     SLAVIO_TIMERState *s = opaque;
     uint32_t saddr;
 
+    DPRINTF("read " TARGET_FMT_plx "\n", addr);
     saddr = (addr & TIMER_MAXADDR) >> 2;
     switch (saddr) {
     case 0:
@@ -160,12 +126,15 @@
 {
     SLAVIO_TIMERState *s = opaque;
     uint32_t saddr;
+    int reload = 0;
 
+    DPRINTF("write " TARGET_FMT_plx " %08x\n", addr, val);
     saddr = (addr & TIMER_MAXADDR) >> 2;
     switch (saddr) {
     case 0:
 	// set limit, reset counter
-	s->count_load_time = qemu_get_clock(vm_clock);
+        reload = 1;
+	pic_set_irq_cpu(s->intctl, s->irq, 0, s->cpu);
 	// fall through
     case 2:
 	// set limit without resetting counter
@@ -173,18 +142,17 @@
 	    s->limit = 0x7fffffff;
 	else
 	    s->limit = val & 0x7fffffff;
-	slavio_timer_irq(s);
+        ptimer_set_limit(s->timer, s->limit, reload);
 	break;
     case 3:
 	// start/stop user counter
 	if (s->mode == 1) {
 	    if (val & 1) {
-		s->stop_time = qemu_get_clock(vm_clock);
+                ptimer_stop(s->timer);
 		s->stopped = 1;
 	    }
 	    else {
-		if (s->stopped)
-		    s->tick_offset += qemu_get_clock(vm_clock) - s->stop_time;
+                ptimer_run(s->timer, 0);
 		s->stopped = 0;
 	    }
 	}
@@ -218,10 +186,6 @@
     qemu_put_be32s(f, &s->limit);
     qemu_put_be32s(f, &s->count);
     qemu_put_be32s(f, &s->counthigh);
-    qemu_put_be64s(f, &s->count_load_time);
-    qemu_put_be64s(f, &s->expire_time);
-    qemu_put_be64s(f, &s->stop_time);
-    qemu_put_be64s(f, &s->tick_offset);
     qemu_put_be32s(f, &s->irq);
     qemu_put_be32s(f, &s->reached);
     qemu_put_be32s(f, &s->stopped);
@@ -232,16 +196,12 @@
 {
     SLAVIO_TIMERState *s = opaque;
     
-    if (version_id != 1)
+    if (version_id != 2)
         return -EINVAL;
 
     qemu_get_be32s(f, &s->limit);
     qemu_get_be32s(f, &s->count);
     qemu_get_be32s(f, &s->counthigh);
-    qemu_get_be64s(f, &s->count_load_time);
-    qemu_get_be64s(f, &s->expire_time);
-    qemu_get_be64s(f, &s->stop_time);
-    qemu_get_be64s(f, &s->tick_offset);
     qemu_get_be32s(f, &s->irq);
     qemu_get_be32s(f, &s->reached);
     qemu_get_be32s(f, &s->stopped);
@@ -253,13 +213,12 @@
 {
     SLAVIO_TIMERState *s = opaque;
 
-    s->limit = 0;
+    s->limit = 0x7fffffff;
     s->count = 0;
-    s->count_load_time = qemu_get_clock(vm_clock);;
-    s->stop_time = s->count_load_time;
-    s->tick_offset = 0;
     s->reached = 0;
     s->mode &= 2;
+    ptimer_set_limit(s->timer, s->limit, 1);
+    ptimer_run(s->timer, 0);
     s->stopped = 1;
     slavio_timer_irq(s);
 }
@@ -269,6 +228,7 @@
 {
     int slavio_timer_io_memory;
     SLAVIO_TIMERState *s;
+    QEMUBH *bh;
 
     s = qemu_mallocz(sizeof(SLAVIO_TIMERState));
     if (!s)
@@ -276,13 +236,15 @@
     s->irq = irq;
     s->mode = mode;
     s->cpu = cpu;
-    s->irq_timer = qemu_new_timer(vm_clock, slavio_timer_irq, s);
+    bh = qemu_bh_new(slavio_timer_irq, s);
+    s->timer = ptimer_init(bh);
+    ptimer_set_period(s->timer, 1ULL);
     s->intctl = intctl;
 
     slavio_timer_io_memory = cpu_register_io_memory(0, slavio_timer_mem_read,
 						    slavio_timer_mem_write, s);
     cpu_register_physical_memory(addr, TIMER_MAXADDR, slavio_timer_io_memory);
-    register_savevm("slavio_timer", addr, 1, slavio_timer_save, slavio_timer_load, s);
+    register_savevm("slavio_timer", addr, 2, slavio_timer_save, slavio_timer_load, s);
     qemu_register_reset(slavio_timer_reset, s);
     slavio_timer_reset(s);
 }
Index: qemu/hw/ptimer.c
===================================================================
--- qemu.orig/hw/ptimer.c	2007-05-23 18:44:20.000000000 +0000
+++ qemu/hw/ptimer.c	2007-05-23 18:46:18.000000000 +0000
@@ -11,8 +11,8 @@
 struct ptimer_state
 {
     int enabled; /* 0 = disabled, 1 = periodic, 2 = oneshot.  */
-    uint32_t limit;
-    uint32_t delta;
+    uint64_t limit;
+    uint64_t delta;
     uint32_t period_frac;
     int64_t period;
     int64_t last_event;
@@ -61,10 +61,10 @@
     }
 }
 
-uint32_t ptimer_get_count(ptimer_state *s)
+uint64_t ptimer_get_count(ptimer_state *s)
 {
     int64_t now;
-    uint32_t counter;
+    uint64_t counter;
 
     if (s->enabled) {
         now = qemu_get_clock(vm_clock);
@@ -88,7 +88,7 @@
     return counter;
 }
 
-void ptimer_set_count(ptimer_state *s, uint32_t count)
+void ptimer_set_count(ptimer_state *s, uint64_t count)
 {
     s->delta = count;
     if (s->enabled) {
@@ -142,7 +142,7 @@
 
 /* Set the initial countdown value.  If reload is nonzero then also set
    count = limit.  */
-void ptimer_set_limit(ptimer_state *s, uint32_t limit, int reload)
+void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
 {
     if (s->enabled) {
         fprintf(stderr, "FIXME: ptimer_set_limit with running timer");
Index: qemu/vl.h
===================================================================
--- qemu.orig/vl.h	2007-05-23 18:44:20.000000000 +0000
+++ qemu/vl.h	2007-05-23 18:46:18.000000000 +0000
@@ -1589,9 +1589,9 @@
 ptimer_state *ptimer_init(QEMUBH *bh);
 void ptimer_set_period(ptimer_state *s, int64_t period);
 void ptimer_set_freq(ptimer_state *s, uint32_t freq);
-void ptimer_set_limit(ptimer_state *s, uint32_t limit, int reload);
-uint32_t ptimer_get_count(ptimer_state *s);
-void ptimer_set_count(ptimer_state *s, uint32_t count);
+void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload);
+uint64_t ptimer_get_count(ptimer_state *s);
+void ptimer_set_count(ptimer_state *s, uint64_t count);
 void ptimer_run(ptimer_state *s, int oneshot);
 void ptimer_stop(ptimer_state *s);
 
Index: qemu/Makefile.target
===================================================================
--- qemu.orig/Makefile.target	2007-05-23 18:44:20.000000000 +0000
+++ qemu/Makefile.target	2007-05-23 18:46:18.000000000 +0000
@@ -449,7 +449,7 @@
 else
 VL_OBJS+= sun4m.o tcx.o pcnet.o iommu.o m48t59.o slavio_intctl.o
 VL_OBJS+= slavio_timer.o slavio_serial.o slavio_misc.o fdc.o esp.o sparc32_dma.o
-VL_OBJS+= cs4231.o
+VL_OBJS+= cs4231.o ptimer.o
 endif
 endif
 ifeq ($(TARGET_BASE_ARCH), arm)

Re: [Qemu-devel] Timers

[Paul Brook]

On Wednesday 23 May 2007, Blue Swirl wrote:
> On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> > On Wednesday 23 May 2007, Blue Swirl wrote:
> > > On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> > > > I get fed up of having to re-implement a simple countdown timer for
> > > > every new board, so I've added a simple periodic timer implementation
> > > > to cvs (ptimer.c). Currently only the Arm PrimeCell based boards use
> > > > this, but I've a few other uses in the pipeline.
> > >
> > > Nice idea! On Sparc the timer can be configured to work in 64-bit
> > > mode, so could the ptimer_get/set_count be changed to use 64-bit
> > > values?
> I made the API change and converted Sparc timers. Looks like it works
> (guest clock runs normally), though there are the following messages
> on startup:
> FIXME: ptimer_set_limit with running timer
>
> Comments? Did I break something?

Code looks reasonable to me.  The FIXME means you're changing the timer 
parameters after starting the timer. I didn't check whether this does 
anything sensible (this may depend on the device), hence the message.
It probably needs some attention when reload == 1 && s->enabled.

Note that save/restore is not implemented.  You may wish to implement this 
before applying your changes.  This doesn't effect the Arm targets because 
they can't save/restore at all.

Paul

Re: [Qemu-devel] Timers

[andrzej zaborowski]

On 23/05/07, Paul Brook <paul@codesourcery.com> wrote:
> On Wednesday 23 May 2007, Blue Swirl wrote:
> > On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> > > On Wednesday 23 May 2007, Blue Swirl wrote:
> > > > On 5/23/07, Paul Brook <paul@codesourcery.com> wrote:
> > > > > I get fed up of having to re-implement a simple countdown timer for
> > > > > every new board, so I've added a simple periodic timer implementation
> > > > > to cvs (ptimer.c). Currently only the Arm PrimeCell based boards use
> > > > > this, but I've a few other uses in the pipeline.
> > > >
> > > > Nice idea! On Sparc the timer can be configured to work in 64-bit
> > > > mode, so could the ptimer_get/set_count be changed to use 64-bit
> > > > values?
> > I made the API change and converted Sparc timers. Looks like it works
> > (guest clock runs normally), though there are the following messages
> > on startup:
> > FIXME: ptimer_set_limit with running timer
> >
> > Comments? Did I break something?
>
> Code looks reasonable to me.  The FIXME means you're changing the timer
> parameters after starting the timer. I didn't check whether this does
> anything sensible (this may depend on the device), hence the message.
> It probably needs some attention when reload == 1 && s->enabled.
>
> Note that save/restore is not implemented.  You may wish to implement this

I was thinking that it should be possible to save/restore all vm_clock
based timers in qemu at QEMUTimer level so that hardware emulation
doesn't have to bother restoring this. (the "ptimer" would still need
to save its internal fields).

Regards,
Andrzej

Re: [Qemu-devel] Timers

[Paul Brook]

> > Code looks reasonable to me.  The FIXME means you're changing the timer
> > parameters after starting the timer. I didn't check whether this does
> > anything sensible (this may depend on the device), hence the message.
> > It probably needs some attention when reload == 1 && s->enabled.
> >
> > Note that save/restore is not implemented.  You may wish to implement
> > this
>
> I was thinking that it should be possible to save/restore all vm_clock
> based timers in qemu at QEMUTimer level so that hardware emulation
> doesn't have to bother restoring this. (the "ptimer" would still need
> to save its internal fields).

The problem is that the timer itself doesn't know which device it is attached 
to. There's no way to ensure that the state is loaded into the correct 
timers. Remember that qemu can be restarted (and all timers reallocated) in 
between the save and the load.

There are already qemu_put_timer and qemu_get_timer routines.
I notice that the existing slavio_timer_{save,load} don't use these, so are 
probably already broken. 

Paul

Re: [Qemu-devel] Timers

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

On 5/24/07, Paul Brook <paul@codesourcery.com> wrote:
> > > Code looks reasonable to me.  The FIXME means you're changing the timer
> > > parameters after starting the timer. I didn't check whether this does
> > > anything sensible (this may depend on the device), hence the message.
> > > It probably needs some attention when reload == 1 && s->enabled.
> > >
> > > Note that save/restore is not implemented.  You may wish to implement
> > > this
> >
> > I was thinking that it should be possible to save/restore all vm_clock
> > based timers in qemu at QEMUTimer level so that hardware emulation
> > doesn't have to bother restoring this. (the "ptimer" would still need
> > to save its internal fields).
>
> The problem is that the timer itself doesn't know which device it is attached
> to. There's no way to ensure that the state is loaded into the correct
> timers. Remember that qemu can be restarted (and all timers reallocated) in
> between the save and the load.
>
> There are already qemu_put_timer and qemu_get_timer routines.
> I notice that the existing slavio_timer_{save,load} don't use these, so are
> probably already broken.

There were bugs in the previous version, this version passes my simple
tests. I implemented save and load methods. If the 64-bit code doesn't
break ARM, it's ready for commit.

[-- Attachment #2: generic_timers.diff --]
[-- Type: text/x-diff, Size: 13203 bytes --]

Index: qemu/hw/slavio_timer.c
===================================================================
--- qemu.orig/hw/slavio_timer.c	2007-05-24 17:12:54.000000000 +0000
+++ qemu/hw/slavio_timer.c	2007-05-24 17:54:12.000000000 +0000
@@ -48,61 +48,29 @@
  */
 
 typedef struct SLAVIO_TIMERState {
-    uint32_t limit, count, counthigh;
-    int64_t count_load_time;
-    int64_t expire_time;
-    int64_t stop_time, tick_offset;
-    QEMUTimer *irq_timer;
+    ptimer_state *timer;
+    uint32_t count, counthigh, reached;
+    uint64_t limit;
     int irq;
-    int reached, stopped;
+    int stopped;
     int mode; // 0 = processor, 1 = user, 2 = system
     unsigned int cpu;
     void *intctl;
 } SLAVIO_TIMERState;
 
 #define TIMER_MAXADDR 0x1f
-#define CNT_FREQ 2000000
 
 // Update count, set irq, update expire_time
+// Convert from ptimer countdown units
 static void slavio_timer_get_out(SLAVIO_TIMERState *s)
 {
-    int out;
-    int64_t diff, ticks, count;
-    uint32_t limit;
-
-    // There are three clock tick units: CPU ticks, register units
-    // (nanoseconds), and counter ticks (500 ns).
-    if (s->mode == 1 && s->stopped)
-	ticks = s->stop_time;
-    else
-	ticks = qemu_get_clock(vm_clock) - s->tick_offset;
-
-    out = (ticks > s->expire_time);
-    if (out)
-	s->reached = 0x80000000;
-    // Convert register units to counter ticks
-    limit = s->limit >> 9;
-
-    if (!limit)
-	limit = 0x7fffffff >> 9;
-
-    // Convert cpu ticks to counter ticks
-    diff = muldiv64(ticks - s->count_load_time, CNT_FREQ, ticks_per_sec);
-
-    // Calculate what the counter should be, convert to register
-    // units
-    count = diff % limit;
-    s->count = count << 9;
-    s->counthigh = count >> 22;
-
-    // Expire time: CPU ticks left to next interrupt
-    // Convert remaining counter ticks to CPU ticks
-    s->expire_time = ticks + muldiv64(limit - count, ticks_per_sec, CNT_FREQ);
+    uint64_t count;
 
-    DPRINTF("irq %d limit %d reached %d d %" PRId64 " count %d s->c %x diff %" PRId64 " stopped %d mode %d\n", s->irq, limit, s->reached?1:0, (ticks-s->count_load_time), count, s->count, s->expire_time - ticks, s->stopped, s->mode);
-
-    if (s->mode != 1)
-	pic_set_irq_cpu(s->intctl, s->irq, out, s->cpu);
+    count = s->limit - (ptimer_get_count(s->timer) << 9);
+    DPRINTF("get_out: limit %" PRIx64 " count %x%08x\n", s->limit, s->counthigh,
+            s->count);
+    s->count = count & 0xfffffe00;
+    s->counthigh = count >> 32;
 }
 
 // timer callback
@@ -110,17 +78,17 @@
 {
     SLAVIO_TIMERState *s = opaque;
 
-    if (!s->irq_timer)
-        return;
     slavio_timer_get_out(s);
+    DPRINTF("callback: count %x%08x\n", s->counthigh, s->count);
+    s->reached = 0x80000000;
     if (s->mode != 1)
-	qemu_mod_timer(s->irq_timer, s->expire_time);
+	pic_set_irq_cpu(s->intctl, s->irq, 1, s->cpu);
 }
 
 static uint32_t slavio_timer_mem_readl(void *opaque, target_phys_addr_t addr)
 {
     SLAVIO_TIMERState *s = opaque;
-    uint32_t saddr;
+    uint32_t saddr, ret;
 
     saddr = (addr & TIMER_MAXADDR) >> 2;
     switch (saddr) {
@@ -131,60 +99,69 @@
 	    // clear irq
 	    pic_set_irq_cpu(s->intctl, s->irq, 0, s->cpu);
 	    s->reached = 0;
-	    return s->limit;
+            ret = s->limit & 0x7fffffff;
 	}
 	else {
 	    slavio_timer_get_out(s);
-	    return s->counthigh & 0x7fffffff;
+            ret = s->counthigh & 0x7fffffff;
 	}
+        break;
     case 1:
 	// read counter and reached bit (system mode) or read lsbits
 	// of counter (user mode)
 	slavio_timer_get_out(s);
 	if (s->mode != 1)
-	    return (s->count & 0x7fffffff) | s->reached;
+            ret = (s->count & 0x7fffffff) | s->reached;
 	else
-	    return s->count;
+            ret = s->count;
+        break;
     case 3:
 	// read start/stop status
-	return s->stopped;
+        ret = s->stopped;
+        break;
     case 4:
 	// read user/system mode
-	return s->mode & 1;
+        ret = s->mode & 1;
+        break;
     default:
-	return 0;
+        ret = 0;
+        break;
     }
+    DPRINTF("read " TARGET_FMT_plx " = %08x\n", addr, ret);
+
+    return ret;
 }
 
 static void slavio_timer_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
     SLAVIO_TIMERState *s = opaque;
     uint32_t saddr;
+    int reload = 0;
 
+    DPRINTF("write " TARGET_FMT_plx " %08x\n", addr, val);
     saddr = (addr & TIMER_MAXADDR) >> 2;
     switch (saddr) {
     case 0:
 	// set limit, reset counter
-	s->count_load_time = qemu_get_clock(vm_clock);
+        reload = 1;
+        pic_set_irq_cpu(s->intctl, s->irq, 0, s->cpu);
 	// fall through
     case 2:
 	// set limit without resetting counter
-	if (!val)
-	    s->limit = 0x7fffffff;
-	else
-	    s->limit = val & 0x7fffffff;
-	slavio_timer_irq(s);
+        s->limit = val & 0x7ffffe00ULL;
+        if (!s->limit)
+            s->limit = 0x7ffffe00ULL;
+        ptimer_set_limit(s->timer, s->limit >> 9, reload);
 	break;
     case 3:
 	// start/stop user counter
 	if (s->mode == 1) {
 	    if (val & 1) {
-		s->stop_time = qemu_get_clock(vm_clock);
+                ptimer_stop(s->timer);
 		s->stopped = 1;
 	    }
 	    else {
-		if (s->stopped)
-		    s->tick_offset += qemu_get_clock(vm_clock) - s->stop_time;
+                ptimer_run(s->timer, 0);
 		s->stopped = 0;
 	    }
 	}
@@ -193,6 +170,11 @@
 	// bit 0: user (1) or system (0) counter mode
 	if (s->mode == 0 || s->mode == 1)
 	    s->mode = val & 1;
+        if (s->mode == 1) {
+            pic_set_irq_cpu(s->intctl, s->irq, 0, s->cpu);
+            s->limit = -1ULL;
+        }
+        ptimer_set_limit(s->timer, s->limit >> 9, 1);
 	break;
     default:
 	break;
@@ -215,37 +197,32 @@
 {
     SLAVIO_TIMERState *s = opaque;
 
-    qemu_put_be32s(f, &s->limit);
+    qemu_put_be64s(f, &s->limit);
     qemu_put_be32s(f, &s->count);
     qemu_put_be32s(f, &s->counthigh);
-    qemu_put_be64s(f, &s->count_load_time);
-    qemu_put_be64s(f, &s->expire_time);
-    qemu_put_be64s(f, &s->stop_time);
-    qemu_put_be64s(f, &s->tick_offset);
     qemu_put_be32s(f, &s->irq);
     qemu_put_be32s(f, &s->reached);
     qemu_put_be32s(f, &s->stopped);
     qemu_put_be32s(f, &s->mode);
+    qemu_put_ptimer(f, s->timer);
 }
 
 static int slavio_timer_load(QEMUFile *f, void *opaque, int version_id)
 {
     SLAVIO_TIMERState *s = opaque;
     
-    if (version_id != 1)
+    if (version_id != 2)
         return -EINVAL;
 
-    qemu_get_be32s(f, &s->limit);
+    qemu_get_be64s(f, &s->limit);
     qemu_get_be32s(f, &s->count);
     qemu_get_be32s(f, &s->counthigh);
-    qemu_get_be64s(f, &s->count_load_time);
-    qemu_get_be64s(f, &s->expire_time);
-    qemu_get_be64s(f, &s->stop_time);
-    qemu_get_be64s(f, &s->tick_offset);
     qemu_get_be32s(f, &s->irq);
     qemu_get_be32s(f, &s->reached);
     qemu_get_be32s(f, &s->stopped);
     qemu_get_be32s(f, &s->mode);
+    qemu_get_ptimer(f, s->timer);
+
     return 0;
 }
 
@@ -253,13 +230,12 @@
 {
     SLAVIO_TIMERState *s = opaque;
 
-    s->limit = 0;
+    s->limit = 0x7ffffe00ULL;
     s->count = 0;
-    s->count_load_time = qemu_get_clock(vm_clock);;
-    s->stop_time = s->count_load_time;
-    s->tick_offset = 0;
     s->reached = 0;
     s->mode &= 2;
+    ptimer_set_limit(s->timer, s->limit >> 9, 1);
+    ptimer_run(s->timer, 0);
     s->stopped = 1;
     slavio_timer_irq(s);
 }
@@ -269,6 +245,7 @@
 {
     int slavio_timer_io_memory;
     SLAVIO_TIMERState *s;
+    QEMUBH *bh;
 
     s = qemu_mallocz(sizeof(SLAVIO_TIMERState));
     if (!s)
@@ -276,13 +253,15 @@
     s->irq = irq;
     s->mode = mode;
     s->cpu = cpu;
-    s->irq_timer = qemu_new_timer(vm_clock, slavio_timer_irq, s);
+    bh = qemu_bh_new(slavio_timer_irq, s);
+    s->timer = ptimer_init(bh);
+    ptimer_set_period(s->timer, 500ULL);
     s->intctl = intctl;
 
     slavio_timer_io_memory = cpu_register_io_memory(0, slavio_timer_mem_read,
 						    slavio_timer_mem_write, s);
     cpu_register_physical_memory(addr, TIMER_MAXADDR, slavio_timer_io_memory);
-    register_savevm("slavio_timer", addr, 1, slavio_timer_save, slavio_timer_load, s);
+    register_savevm("slavio_timer", addr, 2, slavio_timer_save, slavio_timer_load, s);
     qemu_register_reset(slavio_timer_reset, s);
     slavio_timer_reset(s);
 }
Index: qemu/hw/ptimer.c
===================================================================
--- qemu.orig/hw/ptimer.c	2007-05-24 17:12:54.000000000 +0000
+++ qemu/hw/ptimer.c	2007-05-24 17:18:55.000000000 +0000
@@ -11,8 +11,8 @@
 struct ptimer_state
 {
     int enabled; /* 0 = disabled, 1 = periodic, 2 = oneshot.  */
-    uint32_t limit;
-    uint32_t delta;
+    uint64_t limit;
+    uint64_t delta;
     uint32_t period_frac;
     int64_t period;
     int64_t last_event;
@@ -61,10 +61,10 @@
     }
 }
 
-uint32_t ptimer_get_count(ptimer_state *s)
+uint64_t ptimer_get_count(ptimer_state *s)
 {
     int64_t now;
-    uint32_t counter;
+    uint64_t counter;
 
     if (s->enabled) {
         now = qemu_get_clock(vm_clock);
@@ -75,8 +75,8 @@
                triggered.  */
             counter = 0;
         } else {
-            int64_t rem;
-            int64_t div;
+            uint64_t rem;
+            uint64_t div;
 
             rem = s->next_event - now;
             div = s->period;
@@ -88,7 +88,7 @@
     return counter;
 }
 
-void ptimer_set_count(ptimer_state *s, uint32_t count)
+void ptimer_set_count(ptimer_state *s, uint64_t count)
 {
     s->delta = count;
     if (s->enabled) {
@@ -108,7 +108,7 @@
     ptimer_reload(s);
 }
 
-/* Pause a timer.  Note that this may cause it to "loose" time, even if it
+/* Pause a timer.  Note that this may cause it to "lose" time, even if it
    is immediately restarted.  */
 void ptimer_stop(ptimer_state *s)
 {
@@ -123,33 +123,60 @@
 /* Set counter increment interval in nanoseconds.  */
 void ptimer_set_period(ptimer_state *s, int64_t period)
 {
-    if (s->enabled) {
-        fprintf(stderr, "FIXME: ptimer_set_period with running timer");
-    }
     s->period = period;
     s->period_frac = 0;
+    if (s->enabled) {
+        s->next_event = qemu_get_clock(vm_clock);
+        ptimer_reload(s);
+    }
 }
 
 /* Set counter frequency in Hz.  */
 void ptimer_set_freq(ptimer_state *s, uint32_t freq)
 {
-    if (s->enabled) {
-        fprintf(stderr, "FIXME: ptimer_set_freq with running timer");
-    }
     s->period = 1000000000ll / freq;
     s->period_frac = (1000000000ll << 32) / freq;
+    if (s->enabled) {
+        s->next_event = qemu_get_clock(vm_clock);
+        ptimer_reload(s);
+    }
 }
 
 /* Set the initial countdown value.  If reload is nonzero then also set
    count = limit.  */
-void ptimer_set_limit(ptimer_state *s, uint32_t limit, int reload)
+void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
 {
-    if (s->enabled) {
-        fprintf(stderr, "FIXME: ptimer_set_limit with running timer");
-    }
     s->limit = limit;
     if (reload)
         s->delta = limit;
+    if (s->enabled) {
+        s->next_event = qemu_get_clock(vm_clock);
+        ptimer_reload(s);
+    }
+}
+
+void qemu_put_ptimer(QEMUFile *f, ptimer_state *s)
+{
+    qemu_put_byte(f, s->enabled);
+    qemu_put_be64s(f, &s->limit);
+    qemu_put_be64s(f, &s->delta);
+    qemu_put_be32s(f, &s->period_frac);
+    qemu_put_be64s(f, &s->period);
+    qemu_put_be64s(f, &s->last_event);
+    qemu_put_be64s(f, &s->next_event);
+    qemu_put_timer(f, s->timer);
+}
+
+void qemu_get_ptimer(QEMUFile *f, ptimer_state *s)
+{
+    s->enabled = qemu_get_byte(f);
+    qemu_get_be64s(f, &s->limit);
+    qemu_get_be64s(f, &s->delta);
+    qemu_get_be32s(f, &s->period_frac);
+    qemu_get_be64s(f, &s->period);
+    qemu_get_be64s(f, &s->last_event);
+    qemu_get_be64s(f, &s->next_event);
+    qemu_get_timer(f, s->timer);
 }
 
 ptimer_state *ptimer_init(QEMUBH *bh)
Index: qemu/vl.h
===================================================================
--- qemu.orig/vl.h	2007-05-24 17:12:54.000000000 +0000
+++ qemu/vl.h	2007-05-24 17:18:55.000000000 +0000
@@ -1589,11 +1589,13 @@
 ptimer_state *ptimer_init(QEMUBH *bh);
 void ptimer_set_period(ptimer_state *s, int64_t period);
 void ptimer_set_freq(ptimer_state *s, uint32_t freq);
-void ptimer_set_limit(ptimer_state *s, uint32_t limit, int reload);
-uint32_t ptimer_get_count(ptimer_state *s);
-void ptimer_set_count(ptimer_state *s, uint32_t count);
+void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload);
+uint64_t ptimer_get_count(ptimer_state *s);
+void ptimer_set_count(ptimer_state *s, uint64_t count);
 void ptimer_run(ptimer_state *s, int oneshot);
 void ptimer_stop(ptimer_state *s);
+void qemu_put_ptimer(QEMUFile *f, ptimer_state *s);
+void qemu_get_ptimer(QEMUFile *f, ptimer_state *s);
 
 #include "hw/pxa.h"
 
Index: qemu/Makefile.target
===================================================================
--- qemu.orig/Makefile.target	2007-05-24 17:12:54.000000000 +0000
+++ qemu/Makefile.target	2007-05-24 17:18:55.000000000 +0000
@@ -449,7 +449,7 @@
 else
 VL_OBJS+= sun4m.o tcx.o pcnet.o iommu.o m48t59.o slavio_intctl.o
 VL_OBJS+= slavio_timer.o slavio_serial.o slavio_misc.o fdc.o esp.o sparc32_dma.o
-VL_OBJS+= cs4231.o
+VL_OBJS+= cs4231.o ptimer.o
 endif
 endif
 ifeq ($(TARGET_BASE_ARCH), arm)

Re: [Qemu-devel] Timers

[Paul Brook]

> There were bugs in the previous version, this version passes my simple
> tests. I implemented save and load methods. If the 64-bit code doesn't
> break ARM, it's ready for commit.

Works for me on arm.

Paul