Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD

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

On Tue, May 20, 2014 at 09:34:23AM +0000, Chen, Tiejun wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Monday, May 19, 2014 6:13 PM
> > To: Chen, Tiejun
> > Cc: Gerd Hoffmann; anthony.perard@citrix.com; 
> > stefano.stabellini@eu.citrix.com; Kelly.Zytaruk@amd.com; 
> > peter.maydell@linaro.org; xen-devel@lists.xensource.com; Kay, Allen M; 
> > qemu-devel@nongnu.org; jean.guyader@eu.citrix.com; 
> > anthony@codemonkey.ws; Zhang, Yang Z
> > Subject: Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: 
> > reserve
> > 00:02.0 for INTEL IGD
> > 
> > On Mon, May 19, 2014 at 09:25:19AM +0000, Chen, Tiejun wrote:
> > > > -----Original Message-----
> > > > From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> > > > Sent: Monday, May 19, 2014 2:45 PM
> > > > To: Chen, Tiejun
> > > > Cc: 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; Zhang, Yang Z
> > > > Subject: Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough:
> > > > reserve
> > > > 00:02.0 for INTEL IGD
> > > >
> > > >   Hi,
> > > >
> > > > > +    /*
> > > > > +     * Some video bioses and gfx drivers will assume the bdf of 
> > > > > + IGD is
> > > > 00:02.0.
> > > > > +     * So user need to set it to 00:02.0 in Xen configure file explicitly,
> > > > > +     * otherwise IGD will fail to work.
> > > > > +     */
> > > > > +    pci_reserve_pci_devfn(b, PCI_DEVFN(2, 0));
> > > >
> > > > That is asking for trouble.  Slot 2 is used by the qemu vga cards 
> > > > by default, and for quite a while (before memory api was merged) 
> > > > it even was impossible to change it.  libvirt still places the vga 
> > > > card at slot
> > > > 2 for that reason -> boom.  I wouldn't be surprised if you find 
> > > > that assumption in other management libs / apps too.
> > > >
> > > > Why do you need that patch in the first place?  It should be 
> > > > possible to configure qemu to not occupy slot 2 if you need it 
> > > > that way.  Just pass '-vga none' to qemu.  Which you probably want 
> > > > anyway if you pass-through a vga to the guest.  And explicitly 
> > > > configure a slot (via addr=
> > >
> > > I think '-vga none' just guarantees the qemu vga cards doesn't 
> > > occupy 00:02.0,
> > but this doesn't mean others use this specific slot since in qemu 
> > internal, we always pass -1 to assign a slot automatically to register 
> > a PCI device. So in some cases, we can't get this slot as we expect 
> > since that is already assigned previously before we need this.
> > 
> > So stop doing this.
> > Address -1 is a short-cut intended for humans.
> 
> Are you saying we need remove all auto assigned places like this,
> 
> piix.c: i440fx_init()
> 
>     if (xen_enabled()) {
>         piix3 = DO_UPCAST(PIIX3State, dev,
>                 pci_create_simple_multifunction(b, -1, true, "PIIX3-xen"));
>         pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
>                 piix3, XEN_PIIX_NUM_PIRQS);
>     } else {
>         piix3 = DO_UPCAST(PIIX3State, dev,
>                 pci_create_simple_multifunction(b, -1, true, "PIIX3"));

Preferably, these should supply the address explicitly.

> > In particular we have no way to guarantee that migration works unless 
> > you specify addresses explicitly.
> > 
> > > > property) for all your pci devices.  Doing it only for the IGD 
> > > > works too if you list the device before any other pci device on 
> > > > the qemu command
> > line.
> > >
> > > So in my test scenario, we can see this information:
> > >
> > > PCI: slot 2 function 0 not available for xen-pci-passthrough, in use 
> > > by xen-platform Thanks Tiejun
> > 
> > I think the best fix is to give priority to devices that supply a specific slot.
> > 
> 
> Maybe I can extend that bitmap I implemented in patch #1 to do this.
> 
> But anyway you also need to concern if this is compatible with libxl in xen case. 
> 
> Thanks
> Tiejun

libxl should stop using uto-assignment too.


-- 
MST