From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44679)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1UznNK-00031g-A1
	for qemu-devel@nongnu.org; Thu, 18 Jul 2013 08:32:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1UznNG-0002Ba-Jj
	for qemu-devel@nongnu.org; Thu, 18 Jul 2013 08:32:02 -0400
Received: from cantor2.suse.de ([195.135.220.15]:33401 helo=mx2.suse.de)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1UznNG-0002BU-BG
	for qemu-devel@nongnu.org; Thu, 18 Jul 2013 08:31:58 -0400
Message-ID: <51E7E039.70606@suse.de>
Date: Thu, 18 Jul 2013 14:31:53 +0200
From: =?ISO-8859-1?Q?Andreas_F=E4rber?= <afaerber@suse.de>
MIME-Version: 1.0
References: <1373624555-4403-1-git-send-email-fabio.fantoni@m2r.biz>
	<51DFE351.4000102@eu.citrix.com>
In-Reply-To: <51DFE351.4000102@eu.citrix.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
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>, 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, Anthony Liguori <anthony@codemonkey.ws>, pbonzini@redhat.com

Am 12.07.2013 13:06, schrieb George Dunlap:
> On 12/07/13 11:22, Fabio Fantoni wrote:
>> Usage: usbversion=3D1|2|3 (default=3D2)
>> 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.
>>   +=3Ditem B<usbversion=3DNUMBER>
>> +
>> +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.
>> +
>>   =3Ditem B<usbdevice=3D[ "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 =3D 2;
>> +
>>           if (b_info->u.hvm.timer_mode =3D=3D LIBXL_TIMER_MODE_DEFAULT=
)
>>               b_info->u.hvm.timer_mode =3D
>>                   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=3Dusb", NULL);
>> +                break;
>> +            case 2:
>> +                flexarray_vappend(dm_args,
>> "-device","ich9-usb-ehci1,id=3Dusb,"
>> +                    "bus=3Dpci.0,addr=3D0x1d.0x7",
>> "-device","ich9-usb-uhci1,"
>> +                  =20
>> "masterbus=3Dusb.0,firstport=3D0,bus=3Dpci.0,multifunction=3Don,"
>> +                  =20
>> "addr=3D0x1d.0x0","-device","ich9-usb-uhci2,masterbus=3Dusb.0,"
>> +                    "firstport=3D2,bus=3Dpci.0,addr=3D0x1d.0x1", "-de=
vice",
>> +                  =20
>> "ich9-usb-uhci3,masterbus=3Dusb.0,firstport=3D4,bus=3Dpci.0,"
>> +                    "addr=3D0x1d.0x2", NULL);
>=20
> I'm just curious, why is this so complicated?  Is this likely to be
> fragile and break in the future?

As pointed out previously, the bus=3Dpci.0 bit will break with different
PCI host bridges, such as the q35 machine existing today (-M q35 uses
pcie.0 instead and it has been discussed to make q35 the default at some
point). I had thus suggested to use a variable for the bus name to
abstract it. For -M pc-i440fx-1.5 etc. pci.0 should continue to work.

Regards,
Andreas

--=20
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe=
rg

From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?ISO-8859-1?Q?Andreas_F=E4rber?= <afaerber@suse.de>
Subject: Re: [PATCH v3] libxl: usb2 and usb3 controller support
 for upstream qemu
Date: Thu, 18 Jul 2013 14:31:53 +0200
Message-ID: <51E7E039.70606@suse.de>
References: <1373624555-4403-1-git-send-email-fabio.fantoni@m2r.biz>
	<51DFE351.4000102@eu.citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
In-Reply-To: <51DFE351.4000102@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>, 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, Anthony Liguori <anthony@codemonkey.ws>, pbonzini@redhat.com
List-Id: xen-devel@lists.xenproject.org

Am 12.07.2013 13:06, schrieb George Dunlap:
> On 12/07/13 11:22, Fabio Fantoni wrote:
>> Usage: usbversion=3D1|2|3 (default=3D2)
>> 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.
>>   +=3Ditem B<usbversion=3DNUMBER>
>> +
>> +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.
>> +
>>   =3Ditem B<usbdevice=3D[ "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 =3D 2;
>> +
>>           if (b_info->u.hvm.timer_mode =3D=3D LIBXL_TIMER_MODE_DEFAULT=
)
>>               b_info->u.hvm.timer_mode =3D
>>                   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=3Dusb", NULL);
>> +                break;
>> +            case 2:
>> +                flexarray_vappend(dm_args,
>> "-device","ich9-usb-ehci1,id=3Dusb,"
>> +                    "bus=3Dpci.0,addr=3D0x1d.0x7",
>> "-device","ich9-usb-uhci1,"
>> +                  =20
>> "masterbus=3Dusb.0,firstport=3D0,bus=3Dpci.0,multifunction=3Don,"
>> +                  =20
>> "addr=3D0x1d.0x0","-device","ich9-usb-uhci2,masterbus=3Dusb.0,"
>> +                    "firstport=3D2,bus=3Dpci.0,addr=3D0x1d.0x1", "-de=
vice",
>> +                  =20
>> "ich9-usb-uhci3,masterbus=3Dusb.0,firstport=3D4,bus=3Dpci.0,"
>> +                    "addr=3D0x1d.0x2", NULL);
>=20
> I'm just curious, why is this so complicated?  Is this likely to be
> fragile and break in the future?

As pointed out previously, the bus=3Dpci.0 bit will break with different
PCI host bridges, such as the q35 machine existing today (-M q35 uses
pcie.0 instead and it has been discussed to make q35 the default at some
point). I had thus suggested to use a variable for the bus name to
abstract it. For -M pc-i440fx-1.5 etc. pci.0 should continue to work.

Regards,
Andreas

--=20
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe=
rg