qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 00/29] Consolidate PIIX south bridges
@ 2023-10-07 12:38 Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 01/29] hw/i386/pc: Merge two if statements into one Bernhard Beschow
                   ` (29 more replies)
  0 siblings, 30 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

This series consolidates the implementations of the PIIX3 and PIIX4 south
bridges and makes PIIX4 usable in the PC machine via an experimental command
line parameter. The motivation is to resolve duplicate code between the device
models as well as resolving the "Frankenstein" PIIX4-PM problem in PIIX3
discussed on this list before.

The series is structured as follows:

Patches 1-8 are preparational patches necessary for moving all sub devices into
PIIX3, like was done for PIIX4. In isolation these patches can also be seen as
general x86 machine cleanup sub series which has merit in its own right -- and
could be applied to master if the remainder of the series takes longer to
review.

Patches 9-13 move PIIX3 sub devices into one device model like already
done for PIIX4. Together with the previous sub series these patches form a
bigger sub series which also has merit in its own right, and could be applied
independent of the remainder of this series as well.

The remainder of this series consolidates the PIIX3 and PIIX4 device models.
The culmination point is the last commit which makes PIIX4 usable in the PC
machine.

One challenge was dealing with optional devices where Peter already gave advice
in [1] which this series implements. Although PIIX4 is now usable in the PC
machine it still has a different binary layout in its VM state.

Testing done:
* `make check`
* `qemu-system-x86_64 -M pc -m 2G -accel kvm -cdrom
     manjaro-kde-21.3.2-220704-linux515.iso`
* `qemu-system-x86_64 -M pc,x-south-bridge=piix4-isa -m 2G -accel kvm -cdrom
     manjaro-kde-21.3.2-220704-linux515.iso`
* `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cdrom
     manjaro-kde-21.3.2-220704-linux515.iso`
* `qemu-system-mips64el -M malta -cpu 5KEc -m 1G -kernel kernel -initrd initrd
     -append "root=LABEL=rootfs console=ttyS0" -drive file=image.qcow2`
* `qemu-system-mips64el -M malta -bios yamon-02.22.bin`
* Run HVM domU guest under Xen with manjaro-kde-21.3.2-220704-linux515.iso image

v8:
- Wire ISA interrupts before device realization
- Optionally allow a PIC and PIT to be instantiated in PIIX3 for compatiblity
    with PIIX4
- Touch ICH9 LPC as far as required for PIIX consolidation
- Make PIIX4 usable in the PC machine via an experimental option
- Review and rework history, touching every commit and drop R-b tags when
    changes became too large

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

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

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

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

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

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

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

Bernhard Beschow (29):
  hw/i386/pc: Merge two if statements into one
  hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
    south bridge
  hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize()
  hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
  hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs"
    property
  hw/i386/pc_piix: Remove redundant "piix3" variable
  hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in"
  hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its
    realize()
  hw/isa/piix3: Wire PIC IRQs to ISA bus in host device
  hw/i386/pc: Wire RTC ISA IRQs in south bridges
  hw/isa/piix3: Create IDE controller in host device
  hw/isa/piix3: Create USB controller in host device
  hw/isa/piix3: Create power management controller in host device
  hw/isa/piix3: Drop the "3" from PIIX base class name
  hw/isa/piix4: Remove unused inbound ISA interrupt lines
  hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in"
  hw/isa/piix4: Rename reset control operations to match PIIX3
  hw/isa/piix4: Reuse struct PIIXState from PIIX3
  hw/isa/piix3: Merge hw/isa/piix4.c
  hw/isa/piix: Allow for optional PIC creation in PIIX3
  hw/isa/piix: Allow for optional PIT creation in PIIX3
  hw/isa/piix: Harmonize names of reset control memory regions
  hw/isa/piix: Share PIIX3's base class with PIIX4
  hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
  hw/isa/piix: Rename functions to be shared for PCI interrupt
    triggering
  hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4
  hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring
  hw/isa/piix: Implement multi-process QEMU support also for PIIX4
  hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine

 MAINTAINERS                          |   6 +-
 docs/system/target-i386-desc.rst.inc |   8 +
 include/hw/i386/pc.h                 |   2 +
 include/hw/southbridge/piix.h        |  28 ++-
 hw/i386/pc.c                         |  13 +-
 hw/i386/pc_piix.c                    | 125 ++++++++---
 hw/i386/pc_q35.c                     |  14 +-
 hw/isa/lpc_ich9.c                    |   9 +-
 hw/isa/{piix3.c => piix.c}           | 281 ++++++++++++++++++-------
 hw/isa/piix4.c                       | 302 ---------------------------
 hw/mips/malta.c                      |   5 +-
 hw/i386/Kconfig                      |   3 +-
 hw/isa/Kconfig                       |   8 +-
 hw/isa/meson.build                   |   3 +-
 hw/mips/Kconfig                      |   2 +-
 15 files changed, 358 insertions(+), 451 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (52%)
 delete mode 100644 hw/isa/piix4.c

-- 
2.42.0



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

* [PATCH v8 01/29] hw/i386/pc: Merge two if statements into one
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 02/29] hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge Bernhard Beschow
                   ` (28 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

By being the only entity assigning a non-NULL value to "rtc_irq", the first if
statement determines whether the second if statement is executed. So merge the
two statements into one.

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

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index aad7e8ccd1..2fbdff89e0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1200,7 +1200,6 @@ void pc_basic_device_init(struct PCMachineState *pcms,
     DeviceState *hpet = NULL;
     int pit_isa_irq = 0;
     qemu_irq pit_alt_irq = NULL;
-    qemu_irq rtc_irq = NULL;
     ISADevice *pit = NULL;
     MemoryRegion *ioport80_io = g_new(MemoryRegion, 1);
     MemoryRegion *ioportF0_io = g_new(MemoryRegion, 1);
@@ -1220,6 +1219,8 @@ void pc_basic_device_init(struct PCMachineState *pcms,
      */
     if (pcms->hpet_enabled && (!kvm_irqchip_in_kernel() ||
                                kvm_has_pit_state2())) {
+        qemu_irq rtc_irq;
+
         hpet = qdev_try_new(TYPE_HPET);
         if (!hpet) {
             error_report("couldn't create HPET device");
@@ -1244,9 +1245,6 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         pit_isa_irq = -1;
         pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
         rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
-    }
-
-    if (rtc_irq) {
         qdev_connect_gpio_out(DEVICE(rtc_state), 0, rtc_irq);
     } else {
         uint32_t irq = object_property_get_uint(OBJECT(rtc_state),
@@ -1254,6 +1252,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
                                                 &error_fatal);
         isa_connect_gpio_out(rtc_state, 0, irq);
     }
+
     object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(rtc_state),
                               "date");
 
-- 
2.42.0



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

* [PATCH v8 02/29] hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 01/29] hw/i386/pc: Merge two if statements into one Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 03/29] hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize() Bernhard Beschow
                   ` (27 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow, Peter Maydell

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>
---
 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 e36a3262b2..6d2f5509e6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -264,7 +264,8 @@ static void pc_init1(MachineState *machine,
         PIIX3State *piix3;
         PCIDevice *pci_dev;
 
-        pci_dev = pci_create_simple_multifunction(pci_bus, -1, TYPE_PIIX3_DEVICE);
+        pci_dev = pci_new_multifunction(-1, TYPE_PIIX3_DEVICE);
+        pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
         if (xen_enabled()) {
             pci_device_set_intx_routing_notifier(
-- 
2.42.0



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

* [PATCH v8 03/29] hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize()
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 01/29] hw/i386/pc: Merge two if statements into one Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 02/29] hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 04/29] hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS Bernhard Beschow
                   ` (26 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Unlike its PIIX4 counterpart, TYPE_PIIX3_DEVICE doesn't instantiate a PIC
itself. Instead, it relies on the board to do so. This means that the board
needs to wire the ISA IRQs to the PIIX3 device model. As long as the board
assigns the ISA IRQs after PIIX3's realize(), internal devices can't be wired in
pci_piix3_realize() since the qemu_irqs are still NULL. Fix that by assigning
the ISA interrupts before realize(). This will allow for embedding child devices
into the host device as already done for PIIX4.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>

---

Note that this avoids the "PIC proxy" of previous iterations of the PIIX
consolidation series. Assigning the IRQs before realize() has been agreed upon
at KVM forum 2023.
---
 hw/i386/pc_piix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6d2f5509e6..a003923788 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -265,6 +265,8 @@ static void pc_init1(MachineState *machine,
         PCIDevice *pci_dev;
 
         pci_dev = pci_new_multifunction(-1, TYPE_PIIX3_DEVICE);
+        piix3 = PIIX3_PCI_DEVICE(pci_dev);
+        piix3->pic = x86ms->gsi;
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
         if (xen_enabled()) {
@@ -281,8 +283,6 @@ static void pc_init1(MachineState *machine,
                          XEN_IOAPIC_NUM_PIRQS);
         }
 
-        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),
-- 
2.42.0



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

* [PATCH v8 04/29] hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (2 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 03/29] hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize() Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 05/29] hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs" property Bernhard Beschow
                   ` (25 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	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>
---
 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 278171752f..2317bb7974 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -27,7 +27,6 @@
  */
 #define PIIX_RCR_IOPORT 0xcf9
 
-#define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
 #define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
 
 struct PIIXState {
@@ -39,10 +38,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 117024e450..7240c91440 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -48,7 +48,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;
     }
 
@@ -62,7 +62,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;
     }
 
@@ -83,7 +83,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 {
@@ -115,7 +115,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.42.0



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

* [PATCH v8 05/29] hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs" property
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (3 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 04/29] hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 06/29] hw/i386/pc_piix: Remove redundant "piix3" variable Bernhard Beschow
                   ` (24 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Avoid assigning the private member of struct PIIX3State from outside which goes
against best QOM practices. Instead, implement best QOM practice by adding an
"isa-irqs" array property to TYPE_PIIX3_DEVICE and assign it in board code, i.e.
from outside.

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 2317bb7974..bb898c6c88 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -46,7 +46,7 @@ struct PIIXState {
 #endif
     uint64_t pic_levels;
 
-    qemu_irq *pic;
+    qemu_irq pic[ISA_NUM_IRQS];
 
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a003923788..4dc7298c15 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -263,10 +263,15 @@ static void pc_init1(MachineState *machine,
     if (pcmc->pci_enabled) {
         PIIX3State *piix3;
         PCIDevice *pci_dev;
+        DeviceState *dev;
+        size_t i;
 
         pci_dev = pci_new_multifunction(-1, TYPE_PIIX3_DEVICE);
         piix3 = PIIX3_PCI_DEVICE(pci_dev);
-        piix3->pic = x86ms->gsi;
+        dev = DEVICE(pci_dev);
+        for (i = 0; i < ISA_NUM_IRQS; i++) {
+            qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
+        }
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
         if (xen_enabled()) {
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 7240c91440..c17547a2c0 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -312,6 +312,8 @@ static void pci_piix3_init(Object *obj)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(obj);
 
+    qdev_init_gpio_out_named(DEVICE(obj), d->pic, "isa-irqs", ISA_NUM_IRQS);
+
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
 }
 
-- 
2.42.0



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

* [PATCH v8 06/29] hw/i386/pc_piix: Remove redundant "piix3" variable
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (4 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 05/29] hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs" property Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 07/29] hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in" Bernhard Beschow
                   ` (23 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

The variable is never used by its declared type. Eliminate it.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4dc7298c15..cd6c00c0b3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -261,13 +261,11 @@ static void pc_init1(MachineState *machine,
     gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
 
     if (pcmc->pci_enabled) {
-        PIIX3State *piix3;
         PCIDevice *pci_dev;
         DeviceState *dev;
         size_t i;
 
         pci_dev = pci_new_multifunction(-1, TYPE_PIIX3_DEVICE);
-        piix3 = PIIX3_PCI_DEVICE(pci_dev);
         dev = DEVICE(pci_dev);
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
@@ -288,8 +286,8 @@ static void pc_init1(MachineState *machine,
                          XEN_IOAPIC_NUM_PIRQS);
         }
 
-        piix3_devfn = piix3->dev.devfn;
-        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+        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"));
     } else {
-- 
2.42.0



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

* [PATCH v8 07/29] hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in"
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (5 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 06/29] hw/i386/pc_piix: Remove redundant "piix3" variable Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 08/29] hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its realize() Bernhard Beschow
                   ` (22 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

TYPE_PIIX3_DEVICE doesn't instantiate a PIC since it relies on the board to do
so. The "pic" attribute, however, suggests that there is one. Rename the
attribute to reflect that it represents ISA interrupt lines. Use the same naming
convention as in the VIA south bridges as well as in TYPE_I82378.

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index bb898c6c88..b07ff6bb26 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -46,7 +46,7 @@ struct PIIXState {
 #endif
     uint64_t pic_levels;
 
-    qemu_irq pic[ISA_NUM_IRQS];
+    qemu_irq isa_irqs_in[ISA_NUM_IRQS];
 
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c17547a2c0..616f5418fa 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -36,7 +36,7 @@
 
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
-    qemu_set_irq(piix3->pic[pic_irq],
+    qemu_set_irq(piix3->isa_irqs_in[pic_irq],
                  !!(piix3->pic_levels &
                     (((1ULL << PIIX_NUM_PIRQS) - 1) <<
                      (pic_irq * PIIX_NUM_PIRQS))));
@@ -312,7 +312,8 @@ static void pci_piix3_init(Object *obj)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(obj);
 
-    qdev_init_gpio_out_named(DEVICE(obj), d->pic, "isa-irqs", ISA_NUM_IRQS);
+    qdev_init_gpio_out_named(DEVICE(obj), d->isa_irqs_in, "isa-irqs",
+                             ISA_NUM_IRQS);
 
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
 }
-- 
2.42.0



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

* [PATCH v8 08/29] hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its realize()
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (6 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 07/29] hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in" Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 09/29] hw/isa/piix3: Wire PIC IRQs to ISA bus in host device Bernhard Beschow
                   ` (21 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

When the board assigns the ISA IRQs after the device's realize(), internal
devices such as the RTC can't be wired in ich9_lpc_realize() since the qemu_irqs
are still NULL. Fix that by assigning the ISA interrupts before realize().

This change is necessary for PIIX consolidation because PIIX4 wires the RTC
interrupts in its realize() method, so PIIX3 needs to do so as well. Since the
PC and Q35 boards share RTC code, and since PIIX3 needs the change, ICH9 needs
to be adapted as well.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_q35.c  | 14 +++++++-------
 hw/isa/lpc_ich9.c |  6 +++---
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a7386f2ca2..597943ff1b 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -242,11 +242,18 @@ static void pc_q35_init(MachineState *machine)
     host_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pcie.0"));
     pcms->bus = host_bus;
 
+    /* irq lines */
+    gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
+
     /* create ISA bus */
     lpc = pci_new_multifunction(PCI_DEVFN(ICH9_LPC_DEV, ICH9_LPC_FUNC),
                                 TYPE_ICH9_LPC_DEVICE);
     qdev_prop_set_bit(DEVICE(lpc), "smm-enabled",
                       x86_machine_is_smm_enabled(x86ms));
+    lpc_dev = DEVICE(lpc);
+    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+        qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
+    }
     pci_realize_and_unref(lpc, host_bus, &error_fatal);
 
     rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc"));
@@ -273,13 +280,6 @@ static void pc_q35_init(MachineState *machine)
                                    "true", true);
     }
 
-    /* irq lines */
-    gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
-
-    lpc_dev = DEVICE(lpc);
-    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
-        qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
-    }
     isa_bus = ISA_BUS(qdev_get_child_bus(lpc_dev, "isa.0"));
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 3f59980aa0..3fcefc5a8a 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -675,6 +675,9 @@ static void ich9_lpc_initfn(Object *obj)
 
     object_initialize_child(obj, "rtc", &lpc->rtc, TYPE_MC146818_RTC);
 
+    qdev_init_gpio_out_named(DEVICE(lpc), lpc->gsi, ICH9_GPIO_GSI,
+                             IOAPIC_NUM_PINS);
+
     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,
@@ -691,7 +694,6 @@ static void ich9_lpc_initfn(Object *obj)
 static void ich9_lpc_realize(PCIDevice *d, Error **errp)
 {
     ICH9LPCState *lpc = ICH9_LPC_DEVICE(d);
-    DeviceState *dev = DEVICE(d);
     PCIBus *pci_bus = pci_get_bus(d);
     ISABus *isa_bus;
 
@@ -734,8 +736,6 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
                                         ICH9_RST_CNT_IOPORT, &lpc->rst_cnt_mem,
                                         1);
 
-    qdev_init_gpio_out_named(dev, lpc->gsi, ICH9_GPIO_GSI, IOAPIC_NUM_PINS);
-
     isa_bus_register_input_irqs(isa_bus, lpc->gsi);
 
     i8257_dma_init(isa_bus, 0);
-- 
2.42.0



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

* [PATCH v8 09/29] hw/isa/piix3: Wire PIC IRQs to ISA bus in host device
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (7 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 08/29] hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its realize() Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 10/29] hw/i386/pc: Wire RTC ISA IRQs in south bridges Bernhard Beschow
                   ` (20 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Thie PIIX3 south bridge implements both the PIC and the ISA bus, so wiring the
interrupts there makes the device model more self-contained. Furthermore, this
allows the ISA interrupts to be wired to internal child devices in
pci_piix3_realize() which will be performed in subsequent patches.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index cd6c00c0b3..5988656279 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -293,6 +293,7 @@ static void pc_init1(MachineState *machine,
     } else {
         isa_bus = isa_bus_new(NULL, system_memory, system_io,
                               &error_abort);
+        isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
 
         rtc_state = isa_new(TYPE_MC146818_RTC);
         qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
@@ -301,7 +302,6 @@ static void pc_init1(MachineState *machine,
         i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
     }
-    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
         pc_i8259_create(isa_bus, gsi_state->i8259_irq);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 616f5418fa..3e7c42fa68 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -278,6 +278,8 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
+    isa_bus_register_input_irqs(isa_bus, d->isa_irqs_in);
+
     i8257_dma_init(isa_bus, 0);
 
     /* RTC */
-- 
2.42.0



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

* [PATCH v8 10/29] hw/i386/pc: Wire RTC ISA IRQs in south bridges
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (8 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 09/29] hw/isa/piix3: Wire PIC IRQs to ISA bus in host device Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 11/29] hw/isa/piix3: Create IDE controller in host device Bernhard Beschow
                   ` (19 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Makes the south bridges a bit more self-contained and aligns PIIX3 more with
PIIX4. The latter is needed for consolidating the PIIX south bridges.

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

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2fbdff89e0..4e844d02f2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1245,12 +1245,9 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         pit_isa_irq = -1;
         pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
         rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
+
+        /* overwrite connection created by south bridge */
         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),
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 3fcefc5a8a..23eba64f22 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -696,6 +696,7 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     ICH9LPCState *lpc = ICH9_LPC_DEVICE(d);
     PCIBus *pci_bus = pci_get_bus(d);
     ISABus *isa_bus;
+    uint32_t irq;
 
     if ((lpc->smi_host_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)) &&
         !(lpc->smi_host_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT))) {
@@ -745,6 +746,8 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     if (!qdev_realize(DEVICE(&lpc->rtc), BUS(isa_bus), errp)) {
         return;
     }
+    irq = object_property_get_uint(OBJECT(&lpc->rtc), "irq", &error_fatal);
+    isa_connect_gpio_out(ISA_DEVICE(&lpc->rtc), 0, irq);
 
     pci_bus_irqs(pci_bus, ich9_lpc_set_irq, d, ICH9_LPC_NB_PIRQS);
     pci_bus_map_irqs(pci_bus, ich9_lpc_map_irq);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 3e7c42fa68..11d72ca2bb 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -266,6 +266,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(dev);
     ISABus *isa_bus;
+    uint32_t irq;
 
     isa_bus = isa_bus_new(DEVICE(d), pci_address_space(dev),
                           pci_address_space_io(dev), errp);
@@ -287,6 +288,8 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
         return;
     }
+    irq = object_property_get_uint(OBJECT(&d->rtc), "irq", &error_fatal);
+    isa_connect_gpio_out(ISA_DEVICE(&d->rtc), 0, irq);
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
-- 
2.42.0



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

* [PATCH v8 11/29] hw/isa/piix3: Create IDE controller in host device
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (9 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 10/29] hw/i386/pc: Wire RTC ISA IRQs in south bridges Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 12/29] hw/isa/piix3: Create USB " Bernhard Beschow
                   ` (18 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

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

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index b07ff6bb26..1daeff397c 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -13,6 +13,7 @@
 #define HW_SOUTHBRIDGE_PIIX_H
 
 #include "hw/pci/pci_device.h"
+#include "hw/ide/pci.h"
 #include "hw/rtc/mc146818rtc.h"
 
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
@@ -52,6 +53,7 @@ struct PIIXState {
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
     MC146818RtcState rtc;
+    PCIIDEState ide;
 
     /* Reset Control Register contents */
     uint8_t rcr;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5988656279..c98a997482 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -43,7 +43,6 @@
 #include "net/net.h"
 #include "hw/ide/isa.h"
 #include "hw/ide/pci.h"
-#include "hw/ide/piix.h"
 #include "hw/irq.h"
 #include "sysemu/kvm.h"
 #include "hw/i386/kvm/clock.h"
@@ -290,6 +289,10 @@ static void pc_init1(MachineState *machine,
         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"));
+        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 {
         isa_bus = isa_bus_new(NULL, system_memory, system_io,
                               &error_abort);
@@ -301,6 +304,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) {
@@ -329,12 +334,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 11d72ca2bb..3f1dabade0 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 "sysemu/runstate.h"
 #include "migration/vmstate.h"
@@ -265,6 +266,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;
     uint32_t irq;
 
@@ -290,6 +292,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     }
     irq = object_property_get_uint(OBJECT(&d->rtc), "irq", &error_fatal);
     isa_connect_gpio_out(ISA_DEVICE(&d->rtc), 0, irq);
+
+    /* IDE */
+    qdev_prop_set_int32(DEVICE(&d->ide), "addr", dev->devfn + 1);
+    if (!qdev_realize(DEVICE(&d->ide), BUS(pci_bus), errp)) {
+        return;
+    }
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -321,6 +329,7 @@ static void pci_piix3_init(Object *obj)
                              ISA_NUM_IRQS);
 
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+    object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
 }
 
 static void pci_piix3_class_init(ObjectClass *klass, void *data)
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 9051083c1e..ade817f1b6 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -73,7 +73,6 @@ config I440FX
     select PC_ACPI
     select PCI_I440FX
     select PIIX3
-    select IDE_PIIX
     select DIMM
     select SMBIOS
     select FW_CFG_DMA
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index c10cbc5fc1..28345edbb3 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -34,6 +34,7 @@ config PC87312
 config PIIX3
     bool
     select I8257
+    select IDE_PIIX
     select ISA_BUS
     select MC146818RTC
 
-- 
2.42.0



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

* [PATCH v8 12/29] hw/isa/piix3: Create USB controller in host device
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (10 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 11/29] hw/isa/piix3: Create IDE controller in host device Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 13/29] hw/isa/piix3: Create power management " Bernhard Beschow
                   ` (17 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	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>
---
 include/hw/southbridge/piix.h |  4 ++++
 hw/i386/pc_piix.c             |  7 ++-----
 hw/isa/piix3.c                | 16 ++++++++++++++++
 hw/isa/Kconfig                |  1 +
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 1daeff397c..5cd866f1f2 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
 #include "hw/pci/pci_device.h"
 #include "hw/ide/pci.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 {
 
     MC146818RtcState rtc;
     PCIIDEState ide;
+    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 c98a997482..8dcd6851d0 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -51,7 +51,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"
@@ -265,6 +264,8 @@ static void pc_init1(MachineState *machine,
         size_t i;
 
         pci_dev = pci_new_multifunction(-1, TYPE_PIIX3_DEVICE);
+        object_property_set_bool(OBJECT(pci_dev), "has-usb",
+                                 machine_usb(machine), &error_abort);
         dev = DEVICE(pci_dev);
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
@@ -359,10 +360,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 3f1dabade0..aebc0da23b 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -298,6 +298,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     if (!qdev_realize(DEVICE(&d->ide), BUS(pci_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)
@@ -332,6 +342,11 @@ static void pci_piix3_init(Object *obj)
     object_initialize_child(obj, "ide", &d->ide, TYPE_PIIX3_IDE);
 }
 
+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);
@@ -352,6 +367,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 28345edbb3..1076df69ca 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -37,6 +37,7 @@ config PIIX3
     select IDE_PIIX
     select ISA_BUS
     select MC146818RTC
+    select USB_UHCI
 
 config PIIX4
     bool
-- 
2.42.0



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

* [PATCH v8 13/29] hw/isa/piix3: Create power management controller in host device
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (11 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 12/29] hw/isa/piix3: Create USB " Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 14/29] hw/isa/piix3: Drop the "3" from PIIX base class name Bernhard Beschow
                   ` (16 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	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>
---
 include/hw/southbridge/piix.h |  6 ++++++
 hw/i386/pc_piix.c             | 24 +++++++++++-------------
 hw/isa/piix3.c                | 15 +++++++++++++++
 hw/isa/Kconfig                |  1 +
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 5cd866f1f2..c56ce49fd3 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -13,6 +13,7 @@
 #define HW_SOUTHBRIDGE_PIIX_H
 
 #include "hw/pci/pci_device.h"
+#include "hw/acpi/piix4.h"
 #include "hw/ide/pci.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
@@ -56,6 +57,9 @@ struct PIIXState {
     MC146818RtcState rtc;
     PCIIDEState ide;
     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 8dcd6851d0..70cffcfe4f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -50,7 +50,6 @@
 #include "hw/i2c/smbus_eeprom.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"
@@ -115,7 +114,7 @@ static void pc_init1(MachineState *machine,
     MemoryRegion *system_io = get_system_io();
     PCIBus *pci_bus = NULL;
     ISABus *isa_bus;
-    int piix3_devfn = -1;
+    Object *piix4_pm = NULL;
     qemu_irq smi_irq;
     GSIState *gsi_state;
     BusState *idebus[MAX_IDE_BUS];
@@ -266,6 +265,13 @@ static void pc_init1(MachineState *machine,
         pci_dev = pci_new_multifunction(-1, TYPE_PIIX3_DEVICE);
         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);
         dev = DEVICE(pci_dev);
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
@@ -286,10 +292,10 @@ static void pc_init1(MachineState *machine,
                          XEN_IOAPIC_NUM_PIRQS);
         }
 
-        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");
@@ -360,17 +366,9 @@ 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);
         pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
         /* TODO: Populate SPD eeprom data.  */
@@ -382,7 +380,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 aebc0da23b..5b867df299 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -308,6 +308,18 @@ 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;
+        }
+        qdev_connect_gpio_out(DEVICE(&d->pm), 0, d->isa_irqs_in[9]);
+    }
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -343,7 +355,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 1076df69ca..17ddb25afc 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -33,6 +33,7 @@ config PC87312
 
 config PIIX3
     bool
+    select ACPI_PIIX4
     select I8257
     select IDE_PIIX
     select ISA_BUS
-- 
2.42.0



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

* [PATCH v8 14/29] hw/isa/piix3: Drop the "3" from PIIX base class name
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (12 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 13/29] hw/isa/piix3: Create power management " Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 15/29] hw/isa/piix4: Remove unused inbound ISA interrupt lines Bernhard Beschow
                   ` (15 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

TYPE_PIIX3_PCI_DEVICE was the former base class of the Xen and non-Xen variants
of the PIIX3 ISA device models. It will become the base class for the PIIX3 and
PIIX4 device models, so drop the "3" from the type names.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/southbridge/piix.h |  6 ++--
 hw/isa/piix3.c                | 56 +++++++++++++++++------------------
 2 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index c56ce49fd3..0b257e1582 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -71,11 +71,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_PIIX4_PCI_DEVICE "piix4-isa"
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 5b867df299..c7e59249b6 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -35,7 +35,7 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-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->isa_irqs_in[pic_irq],
                  !!(piix3->pic_levels &
@@ -43,7 +43,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;
@@ -58,7 +58,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;
 
@@ -74,13 +74,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;
 
@@ -95,7 +95,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;
@@ -111,7 +111,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));
@@ -124,7 +124,7 @@ static void piix3_write_config(PCIDevice *dev,
 
 static void piix3_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 */
@@ -165,7 +165,7 @@ static void piix3_reset(DeviceState *dev)
 
 static int piix3_post_load(void *opaque, int version_id)
 {
-    PIIX3State *piix3 = opaque;
+    PIIXState *piix3 = opaque;
     int pirq;
 
     /*
@@ -188,7 +188,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] =
@@ -200,7 +200,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);
 }
@@ -211,7 +211,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()
     }
 };
@@ -223,8 +223,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()
     },
@@ -237,7 +237,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);
@@ -248,7 +248,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;
 }
@@ -265,7 +265,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;
     uint32_t irq;
@@ -345,7 +345,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);
 
     qdev_init_gpio_out_named(DEVICE(obj), d->isa_irqs_in, "isa-irqs",
                              ISA_NUM_IRQS);
@@ -355,10 +355,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_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(),
 };
 
@@ -386,10 +386,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,
@@ -403,7 +403,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);
@@ -424,13 +424,13 @@ 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_register_types(void)
 {
-    type_register_static(&piix3_pci_type_info);
+    type_register_static(&piix_pci_type_info);
     type_register_static(&piix3_info);
 }
 
-- 
2.42.0



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

* [PATCH v8 15/29] hw/isa/piix4: Remove unused inbound ISA interrupt lines
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (13 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 14/29] hw/isa/piix3: Drop the "3" from PIIX base class name Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 16/29] hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in" Bernhard Beschow
                   ` (14 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	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>
---
 hw/isa/piix4.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index e0b149f8eb..3c3c7a094c 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -148,12 +148,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)
 {
@@ -197,8 +191,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.42.0



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

* [PATCH v8 16/29] hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in"
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (14 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 15/29] hw/isa/piix4: Remove unused inbound ISA interrupt lines Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 17/29] hw/isa/piix4: Rename reset control operations to match PIIX3 Bernhard Beschow
                   ` (13 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Rename the "isa" attribute to align it with PIIX3 for consolidation.

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

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 3c3c7a094c..9c8b6c98ab 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -45,7 +45,7 @@
 struct PIIX4State {
     PCIDevice dev;
     qemu_irq cpu_intr;
-    qemu_irq *isa;
+    qemu_irq *isa_irqs_in;
 
     MC146818RtcState rtc;
     PCIIDEState ide;
@@ -75,7 +75,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->isa_irqs_in[pic_irq], pic_level);
     }
 }
 
@@ -201,10 +201,10 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 
     /* initialize i8259 pic */
     i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    s->isa = i8259_init(isa_bus, *i8259_out_irq);
+    s->isa_irqs_in = i8259_init(isa_bus, *i8259_out_irq);
 
     /* initialize ISA irqs */
-    isa_bus_register_input_irqs(isa_bus, s->isa);
+    isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
 
     /* initialize pit */
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
@@ -236,7 +236,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
         return;
     }
-    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
+    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa_irqs_in[9]);
 
     pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
 }
-- 
2.42.0



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

* [PATCH v8 17/29] hw/isa/piix4: Rename reset control operations to match PIIX3
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (15 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 16/29] hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in" Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 18/29] hw/isa/piix4: Reuse struct PIIXState from PIIX3 Bernhard Beschow
                   ` (12 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	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>
---
 hw/isa/piix4.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9c8b6c98ab..eb456622c5 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -148,8 +148,8 @@ static void piix4_request_i8259_irq(void *opaque, int irq, int level)
     qemu_set_irq(s->cpu_intr, level);
 }
 
-static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
-                            unsigned int len)
+static void rcr_write(void *opaque, hwaddr addr, uint64_t val,
+                      unsigned int len)
 {
     PIIX4State *s = opaque;
 
@@ -161,16 +161,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)
 {
     PIIX4State *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,
@@ -194,7 +194,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     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,
+    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.42.0



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

* [PATCH v8 18/29] hw/isa/piix4: Reuse struct PIIXState from PIIX3
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (16 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 17/29] hw/isa/piix4: Rename reset control operations to match PIIX3 Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 19/29] hw/isa/piix3: Merge hw/isa/piix4.c Bernhard Beschow
                   ` (11 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

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

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0b257e1582..dd5f7b31c0 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -49,6 +49,7 @@ struct PIIXState {
 #endif
     uint64_t pic_levels;
 
+    qemu_irq cpu_intr;
     qemu_irq isa_irqs_in[ISA_NUM_IRQS];
 
     /* This member isn't used. Just for save/load compatibility */
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index eb456622c5..71899aaa69 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -42,21 +42,9 @@
 #include "sysemu/runstate.h"
 #include "qom/object.h"
 
-struct PIIX4State {
-    PCIDevice dev;
-    qemu_irq cpu_intr;
-    qemu_irq *isa_irqs_in;
-
-    MC146818RtcState rtc;
-    PCIIDEState ide;
-    UHCIState uhci;
-    PIIX4PMState pm;
-    /* Reset Control Register */
-    MemoryRegion rcr_mem;
-    uint8_t rcr;
-};
+typedef struct PIIXState PIIX4State;
 
-OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
+DECLARE_INSTANCE_CHECKER(PIIX4State, PIIX4_PCI_DEVICE, TYPE_PIIX4_PCI_DEVICE)
 
 static void piix4_set_irq(void *opaque, int irq_num, int level)
 {
@@ -184,6 +172,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
     qemu_irq *i8259_out_irq;
+    qemu_irq *i8259;
+    size_t i;
 
     isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
                           pci_address_space_io(dev), errp);
@@ -201,7 +191,13 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 
     /* initialize i8259 pic */
     i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    s->isa_irqs_in = i8259_init(isa_bus, *i8259_out_irq);
+    i8259 = i8259_init(isa_bus, *i8259_out_irq);
+
+    for (i = 0; i < ISA_NUM_IRQS; i++) {
+        s->isa_irqs_in[i] = i8259[i];
+    }
+
+    g_free(i8259);
 
     /* initialize ISA irqs */
     isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
-- 
2.42.0



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

* [PATCH v8 19/29] hw/isa/piix3: Merge hw/isa/piix4.c
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (17 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 18/29] hw/isa/piix4: Reuse struct PIIXState from PIIX3 Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 20/29] hw/isa/piix: Allow for optional PIC creation in PIIX3 Bernhard Beschow
                   ` (10 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Now that the PIIX3 and PIIX4 device models are sufficiently prepared, 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>
---
 MAINTAINERS                |   6 +-
 hw/isa/{piix3.c => piix.c} | 190 +++++++++++++++++++++++-
 hw/isa/piix4.c             | 290 -------------------------------------
 hw/i386/Kconfig            |   2 +-
 hw/isa/Kconfig             |  11 +-
 hw/isa/meson.build         |   3 +-
 hw/mips/Kconfig            |   2 +-
 7 files changed, 195 insertions(+), 309 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (71%)
 delete mode 100644 hw/isa/piix4.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ea91f9e804..6fb609f624 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1287,7 +1287,7 @@ Malta
 M: Philippe Mathieu-Daudé <philmd@linaro.org>
 R: Aurelien Jarno <aurelien@aurel32.net>
 S: Odd Fixes
-F: hw/isa/piix4.c
+F: hw/isa/piix.c
 F: hw/acpi/piix4.c
 F: hw/mips/malta.c
 F: hw/pci-host/gt64120.c
@@ -1706,7 +1706,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
@@ -2459,7 +2459,7 @@ PIIX4 South Bridge (i82371AB)
 M: Hervé Poussineau <hpoussin@reactos.org>
 M: Philippe Mathieu-Daudé <philmd@linaro.org>
 S: Maintained
-F: hw/isa/piix4.c
+F: hw/isa/piix.c
 F: include/hw/southbridge/piix.h
 
 Firmware configuration (fw_cfg)
diff --git a/hw/isa/piix3.c b/hw/isa/piix.c
similarity index 71%
rename from hw/isa/piix3.c
rename to hw/isa/piix.c
index c7e59249b6..f6da334c6f 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,14 +28,20 @@
 #include "qapi/error.h"
 #include "hw/dma/i8257.h"
 #include "hw/southbridge/piix.h"
+#include "hw/timer/i8254.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/ide/piix.h"
+#include "hw/intc/i8259.h"
 #include "hw/isa/isa.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
+typedef struct PIIXState PIIX4State;
+
+DECLARE_INSTANCE_CHECKER(PIIX4State, PIIX4_PCI_DEVICE, TYPE_PIIX4_PCI_DEVICE)
+
 static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
 {
     qemu_set_irq(piix3->isa_irqs_in[pic_irq],
@@ -78,6 +85,33 @@ 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;
+    PIIX4State *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->isa_irqs_in[pic_irq], pic_level);
+    }
+}
+
+static void piix4_request_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIX4State *s = opaque;
+    qemu_set_irq(s->cpu_intr, level);
+}
+
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
     PIIXState *piix3 = opaque;
@@ -122,9 +156,8 @@ static void piix3_write_config(PCIDevice *dev,
     }
 }
 
-static void piix3_reset(DeviceState *dev)
+static void piix_reset(PIIXState *d)
 {
-    PIIXState *d = PIIX_PCI_DEVICE(dev);
     uint8_t *pci_conf = d->dev.config;
 
     pci_conf[0x04] = 0x07; /* master, memory and I/O */
@@ -163,6 +196,13 @@ static void piix3_reset(DeviceState *dev)
     d->rcr = 0;
 }
 
+static void piix3_reset(DeviceState *dev)
+{
+    PIIXState *d = PIIX_PCI_DEVICE(dev);
+
+    piix_reset(d);
+}
+
 static int piix3_post_load(void *opaque, int version_id)
 {
     PIIXState *piix3 = opaque;
@@ -185,6 +225,17 @@ static int piix3_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static int piix4_post_load(void *opaque, int version_id)
+{
+    PIIX4State *s = opaque;
+
+    if (version_id == 2) {
+        s->rcr = 0;
+    }
+
+    return 0;
+}
+
 static int piix3_pre_save(void *opaque)
 {
     int i;
@@ -234,6 +285,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, PIIX4State),
+        VMSTATE_UINT8_V(rcr, PIIX4State, 3),
+        VMSTATE_END_OF_LIST()
+    }
+};
 
 static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
 {
@@ -428,10 +490,134 @@ static const TypeInfo piix3_info = {
     .class_init    = piix3_class_init,
 };
 
+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;
+    qemu_irq *i8259;
+    size_t i;
+
+    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
+                          pci_address_space_io(dev), errp);
+    if (!isa_bus) {
+        return;
+    }
+
+    qdev_init_gpio_out_named(DEVICE(dev), &s->cpu_intr,
+                             "intr", 1);
+
+    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
+                          "reset-control", 1);
+    memory_region_add_subregion_overlap(pci_address_space_io(dev),
+                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
+
+    /* initialize i8259 pic */
+    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
+    i8259 = i8259_init(isa_bus, *i8259_out_irq);
+
+    for (i = 0; i < ISA_NUM_IRQS; i++) {
+        s->isa_irqs_in[i] = i8259[i];
+    }
+
+    g_free(i8259);
+
+    /* initialize ISA irqs */
+    isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
+
+    /* initialize pit */
+    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+
+    /* DMA */
+    i8257_dma_init(isa_bus, 0);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
+    s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq);
+
+    /* IDE */
+    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
+    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
+        return;
+    }
+
+    /* USB */
+    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;
+    }
+    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa_irqs_in[9]);
+
+    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+}
+
+static void piix4_isa_reset(DeviceState *dev)
+{
+    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
+
+    piix_reset(s);
+}
+
+static void piix4_init(Object *obj)
+{
+    PIIX4State *s = PIIX4_PCI_DEVICE(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 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;
+}
+
+static const TypeInfo piix4_info = {
+    .name          = TYPE_PIIX4_PCI_DEVICE,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PIIX4State),
+    .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(&piix4_info);
 }
 
 type_init(piix3_register_types)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
deleted file mode 100644
index 71899aaa69..0000000000
--- a/hw/isa/piix4.c
+++ /dev/null
@@ -1,290 +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"
-
-typedef struct PIIXState PIIX4State;
-
-DECLARE_INSTANCE_CHECKER(PIIX4State, PIIX4_PCI_DEVICE, TYPE_PIIX4_PCI_DEVICE)
-
-static void piix4_set_irq(void *opaque, int irq_num, int level)
-{
-    int i, pic_irq, pic_level;
-    PIIX4State *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->isa_irqs_in[pic_irq], pic_level);
-    }
-}
-
-static void piix4_isa_reset(DeviceState *dev)
-{
-    PIIX4State *d = PIIX4_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->rcr = 0;
-}
-
-static int piix4_post_load(void *opaque, int version_id)
-{
-    PIIX4State *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, PIIX4State),
-        VMSTATE_UINT8_V(rcr, PIIX4State, 3),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static void piix4_request_i8259_irq(void *opaque, int irq, int level)
-{
-    PIIX4State *s = opaque;
-    qemu_set_irq(s->cpu_intr, level);
-}
-
-static void rcr_write(void *opaque, hwaddr addr, uint64_t val,
-                      unsigned int len)
-{
-    PIIX4State *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)
-{
-    PIIX4State *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)
-{
-    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
-    PCIBus *pci_bus = pci_get_bus(dev);
-    ISABus *isa_bus;
-    qemu_irq *i8259_out_irq;
-    qemu_irq *i8259;
-    size_t i;
-
-    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
-                          pci_address_space_io(dev), errp);
-    if (!isa_bus) {
-        return;
-    }
-
-    qdev_init_gpio_out_named(DEVICE(dev), &s->cpu_intr,
-                             "intr", 1);
-
-    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
-                          "reset-control", 1);
-    memory_region_add_subregion_overlap(pci_address_space_io(dev),
-                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
-
-    /* initialize i8259 pic */
-    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-    i8259 = i8259_init(isa_bus, *i8259_out_irq);
-
-    for (i = 0; i < ISA_NUM_IRQS; i++) {
-        s->isa_irqs_in[i] = i8259[i];
-    }
-
-    g_free(i8259);
-
-    /* initialize ISA irqs */
-    isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
-
-    /* initialize pit */
-    i8254_pit_init(isa_bus, 0x40, 0, NULL);
-
-    /* DMA */
-    i8257_dma_init(isa_bus, 0);
-
-    /* RTC */
-    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
-    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
-        return;
-    }
-    s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq);
-
-    /* IDE */
-    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
-    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
-        return;
-    }
-
-    /* USB */
-    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;
-    }
-    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa_irqs_in[9]);
-
-    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
-}
-
-static void piix4_init(Object *obj)
-{
-    PIIX4State *s = PIIX4_PCI_DEVICE(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 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;
-}
-
-static const TypeInfo piix4_info = {
-    .name          = TYPE_PIIX4_PCI_DEVICE,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIX4State),
-    .instance_init = piix4_init,
-    .class_init    = piix4_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
-static void piix4_register_types(void)
-{
-    type_register_static(&piix4_info);
-}
-
-type_init(piix4_register_types)
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index ade817f1b6..94772c726b 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -72,7 +72,7 @@ config I440FX
     select PC_PCI
     select PC_ACPI
     select PCI_I440FX
-    select PIIX3
+    select PIIX
     select DIMM
     select SMBIOS
     select FW_CFG_DMA
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 17ddb25afc..040a18c070 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -31,16 +31,7 @@ config PC87312
     select FDC_ISA
     select IDE_ISA
 
-config PIIX3
-    bool
-    select ACPI_PIIX4
-    select I8257
-    select IDE_PIIX
-    select ISA_BUS
-    select MC146818RTC
-    select USB_UHCI
-
-config PIIX4
+config PIIX
     bool
     # For historical reasons, SuperIO devices are created in the board
     # for PIIX4.
diff --git a/hw/isa/meson.build b/hw/isa/meson.build
index b855e81276..2ab99ce0c6 100644
--- a/hw/isa/meson.build
+++ b/hw/isa/meson.build
@@ -3,8 +3,7 @@ system_ss.add(when: 'CONFIG_I82378', if_true: files('i82378.c'))
 system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
 system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
 system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
-system_ss.add(when: 'CONFIG_PIIX3', if_true: files('piix3.c'))
-system_ss.add(when: 'CONFIG_PIIX4', if_true: files('piix4.c'))
+system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
 system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
 system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
 
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index da3a37e215..ac1eb06a51 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -2,7 +2,7 @@ config MALTA
     bool
     select GT64120
     select ISA_SUPERIO
-    select PIIX4
+    select PIIX
 
 config MIPSSIM
     bool
-- 
2.42.0



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

* [PATCH v8 20/29] hw/isa/piix: Allow for optional PIC creation in PIIX3
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (18 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 19/29] hw/isa/piix3: Merge hw/isa/piix4.c Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 21/29] hw/isa/piix: Allow for optional PIT " Bernhard Beschow
                   ` (9 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

In the PC machine, the PIC is created in board code to allow it to be
virtualized with various virtualization techniques. So explicitly disable its
creation in the PC machine via a property which defaults to enabled. Once the
PIIX implementations are consolidated this default will keep Malta working
without further ado.

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index dd5f7b31c0..08491693b4 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -69,6 +69,7 @@ struct PIIXState {
     MemoryRegion rcr_mem;
 
     bool has_acpi;
+    bool has_pic;
     bool has_usb;
     bool smm_enabled;
 };
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 70cffcfe4f..fa39afd891 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -268,6 +268,8 @@ static void pc_init1(MachineState *machine,
         object_property_set_bool(OBJECT(pci_dev), "has-acpi",
                                  x86_machine_is_acpi_enabled(x86ms),
                                  &error_abort);
+        object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
+                                 &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),
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index f6da334c6f..d6d9ac6473 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -106,7 +106,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int level)
     }
 }
 
-static void piix4_request_i8259_irq(void *opaque, int irq, int level)
+static void piix_request_i8259_irq(void *opaque, int irq, int level)
 {
     PIIX4State *s = opaque;
     qemu_set_irq(s->cpu_intr, level);
@@ -343,6 +343,22 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
+    /* PIC */
+    if (d->has_pic) {
+        qemu_irq *i8259_out_irq = qemu_allocate_irqs(piix_request_i8259_irq, d,
+                                                     1);
+        qemu_irq *i8259 = i8259_init(isa_bus, *i8259_out_irq);
+        size_t i;
+
+        for (i = 0; i < ISA_NUM_IRQS; i++) {
+            d->isa_irqs_in[i] = i8259[i];
+        }
+
+        g_free(i8259);
+
+        qdev_init_gpio_out_named(DEVICE(dev), &d->cpu_intr, "intr", 1);
+    }
+
     isa_bus_register_input_irqs(isa_bus, d->isa_irqs_in);
 
     i8257_dma_init(isa_bus, 0);
@@ -419,6 +435,7 @@ static void pci_piix3_init(Object *obj)
 static Property pci_piix3_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-pic", PIIXState, has_pic, true),
     DEFINE_PROP_BOOL("has-usb", PIIXState, has_usb, true),
     DEFINE_PROP_BOOL("smm-enabled", PIIXState, smm_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
@@ -514,7 +531,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
                                         PIIX_RCR_IOPORT, &s->rcr_mem, 1);
 
     /* initialize i8259 pic */
-    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
+    i8259_out_irq = qemu_allocate_irqs(piix_request_i8259_irq, s, 1);
     i8259 = i8259_init(isa_bus, *i8259_out_irq);
 
     for (i = 0; i < ISA_NUM_IRQS; i++) {
-- 
2.42.0



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

* [PATCH v8 21/29] hw/isa/piix: Allow for optional PIT creation in PIIX3
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (19 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 20/29] hw/isa/piix: Allow for optional PIC creation in PIIX3 Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 22/29] hw/isa/piix: Harmonize names of reset control memory regions Bernhard Beschow
                   ` (8 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

In the PC machine, the PIT is created in board code to allow it to be
virtualized with various virtualization techniques. So explicitly disable its
creation in the PC machine via a property which defaults to enabled. Once the
PIIX implementations are consolidated this default will keep Malta working
without further ado.

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

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 08491693b4..86709ba2e4 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -70,6 +70,7 @@ struct PIIXState {
 
     bool has_acpi;
     bool has_pic;
+    bool has_pit;
     bool has_usb;
     bool smm_enabled;
 };
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fa39afd891..e38942a3c3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -270,6 +270,8 @@ static void pc_init1(MachineState *machine,
                                  &error_abort);
         object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
                                  &error_abort);
+        object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
+                                 &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),
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index d6d9ac6473..270b8eb1f7 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -361,6 +361,11 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 
     isa_bus_register_input_irqs(isa_bus, d->isa_irqs_in);
 
+    /* PIT */
+    if (d->has_pit) {
+        i8254_pit_init(isa_bus, 0x40, 0, NULL);
+    }
+
     i8257_dma_init(isa_bus, 0);
 
     /* RTC */
@@ -436,6 +441,7 @@ static Property pci_piix3_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-pic", PIIXState, has_pic, true),
+    DEFINE_PROP_BOOL("has-pit", PIIXState, has_pit, true),
     DEFINE_PROP_BOOL("has-usb", PIIXState, has_usb, true),
     DEFINE_PROP_BOOL("smm-enabled", PIIXState, smm_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
-- 
2.42.0



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

* [PATCH v8 22/29] hw/isa/piix: Harmonize names of reset control memory regions
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (20 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 21/29] hw/isa/piix: Allow for optional PIT " Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 23/29] hw/isa/piix: Share PIIX3's base class with PIIX4 Bernhard Beschow
                   ` (7 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	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>
---
 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 270b8eb1f7..bd66fb7475 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -339,7 +339,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
     }
 
     memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
-                          "piix3-reset-control", 1);
+                          "piix-reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(dev),
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
@@ -532,7 +532,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
                              "intr", 1);
 
     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.42.0



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

* [PATCH v8 23/29] hw/isa/piix: Share PIIX3's base class with PIIX4
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (21 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 22/29] hw/isa/piix: Harmonize names of reset control memory regions Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 24/29] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 Bernhard Beschow
                   ` (6 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Having a common base class will allow for futher code sharing between PIIX3 and
PIIX4. 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>
---
 hw/isa/piix.c | 85 ++++++++++++++++++---------------------------------
 1 file changed, 30 insertions(+), 55 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index bd66fb7475..8f7d6c56a8 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -38,10 +38,6 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-typedef struct PIIXState PIIX4State;
-
-DECLARE_INSTANCE_CHECKER(PIIX4State, PIIX4_PCI_DEVICE, TYPE_PIIX4_PCI_DEVICE)
-
 static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
 {
     qemu_set_irq(piix3->isa_irqs_in[pic_irq],
@@ -88,7 +84,7 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
 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 */
@@ -108,7 +104,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int level)
 
 static void piix_request_i8259_irq(void *opaque, int irq, int level)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
     qemu_set_irq(s->cpu_intr, level);
 }
 
@@ -156,8 +152,9 @@ static void piix3_write_config(PCIDevice *dev,
     }
 }
 
-static void piix_reset(PIIXState *d)
+static void piix_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 */
@@ -196,13 +193,6 @@ static void piix_reset(PIIXState *d)
     d->rcr = 0;
 }
 
-static void piix3_reset(DeviceState *dev)
-{
-    PIIXState *d = PIIX_PCI_DEVICE(dev);
-
-    piix_reset(d);
-}
-
 static int piix3_post_load(void *opaque, int version_id)
 {
     PIIXState *piix3 = opaque;
@@ -227,7 +217,7 @@ static int piix3_post_load(void *opaque, int version_id)
 
 static int piix4_post_load(void *opaque, int version_id)
 {
-    PIIX4State *s = opaque;
+    PIIXState *s = opaque;
 
     if (version_id == 2) {
         s->rcr = 0;
@@ -291,8 +281,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()
     }
 };
@@ -426,7 +416,7 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     qbus_build_aml(bus, scope);
 }
 
-static void pci_piix3_init(Object *obj)
+static void pci_piix_init(Object *obj)
 {
     PIIXState *d = PIIX_PCI_DEVICE(obj);
 
@@ -434,7 +424,6 @@ static void pci_piix3_init(Object *obj)
                              ISA_NUM_IRQS);
 
     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[] = {
@@ -447,27 +436,22 @@ static Property pci_piix3_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);
     AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
-    k->config_write = piix3_write_config;
-    dc->reset       = piix3_reset;
+    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
+     * Reason: part of PIIX southbridge, needs to be wired up by e.g.
      * 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;
 }
 
@@ -475,9 +459,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 },
@@ -500,22 +484,36 @@ 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 = piix3_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;
+    device_class_set_props(dc, pci_piix3_props);
 }
 
 static const TypeInfo piix3_info = {
     .name          = TYPE_PIIX3_DEVICE,
     .parent        = TYPE_PIIX_PCI_DEVICE,
+    .instance_init = piix3_init,
     .class_init    = piix3_class_init,
 };
 
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
-    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
+    PIIXState *s = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
     ISABus *isa_bus;
     qemu_irq *i8259_out_irq;
@@ -584,18 +582,10 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
 }
 
-static void piix4_isa_reset(DeviceState *dev)
-{
-    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
-
-    piix_reset(s);
-}
-
 static void piix4_init(Object *obj)
 {
-    PIIX4State *s = PIIX4_PCI_DEVICE(obj);
+    PIIXState *s = PIIX_PCI_DEVICE(obj);
 
-    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
     object_initialize_child(obj, "uhci", &s->uhci, TYPE_PIIX4_USB_UHCI);
 
@@ -610,30 +600,15 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     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;
 }
 
 static const TypeInfo piix4_info = {
     .name          = TYPE_PIIX4_PCI_DEVICE,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PIIX4State),
+    .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.42.0



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

* [PATCH v8 24/29] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (22 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 23/29] hw/isa/piix: Share PIIX3's base class with PIIX4 Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 25/29] hw/isa/piix: Rename functions to be shared for PCI interrupt triggering Bernhard Beschow
                   ` (5 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Resolves duplicate code. Also makes PIIX4 respect the PIIX3 properties which get
added, too. This allows for using PIIX4 in the PC machine.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c   | 80 ++++++-------------------------------------------
 hw/mips/malta.c |  5 ++--
 2 files changed, 12 insertions(+), 73 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 8f7d6c56a8..2ab799b95e 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -315,7 +315,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);
@@ -374,8 +375,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;
@@ -426,7 +426,7 @@ static void pci_piix_init(Object *obj)
     object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
 }
 
-static Property pci_piix3_props[] = {
+static Property pci_piix_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-pic", PIIXState, has_pic, true),
@@ -452,6 +452,7 @@ static void pci_piix_class_init(ObjectClass *klass, void *data)
      * pc_piix.c's pc_init1()
      */
     dc->user_creatable = false;
+    device_class_set_props(dc, pci_piix_props);
     adevc->build_dev_aml = build_pci_isa_aml;
 }
 
@@ -475,7 +476,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;
     }
@@ -501,7 +502,6 @@ static void piix3_class_init(ObjectClass *klass, void *data)
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
     dc->vmsd = &vmstate_piix3;
-    device_class_set_props(dc, pci_piix3_props);
 }
 
 static const TypeInfo piix3_info = {
@@ -513,71 +513,14 @@ static const TypeInfo piix3_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;
-    qemu_irq *i8259_out_irq;
-    qemu_irq *i8259;
-    size_t i;
-
-    isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
-                          pci_address_space_io(dev), errp);
-    if (!isa_bus) {
-        return;
-    }
-
-    qdev_init_gpio_out_named(DEVICE(dev), &s->cpu_intr,
-                             "intr", 1);
-
-    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &rcr_ops, s,
-                          "piix-reset-control", 1);
-    memory_region_add_subregion_overlap(pci_address_space_io(dev),
-                                        PIIX_RCR_IOPORT, &s->rcr_mem, 1);
-
-    /* initialize i8259 pic */
-    i8259_out_irq = qemu_allocate_irqs(piix_request_i8259_irq, s, 1);
-    i8259 = i8259_init(isa_bus, *i8259_out_irq);
-
-    for (i = 0; i < ISA_NUM_IRQS; i++) {
-        s->isa_irqs_in[i] = i8259[i];
-    }
-
-    g_free(i8259);
-
-    /* initialize ISA irqs */
-    isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
-
-    /* initialize pit */
-    i8254_pit_init(isa_bus, 0x40, 0, NULL);
 
-    /* DMA */
-    i8257_dma_init(isa_bus, 0);
-
-    /* RTC */
-    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
-    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
-        return;
-    }
-    s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq);
-
-    /* IDE */
-    qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
-    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
-        return;
-    }
-
-    /* USB */
-    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)) {
+    pci_piix_realize(dev, TYPE_PIIX4_USB_UHCI, errp);
+    if (*errp) {
         return;
     }
-    qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa_irqs_in[9]);
 
     pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
 }
@@ -587,11 +530,6 @@ static void piix4_init(Object *obj)
     PIIXState *s = PIIX_PCI_DEVICE(obj);
 
     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 void piix4_class_init(ObjectClass *klass, void *data)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index dac27fad9d..155f3c1cc8 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1238,8 +1238,9 @@ void mips_malta_init(MachineState *machine)
     pci_bus_map_irqs(pci_bus, malta_pci_slot_get_pirq);
 
     /* Southbridge */
-    piix4 = pci_create_simple_multifunction(pci_bus, PIIX4_PCI_DEVFN,
-                                            TYPE_PIIX4_PCI_DEVICE);
+    piix4 = pci_new_multifunction(PIIX4_PCI_DEVFN, 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.42.0



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

* [PATCH v8 25/29] hw/isa/piix: Rename functions to be shared for PCI interrupt triggering
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (23 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 24/29] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 26/29] hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4 Bernhard Beschow
                   ` (4 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	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>
---
 hw/isa/piix.c | 72 +++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 2ab799b95e..449c1baaab 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -38,47 +38,47 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
+static void piix_set_irq_pic(PIIXState *s, int pic_irq)
 {
-    qemu_set_irq(piix3->isa_irqs_in[pic_irq],
-                 !!(piix3->pic_levels &
+    qemu_set_irq(s->isa_irqs_in[pic_irq],
+                 !!(s->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_pci_irq_level_internal(PIIXState *s, int pirq, int level)
 {
     int pic_irq;
     uint64_t mask;
 
-    pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+    pic_irq = s->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;
+    s->pic_levels &= ~mask;
+    s->pic_levels |= mask * !!level;
 }
 
-static void piix3_set_irq_level(PIIXState *piix3, int pirq, int level)
+static void piix_set_pci_irq_level(PIIXState *s, int pirq, int level)
 {
     int pic_irq;
 
-    pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+    pic_irq = s->dev.config[PIIX_PIRQCA + pirq];
     if (pic_irq >= ISA_NUM_IRQS) {
         return;
     }
 
-    piix3_set_irq_level_internal(piix3, pirq, level);
+    piix_set_pci_irq_level_internal(s, pirq, level);
 
-    piix3_set_irq_pic(piix3, pic_irq);
+    piix_set_irq_pic(s, pic_irq);
 }
 
-static void piix3_set_irq(void *opaque, int pirq, int level)
+static void piix_set_pci_irq(void *opaque, int pirq, int level)
 {
-    PIIXState *piix3 = opaque;
-    piix3_set_irq_level(piix3, pirq, level);
+    PIIXState *s = opaque;
+    piix_set_pci_irq_level(s, pirq, level);
 }
 
 static void piix4_set_irq(void *opaque, int irq_num, int level)
@@ -108,10 +108,10 @@ static void piix_request_i8259_irq(void *opaque, int irq, int level)
     qemu_set_irq(s->cpu_intr, level);
 }
 
-static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
+static PCIINTxRoute piix_route_intx_pin_to_irq(void *opaque, int pin)
 {
-    PIIXState *piix3 = opaque;
-    int irq = piix3->dev.config[PIIX_PIRQCA + pin];
+    PCIDevice *pci_dev = opaque;
+    int irq = pci_dev->config[PIIX_PIRQCA + pin];
     PCIINTxRoute route;
 
     if (irq < ISA_NUM_IRQS) {
@@ -125,29 +125,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_pci_irq_levels(PIIXState *s)
 {
-    PCIBus *bus = pci_get_bus(&piix3->dev);
+    PCIBus *bus = pci_get_bus(&s->dev);
     int pirq;
 
-    piix3->pic_levels = 0;
+    s->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_pci_irq_level(s, 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 *s = 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(&s->dev));
+        piix_update_pci_irq_levels(s);
         for (pic_irq = 0; pic_irq < ISA_NUM_IRQS; pic_irq++) {
-            piix3_set_irq_pic(piix3, pic_irq);
+            piix_set_irq_pic(s, pic_irq);
         }
     }
 }
@@ -193,9 +193,9 @@ static void piix_reset(DeviceState *dev)
     d->rcr = 0;
 }
 
-static int piix3_post_load(void *opaque, int version_id)
+static int piix_post_load(void *opaque, int version_id)
 {
-    PIIXState *piix3 = opaque;
+    PIIXState *s = opaque;
     int pirq;
 
     /*
@@ -207,10 +207,10 @@ static int piix3_post_load(void *opaque, int version_id)
      * Here, we update irq levels without raising the interrupt.
      * Interrupt state will be deserialized separately through the i8259.
      */
-    piix3->pic_levels = 0;
+    s->pic_levels = 0;
     for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
-        piix3_set_irq_level_internal(piix3, pirq,
-            pci_bus_get_irq_level(pci_get_bus(&piix3->dev), pirq));
+        piix_set_pci_irq_level_internal(s, pirq,
+            pci_bus_get_irq_level(pci_get_bus(&s->dev), pirq));
     }
     return 0;
 }
@@ -261,7 +261,7 @@ static const VMStateDescription vmstate_piix3 = {
     .name = "PIIX3",
     .version_id = 3,
     .minimum_version_id = 2,
-    .post_load = piix3_post_load,
+    .post_load = piix_post_load,
     .pre_save = piix3_pre_save,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(dev, PIIXState),
@@ -481,8 +481,8 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pci_bus_irqs(pci_bus, piix3_set_irq, piix3, PIIX_NUM_PIRQS);
-    pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
+    pci_bus_irqs(pci_bus, piix_set_pci_irq, piix3, PIIX_NUM_PIRQS);
+    pci_bus_set_route_irq_fn(pci_bus, piix_route_intx_pin_to_irq);
 }
 
 static void piix3_init(Object *obj)
@@ -497,7 +497,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix3_write_config;
+    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;
-- 
2.42.0



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

* [PATCH v8 26/29] hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (24 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 25/29] hw/isa/piix: Rename functions to be shared for PCI interrupt triggering Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 27/29] hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring Bernhard Beschow
                   ` (3 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Speeds up PIIX4 which resolves an old TODO. Also makes PIIX4 compatible with Xen
which relies on pci_bus_fire_intx_routing_notifier() to be fired.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/isa/piix.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 449c1baaab..17677c2126 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -81,27 +81,6 @@ static void piix_set_pci_irq(void *opaque, int pirq, int level)
     piix_set_pci_irq_level(s, 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->isa_irqs_in[pic_irq], pic_level);
-    }
-}
-
 static void piix_request_i8259_irq(void *opaque, int irq, int level)
 {
     PIIXState *s = opaque;
@@ -223,7 +202,7 @@ static int piix4_post_load(void *opaque, int version_id)
         s->rcr = 0;
     }
 
-    return 0;
+    return piix_post_load(opaque, version_id);
 }
 
 static int piix3_pre_save(void *opaque)
@@ -442,6 +421,7 @@ static void pci_piix_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
+    k->config_write = piix_write_config;
     dc->reset       = piix_reset;
     dc->desc        = "ISA bridge";
     dc->hotpluggable   = false;
@@ -497,7 +477,6 @@ 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;
@@ -522,7 +501,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, piix_set_pci_irq, s, PIIX_NUM_PIRQS);
 }
 
 static void piix4_init(Object *obj)
-- 
2.42.0



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

* [PATCH v8 27/29] hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (25 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 26/29] hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4 Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 28/29] hw/isa/piix: Implement multi-process QEMU support also for PIIX4 Bernhard Beschow
                   ` (2 subsequent siblings)
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Now that both PIIX3 and PIIX4 use piix_set_irq() to trigger PCI IRQs the wiring
in the respective realize methods can be shared, too.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/isa/piix.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 17677c2126..cba2098ca2 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -372,6 +372,8 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
         }
         qdev_connect_gpio_out(DEVICE(&d->pm), 0, d->isa_irqs_in[9]);
     }
+
+    pci_bus_irqs(pci_bus, piix_set_pci_irq, d, PIIX_NUM_PIRQS);
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -453,7 +455,6 @@ static const TypeInfo piix_pci_type_info = {
 static void piix3_realize(PCIDevice *dev, Error **errp)
 {
     ERRP_GUARD();
-    PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
     pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
@@ -461,7 +462,6 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pci_bus_irqs(pci_bus, piix_set_pci_irq, piix3, PIIX_NUM_PIRQS);
     pci_bus_set_route_irq_fn(pci_bus, piix_route_intx_pin_to_irq);
 }
 
@@ -492,16 +492,7 @@ static const TypeInfo piix3_info = {
 
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
-    ERRP_GUARD();
-    PIIXState *s = PIIX_PCI_DEVICE(dev);
-    PCIBus *pci_bus = pci_get_bus(dev);
-
     pci_piix_realize(dev, TYPE_PIIX4_USB_UHCI, errp);
-    if (*errp) {
-        return;
-    }
-
-    pci_bus_irqs(pci_bus, piix_set_pci_irq, s, PIIX_NUM_PIRQS);
 }
 
 static void piix4_init(Object *obj)
-- 
2.42.0



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

* [PATCH v8 28/29] hw/isa/piix: Implement multi-process QEMU support also for PIIX4
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (26 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 27/29] hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-07 12:38 ` [PATCH v8 29/29] hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine Bernhard Beschow
  2023-10-08 17:56 ` [PATCH v8 00/29] Consolidate PIIX south bridges Chuck Zmudzinski
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

So far multi-process QEMU was only implemented for PIIX3. Move the support into
the base class to achieve feature parity between both device models.

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

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index cba2098ca2..04ebed5b52 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -374,6 +374,7 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
     }
 
     pci_bus_irqs(pci_bus, piix_set_pci_irq, d, PIIX_NUM_PIRQS);
+    pci_bus_set_route_irq_fn(pci_bus, piix_route_intx_pin_to_irq);
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -454,15 +455,7 @@ static const TypeInfo piix_pci_type_info = {
 
 static void piix3_realize(PCIDevice *dev, Error **errp)
 {
-    ERRP_GUARD();
-    PCIBus *pci_bus = pci_get_bus(dev);
-
     pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
-    if (*errp) {
-        return;
-    }
-
-    pci_bus_set_route_irq_fn(pci_bus, piix_route_intx_pin_to_irq);
 }
 
 static void piix3_init(Object *obj)
-- 
2.42.0



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

* [PATCH v8 29/29] hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (27 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 28/29] hw/isa/piix: Implement multi-process QEMU support also for PIIX4 Bernhard Beschow
@ 2023-10-07 12:38 ` Bernhard Beschow
  2023-10-08 17:56 ` [PATCH v8 00/29] Consolidate PIIX south bridges Chuck Zmudzinski
  29 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-07 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chuck Zmudzinski, Marcel Apfelbaum, Hervé Poussineau,
	Eduardo Habkost, Aurelien Jarno, Michael S. Tsirkin,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

QEMU's PIIX3 implementation actually models the real PIIX4, but with different
PCI IDs. Usually, guests deal just fine with it. Still, in order to provide a
more consistent illusion to guests, allow QEMU's PIIX4 implementation to be used
in the PC machine.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 docs/system/target-i386-desc.rst.inc |  8 ++++
 include/hw/i386/pc.h                 |  2 +
 hw/i386/pc.c                         |  1 +
 hw/i386/pc_piix.c                    | 61 +++++++++++++++++++++++++++-
 4 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/docs/system/target-i386-desc.rst.inc b/docs/system/target-i386-desc.rst.inc
index 7d1fffacbe..5ebbcda9db 100644
--- a/docs/system/target-i386-desc.rst.inc
+++ b/docs/system/target-i386-desc.rst.inc
@@ -71,3 +71,11 @@ machine property, i.e.
    |qemu_system_x86| some.img \
    -audiodev <backend>,id=<name> \
    -machine pcspk-audiodev=<name>
+
+Machine-specific options
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+It supports the following machine-specific options:
+
+- ``x-south-bridge=PIIX3|piix4-isa`` (Experimental option to select a particular
+  south bridge. Default: ``PIIX3``)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index bec38cb92c..29a9724524 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -42,6 +42,7 @@ typedef struct PCMachineState {
     uint64_t max_ram_below_4g;
     OnOffAuto vmport;
     SmbiosEntryPointType smbios_entry_point_type;
+    const char *south_bridge;
 
     bool acpi_build_enabled;
     bool smbus_enabled;
@@ -92,6 +93,7 @@ struct PCMachineClass {
     /* Device configuration: */
     bool pci_enabled;
     bool kvmclock_enabled;
+    const char *default_south_bridge;
 
     /* Compat options: */
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4e844d02f2..c84d1bdf08 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1707,6 +1707,7 @@ static void pc_machine_initfn(Object *obj)
 #endif /* CONFIG_VMPORT */
     pcms->max_ram_below_4g = 0; /* use default */
     pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type;
+    pcms->south_bridge = pcmc->default_south_bridge;
 
     /* acpi build is enabled by default if machine supports it */
     pcms->acpi_build_enabled = pcmc->has_acpi_build;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e38942a3c3..334d9a0299 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -262,7 +262,7 @@ static void pc_init1(MachineState *machine,
         DeviceState *dev;
         size_t i;
 
-        pci_dev = pci_new_multifunction(-1, TYPE_PIIX3_DEVICE);
+        pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
         object_property_set_bool(OBJECT(pci_dev), "has-usb",
                                  machine_usb(machine), &error_abort);
         object_property_set_bool(OBJECT(pci_dev), "has-acpi",
@@ -394,6 +394,56 @@ static void pc_init1(MachineState *machine,
     }
 }
 
+typedef enum PCSouthBridgeOption {
+    PC_SOUTH_BRIDGE_OPTION_PIIX3,
+    PC_SOUTH_BRIDGE_OPTION_PIIX4,
+    PC_SOUTH_BRIDGE_OPTION_MAX,
+} PCSouthBridgeOption;
+
+static const QEnumLookup PCSouthBridgeOption_lookup = {
+    .array = (const char *const[]) {
+        [PC_SOUTH_BRIDGE_OPTION_PIIX3] = TYPE_PIIX3_DEVICE,
+        [PC_SOUTH_BRIDGE_OPTION_PIIX4] = TYPE_PIIX4_PCI_DEVICE,
+    },
+    .size = PC_SOUTH_BRIDGE_OPTION_MAX
+};
+
+#define NotifyVmexitOption_str(val) \
+    qapi_enum_lookup(&NotifyVmexitOption_lookup, (val))
+
+static int pc_get_south_bridge(Object *obj, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+    int i;
+
+    for (i = 0; i < PCSouthBridgeOption_lookup.size; i++) {
+        if (g_strcmp0(PCSouthBridgeOption_lookup.array[i],
+                      pcms->south_bridge) == 0) {
+            return i;
+        }
+    }
+
+    error_setg(errp, "Invalid south bridge value set");
+    return 0;
+}
+
+static void pc_set_south_bridge(Object *obj, int value, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    if (value < 0) {
+        error_setg(errp, "Value can't be negative");
+        return;
+    }
+
+    if (value >= PCSouthBridgeOption_lookup.size) {
+        error_setg(errp, "Value too big");
+        return;
+    }
+
+    pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
+}
+
 /* Looking for a pc_compat_2_4() function? It doesn't exist.
  * pc_compat_*() functions that run on machine-init time and
  * change global QEMU state are deprecated. Please don't create
@@ -473,6 +523,8 @@ static void pc_xen_hvm_init(MachineState *machine)
 static void pc_i440fx_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+    ObjectClass *oc = OBJECT_CLASS(m);
+    pcmc->default_south_bridge = TYPE_PIIX3_DEVICE;
     pcmc->pci_root_uid = 0;
     pcmc->default_cpu_version = 1;
 
@@ -484,6 +536,13 @@ static void pc_i440fx_machine_options(MachineClass *m)
     m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
+
+    object_class_property_add_enum(oc, "x-south-bridge", "PCSouthBridgeOption",
+                                   &PCSouthBridgeOption_lookup,
+                                   pc_get_south_bridge,
+                                   pc_set_south_bridge);
+    object_class_property_set_description(oc, "x-south-bridge",
+                                     "Use a different south bridge than PIIX3");
 }
 
 static void pc_i440fx_8_2_machine_options(MachineClass *m)
-- 
2.42.0



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

* Re: [PATCH v8 00/29] Consolidate PIIX south bridges
  2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
                   ` (28 preceding siblings ...)
  2023-10-07 12:38 ` [PATCH v8 29/29] hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine Bernhard Beschow
@ 2023-10-08 17:56 ` Chuck Zmudzinski
  2023-10-11 18:57   ` Bernhard Beschow
  29 siblings, 1 reply; 34+ messages in thread
From: Chuck Zmudzinski @ 2023-10-08 17:56 UTC (permalink / raw)
  To: Bernhard Beschow, qemu-devel
  Cc: Marcel Apfelbaum, Hervé Poussineau, Eduardo Habkost,
	Aurelien Jarno, Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Paolo Bonzini, Richard Henderson, Anthony PERARD,
	Stefano Stabellini, Paul Durrant, Jason Andryuk

On 10/7/23 8:38 AM, Bernhard Beschow wrote:
> This series consolidates the implementations of the PIIX3 and PIIX4 south
> bridges and makes PIIX4 usable in the PC machine via an experimental command
> line parameter. The motivation is to resolve duplicate code between the device
> models as well as resolving the "Frankenstein" PIIX4-PM problem in PIIX3
> discussed on this list before.
> 
> The series is structured as follows:
> 
> Patches 1-8 are preparational patches necessary for moving all sub devices into
> PIIX3, like was done for PIIX4. In isolation these patches can also be seen as
> general x86 machine cleanup sub series which has merit in its own right -- and
> could be applied to master if the remainder of the series takes longer to
> review.
> 
> Patches 9-13 move PIIX3 sub devices into one device model like already
> done for PIIX4. Together with the previous sub series these patches form a
> bigger sub series which also has merit in its own right, and could be applied
> independent of the remainder of this series as well.
> 
> The remainder of this series consolidates the PIIX3 and PIIX4 device models.
> The culmination point is the last commit which makes PIIX4 usable in the PC
> machine.
> 
> One challenge was dealing with optional devices where Peter already gave advice
> in [1] which this series implements. Although PIIX4 is now usable in the PC
> machine it still has a different binary layout in its VM state.
> 
> Testing done:
> * `make check`
> * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cdrom
>      manjaro-kde-21.3.2-220704-linux515.iso`
> * `qemu-system-x86_64 -M pc,x-south-bridge=piix4-isa -m 2G -accel kvm -cdrom
>      manjaro-kde-21.3.2-220704-linux515.iso`
> * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cdrom
>      manjaro-kde-21.3.2-220704-linux515.iso`
> * `qemu-system-mips64el -M malta -cpu 5KEc -m 1G -kernel kernel -initrd initrd
>      -append "root=LABEL=rootfs console=ttyS0" -drive file=image.qcow2`
> * `qemu-system-mips64el -M malta -bios yamon-02.22.bin`
> * Run HVM domU guest under Xen with manjaro-kde-21.3.2-220704-linux515.iso image

I did some preliminary tests of this patch series on some Xen HVM domU guests I have
that use the xenfv / pc machine and depend on the current PIIX3 implementation.
So far there are no regressions in my tests. I use libxl or libvirt to manage the
Xen guests.

I have not (yet) tested the experimental option that makes PIIX4 useable in the xenfv / pc
machines. IIUC, that would require a patch to hvmloader/pci.c in Xen tools so Xen's
hvmloader recognizes the PIIX4 pci device id [1], and a patch to libxl so libxl can
optionally launch qemu with the new experimental option enabled.

Since this patch series affects the xenfv machine, I added the Xen x86 maintainers to
the Cc list and Jason Andryuk who is credited with discovering the necessary patch to
hvmloader/pci.c.

[1] https://lore.kernel.org/qemu-devel/B0FF78F4-1193-495B-919C-84A1FF8ADF12@gmail.com/

> 
> v8:
> - Wire ISA interrupts before device realization
> - Optionally allow a PIC and PIT to be instantiated in PIIX3 for compatiblity
>     with PIIX4
> - Touch ICH9 LPC as far as required for PIIX consolidation
> - Make PIIX4 usable in the PC machine via an experimental option
> - Review and rework history, touching every commit and drop R-b tags when
>     changes became too large
> 
> v7:
> - Rebase onto master
> - Avoid the PIC proxy (Phil)
>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
>   populated already but pc_piix assigned the interrupts only after realizing
>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
>   are already populated during PIIX3's realize phase where the ISA interrupts
>   are wired up.
> - New patches:
>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>   * hw/isa/piix4: Create the "intr" property during init() already
> - Patches with substantial changes (Reviewed-by dropped):
>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
> 
> v6:
> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
>   within the patch series.
> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
>   bridges" [2] for maintainer convenience.
> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>   created' into
>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
>   similar for Malta.
> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
> 
> v5:
> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
> - Add patch to make usage of the isa_pic global more type-safe
> - Re-introduce isa-pic as PIC specific proxy (Mark)
> 
> v4:
> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
>   since it is already queued via mips-next. This eliminates patches
>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
>   Prefix pci_slot_get_pirq() with "piix4_"'.
> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
>   split these patches since I wasn't sure whether renaming a type was allowed.
> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>   created' for forther cleanup of INTx-to-LNKx route decoupling.
> 
> v3:
> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
>   (Philippe)
> - Make proxy PIC generic (Philippe)
> - Track Malta's PIIX dependencies through KConfig
> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
> - Also rebase onto latest master to resolve merge conflicts. This required
>   copying Philippe's series as first three patches - please ignore.
> 
> v2:
> - Introduce TYPE_ defines for IDE and USB device models (Mark)
> - Omit unexporting of PIIXState (Mark)
> - Improve commit message of patch 5 to mention reset triggering through PCI
>   configuration space (Mark)
> - Move reviewed patches w/o dependencies to the bottom of the series for early
>   upstreaming
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
> 
> Bernhard Beschow (29):
>   hw/i386/pc: Merge two if statements into one
>   hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
>     south bridge
>   hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize()
>   hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
>   hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs"
>     property
>   hw/i386/pc_piix: Remove redundant "piix3" variable
>   hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in"
>   hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its
>     realize()
>   hw/isa/piix3: Wire PIC IRQs to ISA bus in host device
>   hw/i386/pc: Wire RTC ISA IRQs in south bridges
>   hw/isa/piix3: Create IDE controller in host device
>   hw/isa/piix3: Create USB controller in host device
>   hw/isa/piix3: Create power management controller in host device
>   hw/isa/piix3: Drop the "3" from PIIX base class name
>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
>   hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in"
>   hw/isa/piix4: Rename reset control operations to match PIIX3
>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
>   hw/isa/piix3: Merge hw/isa/piix4.c
>   hw/isa/piix: Allow for optional PIC creation in PIIX3
>   hw/isa/piix: Allow for optional PIT creation in PIIX3
>   hw/isa/piix: Harmonize names of reset control memory regions
>   hw/isa/piix: Share PIIX3's base class with PIIX4
>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>   hw/isa/piix: Rename functions to be shared for PCI interrupt
>     triggering
>   hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4
>   hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring
>   hw/isa/piix: Implement multi-process QEMU support also for PIIX4
>   hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine
> 
>  MAINTAINERS                          |   6 +-
>  docs/system/target-i386-desc.rst.inc |   8 +
>  include/hw/i386/pc.h                 |   2 +
>  include/hw/southbridge/piix.h        |  28 ++-
>  hw/i386/pc.c                         |  13 +-
>  hw/i386/pc_piix.c                    | 125 ++++++++---
>  hw/i386/pc_q35.c                     |  14 +-
>  hw/isa/lpc_ich9.c                    |   9 +-
>  hw/isa/{piix3.c => piix.c}           | 281 ++++++++++++++++++-------
>  hw/isa/piix4.c                       | 302 ---------------------------
>  hw/mips/malta.c                      |   5 +-
>  hw/i386/Kconfig                      |   3 +-
>  hw/isa/Kconfig                       |   8 +-
>  hw/isa/meson.build                   |   3 +-
>  hw/mips/Kconfig                      |   2 +-
>  15 files changed, 358 insertions(+), 451 deletions(-)
>  rename hw/isa/{piix3.c => piix.c} (52%)
>  delete mode 100644 hw/isa/piix4.c
> 



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

* Re: [PATCH v8 00/29] Consolidate PIIX south bridges
  2023-10-08 17:56 ` [PATCH v8 00/29] Consolidate PIIX south bridges Chuck Zmudzinski
@ 2023-10-11 18:57   ` Bernhard Beschow
  2023-10-11 20:18     ` Michael S. Tsirkin
  0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-11 18:57 UTC (permalink / raw)
  To: Chuck Zmudzinski, qemu-devel
  Cc: Marcel Apfelbaum, Hervé Poussineau, Eduardo Habkost,
	Aurelien Jarno, Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Paolo Bonzini, Richard Henderson, Anthony PERARD,
	Stefano Stabellini, Paul Durrant, Jason Andryuk



Am 8. Oktober 2023 17:56:48 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>On 10/7/23 8:38 AM, Bernhard Beschow wrote:
>> This series consolidates the implementations of the PIIX3 and PIIX4 south
>> bridges and makes PIIX4 usable in the PC machine via an experimental command
>> line parameter. The motivation is to resolve duplicate code between the device
>> models as well as resolving the "Frankenstein" PIIX4-PM problem in PIIX3
>> discussed on this list before.
>> 
>> The series is structured as follows:
>> 
>> Patches 1-8 are preparational patches necessary for moving all sub devices into
>> PIIX3, like was done for PIIX4. In isolation these patches can also be seen as
>> general x86 machine cleanup sub series which has merit in its own right -- and
>> could be applied to master if the remainder of the series takes longer to
>> review.
>> 
>> Patches 9-13 move PIIX3 sub devices into one device model like already
>> done for PIIX4. Together with the previous sub series these patches form a
>> bigger sub series which also has merit in its own right, and could be applied
>> independent of the remainder of this series as well.
>> 
>> The remainder of this series consolidates the PIIX3 and PIIX4 device models.
>> The culmination point is the last commit which makes PIIX4 usable in the PC
>> machine.
>> 
>> One challenge was dealing with optional devices where Peter already gave advice
>> in [1] which this series implements. Although PIIX4 is now usable in the PC
>> machine it still has a different binary layout in its VM state.
>> 
>> Testing done:
>> * `make check`
>> * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cdrom
>>      manjaro-kde-21.3.2-220704-linux515.iso`
>> * `qemu-system-x86_64 -M pc,x-south-bridge=piix4-isa -m 2G -accel kvm -cdrom
>>      manjaro-kde-21.3.2-220704-linux515.iso`
>> * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cdrom
>>      manjaro-kde-21.3.2-220704-linux515.iso`
>> * `qemu-system-mips64el -M malta -cpu 5KEc -m 1G -kernel kernel -initrd initrd
>>      -append "root=LABEL=rootfs console=ttyS0" -drive file=image.qcow2`
>> * `qemu-system-mips64el -M malta -bios yamon-02.22.bin`
>> * Run HVM domU guest under Xen with manjaro-kde-21.3.2-220704-linux515.iso image
>
>I did some preliminary tests of this patch series on some Xen HVM domU guests I have
>that use the xenfv / pc machine and depend on the current PIIX3 implementation.
>So far there are no regressions in my tests. I use libxl or libvirt to manage the
>Xen guests.

Thanks, nice to read!

>
>I have not (yet) tested the experimental option that makes PIIX4 useable in the xenfv / pc
>machines. IIUC, that would require a patch to hvmloader/pci.c in Xen tools so Xen's
>hvmloader recognizes the PIIX4 pci device id [1], and a patch to libxl so libxl can
>optionally launch qemu with the new experimental option enabled.
>
>Since this patch series affects the xenfv machine, I added the Xen x86 maintainers to
>the Cc list and Jason Andryuk who is credited with discovering the necessary patch to
>hvmloader/pci.c.

Good idea. In the next iteration, I'll cc the respective email addresses from the MAINTAINERS file which hopefully reaches all relevant people.

Best regards,
Bernhard

>
>[1] https://lore.kernel.org/qemu-devel/B0FF78F4-1193-495B-919C-84A1FF8ADF12@gmail.com/
>
>> 
>> v8:
>> - Wire ISA interrupts before device realization
>> - Optionally allow a PIC and PIT to be instantiated in PIIX3 for compatiblity
>>     with PIIX4
>> - Touch ICH9 LPC as far as required for PIIX consolidation
>> - Make PIIX4 usable in the PC machine via an experimental option
>> - Review and rework history, touching every commit and drop R-b tags when
>>     changes became too large
>> 
>> v7:
>> - Rebase onto master
>> - Avoid the PIC proxy (Phil)
>>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
>>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
>>   populated already but pc_piix assigned the interrupts only after realizing
>>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
>>   are already populated during PIIX3's realize phase where the ISA interrupts
>>   are wired up.
>> - New patches:
>>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>   * hw/isa/piix4: Create the "intr" property during init() already
>> - Patches with substantial changes (Reviewed-by dropped):
>>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
>> 
>> v6:
>> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
>>   within the patch series.
>> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
>>   bridges" [2] for maintainer convenience.
>> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>>   created' into
>>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
>>   similar for Malta.
>> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
>>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
>> 
>> v5:
>> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
>> - Add patch to make usage of the isa_pic global more type-safe
>> - Re-introduce isa-pic as PIC specific proxy (Mark)
>> 
>> v4:
>> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
>>   since it is already queued via mips-next. This eliminates patches
>>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
>>   Prefix pci_slot_get_pirq() with "piix4_"'.
>> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
>>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
>>   split these patches since I wasn't sure whether renaming a type was allowed.
>> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>>   created' for forther cleanup of INTx-to-LNKx route decoupling.
>> 
>> v3:
>> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
>>   (Philippe)
>> - Make proxy PIC generic (Philippe)
>> - Track Malta's PIIX dependencies through KConfig
>> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
>> - Also rebase onto latest master to resolve merge conflicts. This required
>>   copying Philippe's series as first three patches - please ignore.
>> 
>> v2:
>> - Introduce TYPE_ defines for IDE and USB device models (Mark)
>> - Omit unexporting of PIIXState (Mark)
>> - Improve commit message of patch 5 to mention reset triggering through PCI
>>   configuration space (Mark)
>> - Move reviewed patches w/o dependencies to the bottom of the series for early
>>   upstreaming
>> 
>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
>> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
>> 
>> Bernhard Beschow (29):
>>   hw/i386/pc: Merge two if statements into one
>>   hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
>>     south bridge
>>   hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize()
>>   hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
>>   hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs"
>>     property
>>   hw/i386/pc_piix: Remove redundant "piix3" variable
>>   hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in"
>>   hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its
>>     realize()
>>   hw/isa/piix3: Wire PIC IRQs to ISA bus in host device
>>   hw/i386/pc: Wire RTC ISA IRQs in south bridges
>>   hw/isa/piix3: Create IDE controller in host device
>>   hw/isa/piix3: Create USB controller in host device
>>   hw/isa/piix3: Create power management controller in host device
>>   hw/isa/piix3: Drop the "3" from PIIX base class name
>>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
>>   hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in"
>>   hw/isa/piix4: Rename reset control operations to match PIIX3
>>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
>>   hw/isa/piix3: Merge hw/isa/piix4.c
>>   hw/isa/piix: Allow for optional PIC creation in PIIX3
>>   hw/isa/piix: Allow for optional PIT creation in PIIX3
>>   hw/isa/piix: Harmonize names of reset control memory regions
>>   hw/isa/piix: Share PIIX3's base class with PIIX4
>>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>>   hw/isa/piix: Rename functions to be shared for PCI interrupt
>>     triggering
>>   hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4
>>   hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring
>>   hw/isa/piix: Implement multi-process QEMU support also for PIIX4
>>   hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine
>> 
>>  MAINTAINERS                          |   6 +-
>>  docs/system/target-i386-desc.rst.inc |   8 +
>>  include/hw/i386/pc.h                 |   2 +
>>  include/hw/southbridge/piix.h        |  28 ++-
>>  hw/i386/pc.c                         |  13 +-
>>  hw/i386/pc_piix.c                    | 125 ++++++++---
>>  hw/i386/pc_q35.c                     |  14 +-
>>  hw/isa/lpc_ich9.c                    |   9 +-
>>  hw/isa/{piix3.c => piix.c}           | 281 ++++++++++++++++++-------
>>  hw/isa/piix4.c                       | 302 ---------------------------
>>  hw/mips/malta.c                      |   5 +-
>>  hw/i386/Kconfig                      |   3 +-
>>  hw/isa/Kconfig                       |   8 +-
>>  hw/isa/meson.build                   |   3 +-
>>  hw/mips/Kconfig                      |   2 +-
>>  15 files changed, 358 insertions(+), 451 deletions(-)
>>  rename hw/isa/{piix3.c => piix.c} (52%)
>>  delete mode 100644 hw/isa/piix4.c
>> 
>


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

* Re: [PATCH v8 00/29] Consolidate PIIX south bridges
  2023-10-11 18:57   ` Bernhard Beschow
@ 2023-10-11 20:18     ` Michael S. Tsirkin
  2023-10-12 18:13       ` Bernhard Beschow
  0 siblings, 1 reply; 34+ messages in thread
From: Michael S. Tsirkin @ 2023-10-11 20:18 UTC (permalink / raw)
  To: Bernhard Beschow
  Cc: Chuck Zmudzinski, qemu-devel, Marcel Apfelbaum,
	Hervé Poussineau, Eduardo Habkost, Aurelien Jarno,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Anthony PERARD, Stefano Stabellini, Paul Durrant, Jason Andryuk

On Wed, Oct 11, 2023 at 06:57:07PM +0000, Bernhard Beschow wrote:
> 
> 
> Am 8. Oktober 2023 17:56:48 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
> >On 10/7/23 8:38 AM, Bernhard Beschow wrote:
> >> This series consolidates the implementations of the PIIX3 and PIIX4 south
> >> bridges and makes PIIX4 usable in the PC machine via an experimental command
> >> line parameter. The motivation is to resolve duplicate code between the device
> >> models as well as resolving the "Frankenstein" PIIX4-PM problem in PIIX3
> >> discussed on this list before.
> >> 
> >> The series is structured as follows:
> >> 
> >> Patches 1-8 are preparational patches necessary for moving all sub devices into
> >> PIIX3, like was done for PIIX4. In isolation these patches can also be seen as
> >> general x86 machine cleanup sub series which has merit in its own right -- and
> >> could be applied to master if the remainder of the series takes longer to
> >> review.
> >> 
> >> Patches 9-13 move PIIX3 sub devices into one device model like already
> >> done for PIIX4. Together with the previous sub series these patches form a
> >> bigger sub series which also has merit in its own right, and could be applied
> >> independent of the remainder of this series as well.
> >> 
> >> The remainder of this series consolidates the PIIX3 and PIIX4 device models.
> >> The culmination point is the last commit which makes PIIX4 usable in the PC
> >> machine.
> >> 
> >> One challenge was dealing with optional devices where Peter already gave advice
> >> in [1] which this series implements. Although PIIX4 is now usable in the PC
> >> machine it still has a different binary layout in its VM state.
> >> 
> >> Testing done:
> >> * `make check`
> >> * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cdrom
> >>      manjaro-kde-21.3.2-220704-linux515.iso`
> >> * `qemu-system-x86_64 -M pc,x-south-bridge=piix4-isa -m 2G -accel kvm -cdrom
> >>      manjaro-kde-21.3.2-220704-linux515.iso`
> >> * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cdrom
> >>      manjaro-kde-21.3.2-220704-linux515.iso`
> >> * `qemu-system-mips64el -M malta -cpu 5KEc -m 1G -kernel kernel -initrd initrd
> >>      -append "root=LABEL=rootfs console=ttyS0" -drive file=image.qcow2`
> >> * `qemu-system-mips64el -M malta -bios yamon-02.22.bin`
> >> * Run HVM domU guest under Xen with manjaro-kde-21.3.2-220704-linux515.iso image
> >
> >I did some preliminary tests of this patch series on some Xen HVM domU guests I have
> >that use the xenfv / pc machine and depend on the current PIIX3 implementation.
> >So far there are no regressions in my tests. I use libxl or libvirt to manage the
> >Xen guests.
> 
> Thanks, nice to read!
> 
> >
> >I have not (yet) tested the experimental option that makes PIIX4 useable in the xenfv / pc
> >machines. IIUC, that would require a patch to hvmloader/pci.c in Xen tools so Xen's
> >hvmloader recognizes the PIIX4 pci device id [1], and a patch to libxl so libxl can
> >optionally launch qemu with the new experimental option enabled.
> >
> >Since this patch series affects the xenfv machine, I added the Xen x86 maintainers to
> >the Cc list and Jason Andryuk who is credited with discovering the necessary patch to
> >hvmloader/pci.c.
> 
> Good idea. In the next iteration, I'll cc the respective email addresses from the MAINTAINERS file which hopefully reaches all relevant people.

there will be a next version then?

> Best regards,
> Bernhard
> 
> >
> >[1] https://lore.kernel.org/qemu-devel/B0FF78F4-1193-495B-919C-84A1FF8ADF12@gmail.com/
> >
> >> 
> >> v8:
> >> - Wire ISA interrupts before device realization
> >> - Optionally allow a PIC and PIT to be instantiated in PIIX3 for compatiblity
> >>     with PIIX4
> >> - Touch ICH9 LPC as far as required for PIIX consolidation
> >> - Make PIIX4 usable in the PC machine via an experimental option
> >> - Review and rework history, touching every commit and drop R-b tags when
> >>     changes became too large
> >> 
> >> v7:
> >> - Rebase onto master
> >> - Avoid the PIC proxy (Phil)
> >>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
> >>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
> >>   populated already but pc_piix assigned the interrupts only after realizing
> >>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
> >>   are already populated during PIIX3's realize phase where the ISA interrupts
> >>   are wired up.
> >> - New patches:
> >>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>   * hw/isa/piix4: Create the "intr" property during init() already
> >> - Patches with substantial changes (Reviewed-by dropped):
> >>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
> >> 
> >> v6:
> >> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
> >>   within the patch series.
> >> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
> >>   bridges" [2] for maintainer convenience.
> >> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
> >>   created' into
> >>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
> >>   similar for Malta.
> >> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
> >>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
> >> 
> >> v5:
> >> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
> >> - Add patch to make usage of the isa_pic global more type-safe
> >> - Re-introduce isa-pic as PIC specific proxy (Mark)
> >> 
> >> v4:
> >> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
> >>   since it is already queued via mips-next. This eliminates patches
> >>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
> >>   Prefix pci_slot_get_pirq() with "piix4_"'.
> >> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
> >>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
> >>   split these patches since I wasn't sure whether renaming a type was allowed.
> >> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
> >>   created' for forther cleanup of INTx-to-LNKx route decoupling.
> >> 
> >> v3:
> >> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
> >>   (Philippe)
> >> - Make proxy PIC generic (Philippe)
> >> - Track Malta's PIIX dependencies through KConfig
> >> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
> >> - Also rebase onto latest master to resolve merge conflicts. This required
> >>   copying Philippe's series as first three patches - please ignore.
> >> 
> >> v2:
> >> - Introduce TYPE_ defines for IDE and USB device models (Mark)
> >> - Omit unexporting of PIIXState (Mark)
> >> - Improve commit message of patch 5 to mention reset triggering through PCI
> >>   configuration space (Mark)
> >> - Move reviewed patches w/o dependencies to the bottom of the series for early
> >>   upstreaming
> >> 
> >> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
> >> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
> >> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
> >> 
> >> Bernhard Beschow (29):
> >>   hw/i386/pc: Merge two if statements into one
> >>   hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
> >>     south bridge
> >>   hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize()
> >>   hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
> >>   hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs"
> >>     property
> >>   hw/i386/pc_piix: Remove redundant "piix3" variable
> >>   hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in"
> >>   hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its
> >>     realize()
> >>   hw/isa/piix3: Wire PIC IRQs to ISA bus in host device
> >>   hw/i386/pc: Wire RTC ISA IRQs in south bridges
> >>   hw/isa/piix3: Create IDE controller in host device
> >>   hw/isa/piix3: Create USB controller in host device
> >>   hw/isa/piix3: Create power management controller in host device
> >>   hw/isa/piix3: Drop the "3" from PIIX base class name
> >>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
> >>   hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in"
> >>   hw/isa/piix4: Rename reset control operations to match PIIX3
> >>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
> >>   hw/isa/piix3: Merge hw/isa/piix4.c
> >>   hw/isa/piix: Allow for optional PIC creation in PIIX3
> >>   hw/isa/piix: Allow for optional PIT creation in PIIX3
> >>   hw/isa/piix: Harmonize names of reset control memory regions
> >>   hw/isa/piix: Share PIIX3's base class with PIIX4
> >>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
> >>   hw/isa/piix: Rename functions to be shared for PCI interrupt
> >>     triggering
> >>   hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4
> >>   hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring
> >>   hw/isa/piix: Implement multi-process QEMU support also for PIIX4
> >>   hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine
> >> 
> >>  MAINTAINERS                          |   6 +-
> >>  docs/system/target-i386-desc.rst.inc |   8 +
> >>  include/hw/i386/pc.h                 |   2 +
> >>  include/hw/southbridge/piix.h        |  28 ++-
> >>  hw/i386/pc.c                         |  13 +-
> >>  hw/i386/pc_piix.c                    | 125 ++++++++---
> >>  hw/i386/pc_q35.c                     |  14 +-
> >>  hw/isa/lpc_ich9.c                    |   9 +-
> >>  hw/isa/{piix3.c => piix.c}           | 281 ++++++++++++++++++-------
> >>  hw/isa/piix4.c                       | 302 ---------------------------
> >>  hw/mips/malta.c                      |   5 +-
> >>  hw/i386/Kconfig                      |   3 +-
> >>  hw/isa/Kconfig                       |   8 +-
> >>  hw/isa/meson.build                   |   3 +-
> >>  hw/mips/Kconfig                      |   2 +-
> >>  15 files changed, 358 insertions(+), 451 deletions(-)
> >>  rename hw/isa/{piix3.c => piix.c} (52%)
> >>  delete mode 100644 hw/isa/piix4.c
> >> 
> >



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

* Re: [PATCH v8 00/29] Consolidate PIIX south bridges
  2023-10-11 20:18     ` Michael S. Tsirkin
@ 2023-10-12 18:13       ` Bernhard Beschow
  0 siblings, 0 replies; 34+ messages in thread
From: Bernhard Beschow @ 2023-10-12 18:13 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Chuck Zmudzinski, qemu-devel, Marcel Apfelbaum,
	Hervé Poussineau, Eduardo Habkost, Aurelien Jarno,
	Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson,
	Anthony PERARD, Stefano Stabellini, Paul Durrant, Jason Andryuk



Am 11. Oktober 2023 20:18:00 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>On Wed, Oct 11, 2023 at 06:57:07PM +0000, Bernhard Beschow wrote:
>> 
>> 
>> Am 8. Oktober 2023 17:56:48 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>> >On 10/7/23 8:38 AM, Bernhard Beschow wrote:
>> >> This series consolidates the implementations of the PIIX3 and PIIX4 south
>> >> bridges and makes PIIX4 usable in the PC machine via an experimental command
>> >> line parameter. The motivation is to resolve duplicate code between the device
>> >> models as well as resolving the "Frankenstein" PIIX4-PM problem in PIIX3
>> >> discussed on this list before.
>> >> 
>> >> The series is structured as follows:
>> >> 
>> >> Patches 1-8 are preparational patches necessary for moving all sub devices into
>> >> PIIX3, like was done for PIIX4. In isolation these patches can also be seen as
>> >> general x86 machine cleanup sub series which has merit in its own right -- and
>> >> could be applied to master if the remainder of the series takes longer to
>> >> review.
>> >> 
>> >> Patches 9-13 move PIIX3 sub devices into one device model like already
>> >> done for PIIX4. Together with the previous sub series these patches form a
>> >> bigger sub series which also has merit in its own right, and could be applied
>> >> independent of the remainder of this series as well.
>> >> 
>> >> The remainder of this series consolidates the PIIX3 and PIIX4 device models.
>> >> The culmination point is the last commit which makes PIIX4 usable in the PC
>> >> machine.
>> >> 
>> >> One challenge was dealing with optional devices where Peter already gave advice
>> >> in [1] which this series implements. Although PIIX4 is now usable in the PC
>> >> machine it still has a different binary layout in its VM state.
>> >> 
>> >> Testing done:
>> >> * `make check`
>> >> * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cdrom
>> >>      manjaro-kde-21.3.2-220704-linux515.iso`
>> >> * `qemu-system-x86_64 -M pc,x-south-bridge=piix4-isa -m 2G -accel kvm -cdrom
>> >>      manjaro-kde-21.3.2-220704-linux515.iso`
>> >> * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cdrom
>> >>      manjaro-kde-21.3.2-220704-linux515.iso`
>> >> * `qemu-system-mips64el -M malta -cpu 5KEc -m 1G -kernel kernel -initrd initrd
>> >>      -append "root=LABEL=rootfs console=ttyS0" -drive file=image.qcow2`
>> >> * `qemu-system-mips64el -M malta -bios yamon-02.22.bin`
>> >> * Run HVM domU guest under Xen with manjaro-kde-21.3.2-220704-linux515.iso image
>> >
>> >I did some preliminary tests of this patch series on some Xen HVM domU guests I have
>> >that use the xenfv / pc machine and depend on the current PIIX3 implementation.
>> >So far there are no regressions in my tests. I use libxl or libvirt to manage the
>> >Xen guests.
>> 
>> Thanks, nice to read!
>> 
>> >
>> >I have not (yet) tested the experimental option that makes PIIX4 useable in the xenfv / pc
>> >machines. IIUC, that would require a patch to hvmloader/pci.c in Xen tools so Xen's
>> >hvmloader recognizes the PIIX4 pci device id [1], and a patch to libxl so libxl can
>> >optionally launch qemu with the new experimental option enabled.
>> >
>> >Since this patch series affects the xenfv machine, I added the Xen x86 maintainers to
>> >the Cc list and Jason Andryuk who is credited with discovering the necessary patch to
>> >hvmloader/pci.c.
>> 
>> Good idea. In the next iteration, I'll cc the respective email addresses from the MAINTAINERS file which hopefully reaches all relevant people.
>
>there will be a next version then?

No, unless review comments will require it.

>
>> Best regards,
>> Bernhard
>> 
>> >
>> >[1] https://lore.kernel.org/qemu-devel/B0FF78F4-1193-495B-919C-84A1FF8ADF12@gmail.com/
>> >
>> >> 
>> >> v8:
>> >> - Wire ISA interrupts before device realization
>> >> - Optionally allow a PIC and PIT to be instantiated in PIIX3 for compatiblity
>> >>     with PIIX4
>> >> - Touch ICH9 LPC as far as required for PIIX consolidation
>> >> - Make PIIX4 usable in the PC machine via an experimental option
>> >> - Review and rework history, touching every commit and drop R-b tags when
>> >>     changes became too large
>> >> 
>> >> v7:
>> >> - Rebase onto master
>> >> - Avoid the PIC proxy (Phil)
>> >>   The motivation for the PIC proxy was to allow for wiring up ISA interrupts in
>> >>   the south bridges. ISA interrupt wiring requires the GPIO lines to be
>> >>   populated already but pc_piix assigned the interrupts only after realizing
>> >>   PIIX3. By shifting interrupt assignment before realizing, the ISA interrupts
>> >>   are already populated during PIIX3's realize phase where the ISA interrupts
>> >>   are wired up.
>> >> - New patches:
>> >>   * hw/isa/piix4: Reuse struct PIIXState from PIIX3
>> >>   * hw/isa/piix4: Create the "intr" property during init() already
>> >> - Patches with substantial changes (Reviewed-by dropped):
>> >>   * hw/isa/piix3: Move ISA bus IRQ assignments into host device
>> >> 
>> >> v6:
>> >> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently
>> >>   within the patch series.
>> >> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south
>> >>   bridges" [2] for maintainer convenience.
>> >> - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>> >>   created' into
>> >>   https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do
>> >>   similar for Malta.
>> >> - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of
>> >>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging")
>> >> 
>> >> v5:
>> >> - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html
>> >> - Add patch to make usage of the isa_pic global more type-safe
>> >> - Re-introduce isa-pic as PIC specific proxy (Mark)
>> >> 
>> >> v4:
>> >> - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges"
>> >>   since it is already queued via mips-next. This eliminates patches
>> >>   'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and 'hw/isa/piix4:
>> >>   Prefix pci_slot_get_pirq() with "piix4_"'.
>> >> - Squash 'hw/isa/piix: Drop the "3" from the PIIX base class' into
>> >>   'hw/isa/piix3: Rename typedef PIIX3State to PIIXState'. I originally only
>> >>   split these patches since I wasn't sure whether renaming a type was allowed.
>> >> - Add new patch 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is
>> >>   created' for forther cleanup of INTx-to-LNKx route decoupling.
>> >> 
>> >> v3:
>> >> - Introduce one TYPE_ICH9_USB_UHCI(fn) rather than several TYPE_ICH9_USB_UHCIx
>> >>   (Philippe)
>> >> - Make proxy PIC generic (Philippe)
>> >> - Track Malta's PIIX dependencies through KConfig
>> >> - Rebase onto Philippe's 'hw/isa/piix4: Remove MIPS Malta specific bits' series [3]
>> >> - Also rebase onto latest master to resolve merge conflicts. This required
>> >>   copying Philippe's series as first three patches - please ignore.
>> >> 
>> >> v2:
>> >> - Introduce TYPE_ defines for IDE and USB device models (Mark)
>> >> - Omit unexporting of PIIXState (Mark)
>> >> - Improve commit message of patch 5 to mention reset triggering through PCI
>> >>   configuration space (Mark)
>> >> - Move reviewed patches w/o dependencies to the bottom of the series for early
>> >>   upstreaming
>> >> 
>> >> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>> >> [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03310.html
>> >> [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg05367.html
>> >> 
>> >> Bernhard Beschow (29):
>> >>   hw/i386/pc: Merge two if statements into one
>> >>   hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
>> >>     south bridge
>> >>   hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize()
>> >>   hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
>> >>   hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs"
>> >>     property
>> >>   hw/i386/pc_piix: Remove redundant "piix3" variable
>> >>   hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in"
>> >>   hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its
>> >>     realize()
>> >>   hw/isa/piix3: Wire PIC IRQs to ISA bus in host device
>> >>   hw/i386/pc: Wire RTC ISA IRQs in south bridges
>> >>   hw/isa/piix3: Create IDE controller in host device
>> >>   hw/isa/piix3: Create USB controller in host device
>> >>   hw/isa/piix3: Create power management controller in host device
>> >>   hw/isa/piix3: Drop the "3" from PIIX base class name
>> >>   hw/isa/piix4: Remove unused inbound ISA interrupt lines
>> >>   hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in"
>> >>   hw/isa/piix4: Rename reset control operations to match PIIX3
>> >>   hw/isa/piix4: Reuse struct PIIXState from PIIX3
>> >>   hw/isa/piix3: Merge hw/isa/piix4.c
>> >>   hw/isa/piix: Allow for optional PIC creation in PIIX3
>> >>   hw/isa/piix: Allow for optional PIT creation in PIIX3
>> >>   hw/isa/piix: Harmonize names of reset control memory regions
>> >>   hw/isa/piix: Share PIIX3's base class with PIIX4
>> >>   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>> >>   hw/isa/piix: Rename functions to be shared for PCI interrupt
>> >>     triggering
>> >>   hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4
>> >>   hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring
>> >>   hw/isa/piix: Implement multi-process QEMU support also for PIIX4
>> >>   hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine
>> >> 
>> >>  MAINTAINERS                          |   6 +-
>> >>  docs/system/target-i386-desc.rst.inc |   8 +
>> >>  include/hw/i386/pc.h                 |   2 +
>> >>  include/hw/southbridge/piix.h        |  28 ++-
>> >>  hw/i386/pc.c                         |  13 +-
>> >>  hw/i386/pc_piix.c                    | 125 ++++++++---
>> >>  hw/i386/pc_q35.c                     |  14 +-
>> >>  hw/isa/lpc_ich9.c                    |   9 +-
>> >>  hw/isa/{piix3.c => piix.c}           | 281 ++++++++++++++++++-------
>> >>  hw/isa/piix4.c                       | 302 ---------------------------
>> >>  hw/mips/malta.c                      |   5 +-
>> >>  hw/i386/Kconfig                      |   3 +-
>> >>  hw/isa/Kconfig                       |   8 +-
>> >>  hw/isa/meson.build                   |   3 +-
>> >>  hw/mips/Kconfig                      |   2 +-
>> >>  15 files changed, 358 insertions(+), 451 deletions(-)
>> >>  rename hw/isa/{piix3.c => piix.c} (52%)
>> >>  delete mode 100644 hw/isa/piix4.c
>> >> 
>> >
>


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

end of thread, other threads:[~2023-10-13  0:23 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-07 12:38 [PATCH v8 00/29] Consolidate PIIX south bridges Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 01/29] hw/i386/pc: Merge two if statements into one Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 02/29] hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 03/29] hw/i386/pc_piix: Assign PIIX3's ISA interrupts before its realize() Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 04/29] hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 05/29] hw/i386/pc_piix: Wire PIIX3's ISA interrupts by new "isa-irqs" property Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 06/29] hw/i386/pc_piix: Remove redundant "piix3" variable Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 07/29] hw/isa/piix3: Rename "pic" attribute to "isa_irqs_in" Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 08/29] hw/i386/pc_q35: Wire ICH9 LPC function's interrupts before its realize() Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 09/29] hw/isa/piix3: Wire PIC IRQs to ISA bus in host device Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 10/29] hw/i386/pc: Wire RTC ISA IRQs in south bridges Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 11/29] hw/isa/piix3: Create IDE controller in host device Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 12/29] hw/isa/piix3: Create USB " Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 13/29] hw/isa/piix3: Create power management " Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 14/29] hw/isa/piix3: Drop the "3" from PIIX base class name Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 15/29] hw/isa/piix4: Remove unused inbound ISA interrupt lines Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 16/29] hw/isa/piix4: Rename "isa" attribute to "isa_irqs_in" Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 17/29] hw/isa/piix4: Rename reset control operations to match PIIX3 Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 18/29] hw/isa/piix4: Reuse struct PIIXState from PIIX3 Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 19/29] hw/isa/piix3: Merge hw/isa/piix4.c Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 20/29] hw/isa/piix: Allow for optional PIC creation in PIIX3 Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 21/29] hw/isa/piix: Allow for optional PIT " Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 22/29] hw/isa/piix: Harmonize names of reset control memory regions Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 23/29] hw/isa/piix: Share PIIX3's base class with PIIX4 Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 24/29] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 25/29] hw/isa/piix: Rename functions to be shared for PCI interrupt triggering Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 26/29] hw/isa/piix: Reuse PIIX3's PCI interrupt triggering in PIIX4 Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 27/29] hw/isa/piix: Resolve duplicate code regarding PCI interrupt wiring Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 28/29] hw/isa/piix: Implement multi-process QEMU support also for PIIX4 Bernhard Beschow
2023-10-07 12:38 ` [PATCH v8 29/29] hw/i386/pc_piix: Make PIIX4 south bridge usable in PC machine Bernhard Beschow
2023-10-08 17:56 ` [PATCH v8 00/29] Consolidate PIIX south bridges Chuck Zmudzinski
2023-10-11 18:57   ` Bernhard Beschow
2023-10-11 20:18     ` Michael S. Tsirkin
2023-10-12 18:13       ` Bernhard Beschow

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