* 6.5.0 broke XHCI URB submissions for count >512
@ 2024-03-02 0:27 Chris Yokum
2024-03-02 6:53 ` Linux regression tracking (Thorsten Leemhuis)
0 siblings, 1 reply; 9+ messages in thread
From: Chris Yokum @ 2024-03-02 0:27 UTC (permalink / raw)
To: stable; +Cc: regressions
[-- Attachment #1.1: Type: text/plain, Size: 1828 bytes --]
We have found a regression bug, where more than 512 URBs cannot be reliably submitted to XHCI. URBs beyond that return 0x00 instead of valid data in the buffer.
Our software works reliably on kernel versions through 6.4.x and fails on versions 6.5, 6.6, 6.7, and 6.8.0-rc6. This was discovered when Ubuntu recently updated their latest kernel package to version 6.5.
The issue is limited to the XHCI driver and appears to be isolated to this specific commit:
[ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675 | https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675 ]
Attached is a test program that demonstrates the problem. We used a few different USB-to-Serial adapters with no driver installed as a convenient way to reproduce. We check the TRB debug information before and after to verify the actual number of allocated TRBs.
With some adapters on unaffected kernels, the TRB map gets expanded correctly. This directly corresponds to correct functional behavior. On affected kernels, the TRB ring does not expand, and our functional tests also will fail.
We don't know exactly why this happens. Some adapters do work correctly, so there seems to also be some subtle problem that was being masked by the liberal expansion of the TRB ring in older kernels. We also saw on one system that the TRB expansion did work correctly with one particular adapter. However, on all systems at least two adapters did exhibit the problem and fail.
Would it be possible to resolve this regression for the 6.8 release and backport the fix to versions 6.5, 6.6, and 6.7?
#regzbot ^introduced: f5af638f0609af889f15c700c60b93c06cc76675
[-- Attachment #1.2: Type: text/html, Size: 20668 bytes --]
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xhci_bug.c --]
[-- Type: text/x-csrc; name=xhci_bug.c, Size: 3923 bytes --]
// Test case for XHCI buffer expansion regression
//
// 1) Compile this program: gcc -o xhci_bug xhci_bug.c
// 2) Plug in a supported USB to serial device
// 3) Get the bus and device numbers from lsusb:
// Bus 003 Device 015: ID 0403:6001 Future Technology Devices International, Ltd FT232 Serial (UART) IC
// 4) Run as root: ./xhci_bug ftdi_sio 3 15 4096
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <linux/usbdevice_fs.h>
#define URB_SIZE 64
struct Device {
const char *driver;
int ep;
const char *ep_debug;
};
static struct Device devices[] = {
{ "ftdi_sio", 0x81, "ep02" },
{ "pl2303", 0x83, "ep06" },
{ "cp210x", 0x82, "ep04" },
{ },
};
int main (int argc, char *argv[])
{
// Parse command line
if (argc < 4) {
char drivers[255] = { };
for (int i = 0; devices[i].driver; ++i) {
strcat(drivers, devices[i].driver);
if (devices[i+1].driver) strcat(drivers, "|");
}
printf("usage: xhci_bug <%s> BUS DEV NUM_URBS\n", drivers);
return 1;
}
const char *driver = argv[1];
int bus = atoi(argv[2]);
int dev = atoi(argv[3]);
int num_urbs = atoi(argv[4]);
if (num_urbs > 4096)
num_urbs = 4096;
// Find the driver
struct Device *device = NULL;
for (int i = 0; devices[i].driver; ++i) {
if (strcmp(devices[i].driver, driver) == 0) {
device = &devices[i];
break;
}
}
if (device == NULL) {
printf("error: driver %s not found\n", driver);
return 1;
}
// Remove the driver
char cmd[1024];
sprintf(cmd, "lsmod | grep %s -q && rmmod %s", driver, driver);
system(cmd);
// Get the debug path
char debug[256] = { };
sprintf(
cmd,
"find /sys/kernel/debug/usb/xhci -name 'name' | "
"while IFS= read -r line; do "
"dir=$(dirname $line); "
"sys=/sys/bus/usb/devices/$(cat $dir/name); "
"grep -q '^%d$' $sys/busnum && "
"grep -q '^%d$' $sys/devnum && "
"echo $dir; "
"done",
bus, dev
);
FILE *fp = popen(cmd, "r");
char *p = fgets(debug, sizeof(debug), fp);
pclose(fp);
if (p == NULL) {
printf("error: debug path not found\n");
return 1;
}
debug[strlen(debug)-1] = '\0';
// Print the number of TRBs
printf("Before URB submission:\n");
sprintf(cmd, "wc -l %s/%s/trbs", debug, device->ep_debug);
system(cmd);
printf("\n");
// Open the device
char path[256];
sprintf(path, "/dev/bus/usb/%03d/%03d", bus, dev);
int fd = open(path, O_RDWR);
if (fd < 0)
return 1;
int rc;
int cfg = 1;
rc = ioctl(fd, USBDEVFS_SETCONFIGURATION, &cfg);
if (rc < 0 && errno != EBUSY) {
printf("error: unable to set configuration\n");
close(fd);
return 1;
}
int ifce = 0;
rc = ioctl(fd, USBDEVFS_CLAIMINTERFACE, &ifce);
if (rc < 0) {
printf("error: unable to claim interface\n");
close(fd);
return 1;
}
// Submit URBs
printf("Submitting %d URBs\n\n", num_urbs);
for (int i = 0; i < num_urbs; ++i) {
struct usbdevfs_urb *ioctl_urb = calloc(1, sizeof(*ioctl_urb));
ioctl_urb->type = USBDEVFS_URB_TYPE_BULK;
ioctl_urb->endpoint = device->ep;
ioctl_urb->buffer = calloc(1, URB_SIZE);
ioctl_urb->buffer_length = URB_SIZE;
rc = ioctl(fd, USBDEVFS_SUBMITURB, ioctl_urb);
if (rc < 0) {
printf("error: could not submit urb #%d\n", i);
return 1;
}
}
// Print the number of TRBs
printf("After URB submission:\n");
system(cmd);
printf("\n");
// Close the device
close(fd);
return 0;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-02 0:27 6.5.0 broke XHCI URB submissions for count >512 Chris Yokum
@ 2024-03-02 6:53 ` Linux regression tracking (Thorsten Leemhuis)
2024-03-02 7:14 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2024-03-02 6:53 UTC (permalink / raw)
To: Mathias Nyman
Cc: regressions, Chris Yokum, stable, Greg Kroah-Hartman,
Linux kernel regressions list
[adding the people involved in developing and applying the culprit to
the list of recipients]
Hi! Thx for the report.
On 02.03.24 01:27, Chris Yokum wrote:
> We have found a regression bug, where more than 512 URBs cannot be
> reliably submitted to XHCI. URBs beyond that return 0x00 instead of
> valid data in the buffer.
>
> Our software works reliably on kernel versions through 6.4.x and fails
> on versions 6.5, 6.6, 6.7, and 6.8.0-rc6. This was discovered when
> Ubuntu recently updated their latest kernel package to version 6.5.
>
> The issue is limited to the XHCI driver and appears to be isolated to
> this specific commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675 <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675>
FWIW, that's f5af638f0609af ("xhci: Fix transfer ring expansion size
calculation") [v6.5-rc1] from Mathias.
> Attached is a test program that demonstrates the problem. We used a few
> different USB-to-Serial adapters with no driver installed as a
> convenient way to reproduce. We check the TRB debug information before
> and after to verify the actual number of allocated TRBs.
>
> With some adapters on unaffected kernels, the TRB map gets expanded
> correctly. This directly corresponds to correct functional behavior. On
> affected kernels, the TRB ring does not expand, and our functional tests
> also will fail.
>
> We don't know exactly why this happens. Some adapters do work correctly,
> so there seems to also be some subtle problem that was being masked by
> the liberal expansion of the TRB ring in older kernels. We also saw on
> one system that the TRB expansion did work correctly with one particular
> adapter. However, on all systems at least two adapters did exhibit the
> problem and fail.
>
> Would it be possible to resolve this regression for the 6.8 release and
> backport the fix to versions 6.5, 6.6, and 6.7?
6.5 is EOL at kernel.org, that's thus up to downstream distros. And with
a bit of luck it might be possible to fix this for 6.8, but it might be
too late for that already. We'll see.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-02 6:53 ` Linux regression tracking (Thorsten Leemhuis)
@ 2024-03-02 7:14 ` Greg Kroah-Hartman
2024-03-02 15:55 ` Chris Yokum
0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2024-03-02 7:14 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Mathias Nyman, Chris Yokum, stable, linux-usb
On Sat, Mar 02, 2024 at 07:53:12AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> [adding the people involved in developing and applying the culprit to
> the list of recipients]
>
> Hi! Thx for the report.
>
> On 02.03.24 01:27, Chris Yokum wrote:
> > We have found a regression bug, where more than 512 URBs cannot be
> > reliably submitted to XHCI. URBs beyond that return 0x00 instead of
> > valid data in the buffer.
You mean 512 outstanding URBS that are not completed? What in-kernel
driver does this?
> > Our software works reliably on kernel versions through 6.4.x and fails
> > on versions 6.5, 6.6, 6.7, and 6.8.0-rc6. This was discovered when
> > Ubuntu recently updated their latest kernel package to version 6.5.
> >
> > The issue is limited to the XHCI driver and appears to be isolated to
> > this specific commit:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675 <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675>
>
> FWIW, that's f5af638f0609af ("xhci: Fix transfer ring expansion size
> calculation") [v6.5-rc1] from Mathias.
>
> > Attached is a test program that demonstrates the problem. We used a few
> > different USB-to-Serial adapters with no driver installed as a
> > convenient way to reproduce. We check the TRB debug information before
> > and after to verify the actual number of allocated TRBs.
Ah, so this is just through usbfs?
> > With some adapters on unaffected kernels, the TRB map gets expanded
> > correctly. This directly corresponds to correct functional behavior. On
> > affected kernels, the TRB ring does not expand, and our functional tests
> > also will fail.
> >
> > We don't know exactly why this happens. Some adapters do work correctly,
> > so there seems to also be some subtle problem that was being masked by
> > the liberal expansion of the TRB ring in older kernels. We also saw on
> > one system that the TRB expansion did work correctly with one particular
> > adapter. However, on all systems at least two adapters did exhibit the
> > problem and fail.
Any chance you can provide the 'lspci' output for the controllers that
work and those that do not?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-02 7:14 ` Greg Kroah-Hartman
@ 2024-03-02 15:55 ` Chris Yokum
2024-03-04 11:57 ` Mathias Nyman
0 siblings, 1 reply; 9+ messages in thread
From: Chris Yokum @ 2024-03-02 15:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linux regressions mailing list, Mathias Nyman, Chris Yokum,
stable, linux-usb
Hi Greg,
Thank you for the quick follow-up!
The submission of >512 URBs is via usbfs, yes. This worked forever, and still works on EHCI, it's just been failing on xHCI once the indicated change was applied.
We see failures of our products due to this, which use usbfs. In order to create a simple repro case, we picked a few USB-to-serial devices that we assumed you would have in your possession, removed the kernel function driver, and accessed it in this way via usbfs.
There is no specific xHCI controller or USB-to-Serial adapter that always works or always fails. This is quite easy to make fail for us though. I primarily note this because the fail is not 100%, but it is pervasive.
We could reproduce within qemu, plus on older Celeron systems, i5-1240p, AMD Ryzen 7 5700U, and many other reported failures from the field.
Here is the output related to a failing case:
user@linux1:~/xhci_bug$ lsusb
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 001 Device 006: ID 10c4:ea60 Silicon Labs CP210x UART Bridge
Bus 001 Device 005: ID 0403:6001 Future Technology Devices International, Ltd FT232 Serial (UART) IC
Bus 001 Device 003: ID 0424:2514 Microchip Technology, Inc. (formerly SMSC) USB 2.0 Hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
user@linux1:~/xhci_bug$ sudo ./xhci_bug cp210x 1 6 1024
Before URB submission:
512 /sys/kernel/debug/usb/xhci/0000:00:14.0/devices/05/ep04/trbs
Submitting 1024 URBs
After URB submission:
512 /sys/kernel/debug/usb/xhci/0000:00:14.0/devices/05/ep04/trbs
user@linux1:~/xhci_bug$ uname -a
Linux linux1 6.8.0-060800rc6-generic #202402251933 SMP PREEMPT_DYNAMIC Mon Feb 26 00:46:39 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
user@linux1:~/xhci_bug$
----- Original Message -----
From: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
To: "Linux regressions mailing list" <regressions@lists.linux.dev>
Cc: "Mathias Nyman" <mathias.nyman@linux.intel.com>, "Chris Yokum" <linux-usb@mail.totalphase.com>, "stable" <stable@vger.kernel.org>, linux-usb@vger.kernel.org
Sent: Friday, March 1, 2024 11:14:50 PM
Subject: Re: 6.5.0 broke XHCI URB submissions for count >512
On Sat, Mar 02, 2024 at 07:53:12AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote:
> [adding the people involved in developing and applying the culprit to
> the list of recipients]
>
> Hi! Thx for the report.
>
> On 02.03.24 01:27, Chris Yokum wrote:
> > We have found a regression bug, where more than 512 URBs cannot be
> > reliably submitted to XHCI. URBs beyond that return 0x00 instead of
> > valid data in the buffer.
You mean 512 outstanding URBS that are not completed? What in-kernel
driver does this?
> > Our software works reliably on kernel versions through 6.4.x and fails
> > on versions 6.5, 6.6, 6.7, and 6.8.0-rc6. This was discovered when
> > Ubuntu recently updated their latest kernel package to version 6.5.
> >
> > The issue is limited to the XHCI driver and appears to be isolated to
> > this specific commit:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675 <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb?h=v6.5&id=f5af638f0609af889f15c700c60b93c06cc76675>
>
> FWIW, that's f5af638f0609af ("xhci: Fix transfer ring expansion size
> calculation") [v6.5-rc1] from Mathias.
>
> > Attached is a test program that demonstrates the problem. We used a few
> > different USB-to-Serial adapters with no driver installed as a
> > convenient way to reproduce. We check the TRB debug information before
> > and after to verify the actual number of allocated TRBs.
Ah, so this is just through usbfs?
> > With some adapters on unaffected kernels, the TRB map gets expanded
> > correctly. This directly corresponds to correct functional behavior. On
> > affected kernels, the TRB ring does not expand, and our functional tests
> > also will fail.
> >
> > We don't know exactly why this happens. Some adapters do work correctly,
> > so there seems to also be some subtle problem that was being masked by
> > the liberal expansion of the TRB ring in older kernels. We also saw on
> > one system that the TRB expansion did work correctly with one particular
> > adapter. However, on all systems at least two adapters did exhibit the
> > problem and fail.
Any chance you can provide the 'lspci' output for the controllers that
work and those that do not?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-02 15:55 ` Chris Yokum
@ 2024-03-04 11:57 ` Mathias Nyman
2024-03-04 15:53 ` Mathias Nyman
0 siblings, 1 reply; 9+ messages in thread
From: Mathias Nyman @ 2024-03-04 11:57 UTC (permalink / raw)
To: Chris Yokum, Greg Kroah-Hartman
Cc: Linux regressions mailing list, stable, linux-usb, Niklas Neronin
On 2.3.2024 17.55, Chris Yokum wrote:
>
> The submission of >512 URBs is via usbfs, yes. This worked forever, and still works on EHCI, it's just been failing on xHCI once the indicated change was applied.
>
>>> We have found a regression bug, where more than 512 URBs cannot be
>>> reliably submitted to XHCI. URBs beyond that return 0x00 instead of
>>> valid data in the buffer.
>
>>
>> FWIW, that's f5af638f0609af ("xhci: Fix transfer ring expansion size
>> calculation") [v6.5-rc1] from Mathias.
>>
>>> Attached is a test program that demonstrates the problem. We used a few
>>> different USB-to-Serial adapters with no driver installed as a
>>> convenient way to reproduce. We check the TRB debug information before
>>> and after to verify the actual number of allocated TRBs.
>
Could you send me that test program as well?
> Ah, so this is just through usbfs?
>
>>> With some adapters on unaffected kernels, the TRB map gets expanded
>>> correctly. This directly corresponds to correct functional behavior. On
>>> affected kernels, the TRB ring does not expand, and our functional tests
>>> also will fail.
>>>
>>> We don't know exactly why this happens. Some adapters do work correctly,
>>> so there seems to also be some subtle problem that was being masked by
>>> the liberal expansion of the TRB ring in older kernels. We also saw on
>>> one system that the TRB expansion did work correctly with one particular
>>> adapter. However, on all systems at least two adapters did exhibit the
>>> problem and fail.
Ok, I see, this could be the empty ring exception check in xhci-ring.c:
It could falsely assume ring is empty when it in fact is filled up in one
go by queuing several small urbs.
static unsigned int xhci_ring_expansion_needed(struct xhci_hcd *xhci, struct xhci_ring *ring,
unsigned int num_trbs)
{
...
/* Empty ring special case, enqueue stuck on link trb while dequeue advanced */
if (trb_is_link(ring->enqueue) && ring->enq_seg->next->trbs == ring->dequeue)
return 0;
...
}
https://elixir.bootlin.com/linux/v6.7/source/drivers/usb/host/xhci-ring.c#L333
Can you help me test some patches on your setup?
Thanks
Mathias
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-04 11:57 ` Mathias Nyman
@ 2024-03-04 15:53 ` Mathias Nyman
2024-03-04 23:50 ` Chris Yokum
0 siblings, 1 reply; 9+ messages in thread
From: Mathias Nyman @ 2024-03-04 15:53 UTC (permalink / raw)
To: Chris Yokum, Greg Kroah-Hartman
Cc: Linux regressions mailing list, stable, linux-usb, Niklas Neronin
On 4.3.2024 13.57, Mathias Nyman wrote:
> On 2.3.2024 17.55, Chris Yokum wrote:
>>>> We have found a regression bug, where more than 512 URBs cannot be
>>>> reliably submitted to XHCI. URBs beyond that return 0x00 instead of
>>>> valid data in the buffer.
>>>
>>> FWIW, that's f5af638f0609af ("xhci: Fix transfer ring expansion size
>>> calculation") [v6.5-rc1] from Mathias.
>>>
>
> Ok, I see, this could be the empty ring exception check in xhci-ring.c:
>
> It could falsely assume ring is empty when it in fact is filled up in one
> go by queuing several small urbs.
Does this help?
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6a29ebd6682d..52278afea94b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -332,7 +332,13 @@ static unsigned int xhci_ring_expansion_needed(struct xhci_hcd *xhci, struct xhc
/* how many trbs will be queued past the enqueue segment? */
trbs_past_seg = enq_used + num_trbs - (TRBS_PER_SEGMENT - 1);
- if (trbs_past_seg <= 0)
+ /*
+ * Consider expanding the ring already if num_trbs fills the current
+ * segment (i.e. trbs_past_seg == 0), not only when num_trbs goes into
+ * the next segment. Avoids confusing full ring with special empty ring
+ * case below
+ */
+ if (trbs_past_seg < 0)
return 0;
Thanks
Mathias
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-04 15:53 ` Mathias Nyman
@ 2024-03-04 23:50 ` Chris Yokum
2024-03-11 17:03 ` Chris Yokum
0 siblings, 1 reply; 9+ messages in thread
From: Chris Yokum @ 2024-03-04 23:50 UTC (permalink / raw)
To: Mathias Nyman
Cc: Chris Yokum, Greg Kroah-Hartman, Linux regressions mailing list,
stable, linux-usb, Niklas Neronin
Hello Mathias,
Yes! This fixed the problem. I've checked with our repro case as well as our functional tests.
I'll email you the repro code directly, you can compare the unpatched and patched kernel behavior.
Best regards,
Chris
----- Original Message -----
From: "Mathias Nyman" <mathias.nyman@linux.intel.com>
To: "Chris Yokum" <linux-usb@mail.totalphase.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: "Linux regressions mailing list" <regressions@lists.linux.dev>, "stable" <stable@vger.kernel.org>, "linux-usb" <linux-usb@vger.kernel.org>, "Niklas Neronin" <niklas.neronin@linux.intel.com>
Sent: Monday, March 4, 2024 7:53:03 AM
Subject: Re: 6.5.0 broke XHCI URB submissions for count >512
On 4.3.2024 13.57, Mathias Nyman wrote:
> On 2.3.2024 17.55, Chris Yokum wrote:
>>>> We have found a regression bug, where more than 512 URBs cannot be
>>>> reliably submitted to XHCI. URBs beyond that return 0x00 instead of
>>>> valid data in the buffer.
>>>
>>> FWIW, that's f5af638f0609af ("xhci: Fix transfer ring expansion size
>>> calculation") [v6.5-rc1] from Mathias.
>>>
>
> Ok, I see, this could be the empty ring exception check in xhci-ring.c:
>
> It could falsely assume ring is empty when it in fact is filled up in one
> go by queuing several small urbs.
Does this help?
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6a29ebd6682d..52278afea94b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -332,7 +332,13 @@ static unsigned int xhci_ring_expansion_needed(struct xhci_hcd *xhci, struct xhc
/* how many trbs will be queued past the enqueue segment? */
trbs_past_seg = enq_used + num_trbs - (TRBS_PER_SEGMENT - 1);
- if (trbs_past_seg <= 0)
+ /*
+ * Consider expanding the ring already if num_trbs fills the current
+ * segment (i.e. trbs_past_seg == 0), not only when num_trbs goes into
+ * the next segment. Avoids confusing full ring with special empty ring
+ * case below
+ */
+ if (trbs_past_seg < 0)
return 0;
Thanks
Mathias
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-04 23:50 ` Chris Yokum
@ 2024-03-11 17:03 ` Chris Yokum
2024-03-12 8:52 ` Mathias Nyman
0 siblings, 1 reply; 9+ messages in thread
From: Chris Yokum @ 2024-03-11 17:03 UTC (permalink / raw)
To: Mathias Nyman
Cc: Greg Kroah-Hartman, Linux regressions mailing list, stable,
linux-usb, Niklas Neronin
Hello Mathias,
Thanks for the help with this! We saw that it's made it into 6.8. Is it possible to get this into 6.6 and 6.7?
Best regards,
Chris
----- Original Message -----
From: "Chris Yokum" <linux-usb@mail.totalphase.com>
To: "Mathias Nyman" <mathias.nyman@linux.intel.com>
Cc: "Chris Yokum" <linux-usb@mail.totalphase.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Linux regressions mailing list" <regressions@lists.linux.dev>, "stable" <stable@vger.kernel.org>, "linux-usb" <linux-usb@vger.kernel.org>, "Niklas Neronin" <niklas.neronin@linux.intel.com>
Sent: Monday, March 4, 2024 3:50:58 PM
Subject: Re: 6.5.0 broke XHCI URB submissions for count >512
Hello Mathias,
Yes! This fixed the problem. I've checked with our repro case as well as our functional tests.
I'll email you the repro code directly, you can compare the unpatched and patched kernel behavior.
Best regards,
Chris
----- Original Message -----
From: "Mathias Nyman" <mathias.nyman@linux.intel.com>
To: "Chris Yokum" <linux-usb@mail.totalphase.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: "Linux regressions mailing list" <regressions@lists.linux.dev>, "stable" <stable@vger.kernel.org>, "linux-usb" <linux-usb@vger.kernel.org>, "Niklas Neronin" <niklas.neronin@linux.intel.com>
Sent: Monday, March 4, 2024 7:53:03 AM
Subject: Re: 6.5.0 broke XHCI URB submissions for count >512
On 4.3.2024 13.57, Mathias Nyman wrote:
> On 2.3.2024 17.55, Chris Yokum wrote:
>>>> We have found a regression bug, where more than 512 URBs cannot be
>>>> reliably submitted to XHCI. URBs beyond that return 0x00 instead of
>>>> valid data in the buffer.
>>>
>>> FWIW, that's f5af638f0609af ("xhci: Fix transfer ring expansion size
>>> calculation") [v6.5-rc1] from Mathias.
>>>
>
> Ok, I see, this could be the empty ring exception check in xhci-ring.c:
>
> It could falsely assume ring is empty when it in fact is filled up in one
> go by queuing several small urbs.
Does this help?
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6a29ebd6682d..52278afea94b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -332,7 +332,13 @@ static unsigned int xhci_ring_expansion_needed(struct xhci_hcd *xhci, struct xhc
/* how many trbs will be queued past the enqueue segment? */
trbs_past_seg = enq_used + num_trbs - (TRBS_PER_SEGMENT - 1);
- if (trbs_past_seg <= 0)
+ /*
+ * Consider expanding the ring already if num_trbs fills the current
+ * segment (i.e. trbs_past_seg == 0), not only when num_trbs goes into
+ * the next segment. Avoids confusing full ring with special empty ring
+ * case below
+ */
+ if (trbs_past_seg < 0)
return 0;
Thanks
Mathias
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: 6.5.0 broke XHCI URB submissions for count >512
2024-03-11 17:03 ` Chris Yokum
@ 2024-03-12 8:52 ` Mathias Nyman
0 siblings, 0 replies; 9+ messages in thread
From: Mathias Nyman @ 2024-03-12 8:52 UTC (permalink / raw)
To: Chris Yokum
Cc: Greg Kroah-Hartman, Linux regressions mailing list, stable,
linux-usb, Niklas Neronin
On 11.3.2024 19.03, Chris Yokum wrote:
> Hello Mathias,
>
> Thanks for the help with this! We saw that it's made it into 6.8. Is it possible to get this into 6.6 and 6.7?
>
Patch is tagged for stable 6.5+.
If all goes well it should end up in 6.5 and later stable kernels
https://lore.kernel.org/all/20240305132312.955171-2-mathias.nyman@linux.intel.com/
Thanks
Mathias
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-03-12 8:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-02 0:27 6.5.0 broke XHCI URB submissions for count >512 Chris Yokum
2024-03-02 6:53 ` Linux regression tracking (Thorsten Leemhuis)
2024-03-02 7:14 ` Greg Kroah-Hartman
2024-03-02 15:55 ` Chris Yokum
2024-03-04 11:57 ` Mathias Nyman
2024-03-04 15:53 ` Mathias Nyman
2024-03-04 23:50 ` Chris Yokum
2024-03-11 17:03 ` Chris Yokum
2024-03-12 8:52 ` Mathias Nyman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).