Re: [Qemu-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough

[Chen, Tiejun]

On 2015/2/3 19:07, Ian Campbell wrote:
> On Tue, 2015-02-03 at 09:04 +0800, Chen, Tiejun wrote:
> > On 2015/2/2 20:54, Ian Jackson wrote:
> > > Wei Liu writes ("Re: [v2][PATCH] libxl: add one machine property to support IGD GFX 
> > > passthrough"):
> > > > On Mon, Feb 02, 2015 at 09:17:23AM +0800, Tiejun Chen wrote:
> > > > > When we're working to support IGD GFX passthrough with qemu
> > > > > upstream, instead of "-gfx_passthru" we'd like to make that
> > > > > a machine option, "-machine xxx,-igd-passthru=on". This need
> > > > > to bring a change on tool side.
> > > ...
> > > > My suggestion has one premise -- if upstream QEMU has already released
> > > > that -gfx_passthru option. If there is no "old one" (in upstream QEMU)
> > > > at all, then there is nothing to keep and deprecate.
> > >
> > > I think the commit message of the xen.git commit should explain what
> > > options are supported by which versions of qemu (including qemu
> > > upstream's future plans).
> > >
> > > That would provide (a) something which summarises the communication
> > > etc. with qemu upstream and can be checked with them if necessary and
> > > (b) something against which the libxl changes can be easily judged.
>
> > Sorry, looks I'm misleading this to everyone.
> >
> > Here I picked my reply from another email:
> >
> > Actually qemu upstream never own this option, '-gfx_passthru' at all.
> > This just exists alone in qemu-xen-traditional. So here I'm trying to
> > introduce a new stuff that doesn't clash anything in qemu upstream.
> >
> > So I guess I should rephrase this as follows:
> >
> >       libxl: add one machine property to support IGD GFX passthrough
> >
> >       When we're working to support IGD GFX passthrough with qemu
> >       upstream, we'd like to introduce a machine option,
>
> Has this new option been acked/accepted into the upstream qemu code base
> yet? I think there should also be a reference to the relevant qemu.git

Yes, Gerd just recommend this option and I guess this should be acked as 
well :)

> changeset as well as to any useful conversations about it.

I just post this patch prior to those qemu patch supporting IGD 
passthrough since I need this option is acked on Xen side. Then I can 
continue submitting all qemu patches.

> >       "-machine xxx,igd-passthru=on", to enable/disable that feature.
> >       And we also remove that old option, "-gfx_passthru", just from
> >       the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually
> >       no any qemu stream version really need or use that.
>                     ^up ?
>
> What happens if you pass this new option to an older version of qemu
> upstream? I suppose it doesn't fail any worse than passing -gfx_passthru
> would have done.

Neither '-gfx_passthru' nor 'igd-passthrou' exists in any version of 
qemu upstream. As I mentioned previously, just now we're starting to 
support this in qemu upstream :)

But you known, on Xen side we have two qemu versions, 
qemu-xen-traditional and qemu-xen. Although '-gfx_passthru' is adopted 
in both two versions, just qemu-xen-traditional supports IGD passthrough 
completely. For qemu-xen, we just have this term definition but without 
that IGD passthrough feature support actually.

And now we're trying to support IGD passthrough in qemu stream. This 
mean we should set 'device_model_version="qemu-xen", right? Then libxl 
still pass '-gfx_passthru' to qemu upstream. But when I post other 
stuffs to qemu upstream community to support IGD passthrough. Gerd 
thought "-machine xxx,igd-passthru=on" is better than '-gfx_passthru'. 
So finally you see this change in Xen/tools/libxl.

> I have one more general concern, which is that hardcoding igd-passthru
> here may make it harder to support gfx_passthru of other cards in the
> future.

Actually gfx_passthrou is introduced to just work for IGD passthrough 
since something specific to IGD is tricky, so we have to need such a 
flag to handle this precisely, like its fixed at 00:02.0, and expose 
some ISA bridge PCI config info and even host bridge PCI config info.

So I don't thing other cards need this.

> Somehow something in the stack needs to either detect or be told which
> kind of card to passthrough. Automatic detection would be preferable,
> but maybe being told by the user is the only possibility.

Based on the above explanation, something should be done before we 
detect to construct that real card , so its difficulty to achieve this 
goal currently.

> Is there any way, given gfx_passthru as a boolean that libxl can
> automatically figure out that IGD passthru is what is actually desired
> -- e.g. by scanning the set of PCI devices given to the guest perhaps?

Sorry I don't understand what you mean here. Currently, we have to set 
something as follows,

gfx_passthru=1
pci=["00:02.0"]

This always works for qemu-xen-traditional.

But you should know '00:02.0' doesn't mean we are passing IGD through.

> If not then that _might_ suggest we should deprecate the gdx_passthru
> option at the libxl interface and switch to something more expressive,
> such as an Enumeration of card types (with a singleton of IGD right
> now), but I'm not really very familiar with passthru nor the qemu side
> of this.
>
> What happens if you try to pass two different GFX cards to a guest?

Are you saying two IGDs? Its not possible since as I said above, IGD is 
tricky because it depends on something from ISA bridge and host bridge. 
So we can't provide two or more different setting configurations to own 
more IGDs just in one platform.

Thanks
Tiejun