xl and GFX passthrough

xl and GFX passthrough

[Sergey Tovpeko]

[-- Attachment #1: Type: text/plain, Size: 615 bytes --]

Hello, everybody!

I knew recently that xl doesn't yet work with GFX passthrough for HVM 
guest. It makes me consider that main job for GFX passthrough are going 
for xm toolstack. My company is very interested to use xl tool, but we 
need also GFX passthrough.

The issue is not very hard to solve. It just needs to teach xl to 
recognize 'gfx_passthru' in config file and pass it to qemu-dm. And 
qemu-dm will choose the specific VGA bios to load into domain's memory 
in case of this option.  

I attached the patch, addressing this issue.

Please, consider to push it to main repository.

Thank you.

Sergey.



[-- Attachment #2: xl_gfx_passthru.patch --]
[-- Type: text/x-patch, Size: 2852 bytes --]

commit 2692d4133b93260aabb7bad438a0c288238d1295
Author: Sergey Tovpeko <tovpeko@altell.ru>
Date:   Fri Jul 9 20:33:39 2010 +0400

    Pass gfx_passthru option to the device_model.
    
    To enable gfx passthru, xl should parse 'gfx_passthru' parameter from config file, and pass it to qemu-dm.

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2c3fb0f..c630193 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1208,6 +1208,11 @@ static char ** libxl_build_device_model_args_old(libxl__gc *gc,
             flexarray_set(dm_args, num++, "-vcpu_avail");
             flexarray_set(dm_args, num++, libxl__sprintf(gc, "0x%x", info->vcpu_avail));
         }
+        if (info->gfx_passthru) {
+            flexarray_set(dm_args, num++, "-gfx_passthru");
+            flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d", info->gfx_passthru));
+        }
+
         for (i = 0; i < num_vifs; i++) {
             if (vifs[i].nictype == NICTYPE_IOEMU) {
                 char *smac = libxl__sprintf(gc, "%02x:%02x:%02x:%02x:%02x:%02x",
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index 81fcfd9..bf3c429 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -96,6 +96,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("disable_migrate", bool),
     ("kernel",          libxl_file_reference),
     ("cpuid",           libxl_cpuid_policy_list),
+    ("gfx_passthru",    integer),
     ("hvm",             integer),
     ("u", KeyedUnion(None, "hvm",
                 [("hvm", "%s", Struct(None,
@@ -161,6 +162,7 @@ libxl_device_model_info = Struct("device_model_info",[
     ("vcpu_avail",       integer,           False, "vcpus actually available"),
     ("xen_platform_pci", integer,           False, "enable/disable the xen platform pci device"),
     ("extra",            libxl_string_list, False, "extra parameters pass directly to qemu, NULL terminated"),
+    ("gfx_passthru",     integer),
     ],
     comment=
 """Device Model information.
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1eace78..415b9b8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -363,6 +363,7 @@ static void init_dm_info(libxl_device_model_info *dm_info,
     dm_info->apic = b_info->u.hvm.apic;
     dm_info->vcpus = b_info->max_vcpus;
     dm_info->vcpu_avail = b_info->cur_vcpus;
+    dm_info->gfx_passthru = b_info->gfx_passthru;
 
     dm_info->stdvga = 0;
     dm_info->vnc = 1;
@@ -900,6 +901,9 @@ static void parse_config_data(const char *configfile_filename_report,
 
     xlu_cfg_replace_string (config, "kernel", &b_info->kernel.path);
 
+    if (!xlu_cfg_get_long (config, "gfx_passthru", &l))
+        b_info->gfx_passthru = l;
+
     if (c_info->hvm == 1) {
         if (!xlu_cfg_get_long (config, "pae", &l))
             b_info->u.hvm.pae = l;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: xl and GFX passthrough

[Ian Campbell]

On Thu, 2011-01-13 at 09:12 +0000, Sergey Tovpeko wrote:
> diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
> index 81fcfd9..bf3c429 100644
> --- a/tools/libxl/libxl.idl
> +++ b/tools/libxl/libxl.idl
> @@ -96,6 +96,7 @@ libxl_domain_build_info =
> Struct("domain_build_info",[
>      ("disable_migrate", bool),
>      ("kernel",          libxl_file_reference),
>      ("cpuid",           libxl_cpuid_policy_list),
> +    ("gfx_passthru",    integer),
>      ("hvm",             integer),
>      ("u", KeyedUnion(None, "hvm",
>                  [("hvm", "%s", Struct(None,
> @@ -161,6 +162,7 @@ libxl_device_model_info =
> Struct("device_model_info",[
>      ("vcpu_avail",       integer,           False, "vcpus actually
> available"),
>      ("xen_platform_pci", integer,           False, "enable/disable
> the xen platform pci device"),
>      ("extra",            libxl_string_list, False, "extra parameters
> pass directly to qemu, NULL terminated"),
> +    ("gfx_passthru",     integer),
>      ],
>      comment=
>  """Device Model information. 

I think gfx_passthrough is actually a boolean, right? In that case it
should be declared as "bool" in the idl.

Ian.

Re: xl and GFX passthrough

[Sergey Tovpeko]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

Ian Campbell wrote:
>
>
> I think gfx_passthrough is actually a boolean, right? In that case it
> should be declared as "bool" in the idl.
>   

Well, the integer value of gfx_passthru had meaning in 2009, I suspect. 
It differentiated between IGD adapter and discrete adapter.
http://xen.1045712.n5.nabble.com/PATCH-0-2-v2-graphics-passthrough-with-VT-d-td2534811.html

At now, I didn't notice integer meaning of this variable in the code. I 
changed it to bool in the new patch.

Sergey.

[-- Attachment #2: xl_gfx_passthru.patch --]
[-- Type: text/x-patch, Size: 2905 bytes --]

commit 258e2a6cbccea92bd4d11cbe15963f139e8810cc
Author: Sergey Tovpeko <tovpeko@altell.ru>
Date:   Fri Jul 9 20:33:39 2010 +0400

    Pass gfx_passthru option to the device_model.
    
    To enable gfx passthru, xl should parse 'gfx_passthru' parameter from config file, and pass it to qemu-dm.

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2c3fb0f..c630193 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1208,6 +1208,11 @@ static char ** libxl_build_device_model_args_old(libxl__gc *gc,
             flexarray_set(dm_args, num++, "-vcpu_avail");
             flexarray_set(dm_args, num++, libxl__sprintf(gc, "0x%x", info->vcpu_avail));
         }
+        if (info->gfx_passthru) {
+            flexarray_set(dm_args, num++, "-gfx_passthru");
+            flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d", info->gfx_passthru));
+        }
+
         for (i = 0; i < num_vifs; i++) {
             if (vifs[i].nictype == NICTYPE_IOEMU) {
                 char *smac = libxl__sprintf(gc, "%02x:%02x:%02x:%02x:%02x:%02x",
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index 81fcfd9..8059328 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -96,6 +96,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("disable_migrate", bool),
     ("kernel",          libxl_file_reference),
     ("cpuid",           libxl_cpuid_policy_list),
+    ("gfx_passthru",    bool),
     ("hvm",             integer),
     ("u", KeyedUnion(None, "hvm",
                 [("hvm", "%s", Struct(None,
@@ -161,6 +162,7 @@ libxl_device_model_info = Struct("device_model_info",[
     ("vcpu_avail",       integer,           False, "vcpus actually available"),
     ("xen_platform_pci", integer,           False, "enable/disable the xen platform pci device"),
     ("extra",            libxl_string_list, False, "extra parameters pass directly to qemu, NULL terminated"),
+    ("gfx_passthru",     bool,              False, "GFX passthrough enabled or disabled"),
     ],
     comment=
 """Device Model information.
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1eace78..415b9b8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -363,6 +363,7 @@ static void init_dm_info(libxl_device_model_info *dm_info,
     dm_info->apic = b_info->u.hvm.apic;
     dm_info->vcpus = b_info->max_vcpus;
     dm_info->vcpu_avail = b_info->cur_vcpus;
+    dm_info->gfx_passthru = b_info->gfx_passthru;
 
     dm_info->stdvga = 0;
     dm_info->vnc = 1;
@@ -900,6 +901,9 @@ static void parse_config_data(const char *configfile_filename_report,
 
     xlu_cfg_replace_string (config, "kernel", &b_info->kernel.path);
 
+    if (!xlu_cfg_get_long (config, "gfx_passthru", &l))
+        b_info->gfx_passthru = l;
+
     if (c_info->hvm == 1) {
         if (!xlu_cfg_get_long (config, "pae", &l))
             b_info->u.hvm.pae = l;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: xl and GFX passthrough

[Ian Campbell]

On Thu, 2011-01-13 at 10:05 +0000, Sergey Tovpeko wrote:
> Ian Campbell wrote:
> >
> >
> > I think gfx_passthrough is actually a boolean, right? In that case it
> > should be declared as "bool" in the idl.
> >   
> 
> Well, the integer value of gfx_passthru had meaning in 2009, I suspect. 
> It differentiated between IGD adapter and discrete adapter.
> http://xen.1045712.n5.nabble.com/PATCH-0-2-v2-graphics-passthrough-with-VT-d-td2534811.html
> 
> At now, I didn't notice integer meaning of this variable in the code. I 
> changed it to bool in the new patch.

I just noticed that this appears to have been fixed by Daniel De Graf
just a few days ago, see 22711:4f6fee3a456e, which may only be in the
staging tree at http://xenbits.xen.org/staging/xen-unstable.hg since we
are waiting for a test pass.

Ian.

Re: xl and GFX passthrough

[Sergey Tovpeko]

>
> I just noticed that this appears to have been fixed by Daniel De Graf
> just a few days ago, see 22711:4f6fee3a456e, which may only be in the
> staging tree at http://xenbits.xen.org/staging/xen-unstable.hg since we
> are waiting for a test pass.
>
>   

Ian, Daniel
 
  my patch is almost like Daniel's one.  Thank you for pointing to it.

And there's one absent thing in Daniel's commit (4f6fee3a456e). To 
complete the gfx passthrough issue for xl we should pass this option to 
qemu-dm, like this :

+        if (info->gfx_passthru) {
+            flexarray_set(dm_args, num++, "-gfx_passthru");
+            flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d", 
info->gfx_passthru));
+        }

I suppose, without it 'qemu-dm' won't  know about gfx_passthru mode and 
won't load passthru'ed vga bios.

Daniel might plan to add this code or added it already?

Re: xl and GFX passthrough

[Ian Campbell]

On Thu, 2011-01-13 at 11:17 +0000, Sergey Tovpeko wrote:
> >
> > I just noticed that this appears to have been fixed by Daniel De Graf
> > just a few days ago, see 22711:4f6fee3a456e, which may only be in the
> > staging tree at http://xenbits.xen.org/staging/xen-unstable.hg since we
> > are waiting for a test pass.
> >
> >   
> 
> Ian, Daniel
>  
>   my patch is almost like Daniel's one.  Thank you for pointing to it.
> 
> And there's one absent thing in Daniel's commit (4f6fee3a456e). To 
> complete the gfx passthrough issue for xl we should pass this option to 
> qemu-dm, like this :
> 
> +        if (info->gfx_passthru) {
> +            flexarray_set(dm_args, num++, "-gfx_passthru");
> +            flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d", 
> info->gfx_passthru));
> +        }
> 
> I suppose, without it 'qemu-dm' won't  know about gfx_passthru mode and 
> won't load passthru'ed vga bios.
> 
> Daniel might plan to add this code or added it already?

Daniel's patch at <4D262B2A.30907@tycho.nsa.gov> includes this bit but
it appears to be missing from the commit in 22711:4f6fee3a456e.

IanJ: Any idea what's going on? Perhaps a missed .rej due to the move of
the dm stuff to libxl_dm.c between post and application?

Ian.

Re: xl and GFX passthrough

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] xl and GFX passthrough"):
> Daniel's patch at <4D262B2A.30907@tycho.nsa.gov> includes this bit but
> it appears to be missing from the commit in 22711:4f6fee3a456e.
> 
> IanJ: Any idea what's going on? Perhaps a missed .rej due to the move of
> the dm stuff to libxl_dm.c between post and application?

Very likely, sorry.

I will fix this up after rc1.

Ian.

Re: xl and GFX passthrough

[Ian Jackson]

I wrote:
> Ian Campbell writes ("Re: [Xen-devel] xl and GFX passthrough"):
> > Daniel's patch at <4D262B2A.30907@tycho.nsa.gov> includes this bit but
> > it appears to be missing from the commit in 22711:4f6fee3a456e.
> > 
> > IanJ: Any idea what's going on? Perhaps a missed .rej due to the move of
> > the dm stuff to libxl_dm.c between post and application?
> 
> Very likely, sorry.
> 
> I will fix this up after rc1.

I've applied the missing change.

Thanks,
Ian.

RE: xl and GFX passthrough

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

Sergey,

Did you get gfx_passthru to work with xl?  I found the latest source still does not seems to pass gfx_passthru to qemu correctly.

By the way, how to output debug messages to xl*.log file?  I tried LOG() and fprintf(stderr...) neither worked for me.

Allen

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Sergey Tovpeko
Sent: Thursday, January 13, 2011 3:18 AM
To: Ian Campbell; Daniel De Graaf
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] xl and GFX passthrough


>
> I just noticed that this appears to have been fixed by Daniel De Graf
> just a few days ago, see 22711:4f6fee3a456e, which may only be in the
> staging tree at http://xenbits.xen.org/staging/xen-unstable.hg since we
> are waiting for a test pass.
>
>   

Ian, Daniel
 
  my patch is almost like Daniel's one.  Thank you for pointing to it.

And there's one absent thing in Daniel's commit (4f6fee3a456e). To 
complete the gfx passthrough issue for xl we should pass this option to 
qemu-dm, like this :

+        if (info->gfx_passthru) {
+            flexarray_set(dm_args, num++, "-gfx_passthru");
+            flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d", 
info->gfx_passthru));
+        }

I suppose, without it 'qemu-dm' won't  know about gfx_passthru mode and 
won't load passthru'ed vga bios.

Daniel might plan to add this code or added it already?






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: xl and GFX passthrough

[Stefano Stabellini]

On Wed, 26 Jan 2011, Kay, Allen M wrote:
> By the way, how to output debug messages to xl*.log file?  I tried LOG() and fprintf(stderr...) neither worked for me.
> 

LOG should print to the logfile, while fprintf(stderr should just print
to stderr.
Considering that when you execute xl the command is just executed directly
by your instance of xl, you can probably debug your issues just using
printf or gdb.

Re: xl and GFX passthrough

[Daniel De Graaf]

On 01/25/2011 09:49 PM, Kay, Allen M wrote:
> Sergey,
> 
> Did you get gfx_passthru to work with xl?  I found the latest source still does not seems to pass gfx_passthru to qemu correctly.
> 
> By the way, how to output debug messages to xl*.log file?  I tried LOG() and fprintf(stderr...) neither worked for me.
> 
> Allen
> 
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Sergey Tovpeko
> Sent: Thursday, January 13, 2011 3:18 AM
> To: Ian Campbell; Daniel De Graaf
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] xl and GFX passthrough
> 
> 
>>
>> I just noticed that this appears to have been fixed by Daniel De Graf
>> just a few days ago, see 22711:4f6fee3a456e, which may only be in the
>> staging tree at http://xenbits.xen.org/staging/xen-unstable.hg since we
>> are waiting for a test pass.
>>
>>   
> 
> Ian, Daniel
>  
>   my patch is almost like Daniel's one.  Thank you for pointing to it.
> 
> And there's one absent thing in Daniel's commit (4f6fee3a456e). To 
> complete the gfx passthrough issue for xl we should pass this option to 
> qemu-dm, like this :
> 
> +        if (info->gfx_passthru) {
> +            flexarray_set(dm_args, num++, "-gfx_passthru");
> +            flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d", 
> info->gfx_passthru));
> +        }
> 
> I suppose, without it 'qemu-dm' won't  know about gfx_passthru mode and 
> won't load passthru'ed vga bios.
> 
> Daniel might plan to add this code or added it already?
> 

Actually, I hadn't noticed that gfx_passthru takes an (apparently optional)
numeric parameter. This change is more correct than the one I posted,
although it also needs gfx_passthru to be changed from bool to int in the
IDL (assuming that -gfx_passthru 2 is still useful, I hadn't heard of it
until recently).

Kay: sounds like you have this fixed in your tree, I'll let you or Ian
fix it.

-- 
Daniel De Graaf
National Security Agency