[PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Zongmin Zhou]

From: Zongmin Zhou <zhouzongmin@kylinos.cn>

This patch fixes an issue that usb device through usbip protocol,
the max_hw_sectors will be limited to 512,and then
read/write rate of high speed USB devices will be limited.

After the commit d74ffae8b8dd ("usb-storage: Add a limitation
for blk_queue_max_hw_sectors()") is applied,
This issue happened on the swiotlb enabled environment.
This commit will checks the maximum size of a mapping for the device,
and adjusts the max_hw_sectors.On vhci-hcd driver,
the dma mask setting follows the platform device default setting(32-bit).
So dma_addressing_limited() will be true,then the maximum mapping size
use the swiotlb max mapping size(512).The max_hw_sectors reset to 512.

To fix this issue,have to get the dma mask bit that
the real USB controllers support,and set this value on vhci-hcd driver,
usbip device will get the correct max_hw_sectors.

Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
 drivers/usb/usbip/stub_dev.c               | 42 ++++++++++++++++++++++
 drivers/usb/usbip/vhci_sysfs.c             | 11 +++---
 tools/usb/usbip/libsrc/usbip_common.h      |  1 +
 tools/usb/usbip/libsrc/usbip_host_common.c | 34 ++++++++++++++++++
 tools/usb/usbip/libsrc/vhci_driver.c       | 10 +++---
 tools/usb/usbip/libsrc/vhci_driver.h       |  4 +--
 tools/usb/usbip/src/usbip_attach.c         |  2 +-
 tools/usb/usbip/src/usbip_network.c        |  1 +
 8 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index ce625b1ce9a5..095de5a8a307 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -7,6 +7,7 @@
 #include <linux/file.h>
 #include <linux/kthread.h>
 #include <linux/module.h>
+#include <linux/dma-mapping.h>
 
 #include "usbip_common.h"
 #include "stub.h"
@@ -34,6 +35,46 @@ static ssize_t usbip_status_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(usbip_status);
 
+/*
+ * The real USB controllers may support larger than 32-bit address memory pointers actually.
+ * But vhci-hcd driver use the default platform device dma mask set(32-bit),
+ * and usbip device's max_sectors will be limited by dma max mapping size.
+ * usbip_dma_bits shows the real dma mask bit of the real usb controller
+ * which usbip device is bound to.
+ */
+static unsigned int mask_convert_to_bits(u64 dma_mask)
+{
+	unsigned int bits = 0;
+
+	while (dma_mask & 0x1) {
+		dma_mask >>= 1;
+		bits++;
+	}
+	return bits;
+}
+
+static ssize_t usbip_dma_bits_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct stub_device *sdev = dev_get_drvdata(dev);
+	struct device *sys_dev = sdev->udev->bus->sysdev;
+	u64 dma_mask = 0;
+	unsigned int dma_bits = 0;
+
+	if (!sdev || !sys_dev) {
+		dev_err(dev, "sdev or sys_dev is null\n");
+		return -ENODEV;
+	}
+
+	spin_lock_irq(&sdev->ud.lock);
+	dma_mask = dma_get_mask(sys_dev);
+	dma_bits = mask_convert_to_bits(dma_mask);
+	spin_unlock_irq(&sdev->ud.lock);
+
+	return sysfs_emit(buf, "%d\n", dma_bits);
+}
+static DEVICE_ATTR_RO(usbip_dma_bits);
+
 /*
  * usbip_sockfd gets a socket descriptor of an established TCP connection that
  * is used to transfer usbip requests by kernel threads. -1 is a magic number
@@ -144,6 +185,7 @@ static DEVICE_ATTR_WO(usbip_sockfd);
 
 static struct attribute *usbip_attrs[] = {
 	&dev_attr_usbip_status.attr,
+	&dev_attr_usbip_dma_bits.attr,
 	&dev_attr_usbip_sockfd.attr,
 	&dev_attr_usbip_debug.attr,
 	NULL,
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index d5865460e82d..972beac653f6 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -12,6 +12,7 @@
 
 /* Hardening for Spectre-v1 */
 #include <linux/nospec.h>
+#include <linux/dma-mapping.h>
 
 #include "usbip_common.h"
 #include "vhci.h"
@@ -311,7 +312,7 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
 {
 	struct socket *socket;
 	int sockfd = 0;
-	__u32 port = 0, pdev_nr = 0, rhport = 0, devid = 0, speed = 0;
+	__u32 port = 0, pdev_nr = 0, rhport = 0, devid = 0, speed = 0, dma_bits = 0;
 	struct usb_hcd *hcd;
 	struct vhci_hcd *vhci_hcd;
 	struct vhci_device *vdev;
@@ -327,15 +328,15 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
 	 * @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)
+	if (sscanf(buf, "%u %u %u %u %u", &port, &sockfd, &devid, &speed, &dma_bits) != 5)
 		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("sockfd(%u) devid(%u) speed(%u) dma_bits(%u)\n",
+			     sockfd, devid, speed, dma_bits);
 
 	/* check received parameters */
 	if (!valid_args(&pdev_nr, &rhport, speed))
@@ -346,6 +347,8 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
 		dev_err(dev, "port %d is not ready\n", port);
 		return -EAGAIN;
 	}
+	/* Set the real USB controller's dma mask to vhci-hcd driver. */
+	dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
 
 	vhci_hcd = hcd_to_vhci_hcd(hcd);
 	vhci = vhci_hcd->vhci;
diff --git a/tools/usb/usbip/libsrc/usbip_common.h b/tools/usb/usbip/libsrc/usbip_common.h
index 73a367a7fa10..c87d74917c2a 100644
--- a/tools/usb/usbip/libsrc/usbip_common.h
+++ b/tools/usb/usbip/libsrc/usbip_common.h
@@ -115,6 +115,7 @@ struct usbip_usb_device {
 	uint32_t busnum;
 	uint32_t devnum;
 	uint32_t speed;
+	uint32_t dma_bits;
 
 	uint16_t idVendor;
 	uint16_t idProduct;
diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
index ca78aa368476..7f5cc44236c9 100644
--- a/tools/usb/usbip/libsrc/usbip_host_common.c
+++ b/tools/usb/usbip/libsrc/usbip_host_common.c
@@ -61,6 +61,38 @@ static int32_t read_attr_usbip_status(struct usbip_usb_device *udev)
 	return value;
 }
 
+static int32_t read_attr_usbip_dma_bits(struct usbip_usb_device *udev)
+{
+	char dma_bits_attr_path[SYSFS_PATH_MAX];
+	int size;
+	int fd;
+	int length;
+	char dma_bits[3] = { 0 };
+
+	size = snprintf(dma_bits_attr_path, sizeof(dma_bits_attr_path),
+			"%s/usbip_dma_bits", udev->path);
+	if (size < 0 || (unsigned int)size >= sizeof(dma_bits_attr_path)) {
+		err("usbip_dma_bits path length %i >= %lu or < 0", size,
+		    (unsigned long)sizeof(dma_bits_attr_path));
+		return -1;
+	}
+
+	fd = open(dma_bits_attr_path, O_RDONLY);
+	if (fd < 0) {
+		err("error opening attribute %s", dma_bits_attr_path);
+		return -1;
+	}
+	length = read(fd, dma_bits, 2);
+	if (length < 0) {
+		err("error reading attribute %s", dma_bits_attr_path);
+		close(fd);
+		return -1;
+	}
+	udev->dma_bits = atoi(dma_bits);
+	close(fd);
+	return 0;
+}
+
 static
 struct usbip_exported_device *usbip_exported_device_new(
 		struct usbip_host_driver *hdriver, const char *sdevpath)
@@ -82,6 +114,8 @@ struct usbip_exported_device *usbip_exported_device_new(
 	if (hdriver->ops.read_device(edev->sudev, &edev->udev) < 0)
 		goto err;
 
+	read_attr_usbip_dma_bits(&edev->udev);
+
 	edev->status = read_attr_usbip_status(&edev->udev);
 	if (edev->status < 0)
 		goto err;
diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c
index 8159fd98680b..e5fc4a048a5f 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.c
+++ b/tools/usb/usbip/libsrc/vhci_driver.c
@@ -355,15 +355,15 @@ int usbip_vhci_get_free_port(uint32_t speed)
 }
 
 int usbip_vhci_attach_device2(uint8_t port, int sockfd, uint32_t devid,
-		uint32_t speed) {
+		uint32_t speed, uint32_t dma_bits) {
 	char buff[200]; /* what size should be ? */
 	char attach_attr_path[SYSFS_PATH_MAX];
 	char attr_attach[] = "attach";
 	const char *path;
 	int ret;
 
-	snprintf(buff, sizeof(buff), "%u %d %u %u",
-			port, sockfd, devid, speed);
+	snprintf(buff, sizeof(buff), "%u %d %u %u %u",
+			port, sockfd, devid, speed, dma_bits);
 	dbg("writing: %s", buff);
 
 	path = udev_device_get_syspath(vhci_driver->hc_device);
@@ -389,11 +389,11 @@ static unsigned long get_devid(uint8_t busnum, uint8_t devnum)
 
 /* will be removed */
 int usbip_vhci_attach_device(uint8_t port, int sockfd, uint8_t busnum,
-		uint8_t devnum, uint32_t speed)
+		uint8_t devnum, uint32_t speed, uint32_t dma_bits)
 {
 	int devid = get_devid(busnum, devnum);
 
-	return usbip_vhci_attach_device2(port, sockfd, devid, speed);
+	return usbip_vhci_attach_device2(port, sockfd, devid, speed, dma_bits);
 }
 
 int usbip_vhci_detach_device(uint8_t port)
diff --git a/tools/usb/usbip/libsrc/vhci_driver.h b/tools/usb/usbip/libsrc/vhci_driver.h
index 6c9aca216705..bf40aa23b6f3 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.h
+++ b/tools/usb/usbip/libsrc/vhci_driver.h
@@ -54,11 +54,11 @@ int  usbip_vhci_refresh_device_list(void);
 
 int usbip_vhci_get_free_port(uint32_t speed);
 int usbip_vhci_attach_device2(uint8_t port, int sockfd, uint32_t devid,
-		uint32_t speed);
+		uint32_t speed, uint32_t dma_bits);
 
 /* will be removed */
 int usbip_vhci_attach_device(uint8_t port, int sockfd, uint8_t busnum,
-		uint8_t devnum, uint32_t speed);
+		uint8_t devnum, uint32_t speed, uint32_t dma_bits);
 
 int usbip_vhci_detach_device(uint8_t port);
 
diff --git a/tools/usb/usbip/src/usbip_attach.c b/tools/usb/usbip/src/usbip_attach.c
index 531a415538f9..453987782eec 100644
--- a/tools/usb/usbip/src/usbip_attach.c
+++ b/tools/usb/usbip/src/usbip_attach.c
@@ -100,7 +100,7 @@ static int import_device(int sockfd, struct usbip_usb_device *udev)
 		dbg("got free port %d", port);
 
 		rc = usbip_vhci_attach_device(port, sockfd, udev->busnum,
-					      udev->devnum, udev->speed);
+					      udev->devnum, udev->speed, udev->dma_bits);
 		if (rc < 0 && errno != EBUSY) {
 			err("import device");
 			goto err_driver_close;
diff --git a/tools/usb/usbip/src/usbip_network.c b/tools/usb/usbip/src/usbip_network.c
index ed4dc8c14269..2c97db9ea567 100644
--- a/tools/usb/usbip/src/usbip_network.c
+++ b/tools/usb/usbip/src/usbip_network.c
@@ -79,6 +79,7 @@ void usbip_net_pack_usb_device(int pack, struct usbip_usb_device *udev)
 	udev->busnum = usbip_net_pack_uint32_t(pack, udev->busnum);
 	udev->devnum = usbip_net_pack_uint32_t(pack, udev->devnum);
 	udev->speed = usbip_net_pack_uint32_t(pack, udev->speed);
+	udev->dma_bits = usbip_net_pack_uint32_t(pack, udev->dma_bits);
 
 	udev->idVendor = usbip_net_pack_uint16_t(pack, udev->idVendor);
 	udev->idProduct = usbip_net_pack_uint16_t(pack, udev->idProduct);
-- 
2.34.1

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Shuah Khan]

On 2/19/25 02:25, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> This patch fixes an issue that usb device through usbip protocol,
> the max_hw_sectors will be limited to 512,and then
> read/write rate of high speed USB devices will be limited.
> 
> After the commit d74ffae8b8dd ("usb-storage: Add a limitation
> for blk_queue_max_hw_sectors()") is applied,
> This issue happened on the swiotlb enabled environment.
> This commit will checks the maximum size of a mapping for the device,
> and adjusts the max_hw_sectors.On vhci-hcd driver,
> the dma mask setting follows the platform device default setting(32-bit).
> So dma_addressing_limited() will be true,then the maximum mapping size
> use the swiotlb max mapping size(512).The max_hw_sectors reset to 512.
> 
> To fix this issue,have to get the dma mask bit that
> the real USB controllers support,and set this value on vhci-hcd driver,
> usbip device will get the correct max_hw_sectors.

I don't have objections to this change. This does change
the API between the kernel and user-space and requires more
testing with mispatched kernel and user-space.

Can you give me more details on testing you have done with
the revision mismatches environments?

Also this would require bumping up the version on kernel and
user-space sides.

thanks,
-- Shuah

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Shuah Khan]

On 2/24/25 01:36, Zongmin Zhou wrote:
> Dear Shuah,
> 
> 
> Yes，the usbip server get the real USB controller message send to usbip client vhci-hcd，
> 
> must have to changethe API between the kernel and user-space.
> 
> The easiest way is to simply set vhci-hcd dma mask to 64 by default,
> 
> but not all USB controllers support 64bit，That doesn't look good？

This is an expetnsive change to fix the problem. In addition this change
is unnecessary for non-storage devices where USB over IP is used.
You mentioned this happens only in swiotlb cases? Can you explain the
configuration on host and client side.

I would like to explore fixes that are simpler than what is proposed
in this patch.

thanks,
-- Shuah

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Shuah Khan]

On 3/2/25 05:37, Zongmin Zhou wrote:
> Dear shuah,
> 
> 
> Yes, I agree with you.It would be better if there have a more simpler fixes than This patch.
> 
> I can just think of the two possible solutions that mentioned before.

What are the two possible solutions?
> 
> 
> If SWIOTLB disabled,dma_max_mapping_size() return SIZE_MAX.

Right when CONFIG_HAS_DMA, if not it returns 0. Perhaps we
can ignore CONFIG_HAS_DMA=n for this for this discussion.

> 
> Only if SWIOTLB is active and dma addressing limited will return the swiotlb max mapping size.
> 
> 
> The swiotlb config seems rely on many other config options like x86_64/IOMMU_SUPPORT and so on,
> 
> and the configuration on host and client side only use the default at all,Like the default ubuntu release version.
> 
> It seems that switlb is enabled by default on most platforms.

If understand correctly the problem happens only when SWIOTLB
is enabled on client or host?

The following combinations are possible:

SWILTLB enabled on client and disabled on host - rate limited?
SWILTLB enabled on client and enabled on host - rate limited?
SWILTLB disabled on client and enabled on host - rate limited?
SWILTLB disabled on client and disabled on host - not a problem

thanks,
-- Shuah

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Shuah Khan]

On 3/5/25 03:03, Zongmin Zhou wrote:
> At 2025-03-05 03:45:28, "Shuah Khan" <skhan@linuxfoundation.org> wrote:
> 
>>On 3/2/25 05:37, Zongmin Zhou wrote:
>>> Dear shuah,
>>> 
>>> 
>>> Yes, I agree with you.It would be better if there have a more simpler fixes than This patch.
>>> 
>>> I can just think of the two possible solutions that mentioned before.
>>
>  >What are the two possible solutions?
> 1. The patch we are discussing now,have to change the API between the kernel and user-space.

2. Simply set vhci-hcd dma mask to 64 by default,just modify the vhci-hcd driver. Then dma_max_mapping_size() will always return SIZE_MAX.

I prefer option #2 - What are the downsides if any with this option?

>>> 
>>> 
>>> If SWIOTLB disabled,dma_max_mapping_size() return SIZE_MAX.
>>
>>Right when CONFIG_HAS_DMA, if not it returns 0. Perhaps we
>  >can ignore CONFIG_HAS_DMA=n for this for this discussion.
> Yeah, let's ignore that.
>>
>>> 
>>> Only if SWIOTLB is active and dma addressing limited will return the swiotlb max mapping size.
>>> 
>>> 
>>> The swiotlb config seems rely on many other config options like x86_64/IOMMU_SUPPORT and so on,
>>> 
>>> and the configuration on host and client side only use the default at all,Like the default ubuntu release version.
>>> 
>>> It seems that switlb is enabled by default on most platforms.
>>
>>If understand correctly the problem happens only when SWIOTLB
>>is enabled on client or host?
>>
>>The following combinations are possible:
>>
>>SWILTLB enabled on client and disabled on host - rate limited?
>>SWILTLB enabled on client and enabled on host - rate limited?
>>SWILTLB disabled on client and enabled on host - rate limited?
>>SWILTLB disabled on client and disabled on host - not a problem
>  >
> This problem happens only when SWIOTLB is enabled on client,have nothing to do with host setting. Because the USB xhci controller may set dma mask to 64bit if controllers support. The combinations may like below: SWILTLB enabled on client and enabled/disabled on host - rate limited SWILTLB disabled on client and enabled/disabled on host - not a problem

Got it. So the problem happens only when SWILTLB enabled on client

thanks,
-- Shuah

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Zongmin Zhou]

On 2025/3/11 00:49, Shuah Khan wrote:
> On 3/5/25 03:03, Zongmin Zhou wrote:
>> At 2025-03-05 03:45:28, "Shuah Khan" <skhan@linuxfoundation.org> wrote:
>>
>>> On 3/2/25 05:37, Zongmin Zhou wrote:
>>>> Dear shuah,
>>>>
>>>>
>>>> Yes, I agree with you.It would be better if there have a more 
>>>> simpler fixes than This patch.
>>>>
>>>> I can just think of the two possible solutions that mentioned before.
>>>
>>  >What are the two possible solutions?
>> 1. The patch we are discussing now,have to change the API between the 
>> kernel and user-space.
>
> 2. Simply set vhci-hcd dma mask to 64 by default,just modify the 
> vhci-hcd driver. Then dma_max_mapping_size() will always return SIZE_MAX.
>
> I prefer option #2 - What are the downsides if any with this option?
>
If set vhci-hcd dma mask to 64 by default,I can't predict what will 
happen when the real USB controller support less than 64bit?

After all, the data flows from vhci-hcd to usbip-host and finally to the 
USB controller to which the device is actually connected.

the data is ultimately processed through the real USB controller?

However, the default setting to 64-bit is equivalent to eliminating the 
impact of

the patch(commit d74ffae8b8dd) on usbip protocol devices, sounds feasible?

I am not very professional in this field, waiting for your evaluation.

>>>>
>>>>
>>>> If SWIOTLB disabled,dma_max_mapping_size() return SIZE_MAX.
>>>
>>> Right when CONFIG_HAS_DMA, if not it returns 0. Perhaps we
>>  >can ignore CONFIG_HAS_DMA=n for this for this discussion.
>> Yeah, let's ignore that.
>>>
>>>>
>>>> Only if SWIOTLB is active and dma addressing limited will return 
>>>> the swiotlb max mapping size.
>>>>
>>>>
>>>> The swiotlb config seems rely on many other config options like 
>>>> x86_64/IOMMU_SUPPORT and so on,
>>>>
>>>> and the configuration on host and client side only use the default 
>>>> at all,Like the default ubuntu release version.
>>>>
>>>> It seems that switlb is enabled by default on most platforms.
>>>
>>> If understand correctly the problem happens only when SWIOTLB
>>> is enabled on client or host?
>>>
>>> The following combinations are possible:
>>>
>>> SWILTLB enabled on client and disabled on host - rate limited?
>>> SWILTLB enabled on client and enabled on host - rate limited?
>>> SWILTLB disabled on client and enabled on host - rate limited?
>>> SWILTLB disabled on client and disabled on host - not a problem
>>  >
>> This problem happens only when SWIOTLB is enabled on client,have 
>> nothing to do with host setting. Because the USB xhci controller may 
>> set dma mask to 64bit if controllers support. The combinations may 
>> like below: SWILTLB enabled on client and enabled/disabled on host - 
>> rate limited SWILTLB disabled on client and enabled/disabled on host 
>> - not a problem
>
> Got it. So the problem happens only when SWILTLB enabled on client
>
Yes.
> thanks,
> -- Shuah

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Shuah Khan]

On 3/13/25 04:02, Zongmin Zhou wrote:
> 
> On 2025/3/11 00:49, Shuah Khan wrote:
>> On 3/5/25 03:03, Zongmin Zhou wrote:
>>> At 2025-03-05 03:45:28, "Shuah Khan" <skhan@linuxfoundation.org> wrote:
>>>
>>>> On 3/2/25 05:37, Zongmin Zhou wrote:
>>>>> Dear shuah,
>>>>>
>>>>>
>>>>> Yes, I agree with you.It would be better if there have a more simpler fixes than This patch.
>>>>>
>>>>> I can just think of the two possible solutions that mentioned before.
>>>>
>>>  >What are the two possible solutions?
>>> 1. The patch we are discussing now,have to change the API between the kernel and user-space.
>>
>> 2. Simply set vhci-hcd dma mask to 64 by default,just modify the vhci-hcd driver. Then dma_max_mapping_size() will always return SIZE_MAX.
>>
>> I prefer option #2 - What are the downsides if any with this option?
>>
> If set vhci-hcd dma mask to 64 by default,I can't predict what will happen when the real USB controller support less than 64bit?
> 
> After all, the data flows from vhci-hcd to usbip-host and finally to the USB controller to which the device is actually connected.
> 
> the data is ultimately processed through the real USB controller?

Sorry for the delay.

That is the case. I have to check the code to see what the host
would do if it receives larger buffers from the client (vhci)
> 
> However, the default setting to 64-bit is equivalent to eliminating the impact of
> 
> the patch(commit d74ffae8b8dd) on usbip protocol devices, sounds feasible?
> 
> I am not very professional in this field, waiting for your evaluation.

We can give this a try. Send me the patch with default testing the
following cases:

Host - swiotlb enabled and disabled in your environment to see what
happens when there is a mismatch swiotlb enabled case and client
side doesn't limit the size.

thanks,
-- Shuah

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Zongmin Zhou]

On 2025/3/29 05:14, Shuah Khan wrote:
> On 3/13/25 04:02, Zongmin Zhou wrote:
>>
>> On 2025/3/11 00:49, Shuah Khan wrote:
>>> On 3/5/25 03:03, Zongmin Zhou wrote:
>>>> At 2025-03-05 03:45:28, "Shuah Khan" <skhan@linuxfoundation.org> 
>>>> wrote:
>>>>
>>>>> On 3/2/25 05:37, Zongmin Zhou wrote:
>>>>>> Dear shuah,
>>>>>>
>>>>>>
>>>>>> Yes, I agree with you.It would be better if there have a more 
>>>>>> simpler fixes than This patch.
>>>>>>
>>>>>> I can just think of the two possible solutions that mentioned 
>>>>>> before.
>>>>>
>>>>  >What are the two possible solutions?
>>>> 1. The patch we are discussing now,have to change the API between 
>>>> the kernel and user-space.
>>>
>>> 2. Simply set vhci-hcd dma mask to 64 by default,just modify the 
>>> vhci-hcd driver. Then dma_max_mapping_size() will always return 
>>> SIZE_MAX.
>>>
>>> I prefer option #2 - What are the downsides if any with this option?
>>>
>> If set vhci-hcd dma mask to 64 by default,I can't predict what will 
>> happen when the real USB controller support less than 64bit?
>>
>> After all, the data flows from vhci-hcd to usbip-host and finally to 
>> the USB controller to which the device is actually connected.
>>
>> the data is ultimately processed through the real USB controller?
>
> Sorry for the delay.
>
> That is the case. I have to check the code to see what the host
> would do if it receives larger buffers from the client (vhci)
>>
>> However, the default setting to 64-bit is equivalent to eliminating 
>> the impact of
>>
>> the patch(commit d74ffae8b8dd) on usbip protocol devices, sounds 
>> feasible?
>>
>> I am not very professional in this field, waiting for your evaluation.
>
> We can give this a try. Send me the patch with default testing the
> following cases:
>
> Host - swiotlb enabled and disabled in your environment to see what
> happens when there is a mismatch swiotlb enabled case and client
> side doesn't limit the size.

If you want to test swiotlb disabled mode, you can modify the kernel cmd 
to force disable swiotlb:

modify the grub.cfg, add the swiotlb=noforce parameter to kernel command 
line,and reboot.

cat /proc/cmdline to check whether modified successfully.


The patch set vhci-hcd dma mask to 64 by default like below:

---
  drivers/usb/usbip/vhci_hcd.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index e70fba9f55d6..fca3a4a6e94d 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1345,6 +1345,9 @@ static int vhci_hcd_probe(struct platform_device 
*pdev)

      usbip_dbg_vhci_hc("name %s id %d\n", pdev->name, pdev->id);

+    /* Set the dma mask to support 64bit for vhci-hcd driver. */
+    dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+
      /*
       * Allocate and initialize hcd.
       * Our private data is also allocated automatically.
-- 
2.34.1

>
> thanks,
> -- Shuah

Re: [PATCH] usbip: Fix the error limitation on max_hw_sectors for usbip device

[Shuah Khan]

On 4/2/25 02:34, Zongmin Zhou wrote:
> 
> On 2025/3/29 05:14, Shuah Khan wrote:
>> On 3/13/25 04:02, Zongmin Zhou wrote:
>>>
>>> On 2025/3/11 00:49, Shuah Khan wrote:
>>>> On 3/5/25 03:03, Zongmin Zhou wrote:
>>>>> At 2025-03-05 03:45:28, "Shuah Khan" <skhan@linuxfoundation.org> wrote:
>>>>>
>>>>>> On 3/2/25 05:37, Zongmin Zhou wrote:
>>>>>>> Dear shuah,
>>>>>>>
>>>>>>>
>>>>>>> Yes, I agree with you.It would be better if there have a more simpler fixes than This patch.
>>>>>>>
>>>>>>> I can just think of the two possible solutions that mentioned before.
>>>>>>
>>>>>  >What are the two possible solutions?
>>>>> 1. The patch we are discussing now,have to change the API between the kernel and user-space.
>>>>
>>>> 2. Simply set vhci-hcd dma mask to 64 by default,just modify the vhci-hcd driver. Then dma_max_mapping_size() will always return SIZE_MAX.
>>>>
>>>> I prefer option #2 - What are the downsides if any with this option?
>>>>
>>> If set vhci-hcd dma mask to 64 by default,I can't predict what will happen when the real USB controller support less than 64bit?
>>>
>>> After all, the data flows from vhci-hcd to usbip-host and finally to the USB controller to which the device is actually connected.
>>>
>>> the data is ultimately processed through the real USB controller?
>>
>> Sorry for the delay.
>>
>> That is the case. I have to check the code to see what the host
>> would do if it receives larger buffers from the client (vhci)
>>>
>>> However, the default setting to 64-bit is equivalent to eliminating the impact of
>>>
>>> the patch(commit d74ffae8b8dd) on usbip protocol devices, sounds feasible?
>>>
>>> I am not very professional in this field, waiting for your evaluation.
>>
>> We can give this a try. Send me the patch with default testing the
>> following cases:
>>
>> Host - swiotlb enabled and disabled in your environment to see what
>> happens when there is a mismatch swiotlb enabled case and client
>> side doesn't limit the size.
> 
> If you want to test swiotlb disabled mode, you can modify the kernel cmd to force disable swiotlb:
> 
> modify the grub.cfg, add the swiotlb=noforce parameter to kernel command line,and reboot.
> 
> cat /proc/cmdline to check whether modified successfully.
> 
> 
> The patch set vhci-hcd dma mask to 64 by default like below:
> 
> ---
>   drivers/usb/usbip/vhci_hcd.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index e70fba9f55d6..fca3a4a6e94d 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1345,6 +1345,9 @@ static int vhci_hcd_probe(struct platform_device *pdev)
> 
>       usbip_dbg_vhci_hc("name %s id %d\n", pdev->name, pdev->id);
> 
> +    /* Set the dma mask to support 64bit for vhci-hcd driver. */
> +    dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> +
>       /*
>        * Allocate and initialize hcd.
>        * Our private data is also allocated automatically.


Please send me a patch I can apply.

thanks,
-- Shuah

[PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Zongmin Zhou]

From: Zongmin Zhou <zhouzongmin@kylinos.cn>

fix usb device limited  max_sectors when use usbip protocol

Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
 drivers/usb/usbip/vhci_hcd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index e70fba9f55d6..fca3a4a6e94d 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1345,6 +1345,9 @@ static int vhci_hcd_probe(struct platform_device *pdev)
 
 	usbip_dbg_vhci_hc("name %s id %d\n", pdev->name, pdev->id);
 
+	/* Set the dma mask to support 64bit for vhci-hcd driver. */
+	dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+
 	/*
 	 * Allocate and initialize hcd.
 	 * Our private data is also allocated automatically.
-- 
2.34.1

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Greg KH]

On Tue, Apr 22, 2025 at 02:34:09PM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> fix usb device limited  max_sectors when use usbip protocol

Please explain more as to why this is needed.

> 
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>

What commit id does this fix?

thanks,

greg k-h

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Christoph Hellwig]

On Tue, Apr 22, 2025 at 02:34:09PM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> fix usb device limited  max_sectors when use usbip protocol

As far as I can tell this is a virtual HCI and can't perform DMA.
Thus nothing should use DMA API calls on it and the mask is irrelevant.

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[kernel test robot]

Hi Zongmin,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.15-rc3 next-20250422]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zongmin-Zhou/usbip-set-the-dma-mask-to-64bit-default-for-vhci-driver/20250422-143646
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250422063409.607859-1-min_halo%40163.com
patch subject: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver
config: arm-randconfig-004-20250423 (https://download.01.org/0day-ci/archive/20250423/202504230805.MkmxQyas-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250423/202504230805.MkmxQyas-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504230805.MkmxQyas-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/usbip/vhci_hcd.c:1349:2: error: call to undeclared function 'dma_set_mask'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1349 |         dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
         |         ^
   drivers/usb/usbip/vhci_hcd.c:1349:2: note: did you mean 'xa_set_mark'?
   include/linux/xarray.h:361:6: note: 'xa_set_mark' declared here
     361 | void xa_set_mark(struct xarray *, unsigned long index, xa_mark_t);
         |      ^
>> drivers/usb/usbip/vhci_hcd.c:1349:27: error: call to undeclared function 'DMA_BIT_MASK'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1349 |         dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
         |                                  ^
   2 errors generated.


vim +/dma_set_mask +1349 drivers/usb/usbip/vhci_hcd.c

  1338	
  1339	static int vhci_hcd_probe(struct platform_device *pdev)
  1340	{
  1341		struct vhci             *vhci = *((void **)dev_get_platdata(&pdev->dev));
  1342		struct usb_hcd		*hcd_hs;
  1343		struct usb_hcd		*hcd_ss;
  1344		int			ret;
  1345	
  1346		usbip_dbg_vhci_hc("name %s id %d\n", pdev->name, pdev->id);
  1347	
  1348		/* Set the dma mask to support 64bit for vhci-hcd driver. */
> 1349		dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
  1350	
  1351		/*
  1352		 * Allocate and initialize hcd.
  1353		 * Our private data is also allocated automatically.
  1354		 */
  1355		hcd_hs = usb_create_hcd(&vhci_hc_driver, &pdev->dev, dev_name(&pdev->dev));
  1356		if (!hcd_hs) {
  1357			pr_err("create primary hcd failed\n");
  1358			return -ENOMEM;
  1359		}
  1360		hcd_hs->has_tt = 1;
  1361	
  1362		/*
  1363		 * Finish generic HCD structure initialization and register.
  1364		 * Call the driver's reset() and start() routines.
  1365		 */
  1366		ret = usb_add_hcd(hcd_hs, 0, 0);
  1367		if (ret != 0) {
  1368			pr_err("usb_add_hcd hs failed %d\n", ret);
  1369			goto put_usb2_hcd;
  1370		}
  1371	
  1372		hcd_ss = usb_create_shared_hcd(&vhci_hc_driver, &pdev->dev,
  1373					       dev_name(&pdev->dev), hcd_hs);
  1374		if (!hcd_ss) {
  1375			ret = -ENOMEM;
  1376			pr_err("create shared hcd failed\n");
  1377			goto remove_usb2_hcd;
  1378		}
  1379	
  1380		ret = usb_add_hcd(hcd_ss, 0, 0);
  1381		if (ret) {
  1382			pr_err("usb_add_hcd ss failed %d\n", ret);
  1383			goto put_usb3_hcd;
  1384		}
  1385	
  1386		usbip_dbg_vhci_hc("bye\n");
  1387		return 0;
  1388	
  1389	put_usb3_hcd:
  1390		usb_put_hcd(hcd_ss);
  1391	remove_usb2_hcd:
  1392		usb_remove_hcd(hcd_hs);
  1393	put_usb2_hcd:
  1394		usb_put_hcd(hcd_hs);
  1395		vhci->vhci_hcd_hs = NULL;
  1396		vhci->vhci_hcd_ss = NULL;
  1397		return ret;
  1398	}
  1399	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[kernel test robot]

Hi Zongmin,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.15-rc3 next-20250422]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zongmin-Zhou/usbip-set-the-dma-mask-to-64bit-default-for-vhci-driver/20250422-143646
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250422063409.607859-1-min_halo%40163.com
patch subject: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20250423/202504231526.8kjh2iap-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250423/202504231526.8kjh2iap-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504231526.8kjh2iap-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/usb/usbip/vhci_hcd.c: In function 'vhci_hcd_probe':
>> drivers/usb/usbip/vhci_hcd.c:1349:9: error: implicit declaration of function 'dma_set_mask'; did you mean 'xa_set_mark'? [-Wimplicit-function-declaration]
    1349 |         dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
         |         ^~~~~~~~~~~~
         |         xa_set_mark
>> drivers/usb/usbip/vhci_hcd.c:1349:34: error: implicit declaration of function 'DMA_BIT_MASK'; did you mean 'BIT_MASK'? [-Wimplicit-function-declaration]
    1349 |         dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
         |                                  ^~~~~~~~~~~~
         |                                  BIT_MASK

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for DRM_AUX_HPD_BRIDGE
   Depends on [n]: HAS_IOMEM [=y] && DRM [=n] && DRM_BRIDGE [=n] && OF [=y]
   Selected by [m]:
   - UCSI_HUAWEI_GAOKUN [=m] && USB_SUPPORT [=y] && TYPEC [=m] && TYPEC_UCSI [=m] && EC_HUAWEI_GAOKUN [=m]


vim +1349 drivers/usb/usbip/vhci_hcd.c

  1338	
  1339	static int vhci_hcd_probe(struct platform_device *pdev)
  1340	{
  1341		struct vhci             *vhci = *((void **)dev_get_platdata(&pdev->dev));
  1342		struct usb_hcd		*hcd_hs;
  1343		struct usb_hcd		*hcd_ss;
  1344		int			ret;
  1345	
  1346		usbip_dbg_vhci_hc("name %s id %d\n", pdev->name, pdev->id);
  1347	
  1348		/* Set the dma mask to support 64bit for vhci-hcd driver. */
> 1349		dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
  1350	
  1351		/*
  1352		 * Allocate and initialize hcd.
  1353		 * Our private data is also allocated automatically.
  1354		 */
  1355		hcd_hs = usb_create_hcd(&vhci_hc_driver, &pdev->dev, dev_name(&pdev->dev));
  1356		if (!hcd_hs) {
  1357			pr_err("create primary hcd failed\n");
  1358			return -ENOMEM;
  1359		}
  1360		hcd_hs->has_tt = 1;
  1361	
  1362		/*
  1363		 * Finish generic HCD structure initialization and register.
  1364		 * Call the driver's reset() and start() routines.
  1365		 */
  1366		ret = usb_add_hcd(hcd_hs, 0, 0);
  1367		if (ret != 0) {
  1368			pr_err("usb_add_hcd hs failed %d\n", ret);
  1369			goto put_usb2_hcd;
  1370		}
  1371	
  1372		hcd_ss = usb_create_shared_hcd(&vhci_hc_driver, &pdev->dev,
  1373					       dev_name(&pdev->dev), hcd_hs);
  1374		if (!hcd_ss) {
  1375			ret = -ENOMEM;
  1376			pr_err("create shared hcd failed\n");
  1377			goto remove_usb2_hcd;
  1378		}
  1379	
  1380		ret = usb_add_hcd(hcd_ss, 0, 0);
  1381		if (ret) {
  1382			pr_err("usb_add_hcd ss failed %d\n", ret);
  1383			goto put_usb3_hcd;
  1384		}
  1385	
  1386		usbip_dbg_vhci_hc("bye\n");
  1387		return 0;
  1388	
  1389	put_usb3_hcd:
  1390		usb_put_hcd(hcd_ss);
  1391	remove_usb2_hcd:
  1392		usb_remove_hcd(hcd_hs);
  1393	put_usb2_hcd:
  1394		usb_put_hcd(hcd_hs);
  1395		vhci->vhci_hcd_hs = NULL;
  1396		vhci->vhci_hcd_ss = NULL;
  1397		return ret;
  1398	}
  1399	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Zongmin Zhou]

Dear Greg and Christoph:

This patch is the simple solution for the issue described in below link:
https://lore.kernel.org/all/20250219092555.112631-1-min_halo@163.com/

This issue has been discussed at length with Shuah.

As Christoph said,vhci-hcd is a virtual hci,
but it serves as a bridge to connect virtual USB devices to the real USB 
controller.
Since the commit d74ffae8b8dd is applied on kernel v5.3,
the virtual USB devices's max_sectors size is limited since v5.3.

Just because on vhci-hcd, the dma mask setting follows the platform 
device default setting(32-bit).

So I want to get the real USB controller dma mask to set on vhci-hcd 
driver，
or set to 64 by default,to fix the max_sectors limited issue
on virtual USB device since the commit d74ffae8b8dd applied.

Thanks.


On 2025/4/22 15:24, Christoph Hellwig wrote:
> On Tue, Apr 22, 2025 at 02:34:09PM +0800, Zongmin Zhou wrote:
>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>
>> fix usb device limited  max_sectors when use usbip protocol
> As far as I can tell this is a virtual HCI and can't perform DMA.
> Thus nothing should use DMA API calls on it and the mask is irrelevant.

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Greg KH]

On Fri, Apr 25, 2025 at 04:08:47PM +0800, Zongmin Zhou wrote:
> Dear Greg and Christoph:
> 
> This patch is the simple solution for the issue described in below link:
> https://lore.kernel.org/all/20250219092555.112631-1-min_halo@163.com/
> 
> This issue has been discussed at length with Shuah.
> 
> As Christoph said,vhci-hcd is a virtual hci,
> but it serves as a bridge to connect virtual USB devices to the real USB
> controller.
> Since the commit d74ffae8b8dd is applied on kernel v5.3,
> the virtual USB devices's max_sectors size is limited since v5.3.
> 
> Just because on vhci-hcd, the dma mask setting follows the platform device
> default setting(32-bit).

Wait, why is vhci-hcd a platform device at all?  It shouldn't be doing
that, try removing that to start with, which will remove that "default"
setting.  I recommend using the faux bus as a potential replacement.

thanks,

greg k-h

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Zongmin Zhou]

Dear Greg and Shuah,

I found out that the vhci-hcd driver added this virtual device
as a platform device from the very beginning since 2014.
I'm just getting in touch with this module and
don't have a deep understanding of it，shuah should be clearer.

I don't know if using the faux bus to replace the platform bus can solve 
the problem
that the error limitation on max_hw_sectors for usbip device since 
commit d74ffae8b8dd applied.
But this change will request user to update kernel version to support 
faux bus.
This will also be an expensive change to fix the problem?

I have been discussing with shuah for a long time.
Is there any simpler way to fix this problem?

On 2025/4/25 16:28, Greg KH wrote:
> On Fri, Apr 25, 2025 at 04:08:47PM +0800, Zongmin Zhou wrote:
>> Dear Greg and Christoph:
>>
>> This patch is the simple solution for the issue described in below link:
>> https://lore.kernel.org/all/20250219092555.112631-1-min_halo@163.com/
>>
>> This issue has been discussed at length with Shuah.
>>
>> As Christoph said,vhci-hcd is a virtual hci,
>> but it serves as a bridge to connect virtual USB devices to the real USB
>> controller.
>> Since the commit d74ffae8b8dd is applied on kernel v5.3,
>> the virtual USB devices's max_sectors size is limited since v5.3.
>>
>> Just because on vhci-hcd, the dma mask setting follows the platform device
>> default setting(32-bit).
> Wait, why is vhci-hcd a platform device at all?  It shouldn't be doing
> that, try removing that to start with, which will remove that "default"
> setting.  I recommend using the faux bus as a potential replacement.
>
> thanks,
>
> greg k-h

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Greg KH]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Mon, Apr 28, 2025 at 05:51:08PM +0800, Zongmin Zhou wrote:
> Dear Greg and Shuah,
> 
> I found out that the vhci-hcd driver added this virtual device
> as a platform device from the very beginning since 2014.

Ah, I should have caught it back then, but at the time there really
wasn't another option.

> I'm just getting in touch with this module and
> don't have a deep understanding of it，shuah should be clearer.

See the recent patches I did converting drivers to use the faux bus
code, it should be pretty simple to do.

> I don't know if using the faux bus to replace the platform bus can solve the
> problem that the error limitation on max_hw_sectors for usbip device
> since commit d74ffae8b8dd applied.

That is for the storage driver, not usbip.  As the faux bus does not
have any real dma operations, this should cause it to work properly
given the default values involed, but that's up to you to test to verify
it does just that.  Try it and see!

> But this change will request user to update kernel version to support faux
> bus.

That's just a normal kernel update to a newer version, what is wrong
with that?

> This will also be an expensive change to fix the problem?

Fixing things properly is the correct thing to do in all cases.

thanks,

greg k-h

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Shuah Khan]

On 4/28/25 04:04, Greg KH wrote:
> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> 
> A: No.
> Q: Should I include quotations after my reply?
> 
> http://daringfireball.net/2007/07/on_top
> 
> On Mon, Apr 28, 2025 at 05:51:08PM +0800, Zongmin Zhou wrote:
>> Dear Greg and Shuah,
>>
>> I found out that the vhci-hcd driver added this virtual device
>> as a platform device from the very beginning since 2014.
> 
> Ah, I should have caught it back then, but at the time there really
> wasn't another option.
> 

>> I'm just getting in touch with this module and
>> don't have a deep understanding of it，shuah should be clearer.

faux_device should work fine for this. We do have to test of course.

There are several examples of converting  platform device to faux device.

72239a78f9f5b9f05ea4bb7a15b92807906dab71
dcd2a9a5550ef556c8fc11601a0f729fb71ead5d

> 
> See the recent patches I did converting drivers to use the faux bus
> code, it should be pretty simple to do.
> 
>> I don't know if using the faux bus to replace the platform bus can solve the
>> problem that the error limitation on max_hw_sectors for usbip device
>> since commit d74ffae8b8dd applied.
> 
> That is for the storage driver, not usbip.  As the faux bus does not
> have any real dma operations, this should cause it to work properly
> given the default values involed, but that's up to you to test to verify
> it does just that.  Try it and see!
> 
>> But this change will request user to update kernel version to support faux
>> bus.
> 
> That's just a normal kernel update to a newer version, what is wrong
> with that?

With one difference that the fix depends on faux_device feature - hence
we can't apply it to stables. I do think it is the right direction to
go to faux_device.

> 
>> This will also be an expensive change to fix the problem?
> 
> Fixing things properly is the correct thing to do in all cases.
> 

Zongmin, do let me know if you are unable to make the change.

thanks,
-- Shuah

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Zongmin Zhou]

On 2025/4/29 07:07, Shuah Khan wrote:
> On 4/28/25 04:04, Greg KH wrote:
>> A: http://en.wikipedia.org/wiki/Top_post
>> Q: Were do I find info about this thing called top-posting?
>> A: Because it messes up the order in which people normally read text.
>> Q: Why is top-posting such a bad thing?
>> A: Top-posting.
>> Q: What is the most annoying thing in e-mail?
>>
>> A: No.
>> Q: Should I include quotations after my reply?
>>
>> http://daringfireball.net/2007/07/on_top
>>
Sorry for that,and thank you for the comment!I've learned it.
>> On Mon, Apr 28, 2025 at 05:51:08PM +0800, Zongmin Zhou wrote:
>>> Dear Greg and Shuah,
>>>
>>> I found out that the vhci-hcd driver added this virtual device
>>> as a platform device from the very beginning since 2014.
>>
>> Ah, I should have caught it back then, but at the time there really
>> wasn't another option.
>>
>
>>> I'm just getting in touch with this module and
>>> don't have a deep understanding of it，shuah should be clearer.
>
> faux_device should work fine for this. We do have to test of course.
>
> There are several examples of converting  platform device to faux device.
>
> 72239a78f9f5b9f05ea4bb7a15b92807906dab71
> dcd2a9a5550ef556c8fc11601a0f729fb71ead5d
>
Ok,I will learn to do it.
Thank you for your guidance.
>>
>> See the recent patches I did converting drivers to use the faux bus
>> code, it should be pretty simple to do.
>>
>>> I don't know if using the faux bus to replace the platform bus can 
>>> solve the
>>> problem that the error limitation on max_hw_sectors for usbip device
>>> since commit d74ffae8b8dd applied.
>>
>> That is for the storage driver, not usbip.  As the faux bus does not
>> have any real dma operations, this should cause it to work properly
>> given the default values involed, but that's up to you to test to verify
>> it does just that.  Try it and see!
>>
>>> But this change will request user to update kernel version to 
>>> support faux
>>> bus.
>>
>> That's just a normal kernel update to a newer version, what is wrong
>> with that?
>
> With one difference that the fix depends on faux_device feature - hence
> we can't apply it to stables. I do think it is the right direction to
> go to faux_device.
>
I just encountered similar users who were using a lower version of the 
kernel and were unwilling to upgrade.
But if reach a consensus to  go to faux_device,
I will try to make the change(converting platform device to faux device 
for vhci-hcd).
>>
>>> This will also be an expensive change to fix the problem?
>>
>> Fixing things properly is the correct thing to do in all cases.
>>
>
> Zongmin, do let me know if you are unable to make the change.
>
> thanks,
> -- Shuah

Re: [PATCH] usbip: set the dma mask to 64bit default for vhci-driver

[Greg KH]

On Wed, Apr 30, 2025 at 01:24:31PM +0800, Zongmin Zhou wrote:
> > With one difference that the fix depends on faux_device feature - hence
> > we can't apply it to stables. I do think it is the right direction to
> > go to faux_device.
> > 
> I just encountered similar users who were using a lower version of the
> kernel and were unwilling to upgrade.

That is not our problem here, sorry.

> But if reach a consensus to  go to faux_device,
> I will try to make the change(converting platform device to faux device for
> vhci-hcd).

We have said that you should try this, no need for us to ask again.

thanks,

greg k-h