[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V4 3/7] libxl: add pvusb API
On Mon, Jun 15, 2015 at 7:26 PM, Juergen Gross <jgross@xxxxxxxx> wrote: > On 06/15/2015 04:34 PM, George Dunlap wrote: >> >> On Mon, Jun 15, 2015 at 3:25 PM, JÃrgen Groà <jgross@xxxxxxxx> wrote: >>> >>> On 06/15/2015 04:17 PM, George Dunlap wrote: >>>> >>>> >>>> On Wed, Jun 10, 2015 at 4:20 AM, Chunyan Liu <cyliu@xxxxxxxx> 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 <bus>-<port> specification. */ >>> + p = strchr(devname, '-'); >>> + if (p && p != devname) { >>> + filter.bus_num = strtoul(devname, NULL, 0); >>> + filter.port = p + 1; >>> + goto out; >>> + } >> >> >> >> On my system bus:addr for my mouse is 002:005, while the "busid" (the >> corresponding directory in sysfs) is 2-3.3. >> >> This code doesn't appear to me to parse the above properly; or did I >> miss something? > > > Filling filter.bus_num and filter.port is enough. This was the only part > missing in qemu, finding the device via libusb using bus_num and port is > already existing. At least it is working for me. :-) Well, one of us is completely wrong. Let's follow the example above: $ ls /sys/bus/usb/devices/2-3* /sys/bus/usb/devices/2-3@ /sys/bus/usb/devices/2-3.1@ /sys/bus/usb/devices/2-3:1.0@ /sys/bus/usb/devices/2-3.1.1@ /sys/bus/usb/devices/2-3.1:1.0@ /sys/bus/usb/devices/2-3.1.1:1.0@ /sys/bus/usb/devices/2-3.1.2@ /sys/bus/usb/devices/2-3.1.2:1.0@ /sys/bus/usb/devices/2-3.1.2:1.1@ /sys/bus/usb/devices/2-3.2@ /sys/bus/usb/devices/2-3.2:1.0@ /sys/bus/usb/devices/2-3.3@ /sys/bus/usb/devices/2-3.3:1.0@ $ for i in /sys/bus/usb/devices/2-3*/; do grep . $i/{bus,dev}num ; done /sys/bus/usb/devices/2-3//busnum:2 /sys/bus/usb/devices/2-3//devnum:2 /sys/bus/usb/devices/2-3.1//busnum:2 /sys/bus/usb/devices/2-3.1//devnum:3 /sys/bus/usb/devices/2-3.1.1//busnum:2 /sys/bus/usb/devices/2-3.1.1//devnum:6 /sys/bus/usb/devices/2-3.1.2//busnum:2 /sys/bus/usb/devices/2-3.1.2//devnum:10 /sys/bus/usb/devices/2-3.2//busnum:2 /sys/bus/usb/devices/2-3.2//devnum:4 /sys/bus/usb/devices/2-3.3//busnum:2 /sys/bus/usb/devices/2-3.3//devnum:5 $ lsusb | grep "Bus 002" Bus 002 Device 005: ID 046d:c016 Logitech, Inc. Optical Wheel Mouse Bus 002 Device 004: ID f617:0905 Bus 002 Device 010: ID 1050:0111 Yubico.com Bus 002 Device 006: ID 0424:4060 Standard Microsystems Corp. Ultra Fast Media Reader Bus 002 Device 003: ID 0424:2640 Standard Microsystems Corp. USB 2.0 Hub Bus 002 Device 002: ID 0424:2514 Standard Microsystems Corp. USB 2.0 Hub Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub In other words, there are 6 distinct devices that correspond to "bus 2 port 3". I don't know what it was you were passing through, but giving qemu (or libusb) only "2-3" is absolutely not enough for it to distinquish between my mouse (002:005, at 2-3.3) and my yubikey (002:010 at 2-3.1.2). That's why the bus:addr convention was invented in the first place, I presume -- to abstract away the topology of the USB hubs for the user. The "busid" interface that Chunyan is describing requires the caller to find out that long name -- 2-3.1.2 -- rather than the traditional short name (002:010). Just accepting "2-3" is not sufficient. -George _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |