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

[Fabio Fantoni <fabio.fantoni@xxxxxxx>]

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.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel