[PATCH 0/3] usbip: Convert vhci-hcd Kconfig related to number of ports to module parameters

[PATCH 0/3] usbip: Convert vhci-hcd Kconfig related to number of ports to module parameters

[Jakub Lecki]

In workflows involving a greater number of remote
USB/IP devices, the default number of available virtual ports may be
insufficient, forcing user to recompile the module with greater number
of configured virtual host controllers and/or number of ports.

Allow a user to configure the number of USB/IP virtual host controllers
and number of its ports using module parameters to simplify the usage of
this module.

Jakub Lecki (3):
  usbip: Convert CONFIG_USBIP_VHCI_NR_HCS to a module parameter.
  usbip: Convert CONFIG_USBIP_VHCI_HC_PORTS to a module parameter.
  usbip: Limit maximum number of virtual host controllers to 31.

 drivers/usb/usbip/Kconfig      | 21 ------------
 drivers/usb/usbip/vhci.h       | 42 +++++++++++++-----------
 drivers/usb/usbip/vhci_hcd.c   | 59 +++++++++++++++++++++++++---------
 drivers/usb/usbip/vhci_sysfs.c | 18 +++++------
 4 files changed, 75 insertions(+), 65 deletions(-)

-- 
2.43.0

[PATCH 1/3] usbip: Convert CONFIG_USBIP_VHCI_NR_HCS to a module parameter.

[Jakub Lecki]

In workflows involving a greater number of remote
USB/IP devices, the default number of available virtual ports may be
insufficient, forcing user to recompile the module with greater number
of configured virtual host controllers and/or number of ports.

Allow a user to configure the number of USB/IP virtual host controllers
via a new 'num_controllers' module parameter to simplify the usage of
this module.

VHCI controller structures are already dynamically allocated during
module initialization, so the only change is switch from assigning
'vhci_num_controllers' via Kconfig to using the module parameter
framework.

- Remove the USBIP_VHCI_NR_HCS Kconfig option and replace it with a
  module parameter.
- Trim the value of the configured 'num_controllers' parameter if it
  exceeds bounds, and emit a warning.

Signed-off-by: Jakub Lecki <lec.jakub@gmail.com>
---
 drivers/usb/usbip/Kconfig    | 11 -----------
 drivers/usb/usbip/vhci.h     |  9 +++------
 drivers/usb/usbip/vhci_hcd.c | 16 ++++++++++++++--
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/usbip/Kconfig b/drivers/usb/usbip/Kconfig
index b9f94e2e278d..bdcb6f4fdbec 100644
--- a/drivers/usb/usbip/Kconfig
+++ b/drivers/usb/usbip/Kconfig
@@ -38,17 +38,6 @@ config USBIP_VHCI_HC_PORTS
 	  host controller driver, this defines number of ports per
 	  USB/IP virtual host controller.
 
-config USBIP_VHCI_NR_HCS
-	int "Number of USB/IP virtual host controllers"
-	range 1 128
-	default 1
-	depends on USBIP_VHCI_HCD
-	help
-	  To increase number of ports available for USB/IP virtual
-	  host controller driver, this defines number of USB/IP
-	  virtual host controllers as if adding physical host
-	  controllers.
-
 config USBIP_HOST
 	tristate "Host driver"
 	depends on USBIP_CORE && USB
diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 5659dce1526e..30b8540e0b49 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -82,11 +82,8 @@ enum hub_speed {
 /* Each VHCI has 2 hubs (USB2 and USB3), each has VHCI_HC_PORTS ports */
 #define VHCI_PORTS	(VHCI_HC_PORTS*2)
 
-#ifdef CONFIG_USBIP_VHCI_NR_HCS
-#define VHCI_NR_HCS CONFIG_USBIP_VHCI_NR_HCS
-#else
-#define VHCI_NR_HCS 1
-#endif
+#define VHCI_DEFAULT_NR_HCS 1
+#define VHCI_MAX_NR_HCS 128
 
 #define MAX_STATUS_NAME 16
 
@@ -118,7 +115,7 @@ struct vhci_hcd {
 	struct vhci_device vdev[VHCI_HC_PORTS];
 };
 
-extern int vhci_num_controllers;
+extern unsigned int vhci_num_controllers;
 extern struct vhci *vhcis;
 extern struct attribute_group vhci_attr_group;
 
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index e70fba9f55d6..93c3fa3e1c53 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -10,6 +10,7 @@
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/printk.h>
 #include <linux/slab.h>
 #include <linux/string_choices.h>
 
@@ -44,7 +45,12 @@ static int vhci_get_frame_number(struct usb_hcd *hcd);
 static const char driver_name[] = "vhci_hcd";
 static const char driver_desc[] = "USB/IP Virtual Host Controller";
 
-int vhci_num_controllers = VHCI_NR_HCS;
+unsigned int vhci_num_controllers = VHCI_DEFAULT_NR_HCS;
+module_param_named(num_controllers, vhci_num_controllers, uint, 0444);
+MODULE_PARM_DESC(num_controllers, "Number of USB/IP virtual host controllers (range: 0-"
+		 __MODULE_STRING(VHCI_MAX_NR_HCS) ", default: "
+		 __MODULE_STRING(VHCI_DEFAULT_NR_HCS) ")");
+
 struct vhci *vhcis;
 
 static const char * const bit_desc[] = {
@@ -1510,8 +1516,14 @@ static int __init vhci_hcd_init(void)
 	if (usb_disabled())
 		return -ENODEV;
 
-	if (vhci_num_controllers < 1)
+	if (vhci_num_controllers < 1) {
+		pr_warn("num_controllers less than 1, setting to 1\n");
 		vhci_num_controllers = 1;
+	} else if (vhci_num_controllers > VHCI_MAX_NR_HCS) {
+		pr_warn("num_controllers too high, limiting to %d\n",
+			VHCI_MAX_NR_HCS);
+		vhci_num_controllers = VHCI_MAX_NR_HCS;
+	}
 
 	vhcis = kcalloc(vhci_num_controllers, sizeof(struct vhci), GFP_KERNEL);
 	if (vhcis == NULL)
-- 
2.43.0

[PATCH 2/3] usbip: Convert CONFIG_USBIP_VHCI_HC_PORTS to a module parameter.

[Jakub Lecki]

In workflows involving a greater number of remote
USB/IP devices, the default number of available virtual ports may be
insufficient, forcing user to recompile the module with greater number
of configured virtual host controllers and/or number of ports.

Allow a user to configure the number of ports per USB/IP virtual host
controller via a new 'hc_ports' module parameter to simplify the usage
of this module.

- Remove the USBIP_VHCI_HC_PORTS Kconfig option and replace it with a
  module parameter.
- Fix the omitted BUILD_BUG_ON check for the upper limit of
  'vhci_hc_ports' when switching from USB_MAXCHILDREN to
  USB_SS_MAXPORTS.
- Resize related arrays to match VHCI_MAX_HC_PORTS = USB_SS_MAXPORTS to
  support a dynamically configurable number of ports.
- Trim the value of the configured 'hc_ports' parameter if it
  exceeds bounds, and emit a warning.

Signed-off-by: Jakub Lecki <lec.jakub@gmail.com>
---
 drivers/usb/usbip/Kconfig      | 10 --------
 drivers/usb/usbip/vhci.h       | 22 ++++++++---------
 drivers/usb/usbip/vhci_hcd.c   | 43 +++++++++++++++++++++++-----------
 drivers/usb/usbip/vhci_sysfs.c | 18 +++++++-------
 4 files changed, 48 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/usbip/Kconfig b/drivers/usb/usbip/Kconfig
index bdcb6f4fdbec..e178867fb5ef 100644
--- a/drivers/usb/usbip/Kconfig
+++ b/drivers/usb/usbip/Kconfig
@@ -28,16 +28,6 @@ config USBIP_VHCI_HCD
 	  To compile this driver as a module, choose M here: the
 	  module will be called vhci-hcd.
 
-config USBIP_VHCI_HC_PORTS
-	int "Number of ports per USB/IP virtual host controller"
-	range 1 15
-	default 8
-	depends on USBIP_VHCI_HCD
-	help
-	  To increase number of ports available for USB/IP virtual
-	  host controller driver, this defines number of ports per
-	  USB/IP virtual host controller.
-
 config USBIP_HOST
 	tristate "Host driver"
 	depends on USBIP_CORE && USB
diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 30b8540e0b49..2772d923a8cb 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -72,15 +72,9 @@ enum hub_speed {
 	HUB_SPEED_SUPER,
 };
 
-/* Number of supported ports. Value has an upperbound of USB_MAXCHILDREN */
-#ifdef CONFIG_USBIP_VHCI_HC_PORTS
-#define VHCI_HC_PORTS CONFIG_USBIP_VHCI_HC_PORTS
-#else
-#define VHCI_HC_PORTS 8
-#endif
-
-/* Each VHCI has 2 hubs (USB2 and USB3), each has VHCI_HC_PORTS ports */
-#define VHCI_PORTS	(VHCI_HC_PORTS*2)
+/* Number of supported ports. Value has an upperbound of USB_SS_MAXPORTS */
+#define VHCI_DEFAULT_HC_PORTS 8
+#define VHCI_MAX_HC_PORTS USB_SS_MAXPORTS
 
 #define VHCI_DEFAULT_NR_HCS 1
 #define VHCI_MAX_NR_HCS 128
@@ -100,7 +94,7 @@ struct vhci {
 struct vhci_hcd {
 	struct vhci *vhci;
 
-	u32 port_status[VHCI_HC_PORTS];
+	u32 port_status[VHCI_MAX_HC_PORTS];
 
 	unsigned resuming:1;
 	unsigned long re_timeout;
@@ -112,13 +106,17 @@ struct vhci_hcd {
 	 * wIndex shows the port number and begins from 1.
 	 * But, the index of this array begins from 0.
 	 */
-	struct vhci_device vdev[VHCI_HC_PORTS];
+	struct vhci_device vdev[VHCI_MAX_HC_PORTS];
 };
 
 extern unsigned int vhci_num_controllers;
+extern unsigned int vhci_hc_ports;
 extern struct vhci *vhcis;
 extern struct attribute_group vhci_attr_group;
 
+/* Each VHCI has 2 hubs (USB2 and USB3), each has vhci_hc_ports ports */
+#define VHCI_PORTS	(vhci_hc_ports * 2)
+
 /* vhci_hcd.c */
 void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed);
 
@@ -135,7 +133,7 @@ int vhci_tx_loop(void *data);
 
 static inline __u32 port_to_rhport(__u32 port)
 {
-	return port % VHCI_HC_PORTS;
+	return port % vhci_hc_ports;
 }
 
 static inline int port_to_pdev_nr(__u32 port)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 93c3fa3e1c53..1989a6f59107 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -51,6 +51,12 @@ MODULE_PARM_DESC(num_controllers, "Number of USB/IP virtual host controllers (ra
 		 __MODULE_STRING(VHCI_MAX_NR_HCS) ", default: "
 		 __MODULE_STRING(VHCI_DEFAULT_NR_HCS) ")");
 
+unsigned int vhci_hc_ports = VHCI_DEFAULT_HC_PORTS;
+module_param_named(hc_ports, vhci_hc_ports, uint, 0444);
+MODULE_PARM_DESC(hc_ports, "Number of ports per USB/IP virtual host controller (range: 1-"
+		 __MODULE_STRING(VHCI_MAX_HC_PORTS) ", default: "
+		 __MODULE_STRING(VHCI_DEFAULT_HC_PORTS) ")");
+
 struct vhci *vhcis;
 
 static const char * const bit_desc[] = {
@@ -236,7 +242,7 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf)
 {
 	struct vhci_hcd	*vhci_hcd = hcd_to_vhci_hcd(hcd);
 	struct vhci *vhci = vhci_hcd->vhci;
-	int		retval = DIV_ROUND_UP(VHCI_HC_PORTS + 1, 8);
+	int		retval = DIV_ROUND_UP(vhci_hc_ports + 1, 8);
 	int		rhport;
 	int		changed = 0;
 	unsigned long	flags;
@@ -250,7 +256,7 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf)
 	}
 
 	/* check pseudo status register for each port */
-	for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
+	for (rhport = 0; rhport < vhci_hc_ports; rhport++) {
 		if ((vhci_hcd->port_status[rhport] & PORT_C_MASK)) {
 			/* The status of a port has been changed, */
 			usbip_dbg_vhci_rh("port %d status changed\n", rhport);
@@ -297,7 +303,7 @@ ss_hub_descriptor(struct usb_hub_descriptor *desc)
 	desc->bDescLength = 12;
 	desc->wHubCharacteristics = cpu_to_le16(
 		HUB_CHAR_INDV_PORT_LPSM | HUB_CHAR_COMMON_OCPM);
-	desc->bNbrPorts = VHCI_HC_PORTS;
+	desc->bNbrPorts = vhci_hc_ports;
 	desc->u.ss.bHubHdrDecLat = 0x04; /* Worst case: 0.4 micro sec*/
 	desc->u.ss.DeviceRemovable = 0xffff;
 }
@@ -311,8 +317,7 @@ static inline void hub_descriptor(struct usb_hub_descriptor *desc)
 	desc->wHubCharacteristics = cpu_to_le16(
 		HUB_CHAR_INDV_PORT_LPSM | HUB_CHAR_COMMON_OCPM);
 
-	desc->bNbrPorts = VHCI_HC_PORTS;
-	BUILD_BUG_ON(VHCI_HC_PORTS > USB_MAXCHILDREN);
+	desc->bNbrPorts = vhci_hc_ports;
 	width = desc->bNbrPorts / 8 + 1;
 	desc->bDescLength = USB_DT_HUB_NONVAR_SIZE + 2 * width;
 	memset(&desc->u.hs.DeviceRemovable[0], 0, width);
@@ -329,7 +334,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 	unsigned long	flags;
 	bool invalid_rhport = false;
 
-	u32 prev_port_status[VHCI_HC_PORTS];
+	u32 prev_port_status[VHCI_MAX_HC_PORTS];
 
 	if (!HCD_HW_ACCESSIBLE(hcd))
 		return -ETIMEDOUT;
@@ -344,17 +349,18 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 	/*
 	 * wIndex can be 0 for some request types (typeReq). rhport is
-	 * in valid range when wIndex >= 1 and < VHCI_HC_PORTS.
+	 * in valid range when wIndex >= 1 and < vhci_hc_ports.
 	 *
 	 * Reference port_status[] only with valid rhport when
 	 * invalid_rhport is false.
 	 */
-	if (wIndex < 1 || wIndex > VHCI_HC_PORTS) {
+	if (wIndex < 1 || wIndex > vhci_hc_ports) {
 		invalid_rhport = true;
-		if (wIndex > VHCI_HC_PORTS)
+		if (wIndex > vhci_hc_ports)
 			pr_err("invalid port number %d\n", wIndex);
-	} else
+	} else {
 		rhport = wIndex - 1;
+	}
 
 	vhci_hcd = hcd_to_vhci_hcd(hcd);
 	vhci = vhci_hcd->vhci;
@@ -707,7 +713,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
 	struct vhci_device *vdev;
 	unsigned long flags;
 
-	if (portnum > VHCI_HC_PORTS) {
+	if (portnum > vhci_hc_ports) {
 		pr_err("invalid port number %d\n", portnum);
 		return -ENODEV;
 	}
@@ -1188,7 +1194,7 @@ static int vhci_start(struct usb_hcd *hcd)
 
 	/* initialize private data of usb_hcd */
 
-	for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
+	for (rhport = 0; rhport < vhci_hc_ports; rhport++) {
 		struct vhci_device *vdev = &vhci_hcd->vdev[rhport];
 
 		vhci_device_init(vdev);
@@ -1244,7 +1250,7 @@ static void vhci_stop(struct usb_hcd *hcd)
 	}
 
 	/* 2. shutdown all the ports of vhci_hcd */
-	for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
+	for (rhport = 0; rhport < vhci_hc_ports; rhport++) {
 		struct vhci_device *vdev = &vhci_hcd->vdev[rhport];
 
 		usbip_event_add(&vdev->ud, VDEV_EVENT_REMOVED);
@@ -1441,7 +1447,7 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
 
 	spin_lock_irqsave(&vhci->lock, flags);
 
-	for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
+	for (rhport = 0; rhport < vhci_hc_ports; rhport++) {
 		if (vhci->vhci_hcd_hs->port_status[rhport] &
 		    USB_PORT_STAT_CONNECTION)
 			connected += 1;
@@ -1525,6 +1531,15 @@ static int __init vhci_hcd_init(void)
 		vhci_num_controllers = VHCI_MAX_NR_HCS;
 	}
 
+	if (vhci_hc_ports < 1) {
+		pr_warn("hc_ports less than 1, setting to 1\n");
+		vhci_hc_ports = 1;
+	} else if (vhci_hc_ports > VHCI_MAX_HC_PORTS) {
+		pr_warn("hc_ports too high, limiting to %d\n",
+			VHCI_MAX_HC_PORTS);
+		vhci_hc_ports = VHCI_MAX_HC_PORTS;
+	}
+
 	vhcis = kcalloc(vhci_num_controllers, sizeof(struct vhci), GFP_KERNEL);
 	if (vhcis == NULL)
 		return -ENOMEM;
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index d5865460e82d..19d67717180e 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -81,7 +81,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
 
 	spin_lock_irqsave(&vhci->lock, flags);
 
-	for (i = 0; i < VHCI_HC_PORTS; i++) {
+	for (i = 0; i < vhci_hc_ports; i++) {
 		struct vhci_device *vdev = &vhci->vhci_hcd_hs->vdev[i];
 
 		spin_lock(&vdev->ud.lock);
@@ -90,12 +90,12 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
 		spin_unlock(&vdev->ud.lock);
 	}
 
-	for (i = 0; i < VHCI_HC_PORTS; i++) {
+	for (i = 0; i < vhci_hc_ports; i++) {
 		struct vhci_device *vdev = &vhci->vhci_hcd_ss->vdev[i];
 
 		spin_lock(&vdev->ud.lock);
 		port_show_vhci(&out, HUB_SPEED_SUPER,
-			       pdev_nr * VHCI_PORTS + VHCI_HC_PORTS + i, vdev);
+			       pdev_nr * VHCI_PORTS + vhci_hc_ports + i, vdev);
 		spin_unlock(&vdev->ud.lock);
 	}
 
@@ -109,7 +109,7 @@ 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++) {
+	for (i = 0; i < vhci_hc_ports; i++) {
 		out += sprintf(out, "hs  %04u %03u ",
 				    (pdev_nr * VHCI_PORTS) + i,
 				    VDEV_ST_NOTASSIGNED);
@@ -117,9 +117,9 @@ static ssize_t status_show_not_ready(int pdev_nr, char *out)
 		out += sprintf(out, "\n");
 	}
 
-	for (i = 0; i < VHCI_HC_PORTS; i++) {
+	for (i = 0; i < vhci_hc_ports; i++) {
 		out += sprintf(out, "ss  %04u %03u ",
-				    (pdev_nr * VHCI_PORTS) + VHCI_HC_PORTS + i,
+				    (pdev_nr * VHCI_PORTS) + vhci_hc_ports + i,
 				    VDEV_ST_NOTASSIGNED);
 		out += sprintf(out, "000 00000000 0000000000000000 0-0");
 		out += sprintf(out, "\n");
@@ -221,11 +221,11 @@ static int valid_port(__u32 *pdev_nr, __u32 *rhport)
 	}
 	*pdev_nr = array_index_nospec(*pdev_nr, vhci_num_controllers);
 
-	if (*rhport >= VHCI_HC_PORTS) {
+	if (*rhport >= vhci_hc_ports) {
 		pr_err("rhport %u\n", *rhport);
 		return 0;
 	}
-	*rhport = array_index_nospec(*rhport, VHCI_HC_PORTS);
+	*rhport = array_index_nospec(*rhport, vhci_hc_ports);
 
 	return 1;
 }
@@ -255,7 +255,7 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr,
 
 	usbip_dbg_vhci_sysfs("rhport %d\n", rhport);
 
-	if ((port / VHCI_HC_PORTS) % 2)
+	if ((port / vhci_hc_ports) % 2)
 		vhci_hcd = hcd_to_vhci_hcd(hcd)->vhci->vhci_hcd_ss;
 	else
 		vhci_hcd = hcd_to_vhci_hcd(hcd)->vhci->vhci_hcd_hs;
-- 
2.43.0

[PATCH 3/3] usbip: Limit maximum number of virtual host controllers to 31.

[Jakub Lecki]

When loading the vhci-hcd module with number of virtual host controllers
configured to max value of 128, the module initialization fails due to
insufficient number of available IDs for USB busses.

Each virtual host controller registers two usb hubs (USB2.0 & USB3.0) to
the usb core, each with a unique bus number. The number of USB busses is
limited by ID allocation range [1 .. USB_MAXBUS - 1] (defined in
usb_register_bus()). Therefore, VHCI_MAX_NR_HCS must not be greater than
(USB_MAXBUS - 1) / 2 = 31.

In real world scenarios the maximum number of virtual host controllers
possible to create may be even lower as other USB host controllers may
be registered. In this case, the module initialization failure is
correct as the number of virtual host controllers must be adjusted by
a user to a given use-case.

Signed-off-by: Jakub Lecki <lec.jakub@gmail.com>
---
 drivers/usb/usbip/vhci.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 2772d923a8cb..3b0ea4038e51 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -76,8 +76,17 @@ enum hub_speed {
 #define VHCI_DEFAULT_HC_PORTS 8
 #define VHCI_MAX_HC_PORTS USB_SS_MAXPORTS
 
+/*
+ * Number of supported virtual host controllers. Value has upperbound of
+ * maximum possible usb busses.
+ * It is limited by a bus ID allocation in [1 .. USB_MAXBUS - 1] range,
+ * resulting in maximum of USB_MAXBUS - 1 usb busses allocated.
+ * Additionally, each virtual host controller registers 2 usb hubs (USB2.0
+ * & USB3.0), therefore maximum number of virtual host controllers is:
+ * (USB_MAXBUS - 1) / 2
+ */
 #define VHCI_DEFAULT_NR_HCS 1
-#define VHCI_MAX_NR_HCS 128
+#define VHCI_MAX_NR_HCS 31
 
 #define MAX_STATUS_NAME 16
 
-- 
2.43.0

Re: [PATCH 1/3] usbip: Convert CONFIG_USBIP_VHCI_NR_HCS to a module parameter.

[Greg Kroah-Hartman]

On Tue, Nov 04, 2025 at 12:32:46PM +0100, Jakub Lecki wrote:
> In workflows involving a greater number of remote
> USB/IP devices, the default number of available virtual ports may be
> insufficient, forcing user to recompile the module with greater number
> of configured virtual host controllers and/or number of ports.
> 
> Allow a user to configure the number of USB/IP virtual host controllers
> via a new 'num_controllers' module parameter to simplify the usage of
> this module.
> 
> VHCI controller structures are already dynamically allocated during
> module initialization, so the only change is switch from assigning
> 'vhci_num_controllers' via Kconfig to using the module parameter
> framework.
> 
> - Remove the USBIP_VHCI_NR_HCS Kconfig option and replace it with a
>   module parameter.
> - Trim the value of the configured 'num_controllers' parameter if it
>   exceeds bounds, and emit a warning.
> 
> Signed-off-by: Jakub Lecki <lec.jakub@gmail.com>
> ---
>  drivers/usb/usbip/Kconfig    | 11 -----------
>  drivers/usb/usbip/vhci.h     |  9 +++------
>  drivers/usb/usbip/vhci_hcd.c | 16 ++++++++++++++--
>  3 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/usb/usbip/Kconfig b/drivers/usb/usbip/Kconfig
> index b9f94e2e278d..bdcb6f4fdbec 100644
> --- a/drivers/usb/usbip/Kconfig
> +++ b/drivers/usb/usbip/Kconfig
> @@ -38,17 +38,6 @@ config USBIP_VHCI_HC_PORTS
>  	  host controller driver, this defines number of ports per
>  	  USB/IP virtual host controller.
>  
> -config USBIP_VHCI_NR_HCS
> -	int "Number of USB/IP virtual host controllers"
> -	range 1 128
> -	default 1
> -	depends on USBIP_VHCI_HCD
> -	help
> -	  To increase number of ports available for USB/IP virtual
> -	  host controller driver, this defines number of USB/IP
> -	  virtual host controllers as if adding physical host
> -	  controllers.
> -
>  config USBIP_HOST
>  	tristate "Host driver"
>  	depends on USBIP_CORE && USB
> diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
> index 5659dce1526e..30b8540e0b49 100644
> --- a/drivers/usb/usbip/vhci.h
> +++ b/drivers/usb/usbip/vhci.h
> @@ -82,11 +82,8 @@ enum hub_speed {
>  /* Each VHCI has 2 hubs (USB2 and USB3), each has VHCI_HC_PORTS ports */
>  #define VHCI_PORTS	(VHCI_HC_PORTS*2)
>  
> -#ifdef CONFIG_USBIP_VHCI_NR_HCS
> -#define VHCI_NR_HCS CONFIG_USBIP_VHCI_NR_HCS
> -#else
> -#define VHCI_NR_HCS 1
> -#endif
> +#define VHCI_DEFAULT_NR_HCS 1
> +#define VHCI_MAX_NR_HCS 128
>  
>  #define MAX_STATUS_NAME 16
>  
> @@ -118,7 +115,7 @@ struct vhci_hcd {
>  	struct vhci_device vdev[VHCI_HC_PORTS];
>  };
>  
> -extern int vhci_num_controllers;
> +extern unsigned int vhci_num_controllers;
>  extern struct vhci *vhcis;
>  extern struct attribute_group vhci_attr_group;
>  
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index e70fba9f55d6..93c3fa3e1c53 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -10,6 +10,7 @@
>  #include <linux/kthread.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/printk.h>
>  #include <linux/slab.h>
>  #include <linux/string_choices.h>
>  
> @@ -44,7 +45,12 @@ static int vhci_get_frame_number(struct usb_hcd *hcd);
>  static const char driver_name[] = "vhci_hcd";
>  static const char driver_desc[] = "USB/IP Virtual Host Controller";
>  
> -int vhci_num_controllers = VHCI_NR_HCS;
> +unsigned int vhci_num_controllers = VHCI_DEFAULT_NR_HCS;
> +module_param_named(num_controllers, vhci_num_controllers, uint, 0444);
> +MODULE_PARM_DESC(num_controllers, "Number of USB/IP virtual host controllers (range: 0-"
> +		 __MODULE_STRING(VHCI_MAX_NR_HCS) ", default: "
> +		 __MODULE_STRING(VHCI_DEFAULT_NR_HCS) ")");

I'm all for making this dynamic, but this is not the 1990's, please do
not add new module parameters.  Use the "proper" api for this, either
sysfs or configfs, instead.

thanks,

greg k-h

Re: [PATCH 3/3] usbip: Limit maximum number of virtual host controllers to 31.

[Greg Kroah-Hartman]

On Tue, Nov 04, 2025 at 12:32:48PM +0100, Jakub Lecki wrote:
> When loading the vhci-hcd module with number of virtual host controllers
> configured to max value of 128, the module initialization fails due to
> insufficient number of available IDs for USB busses.
> 
> Each virtual host controller registers two usb hubs (USB2.0 & USB3.0) to
> the usb core, each with a unique bus number. The number of USB busses is
> limited by ID allocation range [1 .. USB_MAXBUS - 1] (defined in
> usb_register_bus()). Therefore, VHCI_MAX_NR_HCS must not be greater than
> (USB_MAXBUS - 1) / 2 = 31.
> 
> In real world scenarios the maximum number of virtual host controllers
> possible to create may be even lower as other USB host controllers may
> be registered. In this case, the module initialization failure is
> correct as the number of virtual host controllers must be adjusted by
> a user to a given use-case.
> 
> Signed-off-by: Jakub Lecki <lec.jakub@gmail.com>
> ---
>  drivers/usb/usbip/vhci.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
> index 2772d923a8cb..3b0ea4038e51 100644
> --- a/drivers/usb/usbip/vhci.h
> +++ b/drivers/usb/usbip/vhci.h
> @@ -76,8 +76,17 @@ enum hub_speed {
>  #define VHCI_DEFAULT_HC_PORTS 8
>  #define VHCI_MAX_HC_PORTS USB_SS_MAXPORTS
>  
> +/*
> + * Number of supported virtual host controllers. Value has upperbound of
> + * maximum possible usb busses.
> + * It is limited by a bus ID allocation in [1 .. USB_MAXBUS - 1] range,
> + * resulting in maximum of USB_MAXBUS - 1 usb busses allocated.
> + * Additionally, each virtual host controller registers 2 usb hubs (USB2.0
> + * & USB3.0), therefore maximum number of virtual host controllers is:
> + * (USB_MAXBUS - 1) / 2
> + */
>  #define VHCI_DEFAULT_NR_HCS 1
> -#define VHCI_MAX_NR_HCS 128
> +#define VHCI_MAX_NR_HCS 31

Why have any max at all?  Why not just dynamically allocate them when
asked for?

thanks,

greg k-h