Re: [Xen-devel] [PATCH v3 10/11] igd: handle igd-passthrough-isa-bridge setup in realize()

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>  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
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel