[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6] libxl: usb2 and usb3 controller support for upstream qemu



Il 26/09/2013 14:13, Fabio Fantoni ha scritto:
Il 26/09/2013 13:04, Anthony PERARD ha scritto:
On Mon, Sep 23, 2013 at 04:32:02PM +0200, Fabio Fantoni wrote:
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.

Changes from v5:
changed usb2 controller qemu parameters:
- removed bus
- added multifunction on all devices

Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxx>
---
  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..4ee28b3 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,addr=0x1d.0x7,multifunction=on");
+                for (i = 1; i < 4; i++)
+                    flexarray_append_pair(dm_args, "-device",
+ GCSPRINTF("ich9-usb-uhci%d,masterbus=usb.0,"
+ "firstport=%d,addr=0x1d.%#x,multifunction=on",
+                        i, 2*(i-1), 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);
There is one issue here, when using -usbdevice x, QEMU will always add
an usb v1 controller (almost equivalent to the "case 1" us usbversion).
So, this usbversion=x does not seems to belong to the if(usb ||
usbdevice).


On it's on, this patch does not add anything to QEMU, the machine will
just have more usb controller when someone will add an usbdevice
(tablet for example).

usbversion seems only usefull when used with usbredirection, so it's
probably worth adding the controller only when usbversion or
usbredirection is in the vm config file.

In other words, I think this will be better:
- leave intacte the if(usb || usbdevice) block.
- add if (usbversion) {
     add_controller_vX
   }
- and in the second patch that add usbredirection, change
   if(usbversion) to if(usbversion||spice.usbredirectino)

What do you think?

(and now, I'm going to try the second patch with spice usbredirection)


With new qemu parameters it works, all details here:
http://lists.xen.org/archives/html/xen-devel/2013-09/msg02376.html
I'm waiting the patch draft about usb change/hotplug of George Dunlap to see what he did. After that I'll try to include the necessary qemu parameters changes to have usb passthrough working with new controller patch. I'll improve the patch also in order to have unique block with usb passthrough (from dom0), usb redirection (from spice client) or both working, which I know ok from my previous test with all manual qemu parameters.

Since there were no news about, and didn't found George Dunlap's patches around (about adding hotplug only?), i'm thinking to try from myself to write a patch to be able to use the new '-device' parameters and to have a fully working usbpasstrough also with both usbversion and usbredirection patches.

My goal is to have usb2, usb3 and usbredirection in xen 4.4.

I've notice that actually there are no particular checks on the strings passed to a usbdevice and with a simple change we could have the new parameters full working ie by replacing usbdevice="host:058f:6387" with usbdevice="vendorid=0x058f,productid=0x6387".

Yes, there still be a retrocompatibility problem and possible issues when dealing with configurations that differs from 'usb-host' (ie 'tablet' ones).

So, my first thinking was about adding the section after 'usb-' part in a way that it become like this: usbdevice="host,vendorid=0x058f,productid=0x6387" and thus supporting all other cases (ie usbdevice="tablet"), but still we will have a retrocompatibility problem this way.

Another idea could be to retain the old sintax in the libxl but parsing the parameters and adding vendorid=0x e productid=0x when we match 4 hex digits while, in case of integer values adding hostbus= and hostaddr= instead and without ":" (ie 'tablet' case) add nothing.

Unfortunately I've no such knowledge to implement the last idea and since that it will probably takes too much effort and time to reach some good result.

Can anyone share some light or hints on this?

Thanks for any reply.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.