Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D

["Chen, Tiejun" <tiejun.chen@intel.com>]

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Monday, May 19, 2014 9:34 PM
> To: Zhang, Yang Z
> Cc: Chen, Tiejun; anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com;
> mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org;
> xen-devel@lists.xensource.com; weidong.han@intel.com; Kay, Allen M;
> qemu-devel@nongnu.org; jean.guyader@eu.citrix.com;
> anthony@codemonkey.ws
> Subject: Re: [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel
> IGD passthrough with VT-D
> 
> On Mon, May 19, 2014 at 12:58:50AM +0000, Zhang, Yang Z wrote:
> > Konrad Rzeszutek Wilk wrote on 2014-05-16:
> > > On Fri, May 16, 2014 at 06:53:42PM +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.
> >
> > Thanks for your review for the whole series patch.
> 
> Sure thing!
> 
> .. snip..
> > > > +write_default:
> > > > +    pci_default_write_config(pci_dev, config_addr, val, len);
> > >
> > >
> > > and we just allow it through. But what happens if the guest decides
> > > to change the BAR sizes?  Or fiddle with the GTT?
> > >
> > > Ouch. That really looks dangerous - or maybe I am too paranoid?
> > >
> >
> > I do not quite understand your concern. We only pass through PAVPC to
> physical host bridge. The others are handled by current logic. We don't change
> any of it. So what problem will be exposed by this patch?
> 
> Ah, I assumed that pci_default_write_config would be writting everything
> without any checks. But it looks to be doing the right thing and just sets the
> emulated values.
> 
> Could we just add comment saying that it writes to the emulated values? That
> way it won't trip folks.

diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 1342f4f..5e5e3d5 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -367,6 +367,7 @@ void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
     case 0x58:      /* PAVPC Offset */
         break;
     default:
+        /* Just sets the emulated values. */
         goto write_default;
     }
 
@@ -427,6 +428,7 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
     case 0xa8:        /* SNB: base of GTT stolen memory */
         break;
     default:
+        /* Just gets the emulated values. */
         goto read_default;
     }

Thanks
Tiejun