[PATCH v2 0/7] mac_via: fixes and improvements

[PATCH v2 0/7] mac_via: fixes and improvements

[Mark Cave-Ayland]

This patchset consists of a number of mac_via fixes and improvements taken
from my attempts to boot MacOS on the q800 machine.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

v2:
- Rebase on master
- Add R-B tags from Laurent
- Add VIA_60HZ_TIMER_PERIOD_NS define in patch 6 along with a comment referencing
  GTTMFH as suggested by Zoltan and Laurent


Mark Cave-Ayland (7):
  mac_via: switch rtc pram trace-events to use hex rather than decimal
    for addresses
  mac_via: fix up adb_via_receive() trace events
  mac_via: allow long accesses to VIA registers
  mac_via: don't re-inject ADB response when switching to IDLE state
  mac_via: rename VBL timer to 60Hz timer
  mac_via: fix 60Hz VIA1 timer interval
  mac_via: remove VIA1 timer optimisations

 hw/misc/mac_via.c         | 194 +++++++++++++++-----------------------
 hw/misc/trace-events      |   4 +-
 include/hw/misc/mac_via.h |   8 +-
 3 files changed, 81 insertions(+), 125 deletions(-)

-- 
2.20.1

[PATCH v2 1/7] mac_via: switch rtc pram trace-events to use hex rather than decimal for addresses

[Mark Cave-Ayland]

Since all the documentation uses the hex offsets, this makes it much easier
to see what is going on.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/misc/trace-events | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 4b15db8ca4..81a45f7fa3 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -225,8 +225,8 @@ via1_rtc_cmd_test_write(int value) "value=0x%02x"
 via1_rtc_cmd_wprotect_write(int value) "value=0x%02x"
 via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
 via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
-via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
-via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
+via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=0x%x value=0x%02x"
+via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=0x%x value=0x%02x"
 via1_adb_send(const char *state, uint8_t data, const char *vadbint) "state %s data=0x%02x vADBInt=%s"
 via1_adb_receive(const char *state, uint8_t data, const char *vadbint, int status, int index, int size) "state %s data=0x%02x vADBInt=%s status=0x%x index=%d size=%d"
 via1_adb_poll(uint8_t data, const char *vadbint, int status, int index, int size) "data=0x%02x vADBInt=%s status=0x%x index=%d size=%d"
-- 
2.20.1

[PATCH v2 2/7] mac_via: fix up adb_via_receive() trace events

[Mark Cave-Ayland]

The use of the post-increment operator on adb_data_in_index meant that the
trace-event was accidentally displaying the next byte in the incoming ADB
data buffer rather than the current byte.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/misc/mac_via.c | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 488d086a17..0f6586e102 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -816,33 +816,37 @@ static void adb_via_receive(MacVIAState *s, int state, uint8_t *data)
         switch (s->adb_data_in_index) {
         case 0:
             /* First EVEN byte: vADBInt indicates bus timeout */
-            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
-                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
-                                   adb_bus->status, s->adb_data_in_index,
-                                   s->adb_data_in_size);
-
-            *data = s->adb_data_in[s->adb_data_in_index++];
+            *data = s->adb_data_in[s->adb_data_in_index];
             if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
                 ms->b &= ~VIA1B_vADBInt;
             } else {
                 ms->b |= VIA1B_vADBInt;
             }
-            break;
 
-        case 1:
-            /* First ODD byte: vADBInt indicates SRQ */
             trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
                                    *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
                                    adb_bus->status, s->adb_data_in_index,
                                    s->adb_data_in_size);
 
-            *data = s->adb_data_in[s->adb_data_in_index++];
+            s->adb_data_in_index++;
+            break;
+
+        case 1:
+            /* First ODD byte: vADBInt indicates SRQ */
+            *data = s->adb_data_in[s->adb_data_in_index];
             pending = adb_bus->pending & ~(1 << (s->adb_autopoll_cmd >> 4));
             if (pending) {
                 ms->b &= ~VIA1B_vADBInt;
             } else {
                 ms->b |= VIA1B_vADBInt;
             }
+
+            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
+                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
+                                   adb_bus->status, s->adb_data_in_index,
+                                   s->adb_data_in_size);
+
+            s->adb_data_in_index++;
             break;
 
         default:
@@ -852,14 +856,9 @@ static void adb_via_receive(MacVIAState *s, int state, uint8_t *data)
              * end of the poll reply, so provide these extra bytes below to
              * keep it happy
              */
-            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
-                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
-                                   adb_bus->status, s->adb_data_in_index,
-                                   s->adb_data_in_size);
-
             if (s->adb_data_in_index < s->adb_data_in_size) {
                 /* Next data byte */
-                *data = s->adb_data_in[s->adb_data_in_index++];
+                *data = s->adb_data_in[s->adb_data_in_index];
                 ms->b |= VIA1B_vADBInt;
             } else if (s->adb_data_in_index == s->adb_data_in_size) {
                 if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
@@ -869,7 +868,6 @@ static void adb_via_receive(MacVIAState *s, int state, uint8_t *data)
                     /* Return 0x0 after reply */
                     *data = 0;
                 }
-                s->adb_data_in_index++;
                 ms->b &= ~VIA1B_vADBInt;
             } else {
                 /* Bus timeout (no more data) */
@@ -878,6 +876,15 @@ static void adb_via_receive(MacVIAState *s, int state, uint8_t *data)
                 adb_bus->status = 0;
                 adb_autopoll_unblock(adb_bus);
             }
+
+            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
+                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
+                                   adb_bus->status, s->adb_data_in_index,
+                                   s->adb_data_in_size);
+
+            if (s->adb_data_in_index <= s->adb_data_in_size) {
+                s->adb_data_in_index++;
+            }
             break;
         }
 
-- 
2.20.1

[PATCH v2 3/7] mac_via: allow long accesses to VIA registers

[Mark Cave-Ayland]

The MacOS SCSI driver uses a long access to read the VIA registers rather than
just a single byte during the message out phase.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/misc/mac_via.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 0f6586e102..f38d6e2f6e 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -966,7 +966,7 @@ static const MemoryRegionOps mos6522_q800_via1_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 1,
-        .max_access_size = 1,
+        .max_access_size = 4,
     },
 };
 
@@ -995,7 +995,7 @@ static const MemoryRegionOps mos6522_q800_via2_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 1,
-        .max_access_size = 1,
+        .max_access_size = 4,
     },
 };
 
-- 
2.20.1

[PATCH v2 4/7] mac_via: don't re-inject ADB response when switching to IDLE state

[Mark Cave-Ayland]

The current workaround for the Linux ADB state machine in kernels < 5.6 switching
the VIA back to IDLE state between send and receive modes is to re-inject the
first byte of the response in the IDLE state, and then force the state machine
into generating an autopoll reply.

In fact what is happening is much simpler: analysis of traces from a real Quadra
suggest that the existing data is returned as the first autopoll response rather
than generating an immediate response starting whilst still in IDLE state.

Update the ADB receive code to work in the same way, which allows the re-injection
code to be completely removed from adb_via_receive() and for adb_via_poll() to
be simplified accordingly.

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

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index f38d6e2f6e..76f31b8cae 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -609,7 +609,6 @@ static void adb_via_poll(void *opaque)
     uint8_t obuf[9];
     uint8_t *data = &s->sr;
     int olen;
-    uint16_t pending;
 
     /*
      * Setting vADBInt below indicates that an autopoll reply has been
@@ -618,36 +617,36 @@ static void adb_via_poll(void *opaque)
      */
     adb_autopoll_block(adb_bus);
 
-    m->adb_data_in_index = 0;
-    m->adb_data_out_index = 0;
-    olen = adb_poll(adb_bus, obuf, adb_bus->autopoll_mask);
-
-    if (olen > 0) {
-        /* Autopoll response */
-        *data = obuf[0];
-        olen--;
-        memcpy(m->adb_data_in, &obuf[1], olen);
-        m->adb_data_in_size = olen;
+    if (m->adb_data_in_size > 0 && m->adb_data_in_index == 0) {
+        /*
+         * For older Linux kernels that switch to IDLE mode after sending the
+         * ADB command, detect if there is an existing response and return that
+         * as a a "fake" autopoll reply or bus timeout accordingly
+         */
+        *data = m->adb_data_out[0];
+        olen = m->adb_data_in_size;
 
         s->b &= ~VIA1B_vADBInt;
         qemu_irq_raise(m->adb_data_ready);
-    } else if (olen < 0) {
-        /* Bus timeout (device does not exist) */
-        *data = 0xff;
-        s->b |= VIA1B_vADBInt;
-        adb_autopoll_unblock(adb_bus);
     } else {
-        pending = adb_bus->pending & ~(1 << (m->adb_autopoll_cmd >> 4));
+        /*
+         * Otherwise poll as normal
+         */
+        m->adb_data_in_index = 0;
+        m->adb_data_out_index = 0;
+        olen = adb_poll(adb_bus, obuf, adb_bus->autopoll_mask);
+
+        if (olen > 0) {
+            /* Autopoll response */
+            *data = obuf[0];
+            olen--;
+            memcpy(m->adb_data_in, &obuf[1], olen);
+            m->adb_data_in_size = olen;
 
-        if (pending) {
-            /*
-             * Bus timeout (device exists but another device has data). Block
-             * autopoll so the OS can read out the first EVEN and first ODD
-             * byte to determine bus timeout and SRQ status
-             */
-            *data = m->adb_autopoll_cmd;
             s->b &= ~VIA1B_vADBInt;
-
+            qemu_irq_raise(m->adb_data_ready);
+        } else {
+            *data = m->adb_autopoll_cmd;
             obuf[0] = 0xff;
             obuf[1] = 0xff;
             olen = 2;
@@ -655,12 +654,8 @@ static void adb_via_poll(void *opaque)
             memcpy(m->adb_data_in, obuf, olen);
             m->adb_data_in_size = olen;
 
+            s->b &= ~VIA1B_vADBInt;
             qemu_irq_raise(m->adb_data_ready);
-        } else {
-            /* Bus timeout (device exists but no other device has data) */
-            *data = 0;
-            s->b |= VIA1B_vADBInt;
-            adb_autopoll_unblock(adb_bus);
         }
     }
 
@@ -783,27 +778,8 @@ static void adb_via_receive(MacVIAState *s, int state, uint8_t *data)
         return;
 
     case ADB_STATE_IDLE:
-        /*
-         * Since adb_request() will have already consumed the data from the
-         * device, we must detect this extra state change and re-inject the
-         * reponse as either a "fake" autopoll reply or bus timeout
-         * accordingly
-         */
-        if (s->adb_data_in_index == 0) {
-            if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
-                *data = 0xff;
-                ms->b |= VIA1B_vADBInt;
-                qemu_irq_raise(s->adb_data_ready);
-            } else if (s->adb_data_in_size > 0) {
-                adb_bus->status = ADB_STATUS_POLLREPLY;
-                *data = s->adb_autopoll_cmd;
-                ms->b &= ~VIA1B_vADBInt;
-                qemu_irq_raise(s->adb_data_ready);
-            }
-        } else {
-            ms->b |= VIA1B_vADBInt;
-            adb_autopoll_unblock(adb_bus);
-        }
+        ms->b |= VIA1B_vADBInt;
+        adb_autopoll_unblock(adb_bus);
 
         trace_via1_adb_receive("IDLE", *data,
                         (ms->b & VIA1B_vADBInt) ? "+" : "-", adb_bus->status,
-- 
2.20.1

[PATCH v2 5/7] mac_via: rename VBL timer to 60Hz timer

[Mark Cave-Ayland]

According to the "Guide To The Macintosh Family Hardware", the 60Hz VIA1 timer
on newer Macs such as the Quadra only exists for compatibility with old software
and is no longer synced to the VBL interval.

Rename the VBL timer to 60Hz timer to emphasise this and to prevent confusion
when the real VBL interrupt (now handled as a NuBus slot interrupt) is added in
future.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/misc/mac_via.c         | 41 ++++++++++++++++++++-------------------
 include/hw/misc/mac_via.h |  8 ++++----
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 76f31b8cae..f994fefa7c 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -297,18 +297,18 @@ enum {
     REG_EMPTY = 0xff,
 };
 
-static void via1_VBL_update(MOS6522Q800VIA1State *v1s)
+static void via1_sixty_hz_update(MOS6522Q800VIA1State *v1s)
 {
     MOS6522State *s = MOS6522(v1s);
 
     /* 60 Hz irq */
-    v1s->next_VBL = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 16630) /
-                    16630 * 16630;
+    v1s->next_sixty_hz = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 16630) /
+                          16630 * 16630;
 
-    if (s->ier & VIA1_IRQ_VBLANK) {
-        timer_mod(v1s->VBL_timer, v1s->next_VBL);
+    if (s->ier & VIA1_IRQ_60HZ) {
+        timer_mod(v1s->sixty_hz_timer, v1s->next_sixty_hz);
     } else {
-        timer_del(v1s->VBL_timer);
+        timer_del(v1s->sixty_hz_timer);
     }
 }
 
@@ -325,16 +325,16 @@ static void via1_one_second_update(MOS6522Q800VIA1State *v1s)
     }
 }
 
-static void via1_VBL(void *opaque)
+static void via1_sixty_hz(void *opaque)
 {
     MOS6522Q800VIA1State *v1s = opaque;
     MOS6522State *s = MOS6522(v1s);
     MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
 
-    s->ifr |= VIA1_IRQ_VBLANK;
+    s->ifr |= VIA1_IRQ_60HZ;
     mdc->update_irq(s);
 
-    via1_VBL_update(v1s);
+    via1_sixty_hz_update(v1s);
 }
 
 static void via1_one_second(void *opaque)
@@ -897,12 +897,12 @@ static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned size)
 
     /*
      * If IRQs are disabled, timers are disabled, but we need to update
-     * VIA1_IRQ_VBLANK and VIA1_IRQ_ONE_SECOND bits in the IFR
+     * VIA1_IRQ_60HZ and VIA1_IRQ_ONE_SECOND bits in the IFR
      */
 
-    if (now >= s->next_VBL) {
-        ms->ifr |= VIA1_IRQ_VBLANK;
-        via1_VBL_update(s);
+    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;
@@ -933,7 +933,7 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val,
     }
 
     via1_one_second_update(v1s);
-    via1_VBL_update(v1s);
+    via1_sixty_hz_update(v1s);
 }
 
 static const MemoryRegionOps mos6522_q800_via1_ops = {
@@ -983,8 +983,8 @@ static void mac_via_reset(DeviceState *dev)
 
     adb_set_autopoll_enabled(adb_bus, true);
 
-    timer_del(v1s->VBL_timer);
-    v1s->next_VBL = 0;
+    timer_del(v1s->sixty_hz_timer);
+    v1s->next_sixty_hz = 0;
     timer_del(v1s->one_second_timer);
     v1s->next_second = 0;
 
@@ -1026,8 +1026,9 @@ 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);
-    m->mos6522_via1.VBL_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, via1_VBL,
-                                              &m->mos6522_via1);
+    m->mos6522_via1.sixty_hz_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+                                                  via1_sixty_hz,
+                                                  &m->mos6522_via1);
 
     qemu_get_timedate(&tm, 0);
     m->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
@@ -1116,8 +1117,8 @@ static const VMStateDescription vmstate_mac_via = {
         VMSTATE_BUFFER(mos6522_via1.PRAM, MacVIAState),
         VMSTATE_TIMER_PTR(mos6522_via1.one_second_timer, MacVIAState),
         VMSTATE_INT64(mos6522_via1.next_second, MacVIAState),
-        VMSTATE_TIMER_PTR(mos6522_via1.VBL_timer, MacVIAState),
-        VMSTATE_INT64(mos6522_via1.next_VBL, MacVIAState),
+        VMSTATE_TIMER_PTR(mos6522_via1.sixty_hz_timer, MacVIAState),
+        VMSTATE_INT64(mos6522_via1.next_sixty_hz, MacVIAState),
         VMSTATE_STRUCT(mos6522_via2.parent_obj, MacVIAState, 0, vmstate_mos6522,
                        MOS6522State),
         /* RTC */
diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index a59f0bd422..3058b30685 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -17,7 +17,7 @@
 
 /* VIA 1 */
 #define VIA1_IRQ_ONE_SECOND_BIT 0
-#define VIA1_IRQ_VBLANK_BIT     1
+#define VIA1_IRQ_60HZ_BIT       1
 #define VIA1_IRQ_ADB_READY_BIT  2
 #define VIA1_IRQ_ADB_DATA_BIT   3
 #define VIA1_IRQ_ADB_CLOCK_BIT  4
@@ -25,7 +25,7 @@
 #define VIA1_IRQ_NB             8
 
 #define VIA1_IRQ_ONE_SECOND (1 << VIA1_IRQ_ONE_SECOND_BIT)
-#define VIA1_IRQ_VBLANK     (1 << VIA1_IRQ_VBLANK_BIT)
+#define VIA1_IRQ_60HZ       (1 << VIA1_IRQ_60HZ_BIT)
 #define VIA1_IRQ_ADB_READY  (1 << VIA1_IRQ_ADB_READY_BIT)
 #define VIA1_IRQ_ADB_DATA   (1 << VIA1_IRQ_ADB_DATA_BIT)
 #define VIA1_IRQ_ADB_CLOCK  (1 << VIA1_IRQ_ADB_CLOCK_BIT)
@@ -45,8 +45,8 @@ struct MOS6522Q800VIA1State {
     /* external timers */
     QEMUTimer *one_second_timer;
     int64_t next_second;
-    QEMUTimer *VBL_timer;
-    int64_t next_VBL;
+    QEMUTimer *sixty_hz_timer;
+    int64_t next_sixty_hz;
 };
 
 
-- 
2.20.1

Re: [PATCH v2 5/7] mac_via: rename VBL timer to 60Hz timer

[BALATON Zoltan]

On Thu, 11 Mar 2021, Mark Cave-Ayland wrote:
> According to the "Guide To The Macintosh Family Hardware", the 60Hz VIA1 timer
> on newer Macs such as the Quadra only exists for compatibility with old software
> and is no longer synced to the VBL interval.
>
> Rename the VBL timer to 60Hz timer to emphasise this and to prevent confusion
> when the real VBL interrupt (now handled as a NuBus slot interrupt) is added in
> future.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/misc/mac_via.c         | 41 ++++++++++++++++++++-------------------
> include/hw/misc/mac_via.h |  8 ++++----
> 2 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index 76f31b8cae..f994fefa7c 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -297,18 +297,18 @@ enum {
>     REG_EMPTY = 0xff,
> };
>
> -static void via1_VBL_update(MOS6522Q800VIA1State *v1s)
> +static void via1_sixty_hz_update(MOS6522Q800VIA1State *v1s)
> {
>     MOS6522State *s = MOS6522(v1s);
>
>     /* 60 Hz irq */
> -    v1s->next_VBL = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 16630) /
> -                    16630 * 16630;
> +    v1s->next_sixty_hz = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 16630) /
> +                          16630 * 16630;
>
> -    if (s->ier & VIA1_IRQ_VBLANK) {
> -        timer_mod(v1s->VBL_timer, v1s->next_VBL);
> +    if (s->ier & VIA1_IRQ_60HZ) {
> +        timer_mod(v1s->sixty_hz_timer, v1s->next_sixty_hz);
>     } else {
> -        timer_del(v1s->VBL_timer);
> +        timer_del(v1s->sixty_hz_timer);
>     }
> }
>
> @@ -325,16 +325,16 @@ static void via1_one_second_update(MOS6522Q800VIA1State *v1s)
>     }
> }
>
> -static void via1_VBL(void *opaque)
> +static void via1_sixty_hz(void *opaque)
> {
>     MOS6522Q800VIA1State *v1s = opaque;
>     MOS6522State *s = MOS6522(v1s);
>     MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
>
> -    s->ifr |= VIA1_IRQ_VBLANK;
> +    s->ifr |= VIA1_IRQ_60HZ;
>     mdc->update_irq(s);
>
> -    via1_VBL_update(v1s);
> +    via1_sixty_hz_update(v1s);
> }
>
> static void via1_one_second(void *opaque)
> @@ -897,12 +897,12 @@ static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned size)
>
>     /*
>      * If IRQs are disabled, timers are disabled, but we need to update
> -     * VIA1_IRQ_VBLANK and VIA1_IRQ_ONE_SECOND bits in the IFR
> +     * VIA1_IRQ_60HZ and VIA1_IRQ_ONE_SECOND bits in the IFR
>      */
>
> -    if (now >= s->next_VBL) {
> -        ms->ifr |= VIA1_IRQ_VBLANK;
> -        via1_VBL_update(s);
> +    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;
> @@ -933,7 +933,7 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val,
>     }
>
>     via1_one_second_update(v1s);
> -    via1_VBL_update(v1s);
> +    via1_sixty_hz_update(v1s);
> }
>
> static const MemoryRegionOps mos6522_q800_via1_ops = {
> @@ -983,8 +983,8 @@ static void mac_via_reset(DeviceState *dev)
>
>     adb_set_autopoll_enabled(adb_bus, true);
>
> -    timer_del(v1s->VBL_timer);
> -    v1s->next_VBL = 0;
> +    timer_del(v1s->sixty_hz_timer);
> +    v1s->next_sixty_hz = 0;
>     timer_del(v1s->one_second_timer);
>     v1s->next_second = 0;
>
> @@ -1026,8 +1026,9 @@ 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);
> -    m->mos6522_via1.VBL_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, via1_VBL,
> -                                              &m->mos6522_via1);
> +    m->mos6522_via1.sixty_hz_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> +                                                  via1_sixty_hz,
> +                                                  &m->mos6522_via1);
>
>     qemu_get_timedate(&tm, 0);
>     m->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
> @@ -1116,8 +1117,8 @@ static const VMStateDescription vmstate_mac_via = {
>         VMSTATE_BUFFER(mos6522_via1.PRAM, MacVIAState),
>         VMSTATE_TIMER_PTR(mos6522_via1.one_second_timer, MacVIAState),
>         VMSTATE_INT64(mos6522_via1.next_second, MacVIAState),
> -        VMSTATE_TIMER_PTR(mos6522_via1.VBL_timer, MacVIAState),
> -        VMSTATE_INT64(mos6522_via1.next_VBL, MacVIAState),
> +        VMSTATE_TIMER_PTR(mos6522_via1.sixty_hz_timer, MacVIAState),
> +        VMSTATE_INT64(mos6522_via1.next_sixty_hz, MacVIAState),
>         VMSTATE_STRUCT(mos6522_via2.parent_obj, MacVIAState, 0, vmstate_mos6522,
>                        MOS6522State),
>         /* RTC */
> diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
> index a59f0bd422..3058b30685 100644
> --- a/include/hw/misc/mac_via.h
> +++ b/include/hw/misc/mac_via.h
> @@ -17,7 +17,7 @@
>
> /* VIA 1 */
> #define VIA1_IRQ_ONE_SECOND_BIT 0
> -#define VIA1_IRQ_VBLANK_BIT     1
> +#define VIA1_IRQ_60HZ_BIT       1
> #define VIA1_IRQ_ADB_READY_BIT  2
> #define VIA1_IRQ_ADB_DATA_BIT   3
> #define VIA1_IRQ_ADB_CLOCK_BIT  4
> @@ -25,7 +25,7 @@
> #define VIA1_IRQ_NB             8
>
> #define VIA1_IRQ_ONE_SECOND (1 << VIA1_IRQ_ONE_SECOND_BIT)
> -#define VIA1_IRQ_VBLANK     (1 << VIA1_IRQ_VBLANK_BIT)
> +#define VIA1_IRQ_60HZ       (1 << VIA1_IRQ_60HZ_BIT)
> #define VIA1_IRQ_ADB_READY  (1 << VIA1_IRQ_ADB_READY_BIT)
> #define VIA1_IRQ_ADB_DATA   (1 << VIA1_IRQ_ADB_DATA_BIT)
> #define VIA1_IRQ_ADB_CLOCK  (1 << VIA1_IRQ_ADB_CLOCK_BIT)
> @@ -45,8 +45,8 @@ struct MOS6522Q800VIA1State {
>     /* external timers */
>     QEMUTimer *one_second_timer;
>     int64_t next_second;
> -    QEMUTimer *VBL_timer;
> -    int64_t next_VBL;
> +    QEMUTimer *sixty_hz_timer;
> +    int64_t next_sixty_hz;
> };
>
>
>

[PATCH v2 6/7] mac_via: fix 60Hz VIA1 timer interval

[Mark Cave-Ayland]

The 60Hz timer is initialised using timer_new_ns() meaning that the timer
interval should be measured in ns, and therefore its period is a thousand
times too short.

Use a define for the 60Hz timer period taking the more precise value as
documented in the Guide To The Macintosh Family Hardware.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/misc/mac_via.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index f994fefa7c..e7dd31f291 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -279,6 +279,12 @@
 #define VIA_TIMER_FREQ (783360)
 #define VIA_ADB_POLL_FREQ 50 /* XXX: not real */
 
+/*
+ * Guide to the Macintosh Family Hardware ch. 12 "Displays" p. 401 gives the
+ * precise 60Hz interrupt frequency as ~60.15Hz with a period of 16625.8 us
+ */
+#define VIA_60HZ_TIMER_PERIOD_NS   16625800
+
 /* VIA returns time offset from Jan 1, 1904, not 1970 */
 #define RTC_OFFSET 2082844800
 
@@ -302,8 +308,9 @@ 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) + 16630) /
-                          16630 * 16630;
+    v1s->next_sixty_hz = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+                          VIA_60HZ_TIMER_PERIOD_NS) /
+                          VIA_60HZ_TIMER_PERIOD_NS * VIA_60HZ_TIMER_PERIOD_NS;
 
     if (s->ier & VIA1_IRQ_60HZ) {
         timer_mod(v1s->sixty_hz_timer, v1s->next_sixty_hz);
-- 
2.20.1

Re: [PATCH v2 6/7] mac_via: fix 60Hz VIA1 timer interval

[BALATON Zoltan]

On Thu, 11 Mar 2021, Mark Cave-Ayland wrote:
> The 60Hz timer is initialised using timer_new_ns() meaning that the timer
> interval should be measured in ns, and therefore its period is a thousand
> times too short.
>
> Use a define for the 60Hz timer period taking the more precise value as
> documented in the Guide To The Macintosh Family Hardware.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/misc/mac_via.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index f994fefa7c..e7dd31f291 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -279,6 +279,12 @@
> #define VIA_TIMER_FREQ (783360)
> #define VIA_ADB_POLL_FREQ 50 /* XXX: not real */
>
> +/*
> + * Guide to the Macintosh Family Hardware ch. 12 "Displays" p. 401 gives the
> + * precise 60Hz interrupt frequency as ~60.15Hz with a period of 16625.8 us
> + */
> +#define VIA_60HZ_TIMER_PERIOD_NS   16625800
> +
> /* VIA returns time offset from Jan 1, 1904, not 1970 */
> #define RTC_OFFSET 2082844800
>
> @@ -302,8 +308,9 @@ 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) + 16630) /
> -                          16630 * 16630;
> +    v1s->next_sixty_hz = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> +                          VIA_60HZ_TIMER_PERIOD_NS) /
> +                          VIA_60HZ_TIMER_PERIOD_NS * VIA_60HZ_TIMER_PERIOD_NS;
>
>     if (s->ier & VIA1_IRQ_60HZ) {
>         timer_mod(v1s->sixty_hz_timer, v1s->next_sixty_hz);
>

[PATCH v2 7/7] mac_via: remove VIA1 timer optimisations

[Mark Cave-Ayland]

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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 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 e7dd31f291..f164c8c606 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -305,31 +305,18 @@ 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) +
                           VIA_60HZ_TIMER_PERIOD_NS) /
                           VIA_60HZ_TIMER_PERIOD_NS * VIA_60HZ_TIMER_PERIOD_NS;
-
-    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)
@@ -900,21 +887,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);
@@ -938,9 +910,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 = {
@@ -985,16 +954,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;
 }
@@ -1033,9 +996,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;
-- 
2.20.1

Re: [PATCH v2 0/7] mac_via: fixes and improvements

[Laurent Vivier]

Le 11/03/2021 à 11:04, Mark Cave-Ayland a écrit :
> This patchset consists of a number of mac_via fixes and improvements taken
> from my attempts to boot MacOS on the q800 machine.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> v2:
> - Rebase on master
> - Add R-B tags from Laurent
> - Add VIA_60HZ_TIMER_PERIOD_NS define in patch 6 along with a comment referencing
>   GTTMFH as suggested by Zoltan and Laurent
> 
> 
> Mark Cave-Ayland (7):
>   mac_via: switch rtc pram trace-events to use hex rather than decimal
>     for addresses
>   mac_via: fix up adb_via_receive() trace events
>   mac_via: allow long accesses to VIA registers
>   mac_via: don't re-inject ADB response when switching to IDLE state
>   mac_via: rename VBL timer to 60Hz timer
>   mac_via: fix 60Hz VIA1 timer interval
>   mac_via: remove VIA1 timer optimisations
> 
>  hw/misc/mac_via.c         | 194 +++++++++++++++-----------------------
>  hw/misc/trace-events      |   4 +-
>  include/hw/misc/mac_via.h |   8 +-
>  3 files changed, 81 insertions(+), 125 deletions(-)
> 

Applied to my q800 branch for 6.0

Thanks,
Laurent