[PATCH 1/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0
From: Salvador Fandino
Date: Tue Jan 30 2018 - 03:45:06 EST
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