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

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

On 2015/2/2 20:19, Wei Liu wrote:
> 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.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>> v2:
>>
>> * Based on some discussions with Wei we'd like to keep both old
>>    option, -gfx_passthru, and new machine property option,
>>    "-machine xxx,-igd-passthru=on" at the same time but deprecate
>>    the old one. Then finally we remove the old one at that point
>>    that to give downstream (in this case, Xen) time to cope with the
>>    change.
>>
>
> 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.

Understood.

>
>>   tools/libxl/libxl_dm.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index c2b0487..8405f0b 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -701,6 +701,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>
> Note this function is upstream QEMU specfic.

Yeah.

>
>>               flexarray_append(dm_args, "-net");
>>               flexarray_append(dm_args, "none");
>>           }
>> +        /*
>> +         * Although we already introduce 'igd-passthru', but we'd like
>> +         * to remove this until we give downstream time to cope with
>> +         * the change.
>> +         */
>>           if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
>>               flexarray_append(dm_args, "-gfx_passthru");
>>           }
>
> The comment contradicts what I know (or what I think I know). In our
> last email exchange you said there was no "-gfx_passthru" in any version
> of upstream QEMU.
>
> So, shouldn't you just remove this `if' statement?

Right. So what about this?

     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,
     "-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.

     Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index c2b0487..b888f19 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -701,9 +701,6 @@ static char ** 
libxl__build_device_model_args_new(libxl__gc *gc,
              flexarray_append(dm_args, "-net");
              flexarray_append(dm_args, "none");
          }
-        if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
-            flexarray_append(dm_args, "-gfx_passthru");
-        }
      } else {
          if (!sdl && !vnc) {
              flexarray_append(dm_args, "-nographic");
@@ -748,6 +745,11 @@ static char ** 
libxl__build_device_model_args_new(libxl__gc *gc,
                                              machinearg, max_ram_below_4g);
              }
          }
+
+        if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
+            machinearg = libxl__sprintf(gc, "%s,igd-passthru=on", 
machinearg);
+        }
+
          flexarray_append(dm_args, machinearg);
          for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; 
i++)
              flexarray_append(dm_args, b_info->extra_hvm[i]);

Thanks
Tiejun

>
> Wei.
>
>> @@ -748,6 +753,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>                                               machinearg, max_ram_below_4g);
>>               }
>>           }
>> +
>> +        if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
>> +            machinearg = libxl__sprintf(gc, "%s,igd-passthru=on", machinearg);
>> +        }
>> +
>>           flexarray_append(dm_args, machinearg);
>>           for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
>>               flexarray_append(dm_args, b_info->extra_hvm[i]);
>> --
>> 1.9.1
>