From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?windows-1252?Q?J=FCrgen_Gro=DF?= Subject: Re: [PATCH V4 3/7] libxl: add pvusb API Date: Mon, 15 Jun 2015 16:25:03 +0200 Message-ID: <557EE03F.5040108@suse.com> References: <1433906441-3280-1-git-send-email-cyliu@suse.com> <1433906441-3280-4-git-send-email-cyliu@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap , Chunyan Liu Cc: Wei Liu , Ian Campbell , Ian Jackson , "xen-devel@lists.xen.org" , Jim Fehlig , Simon Cao List-Id: xen-devel@lists.xenproject.org On 06/15/2015 04:17 PM, George Dunlap wrote: > On Wed, Jun 10, 2015 at 4:20 AM, Chunyan Liu wrote: >> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> index 23f27d4..4561e1b 100644 >> --- a/tools/libxl/libxl_types.idl >> +++ b/tools/libxl/libxl_types.idl >> @@ -541,6 +541,29 @@ libxl_device_pci = Struct("device_pci", [ >> ("seize", bool), >> ]) >> >> +libxl_usb_protocol = Enumeration("usb_protocol", [ >> + (0, "AUTO"), >> + (1, "PV"), >> + (2, "QEMU"), >> + ]) >> + >> +libxl_device_usbctrl = Struct("device_usbctrl", [ >> + ("protocol", libxl_usb_protocol), >> + ("devid", libxl_devid), >> + ("version", integer), >> + ("ports", integer), >> + ("backend_domid", libxl_domid), >> + ("backend_domname", string), >> + ]) >> + >> +libxl_device_usb = Struct("device_usb", [ >> + ("ctrl", libxl_devid), >> + ("port", integer), >> + ("busid", string), >> + ("hostbus", integer), >> + ("hostaddr", integer), >> + ]) > > Ian / Ian / Wei / Jim: > > Question about the design of the interface here. > > The way that most systems in Linux specify particular USB devices is > with the bus:addr format. It's the output you get when you run tools > like lsusb, for example, and it's the interface that qemu (and thus > KVM) uses when talking about host devices to assign. bus:addr might > look like "002:006". > > But the bus:addr is a "public api" for Linux; internally, it has a > more structured format, which contains more about the USB topology. > Chunyan is calling this "busid". An example is something like this: > "2-3.1.1:1.0". > > Internally, pvusb needs "busid" in order to find the right sysfs > files. qemu, on the other hand, does not take busid; so the > devicemodel / HVM implementation of USB would need bus:addr > internally. A patch for qemu to support the busid is trivial, as the structures already contain the necessary elements: --- a/hw/usb/host-legacy.c +++ b/hw/usb/host-legacy.c @@ -53,11 +53,6 @@ static int parse_filter(const char *spec, struct USBAutoFilter *f) const char *p = spec; int i; - f->bus_num = 0; - f->addr = 0; - f->vendor_id = 0; - f->product_id = 0; - for (i = BUS; i < DONE; i++) { p = strpbrk(p, ":."); if (!p) { @@ -100,32 +95,47 @@ USBDevice *usb_host_device_open(USBBus *bus, const char *devname) dev = usb_create(bus, "usb-host"); + memset(&filter, 0, sizeof(filter)); + if (strstr(devname, "auto:")) { if (parse_filter(devname, &filter) < 0) { goto fail; } - } else { - p = strchr(devname, '.'); - if (p) { - filter.bus_num = strtoul(devname, NULL, 0); - filter.addr = strtoul(p + 1, NULL, 0); - filter.vendor_id = 0; - filter.product_id = 0; - } else { - p = strchr(devname, ':'); - if (p) { - filter.bus_num = 0; - filter.addr = 0; - filter.vendor_id = strtoul(devname, NULL, 16); - filter.product_id = strtoul(p + 1, NULL, 16); - } else { - goto fail; - } - } + goto out; } + /* Check for - specification. */ + p = strchr(devname, '-'); + if (p && p != devname) { + filter.bus_num = strtoul(devname, NULL, 0); + filter.port = p + 1; + goto out; + } + + /* Check for . specification. */ + p = strchr(devname, '.'); + if (p) { + filter.bus_num = strtoul(devname, NULL, 0); + filter.addr = strtoul(p + 1, NULL, 0); + goto out; + } + + /* Check for : specification. */ + p = strchr(devname, ':'); + if (p) { + filter.vendor_id = strtoul(devname, NULL, 16); + filter.product_id = strtoul(p + 1, NULL, 16); + goto out; + } + + goto fail; + +out: qdev_prop_set_uint32(&dev->qdev, "hostbus", filter.bus_num); qdev_prop_set_uint32(&dev->qdev, "hostaddr", filter.addr); + if (filter.port) { + qdev_prop_set_string(&dev->qdev, "port", filter.port); + } qdev_prop_set_uint32(&dev->qdev, "vendorid", filter.vendor_id); qdev_prop_set_uint32(&dev->qdev, "productid", filter.product_id); qdev_init_nofail(&dev->qdev); Juergen