Re: [Qemu-devel] [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Sep 03, 2015 at 12:45:12PM +0200, Juergen Gross wrote:
> When Xen is using the qemu usb framework for pure passthrough of I/Os
> to host devices the handling of isoc jobs is rather complicated if
> multiple isoc frames are transferred with one call.
> 
> Instead of calling the framework with each frame individually, using
> timers to avoid polling in a loop and sampling all responses to
> construct a sum response for the user, just add a capability to
> use the libusb isoc framework instead. This capability is selected
> via a device specific property.
> 
> When the property is selected the host usb driver will use xen specific

You mean it will use generic callbacks - but call Xen specific code?

> callbacks to signal the end of isoc I/Os. For now these callbacks will
> just be nops, they'll be filled with sensible actions when the xen
> pv-usb backend is being added.

The PV USB backend being in the QEMU driver?

But this patch by itself will avoid some complex code right?
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  hw/usb/core.c                | 11 ++++++----
>  hw/usb/host-libusb.c         | 51 ++++++++++++++++++++++++++++++++++++++------
>  include/hw/xen/xen_backend.h |  3 +++
>  3 files changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/usb/core.c b/hw/usb/core.c
> index cf34755..ed2255c 100644
> --- a/hw/usb/core.c
> +++ b/hw/usb/core.c
> @@ -427,10 +427,13 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p)
>      if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline || p->stream) {
>          usb_process_one(p);
>          if (p->status == USB_RET_ASYNC) {
> -            /* hcd drivers cannot handle async for isoc */
> -            assert(p->ep->type != USB_ENDPOINT_XFER_ISOC);
> -            /* using async for interrupt packets breaks migration */
> -            assert(p->ep->type != USB_ENDPOINT_XFER_INT ||
> +            /*
> +             * hcd drivers cannot handle async for isoc.
> +             * Using async for interrupt packets breaks migration.
> +             * Host devices are okay in any case.
> +             */
> +            assert((p->ep->type != USB_ENDPOINT_XFER_ISOC &&
> +                    p->ep->type != USB_ENDPOINT_XFER_INT) ||
>                     (dev->flags & (1 << USB_DEV_FLAG_IS_HOST)));
>              usb_packet_set_state(p, USB_PACKET_ASYNC);
>              QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue);
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index a5f9dab..ce644c3 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -42,6 +42,7 @@
>  #include "trace.h"
>  
>  #include "hw/usb.h"
> +#include "hw/xen/xen_backend.h"
>  
>  /* ------------------------------------------------------------------------ */
>  
> @@ -64,6 +65,7 @@ struct USBAutoFilter {
>  
>  enum USBHostDeviceOptions {
>      USB_HOST_OPT_PIPELINE,
> +    USB_HOST_XEN_ISO_PASSTHROUGH,
>  };
>  
>  struct USBHostDevice {
> @@ -152,6 +154,7 @@ static void usb_host_attach_kernel(USBHostDevice *s);
>  #define CONTROL_TIMEOUT  10000        /* 10 sec    */
>  #define BULK_TIMEOUT         0        /* unlimited */
>  #define INTR_TIMEOUT         0        /* unlimited */
> +#define ISO_TIMEOUT          0        /* unlimited */
>  
>  #if LIBUSBX_API_VERSION >= 0x01000103
>  # define HAVE_STREAMS 1
> @@ -306,14 +309,14 @@ static bool usb_host_use_combining(USBEndpoint *ep)
>  /* ------------------------------------------------------------------------ */
>  
>  static USBHostRequest *usb_host_req_alloc(USBHostDevice *s, USBPacket *p,
> -                                          bool in, size_t bufsize)
> +                                          bool in, size_t bufsize, int packets)
>  {
>      USBHostRequest *r = g_new0(USBHostRequest, 1);
>  
>      r->host = s;
>      r->p = p;
>      r->in = in;
> -    r->xfer = libusb_alloc_transfer(0);
> +    r->xfer = libusb_alloc_transfer(packets);
>      if (bufsize) {
>          r->buffer = g_malloc(bufsize);
>      }
> @@ -376,6 +379,29 @@ out:
>      }
>  }
>  
> +static void usb_host_req_complete_iso_xen(struct libusb_transfer *xfer)
> +{
> +    USBHostRequest *r = xfer->user_data;
> +    USBHostDevice  *s = r->host;
> +    bool disconnect = (xfer->status == LIBUSB_TRANSFER_NO_DEVICE);
> +
> +    if (r->p == NULL) {
> +        goto out; /* request was canceled */
> +    }
> +
> +    r->p->status = status_map[xfer->status];
> +    trace_usb_host_req_complete(s->bus_num, s->addr, r->p,
> +                                r->p->status, r->p->actual_length);
> +    /* copying of buffer is done in complete callback of xen */
> +    usb_packet_complete(USB_DEVICE(s), r->p);
> +
> +out:
> +    usb_host_req_free(r);
> +    if (disconnect) {
> +        usb_host_nodev(s);
> +    }
> +}
> +
>  static void usb_host_req_complete_data(struct libusb_transfer *xfer)
>  {
>      USBHostRequest *r = xfer->user_data;
> @@ -1226,7 +1252,7 @@ static void usb_host_handle_control(USBDevice *udev, USBPacket *p,
>          }
>      }
>  
> -    r = usb_host_req_alloc(s, p, (request >> 8) & USB_DIR_IN, length + 8);
> +    r = usb_host_req_alloc(s, p, (request >> 8) & USB_DIR_IN, length + 8, 0);
>      r->cbuf = data;
>      r->clen = length;
>      memcpy(r->buffer, udev->setup_buf, 8);
> @@ -1264,7 +1290,7 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p)
>      USBHostDevice *s = USB_HOST_DEVICE(udev);
>      USBHostRequest *r;
>      size_t size;
> -    int ep, rc;
> +    int ep, rc, packets;
>  
>      if (usb_host_use_combining(p->ep) && p->state == USB_PACKET_SETUP) {
>          p->status = USB_RET_ADD_TO_QUEUE;
> @@ -1289,7 +1315,7 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p)
>      switch (usb_ep_get_type(udev, p->pid, p->ep->nr)) {
>      case USB_ENDPOINT_XFER_BULK:
>          size = usb_packet_size(p);
> -        r = usb_host_req_alloc(s, p, p->pid == USB_TOKEN_IN, size);
> +        r = usb_host_req_alloc(s, p, p->pid == USB_TOKEN_IN, size, 0);
>          if (!r->in) {
>              usb_packet_copy(p, r->buffer, size);
>          }
> @@ -1313,7 +1339,7 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p)
>          }
>          break;
>      case USB_ENDPOINT_XFER_INT:
> -        r = usb_host_req_alloc(s, p, p->pid == USB_TOKEN_IN, p->iov.size);
> +        r = usb_host_req_alloc(s, p, p->pid == USB_TOKEN_IN, p->iov.size, 0);
>          if (!r->in) {
>              usb_packet_copy(p, r->buffer, p->iov.size);
>          }
> @@ -1324,6 +1350,17 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p)
>                                         INTR_TIMEOUT);
>          break;
>      case USB_ENDPOINT_XFER_ISOC:
> +        if (s->options & (1 << USB_HOST_XEN_ISO_PASSTHROUGH)) {
> +            packets = usbback_get_packets(p);
> +            r = usb_host_req_alloc(s, p, p->pid == USB_TOKEN_IN, p->iov.size,
> +                                   packets);
> +            ep = p->ep->nr | (r->in ? USB_DIR_IN : 0);
> +            libusb_fill_iso_transfer(r->xfer, s->dh, ep, r->buffer, p->iov.size,
> +                                     packets, usb_host_req_complete_iso_xen, r,
> +                                     ISO_TIMEOUT);
> +            usbback_set_iso_desc(p, r->xfer);
> +            break;
> +        }
>          if (p->pid == USB_TOKEN_IN) {
>              usb_host_iso_data_in(s, p);
>          } else {
> @@ -1484,6 +1521,8 @@ static Property usb_host_dev_properties[] = {
>                         LIBUSB_LOG_LEVEL_WARNING),
>      DEFINE_PROP_BIT("pipeline",    USBHostDevice, options,
>                      USB_HOST_OPT_PIPELINE, true),
> +    DEFINE_PROP_BIT("xen-iso-passthrough", USBHostDevice, options,
> +                    USB_HOST_XEN_ISO_PASSTHROUGH, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
> index 911ba6d..f194aae 100644
> --- a/include/hw/xen/xen_backend.h
> +++ b/include/hw/xen/xen_backend.h
> @@ -55,6 +55,9 @@ struct XenDevice {
>  
>  /* ------------------------------------------------------------- */
>  
> +#define usbback_get_packets(p) 0
> +#define usbback_set_iso_desc(p, xfer)
> +
>  /* variables */
>  extern XenXC xen_xc;
>  extern struct xs_handle *xenstore;
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel