qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Blue Swirl
Subject: Re: [Qemu-devel] [PATCH] sparc64: reimplement tick timers v2
Date: Tue, 19 Jan 2010 18:44:13 +0000

On Mon, Jan 18, 2010 at 10:28 PM, Igor V. Kovalenko
<address@hidden> wrote:
> From: Igor V. Kovalenko <address@hidden>
>
> 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.

Does not compile:

  CC    sparc64-softmmu/sun4u.o
cc1: warnings being treated as errors
/src/qemu/hw/sun4u.c: In function 'cpu_tick_set_count':
/src/qemu/hw/sun4u.c:467: error: implicit declaration of function
'TIMER_DPRINTF'
make[1]: *** [sun4u.o] Error 1

If I add the missing TIMER_DPRINTF, Linux still crashes:

Memory: 117376k available (2136k kernel code, 664k data, 184k init)
[fffff80000000000,0000000007e80000]
SLUB: Genslabs=14, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
Hierarchical RCU implementation.
NR_IRQS:255
clocksource: mult[a0000] shift[16]
clockevent: mult[19999999] shift[32]
Console: colour dummy device 80x25
Unable to handle kernel NULL pointer dereference
tsk->{mm,active_mm}->context = 0000000000000000
tsk->{mm,active_mm}->pgd = fffff800006fdaa4
              \|/ ____ \|/
              "@'/ .. \`@"
              /_| \__/ |_\
                 \__U_/
swapper(0): Oops [#1]
TSTATE: 0000004480001607 TPC: 00000000006e32f4 TNPC: 00000000006e32f8
Y: 00000000    Not tainted
TPC: <calibrate_delay+0x94/0x2e0>
g0: 00000000006a6100 g1: 0000000021bd5b03 g2: 0000000000698f10 g3:
00000000ffff0001
g4: 000000000069d300 g5: fffff8000090e000 g6: 0000000000688000 g7:
0000000000000000
o0: 0000000000000000 o1: 0000000000000220 o2: 0000000000000000 o3:
0000000000040000
o4: 0000000000000000 o5: 00000000006a4ec0 sp: 000000000068b581 ret_pc:
00000000006e32ec
RPC: <calibrate_delay+0x8c/0x2e0>
l0: 0000000000000000 l1: 00000000006f3dc8 l2: 0000000000000000 l3:
000000000068be50
l4: 000000000068be40 l5: 0000000000000000 l6: 0000000000000000 l7:
0000000000000000
i0: 0000000000000000 i1: fffff80001002028 i2: 0000000000710470 i3:
0000001000000000
i4: 000000000068be38 i5: fffffffffffffed4 i6: fffff80007e6b501 i7:
000000000048b434
I7: <__rcu_process_callbacks+0x74/0x360>
Disabling lock debugging due to kernel taint
Caller[000000000048b434]: __rcu_process_callbacks+0x74/0x360
Instruction DUMP: 90100013  7ffca49d  c277a7e7 <c25ca100> 80a04010
086ffffb  c25fa7df  9010001d  7ffca496
Kernel panic - not syncing: Attempted to kill the idle task!
Call Trace:
Impossible unaligned trap. insn=81cfe008
              \|/ ____ \|/
              "@'/ .. \`@"
              /_| \__/ |_\
                 \__U_/
swapper(0): Byte sized unaligned access?!?! [#2]
TSTATE: 0000000000000000 TPC: 0000000000000000 TNPC: 0000000000000000
Y: 00000000    Tainted: G      D
TPC: <(null)>

etc.

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

The registered savevm version needs to be increased and we can't
handle the old savevm version 5 format any more.

>
> v0 -> v1:
> - coding style
>
> Signed-off-by: Igor V. Kovalenko <address@hidden>
> ---
>  hw/sun4u.c             |  214 
> +++++++++++++++++++++++++++++++++++++++++-------
>  target-sparc/cpu.h     |    9 ++
>  target-sparc/machine.c |   12 +--
>  3 files changed, 197 insertions(+), 38 deletions(-)
>
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index a39b28e..f9db758 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -280,6 +280,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 +307,68 @@ typedef struct ResetData {
>     uint64_t prom_addr;
>  } ResetData;
>
> +struct CPUTimer
> +{
> +    const char *name;
> +    uint32_t    frequency;
> +    uint32_t    disabled;
> +    uint64_t    disabled_mask;
> +    int64_t     clock_offset;
> +    QEMUTimer  *qtimer;
> +};
> +
> +typedef struct CPUTimer CPUTimer;
> +
> +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);
> +    if (s->qtimer) {
> +        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);
> +    if (s->qtimer) {
> +        qemu_get_timer(f, s->qtimer);
> +    }

Whether the old state had qtimer non-NULL or not shouldn't affect
loading. Likewise, we always want to save. Dynamical state may not
affect savevm format.

This kind of conditional save/load would be OK, if for example some
class of timers didn't ever have the missing piece (because of some
hardware difference).

> +}
> +
> +static CPUTimer* cpu_timer_create(const char* name, CPUState *env,
> +                                  QEMUBHFunc *cb, uint32_t frequency,
> +                                  uint64_t disabled_mask)
> +{
> +    CPUTimer *timer;
> +
> +    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 +376,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 +397,133 @@ 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 = (CPUTimer*) env->tick;

I think the cast could be avoided if the typedef were moved into
cpu.h. Then also cpu_tick_set_count() and friends could take a
non-opaque parameter.

> +
> +    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 = (CPUTimer*) 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)) {
> -        cpu_interrupt(env, CPU_INTERRUPT_TIMER);
> +    CPUTimer* timer = (CPUTimer*) env->hstick;
> +
> +    if (timer->disabled) {
> +        CPUIRQ_DPRINTF("hstick_irq: softint disabled\n");
> +        return;
> +    } else {
> +        CPUIRQ_DPRINTF("hstick: fire\n");
>     }
> +
> +    env->softint |= SOFTINT_STIMER;
> +    cpu_kick_irq(env);
> +}
> +
> +static int64_t cpu_to_timer_ticks(int64_t cpu_ticks, uint32_t frequency)
> +{
> +    return muldiv64(cpu_ticks, get_ticks_per_sec(), frequency);
> +}
> +
> +static uint64_t timer_to_cpu_ticks(int64_t timer_ticks, uint32_t frequency)
> +{
> +    return muldiv64(timer_ticks, frequency, get_ticks_per_sec());
>  }
>
>  void cpu_tick_set_count(void *opaque, uint64_t count)
>  {
> -    ptimer_set_count(opaque, -count);
> +    CPUTimer *timer = opaque;
> +
> +    uint64_t real_count   = count & ~timer->disabled_mask;
> +    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);
> +
> +    timer->disabled     = disabled_bit ? 1 : 0;
> +    timer->clock_offset = vm_clock_offset;
>  }
>
>  uint64_t cpu_tick_get_count(void *opaque)
>  {
> -    return -ptimer_get_count(opaque);
> +    CPUTimer *timer = opaque;
> +
> +    uint64_t real_count = timer_to_cpu_ticks(
> +                    qemu_get_clock(vm_clock) - timer->clock_offset,
> +                    timer->frequency);
> +
> +    TIMER_DPRINTF("%s get_count count=0x%016lx (%s) p=%p\n",
> +           timer->name, real_count,
> +           timer->disabled?"disabled":"enabled", opaque);
> +
> +    if (timer->disabled)
> +        real_count |= timer->disabled_mask;
> +
> +    return real_count;
>  }
>
>  void cpu_tick_set_limit(void *opaque, uint64_t limit)
>  {
> -    ptimer_set_limit(opaque, -limit, 0);
> +    CPUTimer *timer = opaque;
> +
> +    int64_t now = qemu_get_clock(vm_clock);
> +
> +    uint64_t real_limit = limit & ~timer->disabled_mask;
> +    timer->disabled = (limit & timer->disabled_mask) ? 1 : 0;
> +
> +    int64_t expires = cpu_to_timer_ticks(real_limit, timer->frequency) +
> +                    timer->clock_offset;
> +
> +    if (expires < now) {
> +        expires = now + 1;
> +    }
> +
> +    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));
> +
> +    if (!real_limit) {
> +        TIMER_DPRINTF("%s set_limit limit=ZERO - not starting timer\n",
> +                timer->name);
> +        qemu_del_timer(timer->qtimer);
> +    } else if (timer->disabled) {
> +        qemu_del_timer(timer->qtimer);
> +    } else {
> +        qemu_mod_timer(timer->qtimer, expires);
> +    }
>  }
>
>  static void ebus_mmio_mapfunc(PCIDevice *pci_dev, int region_num,
> @@ -557,9 +710,12 @@ device_init(ram_register_devices);
>  static CPUState *cpu_devinit(const char *cpu_model, const struct hwdef 
> *hwdef)
>  {
>     CPUState *env;
> -    QEMUBH *bh;
>     ResetData *reset_info;
>
> +    uint32_t   tick_frequency = 100*1000000;
> +    uint32_t  stick_frequency = 100*1000000;
> +    uint32_t hstick_frequency = 100*1000000;
> +
>     if (!cpu_model)
>         cpu_model = hwdef->default_cpu_model;
>     env = cpu_init(cpu_model);
> @@ -567,17 +723,15 @@ static CPUState *cpu_devinit(const char *cpu_model, 
> const struct hwdef *hwdef)
>         fprintf(stderr, "Unable to find Sparc CPU definition\n");
>         exit(1);
>     }
> -    bh = qemu_bh_new(tick_irq, env);
> -    env->tick = ptimer_init(bh);
> -    ptimer_set_period(env->tick, 1ULL);
>
> -    bh = qemu_bh_new(stick_irq, env);
> -    env->stick = ptimer_init(bh);
> -    ptimer_set_period(env->stick, 1ULL);
> +    env->tick = cpu_timer_create("tick", env, tick_irq,
> +                                  tick_frequency, TICK_NPT_MASK);
> +
> +    env->stick = cpu_timer_create("stick", env, stick_irq,
> +                                   stick_frequency, TICK_INT_DIS);
>
> -    bh = qemu_bh_new(hstick_irq, env);
> -    env->hstick = ptimer_init(bh);
> -    ptimer_set_period(env->hstick, 1ULL);
> +    env->hstick = cpu_timer_create("hstick", env, hstick_irq,
> +                                    hstick_frequency, TICK_INT_DIS);
>
>     reset_info = qemu_mallocz(sizeof(ResetData));
>     reset_info->env = env;
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 50859c7..e50a2ab 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -292,6 +292,11 @@ typedef struct SparcTLBEntry {
>     uint64_t tte;
>  } SparcTLBEntry;
>
> +struct CPUTimer;
> +struct QEMUFile;
> +extern void cpu_put_timer(struct QEMUFile *f, struct CPUTimer *s);
> +extern void cpu_get_timer(struct QEMUFile *f, struct CPUTimer *s);

"extern" is not needed these days.

> +
>  typedef struct CPUSPARCState {
>     target_ulong gregs[8]; /* general registers */
>     target_ulong *regwptr; /* pointer to current register window */
> @@ -393,14 +398,14 @@ typedef struct CPUSPARCState {
>     uint64_t mgregs[8]; /* mmu general registers */
>     uint64_t fprs;
>     uint64_t tick_cmpr, stick_cmpr;
> -    void *tick, *stick;
> +    struct CPUTimer *tick, *stick;
>  #define TICK_NPT_MASK        0x8000000000000000ULL
>  #define TICK_INT_DIS         0x8000000000000000ULL
>     uint64_t gsr;
>     uint32_t gl; // UA2005
>     /* UA 2005 hyperprivileged registers */
>     uint64_t hpstate, htstate[MAXTL_MAX], hintp, htba, hver, hstick_cmpr, ssr;
> -    void *hstick; // UA 2005
> +    struct CPUTimer *hstick; // UA 2005
>     uint32_t softint;
>  #define SOFTINT_TIMER   1
>  #define SOFTINT_STIMER  (1 << 16)
> diff --git a/target-sparc/machine.c b/target-sparc/machine.c
> index c7c03b6..3fa8232 100644
> --- a/target-sparc/machine.c
> +++ b/target-sparc/machine.c
> @@ -84,8 +84,8 @@ void cpu_save(QEMUFile *f, void *opaque)
>     qemu_put_be64s(f, &env->fprs);
>     qemu_put_be64s(f, &env->tick_cmpr);
>     qemu_put_be64s(f, &env->stick_cmpr);
> -    qemu_put_ptimer(f, env->tick);
> -    qemu_put_ptimer(f, env->stick);
> +    cpu_put_timer(f, env->tick);
> +    cpu_put_timer(f, env->stick);
>     qemu_put_be64s(f, &env->gsr);
>     qemu_put_be32s(f, &env->gl);
>     qemu_put_be64s(f, &env->hpstate);
> @@ -96,7 +96,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>     qemu_put_be64s(f, &env->hver);
>     qemu_put_be64s(f, &env->hstick_cmpr);
>     qemu_put_be64s(f, &env->ssr);
> -    qemu_put_ptimer(f, env->hstick);
> +    cpu_put_timer(f, env->hstick);
>  #endif
>  }
>
> @@ -180,8 +180,8 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>     qemu_get_be64s(f, &env->fprs);
>     qemu_get_be64s(f, &env->tick_cmpr);
>     qemu_get_be64s(f, &env->stick_cmpr);
> -    qemu_get_ptimer(f, env->tick);
> -    qemu_get_ptimer(f, env->stick);
> +    cpu_get_timer(f, env->tick);
> +    cpu_get_timer(f, env->stick);
>     qemu_get_be64s(f, &env->gsr);
>     qemu_get_be32s(f, &env->gl);
>     qemu_get_be64s(f, &env->hpstate);
> @@ -192,7 +192,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>     qemu_get_be64s(f, &env->hver);
>     qemu_get_be64s(f, &env->hstick_cmpr);
>     qemu_get_be64s(f, &env->ssr);
> -    qemu_get_ptimer(f, env->hstick);
> +    cpu_get_timer(f, env->hstick);
>  #endif
>     tlb_flush(env, 1);
>     return 0;
>
>
>
>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]