Re: [Xen-devel] [Patch V3 2/3] usb: Introduce Xen pvUSB frontend (xen hcd)

[Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>]

On Tue, Jun 16, 2015 at 04:32:34PM +0200, Juergen Gross wrote:
> Introduces the Xen pvUSB frontend. With pvUSB it is possible for a Xen
> domU to communicate with a USB device assigned to that domU. The
> communication is all done via the pvUSB backend in a driver domain
> (usually Dom0) which is owner of the physical device.
> 
> The pvUSB frontend is a USB hcd for a virtual USB host connector.
> 
> The code is taken from the pvUSB implementation in Xen done by Fujitsu
> based on Linux kernel 2.6.18.
> 
> Changes from the original version are:
> - port to upstream kernel
> - put all code in just one source file
> - move module to appropriate location in kernel tree
> - adapt to Linux style guide
> - minor code modifications to increase readability
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  drivers/usb/host/Kconfig   |   11 +
>  drivers/usb/host/Makefile  |    1 +
>  drivers/usb/host/xen-hcd.c | 1638 
> ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1650 insertions(+)
>  create mode 100644 drivers/usb/host/xen-hcd.c
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 197a6a3..3361b4b 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -788,3 +788,14 @@ config USB_HCD_TEST_MODE
>         This option is of interest only to developers who need to validate
>         their USB hardware designs.  It is not needed for normal use.  If
>         unsure, say N.
> +
> +config USB_XEN_HCD
> +     tristate "Xen usb virtual host driver"
> +     depends on XEN
> +     select XEN_XENBUS_FRONTEND
> +     help
> +       The Xen usb virtual host driver serves as a frontend driver enabling
> +       a Xen guest system to access USB Devices passed through to the guest
> +       by the Xen host (usually Dom0).
> +       Only needed if the kernel is running in a Xen guest and generic
> +       access to a USB device is needed.
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 65b0b6a..3779696 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -75,3 +75,4 @@ obj-$(CONFIG_USB_HCD_SSB)   += ssb-hcd.o
>  obj-$(CONFIG_USB_FUSBH200_HCD)       += fusbh200-hcd.o
>  obj-$(CONFIG_USB_FOTG210_HCD)        += fotg210-hcd.o
>  obj-$(CONFIG_USB_MAX3421_HCD)        += max3421-hcd.o
> +obj-$(CONFIG_USB_XEN_HCD)    += xen-hcd.o
> diff --git a/drivers/usb/host/xen-hcd.c b/drivers/usb/host/xen-hcd.c
> new file mode 100644
> index 0000000..9da2856
> --- /dev/null
> +++ b/drivers/usb/host/xen-hcd.c
> @@ -0,0 +1,1638 @@
> +/*
> + * xen-hcd.c
> + *
> + * Xen USB Virtual Host Controller driver
> + *
> + * Copyright (C) 2009, FUJITSU LABORATORIES LTD.
> + * Author: Noboru Iwamatsu <n_iwamatsu@xxxxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * Or, by your choice:
> + *
> + * When distributed separately from the Linux kernel or incorporated into
> + * other software packages, subject to the following license:
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, 
> and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/usb.h>
> +#include <linux/list.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/io.h>
> +
> +#include <xen/xen.h>
> +#include <xen/xenbus.h>
> +#include <xen/grant_table.h>
> +#include <xen/events.h>
> +#include <xen/page.h>
> +
> +#include <xen/interface/io/usbif.h>
> +
> +/* Private per-URB data */
> +struct urb_priv {
> +     struct list_head list;
> +     struct urb *urb;
> +     int req_id;             /* RING_REQUEST id for submitting */
> +     int unlink_req_id;      /* RING_REQUEST id for unlinking */
> +     int status;
> +     unsigned unlinked:1;    /* dequeued marker */
> +};
> +
> +/* virtual roothub port status */
> +struct rhport_status {
> +     __u32 status;
> +     unsigned resuming:1;            /* in resuming */
> +     unsigned c_connection:1;        /* connection changed */
> +     unsigned long timeout;
> +};
> +
> +/* status of attached device */
> +struct vdevice_status {
> +     int devnum;
> +     enum usb_device_state status;
> +     enum usb_device_speed speed;
> +};
> +
> +/* RING request shadow */
> +struct usb_shadow {
> +     struct xenusb_urb_request req;
> +     struct urb *urb;
> +};
> +
> +struct xenhcd_info {
> +     /* Virtual Host Controller has 4 urb queues */
> +     struct list_head pending_submit_list;
> +     struct list_head pending_unlink_list;
> +     struct list_head in_progress_list;
> +     struct list_head giveback_waiting_list;
> +
> +     spinlock_t lock;
> +
> +     /* timer that kick pending and giveback waiting urbs */
> +     struct timer_list watchdog;
> +     unsigned long actions;
> +
> +     /* virtual root hub */
> +     int rh_numports;
> +     struct rhport_status ports[XENUSB_MAX_PORTNR];
> +     struct vdevice_status devices[XENUSB_MAX_PORTNR];
> +
> +     /* Xen related staff */
> +     struct xenbus_device *xbdev;
> +     int urb_ring_ref;
> +     int conn_ring_ref;
> +     struct xenusb_urb_front_ring urb_ring;
> +     struct xenusb_conn_front_ring conn_ring;
> +
> +     unsigned int evtchn;
> +     unsigned int irq;
> +     struct usb_shadow shadow[XENUSB_URB_RING_SIZE];
> +     unsigned shadow_free;
> +};
> +
> +#define GRANT_INVALID_REF 0
> +
> +#define XENHCD_RING_JIFFIES (HZ/200)
> +#define XENHCD_SCAN_JIFFIES 1
> +
> +enum xenhcd_timer_action {
> +     TIMER_RING_WATCHDOG,
> +     TIMER_SCAN_PENDING_URBS,
> +};
> +
> +static struct kmem_cache *xenhcd_urbp_cachep;
> +
> +static inline struct xenhcd_info *xenhcd_hcd_to_info(struct usb_hcd *hcd)
> +{
> +     return (struct xenhcd_info *)hcd->hcd_priv;
> +}
> +
> +static inline struct usb_hcd *xenhcd_info_to_hcd(struct xenhcd_info *info)
> +{
> +     return container_of((void *)info, struct usb_hcd, hcd_priv);
> +}
> +
> +static inline void xenhcd_timer_action_done(struct xenhcd_info *info,
> +                                         enum xenhcd_timer_action action)
> +{
> +     clear_bit(action, &info->actions);
> +}
> +
> +static void xenhcd_timer_action(struct xenhcd_info *info,
> +                             enum xenhcd_timer_action action)
> +{
> +     if (timer_pending(&info->watchdog) &&
> +         test_bit(TIMER_SCAN_PENDING_URBS, &info->actions))
> +             return;
> +
> +     if (!test_and_set_bit(action, &info->actions)) {
> +             unsigned long t;
> +
> +             switch (action) {
> +             case TIMER_RING_WATCHDOG:
> +                     t = XENHCD_RING_JIFFIES;
> +                     break;
> +             default:
> +                     t = XENHCD_SCAN_JIFFIES;
> +                     break;
> +             }
> +             mod_timer(&info->watchdog, t + jiffies);
> +     }
> +}
> +
> +static ssize_t xenhcd_show_statistics(struct device *dev,
> +                                   struct device_attribute *attr, char *buf)
> +{
> +     struct usb_hcd *hcd;
> +     struct xenhcd_info *info;
> +     unsigned long flags;
> +     unsigned size;
> +
> +     hcd = dev_get_drvdata(dev);
> +     info = xenhcd_hcd_to_info(hcd);
> +
> +     spin_lock_irqsave(&info->lock, flags);
> +
> +     size = scnprintf(buf, PAGE_SIZE, "bus %s, device %s\n%s\n"
> +                      "xenhcd, hcd state %d\n",
> +                      hcd->self.controller->bus->name,
> +                      dev_name(hcd->self.controller), hcd->product_desc,
> +                      hcd->state);
> +
> +     spin_unlock_irqrestore(&info->lock, flags);
> +
> +     return size;
> +}
> +
> +static DEVICE_ATTR(statistics, S_IRUGO, xenhcd_show_statistics, NULL);

DEVICE_ATTR_RO() please.

As you are adding new sysfs files, we need Documentation/ABI/ entries.

Also note that I will reject this sysfs file for the obvious reasons
that it violates the one-value-per-file sysfs rule.  Nice try to sneak
this one by, don't ever do that again.

greg k-h

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