Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jan 19, 2015 at 05:28:40PM +0800, Tiejun Chen wrote:
> Currently IGD drivers always need to access PCH by 1f.0. But we
> don't want to poke that directly to get ID, and although 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).
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/xen/xen_pt.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 126 insertions(+)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index fcc9f1c..5532d6f 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -632,6 +632,129 @@ static const MemoryListener xen_pt_io_listener = {
>      .priority = 10,
>  };
>  
> +typedef struct {
> +    uint16_t gpu_device_id;
> +    uint16_t pch_device_id;
> +    uint8_t pch_revision_id;
> +} XenIGDDeviceIDInfo;
> +
> +/* 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 XenIGDDeviceIDInfo xen_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 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->class_id     = PCI_CLASS_BRIDGE_ISA;
> +};
> +
> +static TypeInfo isa_bridge_info = {
> +    .name          = "xen-igd-passthrough-isa-bridge",
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCIDevice),
> +    .class_init = isa_bridge_class_init,
> +};
> +
> +static void xen_pt_graphics_register_types(void)
> +{
> +    type_register_static(&isa_bridge_info);
> +}
> +type_init(xen_pt_graphics_register_types)
> +
> +static void
> +xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
> +                                      XenHostPCIDevice *dev)
> +{

I suggest this implementation, and the table, are moved
to the same file where igd-passthrough-isa-bridge
is implemented. The function can get PCIDevice *d, this way
it's not xen specific.



> +    struct PCIDevice *pci_dev;

pls rename bridge_dev;

> +    int i, num;
> +    uint16_t gpu_dev_id, pch_dev_id = 0xffff;
> +    uint8_t pch_rev_id;
> +    PCIDevice *d = &s->dev;
> +
> +    if (!is_igd_vga_passthrough(dev)) {
> +        return;
> +    }
> +
> +    gpu_dev_id = dev->device_id;
> +    num = ARRAY_SIZE(xen_igd_combo_id_infos);
> +    for (i = 0; i < num; i++) {
> +        if (gpu_dev_id == xen_igd_combo_id_infos[i].gpu_device_id) {
> +            pch_dev_id = xen_igd_combo_id_infos[i].pch_device_id;
> +            pch_rev_id = xen_igd_combo_id_infos[i].pch_revision_id;
> +        }
> +    }
> +
> +    if (pch_dev_id == 0xffff) {
> +        fprintf(stderr, "unsupported PCH!\n");
> +        return;
> +    }
> +
> +    /* Currently IGD drivers always need to access PCH by 1f.0. */
> +    pci_dev = pci_create_simple(d->bus, PCI_DEVFN(0x1f, 0),
> +                                "xen-igd-passthrough-isa-bridge");
> +
> +    /*
> +     * Identify PCH card with its own real vendor/device ids.
> +     * Note that vendor id is always PCI_VENDOR_ID_INTEL.
> +     */
> +    if (!pci_dev) {
> +        fprintf(stderr, "xen set xen-igd-passthrough-isa-bridge failed!\n");
> +        return;
> +    }
> +    pci_config_set_device_id(pci_dev->config, pch_dev_id);
> +    pci_config_set_revision(pci_dev->config, pch_rev_id);
> +}
> +
>  /* init */
>  
>  static int xen_pt_initfn(PCIDevice *d)
> @@ -680,6 +803,9 @@ static int xen_pt_initfn(PCIDevice *d)
>              xen_host_pci_device_put(&s->real_device);
>              return -1;
>          }
> +
> +        /* Register ISA bridge for passthrough GFX. */
> +        xen_igd_passthrough_isa_bridge_create(s, &s->real_device);
>      }
>  
>      /* Handle real device's MMIO/PIO BARs */
> -- 
> 1.9.1