From: Salvador Fandiño <[email protected]>

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 <[email protected]>
---
 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

Reply via email to