Re: [Xen-devel] [PATCH V4 3/7] libxl: add pvusb API

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

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?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel