diff mbox

[v3,10/11] igd: handle igd-passthrough-isa-bridge setup in realize()

Message ID 1451994098-6972-11-git-send-email-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gerd Hoffmann Jan. 5, 2016, 11:41 a.m. UTC
That way a simple '-device igd-passthrough-isa-bridge,addr=1f' will
do the setup.

Also instead of looking up reasonable PCI IDs based on the graphic
device id simply copy over the ids from the host, thereby reusing the
infrastructure we have in place for the igd host bridges.  Less code,
and should be more robust as we don't have to maintain the id table
to keep things going.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-host/igd.c    | 115 +++++++++++++--------------------------------------
 hw/xen/xen_pt.c      |   2 +-
 include/hw/i386/pc.h |   2 +-
 3 files changed, 30 insertions(+), 89 deletions(-)

Comments

Stefano Stabellini Jan. 6, 2016, 3:29 p.m. UTC | #1
On Tue, 5 Jan 2016, Gerd Hoffmann wrote:
> That way a simple '-device igd-passthrough-isa-bridge,addr=1f' will
> do the setup.

Is this going to change the QEMU command line arguments to use it?


> Also instead of looking up reasonable PCI IDs based on the graphic
> device id simply copy over the ids from the host, thereby reusing the
> infrastructure we have in place for the igd host bridges.  Less code,
> and should be more robust as we don't have to maintain the id table
> to keep things going.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/pci-host/igd.c    | 115 +++++++++++++--------------------------------------
>  hw/xen/xen_pt.c      |   2 +-
>  include/hw/i386/pc.h |   2 +-
>  3 files changed, 30 insertions(+), 89 deletions(-)
> 
> diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
> index 96b679d..8f32c39 100644
> --- a/hw/pci-host/igd.c
> +++ b/hw/pci-host/igd.c
> @@ -123,111 +123,52 @@ static const TypeInfo igd_passthrough_q35_info = {
>      .class_init    = igd_passthrough_q35_class_init,
>  };
>  
> -typedef struct {
> -    uint16_t gpu_device_id;
> -    uint16_t pch_device_id;
> -    uint8_t pch_revision_id;
> -} IGDDeviceIDInfo;
> -
> -/* In real world different GPU should have different PCH. But actually
> - * the different PCH DIDs likely map to different PCH SKUs. We do the
> - * same thing for the GPU. For PCH, the different SKUs are going to be
> - * all the same silicon design and implementation, just different
> - * features turn on and off with fuses. The SW interfaces should be
> - * consistent across all SKUs in a given family (eg LPT). But just same
> - * features may not be supported.
> - *
> - * Most of these different PCH features probably don't matter to the
> - * Gfx driver, but obviously any difference in display port connections
> - * will so it should be fine with any PCH in case of passthrough.
> - *
> - * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
> - * scenarios, 0x9cc3 for BDW(Broadwell).
> - */
> -static const IGDDeviceIDInfo igd_combo_id_infos[] = {
> -    /* HSW Classic */
> -    {0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
> -    {0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
> -    {0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
> -    {0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
> -    {0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
> -    /* HSW ULT */
> -    {0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
> -    {0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
> -    {0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
> -    {0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
> -    {0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
> -    {0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
> -    /* HSW CRW */
> -    {0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
> -    {0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
> -    /* HSW Server */
> -    {0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
> -    /* HSW SRVR */
> -    {0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
> -    /* BSW */
> -    {0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
> -    {0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
> -    {0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
> -    {0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
> -    {0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
> -    {0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
> -    {0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
> -    {0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
> -    {0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
> -    {0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
> -    {0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
> +static const IGDHostInfo igd_isa_bridge_infos[] = {
> +    {PCI_VENDOR_ID,           2},
> +    {PCI_DEVICE_ID,           2},
> +    {PCI_REVISION_ID,         2},
> +    {PCI_SUBSYSTEM_VENDOR_ID, 2},
> +    {PCI_SUBSYSTEM_ID,        2},
>  };
>  
> +static void igd_pt_isa_bridge_realize(PCIDevice *pci_dev, Error **errp)
> +{
> +    Error *err = NULL;
> +
> +    if (pci_dev->devfn != PCI_DEVFN(0x1f, 0)) {
> +        error_setg(errp, "igd isa bridge must have address 1f.0");
> +        return;
> +    }
> +
> +    host_pci_config_copy(pci_dev, "0000:00:1f.0",
> +                         igd_isa_bridge_infos,
> +                         ARRAY_SIZE(igd_isa_bridge_infos),
> +                         &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +}
> +
>  static void isa_bridge_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
>      dc->desc        = "ISA bridge faked to support IGD PT";
> -    k->vendor_id    = PCI_VENDOR_ID_INTEL;
> +    k->realize      = igd_pt_isa_bridge_realize;
>      k->class_id     = PCI_CLASS_BRIDGE_ISA;
>  };
>  
>  static TypeInfo igd_passthrough_isa_bridge_info = {
>      .name          = "igd-passthrough-isa-bridge",
>      .parent        = TYPE_PCI_DEVICE,
> -    .instance_size = sizeof(PCIDevice),
>      .class_init = isa_bridge_class_init,
>  };
>  
> -void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
> +void igd_passthrough_isa_bridge_create(PCIBus *bus)
>  {
> -    struct PCIDevice *bridge_dev;
> -    int i, num;
> -    uint16_t pch_dev_id = 0xffff;
> -    uint8_t pch_rev_id;
> -
> -    num = ARRAY_SIZE(igd_combo_id_infos);
> -    for (i = 0; i < num; i++) {
> -        if (gpu_dev_id == igd_combo_id_infos[i].gpu_device_id) {
> -            pch_dev_id = igd_combo_id_infos[i].pch_device_id;
> -            pch_rev_id = igd_combo_id_infos[i].pch_revision_id;
> -        }
> -    }
> -
> -    if (pch_dev_id == 0xffff) {
> -        return;
> -    }
> -
> -    /* Currently IGD drivers always need to access PCH by 1f.0. */
> -    bridge_dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0),
> -                                   "igd-passthrough-isa-bridge");
> -
> -    /*
> -     * Note that vendor id is always PCI_VENDOR_ID_INTEL.
> -     */
> -    if (!bridge_dev) {
> -        fprintf(stderr, "set igd-passthrough-isa-bridge failed!\n");
> -        return;
> -    }
> -    pci_config_set_device_id(bridge_dev->config, pch_dev_id);
> -    pci_config_set_revision(bridge_dev->config, pch_rev_id);
> +    pci_create_simple(bus, PCI_DEVFN(0x1f, 0), "igd-passthrough-isa-bridge");
>  }
>  
>  static void igd_register_types(void)
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index aa96288..18a7f72 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -693,7 +693,7 @@ xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
>      PCIDevice *d = &s->dev;
>  
>      gpu_dev_id = dev->device_id;
> -    igd_passthrough_isa_bridge_create(d->bus, gpu_dev_id);
> +    igd_passthrough_isa_bridge_create(d->bus);
>  }
>  
>  /* destroy. */
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index b0d6283..48cdd03 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -863,5 +863,5 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>      (m)->compat_props = props; \
>  } while (0)
>  
> -extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id);
> +extern void igd_passthrough_isa_bridge_create(PCIBus *bus);
>  #endif
> -- 
> 1.8.3.1
>
Gerd Hoffmann Jan. 6, 2016, 3:52 p.m. UTC | #2
On Mi, 2016-01-06 at 15:29 +0000, Stefano Stabellini wrote:
> On Tue, 5 Jan 2016, Gerd Hoffmann wrote:
> > That way a simple '-device igd-passthrough-isa-bridge,addr=1f' will
> > do the setup.
> 
> Is this going to change the QEMU command line arguments to use it?

See patch 11 ;)

cheers,
  Gerd
diff mbox

Patch

diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
index 96b679d..8f32c39 100644
--- a/hw/pci-host/igd.c
+++ b/hw/pci-host/igd.c
@@ -123,111 +123,52 @@  static const TypeInfo igd_passthrough_q35_info = {
     .class_init    = igd_passthrough_q35_class_init,
 };
 
-typedef struct {
-    uint16_t gpu_device_id;
-    uint16_t pch_device_id;
-    uint8_t pch_revision_id;
-} IGDDeviceIDInfo;
-
-/* In real world different GPU should have different PCH. But actually
- * the different PCH DIDs likely map to different PCH SKUs. We do the
- * same thing for the GPU. For PCH, the different SKUs are going to be
- * all the same silicon design and implementation, just different
- * features turn on and off with fuses. The SW interfaces should be
- * consistent across all SKUs in a given family (eg LPT). But just same
- * features may not be supported.
- *
- * Most of these different PCH features probably don't matter to the
- * Gfx driver, but obviously any difference in display port connections
- * will so it should be fine with any PCH in case of passthrough.
- *
- * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
- * scenarios, 0x9cc3 for BDW(Broadwell).
- */
-static const IGDDeviceIDInfo igd_combo_id_infos[] = {
-    /* HSW Classic */
-    {0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
-    {0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
-    {0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
-    {0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
-    {0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
-    /* HSW ULT */
-    {0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
-    {0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
-    {0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
-    {0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
-    {0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
-    {0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
-    /* HSW CRW */
-    {0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
-    {0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
-    /* HSW Server */
-    {0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
-    /* HSW SRVR */
-    {0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
-    /* BSW */
-    {0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
-    {0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
-    {0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
-    {0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
-    {0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
-    {0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
-    {0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
-    {0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
-    {0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
-    {0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
-    {0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
+static const IGDHostInfo igd_isa_bridge_infos[] = {
+    {PCI_VENDOR_ID,           2},
+    {PCI_DEVICE_ID,           2},
+    {PCI_REVISION_ID,         2},
+    {PCI_SUBSYSTEM_VENDOR_ID, 2},
+    {PCI_SUBSYSTEM_ID,        2},
 };
 
+static void igd_pt_isa_bridge_realize(PCIDevice *pci_dev, Error **errp)
+{
+    Error *err = NULL;
+
+    if (pci_dev->devfn != PCI_DEVFN(0x1f, 0)) {
+        error_setg(errp, "igd isa bridge must have address 1f.0");
+        return;
+    }
+
+    host_pci_config_copy(pci_dev, "0000:00:1f.0",
+                         igd_isa_bridge_infos,
+                         ARRAY_SIZE(igd_isa_bridge_infos),
+                         &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+}
+
 static void isa_bridge_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     dc->desc        = "ISA bridge faked to support IGD PT";
-    k->vendor_id    = PCI_VENDOR_ID_INTEL;
+    k->realize      = igd_pt_isa_bridge_realize;
     k->class_id     = PCI_CLASS_BRIDGE_ISA;
 };
 
 static TypeInfo igd_passthrough_isa_bridge_info = {
     .name          = "igd-passthrough-isa-bridge",
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCIDevice),
     .class_init = isa_bridge_class_init,
 };
 
-void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
+void igd_passthrough_isa_bridge_create(PCIBus *bus)
 {
-    struct PCIDevice *bridge_dev;
-    int i, num;
-    uint16_t pch_dev_id = 0xffff;
-    uint8_t pch_rev_id;
-
-    num = ARRAY_SIZE(igd_combo_id_infos);
-    for (i = 0; i < num; i++) {
-        if (gpu_dev_id == igd_combo_id_infos[i].gpu_device_id) {
-            pch_dev_id = igd_combo_id_infos[i].pch_device_id;
-            pch_rev_id = igd_combo_id_infos[i].pch_revision_id;
-        }
-    }
-
-    if (pch_dev_id == 0xffff) {
-        return;
-    }
-
-    /* Currently IGD drivers always need to access PCH by 1f.0. */
-    bridge_dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0),
-                                   "igd-passthrough-isa-bridge");
-
-    /*
-     * Note that vendor id is always PCI_VENDOR_ID_INTEL.
-     */
-    if (!bridge_dev) {
-        fprintf(stderr, "set igd-passthrough-isa-bridge failed!\n");
-        return;
-    }
-    pci_config_set_device_id(bridge_dev->config, pch_dev_id);
-    pci_config_set_revision(bridge_dev->config, pch_rev_id);
+    pci_create_simple(bus, PCI_DEVFN(0x1f, 0), "igd-passthrough-isa-bridge");
 }
 
 static void igd_register_types(void)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index aa96288..18a7f72 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -693,7 +693,7 @@  xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
     PCIDevice *d = &s->dev;
 
     gpu_dev_id = dev->device_id;
-    igd_passthrough_isa_bridge_create(d->bus, gpu_dev_id);
+    igd_passthrough_isa_bridge_create(d->bus);
 }
 
 /* destroy. */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index b0d6283..48cdd03 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -863,5 +863,5 @@  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
     (m)->compat_props = props; \
 } while (0)
 
-extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id);
+extern void igd_passthrough_isa_bridge_create(PCIBus *bus);
 #endif