[PATCH v2 00/12] mos6522: switch to gpios, add control line edge-triggering and extra debugging

[PATCH v2 00/12] mos6522: switch to gpios, add control line edge-triggering and extra debugging

[Mark Cave-Ayland]

Here is another patchset taken from my series to enable MacOS to boot on the q800
machine.

Patches 1-3 define the IFR bit flags in terms of the physical control lines and
update mac_via to use them.

Patch 4 does the main switch from custom methods in MOS6522DeviceClass to using
standard gpios whilst patch 5 removes these now-obsolete methods.

Patch 6 updates mos6522 instances to use the recommended method of calling
device_class_set_parent_reset() to propagate the device reset to the parent.

Patches 7 and 8 add more support for debugging guests using the mos6522 devices
by adding register names into the trace-event output and implementing a new
"info via" HMP command to give detailed information about the registers and timer
states.

Patch 9 introduces a new last_irq_levels field within MOS6522State to enable detection
of edge transitions, which is also a migration break for the q800 and g3beige/mac99
machines.

Patch 10 ensures that the SCSI_DATA (DRQ) bit in VIA2 is unlatched since in the q800
machine VIA2 is integrated within the on-board logic, and analysis of the MacOS
toolbox ROM suggests that the DRQ bit is expected to be live compared with older
Macs which use a real (latched) VIA.

Patches 11 implement edge-triggering for the CA1/2 and CB1/2 control lines as
documented in the datasheet, including updating the relevant inputs for negative
edge-triggering if required.

Finally patch 12 removes some old code in the PMU mos6522 instance which is now no
longer required with these latest changes.

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

v2:
- Update patches 1-3 to use the BIT() macro
- Add R-B tags from Peter
- Update "info via" patch to use a target-specific HMP command as discussed on-list
- Add patch 10 "mac_via: make SCSI_DATA (DRQ) bit live rather than latched"


Mark Cave-Ayland (12):
  mos6522: add defines for IFR bit flags
  mac_via: use IFR bit flag constants for VIA1 IRQs
  mac_via: use IFR bit flag constants for VIA2 IRQs
  mos6522: switch over to use qdev gpios for IRQs
  mos6522: remove update_irq() and set_sr_int() methods from
    MOS6522DeviceClass
  mos6522: use device_class_set_parent_reset() to propagate reset to
    parent
  mos6522: add register names to register read/write trace events
  mos6522: add "info via" HMP command for debugging
  mos6522: record last_irq_levels in mos6522_set_irq()
  mac_via: make SCSI_DATA (DRQ) bit live rather than latched
  mos6522: implement edge-triggering for CA1/2 and CB1/2 control line
    IRQs
  macio/pmu.c: remove redundant code

 hmp-commands-info.hx         |  15 +++
 hw/m68k/q800.c               |   9 +-
 hw/misc/mac_via.c            |  87 ++++++--------
 hw/misc/macio/cuda.c         |   8 +-
 hw/misc/macio/pmu.c          |  40 +------
 hw/misc/mos6522.c            | 223 ++++++++++++++++++++++++++++++++---
 hw/misc/trace-events         |   4 +-
 include/hw/misc/mac_via.h    |  46 ++++----
 include/hw/misc/macio/pmu.h  |   2 -
 include/hw/misc/mos6522.h    |  44 +++++--
 include/monitor/hmp-target.h |   1 +
 11 files changed, 333 insertions(+), 146 deletions(-)

-- 
2.20.1

[PATCH v2 01/12] mos6522: add defines for IFR bit flags

[Mark Cave-Ayland]

These are intended to make it easier to see how the physical control lines
are wired for each instance.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 include/hw/misc/mos6522.h | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index fc95d22b0f..be5c90d24d 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -41,13 +41,21 @@
 #define IER_SET            0x80    /* set bits in IER */
 #define IER_CLR            0       /* clear bits in IER */
 
-#define CA2_INT            0x01
-#define CA1_INT            0x02
-#define SR_INT             0x04    /* Shift register full/empty */
-#define CB2_INT            0x08
-#define CB1_INT            0x10
-#define T2_INT             0x20    /* Timer 2 interrupt */
-#define T1_INT             0x40    /* Timer 1 interrupt */
+#define CA2_INT_BIT        0
+#define CA1_INT_BIT        1
+#define SR_INT_BIT         2       /* Shift register full/empty */
+#define CB2_INT_BIT        3
+#define CB1_INT_BIT        4
+#define T2_INT_BIT         5       /* Timer 2 interrupt */
+#define T1_INT_BIT         6       /* Timer 1 interrupt */
+
+#define CA2_INT            BIT(CA2_INT_BIT)
+#define CA1_INT            BIT(CA1_INT_BIT)
+#define SR_INT             BIT(SR_INT_BIT)
+#define CB2_INT            BIT(CB2_INT_BIT)
+#define CB1_INT            BIT(CB1_INT_BIT)
+#define T2_INT             BIT(T2_INT_BIT)
+#define T1_INT             BIT(T1_INT_BIT)
 
 /* Bits in ACR */
 #define T1MODE             0xc0    /* Timer 1 mode */
-- 
2.20.1

Re: [PATCH v2 01/12] mos6522: add defines for IFR bit flags

[Philippe Mathieu-Daudé]

On 24/2/22 12:59, Mark Cave-Ayland wrote:
> These are intended to make it easier to see how the physical control lines
> are wired for each instance.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   include/hw/misc/mos6522.h | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v2 01/12] mos6522: add defines for IFR bit flags

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> These are intended to make it easier to see how the physical control lines
> are wired for each instance.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   include/hw/misc/mos6522.h | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)

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

[PATCH v2 02/12] mac_via: use IFR bit flag constants for VIA1 IRQs

[Mark Cave-Ayland]

This allows us to easily see how the physical control lines are mapped to the
IFR bit flags.

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

diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index b445565866..b0535c84da 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -18,19 +18,19 @@
 #define VIA_SIZE   0x2000
 
 /* VIA 1 */
-#define VIA1_IRQ_ONE_SECOND_BIT 0
-#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
+#define VIA1_IRQ_ONE_SECOND_BIT CA2_INT_BIT
+#define VIA1_IRQ_60HZ_BIT       CA1_INT_BIT
+#define VIA1_IRQ_ADB_READY_BIT  SR_INT_BIT
+#define VIA1_IRQ_ADB_DATA_BIT   CB2_INT_BIT
+#define VIA1_IRQ_ADB_CLOCK_BIT  CB1_INT_BIT
 
 #define VIA1_IRQ_NB             8
 
-#define VIA1_IRQ_ONE_SECOND     (1 << VIA1_IRQ_ONE_SECOND_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)
+#define VIA1_IRQ_ONE_SECOND     BIT(VIA1_IRQ_ONE_SECOND_BIT)
+#define VIA1_IRQ_60HZ           BIT(VIA1_IRQ_60HZ_BIT)
+#define VIA1_IRQ_ADB_READY      BIT(VIA1_IRQ_ADB_READY_BIT)
+#define VIA1_IRQ_ADB_DATA       BIT(VIA1_IRQ_ADB_DATA_BIT)
+#define VIA1_IRQ_ADB_CLOCK      BIT(VIA1_IRQ_ADB_CLOCK_BIT)
 
 
 #define TYPE_MOS6522_Q800_VIA1 "mos6522-q800-via1"
-- 
2.20.1

Re: [PATCH v2 02/12] mac_via: use IFR bit flag constants for VIA1 IRQs

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> This allows us to easily see how the physical control lines are mapped to the
> IFR bit flags.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   include/hw/misc/mac_via.h | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 

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

[PATCH v2 03/12] mac_via: use IFR bit flag constants for VIA2 IRQs

[Mark Cave-Ayland]

This allows us to easily see how the physical control lines are mapped to the
IFR bit flags.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 include/hw/misc/mac_via.h | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index b0535c84da..0af346366e 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -80,19 +80,18 @@ struct MOS6522Q800VIA1State {
 
 
 /* VIA 2 */
-#define VIA2_IRQ_SCSI_DATA_BIT  0
-#define VIA2_IRQ_NUBUS_BIT      1
-#define VIA2_IRQ_UNUSED_BIT     2
-#define VIA2_IRQ_SCSI_BIT       3
-#define VIA2_IRQ_ASC_BIT        4
+#define VIA2_IRQ_SCSI_DATA_BIT  CA2_INT_BIT
+#define VIA2_IRQ_NUBUS_BIT      CA1_INT_BIT
+#define VIA2_IRQ_SCSI_BIT       CB2_INT_BIT
+#define VIA2_IRQ_ASC_BIT        CB1_INT_BIT
 
 #define VIA2_IRQ_NB             8
 
-#define VIA2_IRQ_SCSI_DATA      (1 << VIA2_IRQ_SCSI_DATA_BIT)
-#define VIA2_IRQ_NUBUS          (1 << VIA2_IRQ_NUBUS_BIT)
-#define VIA2_IRQ_UNUSED         (1 << VIA2_IRQ_SCSI_BIT)
-#define VIA2_IRQ_SCSI           (1 << VIA2_IRQ_UNUSED_BIT)
-#define VIA2_IRQ_ASC            (1 << VIA2_IRQ_ASC_BIT)
+#define VIA2_IRQ_SCSI_DATA      BIT(VIA2_IRQ_SCSI_DATA_BIT)
+#define VIA2_IRQ_NUBUS          BIT(VIA2_IRQ_NUBUS_BIT)
+#define VIA2_IRQ_UNUSED         BIT(VIA2_IRQ_SCSI_BIT)
+#define VIA2_IRQ_SCSI           BIT(VIA2_IRQ_UNUSED_BIT)
+#define VIA2_IRQ_ASC            BIT(VIA2_IRQ_ASC_BIT)
 
 #define VIA2_NUBUS_IRQ_NB       7
 
-- 
2.20.1

Re: [PATCH v2 03/12] mac_via: use IFR bit flag constants for VIA2 IRQs

[Philippe Mathieu-Daudé]

On 24/2/22 12:59, Mark Cave-Ayland wrote:
> This allows us to easily see how the physical control lines are mapped to the
> IFR bit flags.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   include/hw/misc/mac_via.h | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v2 03/12] mac_via: use IFR bit flag constants for VIA2 IRQs

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> This allows us to easily see how the physical control lines are mapped to the
> IFR bit flags.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   include/hw/misc/mac_via.h | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
> 

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

[PATCH v2 04/12] mos6522: switch over to use qdev gpios for IRQs

[Mark Cave-Ayland]

For historical reasons each mos6522 instance implements its own setting and
update of the IFR flag bits using methods exposed by MOS6522DeviceClass. As
of today this is no longer required, and it is now possible to implement
the mos6522 IRQs as standard qdev gpios.

Switch over to use qdev gpios for the mos6522 device and update all instances
accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/mac_via.c         | 56 +++++++--------------------------------
 hw/misc/macio/cuda.c      |  5 ++--
 hw/misc/macio/pmu.c       |  4 +--
 hw/misc/mos6522.c         | 15 +++++++++++
 include/hw/misc/mac_via.h |  5 ----
 include/hw/misc/mos6522.h |  2 ++
 6 files changed, 31 insertions(+), 56 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 71b74c3372..80eb433044 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -325,10 +325,9 @@ static void via1_sixty_hz(void *opaque)
 {
     MOS6522Q800VIA1State *v1s = opaque;
     MOS6522State *s = MOS6522(v1s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
+    qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA1_IRQ_60HZ_BIT);
 
-    s->ifr |= VIA1_IRQ_60HZ;
-    mdc->update_irq(s);
+    qemu_set_irq(irq, 1);
 
     via1_sixty_hz_update(v1s);
 }
@@ -337,44 +336,13 @@ static void via1_one_second(void *opaque)
 {
     MOS6522Q800VIA1State *v1s = opaque;
     MOS6522State *s = MOS6522(v1s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
+    qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA1_IRQ_ONE_SECOND_BIT);
 
-    s->ifr |= VIA1_IRQ_ONE_SECOND;
-    mdc->update_irq(s);
+    qemu_set_irq(irq, 1);
 
     via1_one_second_update(v1s);
 }
 
-static void via1_irq_request(void *opaque, int irq, int level)
-{
-    MOS6522Q800VIA1State *v1s = opaque;
-    MOS6522State *s = MOS6522(v1s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
-
-    if (level) {
-        s->ifr |= 1 << irq;
-    } else {
-        s->ifr &= ~(1 << irq);
-    }
-
-    mdc->update_irq(s);
-}
-
-static void via2_irq_request(void *opaque, int irq, int level)
-{
-    MOS6522Q800VIA2State *v2s = opaque;
-    MOS6522State *s = MOS6522(v2s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
-
-    if (level) {
-        s->ifr |= 1 << irq;
-    } else {
-        s->ifr &= ~(1 << irq);
-    }
-
-    mdc->update_irq(s);
-}
-
 
 static void pram_update(MOS6522Q800VIA1State *v1s)
 {
@@ -1061,8 +1029,6 @@ static void mos6522_q800_via1_init(Object *obj)
     qbus_init((BusState *)&v1s->adb_bus, sizeof(v1s->adb_bus),
               TYPE_ADB_BUS, DEVICE(v1s), "adb.0");
 
-    qdev_init_gpio_in(DEVICE(obj), via1_irq_request, VIA1_IRQ_NB);
-
     /* A/UX mode */
     qdev_init_gpio_out(DEVICE(obj), &v1s->auxmode_irq, 1);
 }
@@ -1150,22 +1116,20 @@ static void mos6522_q800_via2_reset(DeviceState *dev)
     ms->a = 0x7f;
 }
 
-static void via2_nubus_irq_request(void *opaque, int irq, int level)
+static void via2_nubus_irq_request(void *opaque, int n, int level)
 {
     MOS6522Q800VIA2State *v2s = opaque;
     MOS6522State *s = MOS6522(v2s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
+    qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA2_IRQ_NUBUS_BIT);
 
     if (level) {
         /* Port A nubus IRQ inputs are active LOW */
-        s->a &= ~(1 << irq);
-        s->ifr |= 1 << VIA2_IRQ_NUBUS_BIT;
+        s->a &= ~(1 << n);
     } else {
-        s->a |= (1 << irq);
-        s->ifr &= ~(1 << VIA2_IRQ_NUBUS_BIT);
+        s->a |= (1 << n);
     }
 
-    mdc->update_irq(s);
+    qemu_set_irq(irq, level);
 }
 
 static void mos6522_q800_via2_init(Object *obj)
@@ -1177,8 +1141,6 @@ static void mos6522_q800_via2_init(Object *obj)
                           "via2", VIA_SIZE);
     sysbus_init_mmio(sbd, &v2s->via_mem);
 
-    qdev_init_gpio_in(DEVICE(obj), via2_irq_request, VIA2_IRQ_NB);
-
     qdev_init_gpio_in_named(DEVICE(obj), via2_nubus_irq_request, "nubus-irq",
                             VIA2_NUBUS_IRQ_NB);
 }
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 233daf1405..693fc82e05 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/irq.h"
 #include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
@@ -96,9 +97,9 @@ static void cuda_set_sr_int(void *opaque)
     CUDAState *s = opaque;
     MOS6522CUDAState *mcs = &s->mos6522_cuda;
     MOS6522State *ms = MOS6522(mcs);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(ms);
+    qemu_irq irq = qdev_get_gpio_in(DEVICE(ms), SR_INT_BIT);
 
-    mdc->set_sr_int(ms);
+    qemu_set_irq(irq, 1);
 }
 
 static void cuda_delay_set_sr_int(CUDAState *s)
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 76c608ee19..b210068ab7 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -75,9 +75,9 @@ static void via_set_sr_int(void *opaque)
     PMUState *s = opaque;
     MOS6522PMUState *mps = MOS6522_PMU(&s->mos6522_pmu);
     MOS6522State *ms = MOS6522(mps);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(ms);
+    qemu_irq irq = qdev_get_gpio_in(DEVICE(ms), SR_INT_BIT);
 
-    mdc->set_sr_int(ms);
+    qemu_set_irq(irq, 1);
 }
 
 static void pmu_update_extirq(PMUState *s)
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 1c57332b40..6be6853dc2 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -52,6 +52,19 @@ static void mos6522_update_irq(MOS6522State *s)
     }
 }
 
+static void mos6522_set_irq(void *opaque, int n, int level)
+{
+    MOS6522State *s = MOS6522(opaque);
+
+    if (level) {
+        s->ifr |= 1 << n;
+    } else {
+        s->ifr &= ~(1 << n);
+    }
+
+    mos6522_update_irq(s);
+}
+
 static uint64_t get_counter_value(MOS6522State *s, MOS6522Timer *ti)
 {
     MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
@@ -488,6 +501,8 @@ static void mos6522_init(Object *obj)
 
     s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s);
     s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s);
+
+    qdev_init_gpio_in(DEVICE(obj), mos6522_set_irq, VIA_NUM_INTS);
 }
 
 static void mos6522_finalize(Object *obj)
diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index 0af346366e..5fe7a7f592 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -24,8 +24,6 @@
 #define VIA1_IRQ_ADB_DATA_BIT   CB2_INT_BIT
 #define VIA1_IRQ_ADB_CLOCK_BIT  CB1_INT_BIT
 
-#define VIA1_IRQ_NB             8
-
 #define VIA1_IRQ_ONE_SECOND     BIT(VIA1_IRQ_ONE_SECOND_BIT)
 #define VIA1_IRQ_60HZ           BIT(VIA1_IRQ_60HZ_BIT)
 #define VIA1_IRQ_ADB_READY      BIT(VIA1_IRQ_ADB_READY_BIT)
@@ -42,7 +40,6 @@ struct MOS6522Q800VIA1State {
 
     MemoryRegion via_mem;
 
-    qemu_irq irqs[VIA1_IRQ_NB];
     qemu_irq auxmode_irq;
     uint8_t last_b;
 
@@ -85,8 +82,6 @@ struct MOS6522Q800VIA1State {
 #define VIA2_IRQ_SCSI_BIT       CB2_INT_BIT
 #define VIA2_IRQ_ASC_BIT        CB1_INT_BIT
 
-#define VIA2_IRQ_NB             8
-
 #define VIA2_IRQ_SCSI_DATA      BIT(VIA2_IRQ_SCSI_DATA_BIT)
 #define VIA2_IRQ_NUBUS          BIT(VIA2_IRQ_NUBUS_BIT)
 #define VIA2_IRQ_UNUSED         BIT(VIA2_IRQ_SCSI_BIT)
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index be5c90d24d..f38ae2b0f0 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -57,6 +57,8 @@
 #define T2_INT             BIT(T2_INT_BIT)
 #define T1_INT             BIT(T1_INT_BIT)
 
+#define VIA_NUM_INTS       5
+
 /* Bits in ACR */
 #define T1MODE             0xc0    /* Timer 1 mode */
 #define T1MODE_CONT        0x40    /*  continuous interrupts */
-- 
2.20.1

Re: [PATCH v2 04/12] mos6522: switch over to use qdev gpios for IRQs

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> For historical reasons each mos6522 instance implements its own setting and
> update of the IFR flag bits using methods exposed by MOS6522DeviceClass. As
> of today this is no longer required, and it is now possible to implement
> the mos6522 IRQs as standard qdev gpios.
> 
> Switch over to use qdev gpios for the mos6522 device and update all instances
> accordingly.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mac_via.c         | 56 +++++++--------------------------------
>   hw/misc/macio/cuda.c      |  5 ++--
>   hw/misc/macio/pmu.c       |  4 +--
>   hw/misc/mos6522.c         | 15 +++++++++++
>   include/hw/misc/mac_via.h |  5 ----
>   include/hw/misc/mos6522.h |  2 ++
>   6 files changed, 31 insertions(+), 56 deletions(-)
> 

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

[PATCH v2 05/12] mos6522: remove update_irq() and set_sr_int() methods from MOS6522DeviceClass

[Mark Cave-Ayland]

Now that the mos6522 IRQs are managed using standard qdev gpios these methods
are no longer required.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/mos6522.c         | 9 ---------
 include/hw/misc/mos6522.h | 2 --
 2 files changed, 11 deletions(-)

diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 6be6853dc2..4c3147a7d1 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -208,13 +208,6 @@ static void mos6522_timer2(void *opaque)
     mos6522_update_irq(s);
 }
 
-static void mos6522_set_sr_int(MOS6522State *s)
-{
-    trace_mos6522_set_sr_int();
-    s->ifr |= SR_INT;
-    mos6522_update_irq(s);
-}
-
 static uint64_t mos6522_get_counter_value(MOS6522State *s, MOS6522Timer *ti)
 {
     return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time,
@@ -527,10 +520,8 @@ static void mos6522_class_init(ObjectClass *oc, void *data)
     dc->vmsd = &vmstate_mos6522;
     device_class_set_props(dc, mos6522_properties);
     mdc->parent_reset = dc->reset;
-    mdc->set_sr_int = mos6522_set_sr_int;
     mdc->portB_write = mos6522_portB_write;
     mdc->portA_write = mos6522_portA_write;
-    mdc->update_irq = mos6522_update_irq;
     mdc->get_timer1_counter_value = mos6522_get_counter_value;
     mdc->get_timer2_counter_value = mos6522_get_counter_value;
     mdc->get_timer1_load_time = mos6522_get_load_time;
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index f38ae2b0f0..f0a614898e 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -140,10 +140,8 @@ struct MOS6522DeviceClass {
     DeviceClass parent_class;
 
     DeviceReset parent_reset;
-    void (*set_sr_int)(MOS6522State *dev);
     void (*portB_write)(MOS6522State *dev);
     void (*portA_write)(MOS6522State *dev);
-    void (*update_irq)(MOS6522State *dev);
     /* These are used to influence the CUDA MacOS timebase calibration */
     uint64_t (*get_timer1_counter_value)(MOS6522State *dev, MOS6522Timer *ti);
     uint64_t (*get_timer2_counter_value)(MOS6522State *dev, MOS6522Timer *ti);
-- 
2.20.1

Re: [PATCH v2 05/12] mos6522: remove update_irq() and set_sr_int() methods from MOS6522DeviceClass

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> Now that the mos6522 IRQs are managed using standard qdev gpios these methods
> are no longer required.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mos6522.c         | 9 ---------
>   include/hw/misc/mos6522.h | 2 --
>   2 files changed, 11 deletions(-)

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

[PATCH v2 06/12] mos6522: use device_class_set_parent_reset() to propagate reset to parent

[Mark Cave-Ayland]

Switch from using a legacy approach to the more formal approach for propagating
device reset to the parent.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/mac_via.c    | 7 +++++--
 hw/misc/macio/cuda.c | 3 ++-
 hw/misc/macio/pmu.c  | 3 ++-
 hw/misc/mos6522.c    | 1 -
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 80eb433044..3f473c3fcf 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -1076,9 +1076,11 @@ static Property mos6522_q800_via1_properties[] = {
 static void mos6522_q800_via1_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
+    MOS6522DeviceClass *mdc = MOS6522_CLASS(oc);
 
     dc->realize = mos6522_q800_via1_realize;
-    dc->reset = mos6522_q800_via1_reset;
+    device_class_set_parent_reset(dc, mos6522_q800_via1_reset,
+                                  &mdc->parent_reset);
     dc->vmsd = &vmstate_q800_via1;
     device_class_set_props(dc, mos6522_q800_via1_properties);
 }
@@ -1161,7 +1163,8 @@ static void mos6522_q800_via2_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
     MOS6522DeviceClass *mdc = MOS6522_CLASS(oc);
 
-    dc->reset = mos6522_q800_via2_reset;
+    device_class_set_parent_reset(dc, mos6522_q800_via2_reset,
+                                  &mdc->parent_reset);
     dc->vmsd = &vmstate_q800_via2;
     mdc->portB_write = mos6522_q800_via2_portB_write;
 }
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 693fc82e05..1498113cfc 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -606,7 +606,8 @@ static void mos6522_cuda_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
     MOS6522DeviceClass *mdc = MOS6522_CLASS(oc);
 
-    dc->reset = mos6522_cuda_reset;
+    device_class_set_parent_reset(dc, mos6522_cuda_reset,
+                                  &mdc->parent_reset);
     mdc->portB_write = mos6522_cuda_portB_write;
     mdc->get_timer1_counter_value = cuda_get_counter_value;
     mdc->get_timer2_counter_value = cuda_get_counter_value;
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index b210068ab7..5b1ec100e2 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -850,7 +850,8 @@ static void mos6522_pmu_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
     MOS6522DeviceClass *mdc = MOS6522_CLASS(oc);
 
-    dc->reset = mos6522_pmu_reset;
+    device_class_set_parent_reset(dc, mos6522_pmu_reset,
+                                  &mdc->parent_reset);
     mdc->portB_write = mos6522_pmu_portB_write;
     mdc->portA_write = mos6522_pmu_portA_write;
 }
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 4c3147a7d1..093cc83dcf 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -519,7 +519,6 @@ static void mos6522_class_init(ObjectClass *oc, void *data)
     dc->reset = mos6522_reset;
     dc->vmsd = &vmstate_mos6522;
     device_class_set_props(dc, mos6522_properties);
-    mdc->parent_reset = dc->reset;
     mdc->portB_write = mos6522_portB_write;
     mdc->portA_write = mos6522_portA_write;
     mdc->get_timer1_counter_value = mos6522_get_counter_value;
-- 
2.20.1

Re: [PATCH v2 06/12] mos6522: use device_class_set_parent_reset() to propagate reset to parent

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> Switch from using a legacy approach to the more formal approach for propagating
> device reset to the parent.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mac_via.c    | 7 +++++--
>   hw/misc/macio/cuda.c | 3 ++-
>   hw/misc/macio/pmu.c  | 3 ++-
>   hw/misc/mos6522.c    | 1 -
>   4 files changed, 9 insertions(+), 5 deletions(-)
> 

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

[PATCH v2 07/12] mos6522: add register names to register read/write trace events

[Mark Cave-Ayland]

This helps to follow how the guest is programming the mos6522 when debugging.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/mos6522.c    | 10 ++++++++--
 hw/misc/trace-events |  4 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 093cc83dcf..aaae195d63 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -36,6 +36,12 @@
 #include "qemu/module.h"
 #include "trace.h"
 
+
+static const char *mos6522_reg_names[16] = {
+    "ORB", "ORA", "DDRB", "DDRA", "T1CL", "T1CH", "T1LL", "T1LH",
+    "T2CL", "T2CH", "SR", "ACR", "PCR", "IFR", "IER", "ANH"
+};
+
 /* XXX: implement all timer modes */
 
 static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti,
@@ -310,7 +316,7 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
     }
 
     if (addr != VIA_REG_IFR || val != 0) {
-        trace_mos6522_read(addr, val);
+        trace_mos6522_read(addr, mos6522_reg_names[addr], val);
     }
 
     return val;
@@ -321,7 +327,7 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     MOS6522State *s = opaque;
     MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
 
-    trace_mos6522_write(addr, val);
+    trace_mos6522_write(addr, mos6522_reg_names[addr], val);
 
     switch (addr) {
     case VIA_REG_B:
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 1c373dd0a4..c1ea57de31 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -95,8 +95,8 @@ imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" PRIx64 "value 0x%08
 mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d"
 mos6522_get_next_irq_time(uint16_t latch, int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 " delta_next=0x%"PRId64
 mos6522_set_sr_int(void) "set sr_int"
-mos6522_write(uint64_t addr, uint64_t val) "reg=0x%"PRIx64 " val=0x%"PRIx64
-mos6522_read(uint64_t addr, unsigned val) "reg=0x%"PRIx64 " val=0x%x"
+mos6522_write(uint64_t addr, const char *name, uint64_t val) "reg=0x%"PRIx64 " [%s] val=0x%"PRIx64
+mos6522_read(uint64_t addr, const char *name, unsigned val) "reg=0x%"PRIx64 " [%s] val=0x%x"
 
 # npcm7xx_clk.c
 npcm7xx_clk_read(uint64_t offset, uint32_t value) " offset: 0x%04" PRIx64 " value: 0x%08" PRIx32
-- 
2.20.1

Re: [PATCH v2 07/12] mos6522: add register names to register read/write trace events

[Philippe Mathieu-Daudé]

On 24/2/22 12:59, Mark Cave-Ayland wrote:
> This helps to follow how the guest is programming the mos6522 when debugging.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mos6522.c    | 10 ++++++++--
>   hw/misc/trace-events |  4 ++--
>   2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> index 093cc83dcf..aaae195d63 100644
> --- a/hw/misc/mos6522.c
> +++ b/hw/misc/mos6522.c
> @@ -36,6 +36,12 @@
>   #include "qemu/module.h"
>   #include "trace.h"
>   

I'd feel safer adding:

   #define MOS6522_IOSIZE 0x10

> +static const char *mos6522_reg_names[16] = {

Then here:

    ... mos6522_reg_names[MOS6522_IOSIZE] ...

> +    "ORB", "ORA", "DDRB", "DDRA", "T1CL", "T1CH", "T1LL", "T1LH",
> +    "T2CL", "T2CH", "SR", "ACR", "PCR", "IFR", "IER", "ANH"
> +};
> +
>   /* XXX: implement all timer modes */
>   
>   static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti,
> @@ -310,7 +316,7 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
>       }
>   
>       if (addr != VIA_REG_IFR || val != 0) {
> -        trace_mos6522_read(addr, val);
> +        trace_mos6522_read(addr, mos6522_reg_names[addr], val);
>       }

And finally:

-- >8 --
@@ -478,7 +478,8 @@ static void mos6522_init(Object *obj)
      MOS6522State *s = MOS6522(obj);
      int i;

-    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522", 0x10);
+    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522",
+                          MOS6522_IOSIZE);
      sysbus_init_mmio(sbd, &s->mem);
      sysbus_init_irq(sbd, &s->irq);

---

Regardless:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v2 07/12] mos6522: add register names to register read/write trace events

[Mark Cave-Ayland]

On 24/02/2022 14:04, Philippe Mathieu-Daudé wrote:

> On 24/2/22 12:59, Mark Cave-Ayland wrote:
>> This helps to follow how the guest is programming the mos6522 when debugging.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>   hw/misc/mos6522.c    | 10 ++++++++--
>>   hw/misc/trace-events |  4 ++--
>>   2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
>> index 093cc83dcf..aaae195d63 100644
>> --- a/hw/misc/mos6522.c
>> +++ b/hw/misc/mos6522.c
>> @@ -36,6 +36,12 @@
>>   #include "qemu/module.h"
>>   #include "trace.h"
> 
> I'd feel safer adding:
> 
>    #define MOS6522_IOSIZE 0x10
> 
>> +static const char *mos6522_reg_names[16] = {
> 
> Then here:
> 
>     ... mos6522_reg_names[MOS6522_IOSIZE] ...
> 
>> +    "ORB", "ORA", "DDRB", "DDRA", "T1CL", "T1CH", "T1LL", "T1LH",
>> +    "T2CL", "T2CH", "SR", "ACR", "PCR", "IFR", "IER", "ANH"
>> +};
>> +
>>   /* XXX: implement all timer modes */
>>   static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti,
>> @@ -310,7 +316,7 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
>>       }
>>       if (addr != VIA_REG_IFR || val != 0) {
>> -        trace_mos6522_read(addr, val);
>> +        trace_mos6522_read(addr, mos6522_reg_names[addr], val);
>>       }
> 
> And finally:
> 
> -- >8 --
> @@ -478,7 +478,8 @@ static void mos6522_init(Object *obj)
>       MOS6522State *s = MOS6522(obj);
>       int i;
> 
> -    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522", 0x10);
> +    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522",
> +                          MOS6522_IOSIZE);
>       sysbus_init_mmio(sbd, &s->mem);
>       sysbus_init_irq(sbd, &s->irq);
> 
> ---
> 
> Regardless:
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I've done this in v3 but using MOS6522_NUM_REGS rather than MOS6522_IOSIZE since IO 
size != number of registers at a higher level (e.g. where the 6522 registers are 
mapped in CUDA/mac_via with a stride).


ATB,

Mark.

Re: [PATCH v2 07/12] mos6522: add register names to register read/write trace events

[Philippe Mathieu-Daudé]

On 5/3/22 15:17, Mark Cave-Ayland wrote:
> On 24/02/2022 14:04, Philippe Mathieu-Daudé wrote:
> 
>> On 24/2/22 12:59, Mark Cave-Ayland wrote:
>>> This helps to follow how the guest is programming the mos6522 when 
>>> debugging.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>   hw/misc/mos6522.c    | 10 ++++++++--
>>>   hw/misc/trace-events |  4 ++--
>>>   2 files changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
>>> index 093cc83dcf..aaae195d63 100644
>>> --- a/hw/misc/mos6522.c
>>> +++ b/hw/misc/mos6522.c
>>> @@ -36,6 +36,12 @@
>>>   #include "qemu/module.h"
>>>   #include "trace.h"
>>
>> I'd feel safer adding:
>>
>>    #define MOS6522_IOSIZE 0x10
>>
>>> +static const char *mos6522_reg_names[16] = {
>>
>> Then here:
>>
>>     ... mos6522_reg_names[MOS6522_IOSIZE] ...
>>
>>> +    "ORB", "ORA", "DDRB", "DDRA", "T1CL", "T1CH", "T1LL", "T1LH",
>>> +    "T2CL", "T2CH", "SR", "ACR", "PCR", "IFR", "IER", "ANH"
>>> +};
>>> +
>>>   /* XXX: implement all timer modes */
>>>   static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti,
>>> @@ -310,7 +316,7 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, 
>>> unsigned size)
>>>       }
>>>       if (addr != VIA_REG_IFR || val != 0) {
>>> -        trace_mos6522_read(addr, val);
>>> +        trace_mos6522_read(addr, mos6522_reg_names[addr], val);
>>>       }
>>
>> And finally:
>>
>> -- >8 --
>> @@ -478,7 +478,8 @@ static void mos6522_init(Object *obj)
>>       MOS6522State *s = MOS6522(obj);
>>       int i;
>>
>> -    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522", 
>> 0x10);
>> +    memory_region_init_io(&s->mem, obj, &mos6522_ops, s, "mos6522",
>> +                          MOS6522_IOSIZE);
>>       sysbus_init_mmio(sbd, &s->mem);
>>       sysbus_init_irq(sbd, &s->irq);
>>
>> ---
>>
>> Regardless:
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> I've done this in v3 but using MOS6522_NUM_REGS rather than 
> MOS6522_IOSIZE since IO size != number of registers at a higher level 
> (e.g. where the 6522 registers are mapped in CUDA/mac_via with a stride).

OK, thanks. R-b stands for v3.

Re: [PATCH v2 07/12] mos6522: add register names to register read/write trace events

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> This helps to follow how the guest is programming the mos6522 when debugging.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mos6522.c    | 10 ++++++++--
>   hw/misc/trace-events |  4 ++--
>   2 files changed, 10 insertions(+), 4 deletions(-)
> 

With changes proposed by Philippe:

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

[PATCH v2 08/12] mos6522: add "info via" HMP command for debugging

[Mark Cave-Ayland]

This displays detailed information about the device registers and timers to aid
debugging problems with timers and interrupts.

Currently the QAPI generators for HumanReadableText don't work correctly if
used in qapi/target-misc.json when a non-specified target is built, so for
now manually add a hmp_info_via() wrapper until direct support for per-device
HMP/QMP commands is implemented.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hmp-commands-info.hx         |  15 +++++
 hw/misc/mos6522.c            | 103 +++++++++++++++++++++++++++++++++++
 include/hw/misc/mos6522.h    |   2 +
 include/monitor/hmp-target.h |   1 +
 4 files changed, 121 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index e90f20a107..adfa085a9b 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -879,3 +879,18 @@ SRST
   ``info sgx``
     Show intel SGX information.
 ERST
+
+#if defined(TARGET_M68K) || defined(TARGET_PPC)
+    {
+        .name         = "via",
+        .args_type    = "",
+        .params       = "",
+        .help         = "show guest mos6522 VIA devices",
+        .cmd          = hmp_info_via,
+    },
+#endif
+
+SRST
+  ``info via``
+    Show guest mos6522 VIA devices.
+ERST
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index aaae195d63..38b5877ffa 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -30,6 +30,9 @@
 #include "hw/misc/mos6522.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
+#include "qapi/type-helpers.h"
 #include "qemu/timer.h"
 #include "qemu/cutils.h"
 #include "qemu/log.h"
@@ -415,6 +418,106 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     }
 }
 
+static int qmp_x_query_via_foreach(Object *obj, void *opaque)
+{
+    GString *buf = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_MOS6522)) {
+        MOS6522State *s = MOS6522(obj);
+        int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+        uint16_t t1counter = get_counter(s, &s->timers[0]);
+        uint16_t t2counter = get_counter(s, &s->timers[1]);
+
+        g_string_append_printf(buf, "%s:\n", object_get_typename(obj));
+
+        g_string_append_printf(buf, "  Registers:\n");
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[0], s->b);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[1], s->a);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[2], s->dirb);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[3], s->dira);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[4], t1counter & 0xff);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[5], t1counter >> 8);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[6],
+                               s->timers[0].latch & 0xff);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[7],
+                               s->timers[0].latch >> 8);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[8], t2counter & 0xff);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[9], t2counter >> 8);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[10], s->sr);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[11], s->acr);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[12], s->pcr);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[13], s->ifr);
+        g_string_append_printf(buf, "    %-*s:    0x%x\n", 4,
+                               mos6522_reg_names[14], s->ier);
+
+        g_string_append_printf(buf, "  Timers:\n");
+        g_string_append_printf(buf, "    Using current time now(ns)=%"PRId64
+                                    "\n", now);
+        g_string_append_printf(buf, "    T1 freq(hz)=%"PRId64
+                               " mode=%s"
+                               " counter=0x%x"
+                               " latch=0x%x\n"
+                               "       load_time(ns)=%"PRId64
+                               " next_irq_time(ns)=%"PRId64 "\n",
+                               s->timers[0].frequency,
+                               ((s->acr & T1MODE) == T1MODE_CONT) ? "continuous"
+                                                                  : "one-shot",
+                               t1counter,
+                               s->timers[0].latch,
+                               s->timers[0].load_time,
+                               get_next_irq_time(s, &s->timers[0], now));
+        g_string_append_printf(buf, "    T2 freq(hz)=%"PRId64
+                               " mode=%s"
+                               " counter=0x%x"
+                               " latch=0x%x\n"
+                               "       load_time(ns)=%"PRId64
+                               " next_irq_time(ns)=%"PRId64 "\n",
+                               s->timers[1].frequency,
+                               "one-shot",
+                               t2counter,
+                               s->timers[1].latch,
+                               s->timers[1].load_time,
+                               get_next_irq_time(s, &s->timers[1], now));
+    }
+
+    return 0;
+}
+
+static HumanReadableText *qmp_x_query_via(Error **errp)
+{
+    g_autoptr(GString) buf = g_string_new("");
+
+    object_child_foreach_recursive(object_get_root(),
+                                   qmp_x_query_via_foreach, buf);
+
+    return human_readable_text_from_str(buf);
+}
+
+void hmp_info_via(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    g_autoptr(HumanReadableText) info = qmp_x_query_via(&err);
+
+    if (hmp_handle_error(mon, err)) {
+        return;
+    }
+    monitor_printf(mon, "%s", info->human_readable_text);
+}
+
 static const MemoryRegionOps mos6522_ops = {
     .read = mos6522_read,
     .write = mos6522_write,
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index f0a614898e..846ded64b7 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -155,4 +155,6 @@ extern const VMStateDescription vmstate_mos6522;
 uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size);
 void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size);
 
+void hmp_info_via(Monitor *mon, const QDict *qdict);
+
 #endif /* MOS6522_H */
diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index ffdc15a34b..1891a19b21 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -50,5 +50,6 @@ void hmp_mce(Monitor *mon, const QDict *qdict);
 void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_sgx(Monitor *mon, const QDict *qdict);
+void hmp_info_via(Monitor *mon, const QDict *qdict);
 
 #endif /* MONITOR_HMP_TARGET_H */
-- 
2.20.1

Re: [PATCH v2 08/12] mos6522: add "info via" HMP command for debugging

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> This displays detailed information about the device registers and timers to aid
> debugging problems with timers and interrupts.
> 
> Currently the QAPI generators for HumanReadableText don't work correctly if
> used in qapi/target-misc.json when a non-specified target is built, so for
> now manually add a hmp_info_via() wrapper until direct support for per-device
> HMP/QMP commands is implemented.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hmp-commands-info.hx         |  15 +++++
>   hw/misc/mos6522.c            | 103 +++++++++++++++++++++++++++++++++++
>   include/hw/misc/mos6522.h    |   2 +
>   include/monitor/hmp-target.h |   1 +
>   4 files changed, 121 insertions(+)
> 

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

[PATCH v2 09/12] mos6522: record last_irq_levels in mos6522_set_irq()

[Mark Cave-Ayland]

To detect edge-triggered IRQs it is necessary to store the last state of each
IRQ in a last_irq_levels bitmap.

Note: this is a migration break for machines which use mos6522 instances which
are g3beige/mac99 (PPC) and q800 (m68k).

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/mos6522.c         | 11 +++++++++--
 include/hw/misc/mos6522.h |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 38b5877ffa..60b3b693d0 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -72,6 +72,12 @@ static void mos6522_set_irq(void *opaque, int n, int level)
     }
 
     mos6522_update_irq(s);
+
+    if (level) {
+        s->last_irq_levels |= 1 << n;
+    } else {
+        s->last_irq_levels &= ~(1 << n);
+    }
 }
 
 static uint64_t get_counter_value(MOS6522State *s, MOS6522Timer *ti)
@@ -544,8 +550,8 @@ static const VMStateDescription vmstate_mos6522_timer = {
 
 const VMStateDescription vmstate_mos6522 = {
     .name = "mos6522",
-    .version_id = 0,
-    .minimum_version_id = 0,
+    .version_id = 1,
+    .minimum_version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(a, MOS6522State),
         VMSTATE_UINT8(b, MOS6522State),
@@ -556,6 +562,7 @@ const VMStateDescription vmstate_mos6522 = {
         VMSTATE_UINT8(pcr, MOS6522State),
         VMSTATE_UINT8(ifr, MOS6522State),
         VMSTATE_UINT8(ier, MOS6522State),
+        VMSTATE_UINT8(last_irq_levels, MOS6522State),
         VMSTATE_STRUCT_ARRAY(timers, MOS6522State, 2, 0,
                              vmstate_mos6522_timer, MOS6522Timer),
         VMSTATE_END_OF_LIST()
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index 846ded64b7..9ec9b57239 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -131,6 +131,7 @@ struct MOS6522State {
     uint64_t frequency;
 
     qemu_irq irq;
+    uint8_t last_irq_levels;
 };
 
 #define TYPE_MOS6522 "mos6522"
-- 
2.20.1

Re: [PATCH v2 09/12] mos6522: record last_irq_levels in mos6522_set_irq()

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> To detect edge-triggered IRQs it is necessary to store the last state of each
> IRQ in a last_irq_levels bitmap.
> 
> Note: this is a migration break for machines which use mos6522 instances which
> are g3beige/mac99 (PPC) and q800 (m68k).
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/misc/mos6522.c         | 11 +++++++++--
>   include/hw/misc/mos6522.h |  1 +
>   2 files changed, 10 insertions(+), 2 deletions(-)
> 

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

[PATCH v2 10/12] mac_via: make SCSI_DATA (DRQ) bit live rather than latched

[Mark Cave-Ayland]

The VIA2 on the Q800 machine is not a separate chip as in older Macs but instead
is integrated into the on-board logic. From analysing the SCSI routines in the
MacOS toolbox ROM (and to a lesser extent NetBSD and Linux) the expectation seems
to be that the SCSI_DATA (DRQ) bit is live on the Q800 and not latched.

Fortunately we can use the recently introduced mos6522 last_irq_levels variable
which tracks the edge-triggered state to return the SCSI_DATA (DRQ) bit live to
the guest OS.

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

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 3f473c3fcf..d8b35e6ca6 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -906,9 +906,24 @@ static uint64_t mos6522_q800_via2_read(void *opaque, hwaddr addr, unsigned size)
 {
     MOS6522Q800VIA2State *s = MOS6522_Q800_VIA2(opaque);
     MOS6522State *ms = MOS6522(s);
+    uint64_t val;
 
     addr = (addr >> 9) & 0xf;
-    return mos6522_read(ms, addr, size);
+    val = mos6522_read(ms, addr, size);
+
+    switch (addr) {
+    case VIA_REG_IFR:
+        /*
+         * On a Q800 an emulated VIA2 is integrated into the onboard logic. The
+         * expectation of most OSs is that the DRQ bit is live, rather than
+         * latched as it would be on a real VIA so do the same here.
+         */
+        val &= ~VIA2_IRQ_SCSI_DATA;
+        val |= (ms->last_irq_levels & VIA2_IRQ_SCSI_DATA);
+        break;
+    }
+
+    return val;
 }
 
 static void mos6522_q800_via2_write(void *opaque, hwaddr addr, uint64_t val,
-- 
2.20.1

Re: [PATCH v2 10/12] mac_via: make SCSI_DATA (DRQ) bit live rather than latched

[Philippe Mathieu-Daudé]

On 24/2/22 12:59, Mark Cave-Ayland wrote:
> The VIA2 on the Q800 machine is not a separate chip as in older Macs but instead
> is integrated into the on-board logic. From analysing the SCSI routines in the
> MacOS toolbox ROM (and to a lesser extent NetBSD and Linux) the expectation seems
> to be that the SCSI_DATA (DRQ) bit is live on the Q800 and not latched.
> 
> Fortunately we can use the recently introduced mos6522 last_irq_levels variable
> which tracks the edge-triggered state to return the SCSI_DATA (DRQ) bit live to
> the guest OS.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/misc/mac_via.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v2 10/12] mac_via: make SCSI_DATA (DRQ) bit live rather than latched

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> The VIA2 on the Q800 machine is not a separate chip as in older Macs but instead
> is integrated into the on-board logic. From analysing the SCSI routines in the
> MacOS toolbox ROM (and to a lesser extent NetBSD and Linux) the expectation seems
> to be that the SCSI_DATA (DRQ) bit is live on the Q800 and not latched.
> 
> Fortunately we can use the recently introduced mos6522 last_irq_levels variable
> which tracks the edge-triggered state to return the SCSI_DATA (DRQ) bit live to
> the guest OS.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/misc/mac_via.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)

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

[PATCH v2 11/12] mos6522: implement edge-triggering for CA1/2 and CB1/2 control line IRQs

[Mark Cave-Ayland]

The mos6522 datasheet describes how the control lines IRQs are edge-triggered
according to the configuration in the PCR register. Implement the logic according
to the datasheet so that the interrupt bits in IFR are latched when the edge is
detected, and cleared when reading portA/portB or writing to IFR as necessary.

To maintain bisectibility this change also updates the SCSI, SCSI data, Nubus
and VIA2 60Hz/1Hz clocks in the q800 machine to be negative edge-triggered as
confirmed by the PCR programming in all of Linux, NetBSD and MacOS.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800.c            |  9 +++--
 hw/misc/mac_via.c         | 15 +++++--
 hw/misc/mos6522.c         | 82 +++++++++++++++++++++++++++++++++++++--
 include/hw/misc/mos6522.h | 15 +++++++
 4 files changed, 109 insertions(+), 12 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 55dfe5036f..66ca5c0df6 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -533,10 +533,11 @@ static void q800_init(MachineState *machine)
 
     sysbus = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
-    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(via2_dev,
-                                                   VIA2_IRQ_SCSI_BIT));
-    sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(via2_dev,
-                                                   VIA2_IRQ_SCSI_DATA_BIT));
+    /* SCSI and SCSI data IRQs are negative edge triggered */
+    sysbus_connect_irq(sysbus, 0, qemu_irq_invert(qdev_get_gpio_in(via2_dev,
+                                                  VIA2_IRQ_SCSI_BIT)));
+    sysbus_connect_irq(sysbus, 1, qemu_irq_invert(qdev_get_gpio_in(via2_dev,
+                                                  VIA2_IRQ_SCSI_DATA_BIT)));
     sysbus_mmio_map(sysbus, 0, ESP_BASE);
     sysbus_mmio_map(sysbus, 1, ESP_PDMA);
 
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index d8b35e6ca6..525e38ce93 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -327,7 +327,9 @@ static void via1_sixty_hz(void *opaque)
     MOS6522State *s = MOS6522(v1s);
     qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA1_IRQ_60HZ_BIT);
 
-    qemu_set_irq(irq, 1);
+    /* Negative edge trigger */
+    qemu_irq_lower(irq);
+    qemu_irq_raise(irq);
 
     via1_sixty_hz_update(v1s);
 }
@@ -338,7 +340,9 @@ static void via1_one_second(void *opaque)
     MOS6522State *s = MOS6522(v1s);
     qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA1_IRQ_ONE_SECOND_BIT);
 
-    qemu_set_irq(irq, 1);
+    /* Negative edge trigger */
+    qemu_irq_lower(irq);
+    qemu_irq_raise(irq);
 
     via1_one_second_update(v1s);
 }
@@ -917,9 +921,11 @@ static uint64_t mos6522_q800_via2_read(void *opaque, hwaddr addr, unsigned size)
          * On a Q800 an emulated VIA2 is integrated into the onboard logic. The
          * expectation of most OSs is that the DRQ bit is live, rather than
          * latched as it would be on a real VIA so do the same here.
+         *
+         * Note: DRQ is negative edge triggered
          */
         val &= ~VIA2_IRQ_SCSI_DATA;
-        val |= (ms->last_irq_levels & VIA2_IRQ_SCSI_DATA);
+        val |= (~ms->last_irq_levels & VIA2_IRQ_SCSI_DATA);
         break;
     }
 
@@ -1146,7 +1152,8 @@ static void via2_nubus_irq_request(void *opaque, int n, int level)
         s->a |= (1 << n);
     }
 
-    qemu_set_irq(irq, level);
+    /* Negative edge trigger */
+    qemu_set_irq(irq, !level);
 }
 
 static void mos6522_q800_via2_init(Object *obj)
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 60b3b693d0..2af32b68ef 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -64,14 +64,62 @@ static void mos6522_update_irq(MOS6522State *s)
 static void mos6522_set_irq(void *opaque, int n, int level)
 {
     MOS6522State *s = MOS6522(opaque);
+    int last_level = !!(s->last_irq_levels & (1 << n));
+    uint8_t last_ifr = s->ifr;
+    bool positive_edge = true;
+    int ctrl;
+
+    /*
+     * SR_INT is managed by mos6522 instances and cleared upon SR
+     * read. It is only the external CA1/2 and CB1/2 lines that
+     * are edge-triggered and latched in IFR
+     */
+    if (n != SR_INT_BIT && level == last_level) {
+        return;
+    }
 
-    if (level) {
+    /* Detect negative edge trigger */
+    if (last_level == 1 && level == 0) {
+        positive_edge = false;
+    }
+
+    switch (n) {
+    case CA2_INT_BIT:
+        ctrl = (s->pcr & CA2_CTRL_MASK) >> CA2_CTRL_SHIFT;
+        if ((positive_edge && (ctrl & C2_POS)) ||
+             (!positive_edge && !(ctrl & C2_POS))) {
+            s->ifr |= 1 << n;
+        }
+        break;
+    case CA1_INT_BIT:
+        ctrl = (s->pcr & CA1_CTRL_MASK) >> CA1_CTRL_SHIFT;
+        if ((positive_edge && (ctrl & C1_POS)) ||
+             (!positive_edge && !(ctrl & C1_POS))) {
+            s->ifr |= 1 << n;
+        }
+        break;
+    case SR_INT_BIT:
         s->ifr |= 1 << n;
-    } else {
-        s->ifr &= ~(1 << n);
+        break;
+    case CB2_INT_BIT:
+        ctrl = (s->pcr & CB2_CTRL_MASK) >> CB2_CTRL_SHIFT;
+        if ((positive_edge && (ctrl & C2_POS)) ||
+             (!positive_edge && !(ctrl & C2_POS))) {
+            s->ifr |= 1 << n;
+        }
+        break;
+    case CB1_INT_BIT:
+        ctrl = (s->pcr & CB1_CTRL_MASK) >> CB1_CTRL_SHIFT;
+        if ((positive_edge && (ctrl & C1_POS)) ||
+             (!positive_edge && !(ctrl & C1_POS))) {
+            s->ifr |= 1 << n;
+        }
+        break;
     }
 
-    mos6522_update_irq(s);
+    if (s->ifr != last_ifr) {
+        mos6522_update_irq(s);
+    }
 
     if (level) {
         s->last_irq_levels |= 1 << n;
@@ -250,6 +298,7 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
 {
     MOS6522State *s = opaque;
     uint32_t val;
+    int ctrl;
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
     if (now >= s->timers[0].next_irq_time) {
@@ -263,12 +312,24 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
     switch (addr) {
     case VIA_REG_B:
         val = s->b;
+        ctrl = (s->pcr & CB2_CTRL_MASK) >> CB2_CTRL_SHIFT;
+        if (!(ctrl & C2_IND)) {
+            s->ifr &= ~CB2_INT;
+        }
+        s->ifr &= ~CB1_INT;
+        mos6522_update_irq(s);
         break;
     case VIA_REG_A:
        qemu_log_mask(LOG_UNIMP, "Read access to register A with handshake");
        /* fall through */
     case VIA_REG_ANH:
         val = s->a;
+        ctrl = (s->pcr & CA2_CTRL_MASK) >> CA2_CTRL_SHIFT;
+        if (!(ctrl & C2_IND)) {
+            s->ifr &= ~CA2_INT;
+        }
+        s->ifr &= ~CA1_INT;
+        mos6522_update_irq(s);
         break;
     case VIA_REG_DIRB:
         val = s->dirb;
@@ -335,6 +396,7 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
 {
     MOS6522State *s = opaque;
     MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
+    int ctrl;
 
     trace_mos6522_write(addr, mos6522_reg_names[addr], val);
 
@@ -342,6 +404,12 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     case VIA_REG_B:
         s->b = (s->b & ~s->dirb) | (val & s->dirb);
         mdc->portB_write(s);
+        ctrl = (s->pcr & CB2_CTRL_MASK) >> CB2_CTRL_SHIFT;
+        if (!(ctrl & C2_IND)) {
+            s->ifr &= ~CB2_INT;
+        }
+        s->ifr &= ~CB1_INT;
+        mos6522_update_irq(s);
         break;
     case VIA_REG_A:
        qemu_log_mask(LOG_UNIMP, "Write access to register A with handshake");
@@ -349,6 +417,12 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     case VIA_REG_ANH:
         s->a = (s->a & ~s->dira) | (val & s->dira);
         mdc->portA_write(s);
+        ctrl = (s->pcr & CA2_CTRL_MASK) >> CA2_CTRL_SHIFT;
+        if (!(ctrl & C2_IND)) {
+            s->ifr &= ~CA2_INT;
+        }
+        s->ifr &= ~CA1_INT;
+        mos6522_update_irq(s);
         break;
     case VIA_REG_DIRB:
         s->dirb = val;
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index 9ec9b57239..0d4724777b 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -63,6 +63,21 @@
 #define T1MODE             0xc0    /* Timer 1 mode */
 #define T1MODE_CONT        0x40    /*  continuous interrupts */
 
+/* Bits in PCR */
+#define CB2_CTRL_MASK      0xe0
+#define CB2_CTRL_SHIFT     5
+#define CB1_CTRL_MASK      0x10
+#define CB1_CTRL_SHIFT     4
+#define CA2_CTRL_MASK      0x0e
+#define CA2_CTRL_SHIFT     1
+#define CA1_CTRL_MASK      0x1
+#define CA1_CTRL_SHIFT     0
+
+#define C2_POS             0x2
+#define C2_IND             0x1
+
+#define C1_POS             0x1
+
 /* VIA registers */
 #define VIA_REG_B       0x00
 #define VIA_REG_A       0x01
-- 
2.20.1

Re: [PATCH v2 11/12] mos6522: implement edge-triggering for CA1/2 and CB1/2 control line IRQs

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> The mos6522 datasheet describes how the control lines IRQs are edge-triggered
> according to the configuration in the PCR register. Implement the logic according
> to the datasheet so that the interrupt bits in IFR are latched when the edge is
> detected, and cleared when reading portA/portB or writing to IFR as necessary.
> 
> To maintain bisectibility this change also updates the SCSI, SCSI data, Nubus
> and VIA2 60Hz/1Hz clocks in the q800 machine to be negative edge-triggered as
> confirmed by the PCR programming in all of Linux, NetBSD and MacOS.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c            |  9 +++--
>   hw/misc/mac_via.c         | 15 +++++--
>   hw/misc/mos6522.c         | 82 +++++++++++++++++++++++++++++++++++++--
>   include/hw/misc/mos6522.h | 15 +++++++
>   4 files changed, 109 insertions(+), 12 deletions(-)
> 

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

[PATCH v2 12/12] macio/pmu.c: remove redundant code

[Mark Cave-Ayland]

Now that the logic related to edge-triggered interrupts is all contained within
the mos6522 device the redundant implementation for the mac99 PMU device can
be removed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/pmu.c         | 33 ---------------------------------
 include/hw/misc/macio/pmu.h |  2 --
 2 files changed, 35 deletions(-)

diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 5b1ec100e2..336502a84b 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -57,19 +57,6 @@
 
 #define VIA_TIMER_FREQ (4700000 / 6)
 
-static void via_update_irq(PMUState *s)
-{
-    MOS6522PMUState *mps = MOS6522_PMU(&s->mos6522_pmu);
-    MOS6522State *ms = MOS6522(mps);
-
-    bool new_state = !!(ms->ifr & ms->ier & (SR_INT | T1_INT | T2_INT));
-
-    if (new_state != s->via_irq_state) {
-        s->via_irq_state = new_state;
-        qemu_set_irq(s->via_irq, new_state);
-    }
-}
-
 static void via_set_sr_int(void *opaque)
 {
     PMUState *s = opaque;
@@ -808,28 +795,9 @@ static void mos6522_pmu_portB_write(MOS6522State *s)
     MOS6522PMUState *mps = container_of(s, MOS6522PMUState, parent_obj);
     PMUState *ps = container_of(mps, PMUState, mos6522_pmu);
 
-    if ((s->pcr & 0xe0) == 0x20 || (s->pcr & 0xe0) == 0x60) {
-        s->ifr &= ~CB2_INT;
-    }
-    s->ifr &= ~CB1_INT;
-
-    via_update_irq(ps);
     pmu_update(ps);
 }
 
-static void mos6522_pmu_portA_write(MOS6522State *s)
-{
-    MOS6522PMUState *mps = container_of(s, MOS6522PMUState, parent_obj);
-    PMUState *ps = container_of(mps, PMUState, mos6522_pmu);
-
-    if ((s->pcr & 0x0e) == 0x02 || (s->pcr & 0x0e) == 0x06) {
-        s->ifr &= ~CA2_INT;
-    }
-    s->ifr &= ~CA1_INT;
-
-    via_update_irq(ps);
-}
-
 static void mos6522_pmu_reset(DeviceState *dev)
 {
     MOS6522State *ms = MOS6522(dev);
@@ -853,7 +821,6 @@ static void mos6522_pmu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_reset(dc, mos6522_pmu_reset,
                                   &mdc->parent_reset);
     mdc->portB_write = mos6522_pmu_portB_write;
-    mdc->portA_write = mos6522_pmu_portA_write;
 }
 
 static const TypeInfo mos6522_pmu_type_info = {
diff --git a/include/hw/misc/macio/pmu.h b/include/hw/misc/macio/pmu.h
index 78237d99a2..00fcdd23f5 100644
--- a/include/hw/misc/macio/pmu.h
+++ b/include/hw/misc/macio/pmu.h
@@ -193,8 +193,6 @@ struct PMUState {
 
     MemoryRegion mem;
     uint64_t frequency;
-    qemu_irq via_irq;
-    bool via_irq_state;
 
     /* PMU state */
     MOS6522PMUState mos6522_pmu;
-- 
2.20.1

Re: [PATCH v2 12/12] macio/pmu.c: remove redundant code

[Laurent Vivier]

Le 24/02/2022 à 12:59, Mark Cave-Ayland a écrit :
> Now that the logic related to edge-triggered interrupts is all contained within
> the mos6522 device the redundant implementation for the mac99 PMU device can
> be removed.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/misc/macio/pmu.c         | 33 ---------------------------------
>   include/hw/misc/macio/pmu.h |  2 --
>   2 files changed, 35 deletions(-)
> 

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