Re: [Qemu-devel] Timers

["Blue Swirl" <blauwirbel@gmail.com>]

[-- 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)