[PATCH v6 00/33] Consolidate PIIX south bridges

[PATCH v6 00/33] 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:

These patches are included for compatibility, please ignore:
* hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
* hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
* hw/isa/piix4: Correct IRQRC[A:D] reset values

'Decouple INTx-to-LNKx routing from south bridges', see [2]:
* hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
* hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
* hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific

* hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
* hw/usb/hcd-uhci: Introduce TYPE_ defines for device models

Allow for making both PIIX south bridges agnostic about the virtualization
technology used by allowing to shift the virtualization policies into board
code:
* hw/intc/i8259: Make using the isa_pic singleton more type-safe
* hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC

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: Create TYPE_ISA_PIC in 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: Use TYPE_ISA_PIC device
* hw/isa/piix4: Reuse struct PIIXState from PIIX3
* 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: Reuse PIIX3 base class' realize method in PIIX4
* hw/isa/piix: Rename functions to be shared for interrupt triggering
* hw/isa/piix: Consolidate IRQ triggering
* hw/isa/piix: Share PIIX3's base class with PIIX4

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

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

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

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

Testing done:
* make check
* Boot live CD:
  * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom manjaro-kde-21.3.2-220704-linux515.iso`
  * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom manjaro-kde-21.3.2-220704-linux515.iso`
* '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"`

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 (30):
  hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
  hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
  hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
  hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
  hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
  hw/intc/i8259: Make using the isa_pic singleton more type-safe
  hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
  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: Create TYPE_ISA_PIC in host device
  hw/isa/piix3: Create IDE controller in host device
  hw/isa/piix3: Wire up ACPI interrupt internally
  hw/isa/piix3: 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: Use TYPE_ISA_PIC device
  hw/isa/piix4: Reuse struct PIIXState from PIIX3
  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: Reuse PIIX3 base class' realize method in PIIX4
  hw/isa/piix: Rename functions to be shared for interrupt triggering
  hw/isa/piix: Consolidate IRQ triggering
  hw/isa/piix: Share PIIX3's base class with PIIX4

Philippe Mathieu-Daudé (3):
  hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
  hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
  hw/isa/piix4: Correct IRQRC[A:D] reset values

 configs/devices/mips-softmmu/common.mak |   2 -
 hw/usb/hcd-uhci.h                       |   4 +
 include/hw/i386/ich9.h                  |   2 +
 include/hw/i386/pc.h                    |   2 +-
 include/hw/intc/i8259.h                 |  24 +-
 include/hw/pci/pci.h                    |   3 +-
 include/hw/southbridge/piix.h           |  31 ++-
 include/qemu/typedefs.h                 |   1 +
 hw/i386/pc.c                            |  16 +-
 hw/i386/pc_piix.c                       |  87 ++++---
 hw/i386/pc_q35.c                        |  20 +-
 hw/intc/i8259.c                         |  38 ++-
 hw/isa/lpc_ich9.c                       |   8 +
 hw/isa/{piix3.c => piix.c}              | 289 +++++++++++++++------
 hw/isa/piix4.c                          | 327 ------------------------
 hw/mips/malta.c                         |  65 ++++-
 hw/pci-host/raven.c                     |   3 +-
 hw/pci-host/versatile.c                 |   3 +-
 hw/pci/pci.c                            |  12 +-
 hw/remote/machine.c                     |   3 +-
 hw/usb/hcd-uhci.c                       |  16 +-
 MAINTAINERS                             |   6 +-
 hw/i386/Kconfig                         |   4 +-
 hw/isa/Kconfig                          |   8 +-
 hw/isa/meson.build                      |   3 +-
 hw/mips/Kconfig                         |   2 +
 26 files changed, 479 insertions(+), 500 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (57%)
 delete mode 100644 hw/isa/piix4.c

-- 
2.39.0

[PATCH v6 01/33] hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition

[Bernhard Beschow]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

The PIIX4 PCI-ISA bridge function is always located at 10:0.
Since we want to re-use its address, add the PIIX4_PCI_DEVFN
definition.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221027204720.33611-2-philmd@linaro.org>
---
 hw/mips/malta.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index c0a2e0ab04..9bffa1b128 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -71,6 +71,8 @@
 
 #define FLASH_SIZE          0x400000
 
+#define PIIX4_PCI_DEVFN     PCI_DEVFN(10, 0)
+
 typedef struct {
     MemoryRegion iomem;
     MemoryRegion iomem_lo; /* 0 - 0x900 */
@@ -1401,7 +1403,7 @@ void mips_malta_init(MachineState *machine)
     empty_slot_init("GT64120", 0, 0x20000000);
 
     /* Southbridge */
-    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
+    piix4 = pci_create_simple_multifunction(pci_bus, PIIX4_PCI_DEVFN, true,
                                             TYPE_PIIX4_PCI_DEVICE);
     isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
 
-- 
2.39.0

[PATCH v6 02/33] hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader

[Bernhard Beschow]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

Linux kernel expects the northbridge & southbridge chipsets
configured by the BIOS firmware. We emulate that by writing
a tiny bootloader code in write_bootloader().

Upon introduction in commit 5c2b87e34d ("PIIX4 support"),
the PIIX4 configuration space included values specific to
the Malta board.

Set the Malta-specific IRQ routing values in the embedded
bootloader, so the next commit can remove the Malta specific
bits from the PIIX4 PCI-ISA bridge and make it generic
(matching the real hardware).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221027204720.33611-3-philmd@linaro.org>
---
 hw/mips/malta.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9bffa1b128..c3dcd43f37 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -803,6 +803,8 @@ static void write_bootloader_nanomips(uint8_t *base, uint64_t run_addr,
     stw_p(p++, 0x8422); stw_p(p++, 0x9088);
                                 /* sw t0, 0x88(t1)              */
 
+    /* TODO set PIIX IRQC[A:D] routing values! */
+
     stw_p(p++, 0xe320 | NM_HI1(kernel_entry));
 
     stw_p(p++, NM_HI2(kernel_entry));
@@ -840,6 +842,9 @@ static void write_bootloader_nanomips(uint8_t *base, uint64_t run_addr,
 static void write_bootloader(uint8_t *base, uint64_t run_addr,
                              uint64_t kernel_entry)
 {
+    const char pci_pins_cfg[PCI_NUM_PINS] = {
+        10, 10, 11, 11 /* PIIX IRQRC[A:D] */
+    };
     uint32_t *p;
 
     /* Small bootloader */
@@ -914,6 +919,20 @@ static void write_bootloader(uint8_t *base, uint64_t run_addr,
 
 #undef cpu_to_gt32
 
+    /*
+     * The PIIX ISA bridge is on PCI bus 0 dev 10 func 0.
+     * Load the PIIX IRQC[A:D] routing config address, then
+     * write routing configuration to the config data register.
+     */
+    bl_gen_write_u32(&p, /* GT_PCI0_CFGADDR */
+                     cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcf8),
+                     tswap32((1 << 31) /* ConfigEn */
+                             | PCI_BUILD_BDF(0, PIIX4_PCI_DEVFN) << 8
+                             | PIIX_PIRQCA));
+    bl_gen_write_u32(&p, /* GT_PCI0_CFGDATA */
+                     cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcfc),
+                     tswap32(ldl_be_p(pci_pins_cfg)));
+
     bl_gen_jump_kernel(&p,
                        true, ENVP_VADDR - 64,
                        /*
-- 
2.39.0

[PATCH v6 03/33] hw/isa/piix4: Correct IRQRC[A:D] reset values

[Bernhard Beschow]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

IRQRC[A:D] registers reset value is 0x80. We were forcing
the MIPS Malta machine routing to be able to boot a Linux
kernel without any bootloader.
We now have these registers initialized in the Malta machine
write_bootloader(), so we can use the correct reset values.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221027204720.33611-4-philmd@linaro.org>
---
 hw/isa/piix4.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 8fc1db6dc9..0d23e11a39 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -116,10 +116,10 @@ static void piix4_isa_reset(DeviceState *dev)
     pci_conf[0x4c] = 0x4d;
     pci_conf[0x4e] = 0x03;
     pci_conf[0x4f] = 0x00;
-    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
-    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
-    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
-    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
+    pci_conf[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;
-- 
2.39.0

[PATCH v6 04/33] hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()

[Bernhard Beschow]

pci_bus_irqs() coupled together the assignment of pci_set_irq_fn and
pci_map_irq_fn to a PCI bus. This coupling gets in the way when the
pci_map_irq_fn is board-specific while the pci_set_irq_fn is device-
specific.

For example, both of QEMU's PIIX south bridge models have different
pci_map_irq_fn implementations which are board-specific rather than
device-specific. These implementations should therefore reside in board
code. The pci_set_irq_fn's, however, should stay in the device models
because they access memory internal to the model.

Factoring out pci_bus_map_irqs() from pci_bus_irqs() allows the
assignments to be decoupled, resolving the problem described above.

Note also how pci_vpb_realize() which gets touched in this commit
assigns different pci_map_irq_fn's depending on the board.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/pci/pci.h    |  3 ++-
 hw/i386/pc_q35.c        |  4 ++--
 hw/isa/piix3.c          |  8 ++++----
 hw/isa/piix4.c          |  3 ++-
 hw/pci-host/raven.c     |  3 ++-
 hw/pci-host/versatile.c |  3 ++-
 hw/pci/pci.c            | 12 +++++++++---
 hw/remote/machine.c     |  3 ++-
 8 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 7048a373d1..85ee458cd2 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -282,8 +282,9 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
                          MemoryRegion *address_space_io,
                          uint8_t devfn_min, const char *typename);
 void pci_root_bus_cleanup(PCIBus *bus);
-void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
                   void *irq_opaque, int nirq);
+void pci_bus_map_irqs(PCIBus *bus, pci_map_irq_fn map_irq);
 void pci_bus_irqs_cleanup(PCIBus *bus);
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 67ceb04bcc..65ea226211 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -268,8 +268,8 @@ static void pc_q35_init(MachineState *machine)
     for (i = 0; i < GSI_NUM_PINS; i++) {
         qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
     }
-    pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc_map_irq, ich9_lpc,
-                 ICH9_LPC_NB_PIRQS);
+    pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc, ICH9_LPC_NB_PIRQS);
+    pci_bus_map_irqs(host_bus, ich9_lpc_map_irq);
     pci_bus_set_route_irq_fn(host_bus, ich9_route_intx_pin_to_irq);
     isa_bus = ich9_lpc->isa_bus;
 
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index eabad7ba58..666e794f77 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -384,8 +384,8 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
-                 piix3, PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, piix3_set_irq, piix3, PIIX_NUM_PIRQS);
+    pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
     pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
 }
 
@@ -420,8 +420,8 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
      * connected to the IOAPIC directly.
      * These additional routes can be discovered through ACPI.
      */
-    pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
-                 piix3, XEN_PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
+    pci_bus_map_irqs(pci_bus, xen_pci_slot_get_pirq);
 }
 
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0d23e11a39..9c79c9677b 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -271,7 +271,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     }
     qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
 
-    pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+    pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
 }
 
 static void piix4_init(Object *obj)
diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index 2c96ddf8fe..5b00b4e462 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -258,7 +258,8 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 
     qdev_init_gpio_in(d, raven_change_gpio, 1);
 
-    pci_bus_irqs(&s->pci_bus, raven_set_irq, raven_map_irq, s, PCI_NUM_PINS);
+    pci_bus_irqs(&s->pci_bus, raven_set_irq, s, PCI_NUM_PINS);
+    pci_bus_map_irqs(&s->pci_bus, raven_map_irq);
 
     memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, s,
                           "pci-conf-idx", 4);
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 0d50ea4cc0..60d4e7cd92 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -422,7 +422,8 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
         mapfn = pci_vpb_map_irq;
     }
 
-    pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4);
+    pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, s->irq, 4);
+    pci_bus_map_irqs(&s->pci_bus, mapfn);
 
     /* Our memory regions are:
      * 0 : our control registers
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index c2fb88f9a3..39a7bb32aa 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -280,6 +280,7 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
     PCIBus *bus;
     for (;;) {
         bus = pci_get_bus(pci_dev);
+        assert(bus->map_irq);
         irq_num = bus->map_irq(pci_dev, irq_num);
         if (bus->set_irq)
             break;
@@ -518,16 +519,20 @@ void pci_root_bus_cleanup(PCIBus *bus)
     qbus_unrealize(BUS(bus));
 }
 
-void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
                   void *irq_opaque, int nirq)
 {
     bus->set_irq = set_irq;
-    bus->map_irq = map_irq;
     bus->irq_opaque = irq_opaque;
     bus->nirq = nirq;
     bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
 }
 
+void pci_bus_map_irqs(PCIBus *bus, pci_map_irq_fn map_irq)
+{
+    bus->map_irq = map_irq;
+}
+
 void pci_bus_irqs_cleanup(PCIBus *bus)
 {
     bus->set_irq = NULL;
@@ -549,7 +554,8 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
 
     bus = pci_root_bus_new(parent, name, address_space_mem,
                            address_space_io, devfn_min, typename);
-    pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
+    pci_bus_irqs(bus, set_irq, irq_opaque, nirq);
+    pci_bus_map_irqs(bus, map_irq);
     return bus;
 }
 
diff --git a/hw/remote/machine.c b/hw/remote/machine.c
index 75d550daae..519f855ec1 100644
--- a/hw/remote/machine.c
+++ b/hw/remote/machine.c
@@ -63,8 +63,9 @@ static void remote_machine_init(MachineState *machine)
     } else {
         remote_iohub_init(&s->iohub);
 
-        pci_bus_irqs(pci_host->bus, remote_iohub_set_irq, remote_iohub_map_irq,
+        pci_bus_irqs(pci_host->bus, remote_iohub_set_irq,
                      &s->iohub, REMOTE_IOHUB_NB_PIRQS);
+        pci_bus_map_irqs(pci_host->bus, remote_iohub_map_irq);
     }
 
     qbus_set_hotplug_handler(BUS(pci_host->bus), OBJECT(s));
-- 
2.39.0

[PATCH v6 05/33] hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific

[Bernhard Beschow]

pci_map_irq_fn's in general seem to be board-specific. So move PIIX3's
pci_slot_get_pirq() to board code to not have PIIX3 make assuptions
about its board.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 15 +++++++++++++++
 hw/isa/piix3.c    | 13 -------------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b48047f50c..bb3b10557f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -47,6 +47,7 @@
 #include "hw/sysbus.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"
@@ -73,6 +74,17 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 #endif
 
+/*
+ * Return the global irq number corresponding to a given device irq
+ * pin. We could also use the bus number to have a more precise mapping.
+ */
+static int pc_pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
+{
+    int slot_addend;
+    slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
+    return (pci_intx + slot_addend) & 3;
+}
+
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      const char *host_type, const char *pci_type)
@@ -216,6 +228,9 @@ static void pc_init1(MachineState *machine,
                               x86ms->below_4g_mem_size,
                               x86ms->above_4g_mem_size,
                               pci_memory, ram_memory);
+        pci_bus_map_irqs(pci_bus,
+                         xen_enabled() ? xen_pci_slot_get_pirq
+                                       : pc_pci_slot_get_pirq);
         pcms->bus = pci_bus;
 
         pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 666e794f77..283b971ec4 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -79,17 +79,6 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
     piix3_set_irq_level(piix3, pirq, level);
 }
 
-/*
- * Return the global irq number corresponding to a given device irq
- * pin. We could also use the bus number to have a more precise mapping.
- */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
-{
-    int slot_addend;
-    slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
-    return (pci_intx + slot_addend) & 3;
-}
-
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
     PIIX3State *piix3 = opaque;
@@ -385,7 +374,6 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
     }
 
     pci_bus_irqs(pci_bus, piix3_set_irq, piix3, PIIX_NUM_PIRQS);
-    pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
     pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
 }
 
@@ -421,7 +409,6 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
      * These additional routes can be discovered through ACPI.
      */
     pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
-    pci_bus_map_irqs(pci_bus, xen_pci_slot_get_pirq);
 }
 
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
-- 
2.39.0

[PATCH v6 06/33] hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific

[Bernhard Beschow]

pci_map_irq_fn's in general seem to be board-specific, and PIIX4's
pci_slot_get_pirq() in particular seems very Malta-specific. So move the
latter to malta.c to 1/ keep the board logic in one place and 2/ avoid
PIIX4 to make assumptions about its board.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/isa/piix4.c  | 26 --------------------------
 hw/mips/malta.c | 27 +++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9c79c9677b..6e9434129d 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -79,31 +79,6 @@ static void piix4_set_irq(void *opaque, int irq_num, int level)
     }
 }
 
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
-{
-    int slot;
-
-    slot = PCI_SLOT(pci_dev->devfn);
-
-    switch (slot) {
-    /* PIIX4 USB */
-    case 10:
-        return 3;
-    /* AMD 79C973 Ethernet */
-    case 11:
-        return 1;
-    /* Crystal 4281 Sound */
-    case 12:
-        return 2;
-    /* PCI slot 1 to 4 */
-    case 18 ... 21:
-        return ((slot - 18) + irq_num) & 0x03;
-    /* Unknown device, don't do any translation */
-    default:
-        return irq_num;
-    }
-}
-
 static void piix4_isa_reset(DeviceState *dev)
 {
     PIIX4State *d = PIIX4_PCI_DEVICE(dev);
@@ -272,7 +247,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
 
     pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
-    pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
 }
 
 static void piix4_init(Object *obj)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index c3dcd43f37..94d4c49bf5 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -39,6 +39,7 @@
 #include "hw/mips/bootloader.h"
 #include "hw/mips/cpudevs.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
 #include "qemu/log.h"
 #include "hw/mips/bios.h"
 #include "hw/ide/pci.h"
@@ -1161,6 +1162,31 @@ static void malta_mips_config(MIPSCPU *cpu)
     }
 }
 
+static int malta_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+    int slot;
+
+    slot = PCI_SLOT(pci_dev->devfn);
+
+    switch (slot) {
+    /* PIIX4 USB */
+    case 10:
+        return 3;
+    /* AMD 79C973 Ethernet */
+    case 11:
+        return 1;
+    /* Crystal 4281 Sound */
+    case 12:
+        return 2;
+    /* PCI slot 1 to 4 */
+    case 18 ... 21:
+        return ((slot - 18) + irq_num) & 0x03;
+    /* Unknown device, don't do any translation */
+    default:
+        return irq_num;
+    }
+}
+
 static void main_cpu_reset(void *opaque)
 {
     MIPSCPU *cpu = opaque;
@@ -1414,6 +1440,7 @@ void mips_malta_init(MachineState *machine)
     /* Northbridge */
     dev = sysbus_create_simple("gt64120", -1, NULL);
     pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci"));
+    pci_bus_map_irqs(pci_bus, malta_pci_slot_get_pirq);
     /*
      * The whole address space decoded by the GT-64120A doesn't generate
      * exception when accessing invalid memory. Create an empty slot to
-- 
2.39.0

[PATCH v6 07/33] hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig

[Bernhard Beschow]

Tracking dependencies via Kconfig seems much cleaner.

Note that PIIX4 already depends on ACPI_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>
---
 configs/devices/mips-softmmu/common.mak | 2 --
 hw/mips/Kconfig                         | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
index 88aff94625..8ed6b62ae7 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -17,9 +17,7 @@ CONFIG_I8254=y
 CONFIG_PCSPK=y
 CONFIG_PCKBD=y
 CONFIG_FDC=y
-CONFIG_ACPI_PIIX4=y
 CONFIG_I8257=y
-CONFIG_PIIX4=y
 CONFIG_IDE_ISA=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_I8259=y
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index 725525358d..4e7042f03d 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,6 +1,7 @@
 config MALTA
     bool
     select ISA_SUPERIO
+    select PIIX4
 
 config MIPSSIM
     bool
-- 
2.39.0

[PATCH v6 08/33] hw/usb/hcd-uhci: Introduce TYPE_ defines for device models

[Bernhard Beschow]

Suggested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
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-10-shentey@gmail.com>
---
 hw/usb/hcd-uhci.h |  4 ++++
 hw/i386/pc_piix.c |  3 ++-
 hw/i386/pc_q35.c  | 13 +++++++------
 hw/isa/piix4.c    |  2 +-
 hw/usb/hcd-uhci.c | 16 ++++++++--------
 5 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/usb/hcd-uhci.h b/hw/usb/hcd-uhci.h
index 5843af504a..e0fdb98ef1 100644
--- a/hw/usb/hcd-uhci.h
+++ b/hw/usb/hcd-uhci.h
@@ -91,4 +91,8 @@ typedef struct UHCIInfo {
 void uhci_data_class_init(ObjectClass *klass, void *data);
 void usb_uhci_common_realize(PCIDevice *dev, Error **errp);
 
+#define TYPE_PIIX3_USB_UHCI "piix3-usb-uhci"
+#define TYPE_PIIX4_USB_UHCI "piix4-usb-uhci"
+#define TYPE_ICH9_USB_UHCI(fn) "ich9-usb-uhci" #fn
+
 #endif
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bb3b10557f..df64dd8dcc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -51,6 +51,7 @@
 #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"
@@ -305,7 +306,7 @@ static void pc_init1(MachineState *machine,
 #endif
 
     if (pcmc->pci_enabled && machine_usb(machine)) {
-        pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
+        pci_create_simple(pci_bus, piix3_devfn + 2, TYPE_PIIX3_USB_UHCI);
     }
 
     if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 65ea226211..83c57c6eb1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -48,6 +48,7 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/usb.h"
+#include "hw/usb/hcd-uhci.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "sysemu/numa.h"
@@ -65,15 +66,15 @@ struct ehci_companions {
 };
 
 static const struct ehci_companions ich9_1d[] = {
-    { .name = "ich9-usb-uhci1", .func = 0, .port = 0 },
-    { .name = "ich9-usb-uhci2", .func = 1, .port = 2 },
-    { .name = "ich9-usb-uhci3", .func = 2, .port = 4 },
+    { .name = TYPE_ICH9_USB_UHCI(1), .func = 0, .port = 0 },
+    { .name = TYPE_ICH9_USB_UHCI(2), .func = 1, .port = 2 },
+    { .name = TYPE_ICH9_USB_UHCI(3), .func = 2, .port = 4 },
 };
 
 static const struct ehci_companions ich9_1a[] = {
-    { .name = "ich9-usb-uhci4", .func = 0, .port = 0 },
-    { .name = "ich9-usb-uhci5", .func = 1, .port = 2 },
-    { .name = "ich9-usb-uhci6", .func = 2, .port = 4 },
+    { .name = TYPE_ICH9_USB_UHCI(4), .func = 0, .port = 0 },
+    { .name = TYPE_ICH9_USB_UHCI(5), .func = 1, .port = 2 },
+    { .name = TYPE_ICH9_USB_UHCI(6), .func = 2, .port = 4 },
 };
 
 static int ehci_create_ich9_with_companions(PCIBus *bus, int slot)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 6e9434129d..de60ceef73 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -255,7 +255,7 @@ 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, "piix4-usb-uhci");
+    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);
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index d1b5657d72..30ae0104bb 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1292,56 +1292,56 @@ void uhci_data_class_init(ObjectClass *klass, void *data)
 
 static UHCIInfo uhci_info[] = {
     {
-        .name       = "piix3-usb-uhci",
+        .name      = TYPE_PIIX3_USB_UHCI,
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82371SB_2,
         .revision  = 0x01,
         .irq_pin   = 3,
         .unplug    = true,
     },{
-        .name      = "piix4-usb-uhci",
+        .name      = TYPE_PIIX4_USB_UHCI,
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82371AB_2,
         .revision  = 0x01,
         .irq_pin   = 3,
         .unplug    = true,
     },{
-        .name      = "ich9-usb-uhci1", /* 00:1d.0 */
+        .name      = TYPE_ICH9_USB_UHCI(1), /* 00:1d.0 */
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1,
         .revision  = 0x03,
         .irq_pin   = 0,
         .unplug    = false,
     },{
-        .name      = "ich9-usb-uhci2", /* 00:1d.1 */
+        .name      = TYPE_ICH9_USB_UHCI(2), /* 00:1d.1 */
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2,
         .revision  = 0x03,
         .irq_pin   = 1,
         .unplug    = false,
     },{
-        .name      = "ich9-usb-uhci3", /* 00:1d.2 */
+        .name      = TYPE_ICH9_USB_UHCI(3), /* 00:1d.2 */
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3,
         .revision  = 0x03,
         .irq_pin   = 2,
         .unplug    = false,
     },{
-        .name      = "ich9-usb-uhci4", /* 00:1a.0 */
+        .name      = TYPE_ICH9_USB_UHCI(4), /* 00:1a.0 */
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI4,
         .revision  = 0x03,
         .irq_pin   = 0,
         .unplug    = false,
     },{
-        .name      = "ich9-usb-uhci5", /* 00:1a.1 */
+        .name      = TYPE_ICH9_USB_UHCI(5), /* 00:1a.1 */
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI5,
         .revision  = 0x03,
         .irq_pin   = 1,
         .unplug    = false,
     },{
-        .name      = "ich9-usb-uhci6", /* 00:1a.2 */
+        .name      = TYPE_ICH9_USB_UHCI(6), /* 00:1a.2 */
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI6,
         .revision  = 0x03,
-- 
2.39.0

[PATCH v6 09/33] hw/intc/i8259: Make using the isa_pic singleton more type-safe

[Bernhard Beschow]

This even spares some casts in hot code paths along the way.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
Note: The next patch will introduce a class "isa-pic", which is
shall not be confused with the isa_pic singleton.
---
 include/hw/intc/i8259.h |  6 +++---
 include/qemu/typedefs.h |  1 +
 hw/intc/i8259.c         | 11 ++++-------
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/include/hw/intc/i8259.h b/include/hw/intc/i8259.h
index e2b1e8c59a..a0e34dd990 100644
--- a/include/hw/intc/i8259.h
+++ b/include/hw/intc/i8259.h
@@ -3,10 +3,10 @@
 
 /* i8259.c */
 
-extern DeviceState *isa_pic;
+extern PICCommonState *isa_pic;
 qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
 qemu_irq *kvm_i8259_init(ISABus *bus);
-int pic_get_output(DeviceState *d);
-int pic_read_irq(DeviceState *d);
+int pic_get_output(PICCommonState *s);
+int pic_read_irq(PICCommonState *s);
 
 #endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 073abab998..fba04875c2 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -99,6 +99,7 @@ typedef struct PCIExpressDevice PCIExpressDevice;
 typedef struct PCIExpressHost PCIExpressHost;
 typedef struct PCIHostDeviceAddress PCIHostDeviceAddress;
 typedef struct PCIHostState PCIHostState;
+typedef struct PICCommonState PICCommonState;
 typedef struct PostcopyDiscardState PostcopyDiscardState;
 typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index cc4e21ffec..0261f087b2 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -55,7 +55,7 @@ struct PICClass {
 #ifdef DEBUG_IRQ_LATENCY
 static int64_t irq_time[16];
 #endif
-DeviceState *isa_pic;
+PICCommonState *isa_pic;
 static PICCommonState *slave_pic;
 
 /* return the highest priority found in mask (highest = smallest
@@ -173,9 +173,8 @@ static void pic_intack(PICCommonState *s, int irq)
     pic_update_irq(s);
 }
 
-int pic_read_irq(DeviceState *d)
+int pic_read_irq(PICCommonState *s)
 {
-    PICCommonState *s = PIC_COMMON(d);
     int irq, intno;
 
     irq = pic_get_irq(s);
@@ -354,10 +353,8 @@ static uint64_t pic_ioport_read(void *opaque, hwaddr addr,
     return ret;
 }
 
-int pic_get_output(DeviceState *d)
+int pic_get_output(PICCommonState *s)
 {
-    PICCommonState *s = PIC_COMMON(d);
-
     return (pic_get_irq(s) >= 0);
 }
 
@@ -426,7 +423,7 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
         irq_set[i] = qdev_get_gpio_in(dev, i);
     }
 
-    isa_pic = dev;
+    isa_pic = PIC_COMMON(dev);
 
     isadev = i8259_init_chip(TYPE_I8259, bus, false);
     dev = DEVICE(isadev);
-- 
2.39.0

[PATCH v6 10/33] hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC

[Bernhard Beschow]

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

Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 include/hw/intc/i8259.h | 18 ++++++++++++++++++
 hw/intc/i8259.c         | 27 +++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/include/hw/intc/i8259.h b/include/hw/intc/i8259.h
index a0e34dd990..7fb403971c 100644
--- a/include/hw/intc/i8259.h
+++ b/include/hw/intc/i8259.h
@@ -1,6 +1,24 @@
 #ifndef HW_I8259_H
 #define HW_I8259_H
 
+#include "qom/object.h"
+#include "hw/isa/isa.h"
+#include "qemu/typedefs.h"
+
+#define TYPE_ISA_PIC "isa-pic"
+OBJECT_DECLARE_SIMPLE_TYPE(ISAPICState, ISA_PIC)
+
+/*
+ * TYPE_ISA_PIC is currently a PIC proxy which allows for interrupt wiring in
+ * a virtualization technology agnostic way.
+ */
+struct ISAPICState {
+    DeviceState parent_obj;
+
+    qemu_irq in_irqs[ISA_NUM_IRQS];
+    qemu_irq out_irqs[ISA_NUM_IRQS];
+};
+
 /* i8259.c */
 
 extern PICCommonState *isa_pic;
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index 0261f087b2..e99d02136d 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -455,9 +455,36 @@ static const TypeInfo i8259_info = {
     .class_size = sizeof(PICClass),
 };
 
+static void isapic_set_irq(void *opaque, int irq, int level)
+{
+    ISAPICState *s = opaque;
+
+    qemu_set_irq(s->out_irqs[irq], level);
+}
+
+static void isapic_init(Object *obj)
+{
+    ISAPICState *s = ISA_PIC(obj);
+
+    qdev_init_gpio_in(DEVICE(s), isapic_set_irq, ISA_NUM_IRQS);
+    qdev_init_gpio_out(DEVICE(s), s->out_irqs, ISA_NUM_IRQS);
+
+    for (int i = 0; i < ISA_NUM_IRQS; ++i) {
+        s->in_irqs[i] = qdev_get_gpio_in(DEVICE(s), i);
+    }
+}
+
+static const TypeInfo isapic_info = {
+    .name          = TYPE_ISA_PIC,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(ISAPICState),
+    .instance_init = isapic_init,
+};
+
 static void pic_register_types(void)
 {
     type_register_static(&i8259_info);
+    type_register_static(&isapic_info);
 }
 
 type_init(pic_register_types)
-- 
2.39.0

[PATCH v6 11/33] 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/i386/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              |  1 +
 hw/isa/lpc_ich9.c             |  8 ++++++++
 hw/isa/piix3.c                | 15 +++++++++++++++
 hw/isa/Kconfig                |  2 ++
 8 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 222781e8b9..dab309d5e3 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -7,6 +7,7 @@
 #include "hw/isa/apm.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
+#include "hw/rtc/mc146818rtc.h"
 #include "qom/object.h"
 
 void ich9_lpc_set_irq(void *opaque, int irq_num, int level);
@@ -35,6 +36,7 @@ struct ICH9LPCState {
     */
     uint8_t irr[PCI_SLOT_MAX][PCI_NUM_PINS];
 
+    RTCState rtc;
     APMState apm;
     ICH9LPCPMRegs pm;
     uint32_t sci_level; /* track sci level */
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0bf48e936d..b06d26fa11 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];
 
+    RTCState rtc;
+
     /* Reset Control Register contents */
     uint8_t rcr;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d489ecc0d1..448557333b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1304,7 +1304,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
         rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
     }
-    *rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
+
+    if (rtc_irq) {
+        qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+    } else {
+        uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+                                                "irq",
+                                                &error_fatal);
+        isa_connect_gpio_out(*rtc_state, 0, irq);
+    }
+    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+                              "date");
 
     qemu_register_boot_set(pc_boot_set, *rtc_state);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df64dd8dcc..8f894714e5 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"
@@ -239,10 +240,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 83c57c6eb1..da97df69f7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -239,6 +239,7 @@ static void pc_q35_init(MachineState *machine)
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
                                           ICH9_LPC_FUNC), true,
                                           TYPE_ICH9_LPC_DEVICE);
+    rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc"));
 
     object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
                              TYPE_HOTPLUG_HANDLER,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 8d541e2b54..498175c1cc 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -663,6 +663,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,
@@ -728,6 +730,12 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     isa_bus_irqs(isa_bus, lpc->gsi);
 
     i8257_dma_init(isa_bus, 0);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&lpc->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&lpc->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
 }
 
 static bool ich9_rst_cnt_needed(void *opaque)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 283b971ec4..e8ddb6a602 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)
@@ -327,6 +334,13 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     }
 }
 
+static void pci_piix3_init(Object *obj)
+{
+    PIIX3State *d = PIIX3_PCI_DEVICE(obj);
+
+    object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+}
+
 static void pci_piix3_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -353,6 +367,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.0

[PATCH v6 12/33] 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 991f905f5d..dd059e8667 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -169,7 +169,7 @@ uint64_t pc_pci_hole64_start(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
+                          ISADevice *rtc_state,
                           bool create_fdctrl,
                           uint32_t hpet_irqs);
 void pc_cmos_init(PCMachineState *pcms,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 448557333b..53a5443e09 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1251,7 +1251,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)
 {
@@ -1306,17 +1306,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 8f894714e5..a577ea2f4e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -276,7 +276,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 da97df69f7..58c51fbd9e 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);
 
     /* connect pm stuff to lpc */
-- 
2.39.0

[PATCH v6 13/33] 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 a577ea2f4e..37afc01d30 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -235,7 +235,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.0

[PATCH v6 14/33] 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 b06d26fa11..762709f2fd 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];
 
     RTCState rtc;
+    UHCIState uhci;
 
     /* Reset Control Register contents */
     uint8_t rcr;
 
     /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
     MemoryRegion rcr_mem;
+
+    bool has_usb;
 };
 typedef struct PIIXState PIIX3State;
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 37afc01d30..61d8152078 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -52,7 +52,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"
@@ -236,6 +235,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;
@@ -314,10 +315,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 e8ddb6a602..45c20dea17 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)
@@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj)
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
 }
 
+static Property pci_piix3_props[] = {
+    DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void pci_piix3_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -360,6 +376,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.0

[PATCH v6 15/33] 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 762709f2fd..b1eaab1d95 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 {
 
     RTCState rtc;
     UHCIState uhci;
+    PIIX4PMState pm;
+
+    uint32_t smb_io_base;
 
     /* Reset Control Register contents */
     uint8_t rcr;
@@ -63,7 +67,9 @@ struct PIIXState {
     /* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
     MemoryRegion rcr_mem;
 
+    bool has_acpi;
     bool has_usb;
+    bool smm_enabled;
 };
 typedef struct PIIXState PIIX3State;
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 61d8152078..5ea8d4a585 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -46,12 +46,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"
@@ -97,6 +97,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;
@@ -237,15 +238,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);
 
@@ -315,15 +326,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);
@@ -337,7 +341,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 45c20dea17..ed7d58bc98 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)
@@ -353,7 +364,10 @@ static void pci_piix3_init(Object *obj)
 }
 
 static Property pci_piix3_props[] = {
+    DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0),
+    DEFINE_PROP_BOOL("has-acpi", PIIX3State, has_acpi, true),
     DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
+    DEFINE_PROP_BOOL("smm-enabled", PIIX3State, smm_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/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.0

[PATCH v6 16/33] hw/isa/piix3: Create TYPE_ISA_PIC in host device

[Bernhard Beschow]

The newly introduced TYPE_ISA_PIC allows for wiring up devices
in the southbridge where the virtualization technology used
(KVM, TCG, Xen) is not yet known.

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index b1eaab1d95..514ccdb7fd 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/intc/i8259.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
 
@@ -50,11 +51,10 @@ struct PIIXState {
 #endif
     uint64_t pic_levels;
 
-    qemu_irq *pic;
-
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
+    ISAPICState pic;
     RTCState rtc;
     UHCIState uhci;
     PIIX4PMState pm;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5ea8d4a585..df8e2e389b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -219,10 +219,11 @@ static void pc_init1(MachineState *machine,
     gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
 
     if (pcmc->pci_enabled) {
-        PIIX3State *piix3;
+        DeviceState *dev;
         PCIDevice *pci_dev;
         const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
                                          : TYPE_PIIX3_DEVICE;
+        int i;
 
         pci_bus = i440fx_init(pci_type,
                               i440fx_host,
@@ -247,10 +248,12 @@ static void pc_init1(MachineState *machine,
                                  &error_abort);
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
-        piix3 = PIIX3_PCI_DEVICE(pci_dev);
-        piix3->pic = x86ms->gsi;
-        piix3_devfn = piix3->dev.devfn;
-        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+        dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "pic"));
+        for (i = 0; i < ISA_NUM_IRQS; i++) {
+            qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
+        }
+        piix3_devfn = pci_dev->devfn;
+        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
         rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
                                                              "rtc"));
         piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
@@ -259,6 +262,7 @@ static void pc_init1(MachineState *machine,
         piix4_pm = NULL;
         isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
                               &error_abort);
+        isa_bus_irqs(isa_bus, x86ms->gsi);
 
         rtc_state = isa_new(TYPE_MC146818_RTC);
         qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
@@ -267,7 +271,6 @@ static void pc_init1(MachineState *machine,
         i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
     }
-    isa_bus_irqs(isa_bus, x86ms->gsi);
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
         pc_i8259_create(isa_bus, gsi_state->i8259_irq);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index ed7d58bc98..88a6bf28ea 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -39,7 +39,7 @@
 
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
-    qemu_set_irq(piix3->pic[pic_irq],
+    qemu_set_irq(piix3->pic.in_irqs[pic_irq],
                  !!(piix3->pic_levels &
                     (((1ULL << PIIX_NUM_PIRQS) - 1) <<
                      (pic_irq * PIIX_NUM_PIRQS))));
@@ -297,6 +297,13 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
+    /* PIC */
+    if (!qdev_realize(DEVICE(&d->pic), NULL, errp)) {
+        return;
+    }
+
+    isa_bus_irqs(isa_bus, d->pic.in_irqs);
+
     memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
                           "piix3-reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
@@ -360,6 +367,7 @@ static void pci_piix3_init(Object *obj)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(obj);
 
+    object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
 }
 
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index c4fb5b49bd..1291f59896 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -71,6 +71,7 @@ config I440FX
     select ACPI_PIIX4
     select PC_PCI
     select PC_ACPI
+    select I8259
     select PCI_I440FX
     select PIIX3
     select IDE_PIIX
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index cf79580384..fccf095d07 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -35,6 +35,7 @@ config PIIX3
     bool
     select ACPI_PIIX4
     select I8257
+    select I8259
     select ISA_BUS
     select MC146818RTC
     select USB_UHCI
-- 
2.39.0

[PATCH v6 17/33] 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             | 15 ++++++---------
 hw/isa/piix3.c                |  8 ++++++++
 hw/i386/Kconfig               |  1 -
 hw/isa/Kconfig                |  1 +
 5 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 514ccdb7fd..e3c35ca16f 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/intc/i8259.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
@@ -56,6 +57,7 @@ struct PIIXState {
 
     ISAPICState pic;
     RTCState rtc;
+    PCIIDEState ide;
     UHCIState uhci;
     PIIX4PMState pm;
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df8e2e389b..4675981021 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -41,7 +41,6 @@
 #include "hw/usb.h"
 #include "net/net.h"
 #include "hw/ide/pci.h"
-#include "hw/ide/piix.h"
 #include "hw/irq.h"
 #include "sysemu/kvm.h"
 #include "hw/kvm/clock.h"
@@ -98,7 +97,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];
@@ -252,11 +250,14 @@ static void pc_init1(MachineState *machine,
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
         }
-        piix3_devfn = pci_dev->devfn;
         isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
         rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
                                                              "rtc"));
         piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
+        dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
+        pci_ide_create_devs(PCI_DEVICE(dev));
+        idebus[0] = qdev_get_child_bus(dev, "ide.0");
+        idebus[1] = qdev_get_child_bus(dev, "ide.1");
     } else {
         pci_bus = NULL;
         piix4_pm = NULL;
@@ -270,6 +271,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) {
@@ -298,12 +301,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 88a6bf28ea..a549b1a8a5 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"
@@ -317,6 +318,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
+    /* IDE */
+    qdev_prop_set_int32(DEVICE(&d->ide), "addr", dev->devfn + 1);
+    if (!qdev_realize(DEVICE(&d->ide), BUS(pci_bus), errp)) {
+        return;
+    }
+
     /* USB */
     if (d->has_usb) {
         object_initialize_child(OBJECT(dev), "uhci", &d->uhci,
@@ -369,6 +376,7 @@ static void pci_piix3_init(Object *obj)
 
     object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+    object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
 }
 
 static Property pci_piix3_props[] = {
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 1291f59896..7e53dc8f82 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -74,7 +74,6 @@ config I440FX
     select I8259
     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 fccf095d07..b911306909 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -36,6 +36,7 @@ config PIIX3
     select ACPI_PIIX4
     select I8257
     select I8259
+    select IDE_PIIX
     select ISA_BUS
     select MC146818RTC
     select USB_UHCI
-- 
2.39.0

[PATCH v6 18/33] 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    | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4675981021..92d9dd1ae2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -329,7 +329,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 a549b1a8a5..6d2ffd449c 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -343,6 +343,8 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
         if (!qdev_realize(DEVICE(&d->pm), BUS(pci_bus), errp)) {
             return;
         }
+        qdev_connect_gpio_out(DEVICE(&d->pm), 0,
+                              qdev_get_gpio_in(DEVICE(&d->pic), 9));
     }
 }
 
-- 
2.39.0

[PATCH v6 19/33] 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 e3c35ca16f..f48cfd7936 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -32,7 +32,6 @@
  */
 #define PIIX_RCR_IOPORT 0xcf9
 
-#define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
 #define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
 
 struct PIIXState {
@@ -44,10 +43,10 @@ struct PIIXState {
      * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
      *
      * PIRQ is mapped to PIC pins, we track it by
-     * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
+     * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
      * pic_irq * PIIX_NUM_PIRQS + pirq
      */
-#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
+#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
 #error "unable to encode pic state in 64bit in pic_levels."
 #endif
     uint64_t pic_levels;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 6d2ffd449c..e813e20639 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.0

[PATCH v6 20/33] 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 e813e20639..c6659bbfda 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -381,7 +381,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),
@@ -408,7 +408,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.0

[PATCH v6 21/33] 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 c6659bbfda..d674130fc9 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;
@@ -395,7 +395,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.0

[PATCH v6 22/33] 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/isa/piix3.c                | 60 +++++++++++++++++------------------
 2 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index f48cfd7936..907c3568b6 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/isa/piix3.c b/hw/isa/piix3.c
index d674130fc9..4ce1653406 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.in_irqs[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;
 
@@ -374,7 +374,7 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
 
 static void pci_piix3_init(Object *obj)
 {
-    PIIX3State *d = PIIX3_PCI_DEVICE(obj);
+    PIIXState *d = PIIX_PCI_DEVICE(obj);
 
     object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
@@ -382,10 +382,10 @@ static void pci_piix3_init(Object *obj)
 }
 
 static Property pci_piix_props[] = {
-    DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0),
-    DEFINE_PROP_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(),
 };
 
@@ -412,10 +412,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,
@@ -429,7 +429,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);
@@ -451,14 +451,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);
@@ -485,13 +485,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.0

[PATCH v6 23/33] 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 de60ceef73..de4133f573 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 94d4c49bf5..4a2e0edd98 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1449,8 +1449,10 @@ void mips_malta_init(MachineState *machine)
     empty_slot_init("GT64120", 0, 0x20000000);
 
     /* 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.0

[PATCH v6 24/33] 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 de4133f573..9edaa5de3e 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.0

[PATCH v6 25/33] hw/isa/piix4: Use TYPE_ISA_PIC device

[Bernhard Beschow]

Aligns the code with PIIX3 such that PIIXState can be used in PIIX4,
too.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-33-shentey@gmail.com>
---
 hw/isa/piix4.c  | 28 ++++++++++------------------
 hw/mips/malta.c | 11 +++++++++--
 hw/mips/Kconfig |  1 +
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9edaa5de3e..eae4db0182 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -44,9 +44,8 @@
 
 struct PIIX4State {
     PCIDevice dev;
-    qemu_irq cpu_intr;
-    qemu_irq *isa;
 
+    ISAPICState pic;
     RTCState rtc;
     PCIIDEState ide;
     UHCIState uhci;
@@ -82,7 +81,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int level)
                 pic_level |= pci_bus_get_irq_level(bus, i);
             }
         }
-        qemu_set_irq(s->isa[pic_irq], pic_level);
+        qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
     }
 }
 
@@ -149,12 +148,6 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
-static void piix4_request_i8259_irq(void *opaque, int irq, int level)
-{
-    PIIX4State *s = opaque;
-    qemu_set_irq(s->cpu_intr, level);
-}
-
 static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {
@@ -190,7 +183,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     PIIX4State *s = PIIX4_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
-    qemu_irq *i8259_out_irq;
 
     isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
                           pci_address_space_io(dev), errp);
@@ -198,20 +190,18 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    qdev_init_gpio_out_named(DEVICE(dev), &s->cpu_intr,
-                             "intr", 1);
-
     memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
                           "reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &s->rcr_mem, 1);
 
     /* initialize i8259 pic */
-    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    s->isa = i8259_init(isa_bus, *i8259_out_irq);
+    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
+        return;
+    }
 
     /* initialize ISA irqs */
-    isa_bus_irqs(isa_bus, s->isa);
+    isa_bus_irqs(isa_bus, s->pic.in_irqs);
 
     /* initialize pit */
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
@@ -224,7 +214,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
         return;
     }
-    s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq);
+    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
 
     /* IDE */
     qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
@@ -251,7 +241,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
             return;
         }
-        qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
+        qdev_connect_gpio_out(DEVICE(&s->pm), 0,
+                              qdev_get_gpio_in(DEVICE(&s->pic), 9));
     }
 
     pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
@@ -261,6 +252,7 @@ static void piix4_init(Object *obj)
 {
     PIIX4State *s = PIIX4_PCI_DEVICE(obj);
 
+    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
 }
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 4a2e0edd98..55f3e0c54a 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -29,6 +29,7 @@
 #include "qemu/guest-random.h"
 #include "hw/clock.h"
 #include "hw/southbridge/piix.h"
+#include "hw/intc/i8259.h"
 #include "hw/isa/superio.h"
 #include "hw/char/serial.h"
 #include "net/net.h"
@@ -1280,10 +1281,11 @@ void mips_malta_init(MachineState *machine)
     PCIBus *pci_bus;
     ISABus *isa_bus;
     qemu_irq cbus_irq, i8259_irq;
+    qemu_irq *i8259;
     I2CBus *smbus;
     DriveInfo *dinfo;
     int fl_idx = 0;
-    int be;
+    int be, i;
     MaltaState *s;
     PCIDevice *piix4;
     DeviceState *dev;
@@ -1459,7 +1461,12 @@ void mips_malta_init(MachineState *machine)
     pci_ide_create_devs(PCI_DEVICE(dev));
 
     /* Interrupt controller */
-    qdev_connect_gpio_out_named(DEVICE(piix4), "intr", 0, i8259_irq);
+    dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pic"));
+    i8259 = i8259_init(isa_bus, i8259_irq);
+    for (i = 0; i < ISA_NUM_IRQS; i++) {
+        qdev_connect_gpio_out(dev, i, i8259[i]);
+    }
+    g_free(i8259);
 
     /* generate SPD EEPROM data */
     dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pm"));
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index 4e7042f03d..d156de812c 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,5 +1,6 @@
 config MALTA
     bool
+    select I8259
     select ISA_SUPERIO
     select PIIX4
 
-- 
2.39.0

[PATCH v6 26/33] hw/isa/piix4: Reuse struct PIIXState from PIIX3

[Bernhard Beschow]

Now that PIIX4 also uses TYPE_ISA_PIC, both implementations
can share the same struct.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-34-shentey@gmail.com>
---
 hw/isa/piix4.c | 51 +++++++++++++++-----------------------------------
 1 file changed, 15 insertions(+), 36 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index eae4db0182..ce88377630 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -42,32 +42,10 @@
 #include "sysemu/runstate.h"
 #include "qom/object.h"
 
-struct PIIX4State {
-    PCIDevice dev;
-
-    ISAPICState pic;
-    RTCState rtc;
-    PCIIDEState ide;
-    UHCIState uhci;
-    PIIX4PMState pm;
-
-    uint32_t smb_io_base;
-
-    /* Reset Control Register */
-    MemoryRegion rcr_mem;
-    uint8_t rcr;
-
-    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 */
@@ -87,7 +65,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int 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
@@ -122,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;
@@ -142,8 +121,8 @@ 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()
     }
 };
@@ -151,7 +130,7 @@ static const VMStateDescription vmstate_piix4 = {
 static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
 
     if (val & 4) {
         qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
@@ -163,7 +142,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;
 }
@@ -180,7 +159,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;
 
@@ -250,7 +229,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 
 static void piix4_init(Object *obj)
 {
-    PIIX4State *s = PIIX4_PCI_DEVICE(obj);
+    PIIXState *s = PIIX_PCI_DEVICE(obj);
 
     object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
@@ -258,10 +237,10 @@ static void piix4_init(Object *obj)
 }
 
 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(),
 };
 
@@ -289,7 +268,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.0

[PATCH v6 27/33] 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 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index ce88377630..de7cde192f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -127,7 +127,7 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
-static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
+static void rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {
     PIIXState *s = opaque;
@@ -140,16 +140,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,
@@ -169,7 +169,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.0

[PATCH v6 28/33] 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>
---
 hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
 hw/isa/piix4.c             | 285 -------------------------------------
 MAINTAINERS                |   6 +-
 hw/i386/Kconfig            |   2 +-
 hw/isa/Kconfig             |  12 +-
 hw/isa/meson.build         |   3 +-
 hw/mips/Kconfig            |   2 +-
 7 files changed, 165 insertions(+), 303 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (75%)
 delete mode 100644 hw/isa/piix4.c

diff --git a/hw/isa/piix3.c b/hw/isa/piix.c
similarity index 75%
rename from hw/isa/piix3.c
rename to hw/isa/piix.c
index 4ce1653406..81cb4e64ab 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,6 +28,7 @@
 #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"
@@ -81,6 +83,27 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
     piix3_set_irq_level(piix3, pirq, level);
 }
 
+static void piix4_set_irq(void *opaque, int irq_num, int level)
+{
+    int i, pic_irq, pic_level;
+    PIIXState *s = opaque;
+    PCIBus *bus = pci_get_bus(&s->dev);
+
+    /* now we change the pic irq level according to the piix irq mappings */
+    /* XXX: optimize */
+    pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
+    if (pic_irq < ISA_NUM_IRQS) {
+        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
+        pic_level = 0;
+        for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+            if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
+                pic_level |= pci_bus_get_irq_level(bus, i);
+            }
+        }
+        qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
+    }
+}
+
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
     PIIXState *piix3 = opaque;
@@ -208,6 +231,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 +291,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)
 {
@@ -489,11 +534,124 @@ static const TypeInfo piix3_xen_info = {
     .class_init    = piix3_xen_class_init,
 };
 
+static void piix4_realize(PCIDevice *dev, Error **errp)
+{
+    PIIXState *s = PIIX_PCI_DEVICE(dev);
+    PCIBus *pci_bus = pci_get_bus(dev);
+    ISABus *isa_bus;
+
+    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
+                          pci_address_space_io(dev), errp);
+    if (!isa_bus) {
+        return;
+    }
+
+    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
+                          "reset-control", 1);
+    memory_region_add_subregion_overlap(pci_address_space_io(dev),
+                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
+
+    /* initialize i8259 pic */
+    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
+        return;
+    }
+
+    /* initialize ISA irqs */
+    isa_bus_irqs(isa_bus, s->pic.in_irqs);
+
+    /* initialize pit */
+    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+
+    /* DMA */
+    i8257_dma_init(isa_bus, 0);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
+    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
+
+    /* IDE */
+    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
+    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
+        return;
+    }
+
+    /* USB */
+    if (s->has_usb) {
+        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
+                                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,
+                              qdev_get_gpio_in(DEVICE(&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);
+
+    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
+    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
+    object_initialize_child(obj, "ide", &s->ide, 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 de7cde192f..0000000000
--- a/hw/isa/piix4.c
+++ /dev/null
@@ -1,285 +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.in_irqs[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 rcr_write(void *opaque, hwaddr addr, uint64_t val,
-                            unsigned int len)
-{
-    PIIXState *s = opaque;
-
-    if (val & 4) {
-        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
-        return;
-    }
-
-    s->rcr = val & 2; /* keep System Reset type only */
-}
-
-static uint64_t rcr_read(void *opaque, hwaddr addr, unsigned int len)
-{
-    PIIXState *s = opaque;
-
-    return s->rcr;
-}
-
-static const MemoryRegionOps rcr_ops = {
-    .read = rcr_read,
-    .write = rcr_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    },
-};
-
-static void piix4_realize(PCIDevice *dev, Error **errp)
-{
-    PIIXState *s = PIIX_PCI_DEVICE(dev);
-    PCIBus *pci_bus = pci_get_bus(dev);
-    ISABus *isa_bus;
-
-    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
-                          pci_address_space_io(dev), errp);
-    if (!isa_bus) {
-        return;
-    }
-
-    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
-                          "reset-control", 1);
-    memory_region_add_subregion_overlap(pci_address_space_io(dev),
-                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
-
-    /* initialize i8259 pic */
-    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
-        return;
-    }
-
-    /* initialize ISA irqs */
-    isa_bus_irqs(isa_bus, s->pic.in_irqs);
-
-    /* initialize pit */
-    i8254_pit_init(isa_bus, 0x40, 0, NULL);
-
-    /* DMA */
-    i8257_dma_init(isa_bus, 0);
-
-    /* RTC */
-    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
-    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
-        return;
-    }
-    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
-
-    /* IDE */
-    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
-    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
-        return;
-    }
-
-    /* USB */
-    if (s->has_usb) {
-        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
-                                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,
-                              qdev_get_gpio_in(DEVICE(&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);
-
-    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
-    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
-    object_initialize_child(obj, "ide", &s->ide, 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/MAINTAINERS b/MAINTAINERS
index 7a40d4d865..569042c3a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1236,7 +1236,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/mips/gt64xxx_pci.c
@@ -1654,7 +1654,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
@@ -2344,7 +2344,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/i386/Kconfig b/hw/i386/Kconfig
index 7e53dc8f82..5e04e07758 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -73,7 +73,7 @@ config I440FX
     select PC_ACPI
     select I8259
     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 b911306909..040a18c070 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -31,17 +31,7 @@ config PC87312
     select FDC_ISA
     select IDE_ISA
 
-config PIIX3
-    bool
-    select ACPI_PIIX4
-    select I8257
-    select I8259
-    select IDE_PIIX
-    select ISA_BUS
-    select MC146818RTC
-    select USB_UHCI
-
-config PIIX4
+config PIIX
     bool
     # For historical reasons, SuperIO devices are created in the board
     # for PIIX4.
diff --git a/hw/isa/meson.build b/hw/isa/meson.build
index 8bf678ca0a..314bbd0860 100644
--- a/hw/isa/meson.build
+++ b/hw/isa/meson.build
@@ -3,8 +3,7 @@ softmmu_ss.add(when: 'CONFIG_I82378', if_true: files('i82378.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
 softmmu_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
-softmmu_ss.add(when: 'CONFIG_PIIX3', if_true: files('piix3.c'))
-softmmu_ss.add(when: 'CONFIG_PIIX4', if_true: files('piix4.c'))
+softmmu_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
 softmmu_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
 softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
 
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index d156de812c..5b16ff4ed2 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -2,7 +2,7 @@ config MALTA
     bool
     select I8259
     select ISA_SUPERIO
-    select PIIX4
+    select PIIX
 
 config MIPSSIM
     bool
-- 
2.39.0

[PATCH v6 29/33] 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 81cb4e64ab..d8cd80e859 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -351,7 +351,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     isa_bus_irqs(isa_bus, d->pic.in_irqs);
 
     memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
-                          "piix3-reset-control", 1);
+                          "piix-reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
@@ -547,7 +547,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.0

[PATCH v6 30/33] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4

[Bernhard Beschow]

Resolves duplicate code.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221022150508.26830-39-shentey@gmail.com>
---
 hw/isa/piix.c | 65 +++++++--------------------------------------------
 1 file changed, 9 insertions(+), 56 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index d8cd80e859..0132f6e70a 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -331,7 +331,8 @@ static const MemoryRegionOps rcr_ops = {
     },
 };
 
-static void pci_piix3_realize(PCIDevice *dev, Error **errp)
+static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
+                             Error **errp)
 {
     PIIXState *d = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
@@ -371,8 +372,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;
@@ -477,7 +477,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
     PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
-    pci_piix3_realize(dev, errp);
+    pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
     if (*errp) {
         return;
     }
@@ -506,7 +506,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
     PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
-    pci_piix3_realize(dev, errp);
+    pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
     if (*errp) {
         return;
     }
@@ -536,71 +536,24 @@ static const TypeInfo piix3_xen_info = {
 
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
+    ERRP_GUARD();
     PIIXState *s = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
 
-    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
-                          pci_address_space_io(dev), errp);
-    if (!isa_bus) {
-        return;
-    }
-
-    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
-                          "piix-reset-control", 1);
-    memory_region_add_subregion_overlap(pci_address_space_io(dev),
-                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
-
-    /* initialize i8259 pic */
-    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
+    pci_piix_realize(dev, TYPE_PIIX4_USB_UHCI, errp);
+    if (*errp) {
         return;
     }
 
-    /* initialize ISA irqs */
-    isa_bus_irqs(isa_bus, s->pic.in_irqs);
+    isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(dev), "isa.0"));
 
     /* initialize pit */
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
 
-    /* DMA */
-    i8257_dma_init(isa_bus, 0);
-
     /* RTC */
-    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
-    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
-        return;
-    }
     s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
 
-    /* IDE */
-    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
-    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
-        return;
-    }
-
-    /* USB */
-    if (s->has_usb) {
-        object_initialize_child(OBJECT(dev), "uhci", &s->uhci,
-                                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,
-                              qdev_get_gpio_in(DEVICE(&s->pic), 9));
-    }
-
     pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
 }
 
-- 
2.39.0

[PATCH v6 31/33] 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 0132f6e70a..33ea5275ec 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -40,47 +40,47 @@
 
 #define XEN_PIIX_NUM_PIRQS      128ULL
 
-static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
+static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
 {
-    qemu_set_irq(piix3->pic.in_irqs[pic_irq],
-                 !!(piix3->pic_levels &
+    qemu_set_irq(piix->pic.in_irqs[pic_irq],
+                 !!(piix->pic_levels &
                     (((1ULL << PIIX_NUM_PIRQS) - 1) <<
                      (pic_irq * PIIX_NUM_PIRQS))));
 }
 
-static void piix3_set_irq_level_internal(PIIXState *piix3, int pirq, int level)
+static void piix_set_irq_level_internal(PIIXState *piix, int pirq, int level)
 {
     int pic_irq;
     uint64_t mask;
 
-    pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+    pic_irq = piix->dev.config[PIIX_PIRQCA + pirq];
     if (pic_irq >= ISA_NUM_IRQS) {
         return;
     }
 
     mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq);
-    piix3->pic_levels &= ~mask;
-    piix3->pic_levels |= mask * !!level;
+    piix->pic_levels &= ~mask;
+    piix->pic_levels |= mask * !!level;
 }
 
-static void piix3_set_irq_level(PIIXState *piix3, int pirq, int level)
+static void piix_set_irq_level(PIIXState *piix, int pirq, int level)
 {
     int pic_irq;
 
-    pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+    pic_irq = piix->dev.config[PIIX_PIRQCA + pirq];
     if (pic_irq >= ISA_NUM_IRQS) {
         return;
     }
 
-    piix3_set_irq_level_internal(piix3, pirq, level);
+    piix_set_irq_level_internal(piix, pirq, level);
 
-    piix3_set_irq_pic(piix3, pic_irq);
+    piix_set_irq_pic(piix, pic_irq);
 }
 
-static void piix3_set_irq(void *opaque, int pirq, int level)
+static void piix_set_irq(void *opaque, int pirq, int level)
 {
-    PIIXState *piix3 = opaque;
-    piix3_set_irq_level(piix3, pirq, level);
+    PIIXState *piix = opaque;
+    piix_set_irq_level(piix, pirq, level);
 }
 
 static void piix4_set_irq(void *opaque, int irq_num, int level)
@@ -121,29 +121,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);
         }
     }
 }
@@ -165,7 +165,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)
@@ -225,7 +225,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;
@@ -482,7 +482,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);
 }
 
@@ -490,7 +490,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.0

[PATCH v6 32/33] 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 33ea5275ec..f125a6175f 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -83,27 +83,6 @@ static void piix_set_irq(void *opaque, int pirq, int level)
     piix_set_irq_level(piix, pirq, level);
 }
 
-static void piix4_set_irq(void *opaque, int irq_num, int level)
-{
-    int i, pic_irq, pic_level;
-    PIIXState *s = opaque;
-    PCIBus *bus = pci_get_bus(&s->dev);
-
-    /* now we change the pic irq level according to the piix irq mappings */
-    /* XXX: optimize */
-    pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
-    if (pic_irq < ISA_NUM_IRQS) {
-        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
-        pic_level = 0;
-        for (i = 0; i < PIIX_NUM_PIRQS; i++) {
-            if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
-                pic_level |= pci_bus_get_irq_level(bus, i);
-            }
-        }
-        qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
-    }
-}
-
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
     PIIXState *piix3 = opaque;
@@ -239,7 +218,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)
@@ -554,7 +533,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     /* RTC */
     s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
 
-    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, piix_set_irq, s, PIIX_NUM_PIRQS);
 }
 
 static void piix4_init(Object *obj)
@@ -571,6 +550,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.0

[PATCH v6 33/33] 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 | 49 ++++++++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index f125a6175f..54a1246a9d 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -396,13 +396,12 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     }
 }
 
-static void pci_piix3_init(Object *obj)
+static void pci_piix_init(Object *obj)
 {
     PIIXState *d = PIIX_PCI_DEVICE(obj);
 
     object_initialize_child(obj, "pic", &d->pic, TYPE_ISA_PIC);
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
-    object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
 }
 
 static Property pci_piix_props[] = {
@@ -413,7 +412,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);
@@ -421,11 +420,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
@@ -440,9 +436,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 },
@@ -465,17 +461,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,
 };
 
@@ -501,15 +509,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,
 };
 
@@ -540,8 +553,6 @@ static void piix4_init(Object *obj)
 {
     PIIXState *s = PIIX_PCI_DEVICE(obj);
 
-    object_initialize_child(obj, "pic", &s->pic, TYPE_ISA_PIC);
-    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
 }
 
@@ -552,31 +563,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.0

Re: [PATCH v6 25/33] hw/isa/piix4: Use TYPE_ISA_PIC device

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> Aligns the code with PIIX3 such that PIIXState can be used in PIIX4,
> too.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Message-Id: <20221022150508.26830-33-shentey@gmail.com>
> ---
>   hw/isa/piix4.c  | 28 ++++++++++------------------
>   hw/mips/malta.c | 11 +++++++++--
>   hw/mips/Kconfig |  1 +
>   3 files changed, 20 insertions(+), 20 deletions(-)

> @@ -1459,7 +1461,12 @@ void mips_malta_init(MachineState *machine)
>       pci_ide_create_devs(PCI_DEVICE(dev));
>   
>       /* Interrupt controller */
> -    qdev_connect_gpio_out_named(DEVICE(piix4), "intr", 0, i8259_irq);
> +    dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pic"));
> +    i8259 = i8259_init(isa_bus, i8259_irq);
> +    for (i = 0; i < ISA_NUM_IRQS; i++) {
> +        qdev_connect_gpio_out(dev, i, i8259[i]);
> +    }
> +    g_free(i8259);
>   
>       /* generate SPD EEPROM data */
>       dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pm"));
> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> index 4e7042f03d..d156de812c 100644
> --- a/hw/mips/Kconfig
> +++ b/hw/mips/Kconfig
> @@ -1,5 +1,6 @@
>   config MALTA
>       bool
> +    select I8259
>       select ISA_SUPERIO
>       select PIIX4

The PIIX4 owns / exposes the I8259, so we don't need to select it here.
The Malta board only initializes it. Why did you have to add this?

Re: [PATCH v6 25/33] hw/isa/piix4: Use TYPE_ISA_PIC device

[Bernhard Beschow]

Am 11. Januar 2023 17:08:28 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 9/1/23 18:23, Bernhard Beschow wrote:
>> Aligns the code with PIIX3 such that PIIXState can be used in PIIX4,
>> too.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>> Message-Id: <20221022150508.26830-33-shentey@gmail.com>
>> ---
>>   hw/isa/piix4.c  | 28 ++++++++++------------------
>>   hw/mips/malta.c | 11 +++++++++--
>>   hw/mips/Kconfig |  1 +
>>   3 files changed, 20 insertions(+), 20 deletions(-)
>
>> @@ -1459,7 +1461,12 @@ void mips_malta_init(MachineState *machine)
>>       pci_ide_create_devs(PCI_DEVICE(dev));
>>         /* Interrupt controller */
>> -    qdev_connect_gpio_out_named(DEVICE(piix4), "intr", 0, i8259_irq);
>> +    dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pic"));
>> +    i8259 = i8259_init(isa_bus, i8259_irq);
>> +    for (i = 0; i < ISA_NUM_IRQS; i++) {
>> +        qdev_connect_gpio_out(dev, i, i8259[i]);
>> +    }
>> +    g_free(i8259);
>>         /* generate SPD EEPROM data */
>>       dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pm"));
>> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>> index 4e7042f03d..d156de812c 100644
>> --- a/hw/mips/Kconfig
>> +++ b/hw/mips/Kconfig
>> @@ -1,5 +1,6 @@
>>   config MALTA
>>       bool
>> +    select I8259
>>       select ISA_SUPERIO
>>       select PIIX4
>
>The PIIX4 owns / exposes the I8259, so we don't need to select it here.
>The Malta board only initializes it. Why did you have to add this?

The Malta board instantiates a real I8259 while PIIX4 instantiates the TYPE_ISA_PIC proxy. Both are implemented pragmatically in the same source file, so both Malta and PIIX4 select it for different reasons.

In previous iterations using the TYPE_PROXY_PIC this might have been clearer: PIIX4 would select PROXY_PIC (and not I8259) while Malta would select I8259.

OK?

Best regards,
Bernhard

Re: [PATCH v6 09/33] hw/intc/i8259: Make using the isa_pic singleton more type-safe

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> This even spares some casts in hot code paths along the way.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> Note: The next patch will introduce a class "isa-pic", which is
> shall not be confused with the isa_pic singleton.
> ---
>   include/hw/intc/i8259.h |  6 +++---
>   include/qemu/typedefs.h |  1 +
>   hw/intc/i8259.c         | 11 ++++-------
>   3 files changed, 8 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 10/33] hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> Having an i8259 proxy allows for ISA PICs to be created and wired up in
> southbridges. This is especially interesting for PIIX3 for two reasons:
> First, the southbridge doesn't need to care about the virtualization
> technology used (KVM, TCG, Xen) due to in-IRQs (where devices get
> attached) and out-IRQs (which will trigger the IRQs of the respective
> virtualization technology) are separated. Second, since the in-IRQs are
> populated with fully initialized qemu_irq's, they can already be wired
> up inside PIIX3.
> 
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   include/hw/intc/i8259.h | 18 ++++++++++++++++++
>   hw/intc/i8259.c         | 27 +++++++++++++++++++++++++++
>   2 files changed, 45 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 13/33] hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> 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(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 14/33] hw/isa/piix3: Create USB controller in host device

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> The USB controller is an integral part of PIIX3 (function 2). So create
> it as part of the south bridge.
> 
> Note that the USB function is optional in QEMU. This is why it gets
> object_initialize_child()'ed in realize rather than in instance_init.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> 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(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 15/33] hw/isa/piix3: Create power management controller in host device

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> 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(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 22/33] hw/isa/piix3: Drop the "3" from PIIX base class

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> 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/isa/piix3.c                | 60 +++++++++++++++++------------------
>   2 files changed, 32 insertions(+), 34 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> 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>
> ---
>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>   hw/isa/piix4.c             | 285 -------------------------------------
>   MAINTAINERS                |   6 +-
>   hw/i386/Kconfig            |   2 +-
>   hw/isa/Kconfig             |  12 +-
>   hw/isa/meson.build         |   3 +-
>   hw/mips/Kconfig            |   2 +-
>   7 files changed, 165 insertions(+), 303 deletions(-)
>   rename hw/isa/{piix3.c => piix.c} (75%)
>   delete mode 100644 hw/isa/piix4.c

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 30/33] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> Resolves duplicate code.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Message-Id: <20221022150508.26830-39-shentey@gmail.com>
> ---
>   hw/isa/piix.c | 65 +++++++--------------------------------------------
>   1 file changed, 9 insertions(+), 56 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Philippe Mathieu-Daudé]

On 12/1/23 13:50, Philippe Mathieu-Daudé wrote:
> On 9/1/23 18:23, Bernhard Beschow wrote:
>> 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>
>> ---
>>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>>   hw/isa/piix4.c             | 285 -------------------------------------
>>   MAINTAINERS                |   6 +-
>>   hw/i386/Kconfig            |   2 +-
>>   hw/isa/Kconfig             |  12 +-
>>   hw/isa/meson.build         |   3 +-
>>   hw/mips/Kconfig            |   2 +-
>>   7 files changed, 165 insertions(+), 303 deletions(-)
>>   rename hw/isa/{piix3.c => piix.c} (75%)
>>   delete mode 100644 hw/isa/piix4.c
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

BTW I wonder why PIIX4 isn't calling pci_bus_set_route_irq_fn()...
Any clue?

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> 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>
> ---
>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>   hw/isa/piix4.c             | 285 -------------------------------------
>   MAINTAINERS                |   6 +-
>   hw/i386/Kconfig            |   2 +-
>   hw/isa/Kconfig             |  12 +-
>   hw/isa/meson.build         |   3 +-
>   hw/mips/Kconfig            |   2 +-
>   7 files changed, 165 insertions(+), 303 deletions(-)
>   rename hw/isa/{piix3.c => piix.c} (75%)
>   delete mode 100644 hw/isa/piix4.c

> +static void piix4_realize(PCIDevice *dev, Error **errp)
> +{
> +    PIIXState *s = PIIX_PCI_DEVICE(dev);
> +    PCIBus *pci_bus = pci_get_bus(dev);
> +    ISABus *isa_bus;
> +
> +    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
> +                          pci_address_space_io(dev), errp);
> +    if (!isa_bus) {
> +        return;
> +    }
> +
> +    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
> +                          "reset-control", 1);
> +    memory_region_add_subregion_overlap(pci_address_space_io(dev),
> +                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
> +
> +    /* initialize i8259 pic */
> +    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
> +        return;
> +    }
> +
> +    /* initialize ISA irqs */
> +    isa_bus_irqs(isa_bus, s->pic.in_irqs);
> +
> +    /* initialize pit */
> +    i8254_pit_init(isa_bus, 0x40, 0, NULL);
> +
> +    /* DMA */
> +    i8257_dma_init(isa_bus, 0);
> +
> +    /* RTC */
> +    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
> +    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
> +        return;
> +    }
> +    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);

Pre-existing; it seems this difference with PIIX3 can be removed
because already taken care by calling isa_connect_gpio_out() in 
mc146818_rtc_init()?

ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq 
intercept_irq)
{
     DeviceState *dev;
     ISADevice *isadev;
     RTCState *s;

     isadev = isa_new(TYPE_MC146818_RTC);
     dev = DEVICE(isadev);
     s = MC146818_RTC(isadev);
     qdev_prop_set_int32(dev, "base_year", base_year);
     isa_realize_and_unref(isadev, bus, &error_fatal);
     if (intercept_irq) {
         qdev_connect_gpio_out(dev, 0, intercept_irq);
     } else {
         isa_connect_gpio_out(isadev, 0, s->isairq);

Having:

void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq)
{
     qemu_irq irq = isa_get_irq(isadev, isairq);
     qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq);
}

What do you think?

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Philippe Mathieu-Daudé]

On 12/1/23 16:04, Philippe Mathieu-Daudé wrote:
> On 9/1/23 18:23, Bernhard Beschow wrote:
>> 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>
>> ---
>>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>>   hw/isa/piix4.c             | 285 -------------------------------------
>>   MAINTAINERS                |   6 +-
>>   hw/i386/Kconfig            |   2 +-
>>   hw/isa/Kconfig             |  12 +-
>>   hw/isa/meson.build         |   3 +-
>>   hw/mips/Kconfig            |   2 +-
>>   7 files changed, 165 insertions(+), 303 deletions(-)
>>   rename hw/isa/{piix3.c => piix.c} (75%)
>>   delete mode 100644 hw/isa/piix4.c
> 
>> +static void piix4_realize(PCIDevice *dev, Error **errp)
>> +{
>> +    PIIXState *s = PIIX_PCI_DEVICE(dev);
>> +    PCIBus *pci_bus = pci_get_bus(dev);
>> +    ISABus *isa_bus;
>> +
>> +    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
>> +                          pci_address_space_io(dev), errp);
>> +    if (!isa_bus) {
>> +        return;
>> +    }
>> +
>> +    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
>> +                          "reset-control", 1);
>> +    memory_region_add_subregion_overlap(pci_address_space_io(dev),
>> +                                        PIIX_RCR_IOPORT, &s->rcr_mem, 
>> 1);
>> +
>> +    /* initialize i8259 pic */
>> +    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
>> +        return;
>> +    }
>> +
>> +    /* initialize ISA irqs */
>> +    isa_bus_irqs(isa_bus, s->pic.in_irqs);
>> +
>> +    /* initialize pit */
>> +    i8254_pit_init(isa_bus, 0x40, 0, NULL);
>> +
>> +    /* DMA */
>> +    i8257_dma_init(isa_bus, 0);
>> +
>> +    /* RTC */
>> +    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
>> +    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
>> +        return;
>> +    }
>> +    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
> 
> Pre-existing; it seems this difference with PIIX3 can be removed
> because already taken care by calling isa_connect_gpio_out() in 
> mc146818_rtc_init()?
> 
> ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq 
> intercept_irq)
> {
>      DeviceState *dev;
>      ISADevice *isadev;
>      RTCState *s;
> 
>      isadev = isa_new(TYPE_MC146818_RTC);
>      dev = DEVICE(isadev);
>      s = MC146818_RTC(isadev);
>      qdev_prop_set_int32(dev, "base_year", base_year);
>      isa_realize_and_unref(isadev, bus, &error_fatal);
>      if (intercept_irq) {
>          qdev_connect_gpio_out(dev, 0, intercept_irq);
>      } else {
>          isa_connect_gpio_out(isadev, 0, s->isairq);
> 

I meant to paste:

static void rtc_realizefn(DeviceState *dev, Error **errp)
{
     ...
     qdev_init_gpio_out(dev, &s->irq, 1);


> Having:
> 
> void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq)
> {
>      qemu_irq irq = isa_get_irq(isadev, isairq);
>      qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq);
> }
> 
> What do you think?

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> 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>
> ---
>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>   hw/isa/piix4.c             | 285 -------------------------------------
>   MAINTAINERS                |   6 +-
>   hw/i386/Kconfig            |   2 +-
>   hw/isa/Kconfig             |  12 +-
>   hw/isa/meson.build         |   3 +-
>   hw/mips/Kconfig            |   2 +-
>   7 files changed, 165 insertions(+), 303 deletions(-)
>   rename hw/isa/{piix3.c => piix.c} (75%)
>   delete mode 100644 hw/isa/piix4.c


> @@ -489,11 +534,124 @@ static const TypeInfo piix3_xen_info = {
>       .class_init    = piix3_xen_class_init,
>   };
>   
> +static void piix4_realize(PCIDevice *dev, Error **errp)
> +{

> +    /* initialize pit */
> +    i8254_pit_init(isa_bus, 0x40, 0, NULL);
Pre-existing, why there is no equivalent PIT creation in the
PIIX3 variant? Due to in-kernel PIT in KVM?

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Bernhard Beschow]

Am 12. Januar 2023 16:31:23 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 12/1/23 16:04, Philippe Mathieu-Daudé wrote:
>> On 9/1/23 18:23, Bernhard Beschow wrote:
>>> 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>
>>> ---
>>>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>>>   hw/isa/piix4.c             | 285 -------------------------------------
>>>   MAINTAINERS                |   6 +-
>>>   hw/i386/Kconfig            |   2 +-
>>>   hw/isa/Kconfig             |  12 +-
>>>   hw/isa/meson.build         |   3 +-
>>>   hw/mips/Kconfig            |   2 +-
>>>   7 files changed, 165 insertions(+), 303 deletions(-)
>>>   rename hw/isa/{piix3.c => piix.c} (75%)
>>>   delete mode 100644 hw/isa/piix4.c
>> 
>>> +static void piix4_realize(PCIDevice *dev, Error **errp)
>>> +{
>>> +    PIIXState *s = PIIX_PCI_DEVICE(dev);
>>> +    PCIBus *pci_bus = pci_get_bus(dev);
>>> +    ISABus *isa_bus;
>>> +
>>> +    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
>>> +                          pci_address_space_io(dev), errp);
>>> +    if (!isa_bus) {
>>> +        return;
>>> +    }
>>> +
>>> +    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
>>> +                          "reset-control", 1);
>>> +    memory_region_add_subregion_overlap(pci_address_space_io(dev),
>>> +                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
>>> +
>>> +    /* initialize i8259 pic */
>>> +    if (!qdev_realize(DEVICE(&s->pic), NULL, errp)) {
>>> +        return;
>>> +    }
>>> +
>>> +    /* initialize ISA irqs */
>>> +    isa_bus_irqs(isa_bus, s->pic.in_irqs);
>>> +
>>> +    /* initialize pit */
>>> +    i8254_pit_init(isa_bus, 0x40, 0, NULL);
>>> +
>>> +    /* DMA */
>>> +    i8257_dma_init(isa_bus, 0);
>>> +
>>> +    /* RTC */
>>> +    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
>>> +    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
>>> +        return;
>>> +    }
>>> +    s->rtc.irq = qdev_get_gpio_in(DEVICE(&s->pic), s->rtc.isairq);
>> 
>> Pre-existing; it seems this difference with PIIX3 can be removed
>> because already taken care by calling isa_connect_gpio_out() in mc146818_rtc_init()?
>> 
>> ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
>> {
>>      DeviceState *dev;
>>      ISADevice *isadev;
>>      RTCState *s;
>> 
>>      isadev = isa_new(TYPE_MC146818_RTC);
>>      dev = DEVICE(isadev);
>>      s = MC146818_RTC(isadev);
>>      qdev_prop_set_int32(dev, "base_year", base_year);
>>      isa_realize_and_unref(isadev, bus, &error_fatal);
>>      if (intercept_irq) {
>>          qdev_connect_gpio_out(dev, 0, intercept_irq);
>>      } else {
>>          isa_connect_gpio_out(isadev, 0, s->isairq);
>> 
>
>I meant to paste:
>
>static void rtc_realizefn(DeviceState *dev, Error **errp)
>{
>    ...
>    qdev_init_gpio_out(dev, &s->irq, 1);
>
>
>> Having:
>> 
>> void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq)
>> {
>>      qemu_irq irq = isa_get_irq(isadev, isairq);
>>      qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq);
>> }
>> 
>> What do you think?
>

In "[PATCH v6 11/33] hw/i386/pc: Create RTC controllers in south bridges" mc146818_rtc_init() got inlined, taking into account the intercept_irq, which required the rtc interrupt to be wired up in board code. Since we don't have to deal with intercept_irq in the Malta code, wiring up of the rtc interrupt could be moved into PIIX4.

I would prefer to wire up the rtc interrupt in PIIX3 as well, and to re-wire it in board code in case of intercept_irq != NULL. That's still an open question which needs to be solved for PIIX4 to become a drop-in replacement for PIIX3. Any ideas?

Best regards,
Bernhard

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Bernhard Beschow]

Am 12. Januar 2023 16:36:30 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 9/1/23 18:23, Bernhard Beschow wrote:
>> 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>
>> ---
>>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>>   hw/isa/piix4.c             | 285 -------------------------------------
>>   MAINTAINERS                |   6 +-
>>   hw/i386/Kconfig            |   2 +-
>>   hw/isa/Kconfig             |  12 +-
>>   hw/isa/meson.build         |   3 +-
>>   hw/mips/Kconfig            |   2 +-
>>   7 files changed, 165 insertions(+), 303 deletions(-)
>>   rename hw/isa/{piix3.c => piix.c} (75%)
>>   delete mode 100644 hw/isa/piix4.c
>
>
>> @@ -489,11 +534,124 @@ static const TypeInfo piix3_xen_info = {
>>       .class_init    = piix3_xen_class_init,
>>   };
>>   +static void piix4_realize(PCIDevice *dev, Error **errp)
>> +{
>
>> +    /* initialize pit */
>> +    i8254_pit_init(isa_bus, 0x40, 0, NULL);
>Pre-existing, why there is no equivalent PIT creation in the
>PIIX3 variant? Due to in-kernel PIT in KVM?

Correct, that's one reason. The other reason is for interrupt wiring in case an HPET is present (see our discussion about the intercept_irq for the rtc).

I would like to create and wire up the PIT in PIIX3 as well. Since we'd have to take KVM into account here we may need a similar solution as for I8259. This is another open question for PIIX4 to become a drop-in replacement for PIIX3. Any ideas?

Best regards,
Bernhard

Re: [PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c

[Bernhard Beschow]

Am 12. Januar 2023 13:32:02 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 12/1/23 13:50, Philippe Mathieu-Daudé wrote:
>> On 9/1/23 18:23, Bernhard Beschow wrote:
>>> 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>
>>> ---
>>>   hw/isa/{piix3.c => piix.c} | 158 ++++++++++++++++++++
>>>   hw/isa/piix4.c             | 285 -------------------------------------
>>>   MAINTAINERS                |   6 +-
>>>   hw/i386/Kconfig            |   2 +-
>>>   hw/isa/Kconfig             |  12 +-
>>>   hw/isa/meson.build         |   3 +-
>>>   hw/mips/Kconfig            |   2 +-
>>>   7 files changed, 165 insertions(+), 303 deletions(-)
>>>   rename hw/isa/{piix3.c => piix.c} (75%)
>>>   delete mode 100644 hw/isa/piix4.c
>> 
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>BTW I wonder why PIIX4 isn't calling pci_bus_set_route_irq_fn()...
>Any clue?

Looks like it gets used for proxying: https://elixir.bootlin.com/qemu/v7.2.0/C/ident/pci_device_route_intx_to_irq

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Philippe Mathieu-Daudé]

Hi Bernhard,

On 9/1/23 18:23, 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.

> Bernhard Beschow (30):
>    hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>    hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>    hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>    hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>    hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>    hw/intc/i8259: Make using the isa_pic singleton more type-safe
>    hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>    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: Create TYPE_ISA_PIC in host device
>    hw/isa/piix3: Create IDE controller in host device
>    hw/isa/piix3: Wire up ACPI interrupt internally
>    hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>    hw/isa/piix4: Reuse struct PIIXState from PIIX3
>    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: Reuse PIIX3 base class' realize method in PIIX4
>    hw/isa/piix: Rename functions to be shared for interrupt triggering
>    hw/isa/piix: Consolidate IRQ triggering
>    hw/isa/piix: Share PIIX3's base class with PIIX4
> 
> Philippe Mathieu-Daudé (3):
>    hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>    hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>    hw/isa/piix4: Correct IRQRC[A:D] reset values

I'm queuing the first 10 patches for now to alleviate the size of this
series, and I'll respin a v7 with the rest to avoid making you suffer
any longer :/ Thanks for insisting in this effort and I apologize it
is taking me so long...

Regards,

Phil.

Re: [PATCH v6 04/33] hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> pci_bus_irqs() coupled together the assignment of pci_set_irq_fn and
> pci_map_irq_fn to a PCI bus. This coupling gets in the way when the
> pci_map_irq_fn is board-specific while the pci_set_irq_fn is device-
> specific.
> 
> For example, both of QEMU's PIIX south bridge models have different
> pci_map_irq_fn implementations which are board-specific rather than
> device-specific. These implementations should therefore reside in board
> code. The pci_set_irq_fn's, however, should stay in the device models
> because they access memory internal to the model.
> 
> Factoring out pci_bus_map_irqs() from pci_bus_irqs() allows the
> assignments to be decoupled, resolving the problem described above.
> 
> Note also how pci_vpb_realize() which gets touched in this commit
> assigns different pci_map_irq_fn's depending on the board.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/pci/pci.h    |  3 ++-
>   hw/i386/pc_q35.c        |  4 ++--
>   hw/isa/piix3.c          |  8 ++++----
>   hw/isa/piix4.c          |  3 ++-
>   hw/pci-host/raven.c     |  3 ++-
>   hw/pci-host/versatile.c |  3 ++-
>   hw/pci/pci.c            | 12 +++++++++---
>   hw/remote/machine.c     |  3 ++-
>   8 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 7048a373d1..85ee458cd2 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -282,8 +282,9 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
>                            MemoryRegion *address_space_io,
>                            uint8_t devfn_min, const char *typename);
>   void pci_root_bus_cleanup(PCIBus *bus);
> -void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> +void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
>                     void *irq_opaque, int nirq);
> +void pci_bus_map_irqs(PCIBus *bus, pci_map_irq_fn map_irq);

I'm squashing:

-- >8 --
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index fe1fdfb5f7..46171f22f7 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -667,4 +667,4 @@ void vfu_object_set_bus_irq(PCIBus *pci_bus)

-    pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
-                 max_bdf);
+    pci_bus_irqs(pci_bus, vfu_object_set_irq, , pci_bus, max_bdf);
+    pci_bus_map_irqs(pci_bus, vfu_object_map_irq);
  }
---

to fix:

../hw/remote/vfio-user-obj.c: In function ‘vfu_object_set_bus_irq’:
../hw/remote/vfio-user-obj.c:668:67: error: passing argument 4 of 
‘pci_bus_irqs’ makes integer from pointer without a cast 
[-Werror=int-conversion]
      pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
                                                                    ^~~~~~~
In file included from include/hw/pci/pci_device.h:4,
                  from include/hw/remote/iohub.h:14,
                  from include/hw/remote/machine.h:18,
                  from ../hw/remote/vfio-user-obj.c:43:
include/hw/pci/pci.h:286:41: note: expected ‘int’ but argument is of 
type ‘PCIBus *’ {aka ‘struct PCIBus *’}
                    void *irq_opaque, int nirq);
                                      ~~~~^~~~
../hw/remote/vfio-user-obj.c:668:5: error: too many arguments to 
function ‘pci_bus_irqs’
      pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
      ^~~~~~~~~~~~
In file included from include/hw/pci/pci_device.h:4,
                  from include/hw/remote/iohub.h:14,
                  from include/hw/remote/machine.h:18,
                  from ../hw/remote/vfio-user-obj.c:43:
include/hw/pci/pci.h:285:6: note: declared here
  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
       ^~~~~~~~~~~~

Re: [PATCH v6 04/33] hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()

[Bernhard Beschow]

Am 13. Januar 2023 10:13:29 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 9/1/23 18:23, Bernhard Beschow wrote:
>> pci_bus_irqs() coupled together the assignment of pci_set_irq_fn and
>> pci_map_irq_fn to a PCI bus. This coupling gets in the way when the
>> pci_map_irq_fn is board-specific while the pci_set_irq_fn is device-
>> specific.
>> 
>> For example, both of QEMU's PIIX south bridge models have different
>> pci_map_irq_fn implementations which are board-specific rather than
>> device-specific. These implementations should therefore reside in board
>> code. The pci_set_irq_fn's, however, should stay in the device models
>> because they access memory internal to the model.
>> 
>> Factoring out pci_bus_map_irqs() from pci_bus_irqs() allows the
>> assignments to be decoupled, resolving the problem described above.
>> 
>> Note also how pci_vpb_realize() which gets touched in this commit
>> assigns different pci_map_irq_fn's depending on the board.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/pci/pci.h    |  3 ++-
>>   hw/i386/pc_q35.c        |  4 ++--
>>   hw/isa/piix3.c          |  8 ++++----
>>   hw/isa/piix4.c          |  3 ++-
>>   hw/pci-host/raven.c     |  3 ++-
>>   hw/pci-host/versatile.c |  3 ++-
>>   hw/pci/pci.c            | 12 +++++++++---
>>   hw/remote/machine.c     |  3 ++-
>>   8 files changed, 25 insertions(+), 14 deletions(-)
>> 
>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>> index 7048a373d1..85ee458cd2 100644
>> --- a/include/hw/pci/pci.h
>> +++ b/include/hw/pci/pci.h
>> @@ -282,8 +282,9 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
>>                            MemoryRegion *address_space_io,
>>                            uint8_t devfn_min, const char *typename);
>>   void pci_root_bus_cleanup(PCIBus *bus);
>> -void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>> +void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
>>                     void *irq_opaque, int nirq);
>> +void pci_bus_map_irqs(PCIBus *bus, pci_map_irq_fn map_irq);
>
>I'm squashing:
>
>-- >8 --
>diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
>index fe1fdfb5f7..46171f22f7 100644
>--- a/hw/remote/vfio-user-obj.c
>+++ b/hw/remote/vfio-user-obj.c
>@@ -667,4 +667,4 @@ void vfu_object_set_bus_irq(PCIBus *pci_bus)
>
>-    pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
>-                 max_bdf);
>+    pci_bus_irqs(pci_bus, vfu_object_set_irq, , pci_bus, max_bdf);
>+    pci_bus_map_irqs(pci_bus, vfu_object_map_irq);
> }
>---
>
>to fix:
>
>../hw/remote/vfio-user-obj.c: In function ‘vfu_object_set_bus_irq’:
>../hw/remote/vfio-user-obj.c:668:67: error: passing argument 4 of ‘pci_bus_irqs’ makes integer from pointer without a cast [-Werror=int-conversion]
>     pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
>                                                                   ^~~~~~~
>In file included from include/hw/pci/pci_device.h:4,
>                 from include/hw/remote/iohub.h:14,
>                 from include/hw/remote/machine.h:18,
>                 from ../hw/remote/vfio-user-obj.c:43:
>include/hw/pci/pci.h:286:41: note: expected ‘int’ but argument is of type ‘PCIBus *’ {aka ‘struct PCIBus *’}
>                   void *irq_opaque, int nirq);
>                                     ~~~~^~~~
>../hw/remote/vfio-user-obj.c:668:5: error: too many arguments to function ‘pci_bus_irqs’
>     pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
>     ^~~~~~~~~~~~
>In file included from include/hw/pci/pci_device.h:4,
>                 from include/hw/remote/iohub.h:14,
>                 from include/hw/remote/machine.h:18,
>                 from ../hw/remote/vfio-user-obj.c:43:
>include/hw/pci/pci.h:285:6: note: declared here
> void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
>      ^~~~~~~~~~~~
>

Thanks!

I've missed enabling vfio-user-server for my builds. Fixed now.

Best regards,
Bernhard

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Hi Bernhard,
>
>On 9/1/23 18:23, 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.
>
>> Bernhard Beschow (30):
>>    hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>    hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>    hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>    hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>    hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>    hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>    hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>    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: Create TYPE_ISA_PIC in host device
>>    hw/isa/piix3: Create IDE controller in host device
>>    hw/isa/piix3: Wire up ACPI interrupt internally
>>    hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>    hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>    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: Reuse PIIX3 base class' realize method in PIIX4
>>    hw/isa/piix: Rename functions to be shared for interrupt triggering
>>    hw/isa/piix: Consolidate IRQ triggering
>>    hw/isa/piix: Share PIIX3's base class with PIIX4
>> 
>> Philippe Mathieu-Daudé (3):
>>    hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>    hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>    hw/isa/piix4: Correct IRQRC[A:D] reset values
>
>I'm queuing the first 10 patches for now to alleviate the size of this
>series, and I'll respin a v7 with the rest to avoid making you suffer
>any longer :/ Thanks for insisting in this effort and I apologize it
>is taking me so long...

Okay... What's the further plan? Is there anything missing?

Thanks,
Bernhard
>
>Regards,
>
>Phil.

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>Hi Bernhard,
>>
>>On 9/1/23 18:23, 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.
>>
>>> Bernhard Beschow (30):
>>>    hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>>    hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>>    hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>>    hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>>    hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>>    hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>>    hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>>    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: Create TYPE_ISA_PIC in host device
>>>    hw/isa/piix3: Create IDE controller in host device
>>>    hw/isa/piix3: Wire up ACPI interrupt internally
>>>    hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>>    hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>    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: Reuse PIIX3 base class' realize method in PIIX4
>>>    hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>    hw/isa/piix: Consolidate IRQ triggering
>>>    hw/isa/piix: Share PIIX3's base class with PIIX4
>>> 
>>> Philippe Mathieu-Daudé (3):
>>>    hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>>    hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>>    hw/isa/piix4: Correct IRQRC[A:D] reset values
>>
>>I'm queuing the first 10 patches for now to alleviate the size of this
>>series, and I'll respin a v7 with the rest to avoid making you suffer
>>any longer :/ Thanks for insisting in this effort and I apologize it
>>is taking me so long...
>
>Okay... What's the further plan? Is there anything missing?

Ping

>
>Thanks,
>Bernhard
>>
>>Regards,
>>
>>Phil.

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Philippe Mathieu-Daudé]

On 20/1/23 13:22, Bernhard Beschow wrote:
> Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>> Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> On 9/1/23 18:23, 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.
>>>
>>>> Bernhard Beschow (30):
>>>>     hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>>>     hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>>>     hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>>>     hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>>>     hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>>>     hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>>>     hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>>>     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: Create TYPE_ISA_PIC in host device
>>>>     hw/isa/piix3: Create IDE controller in host device
>>>>     hw/isa/piix3: Wire up ACPI interrupt internally
>>>>     hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>>>     hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>>     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: Reuse PIIX3 base class' realize method in PIIX4
>>>>     hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>>     hw/isa/piix: Consolidate IRQ triggering
>>>>     hw/isa/piix: Share PIIX3's base class with PIIX4
>>>>
>>>> Philippe Mathieu-Daudé (3):
>>>>     hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>>>     hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>>>     hw/isa/piix4: Correct IRQRC[A:D] reset values
>>>
>>> I'm queuing the first 10 patches for now to alleviate the size of this
>>> series, and I'll respin a v7 with the rest to avoid making you suffer
>>> any longer :/ Thanks for insisting in this effort and I apologize it
>>> is taking me so long...
>>
>> Okay... What's the further plan? Is there anything missing?
> 
> Ping

The plan is "I'll respin a v7 with the rest".

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 23. Januar 2023 09:25:51 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 20/1/23 13:22, Bernhard Beschow wrote:
>> Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>> Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> On 9/1/23 18:23, 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.
>>>> 
>>>>> Bernhard Beschow (30):
>>>>>     hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>>>>     hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>>>>     hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>>>>     hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>>>>     hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>>>>     hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>>>>     hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>>>>     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: Create TYPE_ISA_PIC in host device
>>>>>     hw/isa/piix3: Create IDE controller in host device
>>>>>     hw/isa/piix3: Wire up ACPI interrupt internally
>>>>>     hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>>>>     hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>>>     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: Reuse PIIX3 base class' realize method in PIIX4
>>>>>     hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>>>     hw/isa/piix: Consolidate IRQ triggering
>>>>>     hw/isa/piix: Share PIIX3's base class with PIIX4
>>>>> 
>>>>> Philippe Mathieu-Daudé (3):
>>>>>     hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>>>>     hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>>>>     hw/isa/piix4: Correct IRQRC[A:D] reset values
>>>> 
>>>> I'm queuing the first 10 patches for now to alleviate the size of this
>>>> series, and I'll respin a v7 with the rest to avoid making you suffer
>>>> any longer :/ Thanks for insisting in this effort and I apologize it
>>>> is taking me so long...
>>> 
>>> Okay... What's the further plan? Is there anything missing?
>> 
>> Ping
>
>The plan is "I'll respin a v7 with the rest".

I understood that part. I wonder what the blocking issue is/was.

Best regards,
Bernhard

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 23. Januar 2023 15:51:49 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 23. Januar 2023 09:25:51 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>On 20/1/23 13:22, Bernhard Beschow wrote:
>>> Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>>> Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>>> On 9/1/23 18:23, 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.
>>>>> 
>>>>>> Bernhard Beschow (30):
>>>>>>     hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>>>>>     hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>     hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>     hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>>>>>     hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>>>>>     hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>>>>>     hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>>>>>     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: Create TYPE_ISA_PIC in host device
>>>>>>     hw/isa/piix3: Create IDE controller in host device
>>>>>>     hw/isa/piix3: Wire up ACPI interrupt internally
>>>>>>     hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>>>>>     hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>>>>     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: Reuse PIIX3 base class' realize method in PIIX4
>>>>>>     hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>>>>     hw/isa/piix: Consolidate IRQ triggering
>>>>>>     hw/isa/piix: Share PIIX3's base class with PIIX4
>>>>>> 
>>>>>> Philippe Mathieu-Daudé (3):
>>>>>>     hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>>>>>     hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>>>>>     hw/isa/piix4: Correct IRQRC[A:D] reset values
>>>>> 
>>>>> I'm queuing the first 10 patches for now to alleviate the size of this
>>>>> series, and I'll respin a v7 with the rest to avoid making you suffer
>>>>> any longer :/ Thanks for insisting in this effort and I apologize it
>>>>> is taking me so long...
>>>> 
>>>> Okay... What's the further plan? Is there anything missing?
>>> 
>>> Ping
>>
>>The plan is "I'll respin a v7 with the rest".
>
>I understood that part. I wonder what the blocking issue is/was.

The first part of this series contains piix3 changes such as the ISA proxy pic and movement of rtc. This seems the riskier part of the series to me which I'd like to get feedback on from the field rather sooner than later. The reason is that I can't currently forsee how fast I could react if these changes were merged during (soft) freeze.

Is there a possibility to at least get the piix3 part merged already? Maybe perhaps via the pc tree?

Thanks,
Bernhard

>
>Best regards,
>Bernhard

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Philippe Mathieu-Daudé]

On 10/2/23 17:27, Bernhard Beschow wrote:
> 
> 
> Am 23. Januar 2023 15:51:49 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>
>>
>> Am 23. Januar 2023 09:25:51 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> On 20/1/23 13:22, Bernhard Beschow wrote:
>>>> Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>>>> Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>>>> On 9/1/23 18:23, 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.
>>>>>>
>>>>>>> Bernhard Beschow (30):
>>>>>>>      hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>>>>>>      hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>>      hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>>      hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>>>>>>      hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>>>>>>      hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>>>>>>      hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>>>>>>      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: Create TYPE_ISA_PIC in host device
>>>>>>>      hw/isa/piix3: Create IDE controller in host device
>>>>>>>      hw/isa/piix3: Wire up ACPI interrupt internally
>>>>>>>      hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>>>>>>      hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>>>>>      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: Reuse PIIX3 base class' realize method in PIIX4
>>>>>>>      hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>>>>>      hw/isa/piix: Consolidate IRQ triggering
>>>>>>>      hw/isa/piix: Share PIIX3's base class with PIIX4
>>>>>>>
>>>>>>> Philippe Mathieu-Daudé (3):
>>>>>>>      hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>>>>>>      hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>>>>>>      hw/isa/piix4: Correct IRQRC[A:D] reset values
>>>>>>
>>>>>> I'm queuing the first 10 patches for now to alleviate the size of this
>>>>>> series, and I'll respin a v7 with the rest to avoid making you suffer
>>>>>> any longer :/ Thanks for insisting in this effort and I apologize it
>>>>>> is taking me so long...
>>>>>
>>>>> Okay... What's the further plan? Is there anything missing?
>>>>
>>>> Ping
>>>
>>> The plan is "I'll respin a v7 with the rest".
>>
>> I understood that part. I wonder what the blocking issue is/was.
> 
> The first part of this series contains piix3 changes such as the ISA proxy pic and movement of rtc. This seems the riskier part of the series to me which I'd like to get feedback on from the field rather sooner than later. The reason is that I can't currently forsee how fast I could react if these changes were merged during (soft) freeze.

What bugs me is the i8259 proxy. I applied your patches locally, and am
trying to rework to avoid using it. I amend a 'Co-developed-by' tag
to patches modified:
https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 10. Februar 2023 17:11:06 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 10/2/23 17:27, Bernhard Beschow wrote:
>> 
>> 
>> Am 23. Januar 2023 15:51:49 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>> 
>>> 
>>> Am 23. Januar 2023 09:25:51 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> On 20/1/23 13:22, Bernhard Beschow wrote:
>>>>> Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>>>>> Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>>>>> On 9/1/23 18:23, 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.
>>>>>>> 
>>>>>>>> Bernhard Beschow (30):
>>>>>>>>      hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>>>>>>>      hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>>>      hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>>>      hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>>>>>>>      hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>>>>>>>      hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>>>>>>>      hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>>>>>>>      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: Create TYPE_ISA_PIC in host device
>>>>>>>>      hw/isa/piix3: Create IDE controller in host device
>>>>>>>>      hw/isa/piix3: Wire up ACPI interrupt internally
>>>>>>>>      hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>>>>>>>      hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>>>>>>      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: Reuse PIIX3 base class' realize method in PIIX4
>>>>>>>>      hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>>>>>>      hw/isa/piix: Consolidate IRQ triggering
>>>>>>>>      hw/isa/piix: Share PIIX3's base class with PIIX4
>>>>>>>> 
>>>>>>>> Philippe Mathieu-Daudé (3):
>>>>>>>>      hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>>>>>>>      hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>>>>>>>      hw/isa/piix4: Correct IRQRC[A:D] reset values
>>>>>>> 
>>>>>>> I'm queuing the first 10 patches for now to alleviate the size of this
>>>>>>> series, and I'll respin a v7 with the rest to avoid making you suffer
>>>>>>> any longer :/ Thanks for insisting in this effort and I apologize it
>>>>>>> is taking me so long...
>>>>>> 
>>>>>> Okay... What's the further plan? Is there anything missing?
>>>>> 
>>>>> Ping
>>>> 
>>>> The plan is "I'll respin a v7 with the rest".
>>> 
>>> I understood that part. I wonder what the blocking issue is/was.
>> 
>> The first part of this series contains piix3 changes such as the ISA proxy pic and movement of rtc. This seems the riskier part of the series to me which I'd like to get feedback on from the field rather sooner than later. The reason is that I can't currently forsee how fast I could react if these changes were merged during (soft) freeze.
>
>What bugs me is the i8259 proxy.

I agree that it smells like a workaround for some problem elsewhere.

The patch introducing the proxy to piix3 could be replaced by a patch moving:
- isa_bus_irqs() into piix3's realize method and
- `piix3->pic = x86ms->gsi;` in front of the realize call of piix3.

Would that work for you? I could respin then.

A first test run with the pc machine seems promising. The Malta machine would need similar treatment.

Best regards,
Bernhard

>I applied your patches locally, and am
>trying to rework to avoid using it. I amend a 'Co-developed-by' tag
>to patches modified:
>https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
>

Re: [PATCH v6 00/33] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 11. Februar 2023 16:23:10 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 10. Februar 2023 17:11:06 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>On 10/2/23 17:27, Bernhard Beschow wrote:
>>> 
>>> 
>>> Am 23. Januar 2023 15:51:49 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>>> 
>>>> 
>>>> Am 23. Januar 2023 09:25:51 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>>> On 20/1/23 13:22, Bernhard Beschow wrote:
>>>>>> Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>>>>>> Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>>>>>> On 9/1/23 18:23, 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.
>>>>>>>> 
>>>>>>>>> Bernhard Beschow (30):
>>>>>>>>>      hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
>>>>>>>>>      hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>>>>      hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
>>>>>>>>>      hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
>>>>>>>>>      hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
>>>>>>>>>      hw/intc/i8259: Make using the isa_pic singleton more type-safe
>>>>>>>>>      hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
>>>>>>>>>      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: Create TYPE_ISA_PIC in host device
>>>>>>>>>      hw/isa/piix3: Create IDE controller in host device
>>>>>>>>>      hw/isa/piix3: Wire up ACPI interrupt internally
>>>>>>>>>      hw/isa/piix3: 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: Use TYPE_ISA_PIC device
>>>>>>>>>      hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>>>>>>>>      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: Reuse PIIX3 base class' realize method in PIIX4
>>>>>>>>>      hw/isa/piix: Rename functions to be shared for interrupt triggering
>>>>>>>>>      hw/isa/piix: Consolidate IRQ triggering
>>>>>>>>>      hw/isa/piix: Share PIIX3's base class with PIIX4
>>>>>>>>> 
>>>>>>>>> Philippe Mathieu-Daudé (3):
>>>>>>>>>      hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>>>>>>>>>      hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>>>>>>>>>      hw/isa/piix4: Correct IRQRC[A:D] reset values
>>>>>>>> 
>>>>>>>> I'm queuing the first 10 patches for now to alleviate the size of this
>>>>>>>> series, and I'll respin a v7 with the rest to avoid making you suffer
>>>>>>>> any longer :/ Thanks for insisting in this effort and I apologize it
>>>>>>>> is taking me so long...
>>>>>>> 
>>>>>>> Okay... What's the further plan? Is there anything missing?
>>>>>> 
>>>>>> Ping
>>>>> 
>>>>> The plan is "I'll respin a v7 with the rest".
>>>> 
>>>> I understood that part. I wonder what the blocking issue is/was.
>>> 
>>> The first part of this series contains piix3 changes such as the ISA proxy pic and movement of rtc. This seems the riskier part of the series to me which I'd like to get feedback on from the field rather sooner than later. The reason is that I can't currently forsee how fast I could react if these changes were merged during (soft) freeze.
>>
>>What bugs me is the i8259 proxy.
>
>I agree that it smells like a workaround for some problem elsewhere.
>
>The patch introducing the proxy to piix3 could be replaced by a patch moving:
>- isa_bus_irqs() into piix3's realize method and
>- `piix3->pic = x86ms->gsi;` in front of the realize call of piix3.
>
>Would that work for you? I could respin then.
>
>A first test run with the pc machine seems promising. The Malta machine would need similar treatment.

v7 is out which doesn't need a PIC proxy.

Best regards,
Bernhard

>
>Best regards,
>Bernhard
>
>>I applied your patches locally, and am
>>trying to rework to avoid using it. I amend a 'Co-developed-by' tag
>>to patches modified:
>>https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
>>