qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] macio: remove PIC object property links
@ 2020-12-19 10:42 Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env) Mark Cave-Ayland
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

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>


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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
  2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
@ 2020-12-19 10:42 ` Mark Cave-Ayland
  2020-12-28  7:06   ` David Gibson
  2020-12-19 10:42 ` [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow Mark Cave-Ayland
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

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>
---
 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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow
  2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env) Mark Cave-Ayland
@ 2020-12-19 10:42 ` Mark Cave-Ayland
  2020-12-28  7:07   ` David Gibson
  2020-12-19 10:42 ` [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device Mark Cave-Ayland
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device
  2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env) Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow Mark Cave-Ayland
@ 2020-12-19 10:42 ` Mark Cave-Ayland
  2020-12-28  7:08   ` David Gibson
  2020-12-19 10:42 ` [PATCH 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine Mark Cave-Ayland
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine
  2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
                   ` (2 preceding siblings ...)
  2020-12-19 10:42 ` [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device Mark Cave-Ayland
@ 2020-12-19 10:42 ` Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 5/7] macio: move OpenPIC inside macio-newworld device Mark Cave-Ayland
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

This is to prepare for moving the OpenPIC device into the macio-newworld
device.

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/7] macio: move OpenPIC inside macio-newworld device
  2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
                   ` (3 preceding siblings ...)
  2020-12-19 10:42 ` [PATCH 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine Mark Cave-Ayland
@ 2020-12-19 10:42 ` Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 6/7] macio: wire macio GPIOs to OpenPIC using sysbus IRQs Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 7/7] macio: don't set user_creatable to false Mark Cave-Ayland
  6 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/7] macio: wire macio GPIOs to OpenPIC using sysbus IRQs
  2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
                   ` (4 preceding siblings ...)
  2020-12-19 10:42 ` [PATCH 5/7] macio: move OpenPIC inside macio-newworld device Mark Cave-Ayland
@ 2020-12-19 10:42 ` Mark Cave-Ayland
  2020-12-19 10:42 ` [PATCH 7/7] macio: don't set user_creatable to false Mark Cave-Ayland
  6 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 7/7] macio: don't set user_creatable to false
  2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
                   ` (5 preceding siblings ...)
  2020-12-19 10:42 ` [PATCH 6/7] macio: wire macio GPIOs to OpenPIC using sysbus IRQs Mark Cave-Ayland
@ 2020-12-19 10:42 ` Mark Cave-Ayland
  6 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-19 10:42 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, david, thuth

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
  2020-12-19 10:42 ` [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env) Mark Cave-Ayland
@ 2020-12-28  7:06   ` David Gibson
  0 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2020-12-28  7:06 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: thuth, qemu-ppc, qemu-devel

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

On Sat, Dec 19, 2020 at 10:42:23AM +0000, Mark Cave-Ayland wrote:
> 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);

-- 
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 --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow
  2020-12-19 10:42 ` [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow Mark Cave-Ayland
@ 2020-12-28  7:07   ` David Gibson
  2020-12-29 14:07     ` Mark Cave-Ayland
  0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2020-12-28  7:07 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: thuth, qemu-ppc, qemu-devel

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

On Sat, Dec 19, 2020 at 10:42:24AM +0000, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Looks correct, but it could really do with a rationale in the commit message.

> ---
>  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 --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device
  2020-12-19 10:42 ` [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device Mark Cave-Ayland
@ 2020-12-28  7:08   ` David Gibson
  2020-12-29 14:12     ` Mark Cave-Ayland
  0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2020-12-28  7:08 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: thuth, qemu-ppc, qemu-devel

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

On Sat, Dec 19, 2020 at 10:42:25AM +0000, Mark Cave-Ayland wrote:

Really needs a commit message.

> 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];

-- 
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 --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow
  2020-12-28  7:07   ` David Gibson
@ 2020-12-29 14:07     ` Mark Cave-Ayland
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-29 14:07 UTC (permalink / raw)
  To: David Gibson; +Cc: thuth, qemu-ppc, qemu-devel

On 28/12/2020 07:07, David Gibson wrote:

> On Sat, Dec 19, 2020 at 10:42:24AM +0000, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> Looks correct, but it could really do with a rationale in the commit message.

It's just changing the order of initialisation since as the plan is to move the PIC 
to the macio device, the PCI bus needs to exist before the macio device is initiated 
and also before wiring up to the PIC. I'll come up with something along those lines 
for a v2.

>> ---
>>   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));
>>       }


ATB,

Mark.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device
  2020-12-28  7:08   ` David Gibson
@ 2020-12-29 14:12     ` Mark Cave-Ayland
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2020-12-29 14:12 UTC (permalink / raw)
  To: David Gibson; +Cc: thuth, qemu-ppc, qemu-devel

On 28/12/2020 07:08, David Gibson wrote:

> On Sat, Dec 19, 2020 at 10:42:25AM +0000, Mark Cave-Ayland wrote:
> 
> Really needs a commit message.

This is currently explained in the cover letter: it's moving the PIC to the macio 
device as per real hardware (which also nicely removes the need for compulsory object 
property links which currently trip up some of the automated QOM introspection 
tests). I'll add something along these lines for the next revision.

>> 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];

ATB,

Mark.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-12-29 14:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-19 10:42 [PATCH 0/7] macio: remove PIC object property links Mark Cave-Ayland
2020-12-19 10:42 ` [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env) Mark Cave-Ayland
2020-12-28  7:06   ` David Gibson
2020-12-19 10:42 ` [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow Mark Cave-Ayland
2020-12-28  7:07   ` David Gibson
2020-12-29 14:07     ` Mark Cave-Ayland
2020-12-19 10:42 ` [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device Mark Cave-Ayland
2020-12-28  7:08   ` David Gibson
2020-12-29 14:12     ` Mark Cave-Ayland
2020-12-19 10:42 ` [PATCH 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine Mark Cave-Ayland
2020-12-19 10:42 ` [PATCH 5/7] macio: move OpenPIC inside macio-newworld device Mark Cave-Ayland
2020-12-19 10:42 ` [PATCH 6/7] macio: wire macio GPIOs to OpenPIC using sysbus IRQs Mark Cave-Ayland
2020-12-19 10:42 ` [PATCH 7/7] macio: don't set user_creatable to false Mark Cave-Ayland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).