[PATCH v8 00/23] Consolidate PIIX south bridges

[PATCH v8 00/23] 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:

Move sub devices into the PIIX3 south bridge, like PIIX4 does already:
* 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/isa/piix3: Move ISA bus IRQ assignments into host device
* hw/isa/piix3: Create IDE controller in host device
* hw/isa/piix3: Wire up ACPI interrupt internally

Make PIIX3 and PIIX4 south bridges more similar:
* 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: Drop the "3" from PIIX base class
* hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
* hw/isa/piix4: Remove unused inbound ISA interrupt lines
* hw/isa/piix4: Reuse struct PIIXState from PIIX3
* hw/isa/piix4: Create the "intr" property during init() already
* hw/isa/piix4: Rename reset control operations to match PIIX3

This patch achieves the main goal of the series:
* hw/isa/piix3: Merge hw/isa/piix4.c

Perform some further consolidations which were easier to do after the merge:
* hw/isa/piix: Harmonize names of reset control memory regions
* hw/isa/piix: Rename functions to be shared for interrupt triggering
* hw/isa/piix: Consolidate IRQ triggering
* hw/isa/piix: Share PIIX3's base class with PIIX4
* hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4

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

There are still some differences in the device models:
- PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't
- PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't
- Different binary layout in VM state

v8:
- Rebase onto master
- Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize
  method in PIIX4' since it changed considerably in v7.

Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the "3"
from PIIX base class'):
* `make check`
* `make check-avocado`
* 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`
* 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda
debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"`

v7:
- Rebase onto master
- Avoid the PIC proxy (Phil)
  The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
  the south bridges. ISA interrupt wiring requires the GPIO lines to be
  populated already but pc_piix assigned the interrupts only after realizing
  PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
  are already populated during PIIX3's realize phase where the ISA interrupts
  are wired up.
- New patches:
  * hw/isa/piix4: Reuse struct PIIXState from PIIX3
  * hw/isa/piix4: Create the "intr" property during init() already
- Patches with substantial changes (Reviewed-by dropped):
  * hw/isa/piix3: Move ISA bus IRQ assignments into host device

v6:
- Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
  within the patch series.
- Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
  bridges" [2] for maintainer convenience.
- Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
  created' into
  https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
  similar for Malta.
- Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
  https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")

v5:
- Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
- Add patch to make usage of the isa_pic global more type-safe
- Re-introduce isa-pic as PIC specific proxy (Mark)

v4:
- Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
  since it is already queued via mips-next. This eliminates patches
  'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
  Prefix pci_slot_get_pirq() with "piix4_"'.
- Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
  'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
  split these patches since I wasn't sure whether renaming a type was allowed.
- Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
  created' for forther cleanup of INTx-to-LNKx route decoupling.

v3:
- Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
  (Philippe)
- Make proxy PIC generic (Philippe)
- Track Malta's PIIX dependencies through KConfig
- Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
- Also rebase onto latest master to resolve merge conflicts. This required
  copying Philippe's series as first three patches - please ignore.

v2:
- Introduce TYPE_ defines for IDE and USB device models (Mark)
- Omit unexporting of PIIXState (Mark)
- Improve commit message of patch 5 to mention reset triggering through PCI
  configuration space (Mark)
- Move reviewed patches w/o dependencies to the bottom of the series for early
  upstreaming

[1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
[2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
[3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html

Bernhard Beschow (23):
  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/isa/piix3: Move ISA bus IRQ assignments into host device
  hw/isa/piix3: Create IDE controller in host device
  hw/isa/piix3: Wire up ACPI interrupt internally
  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: Drop the "3" from PIIX base class
  hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
  hw/isa/piix4: Remove unused inbound ISA interrupt lines
  hw/isa/piix4: Reuse struct PIIXState from PIIX3
  hw/isa/piix4: Create the "intr" property during init() already
  hw/isa/piix4: Rename reset control operations to match PIIX3
  hw/isa/piix3: Merge hw/isa/piix4.c
  hw/isa/piix: Harmonize names of reset control memory regions
  hw/isa/piix: Rename functions to be shared for interrupt triggering
  hw/isa/piix: Consolidate IRQ triggering
  hw/isa/piix: Share PIIX3's base class with PIIX4
  hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4

 MAINTAINERS                   |   6 +-
 include/hw/i386/pc.h          |   2 +-
 include/hw/southbridge/ich9.h |   2 +
 include/hw/southbridge/piix.h |  28 +++-
 hw/i386/pc.c                  |  16 +-
 hw/i386/pc_piix.c             |  67 ++++----
 hw/i386/pc_q35.c              |   4 +-
 hw/isa/lpc_ich9.c             |   8 +
 hw/isa/{piix3.c => piix.c}    | 306 ++++++++++++++++++++++++++--------
 hw/isa/piix4.c                | 302 ---------------------------------
 hw/mips/malta.c               |   6 +-
 hw/i386/Kconfig               |   3 +-
 hw/isa/Kconfig                |   8 +-
 hw/isa/meson.build            |   3 +-
 hw/mips/Kconfig               |   2 +-
 15 files changed, 334 insertions(+), 429 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (55%)
 delete mode 100644 hw/isa/piix4.c

-- 
2.39.2

[PATCH v8 01/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20221022150508.26830-11-shentey@gmail.com>
---
 include/hw/southbridge/ich9.h |  2 ++
 include/hw/southbridge/piix.h |  4 ++++
 hw/i386/pc.c                  | 12 +++++++++++-
 hw/i386/pc_piix.c             |  8 ++++++++
 hw/i386/pc_q35.c              |  2 ++
 hw/isa/lpc_ich9.c             |  8 ++++++++
 hw/isa/piix3.c                | 15 +++++++++++++++
 hw/isa/Kconfig                |  2 ++
 8 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/include/hw/southbridge/ich9.h b/include/hw/southbridge/ich9.h
index 7004eecbf9..fd01649d04 100644
--- a/include/hw/southbridge/ich9.h
+++ b/include/hw/southbridge/ich9.h
@@ -6,6 +6,7 @@
 #include "hw/intc/ioapic.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_device.h"
+#include "hw/rtc/mc146818rtc.h"
 #include "exec/memory.h"
 #include "qemu/notify.h"
 #include "qom/object.h"
@@ -30,6 +31,7 @@ struct ICH9LPCState {
     */
     uint8_t irr[PCI_SLOT_MAX][PCI_NUM_PINS];
 
+    MC146818RtcState 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 0bf48e936d..9333221ced 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -13,6 +13,8 @@
 #define HW_SOUTHBRIDGE_PIIX_H
 
 #include "hw/pci/pci_device.h"
+#include "qom/object.h"
+#include "hw/rtc/mc146818rtc.h"
 
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
 #define PIIX_PIRQCA 0x60
@@ -51,6 +53,8 @@ struct PIIXState {
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
+    MC146818RtcState rtc;
+
     /* Reset Control Register contents */
     uint8_t rcr;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 992951c107..8fed01ce5d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1306,7 +1306,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 = ISA_DEVICE(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 126b6c11df..93d2be469e 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"
@@ -240,10 +241,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 09004f3f1f..5ccfad84f6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -242,6 +242,8 @@ static void pc_q35_init(MachineState *machine)
                       x86_machine_is_smm_enabled(x86ms));
     pci_realize_and_unref(lpc, host_bus, &error_fatal);
 
+    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,
                              (Object **)&x86ms->acpi_dev,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index d8303d0322..02d1918fc1 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -658,6 +658,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,
@@ -723,6 +725,12 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
 
     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;
+    }
+
     pci_bus_irqs(pci_bus, ich9_lpc_set_irq, d, ICH9_LPC_NB_PIRQS);
     pci_bus_map_irqs(pci_bus, ich9_lpc_map_irq);
     pci_bus_set_route_irq_fn(pci_bus, ich9_route_intx_pin_to_irq);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index a9cb39bf21..f9103ea45a 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/runstate.h"
@@ -301,6 +302,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
     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)
@@ -324,6 +331,13 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     qbus_build_aml(bus, 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);
@@ -350,6 +364,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/hw/isa/Kconfig b/hw/isa/Kconfig
index 0156a66889..c10cbc5fc1 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
@@ -79,3 +80,4 @@ config LPC_ICH9
     select I8257
     select ISA_BUS
     select ACPI_ICH9
+    select MC146818RTC
-- 
2.39.2

[PATCH v8 02/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221022150508.26830-12-shentey@gmail.com>
---
 include/hw/i386/pc.h |  2 +-
 hw/i386/pc.c         | 12 ++++++------
 hw/i386/pc_piix.c    |  2 +-
 hw/i386/pc_q35.c     |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 66e3d059ef..5ba814aca6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -168,7 +168,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,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8fed01ce5d..209a9c138f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1253,7 +1253,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)
 {
@@ -1308,17 +1308,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 93d2be469e..2b5884d3d2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -277,7 +277,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 5ccfad84f6..ae49146d46 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -293,7 +293,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);
 
     if (pcms->sata_enabled) {
-- 
2.39.2

[PATCH v8 03/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-3-shentey@gmail.com>
---
 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 2b5884d3d2..b9d475a226 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -236,7 +236,8 @@ static void pc_init1(MachineState *machine,
                                        : pc_pci_slot_get_pirq);
         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.39.2

[PATCH v8 04/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-13-shentey@gmail.com>
---
 include/hw/southbridge/piix.h |  4 ++++
 hw/i386/pc_piix.c             |  7 ++-----
 hw/isa/piix3.c                | 17 +++++++++++++++++
 hw/isa/Kconfig                |  1 +
 4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 9333221ced..7b9819ece5 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
 #include "hw/pci/pci_device.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];
 
     MC146818RtcState 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;
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b9d475a226..3a6a54bf50 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -53,7 +53,6 @@
 #include "exec/memory.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/piix4.h"
-#include "hw/usb/hcd-uhci.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "sysemu/xen.h"
@@ -237,6 +236,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;
@@ -315,10 +316,6 @@ static void pc_init1(MachineState *machine,
     }
 #endif
 
-    if (pcmc->pci_enabled && machine_usb(machine)) {
-        pci_create_simple(pci_bus, piix3_devfn + 2, TYPE_PIIX3_USB_UHCI);
-    }
-
     if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
         PCIDevice *piix4_pm;
 
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index f9103ea45a..7ae031f2c5 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -288,6 +288,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), pci_address_space(dev),
@@ -308,6 +309,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,
+                                TYPE_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)
@@ -338,6 +349,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);
@@ -357,6 +373,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/hw/isa/Kconfig b/hw/isa/Kconfig
index c10cbc5fc1..f01bc0dff3 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
-- 
2.39.2

[PATCH v8 05/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-14-shentey@gmail.com>
---
 include/hw/southbridge/piix.h |  6 ++++++
 hw/i386/pc_piix.c             | 24 ++++++++++++++----------
 hw/isa/piix3.c                | 14 ++++++++++++++
 hw/isa/Kconfig                |  1 +
 4 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 7b9819ece5..6e6b519dce 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -14,6 +14,7 @@
 
 #include "hw/pci/pci_device.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 {
 
     MC146818RtcState 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;
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3a6a54bf50..0295336d80 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -47,12 +47,12 @@
 #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 "hw/xen/xen.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"
@@ -98,6 +98,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;
@@ -238,15 +239,25 @@ 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;
         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"));
+        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);
 
@@ -316,15 +327,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);
@@ -338,7 +342,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/piix3.c b/hw/isa/piix3.c
index 7ae031f2c5..27bd4dbf65 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -319,6 +319,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)
@@ -350,7 +361,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/hw/isa/Kconfig b/hw/isa/Kconfig
index f01bc0dff3..cf79580384 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
-- 
2.39.2

[PATCH v8 06/23] hw/isa/piix3: Move ISA bus IRQ assignments into host device

[Bernhard Beschow]

Having the south bridge assign the ISA interrupts to the ISA bus itself
makes the south bridge more self contained. Furthermore, it allows ISA
interrupt wiring in pci_piix3_realize() which will be used in subsequent
patches.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0295336d80..50a64f0326 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -246,10 +246,10 @@ static void pc_init1(MachineState *machine,
         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;
+        pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
+
         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),
@@ -260,6 +260,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_register_input_irqs(isa_bus, x86ms->gsi);
 
         rtc_state = isa_new(TYPE_MC146818_RTC);
         qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
@@ -268,7 +269,6 @@ static void pc_init1(MachineState *machine,
         i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
     }
-    isa_bus_register_input_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/piix3.c b/hw/isa/piix3.c
index 27bd4dbf65..2e32a61647 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -302,6 +302,8 @@ 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);
 
+    isa_bus_register_input_irqs(isa_bus, d->pic);
+
     i8257_dma_init(isa_bus, 0);
 
     /* RTC */
-- 
2.39.2

[PATCH v8 07/23] hw/isa/piix3: Create IDE controller in host device

[Bernhard Beschow]

Now that PIIX3 contains the new TYPE_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 TYPE_ISA_PIC even though the virtualization technology isn't
known yet.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-17-shentey@gmail.com>
---
 include/hw/southbridge/piix.h |  2 ++
 hw/i386/pc_piix.c             | 16 +++++++---------
 hw/isa/piix3.c                |  8 ++++++++
 hw/i386/Kconfig               |  1 -
 hw/isa/Kconfig                |  1 +
 5 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 6e6b519dce..14897a00d9 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
 #include "hw/pci/pci_device.h"
 #include "qom/object.h"
 #include "hw/acpi/piix4.h"
+#include "hw/ide/pci.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
 
@@ -56,6 +57,7 @@ struct PIIXState {
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
     MC146818RtcState rtc;
+    PCIIDEState ide;
     UHCIState uhci;
     PIIX4PMState pm;
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 50a64f0326..8456b8a758 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -42,7 +42,6 @@
 #include "net/net.h"
 #include "hw/ide/isa.h"
 #include "hw/ide/pci.h"
-#include "hw/ide/piix.h"
 #include "hw/irq.h"
 #include "sysemu/kvm.h"
 #include "hw/kvm/clock.h"
@@ -99,7 +98,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];
@@ -220,6 +218,7 @@ static void pc_init1(MachineState *machine,
     gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
 
     if (pcmc->pci_enabled) {
+        DeviceState *dev;
         PIIX3State *piix3;
         PCIDevice *pci_dev;
         const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
@@ -250,11 +249,14 @@ static void pc_init1(MachineState *machine,
         piix3->pic = x86ms->gsi;
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
-        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"));
         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;
@@ -268,6 +270,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) {
@@ -296,12 +300,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, TYPE_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/piix3.c b/hw/isa/piix3.c
index 2e32a61647..60ae018a55 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -29,6 +29,7 @@
 #include "hw/southbridge/piix.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
+#include "hw/ide/piix.h"
 #include "hw/isa/isa.h"
 #include "hw/xen/xen.h"
 #include "sysemu/runstate.h"
@@ -312,6 +313,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,
@@ -360,6 +367,7 @@ static void pci_piix3_init(Object *obj)
     PIIX3State *d = PIIX3_PCI_DEVICE(obj);
 
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+    object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
 }
 
 static Property pci_piix3_props[] = {
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 9fbfe748b5..cfbe75d5b0 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -73,7 +73,6 @@ config I440FX
     select PC_ACPI
     select PCI_I440FX
     select PIIX3
-    select IDE_PIIX
     select DIMM
     select SMBIOS
     select FW_CFG_DMA
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index cf79580384..17ddb25afc 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -35,6 +35,7 @@ config PIIX3
     bool
     select ACPI_PIIX4
     select I8257
+    select IDE_PIIX
     select ISA_BUS
     select MC146818RTC
     select USB_UHCI
-- 
2.39.2

[PATCH v8 08/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-18-shentey@gmail.com>
---
 hw/i386/pc_piix.c | 1 -
 hw/isa/piix3.c    | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8456b8a758..3339793a08 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -328,7 +328,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 60ae018a55..e1d9049b3a 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -338,6 +338,7 @@ 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, d->pic[9]);
     }
 }
 
-- 
2.39.2

[PATCH v8 09/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-21-shentey@gmail.com>
---
 include/hw/southbridge/piix.h | 5 ++---
 hw/isa/piix3.c                | 8 ++++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 14897a00d9..87dde11099 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -31,7 +31,6 @@
  */
 #define PIIX_RCR_IOPORT 0xcf9
 
-#define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
 #define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
 
 struct PIIXState {
@@ -43,10 +42,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;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index e1d9049b3a..9eb45810de 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -52,7 +52,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;
     }
 
@@ -66,7 +66,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;
     }
 
@@ -87,7 +87,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 {
@@ -119,7 +119,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);
         }
     }
-- 
2.39.2

[PATCH v8 10/23] hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4

[Bernhard Beschow]

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-22-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 9eb45810de..e11742ce42 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -371,7 +371,7 @@ static void pci_piix3_init(Object *obj)
     object_initialize_child(obj, "ide", &d->ide, TYPE_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_BOOL("has-acpi", PIIX3State, has_acpi, true),
     DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
@@ -398,7 +398,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.39.2

[PATCH v8 11/23] hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4

[Bernhard Beschow]

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-23-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 e11742ce42..15f0105240 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -145,7 +145,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;
@@ -385,7 +385,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.39.2

[PATCH v8 12/23] hw/isa/piix3: Drop the "3" from PIIX base class

[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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-25-shentey@gmail.com>
---
 include/hw/southbridge/piix.h |  6 ++--
 hw/i386/pc_piix.c             |  4 +--
 hw/isa/piix3.c                | 60 +++++++++++++++++------------------
 3 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 87dde11099..62fc5ddab9 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -72,11 +72,9 @@ struct PIIXState {
     bool has_usb;
     bool smm_enabled;
 };
-typedef struct PIIXState PIIX3State;
 
-#define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
-DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
-                         TYPE_PIIX3_PCI_DEVICE)
+#define TYPE_PIIX_PCI_DEVICE "pci-piix"
+OBJECT_DECLARE_SIMPLE_TYPE(PIIXState, PIIX_PCI_DEVICE)
 
 #define TYPE_PIIX3_DEVICE "PIIX3"
 #define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3339793a08..2946cf69ac 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -219,7 +219,7 @@ static void pc_init1(MachineState *machine,
 
     if (pcmc->pci_enabled) {
         DeviceState *dev;
-        PIIX3State *piix3;
+        PIIXState *piix3;
         PCIDevice *pci_dev;
         const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
                                          : TYPE_PIIX3_DEVICE;
@@ -245,7 +245,7 @@ static void pc_init1(MachineState *machine,
         object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
                                  x86_machine_is_smm_enabled(x86ms),
                                  &error_abort);
-        piix3 = PIIX3_PCI_DEVICE(pci_dev);
+        piix3 = PIIX_PCI_DEVICE(pci_dev);
         piix3->pic = x86ms->gsi;
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 15f0105240..2de1112171 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -38,7 +38,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[pic_irq],
                  !!(piix3->pic_levels &
@@ -46,7 +46,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;
@@ -61,7 +61,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;
 
@@ -77,13 +77,13 @@ 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);
 }
 
 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;
 
@@ -98,7 +98,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;
@@ -114,7 +114,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));
@@ -147,7 +147,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 */
@@ -188,7 +188,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;
 
     /*
@@ -211,7 +211,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] =
@@ -223,7 +223,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);
 }
@@ -234,7 +234,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()
     }
 };
@@ -246,8 +246,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()
     },
@@ -260,7 +260,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);
@@ -271,7 +271,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;
 }
@@ -288,7 +288,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;
 
@@ -365,17 +365,17 @@ 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, "rtc", &d->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
 }
 
 static Property pci_piix_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_UINT32("smb_io_base", PIIXState, smb_io_base, 0),
+    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(),
 };
 
@@ -402,10 +402,10 @@ 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 = {
-    .name = TYPE_PIIX3_PCI_DEVICE,
+static const TypeInfo piix_pci_type_info = {
+    .name = TYPE_PIIX_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,
@@ -419,7 +419,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);
@@ -441,14 +441,14 @@ 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,
     .class_init    = piix3_class_init,
 };
 
 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);
@@ -475,13 +475,13 @@ 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,
     .class_init    = piix3_xen_class_init,
 };
 
 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);
 }
-- 
2.39.2

[PATCH v8 13/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-30-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 e0b149f8eb..c13f68733b 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -51,9 +51,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)
@@ -234,17 +241,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,
+                                TYPE_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, s, PIIX_NUM_PIRQS);
 }
@@ -255,13 +271,16 @@ static void piix4_init(Object *obj)
 
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
-    object_initialize_child(obj, "uhci", &s->uhci, TYPE_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);
@@ -280,6 +299,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 ec172b111a..db5e5fd85c 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1254,8 +1254,10 @@ void mips_malta_init(MachineState *machine)
     pci_bus_map_irqs(pci_bus, malta_pci_slot_get_pirq);
 
     /* Southbridge */
-    piix4 = pci_create_simple_multifunction(pci_bus, PIIX4_PCI_DEVFN, true,
-                                            TYPE_PIIX4_PCI_DEVICE);
+    piix4 = pci_new_multifunction(PIIX4_PCI_DEVFN, 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.39.2

[PATCH v8 14/23] hw/isa/piix4: Remove unused inbound ISA interrupt lines

[Bernhard Beschow]

The Malta board, which is the only user of PIIX4, doesn't connect to the
exported interrupt lines. PIIX3 doesn't expose such interrupt lines
either, so remove them for PIIX4 for simplicity and consistency.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-32-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 c13f68733b..a88ae859c4 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -155,12 +155,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)
 {
@@ -204,8 +198,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.39.2

[PATCH v8 15/23] hw/isa/piix4: Reuse struct PIIXState from PIIX3

[Bernhard Beschow]

PIIX4 has its own, private PIIX4State structure. PIIX3 has almost the
same structure, provided in a public header. So reuse it and add a
cpu_intr attribute which is only used by PIIX4.

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 62fc5ddab9..b60061d2d7 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -50,6 +50,7 @@ struct PIIXState {
 #endif
     uint64_t pic_levels;
 
+    qemu_irq cpu_intr;
     qemu_irq *pic;
 
     /* This member isn't used. Just for save/load compatibility */
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a88ae859c4..b74b7dc5d3 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -42,33 +42,10 @@
 #include "sysemu/runstate.h"
 #include "qom/object.h"
 
-struct PIIX4State {
-    PCIDevice dev;
-    qemu_irq cpu_intr;
-    qemu_irq *isa;
-
-    MC146818RtcState rtc;
-    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)
-
 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 */
@@ -82,13 +59,13 @@ 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[pic_irq], pic_level);
     }
 }
 
 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
@@ -123,12 +100,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_post_load(void *opaque, int version_id)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
 
     if (version_id == 2) {
         s->rcr = 0;
@@ -143,22 +121,22 @@ static const VMStateDescription vmstate_piix4 = {
     .minimum_version_id = 2,
     .post_load = piix4_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()
     }
 };
 
 static void piix4_request_i8259_irq(void *opaque, int irq, int level)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
     qemu_set_irq(s->cpu_intr, level);
 }
 
 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);
@@ -170,7 +148,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;
 }
@@ -187,7 +165,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;
     qemu_irq *i8259_out_irq;
@@ -208,10 +186,10 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 
     /* initialize i8259 pic */
     i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    s->isa = i8259_init(isa_bus, *i8259_out_irq);
+    s->pic = i8259_init(isa_bus, *i8259_out_irq);
 
     /* initialize ISA irqs */
-    isa_bus_register_input_irqs(isa_bus, s->isa);
+    isa_bus_register_input_irqs(isa_bus, s->pic);
 
     /* initialize pit */
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
@@ -251,7 +229,7 @@ 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, s->pic[9]);
     }
 
     pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
@@ -259,17 +237,17 @@ 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, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
 }
 
 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_UINT32("smb_io_base", PIIXState, smb_io_base, 0),
+    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(),
 };
 
@@ -297,7 +275,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.39.2

[PATCH v8 16/23] hw/isa/piix4: Create the "intr" property during init() already

[Bernhard Beschow]

Only PIIX4 provides this property while PIIX3 doesn't.

Unlike initialize() methods, init() methods allow for device-specific
quirks without having to call the parent method. So move the creation of
the property into PIIX4's init() method as a preparation of the code
merge and the subsequent cleanup of the realize methods.

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

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index b74b7dc5d3..d70c74dcf9 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -176,9 +176,6 @@ 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),
@@ -239,6 +236,8 @@ static void piix4_init(Object *obj)
 {
     PIIXState *s = PIIX_PCI_DEVICE(obj);
 
+    qdev_init_gpio_out_named(DEVICE(obj), &s->cpu_intr, "intr", 1);
+
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
 }
-- 
2.39.2

[PATCH v8 17/23] 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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-35-shentey@gmail.com>
---
 hw/isa/piix4.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index d70c74dcf9..9388767d74 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -133,8 +133,8 @@ static void piix4_request_i8259_irq(void *opaque, int irq, int level)
     qemu_set_irq(s->cpu_intr, level);
 }
 
-static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
-                            unsigned int len)
+static void rcr_write(void *opaque, hwaddr addr, uint64_t val,
+                      unsigned int len)
 {
     PIIXState *s = opaque;
 
@@ -146,16 +146,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,
@@ -176,7 +176,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.39.2

[PATCH v8 18/23] hw/isa/piix3: Merge hw/isa/piix4.c

[Bernhard Beschow]

Now that the PIIX3 and PIIX4 device models are sufficiently consolidated,
their implementations can be merged into one file for further
consolidation.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-37-shentey@gmail.com>
---
 MAINTAINERS                |   6 +-
 hw/isa/{piix3.c => piix.c} | 165 +++++++++++++++++++++
 hw/isa/piix4.c             | 291 -------------------------------------
 hw/i386/Kconfig            |   2 +-
 hw/isa/Kconfig             |  11 +-
 hw/isa/meson.build         |   3 +-
 hw/mips/Kconfig            |   2 +-
 7 files changed, 172 insertions(+), 308 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (74%)
 delete mode 100644 hw/isa/piix4.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e96e9dbfe6..3fe5cb359e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1255,7 +1255,7 @@ Malta
 M: Philippe Mathieu-Daudé <philmd@linaro.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/pci-host/gt64120.c
@@ -1674,7 +1674,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
@@ -2363,7 +2363,7 @@ PIIX4 South Bridge (i82371AB)
 M: Hervé Poussineau <hpoussin@reactos.org>
 M: Philippe Mathieu-Daudé <philmd@linaro.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/hw/isa/piix3.c b/hw/isa/piix.c
similarity index 74%
rename from hw/isa/piix3.c
rename to hw/isa/piix.c
index 2de1112171..09d411a6b7 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
@@ -27,9 +28,11 @@
 #include "qapi/error.h"
 #include "hw/dma/i8257.h"
 #include "hw/southbridge/piix.h"
+#include "hw/timer/i8254.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/ide/piix.h"
+#include "hw/intc/i8259.h"
 #include "hw/isa/isa.h"
 #include "hw/xen/xen.h"
 #include "sysemu/runstate.h"
@@ -81,6 +84,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[pic_irq], pic_level);
+    }
+}
+
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
     PIIXState *piix3 = opaque;
@@ -208,6 +232,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;
@@ -257,6 +292,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)
 {
@@ -479,11 +525,130 @@ static const TypeInfo piix3_xen_info = {
     .class_init    = piix3_xen_class_init,
 };
 
+static void piix4_request_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIXState *s = opaque;
+    qemu_set_irq(s->cpu_intr, level);
+}
+
+static void piix4_realize(PCIDevice *dev, Error **errp)
+{
+    PIIXState *s = PIIX_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);
+    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 */
+    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
+    s->pic = i8259_init(isa_bus, *i8259_out_irq);
+
+    /* initialize ISA irqs */
+    isa_bus_register_input_irqs(isa_bus, s->pic);
+
+    /* 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 = 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,
+                                TYPE_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, s->pic[9]);
+    }
+
+    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+}
+
+static void piix4_init(Object *obj)
+{
+    PIIXState *s = PIIX_PCI_DEVICE(obj);
+
+    qdev_init_gpio_out_named(DEVICE(obj), &s->cpu_intr, "intr", 1);
+
+    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
+    object_initialize_child(obj, "ide", &s->ide, TYPE_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(&piix_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 9388767d74..0000000000
--- a/hw/isa/piix4.c
+++ /dev/null
@@ -1,291 +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/ide/piix.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[pic_irq], pic_level);
-    }
-}
-
-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[0x60] = 0x80;
-    pci_conf[0x61] = 0x80;
-    pci_conf[0x62] = 0x80;
-    pci_conf[0x63] = 0x80;
-    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 piix4_request_i8259_irq(void *opaque, int irq, int level)
-{
-    PIIXState *s = opaque;
-    qemu_set_irq(s->cpu_intr, level);
-}
-
-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;
-    qemu_irq *i8259_out_irq;
-
-    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 */
-    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    s->pic = i8259_init(isa_bus, *i8259_out_irq);
-
-    /* initialize ISA irqs */
-    isa_bus_register_input_irqs(isa_bus, s->pic);
-
-    /* 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 = isa_get_irq(ISA_DEVICE(&s->rtc), 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,
-                                TYPE_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, s->pic[9]);
-    }
-
-    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
-}
-
-static void piix4_init(Object *obj)
-{
-    PIIXState *s = PIIX_PCI_DEVICE(obj);
-
-    qdev_init_gpio_out_named(DEVICE(obj), &s->cpu_intr, "intr", 1);
-
-    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
-    object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
-}
-
-static Property piix4_props[] = {
-    DEFINE_PROP_UINT32("smb_io_base", PIIXState, smb_io_base, 0),
-    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)
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index cfbe75d5b0..62adf440f2 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -72,7 +72,7 @@ config I440FX
     select PC_PCI
     select PC_ACPI
     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 17ddb25afc..040a18c070 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -31,16 +31,7 @@ config PC87312
     select FDC_ISA
     select IDE_ISA
 
-config PIIX3
-    bool
-    select ACPI_PIIX4
-    select I8257
-    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/mips/Kconfig b/hw/mips/Kconfig
index da3a37e215..ac1eb06a51 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -2,7 +2,7 @@ config MALTA
     bool
     select GT64120
     select ISA_SUPERIO
-    select PIIX4
+    select PIIX
 
 config MIPSSIM
     bool
-- 
2.39.2

[PATCH v8 19/23] 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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-38-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 09d411a6b7..8206cbebf3 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -345,7 +345,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     }
 
     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);
 
@@ -545,7 +545,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.39.2

[PATCH v8 20/23] 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-40-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 8206cbebf3..d6c39d6f37 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -41,47 +41,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[pic_irq],
-                 !!(piix3->pic_levels &
+    qemu_set_irq(piix->pic[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)
@@ -122,29 +122,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);
         }
     }
 }
@@ -166,7 +166,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)
@@ -226,7 +226,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;
@@ -473,7 +473,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pci_bus_irqs(pci_bus, piix3_set_irq, piix3, PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, piix_set_irq, piix3, PIIX_NUM_PIRQS);
     pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
 }
 
@@ -481,7 +481,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.39.2

[PATCH v8 21/23] hw/isa/piix: Consolidate IRQ triggering

[Bernhard Beschow]

Speeds up PIIX4 which resolves an old TODO.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-41-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 d6c39d6f37..30873b0764 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -84,27 +84,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[pic_irq], pic_level);
-    }
-}
-
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
     PIIXState *piix3 = opaque;
@@ -240,7 +219,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)
@@ -597,7 +576,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->pic[9]);
     }
 
-    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, piix_set_irq, s, PIIX_NUM_PIRQS);
 }
 
 static void piix4_init(Object *obj)
@@ -615,6 +594,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.39.2

[PATCH v8 22/23] hw/isa/piix: Share PIIX3's base class with PIIX4

[Bernhard Beschow]

Having a common base class will allow 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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-42-shentey@gmail.com>
---
 hw/isa/piix.c | 48 ++++++++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 30873b0764..5d4e411e33 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -388,12 +388,11 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     qbus_build_aml(bus, 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, "rtc", &d->rtc, TYPE_MC146818_RTC);
-    object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
 }
 
 static Property pci_piix_props[] = {
@@ -404,7 +403,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);
@@ -412,11 +411,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
@@ -431,9 +427,9 @@ static const TypeInfo piix_pci_type_info = {
     .name = TYPE_PIIX_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 },
@@ -456,17 +452,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, TYPE_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_PIIX_PCI_DEVICE,
+    .instance_init = piix3_init,
     .class_init    = piix3_class_init,
 };
 
@@ -492,15 +500,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_PIIX_PCI_DEVICE,
+    .instance_init = piix3_init,
     .class_init    = piix3_xen_class_init,
 };
 
@@ -585,7 +598,6 @@ static void piix4_init(Object *obj)
 
     qdev_init_gpio_out_named(DEVICE(obj), &s->cpu_intr, "intr", 1);
 
-    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
 }
 
@@ -596,31 +608,15 @@ 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_PIIX_PCI_DEVICE,
     .instance_init = piix4_init,
     .class_init    = piix4_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
 };
 
 static void piix3_register_types(void)
-- 
2.39.2

[PATCH v8 23/23] 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 | 78 ++++++++++++++++-----------------------------------
 1 file changed, 24 insertions(+), 54 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 5d4e411e33..945c47b7e9 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -311,17 +311,11 @@ 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,
+                             ISABus *isa_bus, Error **errp)
 {
     PIIXState *d = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
-    ISABus *isa_bus;
-
-    isa_bus = isa_bus_new(DEVICE(d), pci_address_space(dev),
-                          pci_address_space_io(dev), errp);
-    if (!isa_bus) {
-        return;
-    }
 
     memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
                           "piix-reset-control", 1);
@@ -346,8 +340,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 
     /* USB */
     if (d->has_usb) {
-        object_initialize_child(OBJECT(dev), "uhci", &d->uhci,
-                                TYPE_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;
@@ -442,8 +435,15 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
     ERRP_GUARD();
     PIIXState *piix3 = 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;
+    }
 
-    pci_piix3_realize(dev, errp);
+    pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, isa_bus, errp);
     if (*errp) {
         return;
     }
@@ -483,8 +483,15 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
     ERRP_GUARD();
     PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
+    ISABus *isa_bus;
 
-    pci_piix3_realize(dev, errp);
+    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
+                          pci_address_space_io(dev), errp);
+    if (!isa_bus) {
+        return;
+    }
+
+    pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, isa_bus, errp);
     if (*errp) {
         return;
     }
@@ -525,6 +532,7 @@ static void piix4_request_i8259_irq(void *opaque, int irq, int level)
 
 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;
@@ -536,59 +544,21 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         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 */
     i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
     s->pic = i8259_init(isa_bus, *i8259_out_irq);
 
-    /* initialize ISA irqs */
-    isa_bus_register_input_irqs(isa_bus, s->pic);
+    pci_piix_realize(dev, TYPE_PIIX4_USB_UHCI, isa_bus, errp);
+    if (*errp) {
+        return;
+    }
 
     /* 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 = 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,
-                                TYPE_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, s->pic[9]);
-    }
-
     pci_bus_irqs(pci_bus, piix_set_irq, s, PIIX_NUM_PIRQS);
 }
 
-- 
2.39.2

Re: [PATCH v8 00/23] Consolidate PIIX south bridges

[Michael S. Tsirkin]

On Thu, Mar 02, 2023 at 10:21:38PM +0100, 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.

Hi!
No freeze is over, could you rebase pls?
I could try to resolve the conflicts but this is so big I'd rather
not take the risk of messing it up.

> The series is structured as follows:
> 
> Move sub devices into the PIIX3 south bridge, like PIIX4 does already:
> * 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/isa/piix3: Move ISA bus IRQ assignments into host device
> * hw/isa/piix3: Create IDE controller in host device
> * hw/isa/piix3: Wire up ACPI interrupt internally
> 
> Make PIIX3 and PIIX4 south bridges more similar:
> * 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: Drop the "3" from PIIX base class
> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
> * hw/isa/piix4: Remove unused inbound ISA interrupt lines
> * hw/isa/piix4: Reuse struct PIIXState from PIIX3
> * hw/isa/piix4: Create the "intr" property during init() already
> * hw/isa/piix4: Rename reset control operations to match PIIX3
> 
> This patch achieves the main goal of the series:
> * hw/isa/piix3: Merge hw/isa/piix4.c
> 
> Perform some further consolidations which were easier to do after the merge:
> * hw/isa/piix: Harmonize names of reset control memory regions
> * hw/isa/piix: Rename functions to be shared for interrupt triggering
> * hw/isa/piix: Consolidate IRQ triggering
> * hw/isa/piix: Share PIIX3's base class with PIIX4
> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
> 
> One challenge was dealing with optional devices where Peter already gave advice
> in [1] which this series implements.
> 
> There are still some differences in the device models:
> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't
> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't
> - Different binary layout in VM state
> 
> v8:
> - Rebase onto master
> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize
>   method in PIIX4' since it changed considerably in v7.
> 
> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the "3"
> from PIIX base class'):
> * `make check`
> * `make check-avocado`
> * 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`
> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda
> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"`
> 
> v7:
> - Rebase onto master
> - Avoid the PIC proxy (Phil)
>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
>   populated already but pc_piix assigned the interrupts only after realizing
>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
>   are already populated during PIIX3's realize phase where the ISA interrupts
>   are wired up.
> - New patches:
>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>   * hw/isa/piix4: Create the "intr" property during init() already
> - Patches with substantial changes (Reviewed-by dropped):
>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
> 
> v6:
> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
>   within the patch series.
> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
>   bridges" [2] for maintainer convenience.
> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>   created' into
>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
>   similar for Malta.
> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
> 
> v5:
> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
> - Add patch to make usage of the isa_pic global more type-safe
> - Re-introduce isa-pic as PIC specific proxy (Mark)
> 
> v4:
> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
>   since it is already queued via mips-next. This eliminates patches
>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
>   Prefix pci_slot_get_pirq() with "piix4_"'.
> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
>   split these patches since I wasn't sure whether renaming a type was allowed.
> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>   created' for forther cleanup of INTx-to-LNKx route decoupling.
> 
> v3:
> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
>   (Philippe)
> - Make proxy PIC generic (Philippe)
> - Track Malta's PIIX dependencies through KConfig
> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
> - Also rebase onto latest master to resolve merge conflicts. This required
>   copying Philippe's series as first three patches - please ignore.
> 
> v2:
> - Introduce TYPE_ defines for IDE and USB device models (Mark)
> - Omit unexporting of PIIXState (Mark)
> - Improve commit message of patch 5 to mention reset triggering through PCI
>   configuration space (Mark)
> - Move reviewed patches w/o dependencies to the bottom of the series for early
>   upstreaming
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
> 
> Bernhard Beschow (23):
>   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/isa/piix3: Move ISA bus IRQ assignments into host device
>   hw/isa/piix3: Create IDE controller in host device
>   hw/isa/piix3: Wire up ACPI interrupt internally
>   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: Drop the "3" from PIIX base class
>   hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
>   hw/isa/piix4: Create the "intr" property during init() already
>   hw/isa/piix4: Rename reset control operations to match PIIX3
>   hw/isa/piix3: Merge hw/isa/piix4.c
>   hw/isa/piix: Harmonize names of reset control memory regions
>   hw/isa/piix: Rename functions to be shared for interrupt triggering
>   hw/isa/piix: Consolidate IRQ triggering
>   hw/isa/piix: Share PIIX3's base class with PIIX4
>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
> 
>  MAINTAINERS                   |   6 +-
>  include/hw/i386/pc.h          |   2 +-
>  include/hw/southbridge/ich9.h |   2 +
>  include/hw/southbridge/piix.h |  28 +++-
>  hw/i386/pc.c                  |  16 +-
>  hw/i386/pc_piix.c             |  67 ++++----
>  hw/i386/pc_q35.c              |   4 +-
>  hw/isa/lpc_ich9.c             |   8 +
>  hw/isa/{piix3.c => piix.c}    | 306 ++++++++++++++++++++++++++--------
>  hw/isa/piix4.c                | 302 ---------------------------------
>  hw/mips/malta.c               |   6 +-
>  hw/i386/Kconfig               |   3 +-
>  hw/isa/Kconfig                |   8 +-
>  hw/isa/meson.build            |   3 +-
>  hw/mips/Kconfig               |   2 +-
>  15 files changed, 334 insertions(+), 429 deletions(-)
>  rename hw/isa/{piix3.c => piix.c} (55%)
>  delete mode 100644 hw/isa/piix4.c
> 
> -- 
> 2.39.2
> 

Re: [PATCH v8 00/23] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 21. April 2023 07:15:18 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>On Thu, Mar 02, 2023 at 10:21:38PM +0100, 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.
>
>Hi!
>No freeze is over, could you rebase pls?
>I could try to resolve the conflicts but this is so big I'd rather
>not take the risk of messing it up.

Sure! Since this series is still under discussion I'd wait for the PIIX3 Xen decoupling series to land in master. This will simplify this series a bit by taking Xen out of the equation.

Best regards,
Bernhard

>
>> The series is structured as follows:
>> 
>> Move sub devices into the PIIX3 south bridge, like PIIX4 does already:
>> * 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/isa/piix3: Move ISA bus IRQ assignments into host device
>> * hw/isa/piix3: Create IDE controller in host device
>> * hw/isa/piix3: Wire up ACPI interrupt internally
>> 
>> Make PIIX3 and PIIX4 south bridges more similar:
>> * 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: Drop the "3" from PIIX base class
>> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>> * hw/isa/piix4: Remove unused inbound ISA interrupt lines
>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>> * hw/isa/piix4: Create the "intr" property during init() already
>> * hw/isa/piix4: Rename reset control operations to match PIIX3
>> 
>> This patch achieves the main goal of the series:
>> * hw/isa/piix3: Merge hw/isa/piix4.c
>> 
>> Perform some further consolidations which were easier to do after the merge:
>> * hw/isa/piix: Harmonize names of reset control memory regions
>> * hw/isa/piix: Rename functions to be shared for interrupt triggering
>> * hw/isa/piix: Consolidate IRQ triggering
>> * hw/isa/piix: Share PIIX3's base class with PIIX4
>> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>> 
>> One challenge was dealing with optional devices where Peter already gave advice
>> in [1] which this series implements.
>> 
>> There are still some differences in the device models:
>> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't
>> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't
>> - Different binary layout in VM state
>> 
>> v8:
>> - Rebase onto master
>> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize
>>   method in PIIX4' since it changed considerably in v7.
>> 
>> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the "3"
>> from PIIX base class'):
>> * `make check`
>> * `make check-avocado`
>> * 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`
>> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda
>> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"`
>> 
>> v7:
>> - Rebase onto master
>> - Avoid the PIC proxy (Phil)
>>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
>>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
>>   populated already but pc_piix assigned the interrupts only after realizing
>>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
>>   are already populated during PIIX3's realize phase where the ISA interrupts
>>   are wired up.
>> - New patches:
>>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>   * hw/isa/piix4: Create the "intr" property during init() already
>> - Patches with substantial changes (Reviewed-by dropped):
>>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
>> 
>> v6:
>> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
>>   within the patch series.
>> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
>>   bridges" [2] for maintainer convenience.
>> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>>   created' into
>>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
>>   similar for Malta.
>> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
>>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
>> 
>> v5:
>> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
>> - Add patch to make usage of the isa_pic global more type-safe
>> - Re-introduce isa-pic as PIC specific proxy (Mark)
>> 
>> v4:
>> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
>>   since it is already queued via mips-next. This eliminates patches
>>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
>>   Prefix pci_slot_get_pirq() with "piix4_"'.
>> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
>>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
>>   split these patches since I wasn't sure whether renaming a type was allowed.
>> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>>   created' for forther cleanup of INTx-to-LNKx route decoupling.
>> 
>> v3:
>> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
>>   (Philippe)
>> - Make proxy PIC generic (Philippe)
>> - Track Malta's PIIX dependencies through KConfig
>> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
>> - Also rebase onto latest master to resolve merge conflicts. This required
>>   copying Philippe's series as first three patches - please ignore.
>> 
>> v2:
>> - Introduce TYPE_ defines for IDE and USB device models (Mark)
>> - Omit unexporting of PIIXState (Mark)
>> - Improve commit message of patch 5 to mention reset triggering through PCI
>>   configuration space (Mark)
>> - Move reviewed patches w/o dependencies to the bottom of the series for early
>>   upstreaming
>> 
>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
>> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
>> 
>> Bernhard Beschow (23):
>>   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/isa/piix3: Move ISA bus IRQ assignments into host device
>>   hw/isa/piix3: Create IDE controller in host device
>>   hw/isa/piix3: Wire up ACPI interrupt internally
>>   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: Drop the "3" from PIIX base class
>>   hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
>>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>   hw/isa/piix4: Create the "intr" property during init() already
>>   hw/isa/piix4: Rename reset control operations to match PIIX3
>>   hw/isa/piix3: Merge hw/isa/piix4.c
>>   hw/isa/piix: Harmonize names of reset control memory regions
>>   hw/isa/piix: Rename functions to be shared for interrupt triggering
>>   hw/isa/piix: Consolidate IRQ triggering
>>   hw/isa/piix: Share PIIX3's base class with PIIX4
>>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>> 
>>  MAINTAINERS                   |   6 +-
>>  include/hw/i386/pc.h          |   2 +-
>>  include/hw/southbridge/ich9.h |   2 +
>>  include/hw/southbridge/piix.h |  28 +++-
>>  hw/i386/pc.c                  |  16 +-
>>  hw/i386/pc_piix.c             |  67 ++++----
>>  hw/i386/pc_q35.c              |   4 +-
>>  hw/isa/lpc_ich9.c             |   8 +
>>  hw/isa/{piix3.c => piix.c}    | 306 ++++++++++++++++++++++++++--------
>>  hw/isa/piix4.c                | 302 ---------------------------------
>>  hw/mips/malta.c               |   6 +-
>>  hw/i386/Kconfig               |   3 +-
>>  hw/isa/Kconfig                |   8 +-
>>  hw/isa/meson.build            |   3 +-
>>  hw/mips/Kconfig               |   2 +-
>>  15 files changed, 334 insertions(+), 429 deletions(-)
>>  rename hw/isa/{piix3.c => piix.c} (55%)
>>  delete mode 100644 hw/isa/piix4.c
>> 
>> -- 
>> 2.39.2
>> 
>

Re: [PATCH v8 00/23] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 21. April 2023 16:40:47 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 21. April 2023 07:15:18 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>>On Thu, Mar 02, 2023 at 10:21:38PM +0100, 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.
>>
>>Hi!
>>No freeze is over, could you rebase pls?
>>I could try to resolve the conflicts but this is so big I'd rather
>>not take the risk of messing it up.
>
>Sure! Since this series is still under discussion I'd wait for the PIIX3 Xen decoupling series to land in master. This will simplify this series a bit by taking Xen out of the equation.

Could we queue the first two RTC patches already? IMO they're useful general PC cleanups on their own.

Best regards,
Bernhard

>
>Best regards,
>Bernhard
>
>>
>>> The series is structured as follows:
>>> 
>>> Move sub devices into the PIIX3 south bridge, like PIIX4 does already:
>>> * 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/isa/piix3: Move ISA bus IRQ assignments into host device
>>> * hw/isa/piix3: Create IDE controller in host device
>>> * hw/isa/piix3: Wire up ACPI interrupt internally
>>> 
>>> Make PIIX3 and PIIX4 south bridges more similar:
>>> * 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: Drop the "3" from PIIX base class
>>> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>>> * hw/isa/piix4: Remove unused inbound ISA interrupt lines
>>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>> * hw/isa/piix4: Create the "intr" property during init() already
>>> * hw/isa/piix4: Rename reset control operations to match PIIX3
>>> 
>>> This patch achieves the main goal of the series:
>>> * hw/isa/piix3: Merge hw/isa/piix4.c
>>> 
>>> Perform some further consolidations which were easier to do after the merge:
>>> * hw/isa/piix: Harmonize names of reset control memory regions
>>> * hw/isa/piix: Rename functions to be shared for interrupt triggering
>>> * hw/isa/piix: Consolidate IRQ triggering
>>> * hw/isa/piix: Share PIIX3's base class with PIIX4
>>> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>>> 
>>> One challenge was dealing with optional devices where Peter already gave advice
>>> in [1] which this series implements.
>>> 
>>> There are still some differences in the device models:
>>> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't
>>> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't
>>> - Different binary layout in VM state
>>> 
>>> v8:
>>> - Rebase onto master
>>> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize
>>>   method in PIIX4' since it changed considerably in v7.
>>> 
>>> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the "3"
>>> from PIIX base class'):
>>> * `make check`
>>> * `make check-avocado`
>>> * 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`
>>> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda
>>> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"`
>>> 
>>> v7:
>>> - Rebase onto master
>>> - Avoid the PIC proxy (Phil)
>>>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
>>>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
>>>   populated already but pc_piix assigned the interrupts only after realizing
>>>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
>>>   are already populated during PIIX3's realize phase where the ISA interrupts
>>>   are wired up.
>>> - New patches:
>>>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>   * hw/isa/piix4: Create the "intr" property during init() already
>>> - Patches with substantial changes (Reviewed-by dropped):
>>>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
>>> 
>>> v6:
>>> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
>>>   within the patch series.
>>> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
>>>   bridges" [2] for maintainer convenience.
>>> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>>>   created' into
>>>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
>>>   similar for Malta.
>>> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
>>>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
>>> 
>>> v5:
>>> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
>>> - Add patch to make usage of the isa_pic global more type-safe
>>> - Re-introduce isa-pic as PIC specific proxy (Mark)
>>> 
>>> v4:
>>> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
>>>   since it is already queued via mips-next. This eliminates patches
>>>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
>>>   Prefix pci_slot_get_pirq() with "piix4_"'.
>>> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
>>>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
>>>   split these patches since I wasn't sure whether renaming a type was allowed.
>>> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>>>   created' for forther cleanup of INTx-to-LNKx route decoupling.
>>> 
>>> v3:
>>> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
>>>   (Philippe)
>>> - Make proxy PIC generic (Philippe)
>>> - Track Malta's PIIX dependencies through KConfig
>>> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
>>> - Also rebase onto latest master to resolve merge conflicts. This required
>>>   copying Philippe's series as first three patches - please ignore.
>>> 
>>> v2:
>>> - Introduce TYPE_ defines for IDE and USB device models (Mark)
>>> - Omit unexporting of PIIXState (Mark)
>>> - Improve commit message of patch 5 to mention reset triggering through PCI
>>>   configuration space (Mark)
>>> - Move reviewed patches w/o dependencies to the bottom of the series for early
>>>   upstreaming
>>> 
>>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>>> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
>>> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
>>> 
>>> Bernhard Beschow (23):
>>>   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/isa/piix3: Move ISA bus IRQ assignments into host device
>>>   hw/isa/piix3: Create IDE controller in host device
>>>   hw/isa/piix3: Wire up ACPI interrupt internally
>>>   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: Drop the "3" from PIIX base class
>>>   hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>>>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
>>>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>   hw/isa/piix4: Create the "intr" property during init() already
>>>   hw/isa/piix4: Rename reset control operations to match PIIX3
>>>   hw/isa/piix3: Merge hw/isa/piix4.c
>>>   hw/isa/piix: Harmonize names of reset control memory regions
>>>   hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>   hw/isa/piix: Consolidate IRQ triggering
>>>   hw/isa/piix: Share PIIX3's base class with PIIX4
>>>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>>> 
>>>  MAINTAINERS                   |   6 +-
>>>  include/hw/i386/pc.h          |   2 +-
>>>  include/hw/southbridge/ich9.h |   2 +
>>>  include/hw/southbridge/piix.h |  28 +++-
>>>  hw/i386/pc.c                  |  16 +-
>>>  hw/i386/pc_piix.c             |  67 ++++----
>>>  hw/i386/pc_q35.c              |   4 +-
>>>  hw/isa/lpc_ich9.c             |   8 +
>>>  hw/isa/{piix3.c => piix.c}    | 306 ++++++++++++++++++++++++++--------
>>>  hw/isa/piix4.c                | 302 ---------------------------------
>>>  hw/mips/malta.c               |   6 +-
>>>  hw/i386/Kconfig               |   3 +-
>>>  hw/isa/Kconfig                |   8 +-
>>>  hw/isa/meson.build            |   3 +-
>>>  hw/mips/Kconfig               |   2 +-
>>>  15 files changed, 334 insertions(+), 429 deletions(-)
>>>  rename hw/isa/{piix3.c => piix.c} (55%)
>>>  delete mode 100644 hw/isa/piix4.c
>>> 
>>> -- 
>>> 2.39.2
>>> 
>>

Re: [PATCH v8 00/23] Consolidate PIIX south bridges

[Michael S. Tsirkin]

On Wed, May 10, 2023 at 06:39:49PM +0000, Bernhard Beschow wrote:
> 
> 
> Am 21. April 2023 16:40:47 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
> >
> >
> >Am 21. April 2023 07:15:18 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
> >>On Thu, Mar 02, 2023 at 10:21:38PM +0100, 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.
> >>
> >>Hi!
> >>No freeze is over, could you rebase pls?
> >>I could try to resolve the conflicts but this is so big I'd rather
> >>not take the risk of messing it up.
> >
> >Sure! Since this series is still under discussion I'd wait for the PIIX3 Xen decoupling series to land in master. This will simplify this series a bit by taking Xen out of the equation.
> 
> Could we queue the first two RTC patches already? IMO they're useful general PC cleanups on their own.
> 
> Best regards,
> Bernhard

queues 1 and 2.

> >
> >Best regards,
> >Bernhard
> >
> >>
> >>> The series is structured as follows:
> >>> 
> >>> Move sub devices into the PIIX3 south bridge, like PIIX4 does already:
> >>> * 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/isa/piix3: Move ISA bus IRQ assignments into host device
> >>> * hw/isa/piix3: Create IDE controller in host device
> >>> * hw/isa/piix3: Wire up ACPI interrupt internally
> >>> 
> >>> Make PIIX3 and PIIX4 south bridges more similar:
> >>> * 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: Drop the "3" from PIIX base class
> >>> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
> >>> * hw/isa/piix4: Remove unused inbound ISA interrupt lines
> >>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>> * hw/isa/piix4: Create the "intr" property during init() already
> >>> * hw/isa/piix4: Rename reset control operations to match PIIX3
> >>> 
> >>> This patch achieves the main goal of the series:
> >>> * hw/isa/piix3: Merge hw/isa/piix4.c
> >>> 
> >>> Perform some further consolidations which were easier to do after the merge:
> >>> * hw/isa/piix: Harmonize names of reset control memory regions
> >>> * hw/isa/piix: Rename functions to be shared for interrupt triggering
> >>> * hw/isa/piix: Consolidate IRQ triggering
> >>> * hw/isa/piix: Share PIIX3's base class with PIIX4
> >>> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
> >>> 
> >>> One challenge was dealing with optional devices where Peter already gave advice
> >>> in [1] which this series implements.
> >>> 
> >>> There are still some differences in the device models:
> >>> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't
> >>> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't
> >>> - Different binary layout in VM state
> >>> 
> >>> v8:
> >>> - Rebase onto master
> >>> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize
> >>>   method in PIIX4' since it changed considerably in v7.
> >>> 
> >>> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the "3"
> >>> from PIIX base class'):
> >>> * `make check`
> >>> * `make check-avocado`
> >>> * 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`
> >>> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda
> >>> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"`
> >>> 
> >>> v7:
> >>> - Rebase onto master
> >>> - Avoid the PIC proxy (Phil)
> >>>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
> >>>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
> >>>   populated already but pc_piix assigned the interrupts only after realizing
> >>>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
> >>>   are already populated during PIIX3's realize phase where the ISA interrupts
> >>>   are wired up.
> >>> - New patches:
> >>>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>>   * hw/isa/piix4: Create the "intr" property during init() already
> >>> - Patches with substantial changes (Reviewed-by dropped):
> >>>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
> >>> 
> >>> v6:
> >>> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
> >>>   within the patch series.
> >>> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
> >>>   bridges" [2] for maintainer convenience.
> >>> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
> >>>   created' into
> >>>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
> >>>   similar for Malta.
> >>> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
> >>>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
> >>> 
> >>> v5:
> >>> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
> >>> - Add patch to make usage of the isa_pic global more type-safe
> >>> - Re-introduce isa-pic as PIC specific proxy (Mark)
> >>> 
> >>> v4:
> >>> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
> >>>   since it is already queued via mips-next. This eliminates patches
> >>>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
> >>>   Prefix pci_slot_get_pirq() with "piix4_"'.
> >>> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
> >>>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
> >>>   split these patches since I wasn't sure whether renaming a type was allowed.
> >>> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
> >>>   created' for forther cleanup of INTx-to-LNKx route decoupling.
> >>> 
> >>> v3:
> >>> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
> >>>   (Philippe)
> >>> - Make proxy PIC generic (Philippe)
> >>> - Track Malta's PIIX dependencies through KConfig
> >>> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
> >>> - Also rebase onto latest master to resolve merge conflicts. This required
> >>>   copying Philippe's series as first three patches - please ignore.
> >>> 
> >>> v2:
> >>> - Introduce TYPE_ defines for IDE and USB device models (Mark)
> >>> - Omit unexporting of PIIXState (Mark)
> >>> - Improve commit message of patch 5 to mention reset triggering through PCI
> >>>   configuration space (Mark)
> >>> - Move reviewed patches w/o dependencies to the bottom of the series for early
> >>>   upstreaming
> >>> 
> >>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
> >>> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
> >>> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
> >>> 
> >>> Bernhard Beschow (23):
> >>>   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/isa/piix3: Move ISA bus IRQ assignments into host device
> >>>   hw/isa/piix3: Create IDE controller in host device
> >>>   hw/isa/piix3: Wire up ACPI interrupt internally
> >>>   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: Drop the "3" from PIIX base class
> >>>   hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
> >>>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
> >>>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>>   hw/isa/piix4: Create the "intr" property during init() already
> >>>   hw/isa/piix4: Rename reset control operations to match PIIX3
> >>>   hw/isa/piix3: Merge hw/isa/piix4.c
> >>>   hw/isa/piix: Harmonize names of reset control memory regions
> >>>   hw/isa/piix: Rename functions to be shared for interrupt triggering
> >>>   hw/isa/piix: Consolidate IRQ triggering
> >>>   hw/isa/piix: Share PIIX3's base class with PIIX4
> >>>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
> >>> 
> >>>  MAINTAINERS                   |   6 +-
> >>>  include/hw/i386/pc.h          |   2 +-
> >>>  include/hw/southbridge/ich9.h |   2 +
> >>>  include/hw/southbridge/piix.h |  28 +++-
> >>>  hw/i386/pc.c                  |  16 +-
> >>>  hw/i386/pc_piix.c             |  67 ++++----
> >>>  hw/i386/pc_q35.c              |   4 +-
> >>>  hw/isa/lpc_ich9.c             |   8 +
> >>>  hw/isa/{piix3.c => piix.c}    | 306 ++++++++++++++++++++++++++--------
> >>>  hw/isa/piix4.c                | 302 ---------------------------------
> >>>  hw/mips/malta.c               |   6 +-
> >>>  hw/i386/Kconfig               |   3 +-
> >>>  hw/isa/Kconfig                |   8 +-
> >>>  hw/isa/meson.build            |   3 +-
> >>>  hw/mips/Kconfig               |   2 +-
> >>>  15 files changed, 334 insertions(+), 429 deletions(-)
> >>>  rename hw/isa/{piix3.c => piix.c} (55%)
> >>>  delete mode 100644 hw/isa/piix4.c
> >>> 
> >>> -- 
> >>> 2.39.2
> >>> 
> >>

Re: [PATCH v8 00/23] Consolidate PIIX south bridges

[Michael S. Tsirkin]

On Wed, May 10, 2023 at 06:39:49PM +0000, Bernhard Beschow wrote:
> 
> 
> Am 21. April 2023 16:40:47 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
> >
> >
> >Am 21. April 2023 07:15:18 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
> >>On Thu, Mar 02, 2023 at 10:21:38PM +0100, 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.
> >>
> >>Hi!
> >>No freeze is over, could you rebase pls?
> >>I could try to resolve the conflicts but this is so big I'd rather
> >>not take the risk of messing it up.
> >
> >Sure! Since this series is still under discussion I'd wait for the PIIX3 Xen decoupling series to land in master. This will simplify this series a bit by taking Xen out of the equation.
> 
> Could we queue the first two RTC patches already? IMO they're useful general PC cleanups on their own.
> 
> Best regards,
> Bernhard


Could you please post just these two then?
Preferably rebased.

Thanks!

> >
> >Best regards,
> >Bernhard
> >
> >>
> >>> The series is structured as follows:
> >>> 
> >>> Move sub devices into the PIIX3 south bridge, like PIIX4 does already:
> >>> * 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/isa/piix3: Move ISA bus IRQ assignments into host device
> >>> * hw/isa/piix3: Create IDE controller in host device
> >>> * hw/isa/piix3: Wire up ACPI interrupt internally
> >>> 
> >>> Make PIIX3 and PIIX4 south bridges more similar:
> >>> * 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: Drop the "3" from PIIX base class
> >>> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
> >>> * hw/isa/piix4: Remove unused inbound ISA interrupt lines
> >>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>> * hw/isa/piix4: Create the "intr" property during init() already
> >>> * hw/isa/piix4: Rename reset control operations to match PIIX3
> >>> 
> >>> This patch achieves the main goal of the series:
> >>> * hw/isa/piix3: Merge hw/isa/piix4.c
> >>> 
> >>> Perform some further consolidations which were easier to do after the merge:
> >>> * hw/isa/piix: Harmonize names of reset control memory regions
> >>> * hw/isa/piix: Rename functions to be shared for interrupt triggering
> >>> * hw/isa/piix: Consolidate IRQ triggering
> >>> * hw/isa/piix: Share PIIX3's base class with PIIX4
> >>> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
> >>> 
> >>> One challenge was dealing with optional devices where Peter already gave advice
> >>> in [1] which this series implements.
> >>> 
> >>> There are still some differences in the device models:
> >>> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't
> >>> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't
> >>> - Different binary layout in VM state
> >>> 
> >>> v8:
> >>> - Rebase onto master
> >>> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize
> >>>   method in PIIX4' since it changed considerably in v7.
> >>> 
> >>> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the "3"
> >>> from PIIX base class'):
> >>> * `make check`
> >>> * `make check-avocado`
> >>> * 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`
> >>> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda
> >>> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"`
> >>> 
> >>> v7:
> >>> - Rebase onto master
> >>> - Avoid the PIC proxy (Phil)
> >>>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
> >>>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
> >>>   populated already but pc_piix assigned the interrupts only after realizing
> >>>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
> >>>   are already populated during PIIX3's realize phase where the ISA interrupts
> >>>   are wired up.
> >>> - New patches:
> >>>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>>   * hw/isa/piix4: Create the "intr" property during init() already
> >>> - Patches with substantial changes (Reviewed-by dropped):
> >>>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
> >>> 
> >>> v6:
> >>> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
> >>>   within the patch series.
> >>> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
> >>>   bridges" [2] for maintainer convenience.
> >>> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
> >>>   created' into
> >>>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
> >>>   similar for Malta.
> >>> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
> >>>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
> >>> 
> >>> v5:
> >>> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
> >>> - Add patch to make usage of the isa_pic global more type-safe
> >>> - Re-introduce isa-pic as PIC specific proxy (Mark)
> >>> 
> >>> v4:
> >>> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
> >>>   since it is already queued via mips-next. This eliminates patches
> >>>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
> >>>   Prefix pci_slot_get_pirq() with "piix4_"'.
> >>> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
> >>>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
> >>>   split these patches since I wasn't sure whether renaming a type was allowed.
> >>> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
> >>>   created' for forther cleanup of INTx-to-LNKx route decoupling.
> >>> 
> >>> v3:
> >>> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
> >>>   (Philippe)
> >>> - Make proxy PIC generic (Philippe)
> >>> - Track Malta's PIIX dependencies through KConfig
> >>> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
> >>> - Also rebase onto latest master to resolve merge conflicts. This required
> >>>   copying Philippe's series as first three patches - please ignore.
> >>> 
> >>> v2:
> >>> - Introduce TYPE_ defines for IDE and USB device models (Mark)
> >>> - Omit unexporting of PIIXState (Mark)
> >>> - Improve commit message of patch 5 to mention reset triggering through PCI
> >>>   configuration space (Mark)
> >>> - Move reviewed patches w/o dependencies to the bottom of the series for early
> >>>   upstreaming
> >>> 
> >>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
> >>> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
> >>> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
> >>> 
> >>> Bernhard Beschow (23):
> >>>   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/isa/piix3: Move ISA bus IRQ assignments into host device
> >>>   hw/isa/piix3: Create IDE controller in host device
> >>>   hw/isa/piix3: Wire up ACPI interrupt internally
> >>>   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: Drop the "3" from PIIX base class
> >>>   hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
> >>>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
> >>>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>>   hw/isa/piix4: Create the "intr" property during init() already
> >>>   hw/isa/piix4: Rename reset control operations to match PIIX3
> >>>   hw/isa/piix3: Merge hw/isa/piix4.c
> >>>   hw/isa/piix: Harmonize names of reset control memory regions
> >>>   hw/isa/piix: Rename functions to be shared for interrupt triggering
> >>>   hw/isa/piix: Consolidate IRQ triggering
> >>>   hw/isa/piix: Share PIIX3's base class with PIIX4
> >>>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
> >>> 
> >>>  MAINTAINERS                   |   6 +-
> >>>  include/hw/i386/pc.h          |   2 +-
> >>>  include/hw/southbridge/ich9.h |   2 +
> >>>  include/hw/southbridge/piix.h |  28 +++-
> >>>  hw/i386/pc.c                  |  16 +-
> >>>  hw/i386/pc_piix.c             |  67 ++++----
> >>>  hw/i386/pc_q35.c              |   4 +-
> >>>  hw/isa/lpc_ich9.c             |   8 +
> >>>  hw/isa/{piix3.c => piix.c}    | 306 ++++++++++++++++++++++++++--------
> >>>  hw/isa/piix4.c                | 302 ---------------------------------
> >>>  hw/mips/malta.c               |   6 +-
> >>>  hw/i386/Kconfig               |   3 +-
> >>>  hw/isa/Kconfig                |   8 +-
> >>>  hw/isa/meson.build            |   3 +-
> >>>  hw/mips/Kconfig               |   2 +-
> >>>  15 files changed, 334 insertions(+), 429 deletions(-)
> >>>  rename hw/isa/{piix3.c => piix.c} (55%)
> >>>  delete mode 100644 hw/isa/piix4.c
> >>> 
> >>> -- 
> >>> 2.39.2
> >>> 
> >>

Re: [PATCH v8 00/23] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 18. Mai 2023 21:28:12 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>On Wed, May 10, 2023 at 06:39:49PM +0000, Bernhard Beschow wrote:
>> 
>> 
>> Am 21. April 2023 16:40:47 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>> >
>> >
>> >Am 21. April 2023 07:15:18 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>> >>On Thu, Mar 02, 2023 at 10:21:38PM +0100, 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.
>> >>
>> >>Hi!
>> >>No freeze is over, could you rebase pls?
>> >>I could try to resolve the conflicts but this is so big I'd rather
>> >>not take the risk of messing it up.
>> >
>> >Sure! Since this series is still under discussion I'd wait for the PIIX3 Xen decoupling series to land in master. This will simplify this series a bit by taking Xen out of the equation.
>> 
>> Could we queue the first two RTC patches already? IMO they're useful general PC cleanups on their own.
>> 
>> Best regards,
>> Bernhard
>
>
>Could you please post just these two then?
>Preferably rebased.

Both done: https://lore.kernel.org/qemu-devel/20230519084734.220480-1-shentey@gmail.com/

Best regards,
Bernhard

>
>Thanks!
>
>> >
>> >Best regards,
>> >Bernhard
>> >
>> >>
>> >>> The series is structured as follows:
>> >>> 
>> >>> Move sub devices into the PIIX3 south bridge, like PIIX4 does already:
>> >>> * 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/isa/piix3: Move ISA bus IRQ assignments into host device
>> >>> * hw/isa/piix3: Create IDE controller in host device
>> >>> * hw/isa/piix3: Wire up ACPI interrupt internally
>> >>> 
>> >>> Make PIIX3 and PIIX4 south bridges more similar:
>> >>> * 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: Drop the "3" from PIIX base class
>> >>> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>> >>> * hw/isa/piix4: Remove unused inbound ISA interrupt lines
>> >>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>> >>> * hw/isa/piix4: Create the "intr" property during init() already
>> >>> * hw/isa/piix4: Rename reset control operations to match PIIX3
>> >>> 
>> >>> This patch achieves the main goal of the series:
>> >>> * hw/isa/piix3: Merge hw/isa/piix4.c
>> >>> 
>> >>> Perform some further consolidations which were easier to do after the merge:
>> >>> * hw/isa/piix: Harmonize names of reset control memory regions
>> >>> * hw/isa/piix: Rename functions to be shared for interrupt triggering
>> >>> * hw/isa/piix: Consolidate IRQ triggering
>> >>> * hw/isa/piix: Share PIIX3's base class with PIIX4
>> >>> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>> >>> 
>> >>> One challenge was dealing with optional devices where Peter already gave advice
>> >>> in [1] which this series implements.
>> >>> 
>> >>> There are still some differences in the device models:
>> >>> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't
>> >>> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't
>> >>> - Different binary layout in VM state
>> >>> 
>> >>> v8:
>> >>> - Rebase onto master
>> >>> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize
>> >>>   method in PIIX4' since it changed considerably in v7.
>> >>> 
>> >>> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the "3"
>> >>> from PIIX base class'):
>> >>> * `make check`
>> >>> * `make check-avocado`
>> >>> * 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`
>> >>> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda
>> >>> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"`
>> >>> 
>> >>> v7:
>> >>> - Rebase onto master
>> >>> - Avoid the PIC proxy (Phil)
>> >>>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
>> >>>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
>> >>>   populated already but pc_piix assigned the interrupts only after realizing
>> >>>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
>> >>>   are already populated during PIIX3's realize phase where the ISA interrupts
>> >>>   are wired up.
>> >>> - New patches:
>> >>>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>> >>>   * hw/isa/piix4: Create the "intr" property during init() already
>> >>> - Patches with substantial changes (Reviewed-by dropped):
>> >>>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
>> >>> 
>> >>> v6:
>> >>> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
>> >>>   within the patch series.
>> >>> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
>> >>>   bridges" [2] for maintainer convenience.
>> >>> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>> >>>   created' into
>> >>>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
>> >>>   similar for Malta.
>> >>> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
>> >>>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
>> >>> 
>> >>> v5:
>> >>> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
>> >>> - Add patch to make usage of the isa_pic global more type-safe
>> >>> - Re-introduce isa-pic as PIC specific proxy (Mark)
>> >>> 
>> >>> v4:
>> >>> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
>> >>>   since it is already queued via mips-next. This eliminates patches
>> >>>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
>> >>>   Prefix pci_slot_get_pirq() with "piix4_"'.
>> >>> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
>> >>>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
>> >>>   split these patches since I wasn't sure whether renaming a type was allowed.
>> >>> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>> >>>   created' for forther cleanup of INTx-to-LNKx route decoupling.
>> >>> 
>> >>> v3:
>> >>> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
>> >>>   (Philippe)
>> >>> - Make proxy PIC generic (Philippe)
>> >>> - Track Malta's PIIX dependencies through KConfig
>> >>> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
>> >>> - Also rebase onto latest master to resolve merge conflicts. This required
>> >>>   copying Philippe's series as first three patches - please ignore.
>> >>> 
>> >>> v2:
>> >>> - Introduce TYPE_ defines for IDE and USB device models (Mark)
>> >>> - Omit unexporting of PIIXState (Mark)
>> >>> - Improve commit message of patch 5 to mention reset triggering through PCI
>> >>>   configuration space (Mark)
>> >>> - Move reviewed patches w/o dependencies to the bottom of the series for early
>> >>>   upstreaming
>> >>> 
>> >>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>> >>> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
>> >>> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
>> >>> 
>> >>> Bernhard Beschow (23):
>> >>>   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/isa/piix3: Move ISA bus IRQ assignments into host device
>> >>>   hw/isa/piix3: Create IDE controller in host device
>> >>>   hw/isa/piix3: Wire up ACPI interrupt internally
>> >>>   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: Drop the "3" from PIIX base class
>> >>>   hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>> >>>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
>> >>>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
>> >>>   hw/isa/piix4: Create the "intr" property during init() already
>> >>>   hw/isa/piix4: Rename reset control operations to match PIIX3
>> >>>   hw/isa/piix3: Merge hw/isa/piix4.c
>> >>>   hw/isa/piix: Harmonize names of reset control memory regions
>> >>>   hw/isa/piix: Rename functions to be shared for interrupt triggering
>> >>>   hw/isa/piix: Consolidate IRQ triggering
>> >>>   hw/isa/piix: Share PIIX3's base class with PIIX4
>> >>>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>> >>> 
>> >>>  MAINTAINERS                   |   6 +-
>> >>>  include/hw/i386/pc.h          |   2 +-
>> >>>  include/hw/southbridge/ich9.h |   2 +
>> >>>  include/hw/southbridge/piix.h |  28 +++-
>> >>>  hw/i386/pc.c                  |  16 +-
>> >>>  hw/i386/pc_piix.c             |  67 ++++----
>> >>>  hw/i386/pc_q35.c              |   4 +-
>> >>>  hw/isa/lpc_ich9.c             |   8 +
>> >>>  hw/isa/{piix3.c => piix.c}    | 306 ++++++++++++++++++++++++++--------
>> >>>  hw/isa/piix4.c                | 302 ---------------------------------
>> >>>  hw/mips/malta.c               |   6 +-
>> >>>  hw/i386/Kconfig               |   3 +-
>> >>>  hw/isa/Kconfig                |   8 +-
>> >>>  hw/isa/meson.build            |   3 +-
>> >>>  hw/mips/Kconfig               |   2 +-
>> >>>  15 files changed, 334 insertions(+), 429 deletions(-)
>> >>>  rename hw/isa/{piix3.c => piix.c} (55%)
>> >>>  delete mode 100644 hw/isa/piix4.c
>> >>> 
>> >>> -- 
>> >>> 2.39.2
>> >>> 
>> >>
>