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

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

On 2015/1/29 18:50, Wei Liu wrote:
> On Thu, Jan 29, 2015 at 08:41:24AM +0800, Chen, Tiejun wrote:
>> On 2015/1/28 19:12, Wei Liu wrote:
>>> On Wed, Jan 28, 2015 at 08:42:56AM +0800, Chen, Tiejun wrote:
>>>> On 2015/1/27 22:40, Ian Jackson wrote:
>>>>> Chen, Tiejun writes ("Re: [Qemu-devel] [RFC][PATCH 1/1] libxl: add one machine property to support IGD GFX passthrough"):
>>>>>> On 2015/1/23 8:43, Chen, Tiejun wrote:
>>>>>>> On 2015/1/22 8:51, Chen, Tiejun wrote:
>>>>>>>> At this point I just concern here if we still use 'gfx_passthrou', at
>>>>>>>> least it may look like a backward compatibility with older versions of
>>>>>>>> qemu in Xen side, qemu-xen-traditional. But I'd like to follow your
>>>>>>>> final option.
>>>>> ...
>>>>>>> Any feedback to this option I should follow here?
>>>>>>
>>>>>> Ping...
>>>>>
>>>>> I think this is a question that qemu upstream should answer.
>>>>>
>>>>
>>>> Actually this is just commented by Gerd in qemu upstream. So now looks in
>>>> Xen side you guys don't have any objection to use 'igd-passthru' as well. If
>>>> yes, I'm fine to adopt this.
>>>>
>>>
>>> Yes, we would like to stay in line with upstream.
>>
>> Wei,
>>
>> Thanks for your response.
>>
>>>
>>> Just remember to handle old option in libxl if your old option is already
>>> released by some older version of QEMUs.
>>
>> I just drop that old option, -gfx_passthru, if we're under qemu upstream
>> circumstance, like this,
>>
>
> The question is, is there any version of qemu upstream that has
> been released that has the old option (-gfx-passthru)?

No. Just now we're starting to support IGD passthrough in qemu upstream.

>
> This gives us a situation that we need to support both the old
> (-gfx-passthru) and new (-igd-passthru) options. Presumably we (libxl)
> would need to fork a qemu process to determine which option it has and
> pass the right one.
>
> Or you can try to keep both old and new option at the same time but

Yeah, actually I also have considered to keep both two options at the 
same time. Its really friendly to any qemu version.

> deprecate the old one. Then in a few qemu release cycles later (or

This should be like 'accel=kvm' versus 'enable-kvm' in qemu upstream. 
They're coexisted now but just the former is a modern option.

> probably one year or two?) you can finally remove the old one. The point
> is that to give downstream (in this case, Xen) time to cope with the
> change.

Here I'm fine to this way.

So Gerd,

What about this?

>
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -318,7 +318,10 @@ static char **
>> libxl__build_device_model_args_old(libxl__gc *gc,
>>               flexarray_vappend(dm_args, "-net", "none", NULL);
>>           }
>>           if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
>> -            flexarray_append(dm_args, "-gfx_passthru");
>> +            if (b_info->device_model_version !=
>> +                LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
>> +                flexarray_append(dm_args, "-gfx_passthru");
>> +            }
>
> I don't think this is right if upstream qemu also supports gfx-passthru.
>
> However, you're modifying libxl__build_device_model_args_old, it
> strongly suggests that it only affects qemu-trad. That means you don't
> even need this patch...

You're right and thanks.

Thanks
Tiejun

>
> Wei.
>
>>           }
>>       } else {
>>           if (!sdl && !vnc)
>> @@ -702,7 +705,10 @@ static char **
>> libxl__build_device_model_args_new(libxl__gc *gc,
>>               flexarray_append(dm_args, "none");
>>           }
>>           if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
>> -            flexarray_append(dm_args, "-gfx_passthru");
>> +            if (b_info->device_model_version !=
>> +                LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
>> +                flexarray_append(dm_args, "-gfx_passthru");
>> +            }
>>           }
>>       } else {
>>           if (!sdl && !vnc) {
>>
>> Is this good enough?
>>
>> Thanks
>> Tiejun
>