qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] mos6522: fix T1 and T2 timers


From: Mark Cave-Ayland
Subject: Re: [PATCH] mos6522: fix T1 and T2 timers
Date: Sun, 3 Nov 2019 20:42:12 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0

On 02/11/2019 15:49, Laurent Vivier wrote:

> With the Quadra 800 emulation, mos6522 timers processing can consume
> until 70% of the host CPU time with an idle guest (I guess the problem
> should also happen with PowerMac emulation).
> 
> On a recent system, it can be painless (except if you look at top), but
> on an old host like a PowerMac G5 the guest kernel can be terribly slow
> during the boot sequence (for instance, unpacking initramfs can take 15
> seconds rather than only 3 seconds).
> 
> We can avoid this CPU overload by enabling QEMU internal timers only if
> the mos6522 counter interrupts are enabled. Sometime the guest kernel
> wants to read the counters values, but we don't need the timers to
> update the counters.
> 
> With this patch applied, an idle Q800 consumes only 3% of host CPU time
> (and the guest can boot in a decent time).
> 
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
>  hw/misc/mos6522.c | 67 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 52 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> index 57f13db266..aa3bfe1afd 100644
> --- a/hw/misc/mos6522.c
> +++ b/hw/misc/mos6522.c
> @@ -38,8 +38,10 @@
>  
>  /* XXX: implement all timer modes */
>  
> -static void mos6522_timer_update(MOS6522State *s, MOS6522Timer *ti,
> -                                 int64_t current_time);
> +static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti,
> +                                  int64_t current_time);
> +static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti,
> +                                  int64_t current_time);
>  
>  static void mos6522_update_irq(MOS6522State *s)
>  {
> @@ -98,7 +100,11 @@ static void set_counter(MOS6522State *s, MOS6522Timer 
> *ti, unsigned int val)
>      trace_mos6522_set_counter(1 + ti->index, val);
>      ti->load_time = get_load_time(s, ti);
>      ti->counter_value = val;
> -    mos6522_timer_update(s, ti, ti->load_time);
> +    if (ti->index == 0) {
> +        mos6522_timer1_update(s, ti, ti->load_time);
> +    } else {
> +        mos6522_timer2_update(s, ti, ti->load_time);
> +    }
>  }
>  
>  static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti,
> @@ -130,19 +136,34 @@ static int64_t get_next_irq_time(MOS6522State *s, 
> MOS6522Timer *ti,
>      trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d);
>      next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) +
>                           ti->load_time;
> +
>      if (next_time <= current_time) {
>          next_time = current_time + 1;
>      }
>      return next_time;
>  }
>  
> -static void mos6522_timer_update(MOS6522State *s, MOS6522Timer *ti,
> +static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti,
> +                                 int64_t current_time)
> +{
> +    if (!ti->timer) {
> +        return;
> +    }
> +    if ((s->ier & T1_INT) == 0 || (s->acr & T1MODE) != T1MODE_CONT) {
> +        timer_del(ti->timer);
> +    } else {
> +        ti->next_irq_time = get_next_irq_time(s, ti, current_time);
> +        timer_mod(ti->timer, ti->next_irq_time);
> +    }
> +}
> +
> +static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti,
>                                   int64_t current_time)
>  {
>      if (!ti->timer) {
>          return;
>      }
> -    if (ti->index == 0 && (s->acr & T1MODE) != T1MODE_CONT) {
> +    if ((s->ier & T2_INT) == 0) {
>          timer_del(ti->timer);
>      } else {
>          ti->next_irq_time = get_next_irq_time(s, ti, current_time);
> @@ -155,7 +176,7 @@ static void mos6522_timer1(void *opaque)
>      MOS6522State *s = opaque;
>      MOS6522Timer *ti = &s->timers[0];
>  
> -    mos6522_timer_update(s, ti, ti->next_irq_time);
> +    mos6522_timer1_update(s, ti, ti->next_irq_time);
>      s->ifr |= T1_INT;
>      mos6522_update_irq(s);
>  }
> @@ -165,7 +186,7 @@ static void mos6522_timer2(void *opaque)
>      MOS6522State *s = opaque;
>      MOS6522Timer *ti = &s->timers[1];
>  
> -    mos6522_timer_update(s, ti, ti->next_irq_time);
> +    mos6522_timer2_update(s, ti, ti->next_irq_time);
>      s->ifr |= T2_INT;
>      mos6522_update_irq(s);
>  }
> @@ -204,7 +225,16 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, 
> unsigned size)
>  {
>      MOS6522State *s = opaque;
>      uint32_t val;
> +    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  
> +    if (now >= s->timers[0].next_irq_time) {
> +        mos6522_timer1_update(s, &s->timers[0], now);
> +        s->ifr |= T1_INT;
> +    }
> +    if (now >= s->timers[1].next_irq_time) {
> +        mos6522_timer2_update(s, &s->timers[1], now);
> +        s->ifr |= T2_INT;
> +    }
>      switch (addr) {
>      case VIA_REG_B:
>          val = s->b;
> @@ -299,8 +329,8 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t 
> val, unsigned size)
>          break;
>      case VIA_REG_T1CL:
>          s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> -        mos6522_timer_update(s, &s->timers[0],
> -                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        mos6522_timer1_update(s, &s->timers[0],
> +                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>          break;
>      case VIA_REG_T1CH:
>          s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
> @@ -309,14 +339,14 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t 
> val, unsigned size)
>          break;
>      case VIA_REG_T1LL:
>          s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> -        mos6522_timer_update(s, &s->timers[0],
> -                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        mos6522_timer1_update(s, &s->timers[0],
> +                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>          break;
>      case VIA_REG_T1LH:
>          s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
>          s->ifr &= ~T1_INT;
> -        mos6522_timer_update(s, &s->timers[0],
> -                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        mos6522_timer1_update(s, &s->timers[0],
> +                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>          break;
>      case VIA_REG_T2CL:
>          s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
> @@ -334,8 +364,8 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t 
> val, unsigned size)
>          break;
>      case VIA_REG_ACR:
>          s->acr = val;
> -        mos6522_timer_update(s, &s->timers[0],
> -                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        mos6522_timer1_update(s, &s->timers[0],
> +                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>          break;
>      case VIA_REG_PCR:
>          s->pcr = val;
> @@ -354,6 +384,11 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t 
> val, unsigned size)
>              s->ier &= ~val;
>          }
>          mos6522_update_irq(s);
> +        /* if IER is modified starts needed timers */
> +        mos6522_timer1_update(s, &s->timers[0],
> +                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +        mos6522_timer2_update(s, &s->timers[1],
> +                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>          break;
>      default:
>      case VIA_REG_ANH:
> @@ -426,9 +461,11 @@ static void mos6522_reset(DeviceState *dev)
>      s->timers[0].frequency = s->frequency;
>      s->timers[0].latch = 0xffff;
>      set_counter(s, &s->timers[0], 0xffff);
> +    timer_del(s->timers[0].timer);
>  
>      s->timers[1].frequency = s->frequency;
>      s->timers[1].latch = 0xffff;
> +    timer_del(s->timers[1].timer);
>  }
>  
>  static void mos6522_init(Object *obj)

Nice! I've given this a quick test and seems to boot MacOS without any issues, 
so:

Tested-by: Mark Cave-Ayland <address@hidden>


ATB,

Mark.



reply via email to

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