Xen devel - Jun 2013 - [PATCH 2/3] V3 qemu-xen-trad: Correctly expose PCH ISA bridge for IGD passthrough

I use V3 here to avoid confusing with any similar named patch sent out before.
I have no idea how to probe the host bus, so that part is not changed.

The i915 driver probes chip version through PCH ISA bridge device / vendor ID.
Previously, the PCH ISA bridge is exposed as PCI-PCI bridge in qemu-xen-trad,
which breaks the assumption of the driver. This change fixes the issue by
correctly exposing the ISA bridge to domU.

Note the PIIX3 ISA bridge is still present on the bus (dropping it seems
break qemu all-together), i915 driver still need to be updated to handle this.

Change since last version:
  1. Expose PCIBus structure and switch to pci_bridge_init to clean up the code

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
               Rui Guo <firemeteor@users.sourceforge.net>
Tested-by: Rui Guo <firemeteor@users.sourceforge.net>
Xen-devel: http://marc.info/?l=xen-devel&m=135548433715750
---
 hw/pci.c         |   23 -----------------------
 hw/pci.h         |   23 +++++++++++++++++++++++
 hw/pt-graphics.c |   11 ++++++++---
 3 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index f051de1..fad459a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,24 +39,6 @@ extern int igd_passthru;

 //#define DEBUG_PCI

-struct PCIBus {
-    int bus_num;
-    int devfn_min;
-    pci_set_irq_fn set_irq;
-    pci_map_irq_fn map_irq;
-    uint32_t config_reg; /* XXX: suppress */
-    /* low level pic */
-    SetIRQFunc *low_set_irq;
-    qemu_irq *irq_opaque;
-    PCIDevice *devices[256];
-    PCIDevice *parent_dev;
-    PCIBus *next;
-    /* The bus IRQ state is the logical OR of the connected devices.
-       Keep a count of the number of devices with raised IRQs.  */
-    int nirq;
-    int irq_count[];
-};
-
 static void pci_update_mappings(PCIDevice *d);
 static void pci_set_irq(void *opaque, int irq_num, int level);

@@ -871,11 +853,6 @@ void pci_unplug_netifs(void)
     }
 }

-typedef struct {
-    PCIDevice dev;
-    PCIBus *bus;
-} PCIBridge;
-
 void pci_bridge_write_config(PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
diff --git a/hw/pci.h b/hw/pci.h
index edc58b6..79cdb07 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -222,6 +222,11 @@ struct PCIDevice {
     int irq_state[4];
 };

+typedef struct {
+    PCIDevice dev;
+    PCIBus *bus;
+} PCIBridge;
+
 extern char direct_pci_str[];
 extern int direct_pci_msitranslate;
 extern int direct_pci_power_mgmt;
@@ -361,4 +366,22 @@ void pci_bridge_write_config(PCIDevice *d,
 PCIBus *pci_register_secondary_bus(PCIDevice *dev, pci_map_irq_fn map_irq);


+struct PCIBus {
+    int bus_num;
+    int devfn_min;
+    pci_set_irq_fn set_irq;
+    pci_map_irq_fn map_irq;
+    uint32_t config_reg; /* XXX: suppress */
+    /* low level pic */
+    SetIRQFunc *low_set_irq;
+    qemu_irq *irq_opaque;
+    PCIDevice *devices[256];
+    PCIDevice *parent_dev;
+    PCIBus *next;
+    /* The bus IRQ state is the logical OR of the connected devices.
+       Keep a count of the number of devices with raised IRQs.  */
+    int nirq;
+    int irq_count[];
+};
+
 #endif
diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
index c6f8869..aba7725 100644
--- a/hw/pt-graphics.c
+++ b/hw/pt-graphics.c
@@ -3,6 +3,7 @@
  */

 #include "pass-through.h"
+#include "pci.h"
 #include "pci/header.h"
 #include "pci/pci.h"

@@ -40,9 +41,13 @@ void intel_pch_init(PCIBus *bus)
     did = pt_pci_host_read(pci_dev_1f, PCI_DEVICE_ID, 2);
     rid = pt_pci_host_read(pci_dev_1f, PCI_REVISION, 1);

-    if ( vid == PCI_VENDOR_ID_INTEL )
-        pci_bridge_init(bus, PCI_DEVFN(0x1f, 0), vid, did, rid,
-                        pch_map_irq, "intel_bridge_1f");
+    if (vid == PCI_VENDOR_ID_INTEL) {
+        PCIBus *s = pci_bridge_init(bus, PCI_DEVFN(0x1f, 0), vid, did, rid,
+                                        pch_map_irq,
"intel_bridge_1f");
+
+        pci_config_set_class(s->parent_dev->config,
PCI_CLASS_BRIDGE_ISA);
+        s->parent_dev->config[PCI_HEADER_TYPE] = 0x80;
+    }
 }

 uint32_t igd_read_opregion(struct pt_dev *pci_dev)
--
1.7.10.4

>>> On 01.06.13 at 07:04, "G.R."
<firemeteor@users.sourceforge.net> wrote:
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -39,24 +39,6 @@ extern int igd_passthru;
> 
>  //#define DEBUG_PCI
> 
> -struct PCIBus {
> -    int bus_num;
> -    int devfn_min;
> -    pci_set_irq_fn set_irq;
> -    pci_map_irq_fn map_irq;
> -    uint32_t config_reg; /* XXX: suppress */
> -    /* low level pic */
> -    SetIRQFunc *low_set_irq;
> -    qemu_irq *irq_opaque;
> -    PCIDevice *devices[256];
> -    PCIDevice *parent_dev;
> -    PCIBus *next;
> -    /* The bus IRQ state is the logical OR of the connected devices.
> -       Keep a count of the number of devices with raised IRQs.  */
> -    int nirq;
> -    int irq_count[];
> -};
While it''s obvious that this one needs to be moved to the header
(albeit see below), ...
> @@ -871,11 +853,6 @@ void pci_unplug_netifs(void)
>      }
>  }
> 
> -typedef struct {
> -    PCIDevice dev;
> -    PCIBus *bus;
> -} PCIBridge;
... I don''t see why this one also needs to be.
> @@ -40,9 +41,13 @@ void intel_pch_init(PCIBus *bus)
>      did = pt_pci_host_read(pci_dev_1f, PCI_DEVICE_ID, 2);
>      rid = pt_pci_host_read(pci_dev_1f, PCI_REVISION, 1);
> 
> -    if ( vid == PCI_VENDOR_ID_INTEL )
> -        pci_bridge_init(bus, PCI_DEVFN(0x1f, 0), vid, did, rid,
> -                        pch_map_irq, "intel_bridge_1f");
> +    if (vid == PCI_VENDOR_ID_INTEL) {
> +        PCIBus *s = pci_bridge_init(bus, PCI_DEVFN(0x1f, 0), vid, did,
rid,
> +                                        pch_map_irq,
"intel_bridge_1f");
> +
> +        pci_config_set_class(s->parent_dev->config,
PCI_CLASS_BRIDGE_ISA);
> +        s->parent_dev->config[PCI_HEADER_TYPE] = 0x80;
Making struct PCIBus globally visible could be avoided altogether by
putting these two into one or two little helper functions implemented
in hw/pci.c.

Jan