From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NSZjd-0000pa-5n for qemu-devel@nongnu.org; Wed, 06 Jan 2010 12:31:53 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NSZjY-0000iY-3r for qemu-devel@nongnu.org; Wed, 06 Jan 2010 12:31:52 -0500 Received: from [199.232.76.173] (port=57164 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NSZjX-0000iG-Te for qemu-devel@nongnu.org; Wed, 06 Jan 2010 12:31:47 -0500 Received: from mail-px0-f189.google.com ([209.85.216.189]:58925) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NSZjX-0004Zz-DR for qemu-devel@nongnu.org; Wed, 06 Jan 2010 12:31:47 -0500 Received: by pxi27 with SMTP id 27so11074665pxi.4 for ; Wed, 06 Jan 2010 09:31:46 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: From: Blue Swirl Date: Wed, 6 Jan 2010 17:31:26 +0000 Message-ID: Subject: Re: [Qemu-devel] [PATCH 9/9] sparc64: reimplement tick timers Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Kovalenko Cc: qemu-devel@nongnu.org On Tue, Jan 5, 2010 at 11:47 PM, Igor Kovalenko wrote: > 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. > > This change implements sparc64 timers without > periodic timers. It is not complete yet, > cpu_tick_set_count does not really set counter value. > > Signed-off-by: Igor V. Kovalenko > --- > =C2=A0hw/sun4u.c | =C2=A0182 ++++++++++++++++++++++++++++++++++++++++++++= ++++++---------- > =C2=A01 files changed, 152 insertions(+), 30 deletions(-) > > diff --git a/hw/sun4u.c b/hw/sun4u.c > index 84a8043..35f4c6b 100644 > --- a/hw/sun4u.c > +++ b/hw/sun4u.c > @@ -281,6 +281,12 @@ void cpu_check_irqs(CPUState *env) > =C2=A0 =C2=A0 } > =C2=A0} > > +static void cpu_kick_irq(CPUState *env) > +{ > + =C2=A0 =C2=A0env->halted =3D 0; > + =C2=A0 =C2=A0cpu_check_irqs(env); > +} > + > =C2=A0static void cpu_set_irq(void *opaque, int irq, int level) > =C2=A0{ > =C2=A0 =C2=A0 CPUState *env =3D opaque; > @@ -302,6 +308,41 @@ typedef struct ResetData { > =C2=A0 =C2=A0 uint64_t prom_addr; > =C2=A0} ResetData; > > +struct sun4u_timer > +{ > + =C2=A0 =C2=A0const char *name; > + =C2=A0 =C2=A0uint32_t =C2=A0 =C2=A0frequency; > + =C2=A0 =C2=A0int =C2=A0 =C2=A0 =C2=A0disabled; > + =C2=A0 =C2=A0uint64_t disabled_mask; > + =C2=A0 =C2=A0QEMUTimer *qtimer; > +}; The formatting seems to be off. Please don't use tabs. The prefix 'sun4u_' does not fit hstick and it's not CamelCase, how about CPUTimer? > + > +typedef struct sun4u_timer sun4u_timer; > + > +static sun4u_timer* sun4u_timer_create(const char* name, CPUState *env, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 QEMUBHFunc *= cb, uint32_t frequency, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int64_t peri= od, uint64_t disabled_mask) > +{ > + =C2=A0 =C2=A0sun4u_timer *timer; > + > + =C2=A0 =C2=A0timer =3D qemu_mallocz(sizeof (sun4u_timer)); > + > + =C2=A0 =C2=A0timer->name =3D name; > + =C2=A0 =C2=A0timer->frequency =3D frequency; > + =C2=A0 =C2=A0timer->disabled =3D 1; > + =C2=A0 =C2=A0timer->disabled_mask =3D disabled_mask; > + > + =C2=A0 =C2=A0timer->qtimer =3D qemu_new_timer(vm_clock, cb, env); > + > + =C2=A0 =C2=A0return timer; > +} The parameter 'period' is not used. > + > +static void sun4u_timer_reset(sun4u_timer *timer) > +{ > + =C2=A0 =C2=A0timer->disabled =3D 1; > + =C2=A0 =C2=A0qemu_del_timer(timer->qtimer); > +} > + > =C2=A0static void main_cpu_reset(void *opaque) > =C2=A0{ > =C2=A0 =C2=A0 ResetData *s =3D (ResetData *)opaque; > @@ -309,15 +350,11 @@ static void main_cpu_reset(void *opaque) > =C2=A0 =C2=A0 static unsigned int nr_resets; > > =C2=A0 =C2=A0 cpu_reset(env); > - =C2=A0 =C2=A0env->tick_cmpr =3D TICK_INT_DIS | 0; > - =C2=A0 =C2=A0ptimer_set_limit(env->tick, TICK_MAX, 1); > - =C2=A0 =C2=A0ptimer_run(env->tick, 1); > - =C2=A0 =C2=A0env->stick_cmpr =3D TICK_INT_DIS | 0; > - =C2=A0 =C2=A0ptimer_set_limit(env->stick, TICK_MAX, 1); > - =C2=A0 =C2=A0ptimer_run(env->stick, 1); > - =C2=A0 =C2=A0env->hstick_cmpr =3D TICK_INT_DIS | 0; > - =C2=A0 =C2=A0ptimer_set_limit(env->hstick, TICK_MAX, 1); > - =C2=A0 =C2=A0ptimer_run(env->hstick, 1); > + > + =C2=A0 =C2=A0sun4u_timer_reset(env->tick); > + =C2=A0 =C2=A0sun4u_timer_reset(env->stick); > + =C2=A0 =C2=A0sun4u_timer_reset(env->hstick); > + > =C2=A0 =C2=A0 env->gregs[1] =3D 0; // Memory start > =C2=A0 =C2=A0 env->gregs[2] =3D ram_size; // Memory size > =C2=A0 =C2=A0 env->gregs[3] =3D 0; // Machine description XXX > @@ -334,44 +371,125 @@ static void tick_irq(void *opaque) > =C2=A0{ > =C2=A0 =C2=A0 CPUState *env =3D opaque; > > - =C2=A0 =C2=A0if (!(env->tick_cmpr & TICK_INT_DIS)) { > - =C2=A0 =C2=A0 =C2=A0 =C2=A0env->softint |=3D SOFTINT_TIMER; > - =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu_interrupt(env, CPU_INTERRUPT_TIMER); > + =C2=A0 =C2=A0sun4u_timer* timer =3D (sun4u_timer*) env->tick; > + > + =C2=A0 =C2=A0if (timer->disabled) > + =C2=A0 =C2=A0{ Coding style. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(logfile, "tick_irq: softint disabled= \n"); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return; > =C2=A0 =C2=A0 } > + =C2=A0 =C2=A0else > + =C2=A0 =C2=A0{ Coding style. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(logfile, "tick: fire\n"); > + =C2=A0 =C2=A0} > + > + =C2=A0 =C2=A0env->softint |=3D SOFTINT_TM; > + =C2=A0 =C2=A0cpu_kick_irq(env); > =C2=A0} > > =C2=A0static void stick_irq(void *opaque) > =C2=A0{ > =C2=A0 =C2=A0 CPUState *env =3D opaque; > > - =C2=A0 =C2=A0if (!(env->stick_cmpr & TICK_INT_DIS)) { > - =C2=A0 =C2=A0 =C2=A0 =C2=A0env->softint |=3D SOFTINT_STIMER; > - =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu_interrupt(env, CPU_INTERRUPT_TIMER); > + =C2=A0 =C2=A0sun4u_timer* timer =3D (sun4u_timer*) env->stick; > + > + =C2=A0 =C2=A0if (timer->disabled) > + =C2=A0 =C2=A0{ Coding style. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0CPUIRQ_DPRINTF("stick_irq: softint disabled\= n"); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return; > =C2=A0 =C2=A0 } > + =C2=A0 =C2=A0else > + =C2=A0 =C2=A0{ Coding style. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0CPUIRQ_DPRINTF("stick: fire\n"); > + =C2=A0 =C2=A0} > + > + =C2=A0 =C2=A0env->softint |=3D SOFTINT_SM; > + =C2=A0 =C2=A0cpu_kick_irq(env); > =C2=A0} > > =C2=A0static void hstick_irq(void *opaque) > =C2=A0{ > =C2=A0 =C2=A0 CPUState *env =3D opaque; > > - =C2=A0 =C2=A0if (!(env->hstick_cmpr & TICK_INT_DIS)) { > - =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu_interrupt(env, CPU_INTERRUPT_TIMER); > + =C2=A0 =C2=A0sun4u_timer* timer =3D (sun4u_timer*) env->hstick; > + > + =C2=A0 =C2=A0if (timer->disabled) > + =C2=A0 =C2=A0{ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0CPUIRQ_DPRINTF("hstick_irq: softint disabled= \n"); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return; > + =C2=A0 =C2=A0} > + =C2=A0 =C2=A0else Coding style. > + =C2=A0 =C2=A0{ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0CPUIRQ_DPRINTF("hstick: fire\n"); > =C2=A0 =C2=A0 } > + > + =C2=A0 =C2=A0env->softint |=3D SOFTINT_SM; > + =C2=A0 =C2=A0cpu_kick_irq(env); > =C2=A0} > > =C2=A0void cpu_tick_set_count(void *opaque, uint64_t count) > =C2=A0{ > - =C2=A0 =C2=A0ptimer_set_count(opaque, -count); > + =C2=A0 =C2=A0sun4u_timer *timer =3D opaque; > + > + =C2=A0 =C2=A0uint64_t real_count =3D count & ~timer->disabled_mask; > + =C2=A0 =C2=A0timer->disabled =3D (count & timer->disabled_mask) ? 1 : 0= ; > + > + =C2=A0 =C2=A0fprintf(logfile, "%s (ignored) set_count count=3D0x%016lx = (%s) p=3D%p\n", > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timer->name, real_count, > timer->disabled?"disabled":"enabled", opaque); I think logging this is not necessary, please create TICK_DPRINTF() or something. > + > + =C2=A0 =C2=A0// TODO: save offset in our timer > =C2=A0} > > =C2=A0uint64_t cpu_tick_get_count(void *opaque) > =C2=A0{ > - =C2=A0 =C2=A0return -ptimer_get_count(opaque); > + =C2=A0 =C2=A0sun4u_timer *timer =3D opaque; > + > + =C2=A0 =C2=A0uint64_t real_count =3D muldiv64(qemu_get_clock(vm_clock), > timer->frequency, get_ticks_per_sec()); The line is wrapped, perhaps the line is too long? > + > + =C2=A0 =C2=A0fprintf(logfile, "%s get_count count=3D0x%016lx (%s) p=3D%= p\n", > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timer->name, real_count, > timer->disabled?"disabled":"enabled", opaque); DPRINTF() > + > + =C2=A0 =C2=A0if (timer->disabled) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0real_count |=3D timer->disabled_mask; > + > + =C2=A0 =C2=A0return real_count; > =C2=A0} > > =C2=A0void cpu_tick_set_limit(void *opaque, uint64_t limit) > =C2=A0{ > - =C2=A0 =C2=A0ptimer_set_limit(opaque, -limit, 0); > + =C2=A0 =C2=A0sun4u_timer *timer =3D opaque; > + > + =C2=A0 =C2=A0int64_t now =3D qemu_get_clock(vm_clock); > + > + =C2=A0 =C2=A0int64_t real_limit =3D limit & ~timer->disabled_mask; > + =C2=A0 =C2=A0int64_t expires =3D muldiv64(now, timer->frequency, > get_ticks_per_sec()) & ~timer->disabled_mask; Line too long. > + =C2=A0 =C2=A0int64_t current_tick =3D expires; > + =C2=A0 =C2=A0int64_t delta =3D real_limit - current_tick; > + =C2=A0 =C2=A0if (delta < 0) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0delta =3D 1; > + > + =C2=A0 =C2=A0timer->disabled =3D (limit & timer->disabled_mask) ? 1 : 0= ; > + > + =C2=A0 =C2=A0fprintf(logfile, "%s set_limit limit=3D0x%016lx (%s) p=3D%= p " > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"called with limit=3D0x%016lx = at 0x%016lx (delta=3D0x%016lx)\n", > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timer->name, real_limit, timer->disa= bled?"disabled":"enabled", > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 opaque, limit, current_tick, delta); DPRINTF() > + > + =C2=A0 =C2=A0if (!real_limit) > + =C2=A0 =C2=A0{ Coding style. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(logfile, "%s set_limit limit=3DZERO = - not starting timer\n", > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0timer->name); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_del_timer(timer->qtimer); > + =C2=A0 =C2=A0} > + =C2=A0 =C2=A0else if (timer->disabled) > + =C2=A0 =C2=A0{ Coding style. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_del_timer(timer->qtimer); > + =C2=A0 =C2=A0} > + =C2=A0 =C2=A0else > + =C2=A0 =C2=A0{ Coding style. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_mod_timer(timer->qtimer, now + muldiv64= (delta, > get_ticks_per_sec(), Line too long. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timer->frequency)); > + =C2=A0 =C2=A0} > =C2=A0} > > =C2=A0static void ebus_mmio_mapfunc(PCIDevice *pci_dev, int region_num, > @@ -558,9 +676,12 @@ device_init(ram_register_devices); > =C2=A0static CPUState *cpu_devinit(const char *cpu_model, const struct hw= def *hwdef) > =C2=A0{ > =C2=A0 =C2=A0 CPUState *env; > - =C2=A0 =C2=A0QEMUBH *bh; > =C2=A0 =C2=A0 ResetData *reset_info; > > + =C2=A0 =C2=A0uint32_t =C2=A0 tick_frequency =3D 10*1000000; > + =C2=A0 =C2=A0uint32_t =C2=A0stick_frequency =3D 10*1000000; > + =C2=A0 =C2=A0uint32_t hstick_frequency =3D 10*1000000; > + > =C2=A0 =C2=A0 if (!cpu_model) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 cpu_model =3D hwdef->default_cpu_model; > =C2=A0 =C2=A0 env =3D cpu_init(cpu_model); > @@ -568,17 +689,18 @@ static CPUState *cpu_devinit(const char > *cpu_model, const struct hwdef *hwdef) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf(stderr, "Unable to find Sparc CPU def= inition\n"); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 exit(1); > =C2=A0 =C2=A0 } > - =C2=A0 =C2=A0bh =3D qemu_bh_new(tick_irq, env); > - =C2=A0 =C2=A0env->tick =3D ptimer_init(bh); > - =C2=A0 =C2=A0ptimer_set_period(env->tick, 1ULL); > > - =C2=A0 =C2=A0bh =3D qemu_bh_new(stick_irq, env); > - =C2=A0 =C2=A0env->stick =3D ptimer_init(bh); > - =C2=A0 =C2=A0ptimer_set_period(env->stick, 1ULL); > + =C2=A0 =C2=A0env->tick =3D sun4u_timer_create("tick", env, tick_irq, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0tick_frequency, 1ULL= , > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0TICK_NPT_MASK); > + > + =C2=A0 =C2=A0env->stick =3D sun4u_timer_create("stick", env, stick_irq, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 stick_frequency, 1U= LL, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TICK_SOFTINT_DISABL= E); > > - =C2=A0 =C2=A0bh =3D qemu_bh_new(hstick_irq, env); > - =C2=A0 =C2=A0env->hstick =3D ptimer_init(bh); > - =C2=A0 =C2=A0ptimer_set_period(env->hstick, 1ULL); > + =C2=A0 =C2=A0env->hstick =3D sun4u_timer_create("hstick", env, hstick_i= rq, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hstick_frequency, 1= ULL, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TICK_SOFTINT_DISABL= E); > > =C2=A0 =C2=A0 reset_info =3D qemu_mallocz(sizeof(ResetData)); > =C2=A0 =C2=A0 reset_info->env =3D env; > > > After this, are ptimers not going to be used anymore for Sparc64? Then Makefile etc. changes should be included. Anyway, thank you for your efforts.