xen_intx_set_irq() doesn't depend on PIIX state. 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>
---
 hw/i386/pc_piix.c | 12 ++++++++++++
 hw/isa/piix.c     | 24 +-----------------------
 2 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index aacdb72b7c..792dcd3ce8 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -67,6 +67,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 };
@@ -246,6 +247,17 @@ static void pc_init1(MachineState *machine,
                                  &error_abort);
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
+        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);
+        }
+
         dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "pic"));
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 25707479eb..ac04781f46 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -38,8 +38,6 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-#define XEN_IOAPIC_NUM_PIRQS    128ULL
-
 static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
 {
     qemu_set_irq(piix->pic.in_irqs[pic_irq],
@@ -487,33 +485,13 @@ static const TypeInfo piix3_info = {
     .class_init    = piix3_class_init,
 };
 
-static void piix3_xen_realize(PCIDevice *dev, Error **errp)
-{
-    ERRP_GUARD();
-    PIIXState *piix3 = PIIX_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)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->config_write = piix3_write_config_xen;
-    k->realize = piix3_xen_realize;
+    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;
-- 
2.39.0

On Wed, 2023-01-04 at 15:44 +0100, Bernhard Beschow wrote:
> +        if (xen_enabled()) {

Could this perhaps be if (xen_mode != XEN_DISABLED) once we merge the
Xen-on-KVM series?

Am 6. Januar 2023 17:35:18 UTC schrieb David Woodhouse <dwmw2@infradead.org>:
>On Wed, 2023-01-04 at 15:44 +0100, Bernhard Beschow wrote:
>> +        if (xen_enabled()) {
>
>Could this perhaps be if (xen_mode != XEN_DISABLED) once we merge the
>Xen-on-KVM series?

It's the same condition which selected TYPE_PIIX3_XEN_DEVICE until the last patch of this series. If you had to change this condition in your series then just perform the same change here instead.

Best regards,
Bernhard

On 1/6/23 12:35 PM, David Woodhouse wrote:
> On Wed, 2023-01-04 at 15:44 +0100, Bernhard Beschow wrote:
>> +        if (xen_enabled()) {
> 
> Could this perhaps be if (xen_mode != XEN_DISABLED) once we merge the
> Xen-on-KVM series?

I am not an expert and just on here as a user/tester, but I think it
would depend on whether or not the Xen-on-KVM mode uses Xen PCI IRQ
handling or Linux/KVM PCI IRQ handling.

Chuck

On 1/6/23 12:46 PM, Chuck Zmudzinski wrote:
> On 1/6/23 12:35 PM, David Woodhouse wrote:
>> On Wed, 2023-01-04 at 15:44 +0100, Bernhard Beschow wrote:
>>> +        if (xen_enabled()) {
>> 
>> Could this perhaps be if (xen_mode != XEN_DISABLED) once we merge the
>> Xen-on-KVM series?
> 
> I am not an expert and just on here as a user/tester, but I think it
> would depend on whether or not the Xen-on-KVM mode uses Xen PCI IRQ
> handling or Linux/KVM PCI IRQ handling.
> 
> Chuck

I could try Bernhard's patches in a Xen-on-KVM configuration if that might help.
I would need help configuring the Xen-on-KVM mode to do it, though, because
I have never tried Xen-on-KVM before.

The test I could do would be to:

1) Test my Xen guest that uses the current PIIX3-xen device in the
Xen-on-KVM environment and get that working.

2) Apply Bernhard's patch series and see what, if anything, needs to
be done to make Bernhard's patch work in Xen-on-KVM. I have already
verified that Bernhard's patches work well with Xen on bare metal, so I
don't think we need to answer this question before going forward with
Bernhard's patches. This can be patched later to support Xen-on-KVM if
necessary as part of or in addition to the Xen-on-KVM series.

Chuck

On 4/1/23 15:44, Bernhard Beschow wrote:
> xen_intx_set_irq() doesn't depend on PIIX state. 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>
> ---
>   hw/i386/pc_piix.c | 12 ++++++++++++
>   hw/isa/piix.c     | 24 +-----------------------
>   2 files changed, 13 insertions(+), 23 deletions(-)

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