Re: [Qemu-devel] [PATCH v3 11/11] igd: move igd-passthrough-isa-bridge creation to machine init

[Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Do, 2016-01-07 at 13:10 +0000, Stefano Stabellini wrote:
> CC'ing the Xen x86 maintainers
> 
> On Thu, 7 Jan 2016, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > One thing I don't like about this is that it is going to skip the checks
> > > done in xen_pt_initfn.
> > 
> > Hmm?  Those checks are still done when you assign a igd ...
> 
> Their failure doesn't affect the creation of the bridge.

Doesn't their failure makes qemu throw a fatal error and exit?
So the guest isn't going to run either way?

> > > For example it is going to create the isa bridge,
> > > even if there is going to be an error loading the vga bios or if the
> > > device specified is not even an Intel graphic card.
> > 
> > Creating the special igd-isa-bridge is no longer tied to actually
> > assigning a igd, but to the igd-passthru=on machine option being present
> > (and machine type being 'pc').
> 
> and machine type 'xenfv', unless I am mistaken?

Yes, xenfv too (uses i440fx too and thus is a 'pc' derivate).

> > xen_pt_initfn checks that igd-passthru=on is set in case it finds a igd
> > device is assigned, that will make sure the igd-isa-bridge is present.
> > 
> > But, yes, you can create a igd-isa-bridge now even when not assigning a
> > igd device, either by specifying igd-passthru=on or using -device.  I
> > fail to see why this is a problem though, care to explain?
> 
> It is going to change the PCI layout of any virtual machines with a
> config file containing
> 
> gfx_passthru="igd"
> 
> and no pci config line. A Xen 4.7 user could add gfx_passthru="igd" to
> all her VM config files, because actually it does nothing unless an
> Intel graphic card is assigned to the VM.

No.  It changes the host bridge even when not passing through a igd,
because that is linked to igd-passthru=on only.

So making both host bridge tweak and isa bridge tweak triggered by
igd-passthru=on brings more consistency to the whole thing.

> > Also note that moving this to machine init nicely handles the fact that
> > the igd-isa-bridge is needed on 'pc' only, not on 'q35'.  If you don't
> > want create the igd-isa-bridge in machine init, what is your alternative
> > suggestion to handle this?
> 
> Maybe we could retain the check whether an Intel graphic card has been
> assigned? 

Should be possible, but is not that easy due to initialization order
issues.

cheers,
  Gerd