From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55357)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <George.Dunlap@eu.citrix.com>) id 1UxfLe-000501-BF
	for qemu-devel@nongnu.org; Fri, 12 Jul 2013 11:33:35 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <George.Dunlap@eu.citrix.com>) id 1UxfLZ-0002yS-C8
	for qemu-devel@nongnu.org; Fri, 12 Jul 2013 11:33:30 -0400
Received: from smtp.citrix.com ([66.165.176.89]:34133)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <George.Dunlap@eu.citrix.com>) id 1UxfLZ-0002y8-6Q
	for qemu-devel@nongnu.org; Fri, 12 Jul 2013 11:33:25 -0400
Message-ID: <51E021B7.2050808@eu.citrix.com>
Date: Fri, 12 Jul 2013 16:33:11 +0100
From: George Dunlap <george.dunlap@eu.citrix.com>
MIME-Version: 1.0
References: <1373624555-4403-1-git-send-email-fabio.fantoni@m2r.biz>
	<51DFE351.4000102@eu.citrix.com> <51DFF83A.8030802@m2r.biz>
In-Reply-To: <51DFF83A.8030802@m2r.biz>
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: Fabio Fantoni <fabio.fantoni@m2r.biz>
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

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

From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <george.dunlap@eu.citrix.com>
Subject: Re: [PATCH v3] libxl: usb2 and usb3 controller support
 for upstream qemu
Date: Fri, 12 Jul 2013 16:33:11 +0100
Message-ID: <51E021B7.2050808@eu.citrix.com>
References: <1373624555-4403-1-git-send-email-fabio.fantoni@m2r.biz>
	<51DFE351.4000102@eu.citrix.com> <51DFF83A.8030802@m2r.biz>
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: <51DFF83A.8030802@m2r.biz>
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: Fabio Fantoni <fabio.fantoni@m2r.biz>
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

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