[PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

There is currently a dedicated PIIX3 device model for use under Xen. By reusing
existing PCI API during initialization this device model can be eliminated and
the plain PIIX3 device model can be used instead.

Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
agnostic towards the precise south bridge being used in the PC machine. The
latter might become particularily interesting once PIIX4 becomes usable in the
PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.

Testing done:
- `make check`
- Run `xl create` with the following config:
    name = "Manjaro"
    type = 'hvm'
    memory = 1536
    apic = 1
    usb = 1
    disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
    device_model_override = "/usr/bin/qemu-system-x86_64"
    vga = "stdvga"
    sdl = 1
- `qemu-system-x86_64 -M pc -m 2G -cpu host -accel kvm \
    -cdrom manjaro-kde-21.2.6-220416-linux515.iso`

v4:
- Add patch fixing latent memory leak in pci_bus_irqs() (Anthony)

v3:
- Rebase onto master

v2:
- xen_piix3_set_irq() is already generic. Just rename it. (Chuck)

Tested-by: Chuck Zmudzinski <brchuckz@aol.com>

Bernhard Beschow (7):
  include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()
  hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()
  hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
  hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3
  hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()
  hw/isa/piix3: Resolve redundant k->config_write assignments
  hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE

 include/hw/southbridge/piix.h |  1 -
 include/hw/xen/xen.h          |  2 +-
 hw/i386/pc_piix.c             | 36 +++++++++++++++++++--
 hw/i386/xen/xen-hvm.c         |  2 +-
 hw/isa/piix3.c                | 60 +----------------------------------
 hw/pci/pci.c                  |  2 ++
 stubs/xen-hw-stub.c           |  2 +-
 7 files changed, 39 insertions(+), 66 deletions(-)

-- 
2.40.0

[PATCH v4 1/7] include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()

[Bernhard Beschow]

xen_piix3_set_irq() isn't PIIX specific: PIIX is a single PCI device
while xen_piix3_set_irq() maps multiple PCI devices to their respective
IRQs, which is board-specific. Rename xen_piix3_set_irq() to communicate
this.

Also rename XEN_PIIX_NUM_PIRQS to XEN_IOAPIC_NUM_PIRQS since the Xen's
IOAPIC rather than PIIX has this many interrupt routes.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
Message-Id: <20230312120221.99183-2-shentey@gmail.com>
---
 include/hw/xen/xen.h  | 2 +-
 hw/i386/xen/xen-hvm.c | 2 +-
 hw/isa/piix3.c        | 4 ++--
 stubs/xen-hw-stub.c   | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 2bd8ec742d..37ecc91fc3 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -39,7 +39,7 @@ extern bool xen_domid_restrict;
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 int xen_set_pci_link_route(uint8_t link, uint8_t irq);
-void xen_piix3_set_irq(void *opaque, int irq_num, int level);
+void xen_intx_set_irq(void *opaque, int irq_num, int level);
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
 int xen_is_pirq_msi(uint32_t msi_data);
 
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 56641a550e..ab8f1b61ee 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -143,7 +143,7 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
     return irq_num + (PCI_SLOT(pci_dev->devfn) << 2);
 }
 
-void xen_piix3_set_irq(void *opaque, int irq_num, int level)
+void xen_intx_set_irq(void *opaque, int irq_num, int level)
 {
     xen_set_pci_intx_level(xen_domid, 0, 0, irq_num >> 2,
                            irq_num & 3, level);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index a9cb39bf21..1b3e23f0d7 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -34,7 +34,7 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-#define XEN_PIIX_NUM_PIRQS      128ULL
+#define XEN_IOAPIC_NUM_PIRQS    128ULL
 
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
@@ -405,7 +405,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
      * connected to the IOAPIC directly.
      * These additional routes can be discovered through ACPI.
      */
-    pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_IOAPIC_NUM_PIRQS);
 }
 
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 34a22f2ad7..7d7ffe83a9 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -15,7 +15,7 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
     return -1;
 }
 
-void xen_piix3_set_irq(void *opaque, int irq_num, int level)
+void xen_intx_set_irq(void *opaque, int irq_num, int level)
 {
 }
 
-- 
2.40.0

[PATCH v4 2/7] hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()

[Bernhard Beschow]

When calling pci_bus_irqs() multiple times on the same object without calling
pci_bus_irqs_cleanup() in between PCIBus::irq_count[] is currently leaked.
Let's fix this because Xen will do just that in a few commits, and because
calling pci_bus_irqs_cleanup() in between seems fragile and cumbersome.

Note that pci_bus_irqs_cleanup() now has to NULL irq_count such that
pci_bus_irqs() doesn't do a double free.

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

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index def5000e7b..be1c5d16ec 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -558,6 +558,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
     bus->set_irq = set_irq;
     bus->irq_opaque = irq_opaque;
     bus->nirq = nirq;
+    g_free(bus->irq_count);
     bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
 }
 
@@ -573,6 +574,7 @@ void pci_bus_irqs_cleanup(PCIBus *bus)
     bus->irq_opaque = NULL;
     bus->nirq = 0;
     g_free(bus->irq_count);
+    bus->irq_count = NULL;
 }
 
 PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
-- 
2.40.0

[PATCH v4 3/7] hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()

[Bernhard Beschow]

This is a preparational patch for the next one to make the following
more obvious:

First, pci_bus_irqs() is now called twice in case of Xen where the
second call overrides the pci_set_irq_fn with the Xen variant.

Second, pci_bus_set_route_irq_fn() is now also called in Xen mode.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
Message-Id: <20230312120221.99183-3-shentey@gmail.com>
---
 hw/isa/piix3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 1b3e23f0d7..a86cd23ef4 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -394,7 +394,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
     PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
-    pci_piix3_realize(dev, errp);
+    piix3_realize(dev, errp);
     if (*errp) {
         return;
     }
-- 
2.40.0

[PATCH v4 4/7] hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3

[Bernhard Beschow]

xen_intx_set_irq() doesn't depend on PIIX3State. In order to resolve
TYPE_PIIX3_XEN_DEVICE and in order to make Xen agnostic about the
precise south bridge being used, set up Xen's PCI IRQ handling of PIIX3
in the board.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
Message-Id: <20230312120221.99183-4-shentey@gmail.com>
---
 hw/i386/pc_piix.c | 13 +++++++++++++
 hw/isa/piix3.c    | 24 +-----------------------
 2 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 30eedd62a3..99232701b1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -69,6 +69,7 @@
 #include "kvm/kvm-cpu.h"
 
 #define MAX_IDE_BUS 2
+#define XEN_IOAPIC_NUM_PIRQS 128ULL
 
 #ifdef CONFIG_IDE_ISA
 static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
@@ -236,6 +237,18 @@ static void pc_init1(MachineState *machine,
         pcms->bus = pci_bus;
 
         pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+
+        if (xen_enabled()) {
+            /*
+             * Xen supports additional interrupt routes from the PCI devices to
+             * the IOAPIC: the four pins of each PCI device on the bus are also
+             * connected to the IOAPIC directly.
+             * These additional routes can be discovered through ACPI.
+             */
+            pci_bus_irqs(pci_bus, xen_intx_set_irq, pci_dev,
+                         XEN_IOAPIC_NUM_PIRQS);
+        }
+
         piix3 = PIIX3_PCI_DEVICE(pci_dev);
         piix3->pic = x86ms->gsi;
         piix3_devfn = piix3->dev.devfn;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index a86cd23ef4..7a31caf2b6 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -34,8 +34,6 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-#define XEN_IOAPIC_NUM_PIRQS    128ULL
-
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
     qemu_set_irq(piix3->pic[pic_irq],
@@ -388,32 +386,12 @@ static const TypeInfo piix3_info = {
     .class_init    = piix3_class_init,
 };
 
-static void piix3_xen_realize(PCIDevice *dev, Error **errp)
-{
-    ERRP_GUARD();
-    PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
-    PCIBus *pci_bus = pci_get_bus(dev);
-
-    piix3_realize(dev, errp);
-    if (*errp) {
-        return;
-    }
-
-    /*
-     * Xen supports additional interrupt routes from the PCI devices to
-     * the IOAPIC: the four pins of each PCI device on the bus are also
-     * connected to the IOAPIC directly.
-     * These additional routes can be discovered through ACPI.
-     */
-    pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_IOAPIC_NUM_PIRQS);
-}
-
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->config_write = piix3_write_config_xen;
-    k->realize = piix3_xen_realize;
+    k->realize = piix3_realize;
 }
 
 static const TypeInfo piix3_xen_info = {
-- 
2.40.0

[PATCH v4 5/7] hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()

[Bernhard Beschow]

Subscribe to pci_bus_fire_intx_routing_notifier() instead which allows for
having a common piix3_write_config() for the PIIX3 device models.

While at it, move the subscription into machine code to facilitate resolving
TYPE_PIIX3_XEN_DEVICE.

In a possible future followup, pci_bus_fire_intx_routing_notifier() could
be adjusted in such a way that subscribing to it doesn't require
knowledge of the device firing it.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
Message-Id: <20230312120221.99183-5-shentey@gmail.com>
---
 hw/i386/pc_piix.c | 18 ++++++++++++++++++
 hw/isa/piix3.c    | 22 +---------------------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 99232701b1..1b70470dcd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -88,6 +88,21 @@ static int pc_pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
     return (pci_intx + slot_addend) & 3;
 }
 
+static void piix_intx_routing_notifier_xen(PCIDevice *dev)
+{
+    int i;
+
+    /* Scan for updates to PCI link routes (0x60-0x63). */
+    for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+        uint8_t v = dev->config_read(dev, PIIX_PIRQCA + i, 1);
+        if (v & 0x80) {
+            v = 0;
+        }
+        v &= 0xf;
+        xen_set_pci_link_route(i, v);
+    }
+}
+
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      const char *host_type, const char *pci_type)
@@ -239,6 +254,9 @@ static void pc_init1(MachineState *machine,
         pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
 
         if (xen_enabled()) {
+            pci_device_set_intx_routing_notifier(
+                        pci_dev, piix_intx_routing_notifier_xen);
+
             /*
              * Xen supports additional interrupt routes from the PCI devices to
              * the IOAPIC: the four pins of each PCI device on the bus are also
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 7a31caf2b6..737f5c6a5d 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -121,26 +121,6 @@ static void piix3_write_config(PCIDevice *dev,
     }
 }
 
-static void piix3_write_config_xen(PCIDevice *dev,
-                                   uint32_t address, uint32_t val, int len)
-{
-    int i;
-
-    /* Scan for updates to PCI link routes (0x60-0x63). */
-    for (i = 0; i < len; i++) {
-        uint8_t v = (val >> (8 * i)) & 0xff;
-        if (v & 0x80) {
-            v = 0;
-        }
-        v &= 0xf;
-        if (((address + i) >= PIIX_PIRQCA) && ((address + i) <= PIIX_PIRQCD)) {
-            xen_set_pci_link_route(address + i - PIIX_PIRQCA, v);
-        }
-    }
-
-    piix3_write_config(dev, address, val, len);
-}
-
 static void piix3_reset(DeviceState *dev)
 {
     PIIX3State *d = PIIX3_PCI_DEVICE(dev);
@@ -390,7 +370,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix3_write_config_xen;
+    k->config_write = piix3_write_config;
     k->realize = piix3_realize;
 }
 
-- 
2.40.0

[PATCH v4 6/7] hw/isa/piix3: Resolve redundant k->config_write assignments

[Bernhard Beschow]

The previous patch unified handling of piix3_write_config() accross the
PIIX3 device models which allows for assigning k->config_write once in the
base class.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
Message-Id: <20230312120221.99183-6-shentey@gmail.com>
---
 hw/isa/piix3.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 737f5c6a5d..418940139d 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -308,6 +308,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
+    k->config_write = piix3_write_config;
     dc->reset       = piix3_reset;
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
@@ -356,7 +357,6 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix3_write_config;
     k->realize = piix3_realize;
 }
 
@@ -370,7 +370,6 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix3_write_config;
     k->realize = piix3_realize;
 }
 
-- 
2.40.0

[PATCH v4 7/7] hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of
TYPE_PIIX3_DEVICE. Remove this redundancy.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
Message-Id: <20230312120221.99183-7-shentey@gmail.com>
---
 include/hw/southbridge/piix.h |  1 -
 hw/i386/pc_piix.c             |  5 ++---
 hw/isa/piix3.c                | 15 ---------------
 3 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0bf48e936d..51be04e984 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -64,7 +64,6 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
                          TYPE_PIIX3_PCI_DEVICE)
 
 #define TYPE_PIIX3_DEVICE "PIIX3"
-#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
 
 #endif
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 1b70470dcd..7ca0d6d14e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -237,8 +237,6 @@ static void pc_init1(MachineState *machine,
     if (pcmc->pci_enabled) {
         PIIX3State *piix3;
         PCIDevice *pci_dev;
-        const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
-                                         : TYPE_PIIX3_DEVICE;
 
         pci_bus = i440fx_init(pci_type,
                               i440fx_host,
@@ -251,7 +249,8 @@ static void pc_init1(MachineState *machine,
                                        : pc_pci_slot_get_pirq);
         pcms->bus = pci_bus;
 
-        pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+        pci_dev = pci_create_simple_multifunction(pci_bus, -1, true,
+                                                  TYPE_PIIX3_DEVICE);
 
         if (xen_enabled()) {
             pci_device_set_intx_routing_notifier(
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 418940139d..0d6992af67 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -29,7 +29,6 @@
 #include "hw/southbridge/piix.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
-#include "hw/xen/xen.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
@@ -366,24 +365,10 @@ static const TypeInfo piix3_info = {
     .class_init    = piix3_class_init,
 };
 
-static void piix3_xen_class_init(ObjectClass *klass, void *data)
-{
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->realize = piix3_realize;
-}
-
-static const TypeInfo piix3_xen_info = {
-    .name          = TYPE_PIIX3_XEN_DEVICE,
-    .parent        = TYPE_PIIX3_PCI_DEVICE,
-    .class_init    = piix3_xen_class_init,
-};
-
 static void piix3_register_types(void)
 {
     type_register_static(&piix3_pci_type_info);
     type_register_static(&piix3_info);
-    type_register_static(&piix3_xen_info);
 }
 
 type_init(piix3_register_types)
-- 
2.40.0

Re: [PATCH v4 2/7] hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()

[Bernhard Beschow]

Am 3. April 2023 07:41:19 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>When calling pci_bus_irqs() multiple times on the same object without calling
>pci_bus_irqs_cleanup() in between PCIBus::irq_count[] is currently leaked.
>Let's fix this because Xen will do just that in a few commits, and because
>calling pci_bus_irqs_cleanup() in between seems fragile and cumbersome.
>
>Note that pci_bus_irqs_cleanup() now has to NULL irq_count such that
>pci_bus_irqs() doesn't do a double free.
>
>Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Ping PCI maintainers

>---
> hw/pci/pci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>index def5000e7b..be1c5d16ec 100644
>--- a/hw/pci/pci.c
>+++ b/hw/pci/pci.c
>@@ -558,6 +558,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
>     bus->set_irq = set_irq;
>     bus->irq_opaque = irq_opaque;
>     bus->nirq = nirq;
>+    g_free(bus->irq_count);
>     bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> }
> 
>@@ -573,6 +574,7 @@ void pci_bus_irqs_cleanup(PCIBus *bus)
>     bus->irq_opaque = NULL;
>     bus->nirq = 0;
>     g_free(bus->irq_count);
>+    bus->irq_count = NULL;
> }
> 
> PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,

Re: [PATCH v4 2/7] hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 09:41:19AM +0200, Bernhard Beschow wrote:
> When calling pci_bus_irqs() multiple times on the same object without calling
> pci_bus_irqs_cleanup() in between PCIBus::irq_count[] is currently leaked.
> Let's fix this because Xen will do just that in a few commits, and because
> calling pci_bus_irqs_cleanup() in between seems fragile and cumbersome.
> 
> Note that pci_bus_irqs_cleanup() now has to NULL irq_count such that
> pci_bus_irqs() doesn't do a double free.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

ok

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


> ---
>  hw/pci/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index def5000e7b..be1c5d16ec 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -558,6 +558,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
>      bus->set_irq = set_irq;
>      bus->irq_opaque = irq_opaque;
>      bus->nirq = nirq;
> +    g_free(bus->irq_count);
>      bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
>  }
>  
> @@ -573,6 +574,7 @@ void pci_bus_irqs_cleanup(PCIBus *bus)
>      bus->irq_opaque = NULL;
>      bus->nirq = 0;
>      g_free(bus->irq_count);
> +    bus->irq_count = NULL;
>  }
>  
>  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> -- 
> 2.40.0

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
> existing PCI API during initialization this device model can be eliminated and
> the plain PIIX3 device model can be used instead.
> 
> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
> agnostic towards the precise south bridge being used in the PC machine. The
> latter might become particularily interesting once PIIX4 becomes usable in the
> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.

xen stuff so I assume that tree?

> Testing done:
> - `make check`
> - Run `xl create` with the following config:
>     name = "Manjaro"
>     type = 'hvm'
>     memory = 1536
>     apic = 1
>     usb = 1
>     disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
>     device_model_override = "/usr/bin/qemu-system-x86_64"
>     vga = "stdvga"
>     sdl = 1
> - `qemu-system-x86_64 -M pc -m 2G -cpu host -accel kvm \
>     -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
> 
> v4:
> - Add patch fixing latent memory leak in pci_bus_irqs() (Anthony)
> 
> v3:
> - Rebase onto master
> 
> v2:
> - xen_piix3_set_irq() is already generic. Just rename it. (Chuck)
> 
> Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
> 
> Bernhard Beschow (7):
>   include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()
>   hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()
>   hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
>   hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3
>   hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()
>   hw/isa/piix3: Resolve redundant k->config_write assignments
>   hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE
> 
>  include/hw/southbridge/piix.h |  1 -
>  include/hw/xen/xen.h          |  2 +-
>  hw/i386/pc_piix.c             | 36 +++++++++++++++++++--
>  hw/i386/xen/xen-hvm.c         |  2 +-
>  hw/isa/piix3.c                | 60 +----------------------------------
>  hw/pci/pci.c                  |  2 ++
>  stubs/xen-hw-stub.c           |  2 +-
>  7 files changed, 39 insertions(+), 66 deletions(-)
> 
> -- 
> 2.40.0
> 

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
>> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
>> existing PCI API during initialization this device model can be eliminated and
>> the plain PIIX3 device model can be used instead.
>> 
>> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
>> agnostic towards the precise south bridge being used in the PC machine. The
>> latter might become particularily interesting once PIIX4 becomes usable in the
>> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>
>xen stuff so I assume that tree?

Anthony?

This series is now fully reviewed. Once it lands in master I'd rebase the PIIX consolidation series onto it which is still under discussion.

Best regards,
Bernhard

>
>> Testing done:
>> - `make check`
>> - Run `xl create` with the following config:
>>     name = "Manjaro"
>>     type = 'hvm'
>>     memory = 1536
>>     apic = 1
>>     usb = 1
>>     disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
>>     device_model_override = "/usr/bin/qemu-system-x86_64"
>>     vga = "stdvga"
>>     sdl = 1
>> - `qemu-system-x86_64 -M pc -m 2G -cpu host -accel kvm \
>>     -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
>> 
>> v4:
>> - Add patch fixing latent memory leak in pci_bus_irqs() (Anthony)
>> 
>> v3:
>> - Rebase onto master
>> 
>> v2:
>> - xen_piix3_set_irq() is already generic. Just rename it. (Chuck)
>> 
>> Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
>> 
>> Bernhard Beschow (7):
>>   include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()
>>   hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()
>>   hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
>>   hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3
>>   hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()
>>   hw/isa/piix3: Resolve redundant k->config_write assignments
>>   hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>> 
>>  include/hw/southbridge/piix.h |  1 -
>>  include/hw/xen/xen.h          |  2 +-
>>  hw/i386/pc_piix.c             | 36 +++++++++++++++++++--
>>  hw/i386/xen/xen-hvm.c         |  2 +-
>>  hw/isa/piix3.c                | 60 +----------------------------------
>>  hw/pci/pci.c                  |  2 ++
>>  stubs/xen-hw-stub.c           |  2 +-
>>  7 files changed, 39 insertions(+), 66 deletions(-)
>> 
>> -- 
>> 2.40.0
>> 
>

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
>> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
>> existing PCI API during initialization this device model can be eliminated and
>> the plain PIIX3 device model can be used instead.
>> 
>> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
>> agnostic towards the precise south bridge being used in the PC machine. The
>> latter might become particularily interesting once PIIX4 becomes usable in the
>> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>
>xen stuff so I assume that tree?

Ping

>
>> Testing done:
>> - `make check`
>> - Run `xl create` with the following config:
>>     name = "Manjaro"
>>     type = 'hvm'
>>     memory = 1536
>>     apic = 1
>>     usb = 1
>>     disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
>>     device_model_override = "/usr/bin/qemu-system-x86_64"
>>     vga = "stdvga"
>>     sdl = 1
>> - `qemu-system-x86_64 -M pc -m 2G -cpu host -accel kvm \
>>     -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
>> 
>> v4:
>> - Add patch fixing latent memory leak in pci_bus_irqs() (Anthony)
>> 
>> v3:
>> - Rebase onto master
>> 
>> v2:
>> - xen_piix3_set_irq() is already generic. Just rename it. (Chuck)
>> 
>> Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
>> 
>> Bernhard Beschow (7):
>>   include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()
>>   hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()
>>   hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
>>   hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3
>>   hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()
>>   hw/isa/piix3: Resolve redundant k->config_write assignments
>>   hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>> 
>>  include/hw/southbridge/piix.h |  1 -
>>  include/hw/xen/xen.h          |  2 +-
>>  hw/i386/pc_piix.c             | 36 +++++++++++++++++++--
>>  hw/i386/xen/xen-hvm.c         |  2 +-
>>  hw/isa/piix3.c                | 60 +----------------------------------
>>  hw/pci/pci.c                  |  2 ++
>>  stubs/xen-hw-stub.c           |  2 +-
>>  7 files changed, 39 insertions(+), 66 deletions(-)
>> 
>> -- 
>> 2.40.0
>> 
>

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Stefano Stabellini]

On Sat, 13 May 2023, Bernhard Beschow wrote:
> Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
> >On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
> >> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
> >> existing PCI API during initialization this device model can be eliminated and
> >> the plain PIIX3 device model can be used instead.
> >> 
> >> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
> >> agnostic towards the precise south bridge being used in the PC machine. The
> >> latter might become particularily interesting once PIIX4 becomes usable in the
> >> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
> >
> >xen stuff so I assume that tree?
> 
> Ping

I am OK either way. Michael, what do you prefer?

Normally I would suggest for you to pick up the patches. But as it
happens I'll have to likely send another pull request in a week or two
and I can add these patches to it.

Let me know your preference and I am happy to follow it.


> >
> >> Testing done:
> >> - `make check`
> >> - Run `xl create` with the following config:
> >>     name = "Manjaro"
> >>     type = 'hvm'
> >>     memory = 1536
> >>     apic = 1
> >>     usb = 1
> >>     disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
> >>     device_model_override = "/usr/bin/qemu-system-x86_64"
> >>     vga = "stdvga"
> >>     sdl = 1
> >> - `qemu-system-x86_64 -M pc -m 2G -cpu host -accel kvm \
> >>     -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
> >> 
> >> v4:
> >> - Add patch fixing latent memory leak in pci_bus_irqs() (Anthony)
> >> 
> >> v3:
> >> - Rebase onto master
> >> 
> >> v2:
> >> - xen_piix3_set_irq() is already generic. Just rename it. (Chuck)
> >> 
> >> Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
> >> 
> >> Bernhard Beschow (7):
> >>   include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()
> >>   hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()
> >>   hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
> >>   hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3
> >>   hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()
> >>   hw/isa/piix3: Resolve redundant k->config_write assignments
> >>   hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE
> >> 
> >>  include/hw/southbridge/piix.h |  1 -
> >>  include/hw/xen/xen.h          |  2 +-
> >>  hw/i386/pc_piix.c             | 36 +++++++++++++++++++--
> >>  hw/i386/xen/xen-hvm.c         |  2 +-
> >>  hw/isa/piix3.c                | 60 +----------------------------------
> >>  hw/pci/pci.c                  |  2 ++
> >>  stubs/xen-hw-stub.c           |  2 +-
> >>  7 files changed, 39 insertions(+), 66 deletions(-)
> >> 
> >> -- 
> >> 2.40.0
> >> 
> >
> 

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

Am 15. Mai 2023 20:52:40 UTC schrieb Stefano Stabellini <sstabellini@kernel.org>:
>On Sat, 13 May 2023, Bernhard Beschow wrote:
>> Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>> >On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
>> >> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
>> >> existing PCI API during initialization this device model can be eliminated and
>> >> the plain PIIX3 device model can be used instead.
>> >> 
>> >> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
>> >> agnostic towards the precise south bridge being used in the PC machine. The
>> >> latter might become particularily interesting once PIIX4 becomes usable in the
>> >> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>> >
>> >xen stuff so I assume that tree?
>> 
>> Ping
>
>I am OK either way. Michael, what do you prefer?
>
>Normally I would suggest for you to pick up the patches. But as it
>happens I'll have to likely send another pull request in a week or two
>and I can add these patches to it.
>
>Let me know your preference and I am happy to follow it.

Hi Stefano,

Michael's PR was merged last week. How about including this series into your PR then?

Best regards,
Bernhard

>
>
>> >
>> >> Testing done:
>> >> - `make check`
>> >> - Run `xl create` with the following config:
>> >>     name = "Manjaro"
>> >>     type = 'hvm'
>> >>     memory = 1536
>> >>     apic = 1
>> >>     usb = 1
>> >>     disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
>> >>     device_model_override = "/usr/bin/qemu-system-x86_64"
>> >>     vga = "stdvga"
>> >>     sdl = 1
>> >> - `qemu-system-x86_64 -M pc -m 2G -cpu host -accel kvm \
>> >>     -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
>> >> 
>> >> v4:
>> >> - Add patch fixing latent memory leak in pci_bus_irqs() (Anthony)
>> >> 
>> >> v3:
>> >> - Rebase onto master
>> >> 
>> >> v2:
>> >> - xen_piix3_set_irq() is already generic. Just rename it. (Chuck)
>> >> 
>> >> Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
>> >> 
>> >> Bernhard Beschow (7):
>> >>   include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()
>> >>   hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()
>> >>   hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
>> >>   hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3
>> >>   hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()
>> >>   hw/isa/piix3: Resolve redundant k->config_write assignments
>> >>   hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>> >> 
>> >>  include/hw/southbridge/piix.h |  1 -
>> >>  include/hw/xen/xen.h          |  2 +-
>> >>  hw/i386/pc_piix.c             | 36 +++++++++++++++++++--
>> >>  hw/i386/xen/xen-hvm.c         |  2 +-
>> >>  hw/isa/piix3.c                | 60 +----------------------------------
>> >>  hw/pci/pci.c                  |  2 ++
>> >>  stubs/xen-hw-stub.c           |  2 +-
>> >>  7 files changed, 39 insertions(+), 66 deletions(-)
>> >> 
>> >> -- 
>> >> 2.40.0
>> >> 
>> >
>> 

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

Am 22. Mai 2023 15:42:03 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 15. Mai 2023 20:52:40 UTC schrieb Stefano Stabellini <sstabellini@kernel.org>:
>>On Sat, 13 May 2023, Bernhard Beschow wrote:
>>> Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>>> >On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
>>> >> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
>>> >> existing PCI API during initialization this device model can be eliminated and
>>> >> the plain PIIX3 device model can be used instead.
>>> >> 
>>> >> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
>>> >> agnostic towards the precise south bridge being used in the PC machine. The
>>> >> latter might become particularily interesting once PIIX4 becomes usable in the
>>> >> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>>> >
>>> >xen stuff so I assume that tree?
>>> 
>>> Ping
>>
>>I am OK either way. Michael, what do you prefer?
>>
>>Normally I would suggest for you to pick up the patches. But as it
>>happens I'll have to likely send another pull request in a week or two
>>and I can add these patches to it.
>>
>>Let me know your preference and I am happy to follow it.
>
>Hi Stefano,
>
>Michael's PR was merged last week. How about including this series into your PR then?

Ping

>
>Best regards,
>Bernhard
>
>>
>>
>>> >
>>> >> Testing done:
>>> >> - `make check`
>>> >> - Run `xl create` with the following config:
>>> >>     name = "Manjaro"
>>> >>     type = 'hvm'
>>> >>     memory = 1536
>>> >>     apic = 1
>>> >>     usb = 1
>>> >>     disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
>>> >>     device_model_override = "/usr/bin/qemu-system-x86_64"
>>> >>     vga = "stdvga"
>>> >>     sdl = 1
>>> >> - `qemu-system-x86_64 -M pc -m 2G -cpu host -accel kvm \
>>> >>     -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
>>> >> 
>>> >> v4:
>>> >> - Add patch fixing latent memory leak in pci_bus_irqs() (Anthony)
>>> >> 
>>> >> v3:
>>> >> - Rebase onto master
>>> >> 
>>> >> v2:
>>> >> - xen_piix3_set_irq() is already generic. Just rename it. (Chuck)
>>> >> 
>>> >> Tested-by: Chuck Zmudzinski <brchuckz@aol.com>
>>> >> 
>>> >> Bernhard Beschow (7):
>>> >>   include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq()
>>> >>   hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs()
>>> >>   hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()
>>> >>   hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3
>>> >>   hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()
>>> >>   hw/isa/piix3: Resolve redundant k->config_write assignments
>>> >>   hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>>> >> 
>>> >>  include/hw/southbridge/piix.h |  1 -
>>> >>  include/hw/xen/xen.h          |  2 +-
>>> >>  hw/i386/pc_piix.c             | 36 +++++++++++++++++++--
>>> >>  hw/i386/xen/xen-hvm.c         |  2 +-
>>> >>  hw/isa/piix3.c                | 60 +----------------------------------
>>> >>  hw/pci/pci.c                  |  2 ++
>>> >>  stubs/xen-hw-stub.c           |  2 +-
>>> >>  7 files changed, 39 insertions(+), 66 deletions(-)
>>> >> 
>>> >> -- 
>>> >> 2.40.0
>>> >> 
>>> >
>>> 

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Stefano Stabellini]

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

On Mon, 5 Jun 2023, Bernhard Beschow wrote:
> Am 22. Mai 2023 15:42:03 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
> >
> >
> >Am 15. Mai 2023 20:52:40 UTC schrieb Stefano Stabellini <sstabellini@kernel.org>:
> >>On Sat, 13 May 2023, Bernhard Beschow wrote:
> >>> Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
> >>> >On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
> >>> >> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
> >>> >> existing PCI API during initialization this device model can be eliminated and
> >>> >> the plain PIIX3 device model can be used instead.
> >>> >> 
> >>> >> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
> >>> >> agnostic towards the precise south bridge being used in the PC machine. The
> >>> >> latter might become particularily interesting once PIIX4 becomes usable in the
> >>> >> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
> >>> >
> >>> >xen stuff so I assume that tree?
> >>> 
> >>> Ping
> >>
> >>I am OK either way. Michael, what do you prefer?
> >>
> >>Normally I would suggest for you to pick up the patches. But as it
> >>happens I'll have to likely send another pull request in a week or two
> >>and I can add these patches to it.
> >>
> >>Let me know your preference and I am happy to follow it.
> >
> >Hi Stefano,
> >
> >Michael's PR was merged last week. How about including this series into your PR then?
> 
> Ping

Sorry for the late reply, it looks like patch #3 breaks the build:

[1888/4025] Compiling C object libcommon.fa.p/hw_isa_piix3.c.o
FAILED: libcommon.fa.p/hw_isa_piix3.c.o 
cc -m64 -mcx16 -Ilibcommon.fa.p -Iui -I../ui -I/usr/include/capstone -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/p11-kit-1 -I/usr/include/libusb-1.0 -I/usr/include/SDL2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gio-unix-2.0 -I/usr/include/slirp -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/fribidi -I/usr/include/uuid -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/at-spi-2.0 -I/usr/include/vte-2.91 -I/usr/include/virgl -I/usr/include/cacard -I/usr/include/nss -I/usr/include/nspr -I/usr/include/PCSC -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURC
 E=2 -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -isystem /builds/Vikram.garhwal/qemu-ioreq/linux-headers -isystem linux-headers -iquote . -iquote /builds/Vikram.garhwal/qemu-ioreq -iquote /builds/Vikram.garhwal/qemu-ioreq/include -iquote /builds/Vikram.garhwal/qemu-ioreq/host/include/x86_64 -iquote /builds/Vikram.garhwal/qemu-ioreq/host/include/generic -iquote /builds/Vikram.garhwal/qemu-ioreq/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600 -DNCURSES_WIDECHAR=1 -D_REENTRANT -Wno-undef -DSTRUCT_IOVEC_DEFINED -MD -MQ libcommon.fa.p/hw
 _isa_piix3.c.o -MF libcommon.fa.p/hw_isa_piix3.c.o.d -o libcommon.fa.p/hw_isa_piix3.c.o -c ../hw/isa/piix3.c
../hw/isa/piix3.c:265:13: error: ‘pci_piix3_realize’ defined but not used [-Werror=unused-function]
  265 | static void pci_piix3_realize(PCIDevice *dev, Error **errp)
      |             ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Stefano Stabellini]

On Thu, 8 Jun 2023, Stefano Stabellini wrote:
> On Mon, 5 Jun 2023, Bernhard Beschow wrote:
> > Am 22. Mai 2023 15:42:03 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
> > >
> > >
> > >Am 15. Mai 2023 20:52:40 UTC schrieb Stefano Stabellini <sstabellini@kernel.org>:
> > >>On Sat, 13 May 2023, Bernhard Beschow wrote:
> > >>> Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
> > >>> >On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
> > >>> >> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
> > >>> >> existing PCI API during initialization this device model can be eliminated and
> > >>> >> the plain PIIX3 device model can be used instead.
> > >>> >> 
> > >>> >> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
> > >>> >> agnostic towards the precise south bridge being used in the PC machine. The
> > >>> >> latter might become particularily interesting once PIIX4 becomes usable in the
> > >>> >> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
> > >>> >
> > >>> >xen stuff so I assume that tree?
> > >>> 
> > >>> Ping
> > >>
> > >>I am OK either way. Michael, what do you prefer?
> > >>
> > >>Normally I would suggest for you to pick up the patches. But as it
> > >>happens I'll have to likely send another pull request in a week or two
> > >>and I can add these patches to it.
> > >>
> > >>Let me know your preference and I am happy to follow it.
> > >
> > >Hi Stefano,
> > >
> > >Michael's PR was merged last week. How about including this series into your PR then?
> > 
> > Ping
> 
> Sorry for the late reply, it looks like patch #3 breaks the build:

I noticed now that this patch series got committed (the right version
without the build failure), thanks Anthony for sending the pull request!

Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

[Anthony PERARD]

On Thu, Jun 08, 2023 at 03:43:32PM -0700, Stefano Stabellini wrote:
> On Mon, 5 Jun 2023, Bernhard Beschow wrote:
> > Am 22. Mai 2023 15:42:03 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
> > >
> > >
> > >Am 15. Mai 2023 20:52:40 UTC schrieb Stefano Stabellini <sstabellini@kernel.org>:
> > >>On Sat, 13 May 2023, Bernhard Beschow wrote:
> > >>> Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
> > >>> >On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
> > >>> >> There is currently a dedicated PIIX3 device model for use under Xen. By reusing
> > >>> >> existing PCI API during initialization this device model can be eliminated and
> > >>> >> the plain PIIX3 device model can be used instead.
> > >>> >> 
> > >>> >> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also making Xen
> > >>> >> agnostic towards the precise south bridge being used in the PC machine. The
> > >>> >> latter might become particularily interesting once PIIX4 becomes usable in the
> > >>> >> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
> > >>> >
> > >>> >xen stuff so I assume that tree?
> > >>> 
> > >>> Ping
> > >>
> > >>I am OK either way. Michael, what do you prefer?
> > >>
> > >>Normally I would suggest for you to pick up the patches. But as it
> > >>happens I'll have to likely send another pull request in a week or two
> > >>and I can add these patches to it.
> > >>
> > >>Let me know your preference and I am happy to follow it.
> > >
> > >Hi Stefano,
> > >
> > >Michael's PR was merged last week. How about including this series into your PR then?
> > 
> > Ping
> 
> Sorry for the late reply, it looks like patch #3 breaks the build:

Hi Stefano,

Sorry I forgot to reply to these mails. I've sent a pull request for
this earlier this week (along with other patches I had to send), so the
series should be applied now.

I guess the build issue is due to trying to apply the same patch again.

Cheers,

-- 
Anthony PERARD