* Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
@ 2024-04-09 13:49 Oliver Neukum
2024-04-09 14:56 ` Alan Stern
0 siblings, 1 reply; 16+ messages in thread
From: Oliver Neukum @ 2024-04-09 13:49 UTC (permalink / raw)
To: Alan Stern; +Cc: USB list, Takashi Iwai
[-- Attachment #1: Type: text/plain, Size: 2875 bytes --]
Hi,
with the following device:
Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 3.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0xfb5d
idProduct 0x0001
bcdDevice 0.00
iManufacturer 1 BHYVE
iProduct 2 HID Tablet
iSerial 3 01
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0028
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 4 HID Tablet Device
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 0mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 2 Mouse
iInterface 0
HID Device Descriptor:
bLength 9
bDescriptorType 33
bcdHID 10.01
bCountryCode 0 Not supported
bNumDescriptors 1
bDescriptorType 34 Report
wDescriptorLength 74
Report Descriptors:
** UNAVAILABLE **
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0008 1x 8 bytes
bInterval 10
bMaxBurst 0
Binary Object Store Descriptor:
bLength 5
bDescriptorType 15
wTotalLength 0x000f
bNumDeviceCaps 1
SuperSpeed USB Device Capability:
bLength 10
bDescriptorType 16
bDevCapabilityType 3
bmAttributes 0x00
wSpeedsSupported 0x0008
Device can operate at SuperSpeed (5Gbps)
bFunctionalitySupport 3
Lowest fully-functional device speed is SuperSpeed (5Gbps)
bU1DevExitLat 10 micro seconds
bU2DevExitLat 32 micro seconds
Device Status: 0x0000
(Bus Powered)
we are getting a regression on enumeration. It used to work with the
code prior to your patch. Takashi is proposing the attached fixed.
It looks like we are a bit too restrictive and should just try.
Regards
Oliver
[-- Attachment #2: 0001-USB-hub-Workaround-for-buggy-max-packet-size-with-su.patch --]
[-- Type: text/x-patch, Size: 1982 bytes --]
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
speed
Patch-mainline: Not yet, testing
References: bsc#1220569
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/usb/core/hub.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index e38a4124f610..64193effc456 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
const char *driver_name;
bool do_new_scheme;
const bool initial = !dev_descr;
- int maxp0;
+ int maxp0, ep0_maxp;
struct usb_device_descriptor *buf, *descr;
buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
@@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
else
i = 0; /* Invalid */
}
- if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
+ ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
+ if (ep0_maxp == i) {
; /* Initial ep0 maxpacket guess is right */
} else if ((udev->speed == USB_SPEED_FULL ||
udev->speed == USB_SPEED_HIGH) &&
@@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
usb_ep0_reinit(udev);
+ } else if (udev->speed >= USB_SPEED_SUPER && initial) {
+ /* FIXME: should be more restrictive? */
+ /* Initial guess is wrong; use the descriptor's value */
+ dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
+ udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ usb_ep0_reinit(udev);
} else {
/* Initial guess is wrong and descriptor's value is invalid */
- dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
+ dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
retval = -EMSGSIZE;
goto fail;
}
--
2.35.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-09 13:49 Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization") Oliver Neukum
@ 2024-04-09 14:56 ` Alan Stern
2024-04-22 17:33 ` Linux regression tracking (Thorsten Leemhuis)
2024-04-22 19:22 ` Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization") Takashi Iwai
0 siblings, 2 replies; 16+ messages in thread
From: Alan Stern @ 2024-04-09 14:56 UTC (permalink / raw)
To: Oliver Neukum; +Cc: USB list, Takashi Iwai
On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> Hi,
>
> with the following device:
>
> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> Device Descriptor:
> bLength 18
> bDescriptorType 1
> bcdUSB 3.00
> bDeviceClass 0
> bDeviceSubClass 0
> bDeviceProtocol 0
> bMaxPacketSize0 8
A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8
instead of 9? Presumably this thing never received a USB certification.
Does the packaging use the USB logo?
> idVendor 0xfb5d
> idProduct 0x0001
> bcdDevice 0.00
> iManufacturer 1 BHYVE
> iProduct 2 HID Tablet
> iSerial 3 01
> bNumConfigurations 1
Why on earth would an HID tablet need to run at SuperSpeed?
> Binary Object Store Descriptor:
> bLength 5
> bDescriptorType 15
> wTotalLength 0x000f
> bNumDeviceCaps 1
> SuperSpeed USB Device Capability:
> bLength 10
> bDescriptorType 16
> bDevCapabilityType 3
> bmAttributes 0x00
> wSpeedsSupported 0x0008
> Device can operate at SuperSpeed (5Gbps)
> bFunctionalitySupport 3
> Lowest fully-functional device speed is SuperSpeed (5Gbps)
> bU1DevExitLat 10 micro seconds
> bU2DevExitLat 32 micro seconds
A tablet not capable of running at any speed below 5 Gbps?
> we are getting a regression on enumeration. It used to work with the
> code prior to your patch. Takashi is proposing the attached fixed.
> It looks like we are a bit too restrictive and should just try.
>
> Regards
> Oliver
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
> speed
> Patch-mainline: Not yet, testing
> References: bsc#1220569
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>
> ---
> drivers/usb/core/hub.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index e38a4124f610..64193effc456 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> const char *driver_name;
> bool do_new_scheme;
> const bool initial = !dev_descr;
> - int maxp0;
> + int maxp0, ep0_maxp;
> struct usb_device_descriptor *buf, *descr;
>
> buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> else
> i = 0; /* Invalid */
> }
> - if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> + ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> + if (ep0_maxp == i) {
This variable looks like it was left over from earlier testing. It's
not really needed.
> ; /* Initial ep0 maxpacket guess is right */
> } else if ((udev->speed == USB_SPEED_FULL ||
> udev->speed == USB_SPEED_HIGH) &&
> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> usb_ep0_reinit(udev);
> + } else if (udev->speed >= USB_SPEED_SUPER && initial) {
> + /* FIXME: should be more restrictive? */
> + /* Initial guess is wrong; use the descriptor's value */
> + dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> + usb_ep0_reinit(udev);
This could be merged with the previous case fairly easily.
> } else {
> /* Initial guess is wrong and descriptor's value is invalid */
> - dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> + dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
This also looks like a remnant from earlier testing.
Alan Stern
> retval = -EMSGSIZE;
> goto fail;
> }
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-09 14:56 ` Alan Stern
@ 2024-04-22 17:33 ` Linux regression tracking (Thorsten Leemhuis)
2024-04-22 18:03 ` Alan Stern
2024-04-22 19:22 ` Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization") Takashi Iwai
1 sibling, 1 reply; 16+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2024-04-22 17:33 UTC (permalink / raw)
To: Alan Stern, Oliver Neukum
Cc: USB list, Takashi Iwai, Linux kernel regressions list
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.
Is anyone still working on fixing below regression? From here it looks
stalled, but I might have missed something.
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.
#regzbot poke
On 09.04.24 16:56, Alan Stern wrote:
> On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
>> Hi,
>>
>> with the following device:
>>
>> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
>> Device Descriptor:
>> bLength 18
>> bDescriptorType 1
>> bcdUSB 3.00
>> bDeviceClass 0
>> bDeviceSubClass 0
>> bDeviceProtocol 0
>> bMaxPacketSize0 8
>
> A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8
> instead of 9? Presumably this thing never received a USB certification.
> Does the packaging use the USB logo?
>
>> idVendor 0xfb5d
>> idProduct 0x0001
>> bcdDevice 0.00
>> iManufacturer 1 BHYVE
>> iProduct 2 HID Tablet
>> iSerial 3 01
>> bNumConfigurations 1
>
> Why on earth would an HID tablet need to run at SuperSpeed?
>
>> Binary Object Store Descriptor:
>> bLength 5
>> bDescriptorType 15
>> wTotalLength 0x000f
>> bNumDeviceCaps 1
>> SuperSpeed USB Device Capability:
>> bLength 10
>> bDescriptorType 16
>> bDevCapabilityType 3
>> bmAttributes 0x00
>> wSpeedsSupported 0x0008
>> Device can operate at SuperSpeed (5Gbps)
>> bFunctionalitySupport 3
>> Lowest fully-functional device speed is SuperSpeed (5Gbps)
>> bU1DevExitLat 10 micro seconds
>> bU2DevExitLat 32 micro seconds
>
> A tablet not capable of running at any speed below 5 Gbps?
>
>> we are getting a regression on enumeration. It used to work with the
>> code prior to your patch. Takashi is proposing the attached fixed.
>> It looks like we are a bit too restrictive and should just try.
>>
>> Regards
>> Oliver
>
>> From: Takashi Iwai <tiwai@suse.de>
>> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
>> speed
>> Patch-mainline: Not yet, testing
>> References: bsc#1220569
>>
>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>
>> ---
>> drivers/usb/core/hub.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index e38a4124f610..64193effc456 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>> const char *driver_name;
>> bool do_new_scheme;
>> const bool initial = !dev_descr;
>> - int maxp0;
>> + int maxp0, ep0_maxp;
>> struct usb_device_descriptor *buf, *descr;
>>
>> buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
>> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>> else
>> i = 0; /* Invalid */
>> }
>> - if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
>> + ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
>> + if (ep0_maxp == i) {
>
> This variable looks like it was left over from earlier testing. It's
> not really needed.
>
>> ; /* Initial ep0 maxpacket guess is right */
>> } else if ((udev->speed == USB_SPEED_FULL ||
>> udev->speed == USB_SPEED_HIGH) &&
>> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>> dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
>> udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
>> usb_ep0_reinit(udev);
>> + } else if (udev->speed >= USB_SPEED_SUPER && initial) {
>> + /* FIXME: should be more restrictive? */
>> + /* Initial guess is wrong; use the descriptor's value */
>> + dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
>> + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
>> + usb_ep0_reinit(udev);
>
> This could be merged with the previous case fairly easily.
>
>> } else {
>> /* Initial guess is wrong and descriptor's value is invalid */
>> - dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
>> + dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
>
> This also looks like a remnant from earlier testing.
>
> Alan Stern
>
>> retval = -EMSGSIZE;
>> goto fail;
>> }
>> --
>> 2.35.3
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-22 17:33 ` Linux regression tracking (Thorsten Leemhuis)
@ 2024-04-22 18:03 ` Alan Stern
2024-04-22 19:24 ` Takashi Iwai
0 siblings, 1 reply; 16+ messages in thread
From: Alan Stern @ 2024-04-22 18:03 UTC (permalink / raw)
To: Linux regressions mailing list; +Cc: Oliver Neukum, USB list, Takashi Iwai
On Mon, Apr 22, 2024 at 07:33:21PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> for once, to make this easily accessible to everyone.
>
> Is anyone still working on fixing below regression? From here it looks
> stalled, but I might have missed something.
I've been waiting to hear back from Oliver or Takashi. A revised patch
taking my comments into account would be welcome; it should be a very
small change (just one or two lines of code).
Alan Stern
> 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.
>
> #regzbot poke
>
> On 09.04.24 16:56, Alan Stern wrote:
> > On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> >> Hi,
> >>
> >> with the following device:
> >>
> >> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> >> Device Descriptor:
> >> bLength 18
> >> bDescriptorType 1
> >> bcdUSB 3.00
> >> bDeviceClass 0
> >> bDeviceSubClass 0
> >> bDeviceProtocol 0
> >> bMaxPacketSize0 8
> >
> > A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8
> > instead of 9? Presumably this thing never received a USB certification.
> > Does the packaging use the USB logo?
> >
> >> idVendor 0xfb5d
> >> idProduct 0x0001
> >> bcdDevice 0.00
> >> iManufacturer 1 BHYVE
> >> iProduct 2 HID Tablet
> >> iSerial 3 01
> >> bNumConfigurations 1
> >
> > Why on earth would an HID tablet need to run at SuperSpeed?
> >
> >> Binary Object Store Descriptor:
> >> bLength 5
> >> bDescriptorType 15
> >> wTotalLength 0x000f
> >> bNumDeviceCaps 1
> >> SuperSpeed USB Device Capability:
> >> bLength 10
> >> bDescriptorType 16
> >> bDevCapabilityType 3
> >> bmAttributes 0x00
> >> wSpeedsSupported 0x0008
> >> Device can operate at SuperSpeed (5Gbps)
> >> bFunctionalitySupport 3
> >> Lowest fully-functional device speed is SuperSpeed (5Gbps)
> >> bU1DevExitLat 10 micro seconds
> >> bU2DevExitLat 32 micro seconds
> >
> > A tablet not capable of running at any speed below 5 Gbps?
> >
> >> we are getting a regression on enumeration. It used to work with the
> >> code prior to your patch. Takashi is proposing the attached fixed.
> >> It looks like we are a bit too restrictive and should just try.
> >>
> >> Regards
> >> Oliver
> >
> >> From: Takashi Iwai <tiwai@suse.de>
> >> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
> >> speed
> >> Patch-mainline: Not yet, testing
> >> References: bsc#1220569
> >>
> >> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >>
> >> ---
> >> drivers/usb/core/hub.c | 13 ++++++++++---
> >> 1 file changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >> index e38a4124f610..64193effc456 100644
> >> --- a/drivers/usb/core/hub.c
> >> +++ b/drivers/usb/core/hub.c
> >> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >> const char *driver_name;
> >> bool do_new_scheme;
> >> const bool initial = !dev_descr;
> >> - int maxp0;
> >> + int maxp0, ep0_maxp;
> >> struct usb_device_descriptor *buf, *descr;
> >>
> >> buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> >> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >> else
> >> i = 0; /* Invalid */
> >> }
> >> - if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> >> + ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> >> + if (ep0_maxp == i) {
> >
> > This variable looks like it was left over from earlier testing. It's
> > not really needed.
> >
> >> ; /* Initial ep0 maxpacket guess is right */
> >> } else if ((udev->speed == USB_SPEED_FULL ||
> >> udev->speed == USB_SPEED_HIGH) &&
> >> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >> dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> >> udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> >> usb_ep0_reinit(udev);
> >> + } else if (udev->speed >= USB_SPEED_SUPER && initial) {
> >> + /* FIXME: should be more restrictive? */
> >> + /* Initial guess is wrong; use the descriptor's value */
> >> + dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> >> + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> >> + usb_ep0_reinit(udev);
> >
> > This could be merged with the previous case fairly easily.
> >
> >> } else {
> >> /* Initial guess is wrong and descriptor's value is invalid */
> >> - dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> >> + dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
> >
> > This also looks like a remnant from earlier testing.
> >
> > Alan Stern
> >
> >> retval = -EMSGSIZE;
> >> goto fail;
> >> }
> >> --
> >> 2.35.3
> >>
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-09 14:56 ` Alan Stern
2024-04-22 17:33 ` Linux regression tracking (Thorsten Leemhuis)
@ 2024-04-22 19:22 ` Takashi Iwai
1 sibling, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2024-04-22 19:22 UTC (permalink / raw)
To: Alan Stern; +Cc: Oliver Neukum, USB list, Takashi Iwai
On Tue, 09 Apr 2024 16:56:53 +0200,
Alan Stern wrote:
>
> On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> > Hi,
> >
> > with the following device:
> >
> > Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> > Device Descriptor:
> > bLength 18
> > bDescriptorType 1
> > bcdUSB 3.00
> > bDeviceClass 0
> > bDeviceSubClass 0
> > bDeviceProtocol 0
> > bMaxPacketSize0 8
>
> A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8
> instead of 9? Presumably this thing never received a USB certification.
> Does the packaging use the USB logo?
IIUC from the report, this is a virtualization environment, no real
device:
https://bhyve.npulse.net/
Takashi
> > idVendor 0xfb5d
> > idProduct 0x0001
> > bcdDevice 0.00
> > iManufacturer 1 BHYVE
> > iProduct 2 HID Tablet
> > iSerial 3 01
> > bNumConfigurations 1
>
> Why on earth would an HID tablet need to run at SuperSpeed?
>
> > Binary Object Store Descriptor:
> > bLength 5
> > bDescriptorType 15
> > wTotalLength 0x000f
> > bNumDeviceCaps 1
> > SuperSpeed USB Device Capability:
> > bLength 10
> > bDescriptorType 16
> > bDevCapabilityType 3
> > bmAttributes 0x00
> > wSpeedsSupported 0x0008
> > Device can operate at SuperSpeed (5Gbps)
> > bFunctionalitySupport 3
> > Lowest fully-functional device speed is SuperSpeed (5Gbps)
> > bU1DevExitLat 10 micro seconds
> > bU2DevExitLat 32 micro seconds
>
> A tablet not capable of running at any speed below 5 Gbps?
>
> > we are getting a regression on enumeration. It used to work with the
> > code prior to your patch. Takashi is proposing the attached fixed.
> > It looks like we are a bit too restrictive and should just try.
> >
> > Regards
> > Oliver
>
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
> > speed
> > Patch-mainline: Not yet, testing
> > References: bsc#1220569
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >
> > ---
> > drivers/usb/core/hub.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index e38a4124f610..64193effc456 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > const char *driver_name;
> > bool do_new_scheme;
> > const bool initial = !dev_descr;
> > - int maxp0;
> > + int maxp0, ep0_maxp;
> > struct usb_device_descriptor *buf, *descr;
> >
> > buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> > @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > else
> > i = 0; /* Invalid */
> > }
> > - if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> > + ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> > + if (ep0_maxp == i) {
>
> This variable looks like it was left over from earlier testing. It's
> not really needed.
>
> > ; /* Initial ep0 maxpacket guess is right */
> > } else if ((udev->speed == USB_SPEED_FULL ||
> > udev->speed == USB_SPEED_HIGH) &&
> > @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> > udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> > usb_ep0_reinit(udev);
> > + } else if (udev->speed >= USB_SPEED_SUPER && initial) {
> > + /* FIXME: should be more restrictive? */
> > + /* Initial guess is wrong; use the descriptor's value */
> > + dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> > + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> > + usb_ep0_reinit(udev);
>
> This could be merged with the previous case fairly easily.
>
> > } else {
> > /* Initial guess is wrong and descriptor's value is invalid */
> > - dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> > + dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
>
> This also looks like a remnant from earlier testing.
>
> Alan Stern
>
> > retval = -EMSGSIZE;
> > goto fail;
> > }
> > --
> > 2.35.3
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-22 18:03 ` Alan Stern
@ 2024-04-22 19:24 ` Takashi Iwai
2024-04-23 19:29 ` Alan Stern
0 siblings, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2024-04-22 19:24 UTC (permalink / raw)
To: Alan Stern
Cc: Linux regressions mailing list, Oliver Neukum, USB list, Takashi Iwai
On Mon, 22 Apr 2024 20:03:46 +0200,
Alan Stern wrote:
>
> On Mon, Apr 22, 2024 at 07:33:21PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> > for once, to make this easily accessible to everyone.
> >
> > Is anyone still working on fixing below regression? From here it looks
> > stalled, but I might have missed something.
>
> I've been waiting to hear back from Oliver or Takashi. A revised patch
> taking my comments into account would be welcome; it should be a very
> small change (just one or two lines of code).
As posted in another mail, it's a virtualized environment.
Details are found in the original bug report
https://bugzilla.suse.com/show_bug.cgi?id=1220569
About the patch change: I appreciate if you cook it rather by
yourself since I'm not 100% sure what you suggested. I can
provide the reporter a test kernel with the patch for confirmation, of
course.
thanks,
Takashi
>
> Alan Stern
>
> > 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.
> >
> > #regzbot poke
> >
> > On 09.04.24 16:56, Alan Stern wrote:
> > > On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> > >> Hi,
> > >>
> > >> with the following device:
> > >>
> > >> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> > >> Device Descriptor:
> > >> bLength 18
> > >> bDescriptorType 1
> > >> bcdUSB 3.00
> > >> bDeviceClass 0
> > >> bDeviceSubClass 0
> > >> bDeviceProtocol 0
> > >> bMaxPacketSize0 8
> > >
> > > A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8
> > > instead of 9? Presumably this thing never received a USB certification.
> > > Does the packaging use the USB logo?
> > >
> > >> idVendor 0xfb5d
> > >> idProduct 0x0001
> > >> bcdDevice 0.00
> > >> iManufacturer 1 BHYVE
> > >> iProduct 2 HID Tablet
> > >> iSerial 3 01
> > >> bNumConfigurations 1
> > >
> > > Why on earth would an HID tablet need to run at SuperSpeed?
> > >
> > >> Binary Object Store Descriptor:
> > >> bLength 5
> > >> bDescriptorType 15
> > >> wTotalLength 0x000f
> > >> bNumDeviceCaps 1
> > >> SuperSpeed USB Device Capability:
> > >> bLength 10
> > >> bDescriptorType 16
> > >> bDevCapabilityType 3
> > >> bmAttributes 0x00
> > >> wSpeedsSupported 0x0008
> > >> Device can operate at SuperSpeed (5Gbps)
> > >> bFunctionalitySupport 3
> > >> Lowest fully-functional device speed is SuperSpeed (5Gbps)
> > >> bU1DevExitLat 10 micro seconds
> > >> bU2DevExitLat 32 micro seconds
> > >
> > > A tablet not capable of running at any speed below 5 Gbps?
> > >
> > >> we are getting a regression on enumeration. It used to work with the
> > >> code prior to your patch. Takashi is proposing the attached fixed.
> > >> It looks like we are a bit too restrictive and should just try.
> > >>
> > >> Regards
> > >> Oliver
> > >
> > >> From: Takashi Iwai <tiwai@suse.de>
> > >> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
> > >> speed
> > >> Patch-mainline: Not yet, testing
> > >> References: bsc#1220569
> > >>
> > >> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > >>
> > >> ---
> > >> drivers/usb/core/hub.c | 13 ++++++++++---
> > >> 1 file changed, 10 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > >> index e38a4124f610..64193effc456 100644
> > >> --- a/drivers/usb/core/hub.c
> > >> +++ b/drivers/usb/core/hub.c
> > >> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > >> const char *driver_name;
> > >> bool do_new_scheme;
> > >> const bool initial = !dev_descr;
> > >> - int maxp0;
> > >> + int maxp0, ep0_maxp;
> > >> struct usb_device_descriptor *buf, *descr;
> > >>
> > >> buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> > >> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > >> else
> > >> i = 0; /* Invalid */
> > >> }
> > >> - if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> > >> + ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> > >> + if (ep0_maxp == i) {
> > >
> > > This variable looks like it was left over from earlier testing. It's
> > > not really needed.
> > >
> > >> ; /* Initial ep0 maxpacket guess is right */
> > >> } else if ((udev->speed == USB_SPEED_FULL ||
> > >> udev->speed == USB_SPEED_HIGH) &&
> > >> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > >> dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> > >> udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> > >> usb_ep0_reinit(udev);
> > >> + } else if (udev->speed >= USB_SPEED_SUPER && initial) {
> > >> + /* FIXME: should be more restrictive? */
> > >> + /* Initial guess is wrong; use the descriptor's value */
> > >> + dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> > >> + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> > >> + usb_ep0_reinit(udev);
> > >
> > > This could be merged with the previous case fairly easily.
> > >
> > >> } else {
> > >> /* Initial guess is wrong and descriptor's value is invalid */
> > >> - dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> > >> + dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
> > >
> > > This also looks like a remnant from earlier testing.
> > >
> > > Alan Stern
> > >
> > >> retval = -EMSGSIZE;
> > >> goto fail;
> > >> }
> > >> --
> > >> 2.35.3
> > >>
> > >
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-22 19:24 ` Takashi Iwai
@ 2024-04-23 19:29 ` Alan Stern
2024-04-24 8:56 ` Takashi Iwai
0 siblings, 1 reply; 16+ messages in thread
From: Alan Stern @ 2024-04-23 19:29 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Linux regressions mailing list, Oliver Neukum, USB list
On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> On Mon, 22 Apr 2024 20:03:46 +0200,
> Alan Stern wrote:
> >
> > On Mon, Apr 22, 2024 at 07:33:21PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> > > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> > > for once, to make this easily accessible to everyone.
> > >
> > > Is anyone still working on fixing below regression? From here it looks
> > > stalled, but I might have missed something.
> >
> > I've been waiting to hear back from Oliver or Takashi. A revised patch
> > taking my comments into account would be welcome; it should be a very
> > small change (just one or two lines of code).
>
> As posted in another mail, it's a virtualized environment.
> Details are found in the original bug report
> https://bugzilla.suse.com/show_bug.cgi?id=1220569
Hmmm. If this is a virtualized device, isn't the best solution to fix the
emulation code for the device so that it presents a valid descriptor?
> About the patch change: I appreciate if you cook it rather by
> yourself since I'm not 100% sure what you suggested. I can
> provide the reporter a test kernel with the patch for confirmation, of
> course.
Here's a condensed version of the patch you wrote. But I would prefer not
to add this to the kernel if the problem can be fixed somewhere else.
Alan Stern
Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -5110,9 +5110,10 @@ hub_port_init(struct usb_hub *hub, struc
}
if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
; /* Initial ep0 maxpacket guess is right */
- } else if ((udev->speed == USB_SPEED_FULL ||
+ } else if (((udev->speed == USB_SPEED_FULL ||
udev->speed == USB_SPEED_HIGH) &&
- (i == 8 || i == 16 || i == 32 || i == 64)) {
+ (i == 8 || i == 16 || i == 32 || i == 64)) ||
+ (udev->speed >= USB_SPEED_SUPER && i > 0)) {
/* Initial guess is wrong; use the descriptor's value */
if (udev->speed == USB_SPEED_FULL)
dev_dbg(&udev->dev, "ep0 maxpacket = %d\n", i);
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-23 19:29 ` Alan Stern
@ 2024-04-24 8:56 ` Takashi Iwai
2024-04-24 14:14 ` Alan Stern
2024-04-28 7:57 ` Takashi Iwai
0 siblings, 2 replies; 16+ messages in thread
From: Takashi Iwai @ 2024-04-24 8:56 UTC (permalink / raw)
To: Alan Stern
Cc: Takashi Iwai, Linux regressions mailing list, Oliver Neukum, USB list
On Tue, 23 Apr 2024 21:29:27 +0200,
Alan Stern wrote:
>
> On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > On Mon, 22 Apr 2024 20:03:46 +0200,
> > Alan Stern wrote:
> > >
> > > On Mon, Apr 22, 2024 at 07:33:21PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> > > > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> > > > for once, to make this easily accessible to everyone.
> > > >
> > > > Is anyone still working on fixing below regression? From here it looks
> > > > stalled, but I might have missed something.
> > >
> > > I've been waiting to hear back from Oliver or Takashi. A revised patch
> > > taking my comments into account would be welcome; it should be a very
> > > small change (just one or two lines of code).
> >
> > As posted in another mail, it's a virtualized environment.
> > Details are found in the original bug report
> > https://bugzilla.suse.com/show_bug.cgi?id=1220569
>
> Hmmm. If this is a virtualized device, isn't the best solution to fix the
> emulation code for the device so that it presents a valid descriptor?
Honestly speaking, I don't know, but it smells like a hard way. After
all, we brought some regression of the previously (even casually /
unintended) working "device"...
> > About the patch change: I appreciate if you cook it rather by
> > yourself since I'm not 100% sure what you suggested. I can
> > provide the reporter a test kernel with the patch for confirmation, of
> > course.
>
> Here's a condensed version of the patch you wrote. But I would prefer not
> to add this to the kernel if the problem can be fixed somewhere else.
Thanks, I asked the report for testing the patch now.
thanks,
Takashi
>
> Alan Stern
>
>
>
> Index: usb-devel/drivers/usb/core/hub.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/hub.c
> +++ usb-devel/drivers/usb/core/hub.c
> @@ -5110,9 +5110,10 @@ hub_port_init(struct usb_hub *hub, struc
> }
> if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> ; /* Initial ep0 maxpacket guess is right */
> - } else if ((udev->speed == USB_SPEED_FULL ||
> + } else if (((udev->speed == USB_SPEED_FULL ||
> udev->speed == USB_SPEED_HIGH) &&
> - (i == 8 || i == 16 || i == 32 || i == 64)) {
> + (i == 8 || i == 16 || i == 32 || i == 64)) ||
> + (udev->speed >= USB_SPEED_SUPER && i > 0)) {
> /* Initial guess is wrong; use the descriptor's value */
> if (udev->speed == USB_SPEED_FULL)
> dev_dbg(&udev->dev, "ep0 maxpacket = %d\n", i);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-24 8:56 ` Takashi Iwai
@ 2024-04-24 14:14 ` Alan Stern
2024-04-24 14:22 ` Takashi Iwai
2024-04-28 7:57 ` Takashi Iwai
1 sibling, 1 reply; 16+ messages in thread
From: Alan Stern @ 2024-04-24 14:14 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Linux regressions mailing list, Oliver Neukum, USB list
On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> On Tue, 23 Apr 2024 21:29:27 +0200,
> Alan Stern wrote:
> >
> > On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > > As posted in another mail, it's a virtualized environment.
> > > Details are found in the original bug report
> > > https://bugzilla.suse.com/show_bug.cgi?id=1220569
> >
> > Hmmm. If this is a virtualized device, isn't the best solution to fix the
> > emulation code for the device so that it presents a valid descriptor?
>
> Honestly speaking, I don't know, but it smells like a hard way. After
> all, we brought some regression of the previously (even casually /
> unintended) working "device"...
Still, it would be good to report the incorrect descriptor to the package's
maintainer.
Alan Stern
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-24 14:14 ` Alan Stern
@ 2024-04-24 14:22 ` Takashi Iwai
2024-04-24 14:56 ` Alan Stern
0 siblings, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2024-04-24 14:22 UTC (permalink / raw)
To: Alan Stern
Cc: Takashi Iwai, Linux regressions mailing list, Oliver Neukum, USB list
On Wed, 24 Apr 2024 16:14:23 +0200,
Alan Stern wrote:
>
> On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> > On Tue, 23 Apr 2024 21:29:27 +0200,
> > Alan Stern wrote:
> > >
> > > On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > > > As posted in another mail, it's a virtualized environment.
> > > > Details are found in the original bug report
> > > > https://bugzilla.suse.com/show_bug.cgi?id=1220569
> > >
> > > Hmmm. If this is a virtualized device, isn't the best solution to fix the
> > > emulation code for the device so that it presents a valid descriptor?
> >
> > Honestly speaking, I don't know, but it smells like a hard way. After
> > all, we brought some regression of the previously (even casually /
> > unintended) working "device"...
>
> Still, it would be good to report the incorrect descriptor to the package's
> maintainer.
Well, it's no Linux package.
Takashi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-24 14:22 ` Takashi Iwai
@ 2024-04-24 14:56 ` Alan Stern
2024-04-24 15:07 ` Takashi Iwai
0 siblings, 1 reply; 16+ messages in thread
From: Alan Stern @ 2024-04-24 14:56 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Linux regressions mailing list, Oliver Neukum, USB list
On Wed, Apr 24, 2024 at 04:22:44PM +0200, Takashi Iwai wrote:
> On Wed, 24 Apr 2024 16:14:23 +0200,
> Alan Stern wrote:
> >
> > On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> > > On Tue, 23 Apr 2024 21:29:27 +0200,
> > > Alan Stern wrote:
> > > >
> > > > On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > > > > As posted in another mail, it's a virtualized environment.
> > > > > Details are found in the original bug report
> > > > > https://bugzilla.suse.com/show_bug.cgi?id=1220569
> > > >
> > > > Hmmm. If this is a virtualized device, isn't the best solution to fix the
> > > > emulation code for the device so that it presents a valid descriptor?
> > >
> > > Honestly speaking, I don't know, but it smells like a hard way. After
> > > all, we brought some regression of the previously (even casually /
> > > unintended) working "device"...
> >
> > Still, it would be good to report the incorrect descriptor to the package's
> > maintainer.
>
> Well, it's no Linux package.
Doesn't matter. Bugs should always be reported when possible, for any kind
of package. Especially after you spent so much time and effort to track
this one down.
The fact that xz isn't specifically a Windows package didn't prevent Andres
Freund from reporting the recent backdoor problem.
Alan Stern
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-24 14:56 ` Alan Stern
@ 2024-04-24 15:07 ` Takashi Iwai
0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2024-04-24 15:07 UTC (permalink / raw)
To: Alan Stern
Cc: Takashi Iwai, Linux regressions mailing list, Oliver Neukum, USB list
On Wed, 24 Apr 2024 16:56:53 +0200,
Alan Stern wrote:
>
> On Wed, Apr 24, 2024 at 04:22:44PM +0200, Takashi Iwai wrote:
> > On Wed, 24 Apr 2024 16:14:23 +0200,
> > Alan Stern wrote:
> > >
> > > On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> > > > On Tue, 23 Apr 2024 21:29:27 +0200,
> > > > Alan Stern wrote:
> > > > >
> > > > > On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > > > > > As posted in another mail, it's a virtualized environment.
> > > > > > Details are found in the original bug report
> > > > > > https://bugzilla.suse.com/show_bug.cgi?id=1220569
> > > > >
> > > > > Hmmm. If this is a virtualized device, isn't the best solution to fix the
> > > > > emulation code for the device so that it presents a valid descriptor?
> > > >
> > > > Honestly speaking, I don't know, but it smells like a hard way. After
> > > > all, we brought some regression of the previously (even casually /
> > > > unintended) working "device"...
> > >
> > > Still, it would be good to report the incorrect descriptor to the package's
> > > maintainer.
> >
> > Well, it's no Linux package.
>
> Doesn't matter. Bugs should always be reported when possible, for any kind
> of package. Especially after you spent so much time and effort to track
> this one down.
>
> The fact that xz isn't specifically a Windows package didn't prevent Andres
> Freund from reporting the recent backdoor problem.
Yeah, sure. Any people can report to https://bhyve.npulse.net/
Takashi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-24 8:56 ` Takashi Iwai
2024-04-24 14:14 ` Alan Stern
@ 2024-04-28 7:57 ` Takashi Iwai
2024-04-29 13:28 ` Alan Stern
1 sibling, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2024-04-28 7:57 UTC (permalink / raw)
To: Alan Stern; +Cc: Linux regressions mailing list, Oliver Neukum, USB list
On Wed, 24 Apr 2024 10:56:11 +0200,
Takashi Iwai wrote:
>
> On Tue, 23 Apr 2024 21:29:27 +0200,
> Alan Stern wrote:
> >
> > On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > > On Mon, 22 Apr 2024 20:03:46 +0200,
> > > Alan Stern wrote:
> > > About the patch change: I appreciate if you cook it rather by
> > > yourself since I'm not 100% sure what you suggested. I can
> > > provide the reporter a test kernel with the patch for confirmation, of
> > > course.
> >
> > Here's a condensed version of the patch you wrote. But I would prefer not
> > to add this to the kernel if the problem can be fixed somewhere else.
>
> Thanks, I asked the report for testing the patch now.
The reporter confirmed that it's still working.
thanks,
Takashi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-28 7:57 ` Takashi Iwai
@ 2024-04-29 13:28 ` Alan Stern
2024-04-30 8:02 ` Takashi Iwai
0 siblings, 1 reply; 16+ messages in thread
From: Alan Stern @ 2024-04-29 13:28 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Linux regressions mailing list, Oliver Neukum, USB list
On Sun, Apr 28, 2024 at 09:57:42AM +0200, Takashi Iwai wrote:
> On Wed, 24 Apr 2024 10:56:11 +0200,
> Takashi Iwai wrote:
> >
> > On Tue, 23 Apr 2024 21:29:27 +0200,
> > Alan Stern wrote:
> > >
> > > On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > > > On Mon, 22 Apr 2024 20:03:46 +0200,
> > > > Alan Stern wrote:
> > > > About the patch change: I appreciate if you cook it rather by
> > > > yourself since I'm not 100% sure what you suggested. I can
> > > > provide the reporter a test kernel with the patch for confirmation, of
> > > > course.
> > >
> > > Here's a condensed version of the patch you wrote. But I would prefer not
> > > to add this to the kernel if the problem can be fixed somewhere else.
> >
> > Thanks, I asked the report for testing the patch now.
>
> The reporter confirmed that it's still working.
Can you get a Reported-and-Tested-by: from the reporter?
Alan Stern
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
2024-04-29 13:28 ` Alan Stern
@ 2024-04-30 8:02 ` Takashi Iwai
2024-04-30 14:33 ` [PATCH] usb: Fix regression caused by invalid ep0 maxpacket in virtual SuperSpeed device Alan Stern
0 siblings, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2024-04-30 8:02 UTC (permalink / raw)
To: Alan Stern
Cc: Takashi Iwai, Linux regressions mailing list, Oliver Neukum, USB list
On Mon, 29 Apr 2024 15:28:59 +0200,
Alan Stern wrote:
>
> On Sun, Apr 28, 2024 at 09:57:42AM +0200, Takashi Iwai wrote:
> > On Wed, 24 Apr 2024 10:56:11 +0200,
> > Takashi Iwai wrote:
> > >
> > > On Tue, 23 Apr 2024 21:29:27 +0200,
> > > Alan Stern wrote:
> > > >
> > > > On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > > > > On Mon, 22 Apr 2024 20:03:46 +0200,
> > > > > Alan Stern wrote:
> > > > > About the patch change: I appreciate if you cook it rather by
> > > > > yourself since I'm not 100% sure what you suggested. I can
> > > > > provide the reporter a test kernel with the patch for confirmation, of
> > > > > course.
> > > >
> > > > Here's a condensed version of the patch you wrote. But I would prefer not
> > > > to add this to the kernel if the problem can be fixed somewhere else.
> > >
> > > Thanks, I asked the report for testing the patch now.
> >
> > The reporter confirmed that it's still working.
>
> Can you get a Reported-and-Tested-by: from the reporter?
Yes, I got it from bugzilla entry, please put
Reported-and-tested-by: Roger Whittaker <roger.whittaker@suse.com>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1220569
Thanks!
Takashi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] usb: Fix regression caused by invalid ep0 maxpacket in virtual SuperSpeed device
2024-04-30 8:02 ` Takashi Iwai
@ 2024-04-30 14:33 ` Alan Stern
0 siblings, 0 replies; 16+ messages in thread
From: Alan Stern @ 2024-04-30 14:33 UTC (permalink / raw)
To: Greg KH; +Cc: Takashi Iwai, Roger Whittaker, Oliver Neukum, USB list
A virtual SuperSpeed device in the FreeBSD BVCP package
(https://bhyve.npulse.net/) presents an invalid ep0 maxpacket size of 256.
It stopped working with Linux following a recent commit because now we
check these sizes more carefully than before.
Fix this regression by using the bMaxpacketSize0 value in the device
descriptor for SuperSpeed or faster devices, even if it is invalid. This
is a very simple-minded change; we might want to check more carefully for
values that actually make some sense (for instance, no smaller than 64).
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Roger Whittaker <roger.whittaker@suse.com>
Closes: https://bugzilla.suse.com/show_bug.cgi?id=1220569
Link: https://lore.kernel.org/linux-usb/9efbd569-7059-4575-983f-0ea30df41871@suse.com/
Fixes: 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
Cc: stable@vger.kernel.org
---
drivers/usb/core/hub.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -5110,9 +5110,10 @@ hub_port_init(struct usb_hub *hub, struc
}
if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
; /* Initial ep0 maxpacket guess is right */
- } else if ((udev->speed == USB_SPEED_FULL ||
+ } else if (((udev->speed == USB_SPEED_FULL ||
udev->speed == USB_SPEED_HIGH) &&
- (i == 8 || i == 16 || i == 32 || i == 64)) {
+ (i == 8 || i == 16 || i == 32 || i == 64)) ||
+ (udev->speed >= USB_SPEED_SUPER && i > 0)) {
/* Initial guess is wrong; use the descriptor's value */
if (udev->speed == USB_SPEED_FULL)
dev_dbg(&udev->dev, "ep0 maxpacket = %d\n", i);
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-04-30 14:33 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-09 13:49 Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization") Oliver Neukum
2024-04-09 14:56 ` Alan Stern
2024-04-22 17:33 ` Linux regression tracking (Thorsten Leemhuis)
2024-04-22 18:03 ` Alan Stern
2024-04-22 19:24 ` Takashi Iwai
2024-04-23 19:29 ` Alan Stern
2024-04-24 8:56 ` Takashi Iwai
2024-04-24 14:14 ` Alan Stern
2024-04-24 14:22 ` Takashi Iwai
2024-04-24 14:56 ` Alan Stern
2024-04-24 15:07 ` Takashi Iwai
2024-04-28 7:57 ` Takashi Iwai
2024-04-29 13:28 ` Alan Stern
2024-04-30 8:02 ` Takashi Iwai
2024-04-30 14:33 ` [PATCH] usb: Fix regression caused by invalid ep0 maxpacket in virtual SuperSpeed device Alan Stern
2024-04-22 19:22 ` Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization") Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.