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

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

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.


> > 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?


> 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. With this change she couldn't
migrate their VMs from Xen 4.7 to Xen 4.8 safely because suddenly a new
bridge appears.

That said Xen 4.7 hasn't been released yet, so we could still change
the intended behaviour. But it would require a bit of coordination (the
qemu-xen tree in 4.7 is based on v2.4.1).



> 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?