[v6,08/10] xen, gfx passthrough: support Intel IGD passthrough with VT-D

Some registers of Intel IGD are mapped in host bridge, so it needs to
passthrough these registers of physical host bridge to guest because
emulated host bridge in guest doesn't have these mappings.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 hw/pci-host/piix.c       |  3 ++
 hw/xen/xen_pt.h          |  1 +
 hw/xen/xen_pt_graphics.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

On Mon, Jan 19, 2015 at 05:28:41PM +0800, Tiejun Chen wrote:
> Some registers of Intel IGD are mapped in host bridge, so it needs to
> passthrough these registers of physical host bridge to guest because
> emulated host bridge in guest doesn't have these mappings.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  hw/pci-host/piix.c       |  3 ++
>  hw/xen/xen_pt.h          |  1 +
>  hw/xen/xen_pt_graphics.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 1468961..0a5a4c7 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/i386/ioapic.h"
>  #include "qapi/visitor.h"
> +#include "hw/xen/xen_pt.h"
>  
>  /*
>   * I440FX chipset data sheet.
> @@ -733,8 +734,10 @@ static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
>                                                    void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
>      dc->desc = "IGD PT XEN Host bridge";
> +    k->config_read = xen_igd_pci_read;
>  }
>  
>  static const TypeInfo xen_igd_passthrough_i440fx_info = {
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 0aa5a93..94cde4a 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -5,6 +5,7 @@
>  #include "hw/xen/xen_common.h"
>  #include "hw/pci/pci.h"
>  #include "xen-host-pci-device.h"
> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
>  
>  void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3);
>  
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index 3232296..227089b 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -4,6 +4,7 @@
>  #include "xen_pt.h"
>  #include "xen-host-pci-device.h"
>  #include "hw/xen/xen_backend.h"
> +#include "hw/pci/pci_bus.h"
>  
>  typedef struct VGARegion {
>      int type;           /* Memory or port I/O */
> @@ -188,3 +189,74 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
>      cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
>      return 0;
>  }
> +
> +/*
> + * Currently we just pass this physical host bridge for IGD, 00:02.0.
> + *
> + * Here pci_dev is just that host bridge, so we have to get that real
> + * passthrough device by that given devfn to avoid other devices access.
> + */
> +static int is_igd_passthrough(PCIDevice *pci_dev)
> +{
> +    PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
> +    if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
> +        XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, f);
> +        return (is_igd_vga_passthrough(&s->real_device)
> +                    && (s->real_device.vendor_id == PCI_VENDOR_ID_INTEL));
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
> +{
> +    XenHostPCIDevice dev;
> +    uint32_t val;
> +    int r;
> +
> +    /* IGD read/write is through the host bridge.
> +     */
> +    assert(pci_dev->devfn == 0x00);
> +
> +    if (!is_igd_passthrough(pci_dev)) {
> +        goto read_default;
> +    }
> +
> +    /* Just work for the i915 driver. */
> +    switch (config_addr) {
> +    case 0x08:        /* revision id */
> +    case 0x2c:        /* sybsystem vendor id */
> +    case 0x2e:        /* sybsystem id */
> +    case 0x50:        /* SNB: processor graphics control register */
> +    case 0x52:        /* processor graphics control register */
> +    case 0xa0:        /* top of memory */

Is this host physical memory? If yes how can using it in guest work?

> +    case 0xa4:        /* SNB: graphics base of stolen memory */
> +    case 0xa8:        /* SNB: base of GTT stolen memory */

Same question for above two.

> +        break;
> +    default:
> +        /* Just gets the emulated values. */
> +        goto read_default;
> +    }
> +
> +    /* Host read */
> +    r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
> +    if (r) {
> +        goto err_out;
> +    }
> +
> +    r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
> +    if (r) {
> +        goto err_out;
> +    }
> +
> +    xen_host_pci_device_put(&dev);
> +
> +    return val;
> +
> +read_default:
> +    return pci_default_read_config(pci_dev, config_addr, len);
> +
> +err_out:
> +    XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> +    return -1;
> +}

Do any of the above registers change with time?
Does it work if we just read them when device is created
and put in dev->config?

> -- 
> 1.9.1

>> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
>> +{
>> +    XenHostPCIDevice dev;
>> +    uint32_t val;
>> +    int r;
>> +
>> +    /* IGD read/write is through the host bridge.
>> +     */
>> +    assert(pci_dev->devfn == 0x00);
>> +
>> +    if (!is_igd_passthrough(pci_dev)) {
>> +        goto read_default;
>> +    }
>> +
>> +    /* Just work for the i915 driver. */
>> +    switch (config_addr) {
>> +    case 0x08:        /* revision id */
>> +    case 0x2c:        /* sybsystem vendor id */
>> +    case 0x2e:        /* sybsystem id */
>> +    case 0x50:        /* SNB: processor graphics control register */
>> +    case 0x52:        /* processor graphics control register */
>> +    case 0xa0:        /* top of memory */
>
> Is this host physical memory? If yes how can using it in guest work?

This is just a threshold value, not a start or end address :)

>
>> +    case 0xa4:        /* SNB: graphics base of stolen memory */
>> +    case 0xa8:        /* SNB: base of GTT stolen memory */
>
> Same question for above two.

I shouldn't matter since I remember we already discussed this previously 
but I can't sort out this from those emails now.

Allen,

Could you reexplain this?

>
>> +        break;
>> +    default:
>> +        /* Just gets the emulated values. */
>> +        goto read_default;
>> +    }
>> +
>> +    /* Host read */
>> +    r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
>> +    if (r) {
>> +        goto err_out;
>> +    }
>> +
>> +    r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
>> +    if (r) {
>> +        goto err_out;
>> +    }
>> +
>> +    xen_host_pci_device_put(&dev);
>> +
>> +    return val;
>> +
>> +read_default:
>> +    return pci_default_read_config(pci_dev, config_addr, len);
>> +
>> +err_out:
>> +    XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
>> +    return -1;
>> +}
>
> Do any of the above registers change with time?

Think about we just provide read ops, so they're not changed based on my 
experiential.

> Does it work if we just read them when device is created
> and put in dev->config?

I think this is a good idea so I will go there and thank you.

Tiejun

> -----Original Message-----
> From: Chen, Tiejun
> Sent: Tuesday, January 20, 2015 7:17 PM
> To: Michael S. Tsirkin; Kay, Allen M
> Cc: pbonzini@redhat.com; aliguori@amazon.com; rth@twiddle.net; Zhang,
> Yang Z; qemu-devel@nongnu.org
> Subject: Re: [v6][PATCH 08/10] xen, gfx passthrough: support Intel IGD
> passthrough with VT-D
> 
> >> +uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr,
> >> +int len) {
> >> +    XenHostPCIDevice dev;
> >> +    uint32_t val;
> >> +    int r;
> >> +
> >> +    /* IGD read/write is through the host bridge.
> >> +     */
> >> +    assert(pci_dev->devfn == 0x00);
> >> +
> >> +    if (!is_igd_passthrough(pci_dev)) {
> >> +        goto read_default;
> >> +    }
> >> +
> >> +    /* Just work for the i915 driver. */
> >> +    switch (config_addr) {
> >> +    case 0x08:        /* revision id */
> >> +    case 0x2c:        /* sybsystem vendor id */
> >> +    case 0x2e:        /* sybsystem id */
> >> +    case 0x50:        /* SNB: processor graphics control register */
> >> +    case 0x52:        /* processor graphics control register */
> >> +    case 0xa0:        /* top of memory */
> >
> > Is this host physical memory? If yes how can using it in guest work?
> 
> This is just a threshold value, not a start or end address :)
> 

"top of memory" is no longer used in new versions of Windows drivers.  Tiejun and I have tried BDW/HSW drivers and found they work without the need to passthrough 0xa0 register in the host bridge.  It can be removed.

> >
> >> +    case 0xa4:        /* SNB: graphics base of stolen memory */
> >> +    case 0xa8:        /* SNB: base of GTT stolen memory */
> >
> > Same question for above two.
> 
> I shouldn't matter since I remember we already discussed this previously but
> I can't sort out this from those emails now.
> 
> Allen,
> 
> Could you reexplain this?
> 

These two "stolen memory" regions are part of the VT-d RMRR region.  They are 1:1 mapped in the guest (GPA == HPA).  Tiejun found they are needed by vBIOS during boot.  Modern Windows driver no longer need to access these two registers in MCH.

> >
> >> +        break;
> >> +    default:
> >> +        /* Just gets the emulated values. */
> >> +        goto read_default;
> >> +    }
> >> +
> >> +    /* Host read */
> >> +    r = xen_host_pci_device_get(&dev, 0, 0, 0, 0);
> >> +    if (r) {
> >> +        goto err_out;
> >> +    }
> >> +
> >> +    r = xen_host_pci_get_block(&dev, config_addr, (uint8_t *)&val, len);
> >> +    if (r) {
> >> +        goto err_out;
> >> +    }
> >> +
> >> +    xen_host_pci_device_put(&dev);
> >> +
> >> +    return val;
> >> +
> >> +read_default:
> >> +    return pci_default_read_config(pci_dev, config_addr, len);
> >> +
> >> +err_out:
> >> +    XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n");
> >> +    return -1;
> >> +}
> >
> > Do any of the above registers change with time?
> 
> Think about we just provide read ops, so they're not changed based on my
> experiential.
> 
> > Does it work if we just read them when device is created and put in
> > dev->config?
> 
> I think this is a good idea so I will go there and thank you.
> 
> Tiejun
> 

Allen