All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
@ 2013-09-13 14:25 Fabio Fantoni
  2013-09-17 14:18 ` Ian Campbell
  2013-09-17 17:04 ` Anthony PERARD
  0 siblings, 2 replies; 8+ messages in thread
From: Fabio Fantoni @ 2013-09-13 14:25 UTC (permalink / raw)
  To: xen-devel
  Cc: Ian.Campbell, Stefano.Stabellini, George.Dunlap, Ian.Jackson,
	Fabio Fantoni, anthony.perard

Usage: usbversion=1|2|3 (default=2)
Specifies the type of an emulated USB bus in the guest. 1 for usb1,
2 for usb2 and 3 for usb3, it is available only with upstream qemu.
Default is 2.

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
 docs/man/xl.cfg.pod.5       |    6 ++++++
 tools/libxl/libxl.h         |   14 ++++++++++++++
 tools/libxl/libxl_create.c  |    3 +++
 tools/libxl/libxl_dm.c      |   25 ++++++++++++++++++++++++-
 tools/libxl/libxl_types.idl |    1 +
 tools/libxl/xl_cmdimpl.c    |    2 ++
 6 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 769767b..f768784 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1168,6 +1168,12 @@ device.
 
 Enables or disables an emulated USB bus in the guest.
 
+=item B<usbversion=NUMBER>
+ 
+Specifies the type of an emulated USB bus in the guest. 1 for usb1,
+2 for usb2 and 3 for usb3, it is available only with upstream qemu.
+Default is 2.
+
 =item B<usbdevice=[ "DEVICE", "DEVICE", ...]>
 
 Adds B<DEVICE>s to the emulated USB bus. The USB bus must also be
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 4cab294..e27308e 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -305,6 +305,20 @@
 #define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST 1
 
 /*
+ * LIBXL_HAVE_BUILDINFO_USBVERSION
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain hvm.usbversion, a integer type that contains a USB
+ * controller version to specify on the qemu upstream command-line.
+ *
+ * If it is set, callers may use hvm.usbversion to specify if the usb
+ * controller is usb1, usb2 or usb3.
+ *
+ * If this is not defined, the usb controller is only usb1.
+ */
+#define LIBXL_HAVE_BUILDINFO_USBVERSION 1
+
+/*
  * LIBXL_HAVE_DEVICE_BACKEND_DOMNAME
  *
  * If this is defined, libxl_device_* structures containing a backend_domid
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 7567238..a6bfb0a 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -229,6 +229,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             return ERROR_INVAL;
         }
 
+        if (!b_info->u.hvm.usbversion)
+            b_info->u.hvm.usbversion = 2;
+
         if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
             b_info->u.hvm.timer_mode =
                 LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 43c3bec..e0123b7 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -511,7 +511,30 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                     __func__);
                 return NULL;
             }
-            flexarray_append(dm_args, "-usb");
+
+            switch (b_info->u.hvm.usbversion) {
+            case 1:
+                flexarray_vappend(dm_args,
+                    "-device", "piix3-usb-uhci,id=usb", NULL);
+                break;
+            case 2:
+                flexarray_append_pair(dm_args, "-device",
+                    "ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x1d.0x7");
+                for (i = 1; i < 4; i++)
+                    flexarray_append_pair(dm_args, "-device",
+                        GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
+                        "firstport=%d,bus=pci.0%s,addr=0x1d.%#x", i,
+                        2*(i-1), i == 1 ?",multifunction=on" : "", i-1));
+                break;
+            case 3:
+                flexarray_vappend(dm_args,
+                    "-device", "nec-usb-xhci,id=usb", NULL);
+                break;
+            default:
+                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+                    "usbversion parameter is invalid must be between 1 and 3");
+                return NULL;
+            }
             if (b_info->u.hvm.usbdevice) {
                 flexarray_vappend(dm_args,
                                   "-usbdevice", b_info->u.hvm.usbdevice, NULL);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 049dbb5..34010ee 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -338,6 +338,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("serial",           string),
                                        ("boot",             string),
                                        ("usb",              libxl_defbool),
+                                       ("usbversion",       integer),
                                        # usbdevice:
                                        # - "tablet" for absolute mouse,
                                        # - "mouse" for PS/2 protocol relative mouse
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3d7eaad..0be71be 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1505,6 +1505,8 @@ skip_vfb:
         xlu_cfg_replace_string (config, "serial", &b_info->u.hvm.serial, 0);
         xlu_cfg_replace_string (config, "boot", &b_info->u.hvm.boot, 0);
         xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb, 0);
+        if (!xlu_cfg_get_long (config, "usbversion", &l, 0))
+            b_info->u.hvm.usbversion = l;
         switch (xlu_cfg_get_list_as_string_list(config, "usbdevice",
                                                 &b_info->u.hvm.usbdevice_list,
                                                 1))
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
  2013-09-13 14:25 [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu Fabio Fantoni
@ 2013-09-17 14:18 ` Ian Campbell
  2013-09-17 17:52   ` Fabio Fantoni
  2013-09-17 17:04 ` Anthony PERARD
  1 sibling, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2013-09-17 14:18 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: George.Dunlap, anthony.perard, xen-devel, Ian.Jackson,
	Stefano.Stabellini

On Fri, 2013-09-13 at 16:25 +0200, Fabio Fantoni wrote:

> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 769767b..f768784 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1168,6 +1168,12 @@ device.
>  
>  Enables or disables an emulated USB bus in the guest.
>  
> +=item B<usbversion=NUMBER>
> + 
> +Specifies the type of an emulated USB bus in the guest. 1 for usb1,
> +2 for usb2 and 3 for usb3, it is available only with upstream qemu.
> +Default is 2.

For trad qemu the default is effectively 1 and unchangeable, is that
right?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
  2013-09-13 14:25 [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu Fabio Fantoni
  2013-09-17 14:18 ` Ian Campbell
@ 2013-09-17 17:04 ` Anthony PERARD
       [not found]   ` <CABMPFziYtaUntrfo5C7zag=kKCJhFEGd=eWOFm0M4fsPW1Ke6A@mail.gmail.com>
  1 sibling, 1 reply; 8+ messages in thread
From: Anthony PERARD @ 2013-09-17 17:04 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Ian.Campbell, Stefano.Stabellini, George.Dunlap,
	Ian.Jackson, Anthony PERARD

On Fri, Sep 13, 2013 at 04:25:37PM +0200, Fabio Fantoni wrote:
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 43c3bec..e0123b7 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -511,7 +511,30 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>                      __func__);
>                  return NULL;
>              }
> -            flexarray_append(dm_args, "-usb");
> +
> +            switch (b_info->u.hvm.usbversion) {
> +            case 1:
> +                flexarray_vappend(dm_args,
> +                    "-device", "piix3-usb-uhci,id=usb", NULL);
> +                break;
> +            case 2:
> +                flexarray_append_pair(dm_args, "-device",
> +                    "ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x1d.0x7");
> +                for (i = 1; i < 4; i++)
> +                    flexarray_append_pair(dm_args, "-device",
> +                        GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
> +                        "firstport=%d,bus=pci.0%s,addr=0x1d.%#x", i,
> +                        2*(i-1), i == 1 ?",multifunction=on" : "", i-1));
> +                break;
> +            case 3:
> +                flexarray_vappend(dm_args,
> +                    "-device", "nec-usb-xhci,id=usb", NULL);
> +                break;
> +            default:
> +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> +                    "usbversion parameter is invalid must be between 1 and 3");
> +                return NULL;
> +            }
>              if (b_info->u.hvm.usbdevice) {
>                  flexarray_vappend(dm_args,
>                                    "-usbdevice", b_info->u.hvm.usbdevice, NULL);

I've took a look at the QEMU command line, and especially the usb2 case.
I think the bus=pci.0 is not needed, QEMU does not complain and the usb
controller is visible in the guest.

About the id, I think it would be better to name the different version
with different ids, they could be uhci, ehci and xhci as I saw in few
example, or it could be usb1, usb2, usb3. So when those id are used
later, they will carry which usb version there are providing, without
the need to know how the guest have been configured.

About the multifunction=on, do you know why it's only on applied to one
of the ich9-usb-uhci devices (in the for loop)? Would it change
something to applied to all of them?

Otherwise, there is some docs in the qemu.git tree to make sense of
this, and especially docs/usb2.txt.

-- 
Anthony PERARD

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
  2013-09-17 14:18 ` Ian Campbell
@ 2013-09-17 17:52   ` Fabio Fantoni
  0 siblings, 0 replies; 8+ messages in thread
From: Fabio Fantoni @ 2013-09-17 17:52 UTC (permalink / raw)
  To: Ian Campbell
  Cc: George.Dunlap, anthony.perard, xen-devel, Ian.Jackson,
	Stefano.Stabellini


[-- Attachment #1.1: Type: text/plain, Size: 670 bytes --]

Yes


2013/9/17 Ian Campbell <ian.campbell@citrix.com>

> On Fri, 2013-09-13 at 16:25 +0200, Fabio Fantoni wrote:
>
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index 769767b..f768784 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -1168,6 +1168,12 @@ device.
> >
> >  Enables or disables an emulated USB bus in the guest.
> >
> > +=item B<usbversion=NUMBER>
> > +
> > +Specifies the type of an emulated USB bus in the guest. 1 for usb1,
> > +2 for usb2 and 3 for usb3, it is available only with upstream qemu.
> > +Default is 2.
>
> For trad qemu the default is effectively 1 and unchangeable, is that
> right?
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 1106 bytes --]

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
       [not found]   ` <CABMPFziYtaUntrfo5C7zag=kKCJhFEGd=eWOFm0M4fsPW1Ke6A@mail.gmail.com>
@ 2013-09-18  7:44     ` Fabio Fantoni
  2013-09-20 13:08       ` Fabio Fantoni
  0 siblings, 1 reply; 8+ messages in thread
From: Fabio Fantoni @ 2013-09-18  7:44 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel, Ian Campbell, Stefano Stabellini,
	Ian Jackson, George Dunlap


[-- Attachment #1.1: Type: text/plain, Size: 3605 bytes --]

Il 17/09/2013 20:04, Fabio Fantoni ha scritto:
> About bus and multifunction should be correct based on reply of one 
> qemu developer time ago if I remember good.
> I also require advice of other improve for usb2 parameters if possible 
> without reply.
> On my latest tests:
> - on Ubuntu 12.04 domU usb1, usb2 and usb3 are working
> - on windows 7 64 bit domU usb 2 and usb3 are working, usb1 not (seem 
> windows problem, probably also other windows versions have this problem)
> About id seems needed for usb redirection, next test I'll try to 
> remove the id to check if automatic search and use the usb controller.
>
> Sorry for my bad english.
>

Sorry, I forgot the cc.

>
> 2013/9/17 Anthony PERARD <anthony.perard@citrix.com 
> <mailto:anthony.perard@citrix.com>>
>
>     On Fri, Sep 13, 2013 at 04:25:37PM +0200, Fabio Fantoni wrote:
>     > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>     > index 43c3bec..e0123b7 100644
>     > --- a/tools/libxl/libxl_dm.c
>     > +++ b/tools/libxl/libxl_dm.c
>     > @@ -511,7 +511,30 @@ static char **
>     libxl__build_device_model_args_new(libxl__gc *gc,
>     >                      __func__);
>     >                  return NULL;
>     >              }
>     > -            flexarray_append(dm_args, "-usb");
>     > +
>     > +            switch (b_info->u.hvm.usbversion) {
>     > +            case 1:
>     > +                flexarray_vappend(dm_args,
>     > +                    "-device", "piix3-usb-uhci,id=usb", NULL);
>     > +                break;
>     > +            case 2:
>     > +                flexarray_append_pair(dm_args, "-device",
>     > +  "ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x1d.0x7");
>     > +                for (i = 1; i < 4; i++)
>     > +                    flexarray_append_pair(dm_args, "-device",
>     > +  GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
>     > +  "firstport=%d,bus=pci.0%s,addr=0x1d.%#x", i,
>     > +                        2*(i-1), i == 1 ?",multifunction=on" :
>     "", i-1));
>     > +                break;
>     > +            case 3:
>     > +                flexarray_vappend(dm_args,
>     > +                    "-device", "nec-usb-xhci,id=usb", NULL);
>     > +                break;
>     > +            default:
>     > +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
>     > +                    "usbversion parameter is invalid must be
>     between 1 and 3");
>     > +                return NULL;
>     > +            }
>     >              if (b_info->u.hvm.usbdevice) {
>     >                  flexarray_vappend(dm_args,
>     >                                    "-usbdevice",
>     b_info->u.hvm.usbdevice, NULL);
>
>     I've took a look at the QEMU command line, and especially the usb2
>     case.
>     I think the bus=pci.0 is not needed, QEMU does not complain and
>     the usb
>     controller is visible in the guest.
>
>     About the id, I think it would be better to name the different version
>     with different ids, they could be uhci, ehci and xhci as I saw in few
>     example, or it could be usb1, usb2, usb3. So when those id are used
>     later, they will carry which usb version there are providing, without
>     the need to know how the guest have been configured.
>
>     About the multifunction=on, do you know why it's only on applied
>     to one
>     of the ich9-usb-uhci devices (in the for loop)? Would it change
>     something to applied to all of them?
>
>     Otherwise, there is some docs in the qemu.git tree to make sense of
>     this, and especially docs/usb2.txt.
>
>     --
>     Anthony PERARD
>
>


[-- Attachment #1.2: Type: text/html, Size: 6357 bytes --]

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
  2013-09-18  7:44     ` Fabio Fantoni
@ 2013-09-20 13:08       ` Fabio Fantoni
  2013-09-23  8:56         ` Fabio Fantoni
  0 siblings, 1 reply; 8+ messages in thread
From: Fabio Fantoni @ 2013-09-20 13:08 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel, Ian Campbell, Stefano Stabellini,
	Ian Jackson, George Dunlap


[-- Attachment #1.1: Type: text/plain, Size: 4218 bytes --]

Il 18/09/2013 09:44, Fabio Fantoni ha scritto:
> Il 17/09/2013 20:04, Fabio Fantoni ha scritto:
>> About bus and multifunction should be correct based on reply of one 
>> qemu developer time ago if I remember good.
>> I also require advice of other improve for usb2 parameters if 
>> possible without reply.
>> On my latest tests:
>> - on Ubuntu 12.04 domU usb1, usb2 and usb3 are working
>> - on windows 7 64 bit domU usb 2 and usb3 are working, usb1 not (seem 
>> windows problem, probably also other windows versions have this problem)
>> About id seems needed for usb redirection, next test I'll try to 
>> remove the id to check if automatic search and use the usb controller.
>>
>> Sorry for my bad english.
>>
>
> Sorry, I forgot the cc.
>
>>
>> 2013/9/17 Anthony PERARD <anthony.perard@citrix.com 
>> <mailto:anthony.perard@citrix.com>>
>>
>>     On Fri, Sep 13, 2013 at 04:25:37PM +0200, Fabio Fantoni wrote:
>>     > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>     > index 43c3bec..e0123b7 100644
>>     > --- a/tools/libxl/libxl_dm.c
>>     > +++ b/tools/libxl/libxl_dm.c
>>     > @@ -511,7 +511,30 @@ static char **
>>     libxl__build_device_model_args_new(libxl__gc *gc,
>>     >                      __func__);
>>     >                  return NULL;
>>     >              }
>>     > -            flexarray_append(dm_args, "-usb");
>>     > +
>>     > +            switch (b_info->u.hvm.usbversion) {
>>     > +            case 1:
>>     > +                flexarray_vappend(dm_args,
>>     > +                    "-device", "piix3-usb-uhci,id=usb", NULL);
>>     > +                break;
>>     > +            case 2:
>>     > +                flexarray_append_pair(dm_args, "-device",
>>     > +  "ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x1d.0x7");
>>     > +                for (i = 1; i < 4; i++)
>>     > +  flexarray_append_pair(dm_args, "-device",
>>     > +  GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
>>     > +  "firstport=%d,bus=pci.0%s,addr=0x1d.%#x", i,
>>     > +                        2*(i-1), i == 1 ?",multifunction=on" :
>>     "", i-1));
>>     > +                break;
>>     > +            case 3:
>>     > +                flexarray_vappend(dm_args,
>>     > +                    "-device", "nec-usb-xhci,id=usb", NULL);
>>     > +                break;
>>     > +            default:
>>     > +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
>>     > +                    "usbversion parameter is invalid must be
>>     between 1 and 3");
>>     > +                return NULL;
>>     > +            }
>>     >              if (b_info->u.hvm.usbdevice) {
>>     >                  flexarray_vappend(dm_args,
>>     >                                    "-usbdevice",
>>     b_info->u.hvm.usbdevice, NULL);
>>
>>     I've took a look at the QEMU command line, and especially the
>>     usb2 case.
>>     I think the bus=pci.0 is not needed, QEMU does not complain and
>>     the usb
>>     controller is visible in the guest.
>>

On my new test works also without bus=pci.0, I'll remove it on next 
patch version.

>>
>>     About the id, I think it would be better to name the different
>>     version
>>     with different ids, they could be uhci, ehci and xhci as I saw in few
>>     example, or it could be usb1, usb2, usb3. So when those id are used
>>     later, they will carry which usb version there are providing, without
>>     the need to know how the guest have been configured.
>>

On my new test usbredirection works also without adding the usb 
controller id, so it is possible to change it but I think is unuseful 
change it based on different usb version.

>>
>>     About the multifunction=on, do you know why it's only on applied
>>     to one
>>     of the ich9-usb-uhci devices (in the for loop)? Would it change
>>     something to applied to all of them?
>>

On my new test I added multifunction=on on all devices and all is 
working. I see on qemu docs/ich9-ehci-uhci.cfg and all devices have 
multifunction enabled, on next patch version I'll put it on all devices.

>>
>>     Otherwise, there is some docs in the qemu.git tree to make sense of
>>     this, and especially docs/usb2.txt.
>>
>>     --
>>     Anthony PERARD
>>
>>
>


[-- Attachment #1.2: Type: text/html, Size: 8893 bytes --]

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
  2013-09-20 13:08       ` Fabio Fantoni
@ 2013-09-23  8:56         ` Fabio Fantoni
  2013-09-23  9:52           ` Ian Campbell
  0 siblings, 1 reply; 8+ messages in thread
From: Fabio Fantoni @ 2013-09-23  8:56 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel, Ian Campbell, Stefano Stabellini,
	Ian Jackson, George Dunlap


[-- Attachment #1.1: Type: text/plain, Size: 5035 bytes --]

Il 20/09/2013 15:08, Fabio Fantoni ha scritto:
> Il 18/09/2013 09:44, Fabio Fantoni ha scritto:
>> Il 17/09/2013 20:04, Fabio Fantoni ha scritto:
>>> About bus and multifunction should be correct based on reply of one 
>>> qemu developer time ago if I remember good.
>>> I also require advice of other improve for usb2 parameters if 
>>> possible without reply.
>>> On my latest tests:
>>> - on Ubuntu 12.04 domU usb1, usb2 and usb3 are working
>>> - on windows 7 64 bit domU usb 2 and usb3 are working, usb1 not 
>>> (seem windows problem, probably also other windows versions have 
>>> this problem)
>>> About id seems needed for usb redirection, next test I'll try to 
>>> remove the id to check if automatic search and use the usb controller.
>>>
>>> Sorry for my bad english.
>>>
>>
>> Sorry, I forgot the cc.
>>
>>>
>>> 2013/9/17 Anthony PERARD <anthony.perard@citrix.com 
>>> <mailto:anthony.perard@citrix.com>>
>>>
>>>     On Fri, Sep 13, 2013 at 04:25:37PM +0200, Fabio Fantoni wrote:
>>>     > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>     > index 43c3bec..e0123b7 100644
>>>     > --- a/tools/libxl/libxl_dm.c
>>>     > +++ b/tools/libxl/libxl_dm.c
>>>     > @@ -511,7 +511,30 @@ static char **
>>>     libxl__build_device_model_args_new(libxl__gc *gc,
>>>     >                      __func__);
>>>     >                  return NULL;
>>>     >              }
>>>     > -            flexarray_append(dm_args, "-usb");
>>>     > +
>>>     > +            switch (b_info->u.hvm.usbversion) {
>>>     > +            case 1:
>>>     > +                flexarray_vappend(dm_args,
>>>     > +                    "-device", "piix3-usb-uhci,id=usb", NULL);
>>>     > +                break;
>>>     > +            case 2:
>>>     > +                flexarray_append_pair(dm_args, "-device",
>>>     > +  "ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x1d.0x7");
>>>     > +                for (i = 1; i < 4; i++)
>>>     > +  flexarray_append_pair(dm_args, "-device",
>>>     > +  GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
>>>     > +  "firstport=%d,bus=pci.0%s,addr=0x1d.%#x", i,
>>>     > +                        2*(i-1), i == 1 ?",multifunction=on"
>>>     : "", i-1));
>>>     > +                break;
>>>

I tried to change this part with:
flexarray_vappend(dm_args, "-device",
                         GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
"firstport=%d,addr=0x1d.%#x,multifunction=on",
                         i, 2*(i-1), i-1) );

But on xl create it stops with segfault:
xl -vvv create /etc/xen/W7.cfg
...
libxl: debug: libxl_dm.c:1286:libxl__spawn_local_dm:   -device
libxl: debug: libxl_dm.c:1286:libxl__spawn_local_dm: 
ich9-usb-uhci1,masterbus=usb.0,firstport=0,addr=0x1d.0,multifunction=on
libxl: debug: libxl_dm.c:1286:libxl__spawn_local_dm:
Segfault error

I not understand what is the problem, can someone help me to solve please?

Thanks for any reply.

>>>     > +            case 3:
>>>     > +                flexarray_vappend(dm_args,
>>>     > +                    "-device", "nec-usb-xhci,id=usb", NULL);
>>>     > +                break;
>>>     > +            default:
>>>     > +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
>>>     > +                    "usbversion parameter is invalid must be
>>>     between 1 and 3");
>>>     > +                return NULL;
>>>     > +            }
>>>     >              if (b_info->u.hvm.usbdevice) {
>>>     >                  flexarray_vappend(dm_args,
>>>     >  "-usbdevice", b_info->u.hvm.usbdevice, NULL);
>>>
>>>     I've took a look at the QEMU command line, and especially the
>>>     usb2 case.
>>>     I think the bus=pci.0 is not needed, QEMU does not complain and
>>>     the usb
>>>     controller is visible in the guest.
>>>
>
> On my new test works also without bus=pci.0, I'll remove it on next 
> patch version.
>
>>>
>>>     About the id, I think it would be better to name the different
>>>     version
>>>     with different ids, they could be uhci, ehci and xhci as I saw
>>>     in few
>>>     example, or it could be usb1, usb2, usb3. So when those id are used
>>>     later, they will carry which usb version there are providing,
>>>     without
>>>     the need to know how the guest have been configured.
>>>
>
> On my new test usbredirection works also without adding the usb 
> controller id, so it is possible to change it but I think is unuseful 
> change it based on different usb version.
>
>>>
>>>     About the multifunction=on, do you know why it's only on applied
>>>     to one
>>>     of the ich9-usb-uhci devices (in the for loop)? Would it change
>>>     something to applied to all of them?
>>>
>
> On my new test I added multifunction=on on all devices and all is 
> working. I see on qemu docs/ich9-ehci-uhci.cfg and all devices have 
> multifunction enabled, on next patch version I'll put it on all devices.
>
>>>
>>>     Otherwise, there is some docs in the qemu.git tree to make sense of
>>>     this, and especially docs/usb2.txt.
>>>
>>>     --
>>>     Anthony PERARD
>>>
>>>
>>
>


[-- Attachment #1.2: Type: text/html, Size: 11121 bytes --]

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu
  2013-09-23  8:56         ` Fabio Fantoni
@ 2013-09-23  9:52           ` Ian Campbell
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Campbell @ 2013-09-23  9:52 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: Anthony PERARD, George Dunlap, xen-devel, Ian Jackson,
	Stefano Stabellini

On Mon, 2013-09-23 at 10:56 +0200, Fabio Fantoni wrote:

> 
> I tried to change this part with:
> flexarray_vappend(dm_args, "-device",
>                         GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
>                         "firstport=%d,addr=0x1d.%#x,multifunction=on",
>                         i, 2*(i-1), i-1) );
> 
> But on xl create it stops with segfault:
> xl -vvv create /etc/xen/W7.cfg
> ...
> libxl: debug: libxl_dm.c:1286:libxl__spawn_local_dm:   -device
> libxl: debug: libxl_dm.c:1286:libxl__spawn_local_dm:
> ich9-usb-uhci1,masterbus=usb.0,firstport=0,addr=0x1d.0,multifunction=on
> libxl: debug: libxl_dm.c:1286:libxl__spawn_local_dm:   
> Segfault error
> 
> I not understand what is the problem, can someone help me to solve
> please?

flexarray_vappend needs a NULL terminator. You want ..._append_pair.

Ian.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-09-23  9:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-13 14:25 [PATCH v5] libxl: usb2 and usb3 controller support for upstream qemu Fabio Fantoni
2013-09-17 14:18 ` Ian Campbell
2013-09-17 17:52   ` Fabio Fantoni
2013-09-17 17:04 ` Anthony PERARD
     [not found]   ` <CABMPFziYtaUntrfo5C7zag=kKCJhFEGd=eWOFm0M4fsPW1Ke6A@mail.gmail.com>
2013-09-18  7:44     ` Fabio Fantoni
2013-09-20 13:08       ` Fabio Fantoni
2013-09-23  8:56         ` Fabio Fantoni
2013-09-23  9:52           ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.