[PATCH 0/3] q800: update machine emulation

[PATCH 0/3] q800: update machine emulation

[Laurent Vivier]

On the way to run a MacOS ROM we need a more accurate
emulation of the Quadra 800.

This series fixes the ESCC base address that was wrong but
as the linux kernel uses the one provided by the bootloader
(in our case QEMU) it was not a problem. This value
is hardcoded in the ROM, so QEMU must use the good one.

The two other patches update the description of the machine
by introducing the djMEMC memory controller and the machine id
register.

Laurent Vivier (3):
  q800: fix ESCC base
  q800: add djMEMC memory controller
  q800: add machine id register

 MAINTAINERS              |   2 +
 hw/m68k/Kconfig          |   1 +
 hw/m68k/q800.c           |  85 +++++++++----------
 hw/misc/Kconfig          |   3 +
 hw/misc/Makefile.objs    |   1 +
 hw/misc/djmemc.c         | 176 +++++++++++++++++++++++++++++++++++++++
 hw/misc/trace-events     |   4 +
 include/hw/misc/djmemc.h |  34 ++++++++
 8 files changed, 260 insertions(+), 46 deletions(-)
 create mode 100644 hw/misc/djmemc.c
 create mode 100644 include/hw/misc/djmemc.h

-- 
2.23.0

[PATCH 1/3] q800: fix ESCC base

[Laurent Vivier]

0xc020 is for Q900/Q950, Q800 uses 0xc000.
This value was provided to the kernel, this explains why it was working
even with wrong value

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 4ca8678007..ef0014f4c4 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -67,7 +67,7 @@
 #define VIA_BASE              (IO_BASE + 0x00000)
 #define SONIC_PROM_BASE       (IO_BASE + 0x08000)
 #define SONIC_BASE            (IO_BASE + 0x0a000)
-#define SCC_BASE              (IO_BASE + 0x0c020)
+#define SCC_BASE              (IO_BASE + 0x0c000)
 #define ESP_BASE              (IO_BASE + 0x10000)
 #define ESP_PDMA              (IO_BASE + 0x10100)
 #define ASC_BASE              (IO_BASE + 0x14000)
-- 
2.23.0

Re: [PATCH 1/3] q800: fix ESCC base

[Mark Cave-Ayland]

On 12/12/2019 20:01, Laurent Vivier wrote:

> 0xc020 is for Q900/Q950, Q800 uses 0xc000.
> This value was provided to the kernel, this explains why it was working
> even with wrong value
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/m68k/q800.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 4ca8678007..ef0014f4c4 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -67,7 +67,7 @@
>  #define VIA_BASE              (IO_BASE + 0x00000)
>  #define SONIC_PROM_BASE       (IO_BASE + 0x08000)
>  #define SONIC_BASE            (IO_BASE + 0x0a000)
> -#define SCC_BASE              (IO_BASE + 0x0c020)
> +#define SCC_BASE              (IO_BASE + 0x0c000)
>  #define ESP_BASE              (IO_BASE + 0x10000)
>  #define ESP_PDMA              (IO_BASE + 0x10100)
>  #define ASC_BASE              (IO_BASE + 0x14000)

I *think* this is correct (see the off-list message I've sent you) since if you
assume a CHRP-like mapping then even with a base of 0xc000 then an access to 0xc020
could potentially still be hitting the ESCC registers, but I'm not confident enough
to give it an R-B tag :/


ATB,

Mark.

[PATCH 2/3] q800: add djMEMC memory controller

[Laurent Vivier]

Current implementation is based on GLUE, an early implementation
of the memory controller found in Macintosh II series.

Quadra 800 uses in fact djMEMC:

The djMEMC is an Apple custom integrated circuit chip that performs a
variety of functions (RAM management, clock generation, ...).
It receives interrupt requests from various devices, assign priority to
each, and asserts one or more interrupt line to the CPU.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 MAINTAINERS              |   2 +
 hw/m68k/Kconfig          |   1 +
 hw/m68k/q800.c           |  61 ++++----------
 hw/misc/Kconfig          |   3 +
 hw/misc/Makefile.objs    |   1 +
 hw/misc/djmemc.c         | 176 +++++++++++++++++++++++++++++++++++++++
 hw/misc/trace-events     |   4 +
 include/hw/misc/djmemc.h |  34 ++++++++
 8 files changed, 237 insertions(+), 45 deletions(-)
 create mode 100644 hw/misc/djmemc.c
 create mode 100644 include/hw/misc/djmemc.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 5e5e3e52d6..07224a2fa2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -925,11 +925,13 @@ F: hw/misc/mac_via.c
 F: hw/nubus/*
 F: hw/display/macfb.c
 F: hw/block/swim.c
+F: hw/misc/djmemc.c
 F: hw/m68k/bootinfo.h
 F: include/hw/misc/mac_via.h
 F: include/hw/nubus/*
 F: include/hw/display/macfb.h
 F: include/hw/block/swim.h
+F: include/hw/misc/djmemc.c
 
 MicroBlaze Machines
 -------------------
diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
index c757e7dfa4..bdc43a798a 100644
--- a/hw/m68k/Kconfig
+++ b/hw/m68k/Kconfig
@@ -22,3 +22,4 @@ config Q800
     select ESCC
     select ESP
     select DP8393X
+    select DJMEMC
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index ef0014f4c4..9ee0cb1141 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -46,6 +46,7 @@
 #include "sysemu/qtest.h"
 #include "sysemu/runstate.h"
 #include "sysemu/reset.h"
+#include "hw/misc/djmemc.h"
 
 #define MACROM_ADDR     0x40000000
 #define MACROM_SIZE     0x00100000
@@ -68,6 +69,7 @@
 #define SONIC_PROM_BASE       (IO_BASE + 0x08000)
 #define SONIC_BASE            (IO_BASE + 0x0a000)
 #define SCC_BASE              (IO_BASE + 0x0c000)
+#define DJMEMC_BASE           (IO_BASE + 0x0e000)
 #define ESP_BASE              (IO_BASE + 0x10000)
 #define ESP_PDMA              (IO_BASE + 0x10100)
 #define ASC_BASE              (IO_BASE + 0x14000)
@@ -85,39 +87,6 @@
 
 #define MAC_CLOCK  3686418
 
-/*
- * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
- * that performs a variety of functions (RAM management, clock generation, ...).
- * The GLUE chip receives interrupt requests from various devices,
- * assign priority to each, and asserts one or more interrupt line to the
- * CPU.
- */
-
-typedef struct {
-    M68kCPU *cpu;
-    uint8_t ipr;
-} GLUEState;
-
-static void GLUE_set_irq(void *opaque, int irq, int level)
-{
-    GLUEState *s = opaque;
-    int i;
-
-    if (level) {
-        s->ipr |= 1 << irq;
-    } else {
-        s->ipr &= ~(1 << irq);
-    }
-
-    for (i = 7; i >= 0; i--) {
-        if ((s->ipr >> i) & 1) {
-            m68k_set_irq_level(s->cpu, i + 1, i + 25);
-            return;
-        }
-    }
-    m68k_set_irq_level(s->cpu, 0, 0);
-}
-
 static void main_cpu_reset(void *opaque)
 {
     M68kCPU *cpu = opaque;
@@ -149,6 +118,7 @@ static void q800_init(MachineState *machine)
     const char *kernel_cmdline = machine->kernel_cmdline;
     hwaddr parameters_base;
     CPUState *cs;
+    DeviceState *djmemc_dev;
     DeviceState *dev;
     DeviceState *via_dev;
     SysBusESPState *sysbus_esp;
@@ -156,8 +126,6 @@ static void q800_init(MachineState *machine)
     SysBusDevice *sysbus;
     BusState *adb_bus;
     NubusBus *nubus;
-    GLUEState *irq;
-    qemu_irq *pic;
 
     linux_boot = (kernel_filename != NULL);
 
@@ -191,11 +159,13 @@ static void q800_init(MachineState *machine)
         g_free(name);
     }
 
-    /* IRQ Glue */
+    /* djMEMC memory and interrupt controller */
 
-    irq = g_new0(GLUEState, 1);
-    irq->cpu = cpu;
-    pic = qemu_allocate_irqs(GLUE_set_irq, irq, 8);
+    djmemc_dev = qdev_create(NULL, TYPE_DJMEMC);
+    object_property_set_link(OBJECT(djmemc_dev), OBJECT(cpu), "cpu",
+                             &error_abort);
+    qdev_init_nofail(djmemc_dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(djmemc_dev), 0, DJMEMC_BASE);
 
     /* VIA */
 
@@ -203,9 +173,10 @@ static void q800_init(MachineState *machine)
     qdev_init_nofail(via_dev);
     sysbus = SYS_BUS_DEVICE(via_dev);
     sysbus_mmio_map(sysbus, 0, VIA_BASE);
-    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0, pic[0]);
-    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1, pic[1]);
-
+    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0,
+                                qdev_get_gpio_in(djmemc_dev, 0));
+    qdev_connect_gpio_out_named(DEVICE(sysbus),
+                                "irq", 1, qdev_get_gpio_in(djmemc_dev, 1));
 
     adb_bus = qdev_get_child_bus(via_dev, "adb.0");
     dev = qdev_create(adb_bus, TYPE_ADB_KEYBOARD);
@@ -244,7 +215,7 @@ static void q800_init(MachineState *machine)
     sysbus = SYS_BUS_DEVICE(dev);
     sysbus_mmio_map(sysbus, 0, SONIC_BASE);
     sysbus_mmio_map(sysbus, 1, SONIC_PROM_BASE);
-    sysbus_connect_irq(sysbus, 0, pic[2]);
+    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(djmemc_dev, 2));
 
     /* SCC */
 
@@ -259,8 +230,8 @@ static void q800_init(MachineState *machine)
     qdev_prop_set_uint32(dev, "chnAtype", 0);
     qdev_init_nofail(dev);
     sysbus = SYS_BUS_DEVICE(dev);
-    sysbus_connect_irq(sysbus, 0, pic[3]);
-    sysbus_connect_irq(sysbus, 1, pic[3]);
+    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(djmemc_dev, 3));
+    sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(djmemc_dev, 3));
     sysbus_mmio_map(sysbus, 0, SCC_BASE);
 
     /* SCSI */
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 2164646553..4c68d20c18 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -125,4 +125,7 @@ config MAC_VIA
     select MOS6522
     select ADB
 
+config DJMEMC
+    bool
+
 source macio/Kconfig
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index ba898a5781..598e46d7db 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -20,6 +20,7 @@ common-obj-$(CONFIG_ARM11SCU) += arm11scu.o
 
 # Mac devices
 common-obj-$(CONFIG_MOS6522) += mos6522.o
+obj-$(CONFIG_DJMEMC) += djmemc.o
 
 # PKUnity SoC devices
 common-obj-$(CONFIG_PUV3) += puv3_pm.o
diff --git a/hw/misc/djmemc.c b/hw/misc/djmemc.c
new file mode 100644
index 0000000000..b494e82a60
--- /dev/null
+++ b/hw/misc/djmemc.c
@@ -0,0 +1,176 @@
+/*
+ * djMEMC, macintosh memory and interrupt controller
+ * (Quadra 610/650/800 & Centris 610/650)
+ *
+ *    https://mac68k.info/wiki/display/mac68k/djMEMC+Information
+ *
+ * The djMEMC is an Apple custom integrated circuit chip that performs a
+ * variety of functions (RAM management, clock generation, ...).
+ * It receives interrupt requests from various devices, assign priority to
+ * each, and asserts one or more interrupt line to the CPU.
+ */
+
+#include "qemu/osdep.h"
+#include "migration/vmstate.h"
+#include "hw/misc/djmemc.h"
+#include "hw/qdev-properties.h"
+#include "trace.h"
+
+#define DJMEMC_SIZE       0x2000
+
+#define InterleaveConf    0
+#define Bank0Conf         1
+#define Bank1Conf         2
+#define Bank2Conf         3
+#define Bank3Conf         4
+#define Bank4Conf         5
+#define Bank5Conf         6
+#define Bank6Conf         7
+#define Bank7Conf         8
+#define Bank8Conf         9
+#define Bank9Conf         10
+#define MemTop            11
+#define Config            12
+#define Refresh           13
+
+static const VMStateDescription vmstate_djMEMC = {
+    .name = "djMEMC",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(interleave, DjMEMCState),
+        VMSTATE_UINT32_ARRAY(bank, DjMEMCState, DjMEMCMaxBanks),
+        VMSTATE_UINT32(top, DjMEMCState),
+        VMSTATE_UINT32(config, DjMEMCState),
+        VMSTATE_UINT32(refresh_rate, DjMEMCState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static uint64_t djMEMC_read(void *opaque, hwaddr addr,
+                              unsigned size)
+{
+    DjMEMCState *s = opaque;
+    uint64_t value = 0;
+
+    switch (addr >> 2) {
+    case InterleaveConf:
+        value = s->interleave;
+    case Bank0Conf...Bank9Conf:
+        value = s->bank[(addr >> 2) - Bank0Conf];
+    case MemTop:
+        value = s->top;
+    case Config:
+        value = s->config;
+    case Refresh:
+        value = s->refresh_rate;
+    }
+    trace_djMEMC_read((int)(addr >> 2), size, value);
+
+    return value;
+}
+
+static void djMEMC_write(void *opaque, hwaddr addr, uint64_t value,
+                         unsigned size)
+{
+    DjMEMCState *s = opaque;
+    trace_djMEMC_write((int)(addr >> 2), size, value);
+
+    switch (addr >> 2) {
+    case InterleaveConf:
+        s->interleave = value;
+    case Bank0Conf...Bank9Conf:
+        s->bank[(addr >> 2) - Bank0Conf] = value;
+    case MemTop:
+        s->top = value;
+    case Config:
+        s->config = value;
+    case Refresh:
+        s->refresh_rate = value;
+    }
+}
+
+static const MemoryRegionOps djMEMC_mmio_ops = {
+    .read = djMEMC_read,
+    .write = djMEMC_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void djMEMC_set_irq(void *opaque, int irq, int level)
+{
+    DjMEMCState *s = opaque;
+    int i;
+
+
+    if (level) {
+        s->ipr |= 1 << irq;
+    } else {
+        s->ipr &= ~(1 << irq);
+    }
+
+    for (i = 7; i >= 0; i--) {
+        if ((s->ipr >> i) & 1) {
+            m68k_set_irq_level(s->cpu, i + 1, i + 25);
+            return;
+        }
+    }
+    m68k_set_irq_level(s->cpu, 0, 0);
+}
+
+static void djMEMC_init(Object *obj)
+{
+    DjMEMCState *s = DJMEMC(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+    memory_region_init_io(&s->mem_regs, NULL, &djMEMC_mmio_ops, s, "djMEMC",
+                          DJMEMC_SIZE);
+    sysbus_init_mmio(sbd, &s->mem_regs);
+
+    qdev_init_gpio_in(DEVICE(obj), djMEMC_set_irq, 8);
+    object_property_add_link(obj, "cpu", TYPE_M68K_CPU,
+                             (Object **) &s->cpu,
+                             qdev_prop_allow_set_link_before_realize,
+                             0, NULL);
+}
+
+static void djMEMC_reset(DeviceState *d)
+{
+    DjMEMCState *s = DJMEMC(d);
+    int i;
+
+    s->interleave = 0;
+    s->top = 0;
+    s->refresh_rate = 0;
+    s->config = 0;
+
+    for (i = 0; i < DjMEMCMaxBanks; i++) {
+        s->bank[i] = 0;
+    }
+}
+
+static void djMEMC_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->reset = djMEMC_reset;
+    dc->vmsd = &vmstate_djMEMC;
+}
+
+static TypeInfo djMEMC_info = {
+    .name          = TYPE_DJMEMC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(DjMEMCState),
+    .instance_init = djMEMC_init,
+    .class_init    = djMEMC_class_init,
+};
+
+static void djMEMC_register_types(void)
+{
+    type_register_static(&djMEMC_info);
+}
+
+type_init(djMEMC_register_types)
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 1deb1d08c1..c9bcdd4a54 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -149,3 +149,7 @@ bcm2835_mbox_write(unsigned int size, uint64_t addr, uint64_t value) "mbox write
 bcm2835_mbox_read(unsigned int size, uint64_t addr, uint64_t value) "mbox read sz:%u addr:0x%"PRIx64" data:0x%"PRIx64
 bcm2835_mbox_irq(unsigned level) "mbox irq:ARM level:%u"
 bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
+
+# djmemc.c
+djMEMC_read(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64
+djMEMC_write(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64
diff --git a/include/hw/misc/djmemc.h b/include/hw/misc/djmemc.h
new file mode 100644
index 0000000000..0f29ac1cf3
--- /dev/null
+++ b/include/hw/misc/djmemc.h
@@ -0,0 +1,34 @@
+/*
+ * djMEMC, macintosh memory and interrupt controller
+ * (Quadra 610/650/800 & Centris 610/650)
+ */
+
+#ifndef HW_MISC_DJMEMC_H
+#define HW_MISC_DJMEMC_H
+
+#include "hw/sysbus.h"
+#include "cpu.h"
+
+#define DjMEMCMaxBanks    10
+
+typedef struct DjMEMCState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mem_regs;
+
+    /* Memory controller */
+    uint32_t interleave;
+    uint32_t bank[DjMEMCMaxBanks];
+    uint32_t top;
+    uint32_t config;
+    uint32_t refresh_rate;
+
+    /* interrupt controller */
+    M68kCPU *cpu;
+    uint8_t ipr;
+} DjMEMCState;
+
+#define TYPE_DJMEMC "djMEMC"
+#define DJMEMC(obj) OBJECT_CHECK(DjMEMCState, (obj), TYPE_DJMEMC)
+
+#endif
-- 
2.23.0

Re: [PATCH 2/3] q800: add djMEMC memory controller

[BALATON Zoltan]

On Thu, 12 Dec 2019, Laurent Vivier wrote:
> Current implementation is based on GLUE, an early implementation
> of the memory controller found in Macintosh II series.
>
> Quadra 800 uses in fact djMEMC:
>
> The djMEMC is an Apple custom integrated circuit chip that performs a
> variety of functions (RAM management, clock generation, ...).
> It receives interrupt requests from various devices, assign priority to
> each, and asserts one or more interrupt line to the CPU.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> MAINTAINERS              |   2 +
> hw/m68k/Kconfig          |   1 +
> hw/m68k/q800.c           |  61 ++++----------
> hw/misc/Kconfig          |   3 +
> hw/misc/Makefile.objs    |   1 +
> hw/misc/djmemc.c         | 176 +++++++++++++++++++++++++++++++++++++++
> hw/misc/trace-events     |   4 +
> include/hw/misc/djmemc.h |  34 ++++++++
> 8 files changed, 237 insertions(+), 45 deletions(-)
> create mode 100644 hw/misc/djmemc.c
> create mode 100644 include/hw/misc/djmemc.h
>
[...]
> diff --git a/hw/misc/djmemc.c b/hw/misc/djmemc.c
> new file mode 100644
> index 0000000000..b494e82a60
> --- /dev/null
> +++ b/hw/misc/djmemc.c
> @@ -0,0 +1,176 @@
> +/*
> + * djMEMC, macintosh memory and interrupt controller
> + * (Quadra 610/650/800 & Centris 610/650)
> + *
> + *    https://mac68k.info/wiki/display/mac68k/djMEMC+Information
> + *
> + * The djMEMC is an Apple custom integrated circuit chip that performs a
> + * variety of functions (RAM management, clock generation, ...).
> + * It receives interrupt requests from various devices, assign priority to
> + * each, and asserts one or more interrupt line to the CPU.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "migration/vmstate.h"
> +#include "hw/misc/djmemc.h"
> +#include "hw/qdev-properties.h"
> +#include "trace.h"
> +
> +#define DJMEMC_SIZE       0x2000
> +
> +#define InterleaveConf    0
> +#define Bank0Conf         1
> +#define Bank1Conf         2
> +#define Bank2Conf         3
> +#define Bank3Conf         4
> +#define Bank4Conf         5
> +#define Bank5Conf         6
> +#define Bank6Conf         7
> +#define Bank7Conf         8
> +#define Bank8Conf         9
> +#define Bank9Conf         10
> +#define MemTop            11
> +#define Config            12
> +#define Refresh           13

Should this be an enum so the compiler can better verify values and if all 
cases are handled?

Regards,
BALATON Zoltan

Re: [PATCH 2/3] q800: add djMEMC memory controller

[Philippe Mathieu-Daudé]

On 12/12/19 9:01 PM, Laurent Vivier wrote:
> Current implementation is based on GLUE, an early implementation
> of the memory controller found in Macintosh II series.
> 
> Quadra 800 uses in fact djMEMC:
> 
> The djMEMC is an Apple custom integrated circuit chip that performs a
> variety of functions (RAM management, clock generation, ...).
> It receives interrupt requests from various devices, assign priority to
> each, and asserts one or more interrupt line to the CPU.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   MAINTAINERS              |   2 +
>   hw/m68k/Kconfig          |   1 +
>   hw/m68k/q800.c           |  61 ++++----------
>   hw/misc/Kconfig          |   3 +
>   hw/misc/Makefile.objs    |   1 +
>   hw/misc/djmemc.c         | 176 +++++++++++++++++++++++++++++++++++++++
>   hw/misc/trace-events     |   4 +
>   include/hw/misc/djmemc.h |  34 ++++++++
>   8 files changed, 237 insertions(+), 45 deletions(-)
>   create mode 100644 hw/misc/djmemc.c
>   create mode 100644 include/hw/misc/djmemc.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5e5e3e52d6..07224a2fa2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -925,11 +925,13 @@ F: hw/misc/mac_via.c
>   F: hw/nubus/*
>   F: hw/display/macfb.c
>   F: hw/block/swim.c
> +F: hw/misc/djmemc.c
>   F: hw/m68k/bootinfo.h
>   F: include/hw/misc/mac_via.h
>   F: include/hw/nubus/*
>   F: include/hw/display/macfb.h
>   F: include/hw/block/swim.h
> +F: include/hw/misc/djmemc.c
>   
>   MicroBlaze Machines
>   -------------------
> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
> index c757e7dfa4..bdc43a798a 100644
> --- a/hw/m68k/Kconfig
> +++ b/hw/m68k/Kconfig
> @@ -22,3 +22,4 @@ config Q800
>       select ESCC
>       select ESP
>       select DP8393X
> +    select DJMEMC
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index ef0014f4c4..9ee0cb1141 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -46,6 +46,7 @@
>   #include "sysemu/qtest.h"
>   #include "sysemu/runstate.h"
>   #include "sysemu/reset.h"
> +#include "hw/misc/djmemc.h"
>   
>   #define MACROM_ADDR     0x40000000
>   #define MACROM_SIZE     0x00100000
> @@ -68,6 +69,7 @@
>   #define SONIC_PROM_BASE       (IO_BASE + 0x08000)
>   #define SONIC_BASE            (IO_BASE + 0x0a000)
>   #define SCC_BASE              (IO_BASE + 0x0c000)
> +#define DJMEMC_BASE           (IO_BASE + 0x0e000)
>   #define ESP_BASE              (IO_BASE + 0x10000)
>   #define ESP_PDMA              (IO_BASE + 0x10100)
>   #define ASC_BASE              (IO_BASE + 0x14000)
> @@ -85,39 +87,6 @@
>   
>   #define MAC_CLOCK  3686418
>   
> -/*
> - * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
> - * that performs a variety of functions (RAM management, clock generation, ...).
> - * The GLUE chip receives interrupt requests from various devices,
> - * assign priority to each, and asserts one or more interrupt line to the
> - * CPU.
> - */
> -
> -typedef struct {
> -    M68kCPU *cpu;
> -    uint8_t ipr;
> -} GLUEState;
> -
> -static void GLUE_set_irq(void *opaque, int irq, int level)
> -{
> -    GLUEState *s = opaque;
> -    int i;
> -
> -    if (level) {
> -        s->ipr |= 1 << irq;
> -    } else {
> -        s->ipr &= ~(1 << irq);
> -    }
> -
> -    for (i = 7; i >= 0; i--) {
> -        if ((s->ipr >> i) & 1) {
> -            m68k_set_irq_level(s->cpu, i + 1, i + 25);
> -            return;
> -        }
> -    }
> -    m68k_set_irq_level(s->cpu, 0, 0);
> -}
> -
>   static void main_cpu_reset(void *opaque)
>   {
>       M68kCPU *cpu = opaque;
> @@ -149,6 +118,7 @@ static void q800_init(MachineState *machine)
>       const char *kernel_cmdline = machine->kernel_cmdline;
>       hwaddr parameters_base;
>       CPUState *cs;
> +    DeviceState *djmemc_dev;
>       DeviceState *dev;
>       DeviceState *via_dev;
>       SysBusESPState *sysbus_esp;
> @@ -156,8 +126,6 @@ static void q800_init(MachineState *machine)
>       SysBusDevice *sysbus;
>       BusState *adb_bus;
>       NubusBus *nubus;
> -    GLUEState *irq;
> -    qemu_irq *pic;
>   
>       linux_boot = (kernel_filename != NULL);
>   
> @@ -191,11 +159,13 @@ static void q800_init(MachineState *machine)
>           g_free(name);
>       }
>   
> -    /* IRQ Glue */
> +    /* djMEMC memory and interrupt controller */
>   
> -    irq = g_new0(GLUEState, 1);
> -    irq->cpu = cpu;
> -    pic = qemu_allocate_irqs(GLUE_set_irq, irq, 8);

Glad to see you add a QOM INTC and use QDEV API.

> +    djmemc_dev = qdev_create(NULL, TYPE_DJMEMC);
> +    object_property_set_link(OBJECT(djmemc_dev), OBJECT(cpu), "cpu",
> +                             &error_abort);
> +    qdev_init_nofail(djmemc_dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(djmemc_dev), 0, DJMEMC_BASE);
>   
>       /* VIA */
>   
> @@ -203,9 +173,10 @@ static void q800_init(MachineState *machine)
>       qdev_init_nofail(via_dev);
>       sysbus = SYS_BUS_DEVICE(via_dev);
>       sysbus_mmio_map(sysbus, 0, VIA_BASE);
> -    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0, pic[0]);
> -    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1, pic[1]);
> -
> +    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0,
> +                                qdev_get_gpio_in(djmemc_dev, 0));
> +    qdev_connect_gpio_out_named(DEVICE(sysbus),
> +                                "irq", 1, qdev_get_gpio_in(djmemc_dev, 1));

Can you indent the 2 connect_gpio_out() similarly?

>   
>       adb_bus = qdev_get_child_bus(via_dev, "adb.0");
>       dev = qdev_create(adb_bus, TYPE_ADB_KEYBOARD);
> @@ -244,7 +215,7 @@ static void q800_init(MachineState *machine)
>       sysbus = SYS_BUS_DEVICE(dev);
>       sysbus_mmio_map(sysbus, 0, SONIC_BASE);
>       sysbus_mmio_map(sysbus, 1, SONIC_PROM_BASE);
> -    sysbus_connect_irq(sysbus, 0, pic[2]);
> +    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(djmemc_dev, 2));

:)

>   
>       /* SCC */
>   
> @@ -259,8 +230,8 @@ static void q800_init(MachineState *machine)
>       qdev_prop_set_uint32(dev, "chnAtype", 0);
>       qdev_init_nofail(dev);
>       sysbus = SYS_BUS_DEVICE(dev);
> -    sysbus_connect_irq(sysbus, 0, pic[3]);
> -    sysbus_connect_irq(sysbus, 1, pic[3]);
> +    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(djmemc_dev, 3));
> +    sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(djmemc_dev, 3));
>       sysbus_mmio_map(sysbus, 0, SCC_BASE);
>   
>       /* SCSI */
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 2164646553..4c68d20c18 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -125,4 +125,7 @@ config MAC_VIA
>       select MOS6522
>       select ADB
>   
> +config DJMEMC
> +    bool
> +
>   source macio/Kconfig
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index ba898a5781..598e46d7db 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -20,6 +20,7 @@ common-obj-$(CONFIG_ARM11SCU) += arm11scu.o
>   
>   # Mac devices
>   common-obj-$(CONFIG_MOS6522) += mos6522.o
> +obj-$(CONFIG_DJMEMC) += djmemc.o

Hmm this is target-specific because you use a pointer to M68kCPU in 
DjMEMCState... But then there is nothing m68k specific, we just need to 
deliver an IRQ. Maybe we can use a 'CPUState *' and compile as 
common-obj? Ah no, we can't because m68k_set_irq_level() is target 
specific. OK, not this patch problem.

>   
>   # PKUnity SoC devices
>   common-obj-$(CONFIG_PUV3) += puv3_pm.o
> diff --git a/hw/misc/djmemc.c b/hw/misc/djmemc.c
> new file mode 100644
> index 0000000000..b494e82a60
> --- /dev/null
> +++ b/hw/misc/djmemc.c
> @@ -0,0 +1,176 @@
> +/*

Missing (c).

> + * djMEMC, macintosh memory and interrupt controller
> + * (Quadra 610/650/800 & Centris 610/650)
> + *
> + *    https://mac68k.info/wiki/display/mac68k/djMEMC+Information
> + *
> + * The djMEMC is an Apple custom integrated circuit chip that performs a
> + * variety of functions (RAM management, clock generation, ...).
> + * It receives interrupt requests from various devices, assign priority to
> + * each, and asserts one or more interrupt line to the CPU.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "migration/vmstate.h"
> +#include "hw/misc/djmemc.h"
> +#include "hw/qdev-properties.h"
> +#include "trace.h"
> +
> +#define DJMEMC_SIZE       0x2000
> +
> +#define InterleaveConf    0
> +#define Bank0Conf         1
> +#define Bank1Conf         2
> +#define Bank2Conf         3
> +#define Bank3Conf         4
> +#define Bank4Conf         5
> +#define Bank5Conf         6
> +#define Bank6Conf         7
> +#define Bank7Conf         8
> +#define Bank8Conf         9
> +#define Bank9Conf         10
> +#define MemTop            11
> +#define Config            12
> +#define Refresh           13

I agree with Zoltan about using an enum.

> +
> +static const VMStateDescription vmstate_djMEMC = {
> +    .name = "djMEMC",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT32(interleave, DjMEMCState),
> +        VMSTATE_UINT32_ARRAY(bank, DjMEMCState, DjMEMCMaxBanks),
> +        VMSTATE_UINT32(top, DjMEMCState),
> +        VMSTATE_UINT32(config, DjMEMCState),
> +        VMSTATE_UINT32(refresh_rate, DjMEMCState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static uint64_t djMEMC_read(void *opaque, hwaddr addr,
> +                              unsigned size)
> +{
> +    DjMEMCState *s = opaque;
> +    uint64_t value = 0;
> +
> +    switch (addr >> 2) {
> +    case InterleaveConf:
> +        value = s->interleave;

break?

> +    case Bank0Conf...Bank9Conf:
> +        value = s->bank[(addr >> 2) - Bank0Conf];

break? etc...

> +    case MemTop:
> +        value = s->top;
> +    case Config:
> +        value = s->config;
> +    case Refresh:
> +        value = s->refresh_rate;

break + default?

> +    }
> +    trace_djMEMC_read((int)(addr >> 2), size, value);

I'd use trace funcname in lowercase, but you decide whichever you prefer.

> +
> +    return value;
> +}
> +
> +static void djMEMC_write(void *opaque, hwaddr addr, uint64_t value,
> +                         unsigned size)
> +{
> +    DjMEMCState *s = opaque;
> +    trace_djMEMC_write((int)(addr >> 2), size, value);
> +
> +    switch (addr >> 2) {
> +    case InterleaveConf:
> +        s->interleave = value;

break? etc...

> +    case Bank0Conf...Bank9Conf:
> +        s->bank[(addr >> 2) - Bank0Conf] = value;
> +    case MemTop:
> +        s->top = value;
> +    case Config:
> +        s->config = value;
> +    case Refresh:
> +        s->refresh_rate = value;
> +    }
> +}
> +
> +static const MemoryRegionOps djMEMC_mmio_ops = {
> +    .read = djMEMC_read,
> +    .write = djMEMC_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static void djMEMC_set_irq(void *opaque, int irq, int level)
> +{
> +    DjMEMCState *s = opaque;
> +    int i;
> +
> +
> +    if (level) {
> +        s->ipr |= 1 << irq;
> +    } else {
> +        s->ipr &= ~(1 << irq);
> +    }
> +
> +    for (i = 7; i >= 0; i--) {
> +        if ((s->ipr >> i) & 1) {
> +            m68k_set_irq_level(s->cpu, i + 1, i + 25);
> +            return;
> +        }
> +    }
> +    m68k_set_irq_level(s->cpu, 0, 0);
> +}
> +
> +static void djMEMC_init(Object *obj)
> +{
> +    DjMEMCState *s = DJMEMC(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +
> +    memory_region_init_io(&s->mem_regs, NULL, &djMEMC_mmio_ops, s, "djMEMC",
> +                          DJMEMC_SIZE);
> +    sysbus_init_mmio(sbd, &s->mem_regs);
> +
> +    qdev_init_gpio_in(DEVICE(obj), djMEMC_set_irq, 8);
> +    object_property_add_link(obj, "cpu", TYPE_M68K_CPU,
> +                             (Object **) &s->cpu,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             0, NULL);
> +}
> +
> +static void djMEMC_reset(DeviceState *d)
> +{
> +    DjMEMCState *s = DJMEMC(d);
> +    int i;
> +
> +    s->interleave = 0;
> +    s->top = 0;
> +    s->refresh_rate = 0;
> +    s->config = 0;
> +
> +    for (i = 0; i < DjMEMCMaxBanks; i++) {
> +        s->bank[i] = 0;
> +    }
> +}
> +
> +static void djMEMC_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->reset = djMEMC_reset;
> +    dc->vmsd = &vmstate_djMEMC;
> +}
> +
> +static TypeInfo djMEMC_info = {
> +    .name          = TYPE_DJMEMC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(DjMEMCState),
> +    .instance_init = djMEMC_init,
> +    .class_init    = djMEMC_class_init,
> +};
> +
> +static void djMEMC_register_types(void)
> +{
> +    type_register_static(&djMEMC_info);
> +}
> +
> +type_init(djMEMC_register_types)
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 1deb1d08c1..c9bcdd4a54 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -149,3 +149,7 @@ bcm2835_mbox_write(unsigned int size, uint64_t addr, uint64_t value) "mbox write
>   bcm2835_mbox_read(unsigned int size, uint64_t addr, uint64_t value) "mbox read sz:%u addr:0x%"PRIx64" data:0x%"PRIx64
>   bcm2835_mbox_irq(unsigned level) "mbox irq:ARM level:%u"
>   bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
> +
> +# djmemc.c
> +djMEMC_read(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64
> +djMEMC_write(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64
> diff --git a/include/hw/misc/djmemc.h b/include/hw/misc/djmemc.h
> new file mode 100644
> index 0000000000..0f29ac1cf3
> --- /dev/null
> +++ b/include/hw/misc/djmemc.h
> @@ -0,0 +1,34 @@
> +/*
> + * djMEMC, macintosh memory and interrupt controller
> + * (Quadra 610/650/800 & Centris 610/650)

(C)?

> + */
> +
> +#ifndef HW_MISC_DJMEMC_H
> +#define HW_MISC_DJMEMC_H
> +
> +#include "hw/sysbus.h"
> +#include "cpu.h"
> +
> +#define DjMEMCMaxBanks    10
> +
> +typedef struct DjMEMCState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion mem_regs;
> +
> +    /* Memory controller */
> +    uint32_t interleave;
> +    uint32_t bank[DjMEMCMaxBanks];
> +    uint32_t top;
> +    uint32_t config;
> +    uint32_t refresh_rate;
> +
> +    /* interrupt controller */
> +    M68kCPU *cpu;
> +    uint8_t ipr;
> +} DjMEMCState;
> +
> +#define TYPE_DJMEMC "djMEMC"
> +#define DJMEMC(obj) OBJECT_CHECK(DjMEMCState, (obj), TYPE_DJMEMC)
> +
> +#endif
> 

Re: [PATCH 2/3] q800: add djMEMC memory controller

[Mark Cave-Ayland]

On 12/12/2019 20:01, Laurent Vivier wrote:

> Current implementation is based on GLUE, an early implementation
> of the memory controller found in Macintosh II series.
> 
> Quadra 800 uses in fact djMEMC:
> 
> The djMEMC is an Apple custom integrated circuit chip that performs a
> variety of functions (RAM management, clock generation, ...).
> It receives interrupt requests from various devices, assign priority to
> each, and asserts one or more interrupt line to the CPU.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  MAINTAINERS              |   2 +
>  hw/m68k/Kconfig          |   1 +
>  hw/m68k/q800.c           |  61 ++++----------
>  hw/misc/Kconfig          |   3 +
>  hw/misc/Makefile.objs    |   1 +
>  hw/misc/djmemc.c         | 176 +++++++++++++++++++++++++++++++++++++++
>  hw/misc/trace-events     |   4 +
>  include/hw/misc/djmemc.h |  34 ++++++++
>  8 files changed, 237 insertions(+), 45 deletions(-)
>  create mode 100644 hw/misc/djmemc.c
>  create mode 100644 include/hw/misc/djmemc.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5e5e3e52d6..07224a2fa2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -925,11 +925,13 @@ F: hw/misc/mac_via.c
>  F: hw/nubus/*
>  F: hw/display/macfb.c
>  F: hw/block/swim.c
> +F: hw/misc/djmemc.c
>  F: hw/m68k/bootinfo.h
>  F: include/hw/misc/mac_via.h
>  F: include/hw/nubus/*
>  F: include/hw/display/macfb.h
>  F: include/hw/block/swim.h
> +F: include/hw/misc/djmemc.c
>  
>  MicroBlaze Machines
>  -------------------
> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
> index c757e7dfa4..bdc43a798a 100644
> --- a/hw/m68k/Kconfig
> +++ b/hw/m68k/Kconfig
> @@ -22,3 +22,4 @@ config Q800
>      select ESCC
>      select ESP
>      select DP8393X
> +    select DJMEMC
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index ef0014f4c4..9ee0cb1141 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -46,6 +46,7 @@
>  #include "sysemu/qtest.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/reset.h"
> +#include "hw/misc/djmemc.h"
>  
>  #define MACROM_ADDR     0x40000000
>  #define MACROM_SIZE     0x00100000
> @@ -68,6 +69,7 @@
>  #define SONIC_PROM_BASE       (IO_BASE + 0x08000)
>  #define SONIC_BASE            (IO_BASE + 0x0a000)
>  #define SCC_BASE              (IO_BASE + 0x0c000)
> +#define DJMEMC_BASE           (IO_BASE + 0x0e000)
>  #define ESP_BASE              (IO_BASE + 0x10000)
>  #define ESP_PDMA              (IO_BASE + 0x10100)
>  #define ASC_BASE              (IO_BASE + 0x14000)
> @@ -85,39 +87,6 @@
>  
>  #define MAC_CLOCK  3686418
>  
> -/*
> - * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
> - * that performs a variety of functions (RAM management, clock generation, ...).
> - * The GLUE chip receives interrupt requests from various devices,
> - * assign priority to each, and asserts one or more interrupt line to the
> - * CPU.
> - */
> -
> -typedef struct {
> -    M68kCPU *cpu;
> -    uint8_t ipr;
> -} GLUEState;
> -
> -static void GLUE_set_irq(void *opaque, int irq, int level)
> -{
> -    GLUEState *s = opaque;
> -    int i;
> -
> -    if (level) {
> -        s->ipr |= 1 << irq;
> -    } else {
> -        s->ipr &= ~(1 << irq);
> -    }
> -
> -    for (i = 7; i >= 0; i--) {
> -        if ((s->ipr >> i) & 1) {
> -            m68k_set_irq_level(s->cpu, i + 1, i + 25);
> -            return;
> -        }
> -    }
> -    m68k_set_irq_level(s->cpu, 0, 0);
> -}
> -
>  static void main_cpu_reset(void *opaque)
>  {
>      M68kCPU *cpu = opaque;
> @@ -149,6 +118,7 @@ static void q800_init(MachineState *machine)
>      const char *kernel_cmdline = machine->kernel_cmdline;
>      hwaddr parameters_base;
>      CPUState *cs;
> +    DeviceState *djmemc_dev;
>      DeviceState *dev;
>      DeviceState *via_dev;
>      SysBusESPState *sysbus_esp;
> @@ -156,8 +126,6 @@ static void q800_init(MachineState *machine)
>      SysBusDevice *sysbus;
>      BusState *adb_bus;
>      NubusBus *nubus;
> -    GLUEState *irq;
> -    qemu_irq *pic;
>  
>      linux_boot = (kernel_filename != NULL);
>  
> @@ -191,11 +159,13 @@ static void q800_init(MachineState *machine)
>          g_free(name);
>      }
>  
> -    /* IRQ Glue */
> +    /* djMEMC memory and interrupt controller */
>  
> -    irq = g_new0(GLUEState, 1);
> -    irq->cpu = cpu;
> -    pic = qemu_allocate_irqs(GLUE_set_irq, irq, 8);
> +    djmemc_dev = qdev_create(NULL, TYPE_DJMEMC);
> +    object_property_set_link(OBJECT(djmemc_dev), OBJECT(cpu), "cpu",
> +                             &error_abort);
> +    qdev_init_nofail(djmemc_dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(djmemc_dev), 0, DJMEMC_BASE);
>  
>      /* VIA */
>  
> @@ -203,9 +173,10 @@ static void q800_init(MachineState *machine)
>      qdev_init_nofail(via_dev);
>      sysbus = SYS_BUS_DEVICE(via_dev);
>      sysbus_mmio_map(sysbus, 0, VIA_BASE);
> -    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0, pic[0]);
> -    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1, pic[1]);
> -
> +    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0,
> +                                qdev_get_gpio_in(djmemc_dev, 0));
> +    qdev_connect_gpio_out_named(DEVICE(sysbus),
> +                                "irq", 1, qdev_get_gpio_in(djmemc_dev, 1));
>  
>      adb_bus = qdev_get_child_bus(via_dev, "adb.0");
>      dev = qdev_create(adb_bus, TYPE_ADB_KEYBOARD);
> @@ -244,7 +215,7 @@ static void q800_init(MachineState *machine)
>      sysbus = SYS_BUS_DEVICE(dev);
>      sysbus_mmio_map(sysbus, 0, SONIC_BASE);
>      sysbus_mmio_map(sysbus, 1, SONIC_PROM_BASE);
> -    sysbus_connect_irq(sysbus, 0, pic[2]);
> +    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(djmemc_dev, 2));
>  
>      /* SCC */
>  
> @@ -259,8 +230,8 @@ static void q800_init(MachineState *machine)
>      qdev_prop_set_uint32(dev, "chnAtype", 0);
>      qdev_init_nofail(dev);
>      sysbus = SYS_BUS_DEVICE(dev);
> -    sysbus_connect_irq(sysbus, 0, pic[3]);
> -    sysbus_connect_irq(sysbus, 1, pic[3]);
> +    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(djmemc_dev, 3));
> +    sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(djmemc_dev, 3));
>      sysbus_mmio_map(sysbus, 0, SCC_BASE);
>  
>      /* SCSI */
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 2164646553..4c68d20c18 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -125,4 +125,7 @@ config MAC_VIA
>      select MOS6522
>      select ADB
>  
> +config DJMEMC
> +    bool
> +
>  source macio/Kconfig
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index ba898a5781..598e46d7db 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -20,6 +20,7 @@ common-obj-$(CONFIG_ARM11SCU) += arm11scu.o
>  
>  # Mac devices
>  common-obj-$(CONFIG_MOS6522) += mos6522.o
> +obj-$(CONFIG_DJMEMC) += djmemc.o
>  
>  # PKUnity SoC devices
>  common-obj-$(CONFIG_PUV3) += puv3_pm.o
> diff --git a/hw/misc/djmemc.c b/hw/misc/djmemc.c
> new file mode 100644
> index 0000000000..b494e82a60
> --- /dev/null
> +++ b/hw/misc/djmemc.c
> @@ -0,0 +1,176 @@
> +/*
> + * djMEMC, macintosh memory and interrupt controller
> + * (Quadra 610/650/800 & Centris 610/650)
> + *
> + *    https://mac68k.info/wiki/display/mac68k/djMEMC+Information
> + *
> + * The djMEMC is an Apple custom integrated circuit chip that performs a
> + * variety of functions (RAM management, clock generation, ...).
> + * It receives interrupt requests from various devices, assign priority to
> + * each, and asserts one or more interrupt line to the CPU.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "migration/vmstate.h"
> +#include "hw/misc/djmemc.h"
> +#include "hw/qdev-properties.h"
> +#include "trace.h"
> +
> +#define DJMEMC_SIZE       0x2000
> +
> +#define InterleaveConf    0
> +#define Bank0Conf         1
> +#define Bank1Conf         2
> +#define Bank2Conf         3
> +#define Bank3Conf         4
> +#define Bank4Conf         5
> +#define Bank5Conf         6
> +#define Bank6Conf         7
> +#define Bank7Conf         8
> +#define Bank8Conf         9
> +#define Bank9Conf         10
> +#define MemTop            11
> +#define Config            12
> +#define Refresh           13
> +
> +static const VMStateDescription vmstate_djMEMC = {
> +    .name = "djMEMC",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT32(interleave, DjMEMCState),
> +        VMSTATE_UINT32_ARRAY(bank, DjMEMCState, DjMEMCMaxBanks),
> +        VMSTATE_UINT32(top, DjMEMCState),
> +        VMSTATE_UINT32(config, DjMEMCState),
> +        VMSTATE_UINT32(refresh_rate, DjMEMCState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static uint64_t djMEMC_read(void *opaque, hwaddr addr,
> +                              unsigned size)
> +{
> +    DjMEMCState *s = opaque;
> +    uint64_t value = 0;
> +
> +    switch (addr >> 2) {
> +    case InterleaveConf:
> +        value = s->interleave;
> +    case Bank0Conf...Bank9Conf:
> +        value = s->bank[(addr >> 2) - Bank0Conf];
> +    case MemTop:
> +        value = s->top;
> +    case Config:
> +        value = s->config;
> +    case Refresh:
> +        value = s->refresh_rate;
> +    }
> +    trace_djMEMC_read((int)(addr >> 2), size, value);

Is there any chance that we match existing QEMU style here e.g. all capitals for
constants and all lower case for trace events? The capitalisation of the djMEMC has
caught me out a few times...

> +    return value;
> +}
> +
> +static void djMEMC_write(void *opaque, hwaddr addr, uint64_t value,
> +                         unsigned size)
> +{
> +    DjMEMCState *s = opaque;
> +    trace_djMEMC_write((int)(addr >> 2), size, value);
> +
> +    switch (addr >> 2) {
> +    case InterleaveConf:
> +        s->interleave = value;
> +    case Bank0Conf...Bank9Conf:
> +        s->bank[(addr >> 2) - Bank0Conf] = value;
> +    case MemTop:
> +        s->top = value;
> +    case Config:
> +        s->config = value;
> +    case Refresh:
> +        s->refresh_rate = value;
> +    }
> +}
> +
> +static const MemoryRegionOps djMEMC_mmio_ops = {
> +    .read = djMEMC_read,
> +    .write = djMEMC_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static void djMEMC_set_irq(void *opaque, int irq, int level)
> +{
> +    DjMEMCState *s = opaque;
> +    int i;
> +
> +
> +    if (level) {
> +        s->ipr |= 1 << irq;
> +    } else {
> +        s->ipr &= ~(1 << irq);
> +    }
> +
> +    for (i = 7; i >= 0; i--) {
> +        if ((s->ipr >> i) & 1) {
> +            m68k_set_irq_level(s->cpu, i + 1, i + 25);
> +            return;
> +        }
> +    }
> +    m68k_set_irq_level(s->cpu, 0, 0);
> +}
> +
> +static void djMEMC_init(Object *obj)
> +{
> +    DjMEMCState *s = DJMEMC(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +
> +    memory_region_init_io(&s->mem_regs, NULL, &djMEMC_mmio_ops, s, "djMEMC",
> +                          DJMEMC_SIZE);
> +    sysbus_init_mmio(sbd, &s->mem_regs);
> +
> +    qdev_init_gpio_in(DEVICE(obj), djMEMC_set_irq, 8);
> +    object_property_add_link(obj, "cpu", TYPE_M68K_CPU,
> +                             (Object **) &s->cpu,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             0, NULL);
> +}
> +
> +static void djMEMC_reset(DeviceState *d)
> +{
> +    DjMEMCState *s = DJMEMC(d);
> +    int i;
> +
> +    s->interleave = 0;
> +    s->top = 0;
> +    s->refresh_rate = 0;
> +    s->config = 0;
> +
> +    for (i = 0; i < DjMEMCMaxBanks; i++) {
> +        s->bank[i] = 0;
> +    }
> +}
> +
> +static void djMEMC_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->reset = djMEMC_reset;
> +    dc->vmsd = &vmstate_djMEMC;
> +}
> +
> +static TypeInfo djMEMC_info = {
> +    .name          = TYPE_DJMEMC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(DjMEMCState),
> +    .instance_init = djMEMC_init,
> +    .class_init    = djMEMC_class_init,
> +};
> +
> +static void djMEMC_register_types(void)
> +{
> +    type_register_static(&djMEMC_info);
> +}
> +
> +type_init(djMEMC_register_types)
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 1deb1d08c1..c9bcdd4a54 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -149,3 +149,7 @@ bcm2835_mbox_write(unsigned int size, uint64_t addr, uint64_t value) "mbox write
>  bcm2835_mbox_read(unsigned int size, uint64_t addr, uint64_t value) "mbox read sz:%u addr:0x%"PRIx64" data:0x%"PRIx64
>  bcm2835_mbox_irq(unsigned level) "mbox irq:ARM level:%u"
>  bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
> +
> +# djmemc.c
> +djMEMC_read(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64
> +djMEMC_write(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64
> diff --git a/include/hw/misc/djmemc.h b/include/hw/misc/djmemc.h
> new file mode 100644
> index 0000000000..0f29ac1cf3
> --- /dev/null
> +++ b/include/hw/misc/djmemc.h
> @@ -0,0 +1,34 @@
> +/*
> + * djMEMC, macintosh memory and interrupt controller
> + * (Quadra 610/650/800 & Centris 610/650)
> + */
> +
> +#ifndef HW_MISC_DJMEMC_H
> +#define HW_MISC_DJMEMC_H
> +
> +#include "hw/sysbus.h"
> +#include "cpu.h"
> +
> +#define DjMEMCMaxBanks    10
> +
> +typedef struct DjMEMCState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion mem_regs;
> +
> +    /* Memory controller */
> +    uint32_t interleave;
> +    uint32_t bank[DjMEMCMaxBanks];
> +    uint32_t top;
> +    uint32_t config;
> +    uint32_t refresh_rate;
> +
> +    /* interrupt controller */
> +    M68kCPU *cpu;
> +    uint8_t ipr;
> +} DjMEMCState;
> +
> +#define TYPE_DJMEMC "djMEMC"
> +#define DJMEMC(obj) OBJECT_CHECK(DjMEMCState, (obj), TYPE_DJMEMC)
> +
> +#endif


ATB,

Mark.

[PATCH 3/3] q800: add machine id register

[Laurent Vivier]

MacOS reads this address to identify the hardware.

This is a basic implementation returning the ID of Quadra 800.

Details:

  http://mess.redump.net/mess/driver_info/mac_technical_notes

"There are 3 ID schemes [...]
 The third and most scalable is a machine ID register at 0x5ffffffc.
 The top word must be 0xa55a to be valid. Then bits 15-11 are 0 for
 consumer Macs, 1 for portables, 2 for high-end 68k, and 3 for high-end
 PowerPC. Bit 10 is 1 if additional ID bits appear elsewhere (e.g. in VIA1).
 The rest of the bits are a per-model identifier.

 Model                          Lower 16 bits of ID
...
 Quadra/Centris 610/650/800     0x2BAD"

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 9ee0cb1141..c2b2aa779f 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -97,6 +97,23 @@ static void main_cpu_reset(void *opaque)
     cpu->env.pc = ldl_phys(cs->as, 4);
 }
 
+static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0xa55a2bad; /* Quadra 800 ID */
+}
+
+static void machine_id_write(void *opaque, hwaddr addr, uint64_t val,
+                             unsigned size)
+{
+}
+
+static const MemoryRegionOps machine_id_ops = {
+    .read = machine_id_read,
+    .write = machine_id_write,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+};
+
 static void q800_init(MachineState *machine)
 {
     M68kCPU *cpu = NULL;
@@ -110,6 +127,7 @@ static void q800_init(MachineState *machine)
     MemoryRegion *rom;
     MemoryRegion *ram;
     MemoryRegion *io;
+    MemoryRegion *machine_id;
     const int io_slice_nb = (IO_SIZE / IO_SLICE) - 1;
     int i;
     ram_addr_t ram_size = machine->ram_size;
@@ -159,6 +177,10 @@ static void q800_init(MachineState *machine)
         g_free(name);
     }
 
+    machine_id = g_malloc(sizeof(*machine_id));
+    memory_region_init_io(machine_id, NULL, &machine_id_ops, NULL, "Machine ID", 4);
+    memory_region_add_subregion(get_system_memory(), 0x5ffffffc, machine_id);
+
     /* djMEMC memory and interrupt controller */
 
     djmemc_dev = qdev_create(NULL, TYPE_DJMEMC);
-- 
2.23.0

Re: [PATCH 3/3] q800: add machine id register

[Mark Cave-Ayland]

On 12/12/2019 20:01, Laurent Vivier wrote:

> MacOS reads this address to identify the hardware.
> 
> This is a basic implementation returning the ID of Quadra 800.
> 
> Details:
> 
>   http://mess.redump.net/mess/driver_info/mac_technical_notes
> 
> "There are 3 ID schemes [...]
>  The third and most scalable is a machine ID register at 0x5ffffffc.
>  The top word must be 0xa55a to be valid. Then bits 15-11 are 0 for
>  consumer Macs, 1 for portables, 2 for high-end 68k, and 3 for high-end
>  PowerPC. Bit 10 is 1 if additional ID bits appear elsewhere (e.g. in VIA1).
>  The rest of the bits are a per-model identifier.
> 
>  Model                          Lower 16 bits of ID
> ...
>  Quadra/Centris 610/650/800     0x2BAD"
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/m68k/q800.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 9ee0cb1141..c2b2aa779f 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -97,6 +97,23 @@ static void main_cpu_reset(void *opaque)
>      cpu->env.pc = ldl_phys(cs->as, 4);
>  }
>  
> +static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return 0xa55a2bad; /* Quadra 800 ID */
> +}
> +
> +static void machine_id_write(void *opaque, hwaddr addr, uint64_t val,
> +                             unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps machine_id_ops = {
> +    .read = machine_id_read,
> +    .write = machine_id_write,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +};

Should there be a .endianness here just to be really safe?

>  static void q800_init(MachineState *machine)
>  {
>      M68kCPU *cpu = NULL;
> @@ -110,6 +127,7 @@ static void q800_init(MachineState *machine)
>      MemoryRegion *rom;
>      MemoryRegion *ram;
>      MemoryRegion *io;
> +    MemoryRegion *machine_id;
>      const int io_slice_nb = (IO_SIZE / IO_SLICE) - 1;
>      int i;
>      ram_addr_t ram_size = machine->ram_size;
> @@ -159,6 +177,10 @@ static void q800_init(MachineState *machine)
>          g_free(name);
>      }
>  
> +    machine_id = g_malloc(sizeof(*machine_id));
> +    memory_region_init_io(machine_id, NULL, &machine_id_ops, NULL, "Machine ID", 4);
> +    memory_region_add_subregion(get_system_memory(), 0x5ffffffc, machine_id);
> +
>      /* djMEMC memory and interrupt controller */
>  
>      djmemc_dev = qdev_create(NULL, TYPE_DJMEMC);

Otherwise looks good and I see it being poked by a MacOS ROM so:

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


ATB,

Mark.

Re: [PATCH 3/3] q800: add machine id register

[Philippe Mathieu-Daudé]

On 12/12/19 9:01 PM, Laurent Vivier wrote:
> MacOS reads this address to identify the hardware.
> 
> This is a basic implementation returning the ID of Quadra 800.
> 
> Details:
> 
>    http://mess.redump.net/mess/driver_info/mac_technical_notes
> 
> "There are 3 ID schemes [...]
>   The third and most scalable is a machine ID register at 0x5ffffffc.
>   The top word must be 0xa55a to be valid. Then bits 15-11 are 0 for
>   consumer Macs, 1 for portables, 2 for high-end 68k, and 3 for high-end
>   PowerPC. Bit 10 is 1 if additional ID bits appear elsewhere (e.g. in VIA1).
>   The rest of the bits are a per-model identifier.
> 
>   Model                          Lower 16 bits of ID
> ...
>   Quadra/Centris 610/650/800     0x2BAD"
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   hw/m68k/q800.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 9ee0cb1141..c2b2aa779f 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -97,6 +97,23 @@ static void main_cpu_reset(void *opaque)
>       cpu->env.pc = ldl_phys(cs->as, 4);
>   }
>   
> +static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return 0xa55a2bad; /* Quadra 800 ID */
> +}
> +
> +static void machine_id_write(void *opaque, hwaddr addr, uint64_t val,
> +                             unsigned size)
> +{

Maybe worth adding:

        qemu_log_mask(LOG_GUEST_ERROR, ...);

> +}
> +
> +static const MemoryRegionOps machine_id_ops = {
> +    .read = machine_id_read,
> +    .write = machine_id_write,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,

I think you want s/valid/impl/, because i.e. the guest could use 16-bit 
access right?

> +};
> +
>   static void q800_init(MachineState *machine)
>   {
>       M68kCPU *cpu = NULL;
> @@ -110,6 +127,7 @@ static void q800_init(MachineState *machine)
>       MemoryRegion *rom;
>       MemoryRegion *ram;
>       MemoryRegion *io;
> +    MemoryRegion *machine_id;
>       const int io_slice_nb = (IO_SIZE / IO_SLICE) - 1;
>       int i;
>       ram_addr_t ram_size = machine->ram_size;
> @@ -159,6 +177,10 @@ static void q800_init(MachineState *machine)
>           g_free(name);
>       }
>   
> +    machine_id = g_malloc(sizeof(*machine_id));

We now prefer g_new(MemoryRegion, 1).

> +    memory_region_init_io(machine_id, NULL, &machine_id_ops, NULL, "Machine ID", 4);
> +    memory_region_add_subregion(get_system_memory(), 0x5ffffffc, machine_id);

To keep the style consistent, can you use a MACHINEID_BASE definition 
instead?

> +
>       /* djMEMC memory and interrupt controller */
>   
>       djmemc_dev = qdev_create(NULL, TYPE_DJMEMC);
> 

Re: [PATCH 0/3] q800: update machine emulation

[Mark Cave-Ayland]

On 12/12/2019 20:01, Laurent Vivier wrote:

> On the way to run a MacOS ROM we need a more accurate
> emulation of the Quadra 800.
> 
> This series fixes the ESCC base address that was wrong but
> as the linux kernel uses the one provided by the bootloader
> (in our case QEMU) it was not a problem. This value
> is hardcoded in the ROM, so QEMU must use the good one.
> 
> The two other patches update the description of the machine
> by introducing the djMEMC memory controller and the machine id
> register.
> 
> Laurent Vivier (3):
>   q800: fix ESCC base
>   q800: add djMEMC memory controller
>   q800: add machine id register
> 
>  MAINTAINERS              |   2 +
>  hw/m68k/Kconfig          |   1 +
>  hw/m68k/q800.c           |  85 +++++++++----------
>  hw/misc/Kconfig          |   3 +
>  hw/misc/Makefile.objs    |   1 +
>  hw/misc/djmemc.c         | 176 +++++++++++++++++++++++++++++++++++++++
>  hw/misc/trace-events     |   4 +
>  include/hw/misc/djmemc.h |  34 ++++++++
>  8 files changed, 260 insertions(+), 46 deletions(-)
>  create mode 100644 hw/misc/djmemc.c
>  create mode 100644 include/hw/misc/djmemc.h

I think this is mostly there other than a few minor style and other tweaks, however
I'm wondering that since the q800 seems to have a few esoteric devices if it is worth
creating a separate subdirectory for them all under hw/misc/mac or similar?


ATB,

Mark.