Re: [Qemu-devel] [PATCH] sparc64: reimplement tick timers v3

2010-01-27 Thread Blue Swirl 
On Tue, Jan 26, 2010 at 11:09 PM, Igor Kovalenko
igor.v.kovale...@gmail.com wrote:
 On Fri, Jan 22, 2010 at 11:32 PM, Blue Swirl blauwir...@gmail.com wrote:
 On Tue, Jan 19, 2010 at 10:25 PM, Igor V. Kovalenko
 igor.v.kovale...@gmail.com wrote:
 From: Igor V. Kovalenko igor.v.kovale...@gmail.com

 sparc64 timer has tick counter which can be set and read,
 and tick compare value used as deadline to fire timer interrupt.
 The timer is not used as periodic timer, instead deadline
 is set each time new timer interrupt is needed.

 v2 - v3:
 - added missing timer debug output macro
 - CPUTimer struct and typedef moved to cpu.h
 - change CPU_SAVE_VERSION to 6, older save formats not supported

 v1 - v2:
 - new conversion helpers cpu_to_timer_ticks and timer_to_cpu_ticks
 - save offset from clock source to implement cpu_tick_set_count
 - renamed struct sun4u_timer to CPUTimer
 - load and save cpu timers

 v0 - v1:
 - coding style

 My debugging of Linux panic has not been very fruitful. Once I got the
 panic triggered while single stepping calibrate_delay() with GDB and
 keeping enter key pressed. Then I missed the fault though.

 One possible problem is that 4dc28134f3d7db0033c6b3c5bc4be9a91adb3e2b
 added interrupt checks to the helpers which means that they can cause
 faults, but translation of the instructions was not changed to take
 this into account. But when I added calls to save_state() in
 translate.c, it didn't change anything.

 The issue is with PUT_CWP64 - linux kernel does read cwp which
 happens to be zero, decrements it and writes the result to cwp.
 Expected value is max window id but we store zero cwp,
 and return path from trap handler does restore wrong window.

 I'll post the fix now, and then try to clean up the rest of timer patch.

Excellent analysis! I await your patch anxiously.

Re: [Qemu-devel] [PATCH] sparc64: reimplement tick timers v3

2010-01-26 Thread Igor Kovalenko 
On Fri, Jan 22, 2010 at 11:32 PM, Blue Swirl blauwir...@gmail.com wrote:
 On Tue, Jan 19, 2010 at 10:25 PM, Igor V. Kovalenko
 igor.v.kovale...@gmail.com wrote:
 From: Igor V. Kovalenko igor.v.kovale...@gmail.com

 sparc64 timer has tick counter which can be set and read,
 and tick compare value used as deadline to fire timer interrupt.
 The timer is not used as periodic timer, instead deadline
 is set each time new timer interrupt is needed.

 v2 - v3:
 - added missing timer debug output macro
 - CPUTimer struct and typedef moved to cpu.h
 - change CPU_SAVE_VERSION to 6, older save formats not supported

 v1 - v2:
 - new conversion helpers cpu_to_timer_ticks and timer_to_cpu_ticks
 - save offset from clock source to implement cpu_tick_set_count
 - renamed struct sun4u_timer to CPUTimer
 - load and save cpu timers

 v0 - v1:
 - coding style

 My debugging of Linux panic has not been very fruitful. Once I got the
 panic triggered while single stepping calibrate_delay() with GDB and
 keeping enter key pressed. Then I missed the fault though.

 One possible problem is that 4dc28134f3d7db0033c6b3c5bc4be9a91adb3e2b
 added interrupt checks to the helpers which means that they can cause
 faults, but translation of the instructions was not changed to take
 this into account. But when I added calls to save_state() in
 translate.c, it didn't change anything.

The issue is with PUT_CWP64 - linux kernel does read cwp which
happens to be zero, decrements it and writes the result to cwp.
Expected value is max window id but we store zero cwp,
and return path from trap handler does restore wrong window.

I'll post the fix now, and then try to clean up the rest of timer patch.

-- 
Kind regards,
Igor V. Kovalenko

Re: [Qemu-devel] [PATCH] sparc64: reimplement tick timers v3

2010-01-22 Thread Blue Swirl 
On Tue, Jan 19, 2010 at 10:25 PM, Igor V. Kovalenko
igor.v.kovale...@gmail.com wrote:
 From: Igor V. Kovalenko igor.v.kovale...@gmail.com

 sparc64 timer has tick counter which can be set and read,
 and tick compare value used as deadline to fire timer interrupt.
 The timer is not used as periodic timer, instead deadline
 is set each time new timer interrupt is needed.

 v2 - v3:
 - added missing timer debug output macro
 - CPUTimer struct and typedef moved to cpu.h
 - change CPU_SAVE_VERSION to 6, older save formats not supported

 v1 - v2:
 - new conversion helpers cpu_to_timer_ticks and timer_to_cpu_ticks
 - save offset from clock source to implement cpu_tick_set_count
 - renamed struct sun4u_timer to CPUTimer
 - load and save cpu timers

 v0 - v1:
 - coding style

My debugging of Linux panic has not been very fruitful. Once I got the
panic triggered while single stepping calibrate_delay() with GDB and
keeping enter key pressed. Then I missed the fault though.

One possible problem is that 4dc28134f3d7db0033c6b3c5bc4be9a91adb3e2b
added interrupt checks to the helpers which means that they can cause
faults, but translation of the instructions was not changed to take
this into account. But when I added calls to save_state() in
translate.c, it didn't change anything.

 -void cpu_tick_set_count(void *opaque, uint64_t count)
 +void cpu_tick_set_count(CPUTimer *timer, uint64_t count)
  {
 -    ptimer_set_count(opaque, -count);
 +    uint64_t real_count   = count  ~timer-disabled_mask;

Formatting looks a bit odd. I'd prefer for such a short lists that '='
is close to the left hand value.

 +    uint64_t disabled_bit = count  timer-disabled_mask;
 +
 +    int64_t vm_clock_offset = qemu_get_clock(vm_clock) -
 +                    cpu_to_timer_ticks(real_count, timer-frequency);
 +
 +    TIMER_DPRINTF(%s set_count count=0x%016lx (%s) p=%p\n,
 +                  timer-name, real_count,
 +                  timer-disabled?disabled:enabled, opaque);

opaque is wrong, should be timer. Same problem is in some other debug
messages. Timer name would be more helpful than pointer.

 +
 +    timer-disabled     = disabled_bit ? 1 : 0;
 +    timer-clock_offset = vm_clock_offset;

Formatting.

 +    TIMER_DPRINTF(%s get_count count=0x%016lx (%s) p=%p\n,
 +           timer-name, real_count,
 +           timer-disabled?disabled:enabled, opaque);

opaque

 +    TIMER_DPRINTF(%s set_limit limit=0x%016lx (%s) p=%p 
 +                  called with limit=0x%016lx at 0x%016lx 
 (delta=0x%016lx)\n,
 +                  timer-name, real_limit,
 +                  timer-disabled?disabled:enabled,
 +                  opaque, limit,
 +                  timer_to_cpu_ticks(now - timer-clock_offset,
 +                                     timer-frequency),
 +                  timer_to_cpu_ticks(expires - now, timer-frequency));

opaque

[Qemu-devel] [PATCH] sparc64: reimplement tick timers v3

2010-01-19 Thread Igor V. Kovalenko 
From: Igor V. Kovalenko igor.v.kovale...@gmail.com

sparc64 timer has tick counter which can be set and read,
and tick compare value used as deadline to fire timer interrupt.
The timer is not used as periodic timer, instead deadline
is set each time new timer interrupt is needed.

v2 - v3:
- added missing timer debug output macro
- CPUTimer struct and typedef moved to cpu.h
- change CPU_SAVE_VERSION to 6, older save formats not supported

v1 - v2:
- new conversion helpers cpu_to_timer_ticks and timer_to_cpu_ticks
- save offset from clock source to implement cpu_tick_set_count
- renamed struct sun4u_timer to CPUTimer
- load and save cpu timers

v0 - v1:
- coding style

Signed-off-by: Igor V. Kovalenko igor.v.kovale...@gmail.com
---
 hw/sun4u.c |  206 
 target-sparc/cpu.h |   28 +--
 target-sparc/machine.c |   14 ++-
 3 files changed, 202 insertions(+), 46 deletions(-)

diff --git a/hw/sun4u.c b/hw/sun4u.c
index a39b28e..830ad7f 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -40,6 +40,7 @@
 
 //#define DEBUG_IRQ
 //#define DEBUG_EBUS
+//#define DEBUG_TIMER
 
 #ifdef DEBUG_IRQ
 #define CPUIRQ_DPRINTF(fmt, ...)\
@@ -55,6 +56,13 @@
 #define EBUS_DPRINTF(fmt, ...)
 #endif
 
+#ifdef DEBUG_TIMER
+#define TIMER_DPRINTF(fmt, ...)  \
+do { printf(TIMER:  fmt , ## __VA_ARGS__); } while (0)
+#else
+#define TIMER_DPRINTF(fmt, ...)
+#endif
+
 #define KERNEL_LOAD_ADDR 0x00404000
 #define CMDLINE_ADDR 0x003ff000
 #define INITRD_LOAD_ADDR 0x0030
@@ -280,6 +288,12 @@ void cpu_check_irqs(CPUState *env)
 }
 }
 
+static void cpu_kick_irq(CPUState *env)
+{
+env-halted = 0;
+cpu_check_irqs(env);
+}
+
 static void cpu_set_irq(void *opaque, int irq, int level)
 {
 CPUState *env = opaque;
@@ -301,6 +315,52 @@ typedef struct ResetData {
 uint64_t prom_addr;
 } ResetData;
 
+void cpu_put_timer(QEMUFile *f, CPUTimer *s)
+{
+qemu_put_be32s(f, s-frequency);
+qemu_put_be32s(f, s-disabled);
+qemu_put_be64s(f, s-disabled_mask);
+qemu_put_sbe64s(f, s-clock_offset);
+
+qemu_put_timer(f, s-qtimer);
+}
+
+void cpu_get_timer(QEMUFile *f, CPUTimer *s)
+{
+qemu_get_be32s(f, s-frequency);
+qemu_get_be32s(f, s-disabled);
+qemu_get_be64s(f, s-disabled_mask);
+qemu_get_sbe64s(f, s-clock_offset);
+
+qemu_get_timer(f, s-qtimer);
+}
+
+static CPUTimer* cpu_timer_create(const char* name, CPUState *env,
+  QEMUBHFunc *cb, uint32_t frequency,
+  uint64_t disabled_mask)
+{
+CPUTimer *timer = qemu_mallocz(sizeof (CPUTimer));
+
+timer-name = name;
+timer-frequency = frequency;
+timer-disabled_mask = disabled_mask;
+
+timer-disabled = 1;
+timer-clock_offset = qemu_get_clock(vm_clock);
+
+timer-qtimer = qemu_new_timer(vm_clock, cb, env);
+
+return timer;
+}
+
+static void cpu_timer_reset(CPUTimer *timer)
+{
+timer-disabled = 1;
+timer-clock_offset = qemu_get_clock(vm_clock);
+
+qemu_del_timer(timer-qtimer);
+}
+
 static void main_cpu_reset(void *opaque)
 {
 ResetData *s = (ResetData *)opaque;
@@ -308,15 +368,11 @@ static void main_cpu_reset(void *opaque)
 static unsigned int nr_resets;
 
 cpu_reset(env);
-env-tick_cmpr = TICK_INT_DIS | 0;
-ptimer_set_limit(env-tick, TICK_MAX, 1);
-ptimer_run(env-tick, 1);
-env-stick_cmpr = TICK_INT_DIS | 0;
-ptimer_set_limit(env-stick, TICK_MAX, 1);
-ptimer_run(env-stick, 1);
-env-hstick_cmpr = TICK_INT_DIS | 0;
-ptimer_set_limit(env-hstick, TICK_MAX, 1);
-ptimer_run(env-hstick, 1);
+
+cpu_timer_reset(env-tick);
+cpu_timer_reset(env-stick);
+cpu_timer_reset(env-hstick);
+
 env-gregs[1] = 0; // Memory start
 env-gregs[2] = ram_size; // Memory size
 env-gregs[3] = 0; // Machine description XXX
@@ -333,44 +389,127 @@ static void tick_irq(void *opaque)
 {
 CPUState *env = opaque;
 
-if (!(env-tick_cmpr  TICK_INT_DIS)) {
-env-softint |= SOFTINT_TIMER;
-cpu_interrupt(env, CPU_INTERRUPT_TIMER);
+CPUTimer* timer = env-tick;
+
+if (timer-disabled) {
+CPUIRQ_DPRINTF(tick_irq: softint disabled\n);
+return;
+} else {
+CPUIRQ_DPRINTF(tick: fire\n);
 }
+
+env-softint |= SOFTINT_TIMER;
+cpu_kick_irq(env);
 }
 
 static void stick_irq(void *opaque)
 {
 CPUState *env = opaque;
 
-if (!(env-stick_cmpr  TICK_INT_DIS)) {
-env-softint |= SOFTINT_STIMER;
-cpu_interrupt(env, CPU_INTERRUPT_TIMER);
+CPUTimer* timer = env-stick;
+
+if (timer-disabled) {
+CPUIRQ_DPRINTF(stick_irq: softint disabled\n);
+return;
+} else {
+CPUIRQ_DPRINTF(stick: fire\n);
 }
+
+env-softint |= SOFTINT_STIMER;
+cpu_kick_irq(env);
 }
 
 static void hstick_irq(void *opaque)
 {
 CPUState *env = opaque;
 
-if (!(env-hstick_cmpr  TICK_INT_DIS)) {
-