Bug 220033
Summary: | xhci: Compliance Issue - avg_trb_len not set for EP0 during Address Device Command | ||
---|---|---|---|
Product: | Drivers | Reporter: | Chen-Tzu-Chieh (jay.chen) |
Component: | USB | Assignee: | Default virtual assignee for Drivers/USB (drivers_usb) |
Status: | NEW --- | ||
Severity: | low | CC: | jay.chen, mathias.nyman, michal.pecio |
Priority: | P3 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | kernel 6.15-rc2 (self-built vanilla, no taint, config: x86_64 defconfig with KGDB enabled) | Subsystem: | |
Regression: | No | Bisected commit-id: |
Description
Chen-Tzu-Chieh
2025-04-18 13:31:17 UTC
Just checking in to see if there's any update needed from my side. Happy to provide more info or run tests. This is up for interpretation, spec is ambiguous xhci 1.2 Section 6.2.3.1 "Address Device Command usage" does not mention Average TRB Length at all. But section 4.8.2 "Endpoint Context Initialization" states that: "All fields of an Input Endpoint Context data structure (including the Reserved fields) shall be initialized to ‘0’ with the following exceptions: 4.8.2.1 Default Control Endpoint 0 - Max Packet Size - CErr - TR Dequeue Pointer - Dequeue Cycle State (DCS) According to it the Average TRB Length should be initialized to 0 I don't object to setting the Average TRB Length earlier, especially if it solves device enumeration issues for some xHCI vendor. We do need to make sure it doesn't brake enumeration for other vendors. Can you submit a patch to linux-usb mailing list for this? (In reply to Chen-Tzu-Chieh from comment #0) > Some xHCI hardware vendors may validate the Input Context at Address Device > time and reject contexts with invalid values, potentially causing device > enumeration issues. A scarier (and more likely?) possibility is HCs failing to validate this field and yet assuming that it's non-zero, then dividing by zero or doing some other stupid thing and crashing and burning. Bonus if it only happens once in a blue moon. But as Mathias found, the spec is self-contradictory, so it works both ways... > While xhci_endpoint_init() later sets avg_trb_len correctly, Are you sure? ;) This function is only called from add_endpoint(), which doesn't seem to ever be called on EP 0. But non-default control endpoints would be set to 8 indeed. Hi Mathias & Michał, Thanks for your response. I’ve already submitted a patch to fix this situation (by adding a line of `ep0_ctx->tx_info |= cpu_to_le32(EP_AVG_TRB_LENGTH(8));` in `xhci_setup_addressable_virt_dev`). Link: https://lore.kernel.org/linux-usb/JH0PR06MB7294E46B393F1CA5FE0EE4F78396A@JH0PR06MB7294.apcprd06.prod.outlook.com/T/#u > This function is only called from add_endpoint(), which doesn't seem to ever > be called on EP 0. But non-default control endpoints would be set to 8 > indeed. Yes, I misunderstood that function, and thanks for the explanation. Inside `xhci_endpoint_init`, it sets `avg_trb_len` for the USB device's endpoints while the `xhci_setup_addressable_virt_dev` function initializes the input context (ref: xHCI 1.2, Ch. 6.2.5 Input Context), and EP Context 0 (Default Control Endpoint) is passed to the xHC hardware. |