From: Salvador Fandiño <salva@xxxxxxxxxx> The usbip VHCI layer supports multiple "vhci_hcd" devices, every one emulating both a high speed and a super speed USB hub. These devices are exposed in sysfs as "vhci_hcd.0", "vhci_hcd.1", etc. But instead of controlling then by attributes inside their respective directories, all of then were controlled through "vhci_hcd.0" where the attributes "attach" and "detach" were used to connect and disconnect respectively remote USB devices to any of the virtual hub ports. Also port status was show but a series of "status.X" attributes, all inside "vhci_hcd.0". Besides the rather unusuality of this approach, it precludes users from doing interesting things, as for instance, restricting the access to vhci_hcd devices independently. This patch adds "attach", "detach", "status" and "nports" attributes inside every "vhci_hcd" object, besides "vhci_hcd.0" making then independent. Attribute "debug", allowing to control when debug messages for the usbip subsystems are generated is, as before, only attached to "vhci_hcd.0". Signed-off-by: Salvador Fandiño <salva@xxxxxxxxxx> --- drivers/usb/usbip/vhci.h | 21 ++- drivers/usb/usbip/vhci_hcd.c | 25 ++-- drivers/usb/usbip/vhci_sysfs.c | 311 +++++++++++++---------------------------- 3 files changed, 132 insertions(+), 225 deletions(-) diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h index 5659dce1526e..c1b848775be1 100644 --- a/drivers/usb/usbip/vhci.h +++ b/drivers/usb/usbip/vhci.h @@ -88,7 +88,14 @@ enum hub_speed { #define VHCI_NR_HCS 1 #endif -#define MAX_STATUS_NAME 16 +struct vhci_attrs { + struct dev_ext_attribute dev_attr_status; + struct dev_ext_attribute dev_attr_attach; + struct dev_ext_attribute dev_attr_detach; + struct dev_ext_attribute dev_attr_nports; + + struct attribute_group attribute_group; +}; struct vhci { spinlock_t lock; @@ -97,6 +104,8 @@ struct vhci { struct vhci_hcd *vhci_hcd_hs; struct vhci_hcd *vhci_hcd_ss; + + struct vhci_attrs *attrs; }; /* for usb_hcd.hcd_priv[0] */ @@ -126,8 +135,8 @@ extern struct attribute_group vhci_attr_group; void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed); /* vhci_sysfs.c */ -int vhci_init_attr_group(void); -void vhci_finish_attr_group(void); +int vhci_init_attr_group(struct vhci_hcd *vhci_hcd, int id); +void vhci_finish_attr_group(struct vhci_hcd *vhci_hcd); /* vhci_rx.c */ struct urb *pickup_urb_and_free_priv(struct vhci_device *vdev, __u32 seqnum); @@ -171,4 +180,10 @@ static inline struct vhci_hcd *vdev_to_vhci_hcd(struct vhci_device *vdev) return container_of((void *)(vdev - vdev->rhport), struct vhci_hcd, vdev); } +static inline struct vhci *device_attribute_to_vhci( + struct device_attribute *attr) +{ + return (struct vhci *)((struct dev_ext_attribute *)attr)->var; +} + #endif /* __USBIP_VHCI_H */ diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index c3e1008aa491..daabb06c9f46 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -1110,13 +1110,14 @@ static int vhci_setup(struct usb_hcd *hcd) static int vhci_start(struct usb_hcd *hcd) { struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd); + struct vhci *vhci = vhci_hcd->vhci; int id, rhport; int err; - usbip_dbg_vhci_hc("enter vhci_start\n"); + usbip_dbg_vhci_hc("enter vhci_start for %s\n", hcd_name(hcd)); if (usb_hcd_is_primary_hcd(hcd)) - spin_lock_init(&vhci_hcd->vhci->lock); + spin_lock_init(&vhci->lock); /* initialize private data of usb_hcd */ @@ -1143,16 +1144,17 @@ static int vhci_start(struct usb_hcd *hcd) } /* vhci_hcd is now ready to be controlled through sysfs */ - if (id == 0 && usb_hcd_is_primary_hcd(hcd)) { - err = vhci_init_attr_group(); + if (usb_hcd_is_primary_hcd(hcd)) { + err = vhci_init_attr_group(vhci_hcd, id); if (err) { pr_err("init attr group\n"); return err; } - err = sysfs_create_group(&hcd_dev(hcd)->kobj, &vhci_attr_group); + err = sysfs_create_group(&hcd_dev(hcd)->kobj, + &vhci->attrs->attribute_group); if (err) { pr_err("create sysfs files\n"); - vhci_finish_attr_group(); + vhci_finish_attr_group(vhci_hcd); return err; } pr_info("created sysfs %s\n", hcd_name(hcd)); @@ -1164,15 +1166,16 @@ static int vhci_start(struct usb_hcd *hcd) static void vhci_stop(struct usb_hcd *hcd) { struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd); - int id, rhport; + struct vhci *vhci = vhci_hcd->vhci; + int rhport; usbip_dbg_vhci_hc("stop VHCI controller\n"); /* 1. remove the userland interface of vhci_hcd */ - id = hcd_name_to_id(hcd_name(hcd)); - if (id == 0 && usb_hcd_is_primary_hcd(hcd)) { - sysfs_remove_group(&hcd_dev(hcd)->kobj, &vhci_attr_group); - vhci_finish_attr_group(); + if (usb_hcd_is_primary_hcd(hcd)) { + sysfs_remove_group(&hcd_dev(hcd)->kobj, + &vhci->attrs->attribute_group); + vhci_finish_attr_group(vhci_hcd); } /* 2. shutdown all the ports of vhci_hcd */ diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index 091f76b7196d..9e296ee9b9cb 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -57,24 +57,14 @@ static void port_show_vhci(char **out, int hub, int port, struct vhci_device *vd } /* Sysfs entry to show port status */ -static ssize_t status_show_vhci(int pdev_nr, char *out) +static ssize_t status_show_vhci(struct vhci *vhci, char *out) { - struct platform_device *pdev = vhcis[pdev_nr].pdev; - struct vhci *vhci; - struct usb_hcd *hcd; - struct vhci_hcd *vhci_hcd; char *s = out; int i; unsigned long flags; - if (!pdev || !out) { - usbip_dbg_vhci_sysfs("show status error\n"); + if (WARN_ON(!vhci) || WARN_ON(!out)) return 0; - } - - hcd = platform_get_drvdata(pdev); - vhci_hcd = hcd_to_vhci_hcd(hcd); - vhci = vhci_hcd->vhci; spin_lock_irqsave(&vhci->lock, flags); @@ -83,7 +73,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out) spin_lock(&vdev->ud.lock); port_show_vhci(&out, HUB_SPEED_HIGH, - pdev_nr * VHCI_PORTS + i, vdev); + i, vdev); spin_unlock(&vdev->ud.lock); } @@ -92,7 +82,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out) spin_lock(&vdev->ud.lock); port_show_vhci(&out, HUB_SPEED_SUPER, - pdev_nr * VHCI_PORTS + VHCI_HC_PORTS + i, vdev); + VHCI_HC_PORTS + i, vdev); spin_unlock(&vdev->ud.lock); } @@ -101,77 +91,21 @@ static ssize_t status_show_vhci(int pdev_nr, char *out) return out - s; } -static ssize_t status_show_not_ready(int pdev_nr, char *out) -{ - char *s = out; - int i = 0; - - for (i = 0; i < VHCI_HC_PORTS; i++) { - out += sprintf(out, "hs %04u %03u ", - (pdev_nr * VHCI_PORTS) + i, - VDEV_ST_NOTASSIGNED); - out += sprintf(out, "000 00000000 0000000000000000 0-0"); - out += sprintf(out, "\n"); - } - - for (i = 0; i < VHCI_HC_PORTS; i++) { - out += sprintf(out, "ss %04u %03u ", - (pdev_nr * VHCI_PORTS) + VHCI_HC_PORTS + i, - VDEV_ST_NOTASSIGNED); - out += sprintf(out, "000 00000000 0000000000000000 0-0"); - out += sprintf(out, "\n"); - } - return out - s; -} - -static int status_name_to_id(const char *name) -{ - char *c; - long val; - int ret; - - c = strchr(name, '.'); - if (c == NULL) - return 0; - - ret = kstrtol(c+1, 10, &val); - if (ret < 0) - return ret; - - return val; -} - static ssize_t status_show(struct device *dev, struct device_attribute *attr, char *out) { char *s = out; - int pdev_nr; - out += sprintf(out, "hub port sta spd dev socket local_busid\n"); - - pdev_nr = status_name_to_id(attr->attr.name); - if (pdev_nr < 0) - out += status_show_not_ready(pdev_nr, out); - else - out += status_show_vhci(pdev_nr, out); - + out += status_show_vhci(device_attribute_to_vhci(attr), out); return out - s; } static ssize_t nports_show(struct device *dev, struct device_attribute *attr, char *out) { - char *s = out; - - /* - * Half the ports are for SPEED_HIGH and half for SPEED_SUPER, - * thus the * 2. - */ - out += sprintf(out, "%d\n", VHCI_PORTS * vhci_num_controllers); - return out - s; + return sprintf(out, "%d\n", VHCI_PORTS); } -static DEVICE_ATTR_RO(nports); /* Sysfs entry to shutdown a virtual connection */ static int vhci_port_disconnect(struct vhci_hcd *vhci_hcd, __u32 rhport) @@ -205,14 +139,11 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci_hcd, __u32 rhport) return 0; } -static int valid_port(__u32 pdev_nr, __u32 rhport) +static int validate_port_in_range(__u32 port, __u32 base, __u32 top) { - if (pdev_nr >= vhci_num_controllers) { - pr_err("pdev %u\n", pdev_nr); - return 0; - } - if (rhport >= VHCI_HC_PORTS) { - pr_err("rhport %u\n", rhport); + if (port < base || port >= top) { + pr_err("Port number %u outside of range [%u-%u]\n", + port, base, top - 1); return 0; } return 1; @@ -221,34 +152,24 @@ static int valid_port(__u32 pdev_nr, __u32 rhport) static ssize_t store_detach(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { - __u32 port = 0, pdev_nr = 0, rhport = 0; - struct usb_hcd *hcd; - struct vhci_hcd *vhci_hcd; + struct vhci *vhci = device_attribute_to_vhci(attr); + __u32 port = 0; int ret; if (kstrtoint(buf, 10, &port) < 0) return -EINVAL; - pdev_nr = port_to_pdev_nr(port); - rhport = port_to_rhport(port); + usbip_dbg_vhci_sysfs("%s: detach port %d\n", dev_name(dev), port); - if (!valid_port(pdev_nr, rhport)) + if (!validate_port_in_range(port, 0, VHCI_PORTS)) return -EINVAL; - hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); - if (hcd == NULL) { - dev_err(dev, "port is not ready %u\n", port); - return -EAGAIN; - } - - usbip_dbg_vhci_sysfs("rhport %d\n", rhport); - - if ((port / VHCI_HC_PORTS) % 2) - vhci_hcd = hcd_to_vhci_hcd(hcd)->vhci->vhci_hcd_ss; + if (port >= VHCI_HC_PORTS) + ret = vhci_port_disconnect(vhci->vhci_hcd_ss, + port - VHCI_HC_PORTS); else - vhci_hcd = hcd_to_vhci_hcd(hcd)->vhci->vhci_hcd_hs; + ret = vhci_port_disconnect(vhci->vhci_hcd_hs, port); - ret = vhci_port_disconnect(vhci_hcd, rhport); if (ret < 0) return -EINVAL; @@ -256,29 +177,6 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr, return count; } -static DEVICE_ATTR(detach, S_IWUSR, NULL, store_detach); - -static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed) -{ - if (!valid_port(pdev_nr, rhport)) { - return 0; - } - - switch (speed) { - case USB_SPEED_LOW: - case USB_SPEED_FULL: - case USB_SPEED_HIGH: - case USB_SPEED_WIRELESS: - case USB_SPEED_SUPER: - break; - default: - pr_err("Failed attach request for unsupported USB speed: %s\n", - usb_speed_string(speed)); - return 0; - } - - return 1; -} /* Sysfs entry to establish a virtual connection */ /* @@ -295,50 +193,47 @@ static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed) static ssize_t store_attach(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { + struct vhci *vhci = device_attribute_to_vhci(attr); struct socket *socket; int sockfd = 0; - __u32 port = 0, pdev_nr = 0, rhport = 0, devid = 0, speed = 0; - struct usb_hcd *hcd; - struct vhci_hcd *vhci_hcd; + __u32 port = 0, devid = 0, speed = 0; struct vhci_device *vdev; - struct vhci *vhci; int err; unsigned long flags; /* - * @rhport: port number of vhci_hcd + * @port: port number of vhci_hcd * @sockfd: socket descriptor of an established TCP connection * @devid: unique device identifier in a remote host * @speed: usb device speed in a remote host */ if (sscanf(buf, "%u %u %u %u", &port, &sockfd, &devid, &speed) != 4) return -EINVAL; - pdev_nr = port_to_pdev_nr(port); - rhport = port_to_rhport(port); - usbip_dbg_vhci_sysfs("port(%u) pdev(%d) rhport(%u)\n", - port, pdev_nr, rhport); - usbip_dbg_vhci_sysfs("sockfd(%u) devid(%u) speed(%u)\n", - sockfd, devid, speed); + usbip_dbg_vhci_sysfs("%s: attach port(%u) sockfd(%u) devid(%u) speed(%u)\n", + dev_name(dev), port, sockfd, devid, speed); /* check received parameters */ - if (!valid_args(pdev_nr, rhport, speed)) + switch (speed) { + case USB_SPEED_LOW: + case USB_SPEED_FULL: + case USB_SPEED_HIGH: + case USB_SPEED_WIRELESS: + if (!validate_port_in_range(port, 0, VHCI_HC_PORTS)) + return -EINVAL; + vdev = &vhci->vhci_hcd_hs->vdev[port]; + break; + case USB_SPEED_SUPER: + if (!validate_port_in_range(port, VHCI_HC_PORTS, VHCI_PORTS)) + return -EINVAL; + vdev = &vhci->vhci_hcd_ss->vdev[port - VHCI_HC_PORTS]; + break; + default: + pr_err("Failed attach request for unsupported USB speed: %s\n", + usb_speed_string(speed)); return -EINVAL; - - hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); - if (hcd == NULL) { - dev_err(dev, "port %d is not ready\n", port); - return -EAGAIN; } - vhci_hcd = hcd_to_vhci_hcd(hcd); - vhci = vhci_hcd->vhci; - - if (speed == USB_SPEED_SUPER) - vdev = &vhci->vhci_hcd_ss->vdev[rhport]; - else - vdev = &vhci->vhci_hcd_hs->vdev[rhport]; - /* Extract socket from fd. */ socket = sockfd_lookup(sockfd, &err); if (!socket) @@ -357,7 +252,7 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr, sockfd_put(socket); - dev_err(dev, "port %d already used\n", rhport); + dev_err(dev, "port %u already used\n", port); /* * Will be retried from userspace * if there's another free port. @@ -365,8 +260,8 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr, return -EBUSY; } - dev_info(dev, "pdev(%u) rhport(%u) sockfd(%d)\n", - pdev_nr, rhport, sockfd); + dev_info(dev, "port(%u) sockfd(%d)\n", + port, sockfd); dev_info(dev, "devid(%u) speed(%u) speed_str(%s)\n", devid, speed, usb_speed_string(speed)); @@ -387,83 +282,77 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr, return count; } -static DEVICE_ATTR(attach, S_IWUSR, NULL, store_attach); -#define MAX_STATUS_NAME 16 - -struct status_attr { - struct device_attribute attr; - char name[MAX_STATUS_NAME+1]; -}; - -static struct status_attr *status_attrs; - -static void set_status_attr(int id) +int vhci_init_attr_group(struct vhci_hcd *vhci_hcd, int id) { - struct status_attr *status; + struct vhci_attrs *vhci_attrs; + struct attribute **attrs; + struct vhci *vhci = vhci_hcd->vhci; + int nattrs = ((id == 0) ? 5 : 4); - status = status_attrs + id; - if (id == 0) - strcpy(status->name, "status"); - else - snprintf(status->name, MAX_STATUS_NAME+1, "status.%d", id); - status->attr.attr.name = status->name; - status->attr.attr.mode = S_IRUGO; - status->attr.show = status_show; - sysfs_attr_init(&status->attr.attr); -} + if (WARN_ON(vhci->attrs != NULL)) + return -EADDRINUSE; -static int init_status_attrs(void) -{ - int id; + vhci_attrs = kcalloc(1, sizeof(*vhci_attrs), GFP_KERNEL); + if (vhci_attrs == NULL) + return -ENOMEM; - status_attrs = kcalloc(vhci_num_controllers, sizeof(struct status_attr), - GFP_KERNEL); - if (status_attrs == NULL) + attrs = kmalloc_array(nattrs + 1, sizeof(*attrs), GFP_KERNEL); + if (attrs == NULL) { + kfree(vhci_attrs); return -ENOMEM; + } - for (id = 0; id < vhci_num_controllers; id++) - set_status_attr(id); + vhci->attrs = vhci_attrs; + vhci_attrs->attribute_group.attrs = attrs; + + vhci_attrs->dev_attr_status.attr.attr.name = "status"; + vhci_attrs->dev_attr_status.attr.attr.mode = 0444; + vhci_attrs->dev_attr_status.attr.show = status_show; + vhci_attrs->dev_attr_status.var = vhci; + sysfs_attr_init(&vhci_attrs->dev_attr_status.attr.attr); + attrs[0] = &vhci_attrs->dev_attr_status.attr.attr; + + vhci_attrs->dev_attr_attach.attr.attr.name = "attach"; + vhci_attrs->dev_attr_attach.attr.attr.mode = 0200; + vhci_attrs->dev_attr_attach.attr.store = store_attach; + vhci_attrs->dev_attr_attach.var = vhci; + sysfs_attr_init(&vhci_attrs->dev_attr_attach.attr.attr); + attrs[1] = &vhci_attrs->dev_attr_attach.attr.attr; + + vhci_attrs->dev_attr_detach.attr.attr.name = "detach"; + vhci_attrs->dev_attr_detach.attr.attr.mode = 0200; + vhci_attrs->dev_attr_detach.attr.store = store_detach; + vhci_attrs->dev_attr_detach.var = vhci; + sysfs_attr_init(&vhci_attrs->dev_attr_detach.attr.attr); + attrs[2] = &vhci_attrs->dev_attr_detach.attr.attr; + + vhci_attrs->dev_attr_nports.attr.attr.name = "nports"; + vhci_attrs->dev_attr_nports.attr.attr.mode = 0444; + vhci_attrs->dev_attr_nports.attr.show = nports_show; + vhci_attrs->dev_attr_nports.var = vhci; + sysfs_attr_init(&vhci_attrs->dev_attr_nports.attr.attr); + attrs[3] = &vhci_attrs->dev_attr_nports.attr.attr; + + if (id == 0) { + attrs[4] = &dev_attr_usbip_debug.attr; + attrs[5] = NULL; + } else { + attrs[4] = NULL; + } return 0; } -static void finish_status_attrs(void) +void vhci_finish_attr_group(struct vhci_hcd *vhci_hcd) { - kfree(status_attrs); -} - -struct attribute_group vhci_attr_group = { - .attrs = NULL, -}; + struct vhci_attrs *vhci_attrs = vhci_hcd->vhci->attrs; -int vhci_init_attr_group(void) -{ - struct attribute **attrs; - int ret, i; + if (vhci_attrs) { + struct attribute **attrs = vhci_attrs->attribute_group.attrs; - attrs = kcalloc((vhci_num_controllers + 5), sizeof(struct attribute *), - GFP_KERNEL); - if (attrs == NULL) - return -ENOMEM; - - ret = init_status_attrs(); - if (ret) { kfree(attrs); - return ret; + kfree(vhci_attrs); + vhci_hcd->vhci->attrs = NULL; } - *attrs = &dev_attr_nports.attr; - *(attrs + 1) = &dev_attr_detach.attr; - *(attrs + 2) = &dev_attr_attach.attr; - *(attrs + 3) = &dev_attr_usbip_debug.attr; - for (i = 0; i < vhci_num_controllers; i++) - *(attrs + i + 4) = &((status_attrs + i)->attr.attr); - vhci_attr_group.attrs = attrs; - return 0; -} - -void vhci_finish_attr_group(void) -{ - finish_status_attrs(); - kfree(vhci_attr_group.attrs); } -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html