[PATCH v2 0/7] macio: remove PIC object property links

[PATCH v2 0/7] macio: remove PIC object property links

[Mark Cave-Ayland]

This patchset follows on from the dicussion at https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02630.html
where the user_creatable flag for the macio devices was set back to false just
before the 5.2 release.

The underlying cause was that the PIC object property links were not being set
before realise. Whilst this cannot happen when launching the g3beige and mac99
machines from qemu-system-ppc, it caused some automated tests to fail.

Here we fix the real problem which is to move the PIC for both machines into the
macio device, which not only matches real hardware but also enables the PIC object
property links to be completely removed.

Patch 6 rewires the macio gpios for the mac99 machine as per Ben's original comment
after the OpenPIC device has been moved into the macio-newworld device, and then
finally patch 7 removes setting the user_creatable flag to false on the macio devices
once again.

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

v2:
- Add R-B tag for patch 1 from David
- Update commit messages to included more detail as requested by David


Mark Cave-Ayland (7):
  mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
  mac_oldworld: move initialisation of grackle before heathrow
  macio: move heathrow PIC inside macio-oldworld device
  mac_newworld: delay wiring of PCI IRQs in New World machine
  macio: move OpenPIC inside macio-newworld device
  macio: wire macio GPIOs to OpenPIC using sysbus IRQs
  macio: don't set user_creatable to false

 hw/misc/macio/gpio.c          | 24 +++--------
 hw/misc/macio/macio.c         | 53 ++++++++++++------------
 hw/ppc/mac_newworld.c         | 71 ++++++++++++++++----------------
 hw/ppc/mac_oldworld.c         | 76 ++++++++++++++++-------------------
 include/hw/misc/macio/gpio.h  |  2 -
 include/hw/misc/macio/macio.h |  4 +-
 6 files changed, 104 insertions(+), 126 deletions(-)

-- 
2.20.1

[PATCH v2 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env)

[Mark Cave-Ayland]

This condition will have already been caught when wiring the heathrow PIC
IRQs to the CPU.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/mac_oldworld.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 04f98a4d81..2ead34bdf1 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -251,12 +251,6 @@ static void ppc_heathrow_init(MachineState *machine)
         tbfreq = TBFREQ;
     }
 
-    /* init basic PC hardware */
-    if (PPC_INPUT(env) != PPC_FLAGS_INPUT_6xx) {
-        error_report("Only 6xx bus is supported on heathrow machine");
-        exit(1);
-    }
-
     /* Grackle PCI host bridge */
     dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
     qdev_prop_set_uint32(dev, "ofw-addr", 0x80000000);
-- 
2.20.1

[PATCH v2 2/7] mac_oldworld: move initialisation of grackle before heathrow

[Mark Cave-Ayland]

In order to move the heathrow PIC to the macio device, the PCI bus needs to be
initialised before the macio device and also before wiring the PIC IRQs.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_oldworld.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2ead34bdf1..e58e0525fe 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -227,6 +227,21 @@ static void ppc_heathrow_init(MachineState *machine)
         }
     }
 
+    /* Grackle PCI host bridge */
+    dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+    qdev_prop_set_uint32(dev, "ofw-addr", 0x80000000);
+    s = SYS_BUS_DEVICE(dev);
+    sysbus_realize_and_unref(s, &error_fatal);
+
+    sysbus_mmio_map(s, 0, GRACKLE_BASE);
+    sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
+    /* PCI hole */
+    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
+                                sysbus_mmio_get_region(s, 2));
+    /* Register 2 MB of ISA IO space */
+    memory_region_add_subregion(get_system_memory(), 0xfe000000,
+                                sysbus_mmio_get_region(s, 3));
+
     /* XXX: we register only 1 output pin for heathrow PIC */
     pic_dev = qdev_new(TYPE_HEATHROW);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
@@ -251,21 +266,6 @@ static void ppc_heathrow_init(MachineState *machine)
         tbfreq = TBFREQ;
     }
 
-    /* Grackle PCI host bridge */
-    dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-    qdev_prop_set_uint32(dev, "ofw-addr", 0x80000000);
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(s, &error_fatal);
-
-    sysbus_mmio_map(s, 0, GRACKLE_BASE);
-    sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
-    /* PCI hole */
-    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
-                                sysbus_mmio_get_region(s, 2));
-    /* Register 2 MB of ISA IO space */
-    memory_region_add_subregion(get_system_memory(), 0xfe000000,
-                                sysbus_mmio_get_region(s, 3));
-
     for (i = 0; i < 4; i++) {
         qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
     }
-- 
2.20.1

[PATCH v2 3/7] macio: move heathrow PIC inside macio-oldworld device

[Mark Cave-Ayland]

The heathrow PIC is located within the macio device on real hardware so make it
a child of the macio-oldworld device. This also removes the need for setting and
checking a separate PIC object property link on the macio-oldworld device which
currently causes the automated QOM introspection tests to fail.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/macio.c         | 20 +++++------
 hw/ppc/mac_oldworld.c         | 66 +++++++++++++++++------------------
 include/hw/misc/macio/macio.h |  2 +-
 3 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index bb601f782c..cfb87da6c9 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -140,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
 {
     MacIOState *s = MACIO(d);
     OldWorldMacIOState *os = OLDWORLD_MACIO(d);
-    DeviceState *pic_dev = DEVICE(os->pic);
+    DeviceState *pic_dev = DEVICE(&os->pic);
     Error *err = NULL;
     SysBusDevice *sysbus_dev;
 
@@ -150,6 +150,14 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
         return;
     }
 
+    /* Heathrow PIC */
+    if (!qdev_realize(DEVICE(&os->pic), BUS(&s->macio_bus), errp)) {
+        return;
+    }
+    sysbus_dev = SYS_BUS_DEVICE(&os->pic);
+    memory_region_add_subregion(&s->bar, 0x0,
+                                sysbus_mmio_get_region(sysbus_dev, 0));
+
     qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
                          s->frequency);
     if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
@@ -175,11 +183,6 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
                                 sysbus_mmio_get_region(sysbus_dev, 0));
     pmac_format_nvram_partition(&os->nvram, os->nvram.size);
 
-    /* Heathrow PIC */
-    sysbus_dev = SYS_BUS_DEVICE(os->pic);
-    memory_region_add_subregion(&s->bar, 0x0,
-                                sysbus_mmio_get_region(sysbus_dev, 0));
-
     /* IDE buses */
     macio_realize_ide(s, &os->ide[0],
                       qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
@@ -218,10 +221,7 @@ static void macio_oldworld_init(Object *obj)
     DeviceState *dev;
     int i;
 
-    object_property_add_link(obj, "pic", TYPE_HEATHROW,
-                             (Object **) &os->pic,
-                             qdev_prop_allow_set_link_before_realize,
-                             0);
+    object_initialize_child(OBJECT(s), "pic", &os->pic, TYPE_HEATHROW);
 
     object_initialize_child(OBJECT(s), "cuda", &s->cuda, TYPE_CUDA);
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index e58e0525fe..44ee99be88 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -98,7 +98,7 @@ static void ppc_heathrow_init(MachineState *machine)
     MACIOIDEState *macio_ide;
     ESCCState *escc;
     SysBusDevice *s;
-    DeviceState *dev, *pic_dev;
+    DeviceState *dev, *pic_dev, *grackle_dev;
     BusState *adb_bus;
     uint64_t bios_addr;
     int bios_size;
@@ -227,10 +227,17 @@ static void ppc_heathrow_init(MachineState *machine)
         }
     }
 
+    /* Timebase Frequency */
+    if (kvm_enabled()) {
+        tbfreq = kvmppc_get_tbfreq();
+    } else {
+        tbfreq = TBFREQ;
+    }
+
     /* Grackle PCI host bridge */
-    dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-    qdev_prop_set_uint32(dev, "ofw-addr", 0x80000000);
-    s = SYS_BUS_DEVICE(dev);
+    grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+    qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
+    s = SYS_BUS_DEVICE(grackle_dev);
     sysbus_realize_and_unref(s, &error_fatal);
 
     sysbus_mmio_map(s, 0, GRACKLE_BASE);
@@ -242,14 +249,30 @@ static void ppc_heathrow_init(MachineState *machine)
     memory_region_add_subregion(get_system_memory(), 0xfe000000,
                                 sysbus_mmio_get_region(s, 3));
 
-    /* XXX: we register only 1 output pin for heathrow PIC */
-    pic_dev = qdev_new(TYPE_HEATHROW);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
+    pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
+
+    /* MacIO */
+    macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
+    dev = DEVICE(macio);
+    qdev_prop_set_uint64(dev, "frequency", tbfreq);
+
+    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
+    pci_realize_and_unref(macio, pci_bus, &error_fatal);
+
+    pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
+    for (i = 0; i < 4; i++) {
+        qdev_connect_gpio_out(grackle_dev, i,
+                              qdev_get_gpio_in(pic_dev, 0x15 + i));
+    }
 
     /* Connect the heathrow PIC outputs to the 6xx bus */
     for (i = 0; i < smp_cpus; i++) {
         switch (PPC_INPUT(env)) {
         case PPC_FLAGS_INPUT_6xx:
+            /* XXX: we register only 1 output pin for heathrow PIC */
             qdev_connect_gpio_out(pic_dev, 0,
                 ((qemu_irq *)env->irq_inputs)[PPC6xx_INPUT_INT]);
             break;
@@ -259,40 +282,14 @@ static void ppc_heathrow_init(MachineState *machine)
         }
     }
 
-    /* Timebase Frequency */
-    if (kvm_enabled()) {
-        tbfreq = kvmppc_get_tbfreq();
-    } else {
-        tbfreq = TBFREQ;
-    }
-
-    for (i = 0; i < 4; i++) {
-        qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
-    }
-
-    pci_bus = PCI_HOST_BRIDGE(dev)->bus;
-
     pci_vga_init(pci_bus);
 
     for (i = 0; i < nb_nics; i++) {
         pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
     }
 
+    /* MacIO IDE */
     ide_drive_get(hd, ARRAY_SIZE(hd));
-
-    /* MacIO */
-    macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
-    dev = DEVICE(macio);
-    qdev_prop_set_uint64(dev, "frequency", tbfreq);
-    object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
-                             &error_abort);
-
-    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
-    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
-    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
-
-    pci_realize_and_unref(macio, pci_bus, &error_fatal);
-
     macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
                                                         "ide[0]"));
     macio_ide_init_drives(macio_ide, hd);
@@ -301,6 +298,7 @@ static void ppc_heathrow_init(MachineState *machine)
                                                         "ide[1]"));
     macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]);
 
+    /* MacIO CUDA/ADB */
     dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda"));
     adb_bus = qdev_get_child_bus(dev, "adb.0");
     dev = qdev_new(TYPE_ADB_KEYBOARD);
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 22b4e64b2c..707dfab50c 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -99,7 +99,7 @@ struct OldWorldMacIOState {
     MacIOState parent_obj;
     /*< public >*/
 
-    HeathrowState *pic;
+    HeathrowState pic;
 
     MacIONVRAMState nvram;
     MACIOIDEState ide[2];
-- 
2.20.1

[PATCH v2 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine

[Mark Cave-Ayland]

In order to move the OpenPIC device to the macio device, the PCI bus needs to be
initialised before the macio device and also before wiring the OpenPIC IRQs.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_newworld.c | 46 ++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index c0accda592..708bb2f1ab 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -139,6 +139,7 @@ static void ppc_core99_init(MachineState *machine)
     int machine_arch;
     SysBusDevice *s;
     DeviceState *dev, *pic_dev;
+    DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
     hwaddr nvram_addr = 0xFFF04000;
     uint64_t tbfreq;
     unsigned int smp_cpus = machine->smp.cpus;
@@ -320,35 +321,24 @@ static void ppc_core99_init(MachineState *machine)
         sysbus_mmio_map(s, 0, 0xf0800000);
         sysbus_mmio_map(s, 1, 0xf0c00000);
 
-        for (i = 0; i < 4; i++) {
-            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-        }
-
         machine_arch = ARCH_MAC99_U3;
     } else {
         /* Use values found on a real PowerMac */
         /* Uninorth AGP bus */
-        dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
-        s = SYS_BUS_DEVICE(dev);
+        uninorth_agp_dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
+        s = SYS_BUS_DEVICE(uninorth_agp_dev);
         sysbus_realize_and_unref(s, &error_fatal);
         sysbus_mmio_map(s, 0, 0xf0800000);
         sysbus_mmio_map(s, 1, 0xf0c00000);
 
-        for (i = 0; i < 4; i++) {
-            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-        }
-
         /* Uninorth internal bus */
-        dev = qdev_new(TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
-        s = SYS_BUS_DEVICE(dev);
+        uninorth_internal_dev = qdev_new(
+                                TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
+        s = SYS_BUS_DEVICE(uninorth_internal_dev);
         sysbus_realize_and_unref(s, &error_fatal);
         sysbus_mmio_map(s, 0, 0xf4800000);
         sysbus_mmio_map(s, 1, 0xf4c00000);
 
-        for (i = 0; i < 4; i++) {
-            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-        }
-
         /* Uninorth main bus */
         dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
         qdev_prop_set_uint32(dev, "ofw-addr", 0xf2000000);
@@ -364,10 +354,6 @@ static void ppc_core99_init(MachineState *machine)
         sysbus_mmio_map(s, 0, 0xf2800000);
         sysbus_mmio_map(s, 1, 0xf2c00000);
 
-        for (i = 0; i < 4; i++) {
-            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-        }
-
         machine_arch = ARCH_MAC99;
     }
 
@@ -401,6 +387,26 @@ static void ppc_core99_init(MachineState *machine)
 
     pci_realize_and_unref(macio, pci_bus, &error_fatal);
 
+    for (i = 0; i < 4; i++) {
+        qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
+                              qdev_get_gpio_in(pic_dev, 0x1b + i));
+    }
+
+    /* TODO: additional PCI buses only wired up for 32-bit machines */
+    if (PPC_INPUT(env) != PPC_FLAGS_INPUT_970) {
+        /* Uninorth AGP bus */
+        for (i = 0; i < 4; i++) {
+            qdev_connect_gpio_out(uninorth_agp_dev, i,
+                                  qdev_get_gpio_in(pic_dev, 0x1b + i));
+        }
+
+        /* Uninorth internal bus */
+        for (i = 0; i < 4; i++) {
+            qdev_connect_gpio_out(uninorth_internal_dev, i,
+                                  qdev_get_gpio_in(pic_dev, 0x1b + i));
+        }
+    }
+
     /* We only emulate 2 out of 3 IDE controllers for now */
     ide_drive_get(hd, ARRAY_SIZE(hd));
 
-- 
2.20.1

[PATCH v2 5/7] macio: move OpenPIC inside macio-newworld device

[Mark Cave-Ayland]

The OpenPIC device is located within the macio device on real hardware so make it
a child of the macio-newworld device. This also removes the need for setting and
checking a separate PIC object property link on the macio-newworld device which
currently causes the automated QOM introspection tests to fail.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/macio.c         | 19 +++++++++----------
 hw/ppc/mac_newworld.c         | 25 +++++++++++--------------
 include/hw/misc/macio/macio.h |  2 +-
 3 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index cfb87da6c9..36be77cede 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -273,7 +273,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
 {
     MacIOState *s = MACIO(d);
     NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
-    DeviceState *pic_dev = DEVICE(ns->pic);
+    DeviceState *pic_dev = DEVICE(&ns->pic);
     Error *err = NULL;
     SysBusDevice *sysbus_dev;
     MemoryRegion *timer_memory = NULL;
@@ -284,17 +284,19 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
         return;
     }
 
+    /* OpenPIC */
+    qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
+    sysbus_dev = SYS_BUS_DEVICE(&ns->pic);
+    sysbus_realize_and_unref(sysbus_dev, &error_fatal);
+    memory_region_add_subregion(&s->bar, 0x40000,
+                                sysbus_mmio_get_region(sysbus_dev, 0));
+
     sysbus_dev = SYS_BUS_DEVICE(&s->escc);
     sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
                                                        NEWWORLD_ESCCB_IRQ));
     sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
                                                        NEWWORLD_ESCCA_IRQ));
 
-    /* OpenPIC */
-    sysbus_dev = SYS_BUS_DEVICE(ns->pic);
-    memory_region_add_subregion(&s->bar, 0x40000,
-                                sysbus_mmio_get_region(sysbus_dev, 0));
-
     /* IDE buses */
     macio_realize_ide(s, &ns->ide[0],
                       qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
@@ -369,10 +371,7 @@ static void macio_newworld_init(Object *obj)
     NewWorldMacIOState *ns = NEWWORLD_MACIO(obj);
     int i;
 
-    object_property_add_link(obj, "pic", TYPE_OPENPIC,
-                             (Object **) &ns->pic,
-                             qdev_prop_allow_set_link_before_realize,
-                             0);
+    object_initialize_child(OBJECT(s), "pic", &ns->pic, TYPE_OPENPIC);
 
     object_initialize_child(OBJECT(s), "gpio", &ns->gpio, TYPE_MACIO_GPIO);
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 708bb2f1ab..e991db4add 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -293,18 +293,6 @@ static void ppc_core99_init(MachineState *machine)
         }
     }
 
-    pic_dev = qdev_new(TYPE_OPENPIC);
-    qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
-    s = SYS_BUS_DEVICE(pic_dev);
-    sysbus_realize_and_unref(s, &error_fatal);
-    k = 0;
-    for (i = 0; i < smp_cpus; i++) {
-        for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
-            sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
-        }
-    }
-    g_free(openpic_irqs);
-
     if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
         /* 970 gets a U3 bus */
         /* Uninorth AGP bus */
@@ -378,8 +366,6 @@ static void ppc_core99_init(MachineState *machine)
     qdev_prop_set_uint64(dev, "frequency", tbfreq);
     qdev_prop_set_bit(dev, "has-pmu", has_pmu);
     qdev_prop_set_bit(dev, "has-adb", has_adb);
-    object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
-                             &error_abort);
 
     escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
     qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
@@ -387,6 +373,7 @@ static void ppc_core99_init(MachineState *machine)
 
     pci_realize_and_unref(macio, pci_bus, &error_fatal);
 
+    pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
     for (i = 0; i < 4; i++) {
         qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
                               qdev_get_gpio_in(pic_dev, 0x1b + i));
@@ -407,6 +394,16 @@ static void ppc_core99_init(MachineState *machine)
         }
     }
 
+    /* OpenPIC */
+    s = SYS_BUS_DEVICE(pic_dev);
+    k = 0;
+    for (i = 0; i < smp_cpus; i++) {
+        for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
+            sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
+        }
+    }
+    g_free(openpic_irqs);
+
     /* We only emulate 2 out of 3 IDE controllers for now */
     ide_drive_get(hd, ARRAY_SIZE(hd));
 
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 707dfab50c..6c05f3bfd2 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -115,7 +115,7 @@ struct NewWorldMacIOState {
 
     bool has_pmu;
     bool has_adb;
-    OpenPICState *pic;
+    OpenPICState pic;
     MACIOIDEState ide[2];
     MacIOGPIOState gpio;
 };
-- 
2.20.1

[PATCH v2 6/7] macio: wire macio GPIOs to OpenPIC using sysbus IRQs

[Mark Cave-Ayland]

This both allows the wiring to be done as Ben suggested in his original comment in
gpio.c and also enables the OpenPIC object property link to be removed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/gpio.c         | 24 +++++-------------------
 hw/misc/macio/macio.c        | 12 +++++++-----
 include/hw/misc/macio/gpio.h |  2 --
 3 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index 0fef8fb335..b1bcf830c3 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -57,10 +57,7 @@ void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool state)
 
     s->gpio_regs[gpio] = new_reg;
 
-    /* This is will work until we fix the binding between MacIO and
-     * the MPIC properly so we can route all GPIOs and avoid going
-     * via the top level platform code.
-     *
+    /*
      * Note that we probably need to get access to the MPIC config to
      * decode polarity since qemu always use "raise" regardless.
      *
@@ -152,25 +149,15 @@ static const MemoryRegionOps macio_gpio_ops = {
     },
 };
 
-static void macio_gpio_realize(DeviceState *dev, Error **errp)
-{
-    MacIOGPIOState *s = MACIO_GPIO(dev);
-
-    s->gpio_extirqs[1] = qdev_get_gpio_in(DEVICE(s->pic),
-                                          NEWWORLD_EXTING_GPIO1);
-    s->gpio_extirqs[9] = qdev_get_gpio_in(DEVICE(s->pic),
-                                          NEWWORLD_EXTING_GPIO9);
-}
-
 static void macio_gpio_init(Object *obj)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
     MacIOGPIOState *s = MACIO_GPIO(obj);
+    int i;
 
-    object_property_add_link(obj, "pic", TYPE_OPENPIC,
-                             (Object **) &s->pic,
-                             qdev_prop_allow_set_link_before_realize,
-                             0);
+    for (i = 0; i < 10; i++) {
+        sysbus_init_irq(sbd, &s->gpio_extirqs[i]);
+    }
 
     memory_region_init_io(&s->gpiomem, OBJECT(s), &macio_gpio_ops, obj,
                           "gpio", 0x30);
@@ -207,7 +194,6 @@ static void macio_gpio_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
 
-    dc->realize = macio_gpio_realize;
     dc->reset = macio_gpio_reset;
     dc->vmsd = &vmstate_macio_gpio;
     nc->nmi_monitor_handler = macio_gpio_nmi;
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 36be77cede..d17cffd868 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -324,14 +324,16 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
 
     if (ns->has_pmu) {
         /* GPIOs */
-        sysbus_dev = SYS_BUS_DEVICE(&ns->gpio);
-        object_property_set_link(OBJECT(&ns->gpio), "pic", OBJECT(pic_dev),
-                                 &error_abort);
-        memory_region_add_subregion(&s->bar, 0x50,
-                                    sysbus_mmio_get_region(sysbus_dev, 0));
         if (!qdev_realize(DEVICE(&ns->gpio), BUS(&s->macio_bus), errp)) {
             return;
         }
+        sysbus_dev = SYS_BUS_DEVICE(&ns->gpio);
+        sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
+                           NEWWORLD_EXTING_GPIO1));
+        sysbus_connect_irq(sysbus_dev, 9, qdev_get_gpio_in(pic_dev,
+                           NEWWORLD_EXTING_GPIO9));
+        memory_region_add_subregion(&s->bar, 0x50,
+                                    sysbus_mmio_get_region(sysbus_dev, 0));
 
         /* PMU */
         object_initialize_child(OBJECT(s), "pmu", &s->pmu, TYPE_VIA_PMU);
diff --git a/include/hw/misc/macio/gpio.h b/include/hw/misc/macio/gpio.h
index 4dee09a9dd..7d2aa886c2 100644
--- a/include/hw/misc/macio/gpio.h
+++ b/include/hw/misc/macio/gpio.h
@@ -38,8 +38,6 @@ struct MacIOGPIOState {
     SysBusDevice parent;
     /*< public >*/
 
-    OpenPICState *pic;
-
     MemoryRegion gpiomem;
     qemu_irq gpio_extirqs[10];
     uint8_t gpio_levels[8];
-- 
2.20.1

[PATCH v2 7/7] macio: don't set user_creatable to false

[Mark Cave-Ayland]

Now that all of the object property links to the heathrow PIC and OpenPIC have
been removed from the macio devices, it is safe to allow the macio-oldworld
and macio-neworld devices to be marked as user_creatable.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/macio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index d17cffd868..e6eeb575d5 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -457,8 +457,6 @@ static void macio_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_OTHERS << 8;
     device_class_set_props(dc, macio_properties);
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-    /* Reason: requires PIC property links to be set in macio_*_realize() */
-    dc->user_creatable = false;
 }
 
 static const TypeInfo macio_bus_info = {
-- 
2.20.1

Re: [PATCH v2 2/7] mac_oldworld: move initialisation of grackle before heathrow

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2752 bytes --]

On Tue, Dec 29, 2020 at 05:56:14PM +0000, Mark Cave-Ayland wrote:
> In order to move the heathrow PIC to the macio device, the PCI bus needs to be
> initialised before the macio device and also before wiring the PIC IRQs.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/mac_oldworld.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 2ead34bdf1..e58e0525fe 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -227,6 +227,21 @@ static void ppc_heathrow_init(MachineState *machine)
>          }
>      }
>  
> +    /* Grackle PCI host bridge */
> +    dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> +    qdev_prop_set_uint32(dev, "ofw-addr", 0x80000000);
> +    s = SYS_BUS_DEVICE(dev);
> +    sysbus_realize_and_unref(s, &error_fatal);
> +
> +    sysbus_mmio_map(s, 0, GRACKLE_BASE);
> +    sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
> +    /* PCI hole */
> +    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
> +                                sysbus_mmio_get_region(s, 2));
> +    /* Register 2 MB of ISA IO space */
> +    memory_region_add_subregion(get_system_memory(), 0xfe000000,
> +                                sysbus_mmio_get_region(s, 3));
> +
>      /* XXX: we register only 1 output pin for heathrow PIC */
>      pic_dev = qdev_new(TYPE_HEATHROW);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
> @@ -251,21 +266,6 @@ static void ppc_heathrow_init(MachineState *machine)
>          tbfreq = TBFREQ;
>      }
>  
> -    /* Grackle PCI host bridge */
> -    dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> -    qdev_prop_set_uint32(dev, "ofw-addr", 0x80000000);
> -    s = SYS_BUS_DEVICE(dev);
> -    sysbus_realize_and_unref(s, &error_fatal);
> -
> -    sysbus_mmio_map(s, 0, GRACKLE_BASE);
> -    sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
> -    /* PCI hole */
> -    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
> -                                sysbus_mmio_get_region(s, 2));
> -    /* Register 2 MB of ISA IO space */
> -    memory_region_add_subregion(get_system_memory(), 0xfe000000,
> -                                sysbus_mmio_get_region(s, 3));
> -
>      for (i = 0; i < 4; i++) {
>          qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
>      }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 3/7] macio: move heathrow PIC inside macio-oldworld device

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8189 bytes --]

On Tue, Dec 29, 2020 at 05:56:15PM +0000, Mark Cave-Ayland wrote:
> The heathrow PIC is located within the macio device on real hardware so make it
> a child of the macio-oldworld device. This also removes the need for setting and
> checking a separate PIC object property link on the macio-oldworld device which
> currently causes the automated QOM introspection tests to fail.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/misc/macio/macio.c         | 20 +++++------
>  hw/ppc/mac_oldworld.c         | 66 +++++++++++++++++------------------
>  include/hw/misc/macio/macio.h |  2 +-
>  3 files changed, 43 insertions(+), 45 deletions(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index bb601f782c..cfb87da6c9 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -140,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>  {
>      MacIOState *s = MACIO(d);
>      OldWorldMacIOState *os = OLDWORLD_MACIO(d);
> -    DeviceState *pic_dev = DEVICE(os->pic);
> +    DeviceState *pic_dev = DEVICE(&os->pic);
>      Error *err = NULL;
>      SysBusDevice *sysbus_dev;
>  
> @@ -150,6 +150,14 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>          return;
>      }
>  
> +    /* Heathrow PIC */
> +    if (!qdev_realize(DEVICE(&os->pic), BUS(&s->macio_bus), errp)) {
> +        return;
> +    }
> +    sysbus_dev = SYS_BUS_DEVICE(&os->pic);
> +    memory_region_add_subregion(&s->bar, 0x0,
> +                                sysbus_mmio_get_region(sysbus_dev, 0));
> +
>      qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
>                           s->frequency);
>      if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
> @@ -175,11 +183,6 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>                                  sysbus_mmio_get_region(sysbus_dev, 0));
>      pmac_format_nvram_partition(&os->nvram, os->nvram.size);
>  
> -    /* Heathrow PIC */
> -    sysbus_dev = SYS_BUS_DEVICE(os->pic);
> -    memory_region_add_subregion(&s->bar, 0x0,
> -                                sysbus_mmio_get_region(sysbus_dev, 0));
> -
>      /* IDE buses */
>      macio_realize_ide(s, &os->ide[0],
>                        qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
> @@ -218,10 +221,7 @@ static void macio_oldworld_init(Object *obj)
>      DeviceState *dev;
>      int i;
>  
> -    object_property_add_link(obj, "pic", TYPE_HEATHROW,
> -                             (Object **) &os->pic,
> -                             qdev_prop_allow_set_link_before_realize,
> -                             0);
> +    object_initialize_child(OBJECT(s), "pic", &os->pic, TYPE_HEATHROW);
>  
>      object_initialize_child(OBJECT(s), "cuda", &s->cuda, TYPE_CUDA);
>  
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index e58e0525fe..44ee99be88 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -98,7 +98,7 @@ static void ppc_heathrow_init(MachineState *machine)
>      MACIOIDEState *macio_ide;
>      ESCCState *escc;
>      SysBusDevice *s;
> -    DeviceState *dev, *pic_dev;
> +    DeviceState *dev, *pic_dev, *grackle_dev;
>      BusState *adb_bus;
>      uint64_t bios_addr;
>      int bios_size;
> @@ -227,10 +227,17 @@ static void ppc_heathrow_init(MachineState *machine)
>          }
>      }
>  
> +    /* Timebase Frequency */
> +    if (kvm_enabled()) {
> +        tbfreq = kvmppc_get_tbfreq();
> +    } else {
> +        tbfreq = TBFREQ;
> +    }
> +
>      /* Grackle PCI host bridge */
> -    dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> -    qdev_prop_set_uint32(dev, "ofw-addr", 0x80000000);
> -    s = SYS_BUS_DEVICE(dev);
> +    grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> +    qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
> +    s = SYS_BUS_DEVICE(grackle_dev);
>      sysbus_realize_and_unref(s, &error_fatal);
>  
>      sysbus_mmio_map(s, 0, GRACKLE_BASE);
> @@ -242,14 +249,30 @@ static void ppc_heathrow_init(MachineState *machine)
>      memory_region_add_subregion(get_system_memory(), 0xfe000000,
>                                  sysbus_mmio_get_region(s, 3));
>  
> -    /* XXX: we register only 1 output pin for heathrow PIC */
> -    pic_dev = qdev_new(TYPE_HEATHROW);
> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
> +    pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
> +
> +    /* MacIO */
> +    macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
> +    dev = DEVICE(macio);
> +    qdev_prop_set_uint64(dev, "frequency", tbfreq);
> +
> +    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
> +    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> +    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
> +
> +    pci_realize_and_unref(macio, pci_bus, &error_fatal);
> +
> +    pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
> +    for (i = 0; i < 4; i++) {
> +        qdev_connect_gpio_out(grackle_dev, i,
> +                              qdev_get_gpio_in(pic_dev, 0x15 + i));
> +    }
>  
>      /* Connect the heathrow PIC outputs to the 6xx bus */
>      for (i = 0; i < smp_cpus; i++) {
>          switch (PPC_INPUT(env)) {
>          case PPC_FLAGS_INPUT_6xx:
> +            /* XXX: we register only 1 output pin for heathrow PIC */
>              qdev_connect_gpio_out(pic_dev, 0,
>                  ((qemu_irq *)env->irq_inputs)[PPC6xx_INPUT_INT]);
>              break;
> @@ -259,40 +282,14 @@ static void ppc_heathrow_init(MachineState *machine)
>          }
>      }
>  
> -    /* Timebase Frequency */
> -    if (kvm_enabled()) {
> -        tbfreq = kvmppc_get_tbfreq();
> -    } else {
> -        tbfreq = TBFREQ;
> -    }
> -
> -    for (i = 0; i < 4; i++) {
> -        qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
> -    }
> -
> -    pci_bus = PCI_HOST_BRIDGE(dev)->bus;
> -
>      pci_vga_init(pci_bus);
>  
>      for (i = 0; i < nb_nics; i++) {
>          pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
>      }
>  
> +    /* MacIO IDE */
>      ide_drive_get(hd, ARRAY_SIZE(hd));
> -
> -    /* MacIO */
> -    macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
> -    dev = DEVICE(macio);
> -    qdev_prop_set_uint64(dev, "frequency", tbfreq);
> -    object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
> -                             &error_abort);
> -
> -    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
> -    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> -    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
> -
> -    pci_realize_and_unref(macio, pci_bus, &error_fatal);
> -
>      macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
>                                                          "ide[0]"));
>      macio_ide_init_drives(macio_ide, hd);
> @@ -301,6 +298,7 @@ static void ppc_heathrow_init(MachineState *machine)
>                                                          "ide[1]"));
>      macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]);
>  
> +    /* MacIO CUDA/ADB */
>      dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda"));
>      adb_bus = qdev_get_child_bus(dev, "adb.0");
>      dev = qdev_new(TYPE_ADB_KEYBOARD);
> diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
> index 22b4e64b2c..707dfab50c 100644
> --- a/include/hw/misc/macio/macio.h
> +++ b/include/hw/misc/macio/macio.h
> @@ -99,7 +99,7 @@ struct OldWorldMacIOState {
>      MacIOState parent_obj;
>      /*< public >*/
>  
> -    HeathrowState *pic;
> +    HeathrowState pic;
>  
>      MacIONVRAMState nvram;
>      MACIOIDEState ide[2];

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4384 bytes --]

On Tue, Dec 29, 2020 at 05:56:16PM +0000, Mark Cave-Ayland wrote:
> In order to move the OpenPIC device to the macio device, the PCI bus needs to be
> initialised before the macio device and also before wiring the OpenPIC IRQs.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/mac_newworld.c | 46 ++++++++++++++++++++++++-------------------
>  1 file changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index c0accda592..708bb2f1ab 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -139,6 +139,7 @@ static void ppc_core99_init(MachineState *machine)
>      int machine_arch;
>      SysBusDevice *s;
>      DeviceState *dev, *pic_dev;
> +    DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
>      hwaddr nvram_addr = 0xFFF04000;
>      uint64_t tbfreq;
>      unsigned int smp_cpus = machine->smp.cpus;
> @@ -320,35 +321,24 @@ static void ppc_core99_init(MachineState *machine)
>          sysbus_mmio_map(s, 0, 0xf0800000);
>          sysbus_mmio_map(s, 1, 0xf0c00000);
>  
> -        for (i = 0; i < 4; i++) {
> -            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
> -        }
> -
>          machine_arch = ARCH_MAC99_U3;
>      } else {
>          /* Use values found on a real PowerMac */
>          /* Uninorth AGP bus */
> -        dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
> -        s = SYS_BUS_DEVICE(dev);
> +        uninorth_agp_dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
> +        s = SYS_BUS_DEVICE(uninorth_agp_dev);
>          sysbus_realize_and_unref(s, &error_fatal);
>          sysbus_mmio_map(s, 0, 0xf0800000);
>          sysbus_mmio_map(s, 1, 0xf0c00000);
>  
> -        for (i = 0; i < 4; i++) {
> -            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
> -        }
> -
>          /* Uninorth internal bus */
> -        dev = qdev_new(TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
> -        s = SYS_BUS_DEVICE(dev);
> +        uninorth_internal_dev = qdev_new(
> +                                TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
> +        s = SYS_BUS_DEVICE(uninorth_internal_dev);
>          sysbus_realize_and_unref(s, &error_fatal);
>          sysbus_mmio_map(s, 0, 0xf4800000);
>          sysbus_mmio_map(s, 1, 0xf4c00000);
>  
> -        for (i = 0; i < 4; i++) {
> -            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
> -        }
> -
>          /* Uninorth main bus */
>          dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
>          qdev_prop_set_uint32(dev, "ofw-addr", 0xf2000000);
> @@ -364,10 +354,6 @@ static void ppc_core99_init(MachineState *machine)
>          sysbus_mmio_map(s, 0, 0xf2800000);
>          sysbus_mmio_map(s, 1, 0xf2c00000);
>  
> -        for (i = 0; i < 4; i++) {
> -            qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
> -        }
> -
>          machine_arch = ARCH_MAC99;
>      }
>  
> @@ -401,6 +387,26 @@ static void ppc_core99_init(MachineState *machine)
>  
>      pci_realize_and_unref(macio, pci_bus, &error_fatal);
>  
> +    for (i = 0; i < 4; i++) {
> +        qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
> +                              qdev_get_gpio_in(pic_dev, 0x1b + i));
> +    }
> +
> +    /* TODO: additional PCI buses only wired up for 32-bit machines */
> +    if (PPC_INPUT(env) != PPC_FLAGS_INPUT_970) {
> +        /* Uninorth AGP bus */
> +        for (i = 0; i < 4; i++) {
> +            qdev_connect_gpio_out(uninorth_agp_dev, i,
> +                                  qdev_get_gpio_in(pic_dev, 0x1b + i));
> +        }
> +
> +        /* Uninorth internal bus */
> +        for (i = 0; i < 4; i++) {
> +            qdev_connect_gpio_out(uninorth_internal_dev, i,
> +                                  qdev_get_gpio_in(pic_dev, 0x1b + i));
> +        }
> +    }
> +
>      /* We only emulate 2 out of 3 IDE controllers for now */
>      ide_drive_get(hd, ARRAY_SIZE(hd));
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 5/7] macio: move OpenPIC inside macio-newworld device

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5814 bytes --]

On Tue, Dec 29, 2020 at 05:56:17PM +0000, Mark Cave-Ayland wrote:
> The OpenPIC device is located within the macio device on real hardware so make it
> a child of the macio-newworld device. This also removes the need for setting and
> checking a separate PIC object property link on the macio-newworld device which
> currently causes the automated QOM introspection tests to fail.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/misc/macio/macio.c         | 19 +++++++++----------
>  hw/ppc/mac_newworld.c         | 25 +++++++++++--------------
>  include/hw/misc/macio/macio.h |  2 +-
>  3 files changed, 21 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index cfb87da6c9..36be77cede 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -273,7 +273,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
>  {
>      MacIOState *s = MACIO(d);
>      NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
> -    DeviceState *pic_dev = DEVICE(ns->pic);
> +    DeviceState *pic_dev = DEVICE(&ns->pic);
>      Error *err = NULL;
>      SysBusDevice *sysbus_dev;
>      MemoryRegion *timer_memory = NULL;
> @@ -284,17 +284,19 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
>          return;
>      }
>  
> +    /* OpenPIC */
> +    qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
> +    sysbus_dev = SYS_BUS_DEVICE(&ns->pic);
> +    sysbus_realize_and_unref(sysbus_dev, &error_fatal);
> +    memory_region_add_subregion(&s->bar, 0x40000,
> +                                sysbus_mmio_get_region(sysbus_dev, 0));
> +
>      sysbus_dev = SYS_BUS_DEVICE(&s->escc);
>      sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
>                                                         NEWWORLD_ESCCB_IRQ));
>      sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
>                                                         NEWWORLD_ESCCA_IRQ));
>  
> -    /* OpenPIC */
> -    sysbus_dev = SYS_BUS_DEVICE(ns->pic);
> -    memory_region_add_subregion(&s->bar, 0x40000,
> -                                sysbus_mmio_get_region(sysbus_dev, 0));
> -
>      /* IDE buses */
>      macio_realize_ide(s, &ns->ide[0],
>                        qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
> @@ -369,10 +371,7 @@ static void macio_newworld_init(Object *obj)
>      NewWorldMacIOState *ns = NEWWORLD_MACIO(obj);
>      int i;
>  
> -    object_property_add_link(obj, "pic", TYPE_OPENPIC,
> -                             (Object **) &ns->pic,
> -                             qdev_prop_allow_set_link_before_realize,
> -                             0);
> +    object_initialize_child(OBJECT(s), "pic", &ns->pic, TYPE_OPENPIC);
>  
>      object_initialize_child(OBJECT(s), "gpio", &ns->gpio, TYPE_MACIO_GPIO);
>  
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 708bb2f1ab..e991db4add 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -293,18 +293,6 @@ static void ppc_core99_init(MachineState *machine)
>          }
>      }
>  
> -    pic_dev = qdev_new(TYPE_OPENPIC);
> -    qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
> -    s = SYS_BUS_DEVICE(pic_dev);
> -    sysbus_realize_and_unref(s, &error_fatal);
> -    k = 0;
> -    for (i = 0; i < smp_cpus; i++) {
> -        for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
> -            sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
> -        }
> -    }
> -    g_free(openpic_irqs);
> -
>      if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
>          /* 970 gets a U3 bus */
>          /* Uninorth AGP bus */
> @@ -378,8 +366,6 @@ static void ppc_core99_init(MachineState *machine)
>      qdev_prop_set_uint64(dev, "frequency", tbfreq);
>      qdev_prop_set_bit(dev, "has-pmu", has_pmu);
>      qdev_prop_set_bit(dev, "has-adb", has_adb);
> -    object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
> -                             &error_abort);
>  
>      escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
>      qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> @@ -387,6 +373,7 @@ static void ppc_core99_init(MachineState *machine)
>  
>      pci_realize_and_unref(macio, pci_bus, &error_fatal);
>  
> +    pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
>      for (i = 0; i < 4; i++) {
>          qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
>                                qdev_get_gpio_in(pic_dev, 0x1b + i));
> @@ -407,6 +394,16 @@ static void ppc_core99_init(MachineState *machine)
>          }
>      }
>  
> +    /* OpenPIC */
> +    s = SYS_BUS_DEVICE(pic_dev);
> +    k = 0;
> +    for (i = 0; i < smp_cpus; i++) {
> +        for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
> +            sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
> +        }
> +    }
> +    g_free(openpic_irqs);
> +
>      /* We only emulate 2 out of 3 IDE controllers for now */
>      ide_drive_get(hd, ARRAY_SIZE(hd));
>  
> diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
> index 707dfab50c..6c05f3bfd2 100644
> --- a/include/hw/misc/macio/macio.h
> +++ b/include/hw/misc/macio/macio.h
> @@ -115,7 +115,7 @@ struct NewWorldMacIOState {
>  
>      bool has_pmu;
>      bool has_adb;
> -    OpenPICState *pic;
> +    OpenPICState pic;
>      MACIOIDEState ide[2];
>      MacIOGPIOState gpio;
>  };

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/7] macio: remove PIC object property links

[Mark Cave-Ayland]

On 29/12/2020 17:56, Mark Cave-Ayland wrote:

> This patchset follows on from the dicussion at https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02630.html
> where the user_creatable flag for the macio devices was set back to false just
> before the 5.2 release.
> 
> The underlying cause was that the PIC object property links were not being set
> before realise. Whilst this cannot happen when launching the g3beige and mac99
> machines from qemu-system-ppc, it caused some automated tests to fail.
> 
> Here we fix the real problem which is to move the PIC for both machines into the
> macio device, which not only matches real hardware but also enables the PIC object
> property links to be completely removed.
> 
> Patch 6 rewires the macio gpios for the mac99 machine as per Ben's original comment
> after the OpenPIC device has been moved into the macio-newworld device, and then
> finally patch 7 removes setting the user_creatable flag to false on the macio devices
> once again.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> v2:
> - Add R-B tag for patch 1 from David
> - Update commit messages to included more detail as requested by David
> 
> 
> Mark Cave-Ayland (7):
>    mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
>    mac_oldworld: move initialisation of grackle before heathrow
>    macio: move heathrow PIC inside macio-oldworld device
>    mac_newworld: delay wiring of PCI IRQs in New World machine
>    macio: move OpenPIC inside macio-newworld device
>    macio: wire macio GPIOs to OpenPIC using sysbus IRQs
>    macio: don't set user_creatable to false
> 
>   hw/misc/macio/gpio.c          | 24 +++--------
>   hw/misc/macio/macio.c         | 53 ++++++++++++------------
>   hw/ppc/mac_newworld.c         | 71 ++++++++++++++++----------------
>   hw/ppc/mac_oldworld.c         | 76 ++++++++++++++++-------------------
>   include/hw/misc/macio/gpio.h  |  2 -
>   include/hw/misc/macio/macio.h |  4 +-
>   6 files changed, 104 insertions(+), 126 deletions(-)

Any further thoughts on this? David has reviewed patches 1-5 but I've had no other 
feedback to date. If everyone is happy then I don't mind including this in a 
qemu-macppc PR.


ATB,

Mark.

Re: [PATCH v2 0/7] macio: remove PIC object property links

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]

On Thu, Jan 07, 2021 at 02:26:33PM +0000, Mark Cave-Ayland wrote:
> On 29/12/2020 17:56, Mark Cave-Ayland wrote:
> 
> > This patchset follows on from the dicussion at https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02630.html
> > where the user_creatable flag for the macio devices was set back to false just
> > before the 5.2 release.
> > 
> > The underlying cause was that the PIC object property links were not being set
> > before realise. Whilst this cannot happen when launching the g3beige and mac99
> > machines from qemu-system-ppc, it caused some automated tests to fail.
> > 
> > Here we fix the real problem which is to move the PIC for both machines into the
> > macio device, which not only matches real hardware but also enables the PIC object
> > property links to be completely removed.
> > 
> > Patch 6 rewires the macio gpios for the mac99 machine as per Ben's original comment
> > after the OpenPIC device has been moved into the macio-newworld device, and then
> > finally patch 7 removes setting the user_creatable flag to false on the macio devices
> > once again.
> > 
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > 
> > v2:
> > - Add R-B tag for patch 1 from David
> > - Update commit messages to included more detail as requested by David
> > 
> > 
> > Mark Cave-Ayland (7):
> >    mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
> >    mac_oldworld: move initialisation of grackle before heathrow
> >    macio: move heathrow PIC inside macio-oldworld device
> >    mac_newworld: delay wiring of PCI IRQs in New World machine
> >    macio: move OpenPIC inside macio-newworld device
> >    macio: wire macio GPIOs to OpenPIC using sysbus IRQs
> >    macio: don't set user_creatable to false
> > 
> >   hw/misc/macio/gpio.c          | 24 +++--------
> >   hw/misc/macio/macio.c         | 53 ++++++++++++------------
> >   hw/ppc/mac_newworld.c         | 71 ++++++++++++++++----------------
> >   hw/ppc/mac_oldworld.c         | 76 ++++++++++++++++-------------------
> >   include/hw/misc/macio/gpio.h  |  2 -
> >   include/hw/misc/macio/macio.h |  4 +-
> >   6 files changed, 104 insertions(+), 126 deletions(-)
> 
> Any further thoughts on this? David has reviewed patches 1-5 but I've had no
> other feedback to date. If everyone is happy then I don't mind including
> this in a qemu-macppc PR.

I'm happy for you to do that.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]