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

[Jürgen Groß <jgross@xxxxxxxx>]

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;
+    }
+
+    /* Check for <bus>.<addr> specification. */
+    p = strchr(devname, '.');
+    if (p) {
+        filter.bus_num    = strtoul(devname, NULL, 0);
+        filter.addr       = strtoul(p + 1, NULL, 0);
+        goto out;
+    }
+
+    /* Check for <vendorid>:<productid> 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

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