From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:33655)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <fabio.fantoni@m2r.biz>) id 1Uyenn-00055P-JI
	for qemu-devel@nongnu.org; Mon, 15 Jul 2013 05:10:42 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <fabio.fantoni@m2r.biz>) id 1Uyenk-0007DB-JZ
	for qemu-devel@nongnu.org; Mon, 15 Jul 2013 05:10:39 -0400
Received: from mail-bk0-f51.google.com ([209.85.214.51]:37871)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <fabio.fantoni@m2r.biz>) id 1Uyenk-0007Cy-8G
	for qemu-devel@nongnu.org; Mon, 15 Jul 2013 05:10:36 -0400
Received: by mail-bk0-f51.google.com with SMTP id ji1so4526568bkc.10
	for <qemu-devel@nongnu.org>; Mon, 15 Jul 2013 02:10:34 -0700 (PDT)
Message-ID: <51E3BC9B.8080503@m2r.biz>
Date: Mon, 15 Jul 2013 11:10:51 +0200
From: Fabio Fantoni <fabio.fantoni@m2r.biz>
MIME-Version: 1.0
References: <1373624555-4403-1-git-send-email-fabio.fantoni@m2r.biz>
	<51DFE351.4000102@eu.citrix.com> <51DFF83A.8030802@m2r.biz>
	<51E021B7.2050808@eu.citrix.com>
In-Reply-To: <51E021B7.2050808@eu.citrix.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3] libxl: usb2 and usb3 controller support
 for upstream qemu
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: xen-devel@lists.xensource.com, Ian.Campbell@citrix.com, Stefano.Stabellini@eu.citrix.com, wei.lui2@citrix.com, Ian.Jackson@eu.citrix.com, qemu-devel@nongnu.org, pbonzini@redhat.com

Il 12/07/2013 17:33, George Dunlap ha scritto:
> On 12/07/13 13:36, Fabio Fantoni wrote:
>> Il 12/07/2013 13:06, George Dunlap ha scritto:
>>> On 12/07/13 11:22, 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.
>>>>
>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>> ---
>>>>   docs/man/xl.cfg.pod.5       |    6 ++++++
>>>>   tools/libxl/libxl_create.c  |    3 +++
>>>>   tools/libxl/libxl_dm.c      |   25 ++++++++++++++++++++++++-
>>>>   tools/libxl/libxl_types.idl |    1 +
>>>>   tools/libxl/xl_cmdimpl.c    |    2 ++
>>>>   5 files changed, 36 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>>>> index 069b73f..602d428 100644
>>>> --- a/docs/man/xl.cfg.pod.5
>>>> +++ b/docs/man/xl.cfg.pod.5
>>>> @@ -1154,6 +1154,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_create.c b/tools/libxl/libxl_create.c
>>>> index 0c32d0b..9683740 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 7e54c02..aa8e131 100644
>>>> --- a/tools/libxl/libxl_dm.c
>>>> +++ b/tools/libxl/libxl_dm.c
>>>> @@ -492,7 +492,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_vappend(dm_args, 
>>>> "-device","ich9-usb-ehci1,id=usb,"
>>>> +                    "bus=pci.0,addr=0x1d.0x7", 
>>>> "-device","ich9-usb-uhci1,"
>>>> + "masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,"
>>>> + "addr=0x1d.0x0","-device","ich9-usb-uhci2,masterbus=usb.0,"
>>>> +                    "firstport=2,bus=pci.0,addr=0x1d.0x1", "-device",
>>>> + "ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,"
>>>> +                    "addr=0x1d.0x2", NULL);
>>>
>>> I'm just curious, why is this so complicated?  Is this likely to be 
>>> fragile and break in the future?
>>>
>>> Also, it's worth thinking a bit about the id -- maybe something 
>>> slightly more descriptive, like "usb-hub-root" or something?
>>
>> I tried already but there are problems with retrocompatibility:
>> http://lists.xen.org/archives/html/xen-devel/2013-07/msg00491.html
>> I was also asking if it is possible to remove some hardcoded options 
>> without breaking something but I had no reply.
>
> So this seems to be a response to the first paragraph ("why is this so 
> complicated, is it fragile").  What about the second question -- 
> "id=usb" instead of "id=usb-hub-root"?  Or was there something in the 
> referenced thread I'm not understanding?
>
>  -George
>
Sorry for the forgetfulness.
id=usb is short and I see it also in kvm with libvirt where on usb 
redirection channels are working even if the usb bus is not specified.
Same thing tested and working on xen, probably take usb bus id as default.
May I have to investigate further about it?

From mboxrd@z Thu Jan  1 00:00:00 1970
From: Fabio Fantoni <fabio.fantoni@m2r.biz>
Subject: Re: [PATCH v3] libxl: usb2 and usb3 controller support
 for upstream qemu
Date: Mon, 15 Jul 2013 11:10:51 +0200
Message-ID: <51E3BC9B.8080503@m2r.biz>
References: <1373624555-4403-1-git-send-email-fabio.fantoni@m2r.biz>
	<51DFE351.4000102@eu.citrix.com> <51DFF83A.8030802@m2r.biz>
	<51E021B7.2050808@eu.citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
In-Reply-To: <51E021B7.2050808@eu.citrix.com>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: xen-devel@lists.xensource.com, Ian.Campbell@citrix.com, Stefano.Stabellini@eu.citrix.com, wei.lui2@citrix.com, Ian.Jackson@eu.citrix.com, qemu-devel@nongnu.org, pbonzini@redhat.com
List-Id: xen-devel@lists.xenproject.org

Il 12/07/2013 17:33, George Dunlap ha scritto:
> On 12/07/13 13:36, Fabio Fantoni wrote:
>> Il 12/07/2013 13:06, George Dunlap ha scritto:
>>> On 12/07/13 11:22, 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.
>>>>
>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>> ---
>>>>   docs/man/xl.cfg.pod.5       |    6 ++++++
>>>>   tools/libxl/libxl_create.c  |    3 +++
>>>>   tools/libxl/libxl_dm.c      |   25 ++++++++++++++++++++++++-
>>>>   tools/libxl/libxl_types.idl |    1 +
>>>>   tools/libxl/xl_cmdimpl.c    |    2 ++
>>>>   5 files changed, 36 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>>>> index 069b73f..602d428 100644
>>>> --- a/docs/man/xl.cfg.pod.5
>>>> +++ b/docs/man/xl.cfg.pod.5
>>>> @@ -1154,6 +1154,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_create.c b/tools/libxl/libxl_create.c
>>>> index 0c32d0b..9683740 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 7e54c02..aa8e131 100644
>>>> --- a/tools/libxl/libxl_dm.c
>>>> +++ b/tools/libxl/libxl_dm.c
>>>> @@ -492,7 +492,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_vappend(dm_args, 
>>>> "-device","ich9-usb-ehci1,id=usb,"
>>>> +                    "bus=pci.0,addr=0x1d.0x7", 
>>>> "-device","ich9-usb-uhci1,"
>>>> + "masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,"
>>>> + "addr=0x1d.0x0","-device","ich9-usb-uhci2,masterbus=usb.0,"
>>>> +                    "firstport=2,bus=pci.0,addr=0x1d.0x1", "-device",
>>>> + "ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,"
>>>> +                    "addr=0x1d.0x2", NULL);
>>>
>>> I'm just curious, why is this so complicated?  Is this likely to be 
>>> fragile and break in the future?
>>>
>>> Also, it's worth thinking a bit about the id -- maybe something 
>>> slightly more descriptive, like "usb-hub-root" or something?
>>
>> I tried already but there are problems with retrocompatibility:
>> http://lists.xen.org/archives/html/xen-devel/2013-07/msg00491.html
>> I was also asking if it is possible to remove some hardcoded options 
>> without breaking something but I had no reply.
>
> So this seems to be a response to the first paragraph ("why is this so 
> complicated, is it fragile").  What about the second question -- 
> "id=usb" instead of "id=usb-hub-root"?  Or was there something in the 
> referenced thread I'm not understanding?
>
>  -George
>
Sorry for the forgetfulness.
id=usb is short and I see it also in kvm with libvirt where on usb 
redirection channels are working even if the usb bus is not specified.
Same thing tested and working on xen, probably take usb bus id as default.
May I have to investigate further about it?