[PATCH 00/42] Consolidate PIIX south bridges

[PATCH 00/42] Consolidate PIIX south bridges

[Bernhard Beschow]

This series consolidates the implementations of the PIIX3 and PIIX4 south
bridges and is an extended version of [1]. The motivation is to share as much
code as possible and to bring both device models to feature parity such that
perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This
could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this
list before.

The series is structured as follows: First, PIIX3 is changed to instantiate
internal devices itself, like PIIX4 does already. Second, PIIX3 gets prepared
for the merge with PIIX4 which includes some fixes, cleanups, and renamings.
Third, the same is done for PIIX4. In step four the implementations are merged.
Since some consolidations could be done easier with merged implementations, the
consolidation continues in step five which concludes the series.

One particular challenge in this series was that the PIC of PIIX3 used to be
instantiated outside of the south bridge while some sub functions require a PIC
with populated qemu_irqs. This has been solved by introducing a proxy PIC which
furthermore allows PIIX3 to be agnostic towards the virtualization technology
used (KVM, TCG, Xen). Due to consolidation PIIX4 gained the PIC as well,
possibly allowing the Malta board to gain KVM capabilities in the future.

Another challenge was dealing with optional devices where Peter already gave
advice in [1] which this series implements.

An unsolved problem still is PCI interrupt handling. The first function
passed to pci_bus_irqs() is device-specific while the second one seems
board-specific. This causes both PIIX device models to be coupled to a
particular board. Any advice how to resolve this would be highly appreaciated.

Last but not least there might be some opportunity to consolidate VM state
handling, probably by reusing the one from PIIX3. Since I'm not very familiar
with the requirements I didn't touch it so far.

Testing done:
* make check
* Boot live CD:
  * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom
manjaro-kde-21.3.2-220704-linux515.iso`
  * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom
manjaro-kde-21.3.2-220704-linux515.iso`

[1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html

Bernhard Beschow (42):
  hw/i386/pc: Create DMA controllers in south bridges
  hw/i386/pc: Create RTC controllers in south bridges
  hw/i386/pc: No need for rtc_state to be an out-parameter
  hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
    south bridge
  hw/isa/piix3: Create USB controller in host device
  hw/isa/piix3: Create power management controller in host device
  hw/intc/i8259: Introduce i8259 proxy "isa-pic"
  hw/isa/piix3: Create ISA PIC in host device
  hw/isa/piix3: Create IDE controller in host device
  hw/isa/piix3: Wire up ACPI interrupt internally
  hw/isa/piix3: Remove extra ';' outside of functions
  hw/isa/piix3: Remove unused include
  hw/isa/piix3: Add size constraints to rcr_ops
  hw/isa/piix3: Modernize reset handling
  hw/isa/piix3: Prefer pci_address_space() over get_system_memory()
  hw/isa/piix3: Allow board to provide PCI interrupt routes
  hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
  hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4
  hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4
  hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"
  hw/isa/piix3: Rename typedef PIIX3State to PIIXState
  hw/mips/malta: Reuse dev variable
  meson: Fix dependencies of piix4 southbridge
  hw/isa/piix4: Add missing initialization
  hw/isa/piix4: Move pci_ide_create_devs() call to board code
  hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
  hw/isa/piix4: Allow board to provide PCI interrupt routes
  hw/isa/piix4: Remove unused code
  hw/isa/piix4: Use ISA PIC device
  hw/isa/piix4: Reuse struct PIIXState from PIIX3
  hw/isa/piix4: Rename reset control operations to match PIIX3
  hw/isa/piix4: Rename wrongly named method
  hw/isa/piix4: Prefix pci_slot_get_pirq() with "piix4_"
  hw/isa/piix3: Merge hw/isa/piix4.c
  hw/isa/piix: Harmonize names of reset control memory regions
  hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
  hw/isa/piix: Rename functions to be shared for interrupt triggering
  hw/isa/piix: Consolidate IRQ triggering
  hw/isa/piix: Unexport PIIXState
  hw/isa/piix: Share PIIX3 base class with PIIX4
  hw/isa/piix: Drop the "3" from the PIIX base class
  hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI
    controller

 MAINTAINERS                             |   6 +-
 configs/devices/mips-softmmu/common.mak |   3 +-
 hw/i386/Kconfig                         |   3 +-
 hw/i386/acpi-build.c                    |   4 +-
 hw/i386/pc.c                            |  19 +-
 hw/i386/pc_piix.c                       |  72 +--
 hw/i386/pc_q35.c                        |   3 +-
 hw/intc/i8259.c                         |  27 +
 hw/isa/Kconfig                          |  14 +-
 hw/isa/lpc_ich9.c                       |  11 +
 hw/isa/meson.build                      |   3 +-
 hw/isa/piix.c                           | 669 ++++++++++++++++++++++++
 hw/isa/piix3.c                          | 431 ---------------
 hw/isa/piix4.c                          | 325 ------------
 hw/mips/malta.c                         |  34 +-
 include/hw/i386/ich9.h                  |   2 +
 include/hw/i386/pc.h                    |   2 +-
 include/hw/intc/i8259.h                 |  14 +
 include/hw/southbridge/piix.h           |  41 +-
 19 files changed, 823 insertions(+), 860 deletions(-)
 create mode 100644 hw/isa/piix.c
 delete mode 100644 hw/isa/piix3.c
 delete mode 100644 hw/isa/piix4.c

-- 
2.37.3

[PATCH 01/42] hw/i386/pc: Create DMA controllers in south bridges

[Bernhard Beschow]

Just like in the real hardware (and in PIIX4), create the DMA
controllers in the south bridges.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc.c      | 3 ---
 hw/i386/pc_piix.c | 2 ++
 hw/isa/Kconfig    | 2 ++
 hw/isa/lpc_ich9.c | 3 +++
 hw/isa/piix3.c    | 9 +++++++--
 5 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 566accf7e6..174b6c2ace 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -47,7 +47,6 @@
 #include "multiboot.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/intc/i8259.h"
-#include "hw/dma/i8257.h"
 #include "hw/timer/i8254.h"
 #include "hw/input/i8042.h"
 #include "hw/irq.h"
@@ -1319,8 +1318,6 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         pcspk_init(pcms->pcspk, isa_bus, pit);
     }
 
-    i8257_dma_init(isa_bus, 0);
-
     /* Super I/O */
     pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
                     pcms->vmport != ON_OFF_AUTO_ON);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8043a250ad..fc70a1abc2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -26,6 +26,7 @@
 #include CONFIG_DEVICES
 
 #include "qemu/units.h"
+#include "hw/dma/i8257.h"
 #include "hw/loader.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
@@ -225,6 +226,7 @@ static void pc_init1(MachineState *machine,
         pci_bus = NULL;
         isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
                               &error_abort);
+        i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
     }
     isa_bus_irqs(isa_bus, x86ms->gsi);
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index d42143a991..c65d2d2666 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -33,6 +33,7 @@ config PC87312
 
 config PIIX3
     bool
+    select I8257
     select ISA_BUS
 
 config PIIX4
@@ -67,6 +68,7 @@ config LPC_ICH9
     bool
     # For historical reasons, SuperIO devices are created in the board
     # for ICH9.
+    select I8257
     select ISA_BUS
     select ACPI_SMBUS
     select ACPI_X86_ICH
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 4553b5925b..8694e58b21 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -34,6 +34,7 @@
 #include "qapi/error.h"
 #include "qapi/visitor.h"
 #include "qemu/range.h"
+#include "hw/dma/i8257.h"
 #include "hw/isa/isa.h"
 #include "migration/vmstate.h"
 #include "hw/irq.h"
@@ -722,6 +723,8 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     qdev_init_gpio_out_named(dev, lpc->gsi, ICH9_GPIO_GSI, GSI_NUM_PINS);
 
     isa_bus_irqs(isa_bus, lpc->gsi);
+
+    i8257_dma_init(isa_bus, 0);
 }
 
 static bool ich9_rst_cnt_needed(void *opaque)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 48f9ab1096..44a9998752 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "qemu/range.h"
 #include "qapi/error.h"
+#include "hw/dma/i8257.h"
 #include "hw/southbridge/piix.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
@@ -295,9 +296,11 @@ static const MemoryRegionOps rcr_ops = {
 static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+    ISABus *isa_bus;
 
-    if (!isa_bus_new(DEVICE(d), get_system_memory(),
-                     pci_address_space_io(dev), errp)) {
+    isa_bus = isa_bus_new(DEVICE(d), get_system_memory(),
+                          pci_address_space_io(dev), errp);
+    if (!isa_bus) {
         return;
     }
 
@@ -307,6 +310,8 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
     qemu_register_reset(piix3_reset, d);
+
+    i8257_dma_init(isa_bus, 0);
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
-- 
2.37.3

[PATCH 02/42] hw/i386/pc: Create RTC controllers in south bridges

[Bernhard Beschow]

Just like in the real hardware (and in PIIX4), create the RTC
controllers in the south bridges.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc.c                  | 12 +++++++++++-
 hw/i386/pc_piix.c             |  8 ++++++++
 hw/i386/pc_q35.c              |  1 +
 hw/isa/Kconfig                |  2 ++
 hw/isa/lpc_ich9.c             |  8 ++++++++
 hw/isa/piix3.c                | 15 +++++++++++++++
 include/hw/i386/ich9.h        |  2 ++
 include/hw/southbridge/piix.h |  3 +++
 8 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 174b6c2ace..05d8b0b3d1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1300,7 +1300,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
         rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
     }
-    *rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
+
+    if (rtc_irq) {
+        qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+    } else {
+        uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+                                                "irq",
+                                                &error_fatal);
+        isa_connect_gpio_out(*rtc_state, 0, irq);
+    }
+    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+                              "date");
 
     qemu_register_boot_set(pc_boot_set, *rtc_state);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fc70a1abc2..5f282ff8ad 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -32,6 +32,7 @@
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
 #include "hw/pci-host/i440fx.h"
+#include "hw/rtc/mc146818rtc.h"
 #include "hw/southbridge/piix.h"
 #include "hw/display/ramfb.h"
 #include "hw/firmware/smbios.h"
@@ -222,10 +223,17 @@ static void pc_init1(MachineState *machine,
         piix3->pic = x86ms->gsi;
         piix3_devfn = piix3->dev.devfn;
         isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+        rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
+                                                             "rtc"));
     } else {
         pci_bus = NULL;
         isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
                               &error_abort);
+
+        rtc_state = isa_new(TYPE_MC146818_RTC);
+        qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
+        isa_realize_and_unref(rtc_state, isa_bus, &error_fatal);
+
         i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
     }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 53eda50e81..2eaeab7902 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -238,6 +238,7 @@ static void pc_q35_init(MachineState *machine)
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
                                           ICH9_LPC_FUNC), true,
                                           TYPE_ICH9_LPC_DEVICE);
+    rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc"));
 
     object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
                              TYPE_HOTPLUG_HANDLER,
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index c65d2d2666..6e8f9cac54 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -35,6 +35,7 @@ config PIIX3
     bool
     select I8257
     select ISA_BUS
+    select MC146818RTC
 
 config PIIX4
     bool
@@ -72,3 +73,4 @@ config LPC_ICH9
     select ISA_BUS
     select ACPI_SMBUS
     select ACPI_X86_ICH
+    select MC146818RTC
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 8694e58b21..0051fa66ab 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -660,6 +660,8 @@ static void ich9_lpc_initfn(Object *obj)
     static const uint8_t acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
     static const uint8_t acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
 
+    object_initialize_child(obj, "rtc", &lpc->rtc, TYPE_MC146818_RTC);
+
     object_property_add_uint8_ptr(obj, ACPI_PM_PROP_SCI_INT,
                                   &lpc->sci_gsi, OBJ_PROP_FLAG_READ);
     object_property_add_uint8_ptr(OBJECT(lpc), ACPI_PM_PROP_ACPI_ENABLE_CMD,
@@ -725,6 +727,12 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     isa_bus_irqs(isa_bus, lpc->gsi);
 
     i8257_dma_init(isa_bus, 0);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&lpc->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&lpc->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
 }
 
 static bool ich9_rst_cnt_needed(void *opaque)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 44a9998752..96ab7107e2 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -28,6 +28,7 @@
 #include "hw/dma/i8257.h"
 #include "hw/southbridge/piix.h"
 #include "hw/irq.h"
+#include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
 #include "hw/xen/xen.h"
 #include "sysemu/xen.h"
@@ -312,6 +313,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     qemu_register_reset(piix3_reset, d);
 
     i8257_dma_init(isa_bus, 0);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&d->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -327,6 +334,13 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     }
 }
 
+static void pci_piix3_init(Object *obj)
+{
+    PIIX3State *d = PIIX3_PCI_DEVICE(obj);
+
+    object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+}
+
 static void pci_piix3_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -352,6 +366,7 @@ static const TypeInfo piix3_pci_type_info = {
     .name = TYPE_PIIX3_PCI_DEVICE,
     .parent = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PIIX3State),
+    .instance_init = pci_piix3_init,
     .abstract = true,
     .class_init = pci_piix3_class_init,
     .interfaces = (InterfaceInfo[]) {
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 23ee8e371b..672efc6bce 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -11,6 +11,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
 #include "hw/pci/pci_bus.h"
+#include "hw/rtc/mc146818rtc.h"
 #include "qom/object.h"
 
 void ich9_lpc_set_irq(void *opaque, int irq_num, int level);
@@ -39,6 +40,7 @@ struct ICH9LPCState {
     */
     uint8_t irr[PCI_SLOT_MAX][PCI_NUM_PINS];
 
+    RTCState rtc;
     APMState apm;
     ICH9LPCPMRegs pm;
     uint32_t sci_level; /* track sci level */
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 2693778b23..b1fa08dd2b 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -14,6 +14,7 @@
 
 #include "hw/pci/pci.h"
 #include "qom/object.h"
+#include "hw/rtc/mc146818rtc.h"
 
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
 #define PIIX_PIRQCA 0x60
@@ -52,6 +53,8 @@ struct PIIXState {
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
+    RTCState rtc;
+
     /* Reset Control Register contents */
     uint8_t rcr;
 
-- 
2.37.3

[PATCH 03/42] hw/i386/pc: No need for rtc_state to be an out-parameter

[Bernhard Beschow]

Now that the RTC is created as part of the southbridges it doesn't need
to be an out-parameter any longer.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/i386/pc.c         | 12 ++++++------
 hw/i386/pc_piix.c    |  2 +-
 hw/i386/pc_q35.c     |  2 +-
 include/hw/i386/pc.h |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 05d8b0b3d1..b3a61f5ef2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1247,7 +1247,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
 
 void pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
+                          ISADevice *rtc_state,
                           bool create_fdctrl,
                           uint32_t hpet_irqs)
 {
@@ -1302,17 +1302,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
     }
 
     if (rtc_irq) {
-        qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+        qdev_connect_gpio_out(DEVICE(rtc_state), 0, rtc_irq);
     } else {
-        uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+        uint32_t irq = object_property_get_uint(OBJECT(rtc_state),
                                                 "irq",
                                                 &error_fatal);
-        isa_connect_gpio_out(*rtc_state, 0, irq);
+        isa_connect_gpio_out(rtc_state, 0, irq);
     }
-    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(rtc_state),
                               "date");
 
-    qemu_register_boot_set(pc_boot_set, *rtc_state);
+    qemu_register_boot_set(pc_boot_set, rtc_state);
 
     if (!xen_enabled() &&
         (x86ms->pit == ON_OFF_AUTO_AUTO || x86ms->pit == ON_OFF_AUTO_ON)) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5f282ff8ad..bdad3b6795 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -259,7 +259,7 @@ static void pc_init1(MachineState *machine,
     }
 
     /* init basic PC hardware */
-    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, &rtc_state, true,
+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, true,
                          0x4);
 
     pc_nic_init(pcmc, isa_bus, pci_bus);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2eaeab7902..ca10fa37c6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -292,7 +292,7 @@ static void pc_q35_init(MachineState *machine)
     }
 
     /* init basic PC hardware */
-    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, &rtc_state, !mc->no_floppy,
+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, !mc->no_floppy,
                          0xff0104);
 
     /* connect pm stuff to lpc */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c95333514e..0cf3ccdf0d 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -169,7 +169,7 @@ uint64_t pc_pci_hole64_start(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
+                          ISADevice *rtc_state,
                           bool create_fdctrl,
                           uint32_t hpet_irqs);
 void pc_cmos_init(PCMachineState *pcms,
-- 
2.37.3

[PATCH 04/42] hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge

[Bernhard Beschow]

The next patches will need to take advantage of it.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/i386/pc_piix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bdad3b6795..b08d946992 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -218,7 +218,8 @@ static void pc_init1(MachineState *machine,
                               pci_memory, ram_memory);
         pcms->bus = pci_bus;
 
-        pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+        pci_dev = pci_new_multifunction(-1, true, type);
+        pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
         piix3 = PIIX3_PCI_DEVICE(pci_dev);
         piix3->pic = x86ms->gsi;
         piix3_devfn = piix3->dev.devfn;
-- 
2.37.3

[PATCH 05/42] hw/isa/piix3: Create USB controller in host device

[Bernhard Beschow]

The USB controller is an integral part of PIIX3 (function 2). So create
it as part of the south bridge.

Note that the USB function is optional in QEMU. This is why it gets
object_initialize_child()'ed in realize rather than in instance_init.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c             |  6 ++----
 hw/isa/Kconfig                |  1 +
 hw/isa/piix3.c                | 17 +++++++++++++++++
 include/hw/southbridge/piix.h |  4 ++++
 4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b08d946992..76ac8b2035 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -219,6 +219,8 @@ static void pc_init1(MachineState *machine,
         pcms->bus = pci_bus;
 
         pci_dev = pci_new_multifunction(-1, true, type);
+        object_property_set_bool(OBJECT(pci_dev), "has-usb",
+                                 machine_usb(machine), &error_abort);
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
         piix3 = PIIX3_PCI_DEVICE(pci_dev);
         piix3->pic = x86ms->gsi;
@@ -297,10 +299,6 @@ static void pc_init1(MachineState *machine,
     }
 #endif
 
-    if (pcmc->pci_enabled && machine_usb(machine)) {
-        pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
-    }
-
     if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
         PCIDevice *piix4_pm;
 
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 6e8f9cac54..f02eca3c3e 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -36,6 +36,7 @@ config PIIX3
     select I8257
     select ISA_BUS
     select MC146818RTC
+    select USB_UHCI
 
 config PIIX4
     bool
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 96ab7107e2..27052a5546 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -297,6 +297,7 @@ static const MemoryRegionOps rcr_ops = {
 static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+    PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
 
     isa_bus = isa_bus_new(DEVICE(d), get_system_memory(),
@@ -319,6 +320,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
         return;
     }
+
+    /* USB */
+    if (d->has_usb) {
+        object_initialize_child(OBJECT(dev), "uhci", &d->uhci,
+                                "piix3-usb-uhci");
+        qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2);
+        if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) {
+            return;
+        }
+    }
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj)
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
 }
 
+static Property pci_piix3_props[] = {
+    DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void pci_piix3_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -359,6 +375,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
      * pc_piix.c's pc_init1()
      */
     dc->user_creatable = false;
+    device_class_set_props(dc, pci_piix3_props);
     adevc->build_dev_aml = build_pci_isa_aml;
 }
 
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index b1fa08dd2b..5367917182 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
 #include "hw/pci/pci.h"
 #include "qom/object.h"
 #include "hw/rtc/mc146818rtc.h"
+#include "hw/usb/hcd-uhci.h"
 
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
 #define PIIX_PIRQCA 0x60
@@ -54,12 +55,15 @@ struct PIIXState {
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
     RTCState rtc;
+    UHCIState uhci;
 
     /* Reset Control Register contents */
     uint8_t rcr;
 
     /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
     MemoryRegion rcr_mem;
+
+    bool has_usb;
 };
 typedef struct PIIXState PIIX3State;
 
-- 
2.37.3

[PATCH 06/42] hw/isa/piix3: Create power management controller in host device

[Bernhard Beschow]

The power management controller is an integral part of PIIX3 (function
3). So create it as part of the south bridge.

Note that the ACPI function is optional in QEMU. This is why it gets
object_initialize_child()'ed in realize rather than in instance_init.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c             | 23 +++++++++++++----------
 hw/isa/Kconfig                |  1 +
 hw/isa/piix3.c                | 14 ++++++++++++++
 include/hw/southbridge/piix.h |  6 ++++++
 4 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 76ac8b2035..7efef4f364 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -45,11 +45,11 @@
 #include "sysemu/kvm.h"
 #include "hw/kvm/clock.h"
 #include "hw/sysbus.h"
+#include "hw/i2c/i2c.h"
 #include "hw/i2c/smbus_eeprom.h"
 #include "hw/xen/xen-x86.h"
 #include "exec/memory.h"
 #include "hw/acpi/acpi.h"
-#include "hw/acpi/piix4.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "sysemu/xen.h"
@@ -84,6 +84,7 @@ static void pc_init1(MachineState *machine,
     MemoryRegion *system_io = get_system_io();
     PCIBus *pci_bus;
     ISABus *isa_bus;
+    Object *piix4_pm;
     int piix3_devfn = -1;
     qemu_irq smi_irq;
     GSIState *gsi_state;
@@ -221,6 +222,13 @@ static void pc_init1(MachineState *machine,
         pci_dev = pci_new_multifunction(-1, true, type);
         object_property_set_bool(OBJECT(pci_dev), "has-usb",
                                  machine_usb(machine), &error_abort);
+        object_property_set_bool(OBJECT(pci_dev), "has-acpi",
+                                 x86_machine_is_acpi_enabled(x86ms),
+                                 &error_abort);
+        qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
+        object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
+                                 x86_machine_is_smm_enabled(x86ms),
+                                 &error_abort);
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
         piix3 = PIIX3_PCI_DEVICE(pci_dev);
         piix3->pic = x86ms->gsi;
@@ -228,8 +236,10 @@ static void pc_init1(MachineState *machine,
         isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
         rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
                                                              "rtc"));
+        piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
     } else {
         pci_bus = NULL;
+        piix4_pm = NULL;
         isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
                               &error_abort);
 
@@ -299,15 +309,8 @@ static void pc_init1(MachineState *machine,
     }
 #endif
 
-    if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
-        PCIDevice *piix4_pm;
-
+    if (piix4_pm) {
         smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
-        piix4_pm = pci_new(piix3_devfn + 3, TYPE_PIIX4_PM);
-        qdev_prop_set_uint32(DEVICE(piix4_pm), "smb_io_base", 0xb100);
-        qdev_prop_set_bit(DEVICE(piix4_pm), "smm-enabled",
-                          x86_machine_is_smm_enabled(x86ms));
-        pci_realize_and_unref(piix4_pm, pci_bus, &error_fatal);
 
         qdev_connect_gpio_out(DEVICE(piix4_pm), 0, x86ms->gsi[9]);
         qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
@@ -321,7 +324,7 @@ static void pc_init1(MachineState *machine,
                                  object_property_allow_set_link,
                                  OBJ_PROP_LINK_STRONG);
         object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
-                                 OBJECT(piix4_pm), &error_abort);
+                                 piix4_pm, &error_abort);
     }
 
     if (machine->nvdimms_state->is_enabled) {
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index f02eca3c3e..f10daa26bc 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -33,6 +33,7 @@ config PC87312
 
 config PIIX3
     bool
+    select ACPI_PIIX4
     select I8257
     select ISA_BUS
     select MC146818RTC
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 27052a5546..3bd25013ee 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -330,6 +330,17 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
             return;
         }
     }
+
+    /* Power Management */
+    if (d->has_acpi) {
+        object_initialize_child(OBJECT(d), "pm", &d->pm, TYPE_PIIX4_PM);
+        qdev_prop_set_int32(DEVICE(&d->pm), "addr", dev->devfn + 3);
+        qdev_prop_set_uint32(DEVICE(&d->pm), "smb_io_base", d->smb_io_base);
+        qdev_prop_set_bit(DEVICE(&d->pm), "smm-enabled", d->smm_enabled);
+        if (!qdev_realize(DEVICE(&d->pm), BUS(pci_bus), errp)) {
+            return;
+        }
+    }
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -353,7 +364,10 @@ static void pci_piix3_init(Object *obj)
 }
 
 static Property pci_piix3_props[] = {
+    DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0),
+    DEFINE_PROP_BOOL("has-acpi", PIIX3State, has_acpi, true),
     DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
+    DEFINE_PROP_BOOL("smm-enabled", PIIX3State, smm_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 5367917182..1c291cc954 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -14,6 +14,7 @@
 
 #include "hw/pci/pci.h"
 #include "qom/object.h"
+#include "hw/acpi/piix4.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
 
@@ -56,6 +57,9 @@ struct PIIXState {
 
     RTCState rtc;
     UHCIState uhci;
+    PIIX4PMState pm;
+
+    uint32_t smb_io_base;
 
     /* Reset Control Register contents */
     uint8_t rcr;
@@ -63,7 +67,9 @@ struct PIIXState {
     /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
     MemoryRegion rcr_mem;
 
+    bool has_acpi;
     bool has_usb;
+    bool smm_enabled;
 };
 typedef struct PIIXState PIIX3State;
 
-- 
2.37.3

[PATCH 07/42] hw/intc/i8259: Introduce i8259 proxy "isa-pic"

[Bernhard Beschow]

Having an i8259 proxy allows for ISA PICs to be created and wired up in
southbridges. This is especially interesting for PIIX3 for two reasons:
First, the southbridge doesn't need to care about the virtualization
technology used (KVM, TCG, Xen) due to in-IRQs (where devices get
attached) and out-IRQs (which will trigger the IRQs of the respective
virtzalization technology) are separated. Second, since the in-IRQs are
populated with fully initialized qemu_irq's, they can already be wired
up inside PIIX3.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/intc/i8259.c         | 27 +++++++++++++++++++++++++++
 include/hw/intc/i8259.h | 14 ++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index cc4e21ffec..531f6cca53 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -458,9 +458,36 @@ static const TypeInfo i8259_info = {
     .class_size = sizeof(PICClass),
 };
 
+static void isapic_set_irq(void *opaque, int irq, int level)
+{
+    ISAPICState *s = opaque;
+
+    qemu_set_irq(s->out_irqs[irq], level);
+}
+
+static void isapic_init(Object *obj)
+{
+    ISAPICState *s = ISA_PIC(obj);
+
+    qdev_init_gpio_in(DEVICE(s), isapic_set_irq, ISA_NUM_IRQS);
+    qdev_init_gpio_out(DEVICE(s), s->out_irqs, ISA_NUM_IRQS);
+
+    for (int i = 0; i < ISA_NUM_IRQS; ++i) {
+        s->in_irqs[i] = qdev_get_gpio_in(DEVICE(s), i);
+    }
+}
+
+static const TypeInfo isapic_info = {
+    .name          = TYPE_ISA_PIC,
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(ISAPICState),
+    .instance_init = isapic_init,
+};
+
 static void pic_register_types(void)
 {
     type_register_static(&i8259_info);
+    type_register_static(&isapic_info);
 }
 
 type_init(pic_register_types)
diff --git a/include/hw/intc/i8259.h b/include/hw/intc/i8259.h
index e2b1e8c59a..0246ab6ac6 100644
--- a/include/hw/intc/i8259.h
+++ b/include/hw/intc/i8259.h
@@ -1,6 +1,20 @@
 #ifndef HW_I8259_H
 #define HW_I8259_H
 
+#include "qom/object.h"
+#include "hw/isa/isa.h"
+#include "qemu/typedefs.h"
+
+#define TYPE_ISA_PIC "isa-pic"
+OBJECT_DECLARE_SIMPLE_TYPE(ISAPICState, ISA_PIC)
+
+struct ISAPICState {
+    ISADevice parent_obj;
+
+    qemu_irq in_irqs[ISA_NUM_IRQS];
+    qemu_irq out_irqs[ISA_NUM_IRQS];
+};
+
 /* i8259.c */
 
 extern DeviceState *isa_pic;
-- 
2.37.3

[PATCH 08/42] hw/isa/piix3: Create ISA PIC in host device

[Bernhard Beschow]

Use the newly introduced i8259 proxy "isa-pic" which allows for wiring
up devices in the southbridge where the virtualization technology used
(KVM, TCG, Xen) is not yet known.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c             | 15 +++++++++------
 hw/isa/Kconfig                |  1 +
 hw/isa/piix3.c                | 10 +++++++++-
 include/hw/southbridge/piix.h |  4 ++--
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7efef4f364..b7fcafb77e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -206,10 +206,11 @@ static void pc_init1(MachineState *machine,
     gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
 
     if (pcmc->pci_enabled) {
-        PIIX3State *piix3;
+        DeviceState *dev;
         PCIDevice *pci_dev;
         const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
                                          : TYPE_PIIX3_DEVICE;
+        int i;
 
         pci_bus = i440fx_init(pci_type,
                               i440fx_host,
@@ -230,10 +231,12 @@ static void pc_init1(MachineState *machine,
                                  x86_machine_is_smm_enabled(x86ms),
                                  &error_abort);
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
-        piix3 = PIIX3_PCI_DEVICE(pci_dev);
-        piix3->pic = x86ms->gsi;
-        piix3_devfn = piix3->dev.devfn;
-        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+        dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "pic"));
+        for (i = 0; i < ISA_NUM_IRQS; i++) {
+            qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
+        }
+        piix3_devfn = pci_dev->devfn;
+        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
         rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
                                                              "rtc"));
         piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
@@ -242,6 +245,7 @@ static void pc_init1(MachineState *machine,
         piix4_pm = NULL;
         isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
                               &error_abort);
+        isa_bus_irqs(isa_bus, x86ms->gsi);
 
         rtc_state = isa_new(TYPE_MC146818_RTC);
         qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
@@ -250,7 +254,6 @@ static void pc_init1(MachineState *machine,
         i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
     }
-    isa_bus_irqs(isa_bus, x86ms->gsi);
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
         pc_i8259_create(isa_bus, gsi_state->i8259_irq);
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index f10daa26bc..24e79a9a41 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -35,6 +35,7 @@ config PIIX3
     bool
     select ACPI_PIIX4
     select I8257
+    select I8259
     select ISA_BUS
     select MC146818RTC
     select USB_UHCI
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 3bd25013ee..39d8cb787f 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -41,7 +41,7 @@
 
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
-    qemu_set_irq(piix3->pic[pic_irq],
+    qemu_set_irq(piix3->pic.in_irqs[pic_irq],
                  !!(piix3->pic_levels &
                     (((1ULL << PIIX_NUM_PIRQS) - 1) <<
                      (pic_irq * PIIX_NUM_PIRQS))));
@@ -306,6 +306,13 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
+    /* PIC */
+    if (!qdev_realize(DEVICE(&d->pic), BUS(isa_bus), errp)) {
+        return;
+    }
+
+    isa_bus_irqs(isa_bus, d->pic.in_irqs);
+
     memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
                           "piix3-reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
@@ -360,6 +367,7 @@ static void pci_piix3_init(Object *obj)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(obj);
 
+    object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
 }
 
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 1c291cc954..7178147b75 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
 #include "hw/pci/pci.h"
 #include "qom/object.h"
 #include "hw/acpi/piix4.h"
+#include "hw/intc/i8259.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
 
@@ -50,11 +51,10 @@ struct PIIXState {
 #endif
     uint64_t pic_levels;
 
-    qemu_irq *pic;
-
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
+    ISAPICState pic;
     RTCState rtc;
     UHCIState uhci;
     PIIX4PMState pm;
-- 
2.37.3

[PATCH 09/42] hw/isa/piix3: Create IDE controller in host device

[Bernhard Beschow]

Now that PIIX3 contains the new isa-pic, it is possible to instantiate
PIIX3 IDE in the PIIX3 southbridge. PIIX3 IDE wires up its interrupts to
the ISA bus in its realize method which requires the interrupt
controller to provide fully populated qemu_irqs. This is the case for
isa-pic even though the virtualization technology not known yet.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/Kconfig               |  1 -
 hw/i386/pc_piix.c             | 14 ++++++--------
 hw/isa/Kconfig                |  1 +
 hw/isa/piix3.c                |  7 +++++++
 include/hw/southbridge/piix.h |  2 ++
 5 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index d22ac4a4b9..dd247f215c 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -74,7 +74,6 @@ config I440FX
     select ACPI_SMBUS
     select PCI_I440FX
     select PIIX3
-    select IDE_PIIX
     select DIMM
     select SMBIOS
     select FW_CFG_DMA
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b7fcafb77e..c76afbc7e3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -85,7 +85,6 @@ static void pc_init1(MachineState *machine,
     PCIBus *pci_bus;
     ISABus *isa_bus;
     Object *piix4_pm;
-    int piix3_devfn = -1;
     qemu_irq smi_irq;
     GSIState *gsi_state;
     BusState *idebus[MAX_IDE_BUS];
@@ -235,11 +234,14 @@ static void pc_init1(MachineState *machine,
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
         }
-        piix3_devfn = pci_dev->devfn;
         isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
         rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
                                                              "rtc"));
         piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
+        dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
+        pci_ide_create_devs(PCI_DEVICE(dev));
+        idebus[0] = qdev_get_child_bus(dev, "ide.0");
+        idebus[1] = qdev_get_child_bus(dev, "ide.1");
     } else {
         pci_bus = NULL;
         piix4_pm = NULL;
@@ -253,6 +255,8 @@ static void pc_init1(MachineState *machine,
 
         i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
+        idebus[0] = NULL;
+        idebus[1] = NULL;
     }
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
@@ -281,12 +285,6 @@ static void pc_init1(MachineState *machine,
     pc_nic_init(pcmc, isa_bus, pci_bus);
 
     if (pcmc->pci_enabled) {
-        PCIDevice *dev;
-
-        dev = pci_create_simple(pci_bus, piix3_devfn + 1, "piix3-ide");
-        pci_ide_create_devs(dev);
-        idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
-        idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
         pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
     }
 #ifdef CONFIG_IDE_ISA
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 24e79a9a41..a021e1cbfc 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -36,6 +36,7 @@ config PIIX3
     select ACPI_PIIX4
     select I8257
     select I8259
+    select IDE_PIIX
     select ISA_BUS
     select MC146818RTC
     select USB_UHCI
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 39d8cb787f..0e86a9a3cb 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -328,6 +328,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
+    /* IDE */
+    qdev_prop_set_int32(DEVICE(&d->ide), "addr", dev->devfn + 1);
+    if (!qdev_realize(DEVICE(&d->ide), BUS(pci_bus), errp)) {
+        return;
+    }
+
     /* USB */
     if (d->has_usb) {
         object_initialize_child(OBJECT(dev), "uhci", &d->uhci,
@@ -369,6 +375,7 @@ static void pci_piix3_init(Object *obj)
 
     object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+    object_initialize_child(obj, "ide", &d->ide, "piix3-ide");
 }
 
 static Property pci_piix3_props[] = {
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 7178147b75..1f22eb1444 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
 #include "hw/pci/pci.h"
 #include "qom/object.h"
 #include "hw/acpi/piix4.h"
+#include "hw/ide/pci.h"
 #include "hw/intc/i8259.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
@@ -56,6 +57,7 @@ struct PIIXState {
 
     ISAPICState pic;
     RTCState rtc;
+    PCIIDEState ide;
     UHCIState uhci;
     PIIX4PMState pm;
 
-- 
2.37.3

[PATCH 10/42] hw/isa/piix3: Wire up ACPI interrupt internally

[Bernhard Beschow]

Now that PIIX3 has the PIC integrated, the ACPI controller can be wired
up internally.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c | 1 -
 hw/isa/piix3.c    | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c76afbc7e3..907cf865b8 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -313,7 +313,6 @@ static void pc_init1(MachineState *machine,
     if (piix4_pm) {
         smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
 
-        qdev_connect_gpio_out(DEVICE(piix4_pm), 0, x86ms->gsi[9]);
         qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
         pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
         /* TODO: Populate SPD eeprom data.  */
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 0e86a9a3cb..e85dec3200 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -353,6 +353,8 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
         if (!qdev_realize(DEVICE(&d->pm), BUS(pci_bus), errp)) {
             return;
         }
+        qdev_connect_gpio_out(DEVICE(&d->pm), 0,
+                              qdev_get_gpio_in(DEVICE(&d->pic), 9));
     }
 }
 
-- 
2.37.3

[PATCH 11/42] hw/isa/piix3: Remove extra ';' outside of functions

[Bernhard Beschow]

Fixes the "extra-semi" clang-tidy check.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/isa/piix3.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index e85dec3200..0117f896d2 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -438,7 +438,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
     pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
                  piix3, PIIX_NUM_PIRQS);
     pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
-};
+}
 
 static void piix3_class_init(ObjectClass *klass, void *data)
 {
@@ -473,7 +473,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
      */
     pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
                  piix3, XEN_PIIX_NUM_PIRQS);
-};
+}
 
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
@@ -481,7 +481,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
 
     k->config_write = piix3_write_config_xen;
     k->realize = piix3_xen_realize;
-};
+}
 
 static const TypeInfo piix3_xen_info = {
     .name          = TYPE_PIIX3_XEN_DEVICE,
-- 
2.37.3

[PATCH 12/42] hw/isa/piix3: Remove unused include

[Bernhard Beschow]

Ammends commit 988fb613215993dd0ce642b89ca8182c479d39dd.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 0117f896d2..b54ff1c948 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -31,7 +31,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
 #include "hw/xen/xen.h"
-#include "sysemu/xen.h"
 #include "sysemu/reset.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
-- 
2.37.3

[PATCH 13/42] hw/isa/piix3: Add size constraints to rcr_ops

[Bernhard Beschow]

According to the PIIX3 datasheet, the reset control register is one byte in size.
Moreover, PIIX4 has it, so add it to PIIX3 as well.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index b54ff1c948..c8c2dd6048 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -290,7 +290,11 @@ static uint64_t rcr_read(void *opaque, hwaddr addr, unsigned len)
 static const MemoryRegionOps rcr_ops = {
     .read = rcr_read,
     .write = rcr_write,
-    .endianness = DEVICE_LITTLE_ENDIAN
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
 };
 
 static void pci_piix3_realize(PCIDevice *dev, Error **errp)
-- 
2.37.3

[PATCH 14/42] hw/isa/piix3: Modernize reset handling

[Bernhard Beschow]

Rather than registering the reset handler via a function which
appends the handler to a global list, prefer to implement it as
a virtual method - PIIX4 does the same already.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c8c2dd6048..0350f70706 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -31,7 +31,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
 #include "hw/xen/xen.h"
-#include "sysemu/reset.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
@@ -156,9 +155,9 @@ static void piix3_write_config_xen(PCIDevice *dev,
     piix3_write_config(dev, address, val, len);
 }
 
-static void piix3_reset(void *opaque)
+static void piix3_reset(DeviceState *dev)
 {
-    PIIX3State *d = opaque;
+    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
     uint8_t *pci_conf = d->dev.config;
 
     pci_conf[0x04] = 0x07; /* master, memory and I/O */
@@ -321,8 +320,6 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
-    qemu_register_reset(piix3_reset, d);
-
     i8257_dma_init(isa_bus, 0);
 
     /* RTC */
@@ -397,6 +394,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
+    dc->reset       = piix3_reset;
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
     dc->hotpluggable   = false;
-- 
2.37.3

[PATCH 15/42] hw/isa/piix3: Prefer pci_address_space() over get_system_memory()

[Bernhard Beschow]

get_system_memory() accesses global state while pci_address_space() uses
whatever has been passed to the device instance, so avoid the global.
Moreover, PIIX4 uses pci_address_space() here as well.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 0350f70706..f8fcd47e24 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -302,7 +302,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
 
-    isa_bus = isa_bus_new(DEVICE(d), get_system_memory(),
+    isa_bus = isa_bus_new(DEVICE(d), pci_address_space(dev),
                           pci_address_space_io(dev), errp);
     if (!isa_bus) {
         return;
-- 
2.37.3

[PATCH 16/42] hw/isa/piix3: Allow board to provide PCI interrupt routes

[Bernhard Beschow]

PIIX3 initializes the PIRQx route control registers to the default
values as described in the 82371AB PCI-TO-ISA/IDE XCELERATOR (PIIX4)
April 1997 manual. PIIX4, however, initializes the routes according to
the Malta™ User’s Manual, ch 6.6, which are IRQs 10 and 11. In order to
allow the reset methods to be consolidated, allow board code to specify
the routes.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c                | 12 ++++++++----
 include/hw/southbridge/piix.h |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index f8fcd47e24..a4a5f33d6e 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -167,10 +167,10 @@ static void piix3_reset(DeviceState *dev)
     pci_conf[0x4c] = 0x4d;
     pci_conf[0x4e] = 0x03;
     pci_conf[0x4f] = 0x00;
-    pci_conf[0x60] = 0x80;
-    pci_conf[0x61] = 0x80;
-    pci_conf[0x62] = 0x80;
-    pci_conf[0x63] = 0x80;
+    pci_conf[PIIX_PIRQCA] = d->pci_irq_reset_mappings[0];
+    pci_conf[PIIX_PIRQCB] = d->pci_irq_reset_mappings[1];
+    pci_conf[PIIX_PIRQCC] = d->pci_irq_reset_mappings[2];
+    pci_conf[PIIX_PIRQCD] = d->pci_irq_reset_mappings[3];
     pci_conf[0x69] = 0x02;
     pci_conf[0x70] = 0x80;
     pci_conf[0x76] = 0x0c;
@@ -382,6 +382,10 @@ static void pci_piix3_init(Object *obj)
 
 static Property pci_piix3_props[] = {
     DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0),
+    DEFINE_PROP_UINT8("pirqa", PIIX3State, pci_irq_reset_mappings[0], 0x80),
+    DEFINE_PROP_UINT8("pirqb", PIIX3State, pci_irq_reset_mappings[1], 0x80),
+    DEFINE_PROP_UINT8("pirqc", PIIX3State, pci_irq_reset_mappings[2], 0x80),
+    DEFINE_PROP_UINT8("pirqd", PIIX3State, pci_irq_reset_mappings[3], 0x80),
     DEFINE_PROP_BOOL("has-acpi", PIIX3State, has_acpi, true),
     DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
     DEFINE_PROP_BOOL("smm-enabled", PIIX3State, smm_enabled, false),
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 1f22eb1444..df3e0084c5 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -54,6 +54,7 @@ struct PIIXState {
 
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
+    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
 
     ISAPICState pic;
     RTCState rtc;
-- 
2.37.3

[PATCH 17/42] hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS

[Bernhard Beschow]

PIIX_NUM_PIC_IRQS is assumed to be the same as ISA_NUM_IRQS, otherwise
inconsistencies can occur.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c                | 8 ++++----
 include/hw/southbridge/piix.h | 5 ++---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index a4a5f33d6e..ae1df8e73e 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -51,7 +51,7 @@ static void piix3_set_irq_level_internal(PIIX3State *piix3, int pirq, int level)
     uint64_t mask;
 
     pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
-    if (pic_irq >= PIIX_NUM_PIC_IRQS) {
+    if (pic_irq >= ISA_NUM_IRQS) {
         return;
     }
 
@@ -65,7 +65,7 @@ static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
     int pic_irq;
 
     pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
-    if (pic_irq >= PIIX_NUM_PIC_IRQS) {
+    if (pic_irq >= ISA_NUM_IRQS) {
         return;
     }
 
@@ -97,7 +97,7 @@ static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
     int irq = piix3->dev.config[PIIX_PIRQCA + pin];
     PCIINTxRoute route;
 
-    if (irq < PIIX_NUM_PIC_IRQS) {
+    if (irq < ISA_NUM_IRQS) {
         route.mode = PCI_INTX_ENABLED;
         route.irq = irq;
     } else {
@@ -129,7 +129,7 @@ static void piix3_write_config(PCIDevice *dev,
 
         pci_bus_fire_intx_routing_notifier(pci_get_bus(&piix3->dev));
         piix3_update_irq_levels(piix3);
-        for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) {
+        for (pic_irq = 0; pic_irq < ISA_NUM_IRQS; pic_irq++) {
             piix3_set_irq_pic(piix3, pic_irq);
         }
     }
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index df3e0084c5..ae3b49fe93 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -32,7 +32,6 @@
  */
 #define PIIX_RCR_IOPORT 0xcf9
 
-#define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
 #define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
 
 struct PIIXState {
@@ -44,10 +43,10 @@ struct PIIXState {
      * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
      *
      * PIRQ is mapped to PIC pins, we track it by
-     * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
+     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
      * pic_irq * PIIX_NUM_PIRQS + pirq
      */
-#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
+#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
 #error "unable to encode pic state in 64bit in pic_levels."
 #endif
     uint64_t pic_levels;
-- 
2.37.3

[PATCH 18/42] hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4

[Bernhard Beschow]

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index ae1df8e73e..cb3fef7654 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -380,7 +380,7 @@ static void pci_piix3_init(Object *obj)
     object_initialize_child(obj, "ide", &d->ide, "piix3-ide");
 }
 
-static Property pci_piix3_props[] = {
+static Property pci_piix_props[] = {
     DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0),
     DEFINE_PROP_UINT8("pirqa", PIIX3State, pci_irq_reset_mappings[0], 0x80),
     DEFINE_PROP_UINT8("pirqb", PIIX3State, pci_irq_reset_mappings[1], 0x80),
@@ -411,7 +411,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
      * pc_piix.c's pc_init1()
      */
     dc->user_creatable = false;
-    device_class_set_props(dc, pci_piix3_props);
+    device_class_set_props(dc, pci_piix_props);
     adevc->build_dev_aml = build_pci_isa_aml;
 }
 
-- 
2.37.3

[PATCH 19/42] hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4

[Bernhard Beschow]

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index cb3fef7654..d027c5b61e 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -155,7 +155,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
     piix3_write_config(dev, address, val, len);
 }
 
-static void piix3_reset(DeviceState *dev)
+static void piix_reset(DeviceState *dev)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(dev);
     uint8_t *pci_conf = d->dev.config;
@@ -398,7 +398,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
-    dc->reset       = piix3_reset;
+    dc->reset       = piix_reset;
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
     dc->hotpluggable   = false;
-- 
2.37.3

[PATCH 20/42] hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"

[Bernhard Beschow]

The additional prefix aligns the function name with both other
piix3-internal functions as well as QEMU conventions. Furthermore, it
will help to distinguish the function from its PIIX4 counterpart once
merged.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index d027c5b61e..e5772475be 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -84,7 +84,7 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
  * Return the global irq number corresponding to a given device irq
  * pin. We could also use the bus number to have a more precise mapping.
  */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
+static int piix3_pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 {
     int slot_addend;
     slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
@@ -440,7 +440,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
+    pci_bus_irqs(pci_bus, piix3_set_irq, piix3_pci_slot_get_pirq,
                  piix3, PIIX_NUM_PIRQS);
     pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
 }
-- 
2.37.3

[PATCH 21/42] hw/isa/piix3: Rename typedef PIIX3State to PIIXState

[Bernhard Beschow]

This commit marks the finalization of the PIIX3 preparations
to be merged with PIIX4. In particular, PIIXState is prepared
to be reused in piix4.c.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix3.c                | 58 +++++++++++++++++------------------
 include/hw/southbridge/piix.h |  4 +--
 2 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index e5772475be..75705a1fc1 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -37,7 +37,7 @@
 
 #define XEN_PIIX_NUM_PIRQS      128ULL
 
-static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
+static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
 {
     qemu_set_irq(piix3->pic.in_irqs[pic_irq],
                  !!(piix3->pic_levels &
@@ -45,7 +45,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
                      (pic_irq * PIIX_NUM_PIRQS))));
 }
 
-static void piix3_set_irq_level_internal(PIIX3State *piix3, int pirq, int level)
+static void piix3_set_irq_level_internal(PIIXState *piix3, int pirq, int level)
 {
     int pic_irq;
     uint64_t mask;
@@ -60,7 +60,7 @@ static void piix3_set_irq_level_internal(PIIX3State *piix3, int pirq, int level)
     piix3->pic_levels |= mask * !!level;
 }
 
-static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
+static void piix3_set_irq_level(PIIXState *piix3, int pirq, int level)
 {
     int pic_irq;
 
@@ -76,7 +76,7 @@ static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
 
 static void piix3_set_irq(void *opaque, int pirq, int level)
 {
-    PIIX3State *piix3 = opaque;
+    PIIXState *piix3 = opaque;
     piix3_set_irq_level(piix3, pirq, level);
 }
 
@@ -93,7 +93,7 @@ static int piix3_pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
-    PIIX3State *piix3 = opaque;
+    PIIXState *piix3 = opaque;
     int irq = piix3->dev.config[PIIX_PIRQCA + pin];
     PCIINTxRoute route;
 
@@ -108,7 +108,7 @@ static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 }
 
 /* irq routing is changed. so rebuild bitmap */
-static void piix3_update_irq_levels(PIIX3State *piix3)
+static void piix3_update_irq_levels(PIIXState *piix3)
 {
     PCIBus *bus = pci_get_bus(&piix3->dev);
     int pirq;
@@ -124,7 +124,7 @@ static void piix3_write_config(PCIDevice *dev,
 {
     pci_default_write_config(dev, address, val, len);
     if (ranges_overlap(address, len, PIIX_PIRQCA, 4)) {
-        PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+        PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
         int pic_irq;
 
         pci_bus_fire_intx_routing_notifier(pci_get_bus(&piix3->dev));
@@ -157,7 +157,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
 
 static void piix_reset(DeviceState *dev)
 {
-    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+    PIIXState *d = PIIX_PCI_DEVICE(dev);
     uint8_t *pci_conf = d->dev.config;
 
     pci_conf[0x04] = 0x07; /* master, memory and I/O */
@@ -198,7 +198,7 @@ static void piix_reset(DeviceState *dev)
 
 static int piix3_post_load(void *opaque, int version_id)
 {
-    PIIX3State *piix3 = opaque;
+    PIIXState *piix3 = opaque;
     int pirq;
 
     /*
@@ -221,7 +221,7 @@ static int piix3_post_load(void *opaque, int version_id)
 static int piix3_pre_save(void *opaque)
 {
     int i;
-    PIIX3State *piix3 = opaque;
+    PIIXState *piix3 = opaque;
 
     for (i = 0; i < ARRAY_SIZE(piix3->pci_irq_levels_vmstate); i++) {
         piix3->pci_irq_levels_vmstate[i] =
@@ -233,7 +233,7 @@ static int piix3_pre_save(void *opaque)
 
 static bool piix3_rcr_needed(void *opaque)
 {
-    PIIX3State *piix3 = opaque;
+    PIIXState *piix3 = opaque;
 
     return (piix3->rcr != 0);
 }
@@ -244,7 +244,7 @@ static const VMStateDescription vmstate_piix3_rcr = {
     .minimum_version_id = 1,
     .needed = piix3_rcr_needed,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT8(rcr, PIIX3State),
+        VMSTATE_UINT8(rcr, PIIXState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -256,8 +256,8 @@ static const VMStateDescription vmstate_piix3 = {
     .post_load = piix3_post_load,
     .pre_save = piix3_pre_save,
     .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, PIIX3State),
-        VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIX3State,
+        VMSTATE_PCI_DEVICE(dev, PIIXState),
+        VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIXState,
                               PIIX_NUM_PIRQS, 3),
         VMSTATE_END_OF_LIST()
     },
@@ -270,7 +270,7 @@ static const VMStateDescription vmstate_piix3 = {
 
 static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
 {
-    PIIX3State *d = opaque;
+    PIIXState *d = opaque;
 
     if (val & 4) {
         qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
@@ -281,7 +281,7 @@ static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
 
 static uint64_t rcr_read(void *opaque, hwaddr addr, unsigned len)
 {
-    PIIX3State *d = opaque;
+    PIIXState *d = opaque;
 
     return d->rcr;
 }
@@ -298,7 +298,7 @@ static const MemoryRegionOps rcr_ops = {
 
 static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 {
-    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+    PIIXState *d = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
 
@@ -373,7 +373,7 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
 
 static void pci_piix3_init(Object *obj)
 {
-    PIIX3State *d = PIIX3_PCI_DEVICE(obj);
+    PIIXState *d = PIIX_PCI_DEVICE(obj);
 
     object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
@@ -381,14 +381,14 @@ static void pci_piix3_init(Object *obj)
 }
 
 static Property pci_piix_props[] = {
-    DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0),
-    DEFINE_PROP_UINT8("pirqa", PIIX3State, pci_irq_reset_mappings[0], 0x80),
-    DEFINE_PROP_UINT8("pirqb", PIIX3State, pci_irq_reset_mappings[1], 0x80),
-    DEFINE_PROP_UINT8("pirqc", PIIX3State, pci_irq_reset_mappings[2], 0x80),
-    DEFINE_PROP_UINT8("pirqd", PIIX3State, pci_irq_reset_mappings[3], 0x80),
-    DEFINE_PROP_BOOL("has-acpi", PIIX3State, has_acpi, true),
-    DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
-    DEFINE_PROP_BOOL("smm-enabled", PIIX3State, smm_enabled, false),
+    DEFINE_PROP_UINT32("smb_io_base", PIIXState, smb_io_base, 0),
+    DEFINE_PROP_UINT8("pirqa", PIIXState, pci_irq_reset_mappings[0], 0x80),
+    DEFINE_PROP_UINT8("pirqb", PIIXState, pci_irq_reset_mappings[1], 0x80),
+    DEFINE_PROP_UINT8("pirqc", PIIXState, pci_irq_reset_mappings[2], 0x80),
+    DEFINE_PROP_UINT8("pirqd", PIIXState, pci_irq_reset_mappings[3], 0x80),
+    DEFINE_PROP_BOOL("has-acpi", PIIXState, has_acpi, true),
+    DEFINE_PROP_BOOL("has-usb", PIIXState, has_usb, true),
+    DEFINE_PROP_BOOL("smm-enabled", PIIXState, smm_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -418,7 +418,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
 static const TypeInfo piix3_pci_type_info = {
     .name = TYPE_PIIX3_PCI_DEVICE,
     .parent = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIX3State),
+    .instance_size = sizeof(PIIXState),
     .instance_init = pci_piix3_init,
     .abstract = true,
     .class_init = pci_piix3_class_init,
@@ -432,7 +432,7 @@ static const TypeInfo piix3_pci_type_info = {
 static void piix3_realize(PCIDevice *dev, Error **errp)
 {
     ERRP_GUARD();
-    PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+    PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
     pci_piix3_realize(dev, errp);
@@ -462,7 +462,7 @@ static const TypeInfo piix3_info = {
 static void piix3_xen_realize(PCIDevice *dev, Error **errp)
 {
     ERRP_GUARD();
-    PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+    PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
     pci_piix3_realize(dev, errp);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index ae3b49fe93..c9fa0f1aa6 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -73,10 +73,10 @@ struct PIIXState {
     bool has_usb;
     bool smm_enabled;
 };
-typedef struct PIIXState PIIX3State;
+typedef struct PIIXState PIIXState;
 
 #define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
-DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
+DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
                          TYPE_PIIX3_PCI_DEVICE)
 
 #define TYPE_PIIX3_DEVICE "PIIX3"
-- 
2.37.3

[PATCH 22/42] hw/mips/malta: Reuse dev variable

[Bernhard Beschow]

While at it, move the assignments closer to where they are used.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/mips/malta.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 0e932988e0..0ec2ac2eaf 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1239,7 +1239,6 @@ void mips_malta_init(MachineState *machine)
     MaltaState *s;
     PCIDevice *piix4;
     DeviceState *dev;
-    DeviceState *pm_dev;
 
     s = MIPS_MALTA(qdev_new(TYPE_MIPS_MALTA));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(s), &error_fatal);
@@ -1405,13 +1404,13 @@ void mips_malta_init(MachineState *machine)
                                             TYPE_PIIX4_PCI_DEVICE);
     dev = DEVICE(piix4);
     isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
-    pm_dev = DEVICE(object_resolve_path_component(OBJECT(dev), "pm"));
-    smbus = I2C_BUS(qdev_get_child_bus(pm_dev, "i2c"));
 
     /* Interrupt controller */
     qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
 
     /* generate SPD EEPROM data */
+    dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pm"));
+    smbus = I2C_BUS(qdev_get_child_bus(dev, "i2c"));
     generate_eeprom_spd(&smbus_eeprom_buf[0 * 256], ram_size);
     generate_eeprom_serial(&smbus_eeprom_buf[6 * 256]);
     smbus_eeprom_init(smbus, 8, smbus_eeprom_buf, smbus_eeprom_size);
-- 
2.37.3

[PATCH 23/42] meson: Fix dependencies of piix4 southbridge

[Bernhard Beschow]

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 configs/devices/mips-softmmu/common.mak | 1 -
 hw/isa/Kconfig                          | 6 ++++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
index d2202c839e..416161f833 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -23,7 +23,6 @@ CONFIG_APM=y
 CONFIG_I8257=y
 CONFIG_PIIX4=y
 CONFIG_IDE_ISA=y
-CONFIG_IDE_PIIX=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_I8259=y
 CONFIG_MC146818RTC=y
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index a021e1cbfc..1aa10f84f2 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -45,7 +45,13 @@ config PIIX4
     bool
     # For historical reasons, SuperIO devices are created in the board
     # for PIIX4.
+    select ACPI_PIIX4
+    select I8254
+    select I8257
+    select I8259
+    select IDE_PIIX
     select ISA_BUS
+    select MC146818RTC
     select USB_UHCI
 
 config VT82C686
-- 
2.37.3

[PATCH 24/42] hw/isa/piix4: Add missing initialization

[Bernhard Beschow]

PIIX3 clears its reset control register, so do the same in PIIX4.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 15f344dbb7..9e2f7b9b71 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -139,6 +139,8 @@ static void piix4_isa_reset(DeviceState *dev)
     pci_conf[0xab] = 0x00;
     pci_conf[0xac] = 0x00;
     pci_conf[0xae] = 0x00;
+
+    d->rcr = 0;
 }
 
 static int piix4_ide_post_load(void *opaque, int version_id)
-- 
2.37.3

[PATCH 25/42] hw/isa/piix4: Move pci_ide_create_devs() call to board code

[Bernhard Beschow]

For the VIA south bridges there was a comment to have the call in board code.
Move it there for PIIX4 as well for consistency.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c  |  1 -
 hw/mips/malta.c | 10 ++++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9e2f7b9b71..67881e3a75 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -256,7 +256,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
         return;
     }
-    pci_ide_create_devs(PCI_DEVICE(&s->ide));
 
     /* USB */
     qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 0ec2ac2eaf..a4b866a2cf 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -39,7 +39,7 @@
 #include "hw/pci/pci.h"
 #include "qemu/log.h"
 #include "hw/mips/bios.h"
-#include "hw/ide.h"
+#include "hw/ide/pci.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
 #include "elf.h"
@@ -1402,11 +1402,13 @@ void mips_malta_init(MachineState *machine)
     /* Southbridge */
     piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
                                             TYPE_PIIX4_PCI_DEVICE);
-    dev = DEVICE(piix4);
-    isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
+    isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
+
+    dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "ide"));
+    pci_ide_create_devs(PCI_DEVICE(dev));
 
     /* Interrupt controller */
-    qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
+    qdev_connect_gpio_out_named(DEVICE(piix4), "intr", 0, i8259_irq);
 
     /* generate SPD EEPROM data */
     dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pm"));
-- 
2.37.3

[PATCH 26/42] hw/isa/piix4: Make PIIX4's ACPI and USB functions optional

[Bernhard Beschow]

This aligns PIIX4 with PIIX3.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c  | 44 ++++++++++++++++++++++++++++++++------------
 hw/mips/malta.c |  6 ++++--
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 67881e3a75..ed9eca715f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -50,9 +50,16 @@ struct PIIX4State {
     PCIIDEState ide;
     UHCIState uhci;
     PIIX4PMState pm;
+
+    uint32_t smb_io_base;
+
     /* Reset Control Register */
     MemoryRegion rcr_mem;
     uint8_t rcr;
+
+    bool has_acpi;
+    bool has_usb;
+    bool smm_enabled;
 };
 
 OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
@@ -258,17 +265,26 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     }
 
     /* USB */
-    qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
-    if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
-        return;
+    if (s->has_usb) {
+        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
+                                "piix4-usb-uhci");
+        qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
+        if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
+            return;
+        }
     }
 
     /* ACPI controller */
-    qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
-    if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
-        return;
+    if (s->has_acpi) {
+        object_initialize_child(OBJECT(s), "pm", &s->pm, TYPE_PIIX4_PM);
+        qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
+        qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", s->smb_io_base);
+        qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", s->smm_enabled);
+        if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
+            return;
+        }
+        qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
     }
-    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
 
     pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
 }
@@ -279,13 +295,16 @@ static void piix4_init(Object *obj)
 
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
-    object_initialize_child(obj, "uhci", &s->uhci, "piix4-usb-uhci");
-
-    object_initialize_child(obj, "pm", &s->pm, TYPE_PIIX4_PM);
-    qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", 0x1100);
-    qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", 0);
 }
 
+static Property piix4_props[] = {
+    DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
+    DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
+    DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
+    DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void piix4_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -304,6 +323,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
      */
     dc->user_creatable = false;
     dc->hotpluggable = false;
+    device_class_set_props(dc, piix4_props);
 }
 
 static const TypeInfo piix4_info = {
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index a4b866a2cf..6339b0d66c 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1400,8 +1400,10 @@ void mips_malta_init(MachineState *machine)
     empty_slot_init("GT64120", 0, 0x20000000);
 
     /* Southbridge */
-    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
-                                            TYPE_PIIX4_PCI_DEVICE);
+    piix4 = pci_new_multifunction(PCI_DEVFN(10, 0), true,
+                                  TYPE_PIIX4_PCI_DEVICE);
+    qdev_prop_set_uint32(DEVICE(piix4), "smb_io_base", 0x1100);
+    pci_realize_and_unref(piix4, pci_bus, &error_fatal);
     isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
 
     dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "ide"));
-- 
2.37.3

[PATCH 27/42] hw/isa/piix4: Allow board to provide PCI interrupt routes

[Bernhard Beschow]

PIIX3 initializes the PIRQx route control registers to the default
values as described in the 82371AB PCI-TO-ISA/IDE XCELERATOR (PIIX4)
April 1997 manual. PIIX4, however, initializes the routes according to
the Malta™ User’s Manual, ch 6.6, which are IRQs 10 and 11. In order to
allow the reset methods to be consolidated, allow board code to specify
the routes.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c  | 14 ++++++++++----
 hw/mips/malta.c |  4 ++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index ed9eca715f..763c98b565 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -57,6 +57,8 @@ struct PIIX4State {
     MemoryRegion rcr_mem;
     uint8_t rcr;
 
+    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
+
     bool has_acpi;
     bool has_usb;
     bool smm_enabled;
@@ -122,10 +124,10 @@ static void piix4_isa_reset(DeviceState *dev)
     pci_conf[0x4c] = 0x4d;
     pci_conf[0x4e] = 0x03;
     pci_conf[0x4f] = 0x00;
-    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
-    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
-    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
-    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
+    pci_conf[PIIX_PIRQCA] = d->pci_irq_reset_mappings[0];
+    pci_conf[PIIX_PIRQCB] = d->pci_irq_reset_mappings[1];
+    pci_conf[PIIX_PIRQCC] = d->pci_irq_reset_mappings[2];
+    pci_conf[PIIX_PIRQCD] = d->pci_irq_reset_mappings[3];
     pci_conf[0x69] = 0x02;
     pci_conf[0x70] = 0x80;
     pci_conf[0x76] = 0x0c;
@@ -299,6 +301,10 @@ static void piix4_init(Object *obj)
 
 static Property piix4_props[] = {
     DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
+    DEFINE_PROP_UINT8("pirqa", PIIX4State, pci_irq_reset_mappings[0], 0x80),
+    DEFINE_PROP_UINT8("pirqb", PIIX4State, pci_irq_reset_mappings[1], 0x80),
+    DEFINE_PROP_UINT8("pirqc", PIIX4State, pci_irq_reset_mappings[2], 0x80),
+    DEFINE_PROP_UINT8("pirqd", PIIX4State, pci_irq_reset_mappings[3], 0x80),
     DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
     DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
     DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 6339b0d66c..44b6b14f3d 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1403,6 +1403,10 @@ void mips_malta_init(MachineState *machine)
     piix4 = pci_new_multifunction(PCI_DEVFN(10, 0), true,
                                   TYPE_PIIX4_PCI_DEVICE);
     qdev_prop_set_uint32(DEVICE(piix4), "smb_io_base", 0x1100);
+    qdev_prop_set_uint8(DEVICE(piix4), "pirqa", 10);
+    qdev_prop_set_uint8(DEVICE(piix4), "pirqb", 10);
+    qdev_prop_set_uint8(DEVICE(piix4), "pirqc", 11);
+    qdev_prop_set_uint8(DEVICE(piix4), "pirqd", 11);
     pci_realize_and_unref(piix4, pci_bus, &error_fatal);
     isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
 
-- 
2.37.3

[PATCH 28/42] hw/isa/piix4: Remove unused code

[Bernhard Beschow]

The Malta board, which is the only user of PIIX4, doesn't connect to the
exported interrupt lines.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 763c98b565..3e9a84de8b 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -181,12 +181,6 @@ static void piix4_request_i8259_irq(void *opaque, int irq, int level)
     qemu_set_irq(s->cpu_intr, level);
 }
 
-static void piix4_set_i8259_irq(void *opaque, int irq, int level)
-{
-    PIIX4State *s = opaque;
-    qemu_set_irq(s->isa[irq], level);
-}
-
 static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {
@@ -230,8 +224,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    qdev_init_gpio_in_named(DEVICE(dev), piix4_set_i8259_irq,
-                            "isa", ISA_NUM_IRQS);
     qdev_init_gpio_out_named(DEVICE(dev), &s->cpu_intr,
                              "intr", 1);
 
-- 
2.37.3

[PATCH 29/42] hw/isa/piix4: Use ISA PIC device

[Bernhard Beschow]

Aligns the code with PIIX3 such that PIIXState can be used in PIIX4,
too. Furthermore, using the isa-pic device in PIIX4 could allow the
Malta board to gain KVM accelleration capabilities.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c  | 28 ++++++++++------------------
 hw/mips/malta.c | 11 +++++++++--
 2 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 3e9a84de8b..128284bd0a 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -43,9 +43,8 @@
 
 struct PIIX4State {
     PCIDevice dev;
-    qemu_irq cpu_intr;
-    qemu_irq *isa;
 
+    ISAPICState pic;
     RTCState rtc;
     PCIIDEState ide;
     UHCIState uhci;
@@ -83,7 +82,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int level)
                 pic_level |= pci_bus_get_irq_level(bus, i);
             }
         }
-        qemu_set_irq(s->isa[pic_irq], pic_level);
+        qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
     }
 }
 
@@ -175,12 +174,6 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
-static void piix4_request_i8259_irq(void *opaque, int irq, int level)
-{
-    PIIX4State *s = opaque;
-    qemu_set_irq(s->cpu_intr, level);
-}
-
 static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {
@@ -216,7 +209,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     PIIX4State *s = PIIX4_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
-    qemu_irq *i8259_out_irq;
 
     isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
                           pci_address_space_io(dev), errp);
@@ -224,20 +216,18 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    qdev_init_gpio_out_named(DEVICE(dev), &s->cpu_intr,
-                             "intr", 1);
-
     memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
                           "reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &s->rcr_mem, 1);
 
     /* initialize i8259 pic */
-    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    s->isa = i8259_init(isa_bus, *i8259_out_irq);
+    if (!qdev_realize(DEVICE(&s->pic), BUS(isa_bus), errp)) {
+        return;
+    }
 
     /* initialize ISA irqs */
-    isa_bus_irqs(isa_bus, s->isa);
+    isa_bus_irqs(isa_bus, s->pic.in_irqs);
 
     /* initialize pit */
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
@@ -250,7 +240,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
         return;
     }
-    s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq);
+    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
 
     /* IDE */
     qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
@@ -277,7 +267,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
             return;
         }
-        qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
+        qdev_connect_gpio_out(DEVICE(&s->pm), 0,
+                              qdev_get_gpio_in(DEVICE(&s->pic), 9));
     }
 
     pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
@@ -287,6 +278,7 @@ static void piix4_init(Object *obj)
 {
     PIIX4State *s = PIIX4_PCI_DEVICE(obj);
 
+    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
 }
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 44b6b14f3d..68e800b00f 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -28,6 +28,7 @@
 #include "qemu/datadir.h"
 #include "hw/clock.h"
 #include "hw/southbridge/piix.h"
+#include "hw/intc/i8259.h"
 #include "hw/isa/superio.h"
 #include "hw/char/serial.h"
 #include "net/net.h"
@@ -1232,10 +1233,11 @@ void mips_malta_init(MachineState *machine)
     PCIBus *pci_bus;
     ISABus *isa_bus;
     qemu_irq cbus_irq, i8259_irq;
+    qemu_irq *i8259;
     I2CBus *smbus;
     DriveInfo *dinfo;
     int fl_idx = 0;
-    int be;
+    int be, i;
     MaltaState *s;
     PCIDevice *piix4;
     DeviceState *dev;
@@ -1414,7 +1416,12 @@ void mips_malta_init(MachineState *machine)
     pci_ide_create_devs(PCI_DEVICE(dev));
 
     /* Interrupt controller */
-    qdev_connect_gpio_out_named(DEVICE(piix4), "intr", 0, i8259_irq);
+    dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pic"));
+    i8259 = i8259_init(isa_bus, i8259_irq);
+    for (i = 0; i < ISA_NUM_IRQS; i++) {
+        qdev_connect_gpio_out(dev, i, i8259[i]);
+    }
+    g_free(i8259);
 
     /* generate SPD EEPROM data */
     dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pm"));
-- 
2.37.3

[PATCH 30/42] hw/isa/piix4: Reuse struct PIIXState from PIIX3

[Bernhard Beschow]

Now that PIIX4 also uses the "isa-pic" proxy, both implementations
can share the same struct.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c | 61 ++++++++++++++++----------------------------------
 1 file changed, 19 insertions(+), 42 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 128284bd0a..95e4a9f3c1 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -41,34 +41,10 @@
 #include "sysemu/runstate.h"
 #include "qom/object.h"
 
-struct PIIX4State {
-    PCIDevice dev;
-
-    ISAPICState pic;
-    RTCState rtc;
-    PCIIDEState ide;
-    UHCIState uhci;
-    PIIX4PMState pm;
-
-    uint32_t smb_io_base;
-
-    /* Reset Control Register */
-    MemoryRegion rcr_mem;
-    uint8_t rcr;
-
-    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
-
-    bool has_acpi;
-    bool has_usb;
-    bool smm_enabled;
-};
-
-OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
-
 static void piix4_set_irq(void *opaque, int irq_num, int level)
 {
     int i, pic_irq, pic_level;
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
     PCIBus *bus = pci_get_bus(&s->dev);
 
     /* now we change the pic irq level according to the piix irq mappings */
@@ -113,7 +89,7 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
 
 static void piix4_isa_reset(DeviceState *dev)
 {
-    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
+    PIIXState *d = PIIX_PCI_DEVICE(dev);
     uint8_t *pci_conf = d->dev.config;
 
     pci_conf[0x04] = 0x07; // master, memory and I/O
@@ -148,12 +124,13 @@ static void piix4_isa_reset(DeviceState *dev)
     pci_conf[0xac] = 0x00;
     pci_conf[0xae] = 0x00;
 
+    d->pic_levels = 0; /* not used in PIIX4 */
     d->rcr = 0;
 }
 
 static int piix4_ide_post_load(void *opaque, int version_id)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
 
     if (version_id == 2) {
         s->rcr = 0;
@@ -168,8 +145,8 @@ static const VMStateDescription vmstate_piix4 = {
     .minimum_version_id = 2,
     .post_load = piix4_ide_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, PIIX4State),
-        VMSTATE_UINT8_V(rcr, PIIX4State, 3),
+        VMSTATE_PCI_DEVICE(dev, PIIXState),
+        VMSTATE_UINT8_V(rcr, PIIXState, 3),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -177,7 +154,7 @@ static const VMStateDescription vmstate_piix4 = {
 static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
 
     if (val & 4) {
         qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
@@ -189,7 +166,7 @@ static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
 
 static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
 
     return s->rcr;
 }
@@ -206,7 +183,7 @@ static const MemoryRegionOps piix4_rcr_ops = {
 
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
-    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
+    PIIXState *s = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
 
@@ -276,7 +253,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 
 static void piix4_init(Object *obj)
 {
-    PIIX4State *s = PIIX4_PCI_DEVICE(obj);
+    PIIXState *s = PIIX_PCI_DEVICE(obj);
 
     object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
@@ -284,14 +261,14 @@ static void piix4_init(Object *obj)
 }
 
 static Property piix4_props[] = {
-    DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
-    DEFINE_PROP_UINT8("pirqa", PIIX4State, pci_irq_reset_mappings[0], 0x80),
-    DEFINE_PROP_UINT8("pirqb", PIIX4State, pci_irq_reset_mappings[1], 0x80),
-    DEFINE_PROP_UINT8("pirqc", PIIX4State, pci_irq_reset_mappings[2], 0x80),
-    DEFINE_PROP_UINT8("pirqd", PIIX4State, pci_irq_reset_mappings[3], 0x80),
-    DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
-    DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
-    DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
+    DEFINE_PROP_UINT32("smb_io_base", PIIXState, smb_io_base, 0),
+    DEFINE_PROP_UINT8("pirqa", PIIXState, pci_irq_reset_mappings[0], 0x80),
+    DEFINE_PROP_UINT8("pirqb", PIIXState, pci_irq_reset_mappings[1], 0x80),
+    DEFINE_PROP_UINT8("pirqc", PIIXState, pci_irq_reset_mappings[2], 0x80),
+    DEFINE_PROP_UINT8("pirqd", PIIXState, pci_irq_reset_mappings[3], 0x80),
+    DEFINE_PROP_BOOL("has-acpi", PIIXState, has_acpi, true),
+    DEFINE_PROP_BOOL("has-usb", PIIXState, has_usb, true),
+    DEFINE_PROP_BOOL("smm-enabled", PIIXState, smm_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -319,7 +296,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
 static const TypeInfo piix4_info = {
     .name          = TYPE_PIIX4_PCI_DEVICE,
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIX4State),
+    .instance_size = sizeof(PIIXState),
     .instance_init = piix4_init,
     .class_init    = piix4_class_init,
     .interfaces = (InterfaceInfo[]) {
-- 
2.37.3

[PATCH 31/42] hw/isa/piix4: Rename reset control operations to match PIIX3

[Bernhard Beschow]

Both implementations are the same and will be shared upon merging.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 95e4a9f3c1..e682370887 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -151,7 +151,7 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
-static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
+static void rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {
     PIIXState *s = opaque;
@@ -164,16 +164,16 @@ static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
     s->rcr = val & 2; /* keep System Reset type only */
 }
 
-static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
+static uint64_t rcr_read(void *opaque, hwaddr addr, unsigned int len)
 {
     PIIXState *s = opaque;
 
     return s->rcr;
 }
 
-static const MemoryRegionOps piix4_rcr_ops = {
-    .read = piix4_rcr_read,
-    .write = piix4_rcr_write,
+static const MemoryRegionOps rcr_ops = {
+    .read = rcr_read,
+    .write = rcr_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
         .min_access_size = 1,
@@ -193,7 +193,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
+    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
                           "reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &s->rcr_mem, 1);
-- 
2.37.3

[PATCH 32/42] hw/isa/piix4: Rename wrongly named method

[Bernhard Beschow]

This method post-loads the southbridge, not the IDE device.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index e682370887..72bd9ad74d 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -128,7 +128,7 @@ static void piix4_isa_reset(DeviceState *dev)
     d->rcr = 0;
 }
 
-static int piix4_ide_post_load(void *opaque, int version_id)
+static int piix4_post_load(void *opaque, int version_id)
 {
     PIIXState *s = opaque;
 
@@ -143,7 +143,7 @@ static const VMStateDescription vmstate_piix4 = {
     .name = "PIIX4",
     .version_id = 3,
     .minimum_version_id = 2,
-    .post_load = piix4_ide_post_load,
+    .post_load = piix4_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(dev, PIIXState),
         VMSTATE_UINT8_V(rcr, PIIXState, 3),
-- 
2.37.3

[PATCH 33/42] hw/isa/piix4: Prefix pci_slot_get_pirq() with "piix4_"

[Bernhard Beschow]

Prefixing with "piix4_" makes the method distinguishable from its
PIIX3 counterpart upon merging and also complies more with QEMU
conventions.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix4.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 72bd9ad74d..01a98990d6 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -62,7 +62,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int level)
     }
 }
 
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+static int piix4_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
 {
     int slot;
 
@@ -248,7 +248,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
                               qdev_get_gpio_in(DEVICE(&s->pic), 9));
     }
 
-    pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, piix4_set_irq, piix4_pci_slot_get_pirq, s,
+                 PIIX_NUM_PIRQS);
 }
 
 static void piix4_init(Object *obj)
-- 
2.37.3

[PATCH 34/42] hw/isa/piix3: Merge hw/isa/piix4.c

[Bernhard Beschow]

Now that the PIIX3 and PIIX4 device models are sufficiently consolidated

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 MAINTAINERS                             |   6 +-
 configs/devices/mips-softmmu/common.mak |   2 +-
 hw/i386/Kconfig                         |   2 +-
 hw/isa/Kconfig                          |  12 +-
 hw/isa/meson.build                      |   3 +-
 hw/isa/{piix3.c => piix.c}              | 185 ++++++++++++++
 hw/isa/piix4.c                          | 314 ------------------------
 7 files changed, 192 insertions(+), 332 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (73%)
 delete mode 100644 hw/isa/piix4.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5ce4227ff6..f727f64a25 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1228,7 +1228,7 @@ Malta
 M: Philippe Mathieu-Daudé <f4bug@amsat.org>
 R: Aurelien Jarno <aurelien@aurel32.net>
 S: Odd Fixes
-F: hw/isa/piix4.c
+F: hw/isa/piix.c
 F: hw/acpi/piix4.c
 F: hw/mips/malta.c
 F: hw/mips/gt64xxx_pci.c
@@ -1643,7 +1643,7 @@ F: hw/pci-host/pam.c
 F: include/hw/pci-host/i440fx.h
 F: include/hw/pci-host/q35.h
 F: include/hw/pci-host/pam.h
-F: hw/isa/piix3.c
+F: hw/isa/piix.c
 F: hw/isa/lpc_ich9.c
 F: hw/i2c/smbus_ich9.c
 F: hw/acpi/piix4.c
@@ -2302,7 +2302,7 @@ PIIX4 South Bridge (i82371AB)
 M: Hervé Poussineau <hpoussin@reactos.org>
 M: Philippe Mathieu-Daudé <f4bug@amsat.org>
 S: Maintained
-F: hw/isa/piix4.c
+F: hw/isa/piix.c
 F: include/hw/southbridge/piix.h
 
 Firmware configuration (fw_cfg)
diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
index 416161f833..ef3b7390a6 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -21,7 +21,7 @@ CONFIG_ACPI=y
 CONFIG_ACPI_PIIX4=y
 CONFIG_APM=y
 CONFIG_I8257=y
-CONFIG_PIIX4=y
+CONFIG_PIIX=y
 CONFIG_IDE_ISA=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_I8259=y
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index dd247f215c..295693b32b 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -73,7 +73,7 @@ config I440FX
     select PC_ACPI
     select ACPI_SMBUS
     select PCI_I440FX
-    select PIIX3
+    select PIIX
     select DIMM
     select SMBIOS
     select FW_CFG_DMA
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 1aa10f84f2..000c2312ab 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -31,17 +31,7 @@ config PC87312
     select FDC_ISA
     select IDE_ISA
 
-config PIIX3
-    bool
-    select ACPI_PIIX4
-    select I8257
-    select I8259
-    select IDE_PIIX
-    select ISA_BUS
-    select MC146818RTC
-    select USB_UHCI
-
-config PIIX4
+config PIIX
     bool
     # For historical reasons, SuperIO devices are created in the board
     # for PIIX4.
diff --git a/hw/isa/meson.build b/hw/isa/meson.build
index 8bf678ca0a..314bbd0860 100644
--- a/hw/isa/meson.build
+++ b/hw/isa/meson.build
@@ -3,8 +3,7 @@ softmmu_ss.add(when: 'CONFIG_I82378', if_true: files('i82378.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
 softmmu_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
-softmmu_ss.add(when: 'CONFIG_PIIX3', if_true: files('piix3.c'))
-softmmu_ss.add(when: 'CONFIG_PIIX4', if_true: files('piix4.c'))
+softmmu_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
 softmmu_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
 softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
 
diff --git a/hw/isa/piix3.c b/hw/isa/piix.c
similarity index 73%
rename from hw/isa/piix3.c
rename to hw/isa/piix.c
index 75705a1fc1..5d3715b64e 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix.c
@@ -2,6 +2,7 @@
  * QEMU PIIX PCI ISA Bridge Emulation
  *
  * Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2018 Hervé Poussineau
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -26,7 +27,9 @@
 #include "qemu/range.h"
 #include "qapi/error.h"
 #include "hw/dma/i8257.h"
+#include "hw/intc/i8259.h"
 #include "hw/southbridge/piix.h"
+#include "hw/timer/i8254.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
@@ -80,6 +83,27 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
     piix3_set_irq_level(piix3, pirq, level);
 }
 
+static void piix4_set_irq(void *opaque, int irq_num, int level)
+{
+    int i, pic_irq, pic_level;
+    PIIXState *s = opaque;
+    PCIBus *bus = pci_get_bus(&s->dev);
+
+    /* now we change the pic irq level according to the piix irq mappings */
+    /* XXX: optimize */
+    pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
+    if (pic_irq < ISA_NUM_IRQS) {
+        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
+        pic_level = 0;
+        for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+            if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
+                pic_level |= pci_bus_get_irq_level(bus, i);
+            }
+        }
+        qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
+    }
+}
+
 /*
  * Return the global irq number corresponding to a given device irq
  * pin. We could also use the bus number to have a more precise mapping.
@@ -91,6 +115,31 @@ static int piix3_pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
     return (pci_intx + slot_addend) & 3;
 }
 
+static int piix4_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+    int slot;
+
+    slot = PCI_SLOT(pci_dev->devfn);
+
+    switch (slot) {
+    /* PIIX4 USB */
+    case 10:
+        return 3;
+    /* AMD 79C973 Ethernet */
+    case 11:
+        return 1;
+    /* Crystal 4281 Sound */
+    case 12:
+        return 2;
+    /* PCI slot 1 to 4 */
+    case 18 ... 21:
+        return ((slot - 18) + irq_num) & 0x03;
+    /* Unknown device, don't do any translation */
+    default:
+        return irq_num;
+    }
+}
+
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
     PIIXState *piix3 = opaque;
@@ -218,6 +267,17 @@ static int piix3_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static int piix4_post_load(void *opaque, int version_id)
+{
+    PIIXState *s = opaque;
+
+    if (version_id == 2) {
+        s->rcr = 0;
+    }
+
+    return 0;
+}
+
 static int piix3_pre_save(void *opaque)
 {
     int i;
@@ -267,6 +327,17 @@ static const VMStateDescription vmstate_piix3 = {
     }
 };
 
+static const VMStateDescription vmstate_piix4 = {
+    .name = "PIIX4",
+    .version_id = 3,
+    .minimum_version_id = 2,
+    .post_load = piix4_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PIIXState),
+        VMSTATE_UINT8_V(rcr, PIIXState, 3),
+        VMSTATE_END_OF_LIST()
+    }
+};
 
 static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
 {
@@ -494,11 +565,125 @@ static const TypeInfo piix3_xen_info = {
     .class_init    = piix3_xen_class_init,
 };
 
+static void piix4_realize(PCIDevice *dev, Error **errp)
+{
+    PIIXState *s = PIIX_PCI_DEVICE(dev);
+    PCIBus *pci_bus = pci_get_bus(dev);
+    ISABus *isa_bus;
+
+    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
+                          pci_address_space_io(dev), errp);
+    if (!isa_bus) {
+        return;
+    }
+
+    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
+                          "reset-control", 1);
+    memory_region_add_subregion_overlap(pci_address_space_io(dev),
+                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
+
+    /* initialize i8259 pic */
+    if (!qdev_realize(DEVICE(&s->pic), BUS(isa_bus), errp)) {
+        return;
+    }
+
+    /* initialize ISA irqs */
+    isa_bus_irqs(isa_bus, s->pic.in_irqs);
+
+    /* initialize pit */
+    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+
+    /* DMA */
+    i8257_dma_init(isa_bus, 0);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
+    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
+
+    /* IDE */
+    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
+    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
+        return;
+    }
+
+    /* USB */
+    if (s->has_usb) {
+        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
+                                "piix4-usb-uhci");
+        qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
+        if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
+            return;
+        }
+    }
+
+    /* ACPI controller */
+    if (s->has_acpi) {
+        object_initialize_child(OBJECT(s), "pm", &s->pm, TYPE_PIIX4_PM);
+        qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
+        qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", s->smb_io_base);
+        qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", s->smm_enabled);
+        if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
+            return;
+        }
+        qdev_connect_gpio_out(DEVICE(&s->pm), 0,
+                              qdev_get_gpio_in(DEVICE(&s->pic), 9));
+    }
+
+    pci_bus_irqs(pci_bus, piix4_set_irq, piix4_pci_slot_get_pirq, s,
+                 PIIX_NUM_PIRQS);
+}
+
+static void piix4_init(Object *obj)
+{
+    PIIXState *s = PIIX_PCI_DEVICE(obj);
+
+    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
+    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
+    object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
+}
+
+static void piix4_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->realize = piix4_realize;
+    k->vendor_id = PCI_VENDOR_ID_INTEL;
+    k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
+    k->class_id = PCI_CLASS_BRIDGE_ISA;
+    dc->reset = piix_reset;
+    dc->desc = "ISA bridge";
+    dc->vmsd = &vmstate_piix4;
+    /*
+     * Reason: part of PIIX4 southbridge, needs to be wired up,
+     * e.g. by mips_malta_init()
+     */
+    dc->user_creatable = false;
+    dc->hotpluggable = false;
+    device_class_set_props(dc, pci_piix_props);
+}
+
+static const TypeInfo piix4_info = {
+    .name          = TYPE_PIIX4_PCI_DEVICE,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PIIXState),
+    .instance_init = piix4_init,
+    .class_init    = piix4_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
 static void piix3_register_types(void)
 {
     type_register_static(&piix3_pci_type_info);
     type_register_static(&piix3_info);
     type_register_static(&piix3_xen_info);
+    type_register_static(&piix4_info);
 }
 
 type_init(piix3_register_types)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
deleted file mode 100644
index 01a98990d6..0000000000
--- a/hw/isa/piix4.c
+++ /dev/null
@@ -1,314 +0,0 @@
-/*
- * QEMU PIIX4 PCI Bridge Emulation
- *
- * Copyright (c) 2006 Fabrice Bellard
- * Copyright (c) 2018 Hervé Poussineau
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include "qemu/osdep.h"
-#include "qapi/error.h"
-#include "hw/irq.h"
-#include "hw/southbridge/piix.h"
-#include "hw/pci/pci.h"
-#include "hw/isa/isa.h"
-#include "hw/intc/i8259.h"
-#include "hw/dma/i8257.h"
-#include "hw/timer/i8254.h"
-#include "hw/rtc/mc146818rtc.h"
-#include "hw/ide/pci.h"
-#include "hw/acpi/piix4.h"
-#include "hw/usb/hcd-uhci.h"
-#include "migration/vmstate.h"
-#include "sysemu/reset.h"
-#include "sysemu/runstate.h"
-#include "qom/object.h"
-
-static void piix4_set_irq(void *opaque, int irq_num, int level)
-{
-    int i, pic_irq, pic_level;
-    PIIXState *s = opaque;
-    PCIBus *bus = pci_get_bus(&s->dev);
-
-    /* now we change the pic irq level according to the piix irq mappings */
-    /* XXX: optimize */
-    pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
-    if (pic_irq < ISA_NUM_IRQS) {
-        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
-        pic_level = 0;
-        for (i = 0; i < PIIX_NUM_PIRQS; i++) {
-            if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
-                pic_level |= pci_bus_get_irq_level(bus, i);
-            }
-        }
-        qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
-    }
-}
-
-static int piix4_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
-{
-    int slot;
-
-    slot = PCI_SLOT(pci_dev->devfn);
-
-    switch (slot) {
-    /* PIIX4 USB */
-    case 10:
-        return 3;
-    /* AMD 79C973 Ethernet */
-    case 11:
-        return 1;
-    /* Crystal 4281 Sound */
-    case 12:
-        return 2;
-    /* PCI slot 1 to 4 */
-    case 18 ... 21:
-        return ((slot - 18) + irq_num) & 0x03;
-    /* Unknown device, don't do any translation */
-    default:
-        return irq_num;
-    }
-}
-
-static void piix4_isa_reset(DeviceState *dev)
-{
-    PIIXState *d = PIIX_PCI_DEVICE(dev);
-    uint8_t *pci_conf = d->dev.config;
-
-    pci_conf[0x04] = 0x07; // master, memory and I/O
-    pci_conf[0x05] = 0x00;
-    pci_conf[0x06] = 0x00;
-    pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
-    pci_conf[0x4c] = 0x4d;
-    pci_conf[0x4e] = 0x03;
-    pci_conf[0x4f] = 0x00;
-    pci_conf[PIIX_PIRQCA] = d->pci_irq_reset_mappings[0];
-    pci_conf[PIIX_PIRQCB] = d->pci_irq_reset_mappings[1];
-    pci_conf[PIIX_PIRQCC] = d->pci_irq_reset_mappings[2];
-    pci_conf[PIIX_PIRQCD] = d->pci_irq_reset_mappings[3];
-    pci_conf[0x69] = 0x02;
-    pci_conf[0x70] = 0x80;
-    pci_conf[0x76] = 0x0c;
-    pci_conf[0x77] = 0x0c;
-    pci_conf[0x78] = 0x02;
-    pci_conf[0x79] = 0x00;
-    pci_conf[0x80] = 0x00;
-    pci_conf[0x82] = 0x00;
-    pci_conf[0xa0] = 0x08;
-    pci_conf[0xa2] = 0x00;
-    pci_conf[0xa3] = 0x00;
-    pci_conf[0xa4] = 0x00;
-    pci_conf[0xa5] = 0x00;
-    pci_conf[0xa6] = 0x00;
-    pci_conf[0xa7] = 0x00;
-    pci_conf[0xa8] = 0x0f;
-    pci_conf[0xaa] = 0x00;
-    pci_conf[0xab] = 0x00;
-    pci_conf[0xac] = 0x00;
-    pci_conf[0xae] = 0x00;
-
-    d->pic_levels = 0; /* not used in PIIX4 */
-    d->rcr = 0;
-}
-
-static int piix4_post_load(void *opaque, int version_id)
-{
-    PIIXState *s = opaque;
-
-    if (version_id == 2) {
-        s->rcr = 0;
-    }
-
-    return 0;
-}
-
-static const VMStateDescription vmstate_piix4 = {
-    .name = "PIIX4",
-    .version_id = 3,
-    .minimum_version_id = 2,
-    .post_load = piix4_post_load,
-    .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, PIIXState),
-        VMSTATE_UINT8_V(rcr, PIIXState, 3),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static void rcr_write(void *opaque, hwaddr addr, uint64_t val,
-                            unsigned int len)
-{
-    PIIXState *s = opaque;
-
-    if (val & 4) {
-        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
-        return;
-    }
-
-    s->rcr = val & 2; /* keep System Reset type only */
-}
-
-static uint64_t rcr_read(void *opaque, hwaddr addr, unsigned int len)
-{
-    PIIXState *s = opaque;
-
-    return s->rcr;
-}
-
-static const MemoryRegionOps rcr_ops = {
-    .read = rcr_read,
-    .write = rcr_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    },
-};
-
-static void piix4_realize(PCIDevice *dev, Error **errp)
-{
-    PIIXState *s = PIIX_PCI_DEVICE(dev);
-    PCIBus *pci_bus = pci_get_bus(dev);
-    ISABus *isa_bus;
-
-    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
-                          pci_address_space_io(dev), errp);
-    if (!isa_bus) {
-        return;
-    }
-
-    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
-                          "reset-control", 1);
-    memory_region_add_subregion_overlap(pci_address_space_io(dev),
-                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
-
-    /* initialize i8259 pic */
-    if (!qdev_realize(DEVICE(&s->pic), BUS(isa_bus), errp)) {
-        return;
-    }
-
-    /* initialize ISA irqs */
-    isa_bus_irqs(isa_bus, s->pic.in_irqs);
-
-    /* initialize pit */
-    i8254_pit_init(isa_bus, 0x40, 0, NULL);
-
-    /* DMA */
-    i8257_dma_init(isa_bus, 0);
-
-    /* RTC */
-    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
-    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
-        return;
-    }
-    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
-
-    /* IDE */
-    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
-    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
-        return;
-    }
-
-    /* USB */
-    if (s->has_usb) {
-        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
-                                "piix4-usb-uhci");
-        qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
-        if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
-            return;
-        }
-    }
-
-    /* ACPI controller */
-    if (s->has_acpi) {
-        object_initialize_child(OBJECT(s), "pm", &s->pm, TYPE_PIIX4_PM);
-        qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
-        qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", s->smb_io_base);
-        qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", s->smm_enabled);
-        if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
-            return;
-        }
-        qdev_connect_gpio_out(DEVICE(&s->pm), 0,
-                              qdev_get_gpio_in(DEVICE(&s->pic), 9));
-    }
-
-    pci_bus_irqs(pci_bus, piix4_set_irq, piix4_pci_slot_get_pirq, s,
-                 PIIX_NUM_PIRQS);
-}
-
-static void piix4_init(Object *obj)
-{
-    PIIXState *s = PIIX_PCI_DEVICE(obj);
-
-    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
-    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
-    object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
-}
-
-static Property piix4_props[] = {
-    DEFINE_PROP_UINT32("smb_io_base", PIIXState, smb_io_base, 0),
-    DEFINE_PROP_UINT8("pirqa", PIIXState, pci_irq_reset_mappings[0], 0x80),
-    DEFINE_PROP_UINT8("pirqb", PIIXState, pci_irq_reset_mappings[1], 0x80),
-    DEFINE_PROP_UINT8("pirqc", PIIXState, pci_irq_reset_mappings[2], 0x80),
-    DEFINE_PROP_UINT8("pirqd", PIIXState, pci_irq_reset_mappings[3], 0x80),
-    DEFINE_PROP_BOOL("has-acpi", PIIXState, has_acpi, true),
-    DEFINE_PROP_BOOL("has-usb", PIIXState, has_usb, true),
-    DEFINE_PROP_BOOL("smm-enabled", PIIXState, smm_enabled, false),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void piix4_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->realize = piix4_realize;
-    k->vendor_id = PCI_VENDOR_ID_INTEL;
-    k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
-    k->class_id = PCI_CLASS_BRIDGE_ISA;
-    dc->reset = piix4_isa_reset;
-    dc->desc = "ISA bridge";
-    dc->vmsd = &vmstate_piix4;
-    /*
-     * Reason: part of PIIX4 southbridge, needs to be wired up,
-     * e.g. by mips_malta_init()
-     */
-    dc->user_creatable = false;
-    dc->hotpluggable = false;
-    device_class_set_props(dc, piix4_props);
-}
-
-static const TypeInfo piix4_info = {
-    .name          = TYPE_PIIX4_PCI_DEVICE,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIXState),
-    .instance_init = piix4_init,
-    .class_init    = piix4_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
-static void piix4_register_types(void)
-{
-    type_register_static(&piix4_info);
-}
-
-type_init(piix4_register_types)
-- 
2.37.3

[PATCH 35/42] hw/isa/piix: Harmonize names of reset control memory regions

[Bernhard Beschow]

There is no need for having different names here. Having the same name
further allows code to be shared between PIIX3 and PIIX4.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 5d3715b64e..ba7b5d953f 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -387,7 +387,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     isa_bus_irqs(isa_bus, d->pic.in_irqs);
 
     memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
-                          "piix3-reset-control", 1);
+                          "piix-reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
@@ -578,7 +578,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     }
 
     memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
-                          "reset-control", 1);
+                          "piix-reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &s->rcr_mem, 1);
 
-- 
2.37.3

[PATCH 36/42] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4

[Bernhard Beschow]

Resolves duplicate code.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c | 65 +++++++--------------------------------------------
 1 file changed, 9 insertions(+), 56 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index ba7b5d953f..af6d920eff 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -367,7 +367,8 @@ static const MemoryRegionOps rcr_ops = {
     },
 };
 
-static void pci_piix3_realize(PCIDevice *dev, Error **errp)
+static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
+                             Error **errp)
 {
     PIIXState *d = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
@@ -407,8 +408,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 
     /* USB */
     if (d->has_usb) {
-        object_initialize_child(OBJECT(dev), "uhci", &d->uhci,
-                                "piix3-usb-uhci");
+        object_initialize_child(OBJECT(dev), "uhci", &d->uhci, uhci_type);
         qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2);
         if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) {
             return;
@@ -506,7 +506,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
     PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
-    pci_piix3_realize(dev, errp);
+    pci_piix_realize(dev, "piix3-usb-uhci", errp);
     if (*errp) {
         return;
     }
@@ -536,7 +536,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
     PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
-    pci_piix3_realize(dev, errp);
+    pci_piix_realize(dev, "piix3-usb-uhci", errp);
     if (*errp) {
         return;
     }
@@ -567,71 +567,24 @@ static const TypeInfo piix3_xen_info = {
 
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
+    ERRP_GUARD();
     PIIXState *s = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
 
-    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
-                          pci_address_space_io(dev), errp);
-    if (!isa_bus) {
-        return;
-    }
-
-    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
-                          "piix-reset-control", 1);
-    memory_region_add_subregion_overlap(pci_address_space_io(dev),
-                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
-
-    /* initialize i8259 pic */
-    if (!qdev_realize(DEVICE(&s->pic), BUS(isa_bus), errp)) {
+    pci_piix_realize(dev, "piix4-usb-uhci", errp);
+    if (*errp) {
         return;
     }
 
-    /* initialize ISA irqs */
-    isa_bus_irqs(isa_bus, s->pic.in_irqs);
+    isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(dev), "isa.0"));
 
     /* initialize pit */
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
 
-    /* DMA */
-    i8257_dma_init(isa_bus, 0);
-
     /* RTC */
-    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
-    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
-        return;
-    }
     s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
 
-    /* IDE */
-    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
-    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
-        return;
-    }
-
-    /* USB */
-    if (s->has_usb) {
-        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
-                                "piix4-usb-uhci");
-        qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
-        if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
-            return;
-        }
-    }
-
-    /* ACPI controller */
-    if (s->has_acpi) {
-        object_initialize_child(OBJECT(s), "pm", &s->pm, TYPE_PIIX4_PM);
-        qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
-        qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", s->smb_io_base);
-        qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", s->smm_enabled);
-        if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
-            return;
-        }
-        qdev_connect_gpio_out(DEVICE(&s->pm), 0,
-                              qdev_get_gpio_in(DEVICE(&s->pic), 9));
-    }
-
     pci_bus_irqs(pci_bus, piix4_set_irq, piix4_pci_slot_get_pirq, s,
                  PIIX_NUM_PIRQS);
 }
-- 
2.37.3

[PATCH 37/42] hw/isa/piix: Rename functions to be shared for interrupt triggering

[Bernhard Beschow]

PIIX4 will get the same optimizations which are already implemented for
PIIX3.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c | 56 +++++++++++++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index af6d920eff..1d516de5cc 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -40,47 +40,47 @@
 
 #define XEN_PIIX_NUM_PIRQS      128ULL
 
-static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
+static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
 {
-    qemu_set_irq(piix3->pic.in_irqs[pic_irq],
-                 !!(piix3->pic_levels &
+    qemu_set_irq(piix->pic.in_irqs[pic_irq],
+                 !!(piix->pic_levels &
                     (((1ULL << PIIX_NUM_PIRQS) - 1) <<
                      (pic_irq * PIIX_NUM_PIRQS))));
 }
 
-static void piix3_set_irq_level_internal(PIIXState *piix3, int pirq, int level)
+static void piix_set_irq_level_internal(PIIXState *piix, int pirq, int level)
 {
     int pic_irq;
     uint64_t mask;
 
-    pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+    pic_irq = piix->dev.config[PIIX_PIRQCA + pirq];
     if (pic_irq >= ISA_NUM_IRQS) {
         return;
     }
 
     mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq);
-    piix3->pic_levels &= ~mask;
-    piix3->pic_levels |= mask * !!level;
+    piix->pic_levels &= ~mask;
+    piix->pic_levels |= mask * !!level;
 }
 
-static void piix3_set_irq_level(PIIXState *piix3, int pirq, int level)
+static void piix_set_irq_level(PIIXState *piix, int pirq, int level)
 {
     int pic_irq;
 
-    pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+    pic_irq = piix->dev.config[PIIX_PIRQCA + pirq];
     if (pic_irq >= ISA_NUM_IRQS) {
         return;
     }
 
-    piix3_set_irq_level_internal(piix3, pirq, level);
+    piix_set_irq_level_internal(piix, pirq, level);
 
-    piix3_set_irq_pic(piix3, pic_irq);
+    piix_set_irq_pic(piix, pic_irq);
 }
 
-static void piix3_set_irq(void *opaque, int pirq, int level)
+static void piix_set_irq(void *opaque, int pirq, int level)
 {
-    PIIXState *piix3 = opaque;
-    piix3_set_irq_level(piix3, pirq, level);
+    PIIXState *piix = opaque;
+    piix_set_irq_level(piix, pirq, level);
 }
 
 static void piix4_set_irq(void *opaque, int irq_num, int level)
@@ -157,29 +157,29 @@ static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 }
 
 /* irq routing is changed. so rebuild bitmap */
-static void piix3_update_irq_levels(PIIXState *piix3)
+static void piix_update_irq_levels(PIIXState *piix)
 {
-    PCIBus *bus = pci_get_bus(&piix3->dev);
+    PCIBus *bus = pci_get_bus(&piix->dev);
     int pirq;
 
-    piix3->pic_levels = 0;
+    piix->pic_levels = 0;
     for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
-        piix3_set_irq_level(piix3, pirq, pci_bus_get_irq_level(bus, pirq));
+        piix_set_irq_level(piix, pirq, pci_bus_get_irq_level(bus, pirq));
     }
 }
 
-static void piix3_write_config(PCIDevice *dev,
-                               uint32_t address, uint32_t val, int len)
+static void piix_write_config(PCIDevice *dev, uint32_t address, uint32_t val,
+                              int len)
 {
     pci_default_write_config(dev, address, val, len);
     if (ranges_overlap(address, len, PIIX_PIRQCA, 4)) {
-        PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
+        PIIXState *piix = PIIX_PCI_DEVICE(dev);
         int pic_irq;
 
-        pci_bus_fire_intx_routing_notifier(pci_get_bus(&piix3->dev));
-        piix3_update_irq_levels(piix3);
+        pci_bus_fire_intx_routing_notifier(pci_get_bus(&piix->dev));
+        piix_update_irq_levels(piix);
         for (pic_irq = 0; pic_irq < ISA_NUM_IRQS; pic_irq++) {
-            piix3_set_irq_pic(piix3, pic_irq);
+            piix_set_irq_pic(piix, pic_irq);
         }
     }
 }
@@ -201,7 +201,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
         }
     }
 
-    piix3_write_config(dev, address, val, len);
+    piix_write_config(dev, address, val, len);
 }
 
 static void piix_reset(DeviceState *dev)
@@ -261,7 +261,7 @@ static int piix3_post_load(void *opaque, int version_id)
      */
     piix3->pic_levels = 0;
     for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
-        piix3_set_irq_level_internal(piix3, pirq,
+        piix_set_irq_level_internal(piix3, pirq,
             pci_bus_get_irq_level(pci_get_bus(&piix3->dev), pirq));
     }
     return 0;
@@ -511,7 +511,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pci_bus_irqs(pci_bus, piix3_set_irq, piix3_pci_slot_get_pirq,
+    pci_bus_irqs(pci_bus, piix_set_irq, piix3_pci_slot_get_pirq,
                  piix3, PIIX_NUM_PIRQS);
     pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
 }
@@ -520,7 +520,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix3_write_config;
+    k->config_write = piix_write_config;
     k->realize = piix3_realize;
 }
 
-- 
2.37.3

[PATCH 38/42] hw/isa/piix: Consolidate IRQ triggering

[Bernhard Beschow]

Speeds up PIIX4 which resolves an old TODO.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 1d516de5cc..e413d7e792 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -83,27 +83,6 @@ static void piix_set_irq(void *opaque, int pirq, int level)
     piix_set_irq_level(piix, pirq, level);
 }
 
-static void piix4_set_irq(void *opaque, int irq_num, int level)
-{
-    int i, pic_irq, pic_level;
-    PIIXState *s = opaque;
-    PCIBus *bus = pci_get_bus(&s->dev);
-
-    /* now we change the pic irq level according to the piix irq mappings */
-    /* XXX: optimize */
-    pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
-    if (pic_irq < ISA_NUM_IRQS) {
-        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
-        pic_level = 0;
-        for (i = 0; i < PIIX_NUM_PIRQS; i++) {
-            if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
-                pic_level |= pci_bus_get_irq_level(bus, i);
-            }
-        }
-        qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
-    }
-}
-
 /*
  * Return the global irq number corresponding to a given device irq
  * pin. We could also use the bus number to have a more precise mapping.
@@ -275,7 +254,7 @@ static int piix4_post_load(void *opaque, int version_id)
         s->rcr = 0;
     }
 
-    return 0;
+    return piix3_post_load(opaque, version_id);
 }
 
 static int piix3_pre_save(void *opaque)
@@ -585,7 +564,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     /* RTC */
     s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
 
-    pci_bus_irqs(pci_bus, piix4_set_irq, piix4_pci_slot_get_pirq, s,
+    pci_bus_irqs(pci_bus, piix_set_irq, piix4_pci_slot_get_pirq, s,
                  PIIX_NUM_PIRQS);
 }
 
@@ -603,6 +582,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+    k->config_write = piix_write_config;
     k->realize = piix4_realize;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
-- 
2.37.3

[PATCH 39/42] hw/isa/piix: Unexport PIIXState

[Bernhard Beschow]

The - deliberately exported - components can still be accessed
via QOM properties.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c                 | 52 +++++++++++++++++++++++++++++++++
 include/hw/southbridge/piix.h | 54 -----------------------------------
 2 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index e413d7e792..c503a6e836 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -26,20 +26,72 @@
 #include "qemu/osdep.h"
 #include "qemu/range.h"
 #include "qapi/error.h"
+#include "qom/object.h"
+#include "hw/acpi/piix4.h"
 #include "hw/dma/i8257.h"
+#include "hw/ide/pci.h"
 #include "hw/intc/i8259.h"
 #include "hw/southbridge/piix.h"
 #include "hw/timer/i8254.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
+#include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
+#include "hw/rtc/mc146818rtc.h"
+#include "hw/usb/hcd-uhci.h"
 #include "hw/xen/xen.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
+#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
 #define XEN_PIIX_NUM_PIRQS      128ULL
 
+struct PIIXState {
+    PCIDevice dev;
+
+    /*
+     * bitmap to track pic levels.
+     * The pic level is the logical OR of all the PCI irqs mapped to it
+     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
+     *
+     * PIRQ is mapped to PIC pins, we track it by
+     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
+     * pic_irq * PIIX_NUM_PIRQS + pirq
+     */
+#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
+#error "unable to encode pic state in 64bit in pic_levels."
+#endif
+    uint64_t pic_levels;
+
+    /* This member isn't used. Just for save/load compatibility */
+    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
+    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
+
+    ISAPICState pic;
+    RTCState rtc;
+    PCIIDEState ide;
+    UHCIState uhci;
+    PIIX4PMState pm;
+
+    uint32_t smb_io_base;
+
+    /* Reset Control Register contents */
+    uint8_t rcr;
+
+    /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
+    MemoryRegion rcr_mem;
+
+    bool has_acpi;
+    bool has_usb;
+    bool smm_enabled;
+};
+typedef struct PIIXState PIIXState;
+
+DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
+                         TYPE_PIIX3_PCI_DEVICE)
+
 static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
 {
     qemu_set_irq(piix->pic.in_irqs[pic_irq],
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index c9fa0f1aa6..0edc23710c 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -12,14 +12,6 @@
 #ifndef HW_SOUTHBRIDGE_PIIX_H
 #define HW_SOUTHBRIDGE_PIIX_H
 
-#include "hw/pci/pci.h"
-#include "qom/object.h"
-#include "hw/acpi/piix4.h"
-#include "hw/ide/pci.h"
-#include "hw/intc/i8259.h"
-#include "hw/rtc/mc146818rtc.h"
-#include "hw/usb/hcd-uhci.h"
-
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
 #define PIIX_PIRQCA 0x60
 #define PIIX_PIRQCB 0x61
@@ -32,53 +24,7 @@
  */
 #define PIIX_RCR_IOPORT 0xcf9
 
-#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
-
-struct PIIXState {
-    PCIDevice dev;
-
-    /*
-     * bitmap to track pic levels.
-     * The pic level is the logical OR of all the PCI irqs mapped to it
-     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
-     *
-     * PIRQ is mapped to PIC pins, we track it by
-     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
-     * pic_irq * PIIX_NUM_PIRQS + pirq
-     */
-#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
-#error "unable to encode pic state in 64bit in pic_levels."
-#endif
-    uint64_t pic_levels;
-
-    /* This member isn't used. Just for save/load compatibility */
-    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
-    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
-
-    ISAPICState pic;
-    RTCState rtc;
-    PCIIDEState ide;
-    UHCIState uhci;
-    PIIX4PMState pm;
-
-    uint32_t smb_io_base;
-
-    /* Reset Control Register contents */
-    uint8_t rcr;
-
-    /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
-    MemoryRegion rcr_mem;
-
-    bool has_acpi;
-    bool has_usb;
-    bool smm_enabled;
-};
-typedef struct PIIXState PIIXState;
-
 #define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
-DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
-                         TYPE_PIIX3_PCI_DEVICE)
-
 #define TYPE_PIIX3_DEVICE "PIIX3"
 #define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
-- 
2.37.3

[PATCH 40/42] hw/isa/piix: Share PIIX3 base class with PIIX4

[Bernhard Beschow]

Having a common base class allows for substituting PIIX3 with PIIX4
and vice versa. Moreover, it makes PIIX4 implement the
acpi-dev-aml-interface.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c | 53 +++++++++++++++++++++++----------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index c503a6e836..25b86ddf17 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -473,13 +473,12 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     }
 }
 
-static void pci_piix3_init(Object *obj)
+static void pci_piix_init(Object *obj)
 {
     PIIXState *d = PIIX_PCI_DEVICE(obj);
 
     object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
-    object_initialize_child(obj, "ide", &d->ide, "piix3-ide");
 }
 
 static Property pci_piix_props[] = {
@@ -494,7 +493,7 @@ static Property pci_piix_props[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void pci_piix3_class_init(ObjectClass *klass, void *data)
+static void pci_piix_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
@@ -502,11 +501,8 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
 
     dc->reset       = piix_reset;
     dc->desc        = "ISA bridge";
-    dc->vmsd        = &vmstate_piix3;
     dc->hotpluggable   = false;
     k->vendor_id    = PCI_VENDOR_ID_INTEL;
-    /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
-    k->device_id    = PCI_DEVICE_ID_INTEL_82371SB_0;
     k->class_id     = PCI_CLASS_BRIDGE_ISA;
     /*
      * Reason: part of PIIX3 southbridge, needs to be wired up by
@@ -517,13 +513,13 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
     adevc->build_dev_aml = build_pci_isa_aml;
 }
 
-static const TypeInfo piix3_pci_type_info = {
+static const TypeInfo piix_pci_type_info = {
     .name = TYPE_PIIX3_PCI_DEVICE,
     .parent = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PIIXState),
-    .instance_init = pci_piix3_init,
+    .instance_init = pci_piix_init,
     .abstract = true,
-    .class_init = pci_piix3_class_init,
+    .class_init = pci_piix_class_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { TYPE_ACPI_DEV_AML_IF },
@@ -547,17 +543,29 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
     pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
 }
 
+static void piix3_init(Object *obj)
+{
+    PIIXState *d = PIIX_PCI_DEVICE(obj);
+
+    object_initialize_child(obj, "ide", &d->ide, "piix3-ide");
+}
+
 static void piix3_class_init(ObjectClass *klass, void *data)
 {
+    DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->config_write = piix_write_config;
     k->realize = piix3_realize;
+    /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
+    k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
+    dc->vmsd = &vmstate_piix3;
 }
 
 static const TypeInfo piix3_info = {
     .name          = TYPE_PIIX3_DEVICE,
     .parent        = TYPE_PIIX3_PCI_DEVICE,
+    .instance_init = piix3_init,
     .class_init    = piix3_class_init,
 };
 
@@ -584,15 +592,20 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
 
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
+    DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->config_write = piix3_write_config_xen;
     k->realize = piix3_xen_realize;
+    /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
+    k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
+    dc->vmsd = &vmstate_piix3;
 }
 
 static const TypeInfo piix3_xen_info = {
     .name          = TYPE_PIIX3_XEN_DEVICE,
     .parent        = TYPE_PIIX3_PCI_DEVICE,
+    .instance_init = piix3_init,
     .class_init    = piix3_xen_class_init,
 };
 
@@ -624,8 +637,6 @@ static void piix4_init(Object *obj)
 {
     PIIXState *s = PIIX_PCI_DEVICE(obj);
 
-    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
-    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
 }
 
@@ -636,36 +647,20 @@ static void piix4_class_init(ObjectClass *klass, void *data)
 
     k->config_write = piix_write_config;
     k->realize = piix4_realize;
-    k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
-    k->class_id = PCI_CLASS_BRIDGE_ISA;
-    dc->reset = piix_reset;
-    dc->desc = "ISA bridge";
     dc->vmsd = &vmstate_piix4;
-    /*
-     * Reason: part of PIIX4 southbridge, needs to be wired up,
-     * e.g. by mips_malta_init()
-     */
-    dc->user_creatable = false;
-    dc->hotpluggable = false;
-    device_class_set_props(dc, pci_piix_props);
 }
 
 static const TypeInfo piix4_info = {
     .name          = TYPE_PIIX4_PCI_DEVICE,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIXState),
+    .parent        = TYPE_PIIX3_PCI_DEVICE,
     .instance_init = piix4_init,
     .class_init    = piix4_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
 };
 
 static void piix3_register_types(void)
 {
-    type_register_static(&piix3_pci_type_info);
+    type_register_static(&piix_pci_type_info);
     type_register_static(&piix3_info);
     type_register_static(&piix3_xen_info);
     type_register_static(&piix4_info);
-- 
2.37.3

[PATCH 41/42] hw/isa/piix: Drop the "3" from the PIIX base class

[Bernhard Beschow]

Now that the base class is used for both PIIX3 and PIIX4, the "3"
became misleading.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/acpi-build.c          |  2 +-
 hw/isa/piix.c                 | 10 +++++-----
 include/hw/southbridge/piix.h |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 0355bd3dda..8af75b1e22 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1276,7 +1276,7 @@ static void build_piix4_isa_bridge(Aml *table)
      * once PCI is converted to AcpiDevAmlIf and would be ble to generate
      * AML for bridge itself
      */
-    obj = object_resolve_path_type("", TYPE_PIIX3_PCI_DEVICE, &ambiguous);
+    obj = object_resolve_path_type("", TYPE_PIIX_PCI_DEVICE, &ambiguous);
     assert(obj && !ambiguous);
 
     scope =  aml_scope("_SB.PCI0");
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 25b86ddf17..f70855541b 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -90,7 +90,7 @@ struct PIIXState {
 typedef struct PIIXState PIIXState;
 
 DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
-                         TYPE_PIIX3_PCI_DEVICE)
+                         TYPE_PIIX_PCI_DEVICE)
 
 static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
 {
@@ -514,7 +514,7 @@ static void pci_piix_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo piix_pci_type_info = {
-    .name = TYPE_PIIX3_PCI_DEVICE,
+    .name = TYPE_PIIX_PCI_DEVICE,
     .parent = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PIIXState),
     .instance_init = pci_piix_init,
@@ -564,7 +564,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo piix3_info = {
     .name          = TYPE_PIIX3_DEVICE,
-    .parent        = TYPE_PIIX3_PCI_DEVICE,
+    .parent        = TYPE_PIIX_PCI_DEVICE,
     .instance_init = piix3_init,
     .class_init    = piix3_class_init,
 };
@@ -604,7 +604,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo piix3_xen_info = {
     .name          = TYPE_PIIX3_XEN_DEVICE,
-    .parent        = TYPE_PIIX3_PCI_DEVICE,
+    .parent        = TYPE_PIIX_PCI_DEVICE,
     .instance_init = piix3_init,
     .class_init    = piix3_xen_class_init,
 };
@@ -653,7 +653,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo piix4_info = {
     .name          = TYPE_PIIX4_PCI_DEVICE,
-    .parent        = TYPE_PIIX3_PCI_DEVICE,
+    .parent        = TYPE_PIIX_PCI_DEVICE,
     .instance_init = piix4_init,
     .class_init    = piix4_class_init,
 };
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0edc23710c..60ff6d222a 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -24,7 +24,7 @@
  */
 #define PIIX_RCR_IOPORT 0xcf9
 
-#define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
+#define TYPE_PIIX_PCI_DEVICE "pci-piix"
 #define TYPE_PIIX3_DEVICE "PIIX3"
 #define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
-- 
2.37.3

[PATCH 42/42] hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI controller

[Bernhard Beschow]

Resolving the PIIX ISA bridge rather than the PIIX ACPI controller mirrors
the ICH9 code one line below.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 8af75b1e22..d7bb1ccb26 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -288,7 +288,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
 {
-    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
+    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX_PCI_DEVICE);
     Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
     assert(!!piix != !!lpc);
 
-- 
2.37.3

Re: [PATCH 01/42] hw/i386/pc: Create DMA controllers in south bridges

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> Just like in the real hardware (and in PIIX4), create the DMA
> controllers in the south bridges.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i386/pc.c      | 3 ---
>   hw/i386/pc_piix.c | 2 ++
>   hw/isa/Kconfig    | 2 ++
>   hw/isa/lpc_ich9.c | 3 +++
>   hw/isa/piix3.c    | 9 +++++++--
>   5 files changed, 14 insertions(+), 5 deletions(-)

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

Re: [PATCH 05/42] hw/isa/piix3: Create USB controller in host device

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> The USB controller is an integral part of PIIX3 (function 2). So create
> it as part of the south bridge.
> 
> Note that the USB function is optional in QEMU. This is why it gets
> object_initialize_child()'ed in realize rather than in instance_init.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i386/pc_piix.c             |  6 ++----
>   hw/isa/Kconfig                |  1 +
>   hw/isa/piix3.c                | 17 +++++++++++++++++
>   include/hw/southbridge/piix.h |  4 ++++
>   4 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index b08d946992..76ac8b2035 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -219,6 +219,8 @@ static void pc_init1(MachineState *machine,
>           pcms->bus = pci_bus;
>   
>           pci_dev = pci_new_multifunction(-1, true, type);
> +        object_property_set_bool(OBJECT(pci_dev), "has-usb",
> +                                 machine_usb(machine), &error_abort);
>           pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
>           piix3 = PIIX3_PCI_DEVICE(pci_dev);
>           piix3->pic = x86ms->gsi;
> @@ -297,10 +299,6 @@ static void pc_init1(MachineState *machine,
>       }
>   #endif
>   
> -    if (pcmc->pci_enabled && machine_usb(machine)) {
> -        pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
> -    }
> -
>       if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
>           PCIDevice *piix4_pm;
>   
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index 6e8f9cac54..f02eca3c3e 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -36,6 +36,7 @@ config PIIX3
>       select I8257
>       select ISA_BUS
>       select MC146818RTC
> +    select USB_UHCI
>   
>   config PIIX4
>       bool
> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> index 96ab7107e2..27052a5546 100644
> --- a/hw/isa/piix3.c
> +++ b/hw/isa/piix3.c
> @@ -297,6 +297,7 @@ static const MemoryRegionOps rcr_ops = {
>   static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>   {
>       PIIX3State *d = PIIX3_PCI_DEVICE(dev);
> +    PCIBus *pci_bus = pci_get_bus(dev);
>       ISABus *isa_bus;
>   
>       isa_bus = isa_bus_new(DEVICE(d), get_system_memory(),
> @@ -319,6 +320,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>       if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
>           return;
>       }
> +
> +    /* USB */
> +    if (d->has_usb) {
> +        object_initialize_child(OBJECT(dev), "uhci", &d->uhci,
> +                                "piix3-usb-uhci");
> +        qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2);
> +        if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) {
> +            return;
> +        }
> +    }
>   }
>   
>   static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
> @@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj)
>       object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
>   }
>   
> +static Property pci_piix3_props[] = {
> +    DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),

Maybe s/has-usb/usb-enabled/?

> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>   static void pci_piix3_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -359,6 +375,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
>        * pc_piix.c's pc_init1()
>        */
>       dc->user_creatable = false;
> +    device_class_set_props(dc, pci_piix3_props);
>       adevc->build_dev_aml = build_pci_isa_aml;
>   }
>   
> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index b1fa08dd2b..5367917182 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -15,6 +15,7 @@
>   #include "hw/pci/pci.h"
>   #include "qom/object.h"
>   #include "hw/rtc/mc146818rtc.h"
> +#include "hw/usb/hcd-uhci.h"
>   
>   /* PIRQRC[A:D]: PIRQx Route Control Registers */
>   #define PIIX_PIRQCA 0x60
> @@ -54,12 +55,15 @@ struct PIIXState {
>       int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>   
>       RTCState rtc;
> +    UHCIState uhci;
>   
>       /* Reset Control Register contents */
>       uint8_t rcr;
>   
>       /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
>       MemoryRegion rcr_mem;
> +
> +    bool has_usb;
>   };
>   typedef struct PIIXState PIIX3State;
>   

Re: [PATCH 11/42] hw/isa/piix3: Remove extra '; ' outside of functions

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> Fixes the "extra-semi" clang-tidy check.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/isa/piix3.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

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

Re: [PATCH 14/42] hw/isa/piix3: Modernize reset handling

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> Rather than registering the reset handler via a function which
> appends the handler to a global list, prefer to implement it as
> a virtual method - PIIX4 does the same already.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix3.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> index c8c2dd6048..0350f70706 100644
> --- a/hw/isa/piix3.c
> +++ b/hw/isa/piix3.c
> @@ -31,7 +31,6 @@
>   #include "hw/qdev-properties.h"
>   #include "hw/isa/isa.h"
>   #include "hw/xen/xen.h"
> -#include "sysemu/reset.h"
>   #include "sysemu/runstate.h"
>   #include "migration/vmstate.h"
>   #include "hw/acpi/acpi_aml_interface.h"
> @@ -156,9 +155,9 @@ static void piix3_write_config_xen(PCIDevice *dev,
>       piix3_write_config(dev, address, val, len);
>   }
>   
> -static void piix3_reset(void *opaque)
> +static void piix3_reset(DeviceState *dev)
>   {
> -    PIIX3State *d = opaque;
> +    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
>       uint8_t *pci_conf = d->dev.config;
>   
>       pci_conf[0x04] = 0x07; /* master, memory and I/O */
> @@ -321,8 +320,6 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>       memory_region_add_subregion_overlap(pci_address_space_io(dev),
>                                           PIIX_RCR_IOPORT, &d->rcr_mem, 1);
>   
> -    qemu_register_reset(piix3_reset, d);
> -
>       i8257_dma_init(isa_bus, 0);
>   
>       /* RTC */
> @@ -397,6 +394,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>       AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
>   
> +    dc->reset       = piix3_reset;
>       dc->desc        = "ISA bridge";
>       dc->vmsd        = &vmstate_piix3;
>       dc->hotpluggable   = false;

Yay 4 years later...
https://lore.kernel.org/qemu-devel/20180108024558.17983-28-f4bug@amsat.org/

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

Re: [PATCH 15/42] hw/isa/piix3: Prefer pci_address_space() over get_system_memory()

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> get_system_memory() accesses global state while pci_address_space() uses
> whatever has been passed to the device instance, so avoid the global.
> Moreover, PIIX4 uses pci_address_space() here as well.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix3.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

Re: [PATCH 22/42] hw/mips/malta: Reuse dev variable

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> While at it, move the assignments closer to where they are used.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/mips/malta.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)

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

Re: [PATCH 25/42] hw/isa/piix4: Move pci_ide_create_devs() call to board code

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> For the VIA south bridges there was a comment to have the call in board code.
> Move it there for PIIX4 as well for consistency.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix4.c  |  1 -
>   hw/mips/malta.c | 10 ++++++----
>   2 files changed, 6 insertions(+), 5 deletions(-)

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

Re: [PATCH 27/42] hw/isa/piix4: Allow board to provide PCI interrupt routes

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> PIIX3 initializes the PIRQx route control registers to the default
> values as described in the 82371AB PCI-TO-ISA/IDE XCELERATOR (PIIX4)
> April 1997 manual. PIIX4, however, initializes the routes according to
> the Malta™ User’s Manual, ch 6.6, which are IRQs 10 and 11. In order to
> allow the reset methods to be consolidated, allow board code to specify
> the routes.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix4.c  | 14 ++++++++++----
>   hw/mips/malta.c |  4 ++++
>   2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index ed9eca715f..763c98b565 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -57,6 +57,8 @@ struct PIIX4State {
>       MemoryRegion rcr_mem;
>       uint8_t rcr;
>   
> +    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
> +
>       bool has_acpi;
>       bool has_usb;
>       bool smm_enabled;
> @@ -122,10 +124,10 @@ static void piix4_isa_reset(DeviceState *dev)
>       pci_conf[0x4c] = 0x4d;
>       pci_conf[0x4e] = 0x03;
>       pci_conf[0x4f] = 0x00;
> -    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
> -    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
> -    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
> -    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
> +    pci_conf[PIIX_PIRQCA] = d->pci_irq_reset_mappings[0];
> +    pci_conf[PIIX_PIRQCB] = d->pci_irq_reset_mappings[1];
> +    pci_conf[PIIX_PIRQCC] = d->pci_irq_reset_mappings[2];
> +    pci_conf[PIIX_PIRQCD] = d->pci_irq_reset_mappings[3];
>       pci_conf[0x69] = 0x02;
>       pci_conf[0x70] = 0x80;
>       pci_conf[0x76] = 0x0c;
> @@ -299,6 +301,10 @@ static void piix4_init(Object *obj)
>   
>   static Property piix4_props[] = {
>       DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
> +    DEFINE_PROP_UINT8("pirqa", PIIX4State, pci_irq_reset_mappings[0], 0x80),
> +    DEFINE_PROP_UINT8("pirqb", PIIX4State, pci_irq_reset_mappings[1], 0x80),
> +    DEFINE_PROP_UINT8("pirqc", PIIX4State, pci_irq_reset_mappings[2], 0x80),
> +    DEFINE_PROP_UINT8("pirqd", PIIX4State, pci_irq_reset_mappings[3], 0x80),
>       DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
>       DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
>       DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index 6339b0d66c..44b6b14f3d 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -1403,6 +1403,10 @@ void mips_malta_init(MachineState *machine)
>       piix4 = pci_new_multifunction(PCI_DEVFN(10, 0), true,
>                                     TYPE_PIIX4_PCI_DEVICE);
>       qdev_prop_set_uint32(DEVICE(piix4), "smb_io_base", 0x1100);
> +    qdev_prop_set_uint8(DEVICE(piix4), "pirqa", 10);
> +    qdev_prop_set_uint8(DEVICE(piix4), "pirqb", 10);
> +    qdev_prop_set_uint8(DEVICE(piix4), "pirqc", 11);
> +    qdev_prop_set_uint8(DEVICE(piix4), "pirqd", 11);
>       pci_realize_and_unref(piix4, pci_bus, &error_fatal);
>       isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
>   

Nice!

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

Re: [PATCH 32/42] hw/isa/piix4: Rename wrongly named method

[Philippe Mathieu-Daudé via]

On 1/9/22 18:26, Bernhard Beschow wrote:
> This method post-loads the southbridge, not the IDE device.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix4.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

Re: [PATCH 42/42] hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI controller

[Philippe Mathieu-Daudé via]

On 1/9/22 18:26, Bernhard Beschow wrote:
> Resolving the PIIX ISA bridge rather than the PIIX ACPI controller mirrors
> the ICH9 code one line below.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i386/acpi-build.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 8af75b1e22..d7bb1ccb26 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -288,7 +288,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
>   
>   static void acpi_get_misc_info(AcpiMiscInfo *info)
>   {
> -    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
> +    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX_PCI_DEVICE);
>       Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
>       assert(!!piix != !!lpc);
>   

This looks correct to  me w.r.t the hardware, but my understanding is
some x86 machines allow abusing the PIIX ACPI PCI function, by plugging
it alone, without the rest of the south bridge... Then this patch would
regress such Frankenstein use :/

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

Re: [PATCH 13/42] hw/isa/piix3: Add size constraints to rcr_ops

[Philippe Mathieu-Daudé via]

On 1/9/22 18:25, Bernhard Beschow wrote:
> According to the PIIX3 datasheet, the reset control register is one byte in size.
> Moreover, PIIX4 has it, so add it to PIIX3 as well.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix3.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

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

Re: [PATCH 00/42] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 1. September 2022 16:25:31 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>This series consolidates the implementations of the PIIX3 and PIIX4 south
>
>bridges and is an extended version of [1]. The motivation is to share as much
>
>code as possible and to bring both device models to feature parity such that
>
>perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This
>
>could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this
>
>list before.
>
>
>
>The series is structured as follows: First, PIIX3 is changed to instantiate
>
>internal devices itself, like PIIX4 does already. Second, PIIX3 gets prepared
>
>for the merge with PIIX4 which includes some fixes, cleanups, and renamings.
>
>Third, the same is done for PIIX4. In step four the implementations are merged.
>
>Since some consolidations could be done easier with merged implementations, the
>
>consolidation continues in step five which concludes the series.
>
>
>
>One particular challenge in this series was that the PIC of PIIX3 used to be
>
>instantiated outside of the south bridge while some sub functions require a PIC
>
>with populated qemu_irqs. This has been solved by introducing a proxy PIC which
>
>furthermore allows PIIX3 to be agnostic towards the virtualization technology
>
>used (KVM, TCG, Xen). Due to consolidation PIIX4 gained the PIC as well,
>
>possibly allowing the Malta board to gain KVM capabilities in the future.
>

Ping

Never mind the comment about Malta. I think it supports KVM just fine.

>
>
>Another challenge was dealing with optional devices where Peter already gave
>
>advice in [1] which this series implements.
>
>
>
>An unsolved problem still is PCI interrupt handling. The first function
>
>passed to pci_bus_irqs() is device-specific while the second one seems
>
>board-specific. This causes both PIIX device models to be coupled to a
>
>particular board. Any advice how to resolve this would be highly appreaciated.
>
>
>
>Last but not least there might be some opportunity to consolidate VM state
>
>handling, probably by reusing the one from PIIX3. Since I'm not very familiar
>
>with the requirements I didn't touch it so far.
>
>
>
>Testing done:
>
>* make check
>
>* Boot live CD:
>
>  * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom
>
>manjaro-kde-21.3.2-220704-linux515.iso`
>
>  * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom
>
>manjaro-kde-21.3.2-220704-linux515.iso`
>
>
>
>[1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>
>
>
>Bernhard Beschow (42):
>
>  hw/i386/pc: Create DMA controllers in south bridges
>
>  hw/i386/pc: Create RTC controllers in south bridges
>
>  hw/i386/pc: No need for rtc_state to be an out-parameter
>
>  hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
>
>    south bridge
>
>  hw/isa/piix3: Create USB controller in host device
>
>  hw/isa/piix3: Create power management controller in host device
>
>  hw/intc/i8259: Introduce i8259 proxy "isa-pic"
>
>  hw/isa/piix3: Create ISA PIC in host device
>
>  hw/isa/piix3: Create IDE controller in host device
>
>  hw/isa/piix3: Wire up ACPI interrupt internally
>
>  hw/isa/piix3: Remove extra ';' outside of functions
>
>  hw/isa/piix3: Remove unused include
>
>  hw/isa/piix3: Add size constraints to rcr_ops
>
>  hw/isa/piix3: Modernize reset handling
>
>  hw/isa/piix3: Prefer pci_address_space() over get_system_memory()
>
>  hw/isa/piix3: Allow board to provide PCI interrupt routes
>
>  hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
>
>  hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4
>
>  hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4
>
>  hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"
>
>  hw/isa/piix3: Rename typedef PIIX3State to PIIXState
>
>  hw/mips/malta: Reuse dev variable
>
>  meson: Fix dependencies of piix4 southbridge
>
>  hw/isa/piix4: Add missing initialization
>
>  hw/isa/piix4: Move pci_ide_create_devs() call to board code
>
>  hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>
>  hw/isa/piix4: Allow board to provide PCI interrupt routes
>
>  hw/isa/piix4: Remove unused code
>
>  hw/isa/piix4: Use ISA PIC device
>
>  hw/isa/piix4: Reuse struct PIIXState from PIIX3
>
>  hw/isa/piix4: Rename reset control operations to match PIIX3
>
>  hw/isa/piix4: Rename wrongly named method
>
>  hw/isa/piix4: Prefix pci_slot_get_pirq() with "piix4_"
>
>  hw/isa/piix3: Merge hw/isa/piix4.c
>
>  hw/isa/piix: Harmonize names of reset control memory regions
>
>  hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>
>  hw/isa/piix: Rename functions to be shared for interrupt triggering
>
>  hw/isa/piix: Consolidate IRQ triggering
>
>  hw/isa/piix: Unexport PIIXState
>
>  hw/isa/piix: Share PIIX3 base class with PIIX4
>
>  hw/isa/piix: Drop the "3" from the PIIX base class
>
>  hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI
>
>    controller
>
>
>
> MAINTAINERS                             |   6 +-
>
> configs/devices/mips-softmmu/common.mak |   3 +-
>
> hw/i386/Kconfig                         |   3 +-
>
> hw/i386/acpi-build.c                    |   4 +-
>
> hw/i386/pc.c                            |  19 +-
>
> hw/i386/pc_piix.c                       |  72 +--
>
> hw/i386/pc_q35.c                        |   3 +-
>
> hw/intc/i8259.c                         |  27 +
>
> hw/isa/Kconfig                          |  14 +-
>
> hw/isa/lpc_ich9.c                       |  11 +
>
> hw/isa/meson.build                      |   3 +-
>
> hw/isa/piix.c                           | 669 ++++++++++++++++++++++++
>
> hw/isa/piix3.c                          | 431 ---------------
>
> hw/isa/piix4.c                          | 325 ------------
>
> hw/mips/malta.c                         |  34 +-
>
> include/hw/i386/ich9.h                  |   2 +
>
> include/hw/i386/pc.h                    |   2 +-
>
> include/hw/intc/i8259.h                 |  14 +
>
> include/hw/southbridge/piix.h           |  41 +-
>
> 19 files changed, 823 insertions(+), 860 deletions(-)
>
> create mode 100644 hw/isa/piix.c
>
> delete mode 100644 hw/isa/piix3.c
>
> delete mode 100644 hw/isa/piix4.c
>
>
>
>-- >
>2.37.3
>
>
>

Re: [PATCH 42/42] hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI controller

[Bernhard Beschow]

Am 1. September 2022 21:05:03 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 1/9/22 18:26, Bernhard Beschow wrote:
>> Resolving the PIIX ISA bridge rather than the PIIX ACPI controller mirrors
>> the ICH9 code one line below.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/i386/acpi-build.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 8af75b1e22..d7bb1ccb26 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -288,7 +288,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
>>     static void acpi_get_misc_info(AcpiMiscInfo *info)
>>   {
>> -    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
>> +    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX_PCI_DEVICE);
>>       Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
>>       assert(!!piix != !!lpc);
>>   
>
>This looks correct to  me w.r.t the hardware, but my understanding is
>some x86 machines allow abusing the PIIX ACPI PCI function, by plugging
>it alone, without the rest of the south bridge... Then this patch would
>regress such Frankenstein use :/

TYPE_PIIX4_PM is not user-creatable and is only instantiated in pc_piix.c if the southbridge is as well, so those should be equivalent. Since acpi_get_misc_info() is more about the board and south bridge and not about PM, I think checking for TYPE_PIIX_PCI_DEVICE is therefore more appropriate here.

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

Re: [PATCH 07/42] hw/intc/i8259: Introduce i8259 proxy "isa-pic"

[Mark Cave-Ayland]

On 01/09/2022 17:25, Bernhard Beschow wrote:

> Having an i8259 proxy allows for ISA PICs to be created and wired up in
> southbridges. This is especially interesting for PIIX3 for two reasons:
> First, the southbridge doesn't need to care about the virtualization
> technology used (KVM, TCG, Xen) due to in-IRQs (where devices get
> attached) and out-IRQs (which will trigger the IRQs of the respective
> virtzalization technology) are separated. Second, since the in-IRQs are
> populated with fully initialized qemu_irq's, they can already be wired
> up inside PIIX3.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/intc/i8259.c         | 27 +++++++++++++++++++++++++++
>   include/hw/intc/i8259.h | 14 ++++++++++++++
>   2 files changed, 41 insertions(+)
> 
> diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
> index cc4e21ffec..531f6cca53 100644
> --- a/hw/intc/i8259.c
> +++ b/hw/intc/i8259.c
> @@ -458,9 +458,36 @@ static const TypeInfo i8259_info = {
>       .class_size = sizeof(PICClass),
>   };
>   
> +static void isapic_set_irq(void *opaque, int irq, int level)
> +{
> +    ISAPICState *s = opaque;
> +
> +    qemu_set_irq(s->out_irqs[irq], level);
> +}
> +
> +static void isapic_init(Object *obj)
> +{
> +    ISAPICState *s = ISA_PIC(obj);
> +
> +    qdev_init_gpio_in(DEVICE(s), isapic_set_irq, ISA_NUM_IRQS);
> +    qdev_init_gpio_out(DEVICE(s), s->out_irqs, ISA_NUM_IRQS);
> +
> +    for (int i = 0; i < ISA_NUM_IRQS; ++i) {
> +        s->in_irqs[i] = qdev_get_gpio_in(DEVICE(s), i);
> +    }
> +}
> +
> +static const TypeInfo isapic_info = {
> +    .name          = TYPE_ISA_PIC,
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(ISAPICState),
> +    .instance_init = isapic_init,
> +};
> +
>   static void pic_register_types(void)
>   {
>       type_register_static(&i8259_info);
> +    type_register_static(&isapic_info);
>   }
>   
>   type_init(pic_register_types)
> diff --git a/include/hw/intc/i8259.h b/include/hw/intc/i8259.h
> index e2b1e8c59a..0246ab6ac6 100644
> --- a/include/hw/intc/i8259.h
> +++ b/include/hw/intc/i8259.h
> @@ -1,6 +1,20 @@
>   #ifndef HW_I8259_H
>   #define HW_I8259_H
>   
> +#include "qom/object.h"
> +#include "hw/isa/isa.h"
> +#include "qemu/typedefs.h"
> +
> +#define TYPE_ISA_PIC "isa-pic"
> +OBJECT_DECLARE_SIMPLE_TYPE(ISAPICState, ISA_PIC)
> +
> +struct ISAPICState {
> +    ISADevice parent_obj;
> +
> +    qemu_irq in_irqs[ISA_NUM_IRQS];
> +    qemu_irq out_irqs[ISA_NUM_IRQS];
> +};
> +
>   /* i8259.c */
>   
>   extern DeviceState *isa_pic;

So effectively this proxy device is working around the issue that we don't yet have a 
qdev-ified PIC that can be used to wire up ISA bus IRQs via qdev_connect_gpio_out(). 
Whilst ultimately this is the goal, I think having the proxy is a nice intermediate 
step as long as we can ensure that the device is not user-visible (i.e. it won't be 
exposed via the command line) so that this can be improved later.


ATB,

Mark.

Re: [PATCH 14/42] hw/isa/piix3: Modernize reset handling

[Mark Cave-Ayland]

On 01/09/2022 17:25, Bernhard Beschow wrote:

> Rather than registering the reset handler via a function which
> appends the handler to a global list, prefer to implement it as
> a virtual method - PIIX4 does the same already.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix3.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> index c8c2dd6048..0350f70706 100644
> --- a/hw/isa/piix3.c
> +++ b/hw/isa/piix3.c
> @@ -31,7 +31,6 @@
>   #include "hw/qdev-properties.h"
>   #include "hw/isa/isa.h"
>   #include "hw/xen/xen.h"
> -#include "sysemu/reset.h"
>   #include "sysemu/runstate.h"
>   #include "migration/vmstate.h"
>   #include "hw/acpi/acpi_aml_interface.h"
> @@ -156,9 +155,9 @@ static void piix3_write_config_xen(PCIDevice *dev,
>       piix3_write_config(dev, address, val, len);
>   }
>   
> -static void piix3_reset(void *opaque)
> +static void piix3_reset(DeviceState *dev)
>   {
> -    PIIX3State *d = opaque;
> +    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
>       uint8_t *pci_conf = d->dev.config;
>   
>       pci_conf[0x04] = 0x07; /* master, memory and I/O */
> @@ -321,8 +320,6 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>       memory_region_add_subregion_overlap(pci_address_space_io(dev),
>                                           PIIX_RCR_IOPORT, &d->rcr_mem, 1);
>   
> -    qemu_register_reset(piix3_reset, d);
> -
>       i8257_dma_init(isa_bus, 0);
>   
>       /* RTC */
> @@ -397,6 +394,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>       AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
>   
> +    dc->reset       = piix3_reset;
>       dc->desc        = "ISA bridge";
>       dc->vmsd        = &vmstate_piix3;
>       dc->hotpluggable   = false;

One minor point to be aware of here is that qdev reset is a PCI bus level reset 
compared with the existing qemu_register_reset() which is a machine level reset. What 
this means is that dc->reset can also be called writing to the relevant configuration 
space register on a PCI bridge - it may not be an issue here, but worth a mention.


ATB,

Mark.

Re: [PATCH 26/42] hw/isa/piix4: Make PIIX4's ACPI and USB functions optional

[Mark Cave-Ayland]

On 01/09/2022 17:25, Bernhard Beschow wrote:

> This aligns PIIX4 with PIIX3.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix4.c  | 44 ++++++++++++++++++++++++++++++++------------
>   hw/mips/malta.c |  6 ++++--
>   2 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 67881e3a75..ed9eca715f 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -50,9 +50,16 @@ struct PIIX4State {
>       PCIIDEState ide;
>       UHCIState uhci;
>       PIIX4PMState pm;
> +
> +    uint32_t smb_io_base;
> +
>       /* Reset Control Register */
>       MemoryRegion rcr_mem;
>       uint8_t rcr;
> +
> +    bool has_acpi;
> +    bool has_usb;
> +    bool smm_enabled;
>   };
>   
>   OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
> @@ -258,17 +265,26 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
>       }
>   
>       /* USB */
> -    qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
> -    if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
> -        return;
> +    if (s->has_usb) {
> +        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
> +                                "piix4-usb-uhci");

Can you use the relevant TYPE_ macro here for the USB-UHCI device?

> +        qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
> +        if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
> +            return;
> +        }
>       }
>   
>       /* ACPI controller */
> -    qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
> -    if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
> -        return;
> +    if (s->has_acpi) {
> +        object_initialize_child(OBJECT(s), "pm", &s->pm, TYPE_PIIX4_PM);
> +        qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
> +        qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", s->smb_io_base);
> +        qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", s->smm_enabled);
> +        if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
> +            return;
> +        }
> +        qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
>       }
> -    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
>   
>       pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
>   }
> @@ -279,13 +295,16 @@ static void piix4_init(Object *obj)
>   
>       object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
>       object_initialize_child(obj, "ide", &s->ide, "piix4-ide");

... and same here for IDE?

> -    object_initialize_child(obj, "uhci", &s->uhci, "piix4-usb-uhci");
> -
> -    object_initialize_child(obj, "pm", &s->pm, TYPE_PIIX4_PM);
> -    qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", 0x1100);
> -    qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", 0);
>   }
>   
> +static Property piix4_props[] = {
> +    DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
> +    DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
> +    DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
> +    DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>   static void piix4_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -304,6 +323,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
>        */
>       dc->user_creatable = false;
>       dc->hotpluggable = false;
> +    device_class_set_props(dc, piix4_props);
>   }
>   
>   static const TypeInfo piix4_info = {
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index a4b866a2cf..6339b0d66c 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -1400,8 +1400,10 @@ void mips_malta_init(MachineState *machine)
>       empty_slot_init("GT64120", 0, 0x20000000);
>   
>       /* Southbridge */
> -    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
> -                                            TYPE_PIIX4_PCI_DEVICE);
> +    piix4 = pci_new_multifunction(PCI_DEVFN(10, 0), true,
> +                                  TYPE_PIIX4_PCI_DEVICE);
> +    qdev_prop_set_uint32(DEVICE(piix4), "smb_io_base", 0x1100);
> +    pci_realize_and_unref(piix4, pci_bus, &error_fatal);
>       isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
>   
>       dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "ide"));


ATB,

Mark.

Re: [PATCH 39/42] hw/isa/piix: Unexport PIIXState

[Mark Cave-Ayland]

On 01/09/2022 17:26, Bernhard Beschow wrote:

> The - deliberately exported - components can still be accessed
> via QOM properties.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix.c                 | 52 +++++++++++++++++++++++++++++++++
>   include/hw/southbridge/piix.h | 54 -----------------------------------
>   2 files changed, 52 insertions(+), 54 deletions(-)
> 
> diff --git a/hw/isa/piix.c b/hw/isa/piix.c
> index e413d7e792..c503a6e836 100644
> --- a/hw/isa/piix.c
> +++ b/hw/isa/piix.c
> @@ -26,20 +26,72 @@
>   #include "qemu/osdep.h"
>   #include "qemu/range.h"
>   #include "qapi/error.h"
> +#include "qom/object.h"
> +#include "hw/acpi/piix4.h"
>   #include "hw/dma/i8257.h"
> +#include "hw/ide/pci.h"
>   #include "hw/intc/i8259.h"
>   #include "hw/southbridge/piix.h"
>   #include "hw/timer/i8254.h"
>   #include "hw/irq.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/isa/isa.h"
> +#include "hw/pci/pci.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/rtc/mc146818rtc.h"
> +#include "hw/usb/hcd-uhci.h"
>   #include "hw/xen/xen.h"
>   #include "sysemu/runstate.h"
>   #include "migration/vmstate.h"
>   #include "hw/acpi/acpi_aml_interface.h"
>   
> +#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
>   #define XEN_PIIX_NUM_PIRQS      128ULL
>   
> +struct PIIXState {
> +    PCIDevice dev;
> +
> +    /*
> +     * bitmap to track pic levels.
> +     * The pic level is the logical OR of all the PCI irqs mapped to it
> +     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
> +     *
> +     * PIRQ is mapped to PIC pins, we track it by
> +     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
> +     * pic_irq * PIIX_NUM_PIRQS + pirq
> +     */
> +#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
> +#error "unable to encode pic state in 64bit in pic_levels."
> +#endif
> +    uint64_t pic_levels;
> +
> +    /* This member isn't used. Just for save/load compatibility */
> +    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
> +    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
> +
> +    ISAPICState pic;
> +    RTCState rtc;
> +    PCIIDEState ide;
> +    UHCIState uhci;
> +    PIIX4PMState pm;
> +
> +    uint32_t smb_io_base;
> +
> +    /* Reset Control Register contents */
> +    uint8_t rcr;
> +
> +    /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
> +    MemoryRegion rcr_mem;
> +
> +    bool has_acpi;
> +    bool has_usb;
> +    bool smm_enabled;
> +};
> +typedef struct PIIXState PIIXState;
> +
> +DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
> +                         TYPE_PIIX3_PCI_DEVICE)
> +
>   static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
>   {
>       qemu_set_irq(piix->pic.in_irqs[pic_irq],
> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index c9fa0f1aa6..0edc23710c 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -12,14 +12,6 @@
>   #ifndef HW_SOUTHBRIDGE_PIIX_H
>   #define HW_SOUTHBRIDGE_PIIX_H
>   
> -#include "hw/pci/pci.h"
> -#include "qom/object.h"
> -#include "hw/acpi/piix4.h"
> -#include "hw/ide/pci.h"
> -#include "hw/intc/i8259.h"
> -#include "hw/rtc/mc146818rtc.h"
> -#include "hw/usb/hcd-uhci.h"
> -
>   /* PIRQRC[A:D]: PIRQx Route Control Registers */
>   #define PIIX_PIRQCA 0x60
>   #define PIIX_PIRQCB 0x61
> @@ -32,53 +24,7 @@
>    */
>   #define PIIX_RCR_IOPORT 0xcf9
>   
> -#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
> -
> -struct PIIXState {
> -    PCIDevice dev;
> -
> -    /*
> -     * bitmap to track pic levels.
> -     * The pic level is the logical OR of all the PCI irqs mapped to it
> -     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
> -     *
> -     * PIRQ is mapped to PIC pins, we track it by
> -     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
> -     * pic_irq * PIIX_NUM_PIRQS + pirq
> -     */
> -#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
> -#error "unable to encode pic state in 64bit in pic_levels."
> -#endif
> -    uint64_t pic_levels;
> -
> -    /* This member isn't used. Just for save/load compatibility */
> -    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
> -    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
> -
> -    ISAPICState pic;
> -    RTCState rtc;
> -    PCIIDEState ide;
> -    UHCIState uhci;
> -    PIIX4PMState pm;
> -
> -    uint32_t smb_io_base;
> -
> -    /* Reset Control Register contents */
> -    uint8_t rcr;
> -
> -    /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
> -    MemoryRegion rcr_mem;
> -
> -    bool has_acpi;
> -    bool has_usb;
> -    bool smm_enabled;
> -};
> -typedef struct PIIXState PIIXState;
> -
>   #define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
> -DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
> -                         TYPE_PIIX3_PCI_DEVICE)
> -
>   #define TYPE_PIIX3_DEVICE "PIIX3"
>   #define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
>   #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"

I don't think that this is the right way to go here - whilst the definition of public 
and private can be a little vague, the general aim should be for the QOM type struct 
and macros to be in the corresponding .h file and the implementation in the .c file. 
In effect this ensures that anyone who wants to use a TYPE_FOO will include foo.h 
which helps make it easier to keep track of dependencies.

Looking at TYPE_PIIX3_PCI_DEVICE I'm wondering why this couldn't be 
OBJECT_SIMPLE_TYPE instead of DECLARE_INSTANCE_CHECKER with this series?


ATB,

Mark.

Re: [PATCH 00/42] Consolidate PIIX south bridges

[Mark Cave-Ayland]

On 01/09/2022 17:25, Bernhard Beschow wrote:

> This series consolidates the implementations of the PIIX3 and PIIX4 south
> bridges and is an extended version of [1]. The motivation is to share as much
> code as possible and to bring both device models to feature parity such that
> perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This
> could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this
> list before.
> 
> The series is structured as follows: First, PIIX3 is changed to instantiate
> internal devices itself, like PIIX4 does already. Second, PIIX3 gets prepared
> for the merge with PIIX4 which includes some fixes, cleanups, and renamings.
> Third, the same is done for PIIX4. In step four the implementations are merged.
> Since some consolidations could be done easier with merged implementations, the
> consolidation continues in step five which concludes the series.
> 
> One particular challenge in this series was that the PIC of PIIX3 used to be
> instantiated outside of the south bridge while some sub functions require a PIC
> with populated qemu_irqs. This has been solved by introducing a proxy PIC which
> furthermore allows PIIX3 to be agnostic towards the virtualization technology
> used (KVM, TCG, Xen). Due to consolidation PIIX4 gained the PIC as well,
> possibly allowing the Malta board to gain KVM capabilities in the future.
> 
> Another challenge was dealing with optional devices where Peter already gave
> advice in [1] which this series implements.
> 
> An unsolved problem still is PCI interrupt handling. The first function
> passed to pci_bus_irqs() is device-specific while the second one seems
> board-specific. This causes both PIIX device models to be coupled to a
> particular board. Any advice how to resolve this would be highly appreaciated.

Could you explain this in a bit more detail?

> Last but not least there might be some opportunity to consolidate VM state
> handling, probably by reusing the one from PIIX3. Since I'm not very familiar
> with the requirements I didn't touch it so far.
> 
> Testing done:
> * make check
> * Boot live CD:
>    * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom
> manjaro-kde-21.3.2-220704-linux515.iso`
>    * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom
> manjaro-kde-21.3.2-220704-linux515.iso`
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
> 
> Bernhard Beschow (42):
>    hw/i386/pc: Create DMA controllers in south bridges
>    hw/i386/pc: Create RTC controllers in south bridges
>    hw/i386/pc: No need for rtc_state to be an out-parameter
>    hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
>      south bridge
>    hw/isa/piix3: Create USB controller in host device
>    hw/isa/piix3: Create power management controller in host device
>    hw/intc/i8259: Introduce i8259 proxy "isa-pic"
>    hw/isa/piix3: Create ISA PIC in host device
>    hw/isa/piix3: Create IDE controller in host device
>    hw/isa/piix3: Wire up ACPI interrupt internally
>    hw/isa/piix3: Remove extra ';' outside of functions
>    hw/isa/piix3: Remove unused include
>    hw/isa/piix3: Add size constraints to rcr_ops
>    hw/isa/piix3: Modernize reset handling
>    hw/isa/piix3: Prefer pci_address_space() over get_system_memory()
>    hw/isa/piix3: Allow board to provide PCI interrupt routes
>    hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
>    hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4
>    hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4
>    hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"
>    hw/isa/piix3: Rename typedef PIIX3State to PIIXState
>    hw/mips/malta: Reuse dev variable
>    meson: Fix dependencies of piix4 southbridge
>    hw/isa/piix4: Add missing initialization
>    hw/isa/piix4: Move pci_ide_create_devs() call to board code
>    hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>    hw/isa/piix4: Allow board to provide PCI interrupt routes
>    hw/isa/piix4: Remove unused code
>    hw/isa/piix4: Use ISA PIC device
>    hw/isa/piix4: Reuse struct PIIXState from PIIX3
>    hw/isa/piix4: Rename reset control operations to match PIIX3
>    hw/isa/piix4: Rename wrongly named method
>    hw/isa/piix4: Prefix pci_slot_get_pirq() with "piix4_"
>    hw/isa/piix3: Merge hw/isa/piix4.c
>    hw/isa/piix: Harmonize names of reset control memory regions
>    hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>    hw/isa/piix: Rename functions to be shared for interrupt triggering
>    hw/isa/piix: Consolidate IRQ triggering
>    hw/isa/piix: Unexport PIIXState
>    hw/isa/piix: Share PIIX3 base class with PIIX4
>    hw/isa/piix: Drop the "3" from the PIIX base class
>    hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI
>      controller
> 
>   MAINTAINERS                             |   6 +-
>   configs/devices/mips-softmmu/common.mak |   3 +-
>   hw/i386/Kconfig                         |   3 +-
>   hw/i386/acpi-build.c                    |   4 +-
>   hw/i386/pc.c                            |  19 +-
>   hw/i386/pc_piix.c                       |  72 +--
>   hw/i386/pc_q35.c                        |   3 +-
>   hw/intc/i8259.c                         |  27 +
>   hw/isa/Kconfig                          |  14 +-
>   hw/isa/lpc_ich9.c                       |  11 +
>   hw/isa/meson.build                      |   3 +-
>   hw/isa/piix.c                           | 669 ++++++++++++++++++++++++
>   hw/isa/piix3.c                          | 431 ---------------
>   hw/isa/piix4.c                          | 325 ------------
>   hw/mips/malta.c                         |  34 +-
>   include/hw/i386/ich9.h                  |   2 +
>   include/hw/i386/pc.h                    |   2 +-
>   include/hw/intc/i8259.h                 |  14 +
>   include/hw/southbridge/piix.h           |  41 +-
>   19 files changed, 823 insertions(+), 860 deletions(-)
>   create mode 100644 hw/isa/piix.c
>   delete mode 100644 hw/isa/piix3.c
>   delete mode 100644 hw/isa/piix4.c

I've had a quick skim over this series and commented on the parts that caught my eye, 
however I'm generally happy with the way this series is going and it seems like a 
nice tidy-up - thanks!


ATB,

Mark.

Re: [PATCH 39/42] hw/isa/piix: Unexport PIIXState

[Bernhard Beschow]

Am 18. September 2022 20:21:09 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>On 01/09/2022 17:26, Bernhard Beschow wrote:
>
>> The - deliberately exported - components can still be accessed
>> via QOM properties.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/isa/piix.c                 | 52 +++++++++++++++++++++++++++++++++
>>   include/hw/southbridge/piix.h | 54 -----------------------------------
>>   2 files changed, 52 insertions(+), 54 deletions(-)
>> 
>> diff --git a/hw/isa/piix.c b/hw/isa/piix.c
>> index e413d7e792..c503a6e836 100644
>> --- a/hw/isa/piix.c
>> +++ b/hw/isa/piix.c
>> @@ -26,20 +26,72 @@
>>   #include "qemu/osdep.h"
>>   #include "qemu/range.h"
>>   #include "qapi/error.h"
>> +#include "qom/object.h"
>> +#include "hw/acpi/piix4.h"
>>   #include "hw/dma/i8257.h"
>> +#include "hw/ide/pci.h"
>>   #include "hw/intc/i8259.h"
>>   #include "hw/southbridge/piix.h"
>>   #include "hw/timer/i8254.h"
>>   #include "hw/irq.h"
>>   #include "hw/qdev-properties.h"
>>   #include "hw/isa/isa.h"
>> +#include "hw/pci/pci.h"
>> +#include "hw/qdev-properties.h"
>> +#include "hw/rtc/mc146818rtc.h"
>> +#include "hw/usb/hcd-uhci.h"
>>   #include "hw/xen/xen.h"
>>   #include "sysemu/runstate.h"
>>   #include "migration/vmstate.h"
>>   #include "hw/acpi/acpi_aml_interface.h"
>>   +#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
>>   #define XEN_PIIX_NUM_PIRQS      128ULL
>>   +struct PIIXState {
>> +    PCIDevice dev;
>> +
>> +    /*
>> +     * bitmap to track pic levels.
>> +     * The pic level is the logical OR of all the PCI irqs mapped to it
>> +     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
>> +     *
>> +     * PIRQ is mapped to PIC pins, we track it by
>> +     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
>> +     * pic_irq * PIIX_NUM_PIRQS + pirq
>> +     */
>> +#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
>> +#error "unable to encode pic state in 64bit in pic_levels."
>> +#endif
>> +    uint64_t pic_levels;
>> +
>> +    /* This member isn't used. Just for save/load compatibility */
>> +    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>> +    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
>> +
>> +    ISAPICState pic;
>> +    RTCState rtc;
>> +    PCIIDEState ide;
>> +    UHCIState uhci;
>> +    PIIX4PMState pm;
>> +
>> +    uint32_t smb_io_base;
>> +
>> +    /* Reset Control Register contents */
>> +    uint8_t rcr;
>> +
>> +    /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
>> +    MemoryRegion rcr_mem;
>> +
>> +    bool has_acpi;
>> +    bool has_usb;
>> +    bool smm_enabled;
>> +};
>> +typedef struct PIIXState PIIXState;
>> +
>> +DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
>> +                         TYPE_PIIX3_PCI_DEVICE)
>> +
>>   static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
>>   {
>>       qemu_set_irq(piix->pic.in_irqs[pic_irq],
>> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
>> index c9fa0f1aa6..0edc23710c 100644
>> --- a/include/hw/southbridge/piix.h
>> +++ b/include/hw/southbridge/piix.h
>> @@ -12,14 +12,6 @@
>>   #ifndef HW_SOUTHBRIDGE_PIIX_H
>>   #define HW_SOUTHBRIDGE_PIIX_H
>>   -#include "hw/pci/pci.h"
>> -#include "qom/object.h"
>> -#include "hw/acpi/piix4.h"
>> -#include "hw/ide/pci.h"
>> -#include "hw/intc/i8259.h"
>> -#include "hw/rtc/mc146818rtc.h"
>> -#include "hw/usb/hcd-uhci.h"
>> -
>>   /* PIRQRC[A:D]: PIRQx Route Control Registers */
>>   #define PIIX_PIRQCA 0x60
>>   #define PIIX_PIRQCB 0x61
>> @@ -32,53 +24,7 @@
>>    */
>>   #define PIIX_RCR_IOPORT 0xcf9
>>   -#define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
>> -
>> -struct PIIXState {
>> -    PCIDevice dev;
>> -
>> -    /*
>> -     * bitmap to track pic levels.
>> -     * The pic level is the logical OR of all the PCI irqs mapped to it
>> -     * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
>> -     *
>> -     * PIRQ is mapped to PIC pins, we track it by
>> -     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
>> -     * pic_irq * PIIX_NUM_PIRQS + pirq
>> -     */
>> -#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
>> -#error "unable to encode pic state in 64bit in pic_levels."
>> -#endif
>> -    uint64_t pic_levels;
>> -
>> -    /* This member isn't used. Just for save/load compatibility */
>> -    int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>> -    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];
>> -
>> -    ISAPICState pic;
>> -    RTCState rtc;
>> -    PCIIDEState ide;
>> -    UHCIState uhci;
>> -    PIIX4PMState pm;
>> -
>> -    uint32_t smb_io_base;
>> -
>> -    /* Reset Control Register contents */
>> -    uint8_t rcr;
>> -
>> -    /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
>> -    MemoryRegion rcr_mem;
>> -
>> -    bool has_acpi;
>> -    bool has_usb;
>> -    bool smm_enabled;
>> -};
>> -typedef struct PIIXState PIIXState;
>> -
>>   #define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
>> -DECLARE_INSTANCE_CHECKER(PIIXState, PIIX_PCI_DEVICE,
>> -                         TYPE_PIIX3_PCI_DEVICE)
>> -
>>   #define TYPE_PIIX3_DEVICE "PIIX3"
>>   #define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
>>   #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
>
>I don't think that this is the right way to go here - whilst the definition of public and private can be a little vague, the general aim should be for the QOM type struct and macros to be in the corresponding .h file and the implementation in the .c file. In effect this ensures that anyone who wants to use a TYPE_FOO will include foo.h which helps make it easier to keep track of dependencies.

So essentially I'd omit this patch from the series...

>Looking at TYPE_PIIX3_PCI_DEVICE I'm wondering why this couldn't be OBJECT_SIMPLE_TYPE instead of DECLARE_INSTANCE_CHECKER with this series?

... and instead add one which replaces DECLARE_INSTANCE_CHECKER with OBJECT_SIMPLE_TYPE here?

Best regards,
Bernhard
>
>
>ATB,
>
>Mark.

Re: [PATCH 05/42] hw/isa/piix3: Create USB controller in host device

[Bernhard Beschow]

Am 1. September 2022 19:13:22 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 1/9/22 18:25, Bernhard Beschow wrote:
>> The USB controller is an integral part of PIIX3 (function 2). So create
>> it as part of the south bridge.
>> 
>> Note that the USB function is optional in QEMU. This is why it gets
>> object_initialize_child()'ed in realize rather than in instance_init.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/i386/pc_piix.c             |  6 ++----
>>   hw/isa/Kconfig                |  1 +
>>   hw/isa/piix3.c                | 17 +++++++++++++++++
>>   include/hw/southbridge/piix.h |  4 ++++
>>   4 files changed, 24 insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index b08d946992..76ac8b2035 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -219,6 +219,8 @@ static void pc_init1(MachineState *machine,
>>           pcms->bus = pci_bus;
>>             pci_dev = pci_new_multifunction(-1, true, type);
>> +        object_property_set_bool(OBJECT(pci_dev), "has-usb",
>> +                                 machine_usb(machine), &error_abort);
>>           pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
>>           piix3 = PIIX3_PCI_DEVICE(pci_dev);
>>           piix3->pic = x86ms->gsi;
>> @@ -297,10 +299,6 @@ static void pc_init1(MachineState *machine,
>>       }
>>   #endif
>>   -    if (pcmc->pci_enabled && machine_usb(machine)) {
>> -        pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
>> -    }
>> -
>>       if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
>>           PCIDevice *piix4_pm;
>>   diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
>> index 6e8f9cac54..f02eca3c3e 100644
>> --- a/hw/isa/Kconfig
>> +++ b/hw/isa/Kconfig
>> @@ -36,6 +36,7 @@ config PIIX3
>>       select I8257
>>       select ISA_BUS
>>       select MC146818RTC
>> +    select USB_UHCI
>>     config PIIX4
>>       bool
>> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
>> index 96ab7107e2..27052a5546 100644
>> --- a/hw/isa/piix3.c
>> +++ b/hw/isa/piix3.c
>> @@ -297,6 +297,7 @@ static const MemoryRegionOps rcr_ops = {
>>   static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>>   {
>>       PIIX3State *d = PIIX3_PCI_DEVICE(dev);
>> +    PCIBus *pci_bus = pci_get_bus(dev);
>>       ISABus *isa_bus;
>>         isa_bus = isa_bus_new(DEVICE(d), get_system_memory(),
>> @@ -319,6 +320,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>>       if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
>>           return;
>>       }
>> +
>> +    /* USB */
>> +    if (d->has_usb) {
>> +        object_initialize_child(OBJECT(dev), "uhci", &d->uhci,
>> +                                "piix3-usb-uhci");
>> +        qdev_prop_set_int32(DEVICE(&d->uhci), "addr", dev->devfn + 2);
>> +        if (!qdev_realize(DEVICE(&d->uhci), BUS(pci_bus), errp)) {
>> +            return;
>> +        }
>> +    }
>>   }
>>     static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
>> @@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj)
>>       object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
>>   }
>>   +static Property pci_piix3_props[] = {
>> +    DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
>
>Maybe s/has-usb/usb-enabled/?

I consisered that. I chose has-foo for now since I took inspiration from the board. Also, "enabled" may refer to a device being present but its enable bit is cleared in PCI configuration space, c.f. vt82c686. Let me know if you still suggest a change here, in which case I'll consult you for naming suggestions once I consolidate piix and vt82c686 ;)

Best regards,
Bernhard
>
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>>   static void pci_piix3_class_init(ObjectClass *klass, void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -359,6 +375,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
>>        * pc_piix.c's pc_init1()
>>        */
>>       dc->user_creatable = false;
>> +    device_class_set_props(dc, pci_piix3_props);
>>       adevc->build_dev_aml = build_pci_isa_aml;
>>   }
>>   diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
>> index b1fa08dd2b..5367917182 100644
>> --- a/include/hw/southbridge/piix.h
>> +++ b/include/hw/southbridge/piix.h
>> @@ -15,6 +15,7 @@
>>   #include "hw/pci/pci.h"
>>   #include "qom/object.h"
>>   #include "hw/rtc/mc146818rtc.h"
>> +#include "hw/usb/hcd-uhci.h"
>>     /* PIRQRC[A:D]: PIRQx Route Control Registers */
>>   #define PIIX_PIRQCA 0x60
>> @@ -54,12 +55,15 @@ struct PIIXState {
>>       int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>>         RTCState rtc;
>> +    UHCIState uhci;
>>         /* Reset Control Register contents */
>>       uint8_t rcr;
>>         /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
>>       MemoryRegion rcr_mem;
>> +
>> +    bool has_usb;
>>   };
>>   typedef struct PIIXState PIIX3State;
>>   
>

Re: [PATCH 26/42] hw/isa/piix4: Make PIIX4's ACPI and USB functions optional

[Bernhard Beschow]

Am 18. September 2022 20:10:51 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>On 01/09/2022 17:25, Bernhard Beschow wrote:
>
>> This aligns PIIX4 with PIIX3.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/isa/piix4.c  | 44 ++++++++++++++++++++++++++++++++------------
>>   hw/mips/malta.c |  6 ++++--
>>   2 files changed, 36 insertions(+), 14 deletions(-)
>> 
>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>> index 67881e3a75..ed9eca715f 100644
>> --- a/hw/isa/piix4.c
>> +++ b/hw/isa/piix4.c
>> @@ -50,9 +50,16 @@ struct PIIX4State {
>>       PCIIDEState ide;
>>       UHCIState uhci;
>>       PIIX4PMState pm;
>> +
>> +    uint32_t smb_io_base;
>> +
>>       /* Reset Control Register */
>>       MemoryRegion rcr_mem;
>>       uint8_t rcr;
>> +
>> +    bool has_acpi;
>> +    bool has_usb;
>> +    bool smm_enabled;
>>   };
>>     OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
>> @@ -258,17 +265,26 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
>>       }
>>         /* USB */
>> -    qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
>> -    if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
>> -        return;
>> +    if (s->has_usb) {
>> +        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
>> +                                "piix4-usb-uhci");
>
>Can you use the relevant TYPE_ macro here for the USB-UHCI device?
>
>> +        qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
>> +        if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
>> +            return;
>> +        }
>>       }
>>         /* ACPI controller */
>> -    qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
>> -    if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
>> -        return;
>> +    if (s->has_acpi) {
>> +        object_initialize_child(OBJECT(s), "pm", &s->pm, TYPE_PIIX4_PM);
>> +        qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
>> +        qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", s->smb_io_base);
>> +        qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", s->smm_enabled);
>> +        if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
>> +            return;
>> +        }
>> +        qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
>>       }
>> -    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
>>         pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
>>   }
>> @@ -279,13 +295,16 @@ static void piix4_init(Object *obj)
>>         object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
>>       object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
>
>... and same here for IDE?

Sure!

Best regards,
Bernhard
>
>> -    object_initialize_child(obj, "uhci", &s->uhci, "piix4-usb-uhci");
>> -
>> -    object_initialize_child(obj, "pm", &s->pm, TYPE_PIIX4_PM);
>> -    qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", 0x1100);
>> -    qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", 0);
>>   }
>>   +static Property piix4_props[] = {
>> +    DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
>> +    DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
>> +    DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
>> +    DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>>   static void piix4_class_init(ObjectClass *klass, void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -304,6 +323,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
>>        */
>>       dc->user_creatable = false;
>>       dc->hotpluggable = false;
>> +    device_class_set_props(dc, piix4_props);
>>   }
>>     static const TypeInfo piix4_info = {
>> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
>> index a4b866a2cf..6339b0d66c 100644
>> --- a/hw/mips/malta.c
>> +++ b/hw/mips/malta.c
>> @@ -1400,8 +1400,10 @@ void mips_malta_init(MachineState *machine)
>>       empty_slot_init("GT64120", 0, 0x20000000);
>>         /* Southbridge */
>> -    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
>> -                                            TYPE_PIIX4_PCI_DEVICE);
>> +    piix4 = pci_new_multifunction(PCI_DEVFN(10, 0), true,
>> +                                  TYPE_PIIX4_PCI_DEVICE);
>> +    qdev_prop_set_uint32(DEVICE(piix4), "smb_io_base", 0x1100);
>> +    pci_realize_and_unref(piix4, pci_bus, &error_fatal);
>>       isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
>>         dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "ide"));
>
>
>ATB,
>
>Mark.

Re: [PATCH 00/42] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 18. September 2022 20:22:55 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>On 01/09/2022 17:25, Bernhard Beschow wrote:
>
>> This series consolidates the implementations of the PIIX3 and PIIX4 south
>> bridges and is an extended version of [1]. The motivation is to share as much
>> code as possible and to bring both device models to feature parity such that
>> perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This
>> could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this
>> list before.
>> 
>> The series is structured as follows: First, PIIX3 is changed to instantiate
>> internal devices itself, like PIIX4 does already. Second, PIIX3 gets prepared
>> for the merge with PIIX4 which includes some fixes, cleanups, and renamings.
>> Third, the same is done for PIIX4. In step four the implementations are merged.
>> Since some consolidations could be done easier with merged implementations, the
>> consolidation continues in step five which concludes the series.
>> 
>> One particular challenge in this series was that the PIC of PIIX3 used to be
>> instantiated outside of the south bridge while some sub functions require a PIC
>> with populated qemu_irqs. This has been solved by introducing a proxy PIC which
>> furthermore allows PIIX3 to be agnostic towards the virtualization technology
>> used (KVM, TCG, Xen). Due to consolidation PIIX4 gained the PIC as well,
>> possibly allowing the Malta board to gain KVM capabilities in the future.
>> 
>> Another challenge was dealing with optional devices where Peter already gave
>> advice in [1] which this series implements.
>> 
>> An unsolved problem still is PCI interrupt handling. The first function
>> passed to pci_bus_irqs() is device-specific while the second one seems
>> board-specific. This causes both PIIX device models to be coupled to a
>> particular board. Any advice how to resolve this would be highly appreaciated.
>
>Could you explain this in a bit more detail?

Sure!

Even after the consolidation there are piix3_pci_slot_get_pirq() and piix4_pci_slot_get_pirq() which seem board-specific rather than south bridge-specific. So they seem to belong into board code (pc_piix and Malta). piix_set_irq(), OTOH, seems appropriate in piix.c and is even shared between 3 and 4. However, pci_bus_irqs() assigns both piix_set_irq() and piix{3,4}_pci_slot_get_pirq() in one go. So it is unclear to me how to pass the board-specific piix{3,4}_pci_slot_get_pirq() into the south bridge for pci_bus_irqs() if this call is performed there.

I'm really curious about an answer. Let me know if I was still unclear.

>
>> Last but not least there might be some opportunity to consolidate VM state
>> handling, probably by reusing the one from PIIX3. Since I'm not very familiar
>> with the requirements I didn't touch it so far.
>> 
>> Testing done:
>> * make check
>> * Boot live CD:
>>    * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom
>> manjaro-kde-21.3.2-220704-linux515.iso`
>>    * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom
>> manjaro-kde-21.3.2-220704-linux515.iso`
>> 
>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>> 
>> Bernhard Beschow (42):
>>    hw/i386/pc: Create DMA controllers in south bridges
>>    hw/i386/pc: Create RTC controllers in south bridges
>>    hw/i386/pc: No need for rtc_state to be an out-parameter
>>    hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
>>      south bridge
>>    hw/isa/piix3: Create USB controller in host device
>>    hw/isa/piix3: Create power management controller in host device
>>    hw/intc/i8259: Introduce i8259 proxy "isa-pic"
>>    hw/isa/piix3: Create ISA PIC in host device
>>    hw/isa/piix3: Create IDE controller in host device
>>    hw/isa/piix3: Wire up ACPI interrupt internally
>>    hw/isa/piix3: Remove extra ';' outside of functions
>>    hw/isa/piix3: Remove unused include
>>    hw/isa/piix3: Add size constraints to rcr_ops
>>    hw/isa/piix3: Modernize reset handling
>>    hw/isa/piix3: Prefer pci_address_space() over get_system_memory()
>>    hw/isa/piix3: Allow board to provide PCI interrupt routes
>>    hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
>>    hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4
>>    hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4
>>    hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"
>>    hw/isa/piix3: Rename typedef PIIX3State to PIIXState
>>    hw/mips/malta: Reuse dev variable
>>    meson: Fix dependencies of piix4 southbridge
>>    hw/isa/piix4: Add missing initialization
>>    hw/isa/piix4: Move pci_ide_create_devs() call to board code
>>    hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>>    hw/isa/piix4: Allow board to provide PCI interrupt routes
>>    hw/isa/piix4: Remove unused code
>>    hw/isa/piix4: Use ISA PIC device
>>    hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>    hw/isa/piix4: Rename reset control operations to match PIIX3
>>    hw/isa/piix4: Rename wrongly named method
>>    hw/isa/piix4: Prefix pci_slot_get_pirq() with "piix4_"
>>    hw/isa/piix3: Merge hw/isa/piix4.c
>>    hw/isa/piix: Harmonize names of reset control memory regions
>>    hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>>    hw/isa/piix: Rename functions to be shared for interrupt triggering
>>    hw/isa/piix: Consolidate IRQ triggering
>>    hw/isa/piix: Unexport PIIXState
>>    hw/isa/piix: Share PIIX3 base class with PIIX4
>>    hw/isa/piix: Drop the "3" from the PIIX base class
>>    hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI
>>      controller
>> 
>>   MAINTAINERS                             |   6 +-
>>   configs/devices/mips-softmmu/common.mak |   3 +-
>>   hw/i386/Kconfig                         |   3 +-
>>   hw/i386/acpi-build.c                    |   4 +-
>>   hw/i386/pc.c                            |  19 +-
>>   hw/i386/pc_piix.c                       |  72 +--
>>   hw/i386/pc_q35.c                        |   3 +-
>>   hw/intc/i8259.c                         |  27 +
>>   hw/isa/Kconfig                          |  14 +-
>>   hw/isa/lpc_ich9.c                       |  11 +
>>   hw/isa/meson.build                      |   3 +-
>>   hw/isa/piix.c                           | 669 ++++++++++++++++++++++++
>>   hw/isa/piix3.c                          | 431 ---------------
>>   hw/isa/piix4.c                          | 325 ------------
>>   hw/mips/malta.c                         |  34 +-
>>   include/hw/i386/ich9.h                  |   2 +
>>   include/hw/i386/pc.h                    |   2 +-
>>   include/hw/intc/i8259.h                 |  14 +
>>   include/hw/southbridge/piix.h           |  41 +-
>>   19 files changed, 823 insertions(+), 860 deletions(-)
>>   create mode 100644 hw/isa/piix.c
>>   delete mode 100644 hw/isa/piix3.c
>>   delete mode 100644 hw/isa/piix4.c
>
>I've had a quick skim over this series and commented on the parts that caught my eye, however I'm generally happy with the way this series is going and it seems like a nice tidy-up - thanks!

I'm glad to read this!

Best regards,
Bernhard
>
>
>ATB,
>
>Mark.