[7/7] mac_via: remove VIA1 timer optimisations

The original implementation of the Macintosh VIA devices in commit 6dca62a000
"hw/m68k: add VIA support" used timer optimisations to reduce high CPU usage on
the host when booting Linux. These optimisations worked by waiting until VIA1
port B was accessed before re-arming the timers.

The MacOS toolbox ROM constantly writes to VIA1 port B which calls
via1_one_second_update() and via1_sixty_hz_update() to calculate the new expiry
time, causing the timers to constantly reset and never fire. The effect of this
is that the Ticks (0x16a) global variable holding the number of 60Hz timer ticks
since reset is never incremented by the interrupt causing time to stand still.

Whilst the code was introduced as a performance optimisation, it is likely that
the high CPU usage was actually caused by the incorrect 60Hz timer interval
fixed in the previous patch. Remove the optimisation to keep everything simple
and enable the MacOS toolbox ROM to start keeping time.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/mac_via.c | 43 ++++---------------------------------------
 1 file changed, 4 insertions(+), 39 deletions(-)

Le 10/03/2021 à 09:09, Mark Cave-Ayland a écrit :
> The original implementation of the Macintosh VIA devices in commit 6dca62a000
> "hw/m68k: add VIA support" used timer optimisations to reduce high CPU usage on
> the host when booting Linux. These optimisations worked by waiting until VIA1
> port B was accessed before re-arming the timers.
> 
> The MacOS toolbox ROM constantly writes to VIA1 port B which calls
> via1_one_second_update() and via1_sixty_hz_update() to calculate the new expiry
> time, causing the timers to constantly reset and never fire. The effect of this
> is that the Ticks (0x16a) global variable holding the number of 60Hz timer ticks
> since reset is never incremented by the interrupt causing time to stand still.
> 
> Whilst the code was introduced as a performance optimisation, it is likely that
> the high CPU usage was actually caused by the incorrect 60Hz timer interval
> fixed in the previous patch. Remove the optimisation to keep everything simple
> and enable the MacOS toolbox ROM to start keeping time.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/misc/mac_via.c | 43 ++++---------------------------------------
>  1 file changed, 4 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index c6e1552a59..23b11dd522 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -299,30 +299,17 @@ enum {
>  
>  static void via1_sixty_hz_update(MOS6522Q800VIA1State *v1s)
>  {
> -    MOS6522State *s = MOS6522(v1s);
> -
>      /* 60 Hz irq */
>      v1s->next_sixty_hz = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 16630000) /
>                            16630000 * 16630000;
> -
> -    if (s->ier & VIA1_IRQ_60HZ) {
> -        timer_mod(v1s->sixty_hz_timer, v1s->next_sixty_hz);
> -    } else {
> -        timer_del(v1s->sixty_hz_timer);
> -    }
> +    timer_mod(v1s->sixty_hz_timer, v1s->next_sixty_hz);
>  }
>  
>  static void via1_one_second_update(MOS6522Q800VIA1State *v1s)
>  {
> -    MOS6522State *s = MOS6522(v1s);
> -
>      v1s->next_second = (qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000) /
>                         1000 * 1000;
> -    if (s->ier & VIA1_IRQ_ONE_SECOND) {
> -        timer_mod(v1s->one_second_timer, v1s->next_second);
> -    } else {
> -        timer_del(v1s->one_second_timer);
> -    }
> +    timer_mod(v1s->one_second_timer, v1s->next_second);
>  }
>  
>  static void via1_sixty_hz(void *opaque)
> @@ -893,21 +880,6 @@ static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      MOS6522Q800VIA1State *s = MOS6522_Q800_VIA1(opaque);
>      MOS6522State *ms = MOS6522(s);
> -    int64_t now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> -
> -    /*
> -     * If IRQs are disabled, timers are disabled, but we need to update
> -     * VIA1_IRQ_60HZ and VIA1_IRQ_ONE_SECOND bits in the IFR
> -     */
> -
> -    if (now >= s->next_sixty_hz) {
> -        ms->ifr |= VIA1_IRQ_60HZ;
> -        via1_sixty_hz_update(s);
> -    }
> -    if (now >= s->next_second) {
> -        ms->ifr |= VIA1_IRQ_ONE_SECOND;
> -        via1_one_second_update(s);
> -    }
>  
>      addr = (addr >> 9) & 0xf;
>      return mos6522_read(ms, addr, size);
> @@ -931,9 +903,6 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val,
>          v1s->last_b = ms->b;
>          break;
>      }
> -
> -    via1_one_second_update(v1s);
> -    via1_sixty_hz_update(v1s);
>  }
>  
>  static const MemoryRegionOps mos6522_q800_via1_ops = {
> @@ -978,16 +947,10 @@ static const MemoryRegionOps mos6522_q800_via2_ops = {
>  static void mac_via_reset(DeviceState *dev)
>  {
>      MacVIAState *m = MAC_VIA(dev);
> -    MOS6522Q800VIA1State *v1s = &m->mos6522_via1;
>      ADBBusState *adb_bus = &m->adb_bus;
>  
>      adb_set_autopoll_enabled(adb_bus, true);
>  
> -    timer_del(v1s->sixty_hz_timer);
> -    v1s->next_sixty_hz = 0;
> -    timer_del(v1s->one_second_timer);
> -    v1s->next_second = 0;
> -
>      m->cmd = REG_EMPTY;
>      m->alt = REG_EMPTY;
>  }
> @@ -1026,9 +989,11 @@ static void mac_via_realize(DeviceState *dev, Error **errp)
>      m->mos6522_via1.one_second_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
>                                                       via1_one_second,
>                                                       &m->mos6522_via1);
> +    via1_one_second_update(&m->mos6522_via1);
>      m->mos6522_via1.sixty_hz_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
>                                                    via1_sixty_hz,
>                                                    &m->mos6522_via1);
> +    via1_sixty_hz_update(&m->mos6522_via1);
>  
>      qemu_get_timedate(&tm, 0);
>      m->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>