Re: [Qemu-devel] [v5][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D

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

On Thu, Jun 26, 2014 at 01:34:14PM +0800, Chen, Tiejun wrote:
> On 2014/6/25 22:05, Michael S. Tsirkin wrote:
> >On Wed, Jun 25, 2014 at 10:17:19AM +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.
> >>
> >>The original patch is from Weidong Han <weidong.han@intel.com>
> >>
> >>Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>Cc: Weidong Han <weidong.han@intel.com>
> >>---
> 
> [snip]
> 
> >>+
> >>+uint32_t 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.
> >>+     * ISA bridge is only for detect purpose. In i915 driver it will
> >>+     * probe ISA bridge to discover the IGD, see comment in i915_drv.c:
> >>+     * intel_detect_pch().
> >>+     */
> >>+    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 0x44:        /* MCHBAR I915 */
> >>+    case 0x48:        /* MCHBAR I965 */
> >
> >In fact, this is returning host addresses to guest, right?
> 
> Yes. These two registers includes the address and some bits to control
> something. And the i915 driver just need to *read* address to reserve for
> PHP, but actually we don't support PHP now. So these address space are just
> be reserved.
> 
> And then it will *write* them with the PCI resources allocated in guest. And
> update those bits to enable or disable something in different occasions.
> 
> Actually we didn't introduce 0x44/0x48 originally but when Paolo saw the
> i915 driver will access them, so ask me what will be occurred. So I just add
> them simply to test again. But now looks they should be emulated, so I think
> we can get them out now. Then this should be enough to make sense.

Yes.

> >Don't see how this can work except by luck.
> >
> >>+    case 0x50:        /* SNB: processor graphics control register */
> >>+    case 0x52:        /* processor graphics control register */
> >>+    case 0xa0:        /* top of memory */
> >
> >which memory? Could be similar.
> 
> This is just a total memory used to the video device so we call top of
> memory. And this is used to limit that range, but as you know we never go
> over this so we can always live here.

Sorry I don't understand what you are saying here.

> >
> >>+    case 0xb0:        /* ILK: BSM: should read from dev 2 offset 0x5c */
> >>+    case 0x58:        /* SNB: PAVPC Offset */
> >>+    case 0xa4:        /* SNB: graphics base of stolen memory */
> >
> >Same thing.
> >
> >>+    case 0xa8:        /* SNB: base of GTT stolen memory */
> >
> >Here too.
> >
> 
> We have 1:1 mapping for these two ranges so its fine.

Could you clarify? My question, again, is whether this is
returning host addresses to guest.

> These info are known to Linux. For Windows, it is opaque to us so we can't
> know what will be happened to Windows exactly, but I think this should be
> similar with Linux i915 driver mostly. And especially we also validate this
> on Windows 7 to confirm everything can work under Windows.
> 
> Thanks
> Tiejun