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

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

On 2015/2/4 18:41, Ian Campbell wrote:
> On Wed, 2015-02-04 at 09:34 +0800, Chen, Tiejun wrote:
>>>
>>>>        "-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.
>
> I'm afraid I don't follow, you seem to be simultaneously saying that
> qemu-xen both does and does not support -gfx_passthru, which cannot be
> right. I think from the following paragraph that what you mean is that
> upstream qemu-xen has no support for any kind of -gfx_passthru command
> line option in any form in any existing version, including the git tree.
> Is that correct?

Yes.

>
> (for the purposes of this conversation qemu-traditional is not of
> interest)

Yes.

>
>> 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.
>
> If one type VGA device needs these sorts of workaround it is not
> inconceivable that some other one will also need workarounds in the
> future.
>

Indeed this is not something workaround, and I think in any type of VGA 
devices, we'd like to diminish this sort of thing gradually, right?

This mightn't come true in real world :)

> Even if you don't consider non-IGD, what about the possibility of a
> future IGD device which needs different (or additional, or fewer)
> workarounds?

As far as I know we're trying to drop off those dependencies on ISA 
bridge and host bridge in our IGD's roadmap. Because in any pass through 
cases, theoretically we should access those resources dedicated to that 
device.

>
>>> 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.
>
> "gfx_passthru" is a generically named option, but it is being
> implemented in an IGD specific way. We need to consider the possibility
> of other graphics devices needing special handling in the future and
> plan accordingly such that we can try and maintain our API guarantees
> when this happens.

Agreed.

>
> I think there are three ways to achieve that:
>
>        * Make the libxl/xl option something which is not generic e.g.
>          igd_passthru=1
>        * Add a second option to allow the user to configure the kind of
>          graphics device being passed thru (e.g. gfx_passthru=1,
>          passthru_device="igd"), or combine the two by making the
>          gfx_passthru option a string instead of a boolean.

It makes more sense but this mean we're going to change that existing 
rule in qemu-traditional. But here I guess we shouldn't consider that case.

>        * Make libxl detect the type of graphics device somehow and
>          therefore automatically determine when gfx_passthru=1 =>
>          igd-passthru

This way confounds me all. Can libxl detect the graphics device *before* 
we intend to pass a parameter to active qemu?

>
>>   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.
>
> But by looking at the device 00:02.0 (e.g. its PCI vendor and device ID
> and other properties) we can unambiguously determine if it is an IGD
> device or not, can't we?

Again, like what I said above, I'm not sure if its possible in my case. 
If I'm wrong please correct me.

>
>>> 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?
>
> Yes, or any combination of two cards, perhaps from different vendors
> (AIUI some laptops have this with IGD and Nvidia or ATI?).

One IGD and multiple other type of Graphic display cards can coexist.

>
>>   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.
>
> This is because IGD must be a "primary" VGA device? I understand that

No. I mean ISA bridge and host bridge just provide one set of IGD 
resource so its difficult to configure two or more IGDs.

> there can only be one of those in a system, but I think it is possible
> to have multiple secondary VGA devices or different types in one system.
>

What I'm saying is, its impossible to own two same IGDs in our current 
platform :)

Thanks
Tiejun