arduino / ArduinoCore-avr

The Official Arduino AVR core
https://www.arduino.cc
1.25k stars 1.06k forks source link

USB enumeration timeouts due to missing ZLPs #553

Open tlyu opened 9 months ago

tlyu commented 9 months ago

USBCore doesn't send required ZLPs for device-to-host transfers. This can cause timeouts during enumeration, among other problems.

Test case outline: create a HID interface with a report descriptor that's exactly 64 bytes long. Attach the device to Windows while monitoring the USB bus. Observe multiple control transfer timeouts and bus resets as Windows tries to get the device to enumerate correctly.

For whatever reason, Windows often (always?) requests 64 bytes more than were advertised for a HID report descriptor. This means that if the descriptor is an exact multiple of wMaxPacketSize, and the device sends fewer bytes than requested by the host, the device must send a ZLP to terminate the transfer. (see USB 2.0 §5.5.3) The missing ZLP causes Windows to time out.

It's also possible to replicate the timeout on other platforms (tested on macOS) by explicitly requesting a control read for more than 64 bytes to retrieve a descriptor that is exactly 64 bytes long.

I don't have a minimized test case yet, but the patch in keyboardio/Kaleidoscope-Bundle-Keyboardio#53 appears to resolve the problem for us.

It's a bit surprising that nobody has (knowingly?) encountered this before. It could also cause problems for configuration descriptor sets or string descriptors.

I can submit the patch as a PR against this repository directly, but I don't currently have a lot of time to create a minimized test case or to do exhaustive testing of the patch.

per1234 commented 9 months ago

Hi @tlyu. Thanks for your report! Is this the same bug that was previously reported here: https://github.com/arduino/ArduinoCore-avr/issues/112 ?

tlyu commented 9 months ago

No. #112 is a related issue with receiving ZLPs in non-control transactions, very likely fixed by the combination of arduino/Arduino#6886 and keyboardio/Kaleidoscope-Bundle-Keyboardio#51. Note that arduino/Arduino#6886 seems to have been stalled for years due to lack of a signed CLA.

tlyu commented 9 months ago

Minimal test case: With an Arduino Micro, edit the USB product string in boards.txt to be

Arduino Micro f0123456789abcdef

which is 31 ASCII characters long, and produces a string descriptor exactly 64 bytes long. Compile and upload any sketch that enables the CDC interface (it's enabled by default, I think). That will demonstrate the bug.

I've tested the above case via manual control read on macOS of more bytes than are in the descriptor. On Windows, there is a 5-second timeout on an initial attempt to fetch the string descriptor by requesting 255 bytes. Subsequent requests for the string descriptor do provide correct lengths, and succeed.

(For a HID report descriptor, a timeout will cause Windows to ignore that HID interface, but that's harder to produce a minimal test case for.)

tlyu commented 9 months ago

Demo script, using the Python usb1 module (requires libusb):

import usb1

with usb1.USBContext() as uc:
    # uc.setDebug(usb1.LOG_LEVEL_DEBUG)
    dh = uc.openByVendorIDAndProductID(0x2341, 0x8037)
    b1 = dh.controlRead(0x80, 6, 0x0302, 0, 64, 100)
    print(f"requested 64 bytes, got {len(b1)}")
    b2 = dh.controlRead(0x80, 6, 0x0302, 0, 255, 100)
    print(f"requested 255 bytes, got {len(b2)}")

Failure output:

requested 64 bytes, got 64
Traceback (most recent call last):
  File "/Users/tlyu/src/py-hid-utils/avrdesc-demo.py", line 8, in <module>
    b2 = dh.controlRead(0x80, 6, 0x0302, 0, 255, 100)
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 1350, in controlRead
    transferred = self._controlTransfer(
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 1307, in _controlTransfer
    mayRaiseUSBError(result)
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 127, in mayRaiseUSBError
    __raiseUSBError(value)
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 119, in raiseUSBError
    raise __STATUS_TO_EXCEPTION_DICT.get(value, __USBError)(value)
usb1.USBErrorTimeout: LIBUSB_ERROR_TIMEOUT [-7]

Verbose fail log (debug logging): avrdesc-fail-long.txt