Message ID | 20250516033908.7386-1-shawn2000100@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [v3] usb: xhci: Set avg_trb_len = 8 for EP0 during Address Device Command | expand |
On 16.5.2025 6.39, Jay Chen wrote: > According to the xHCI 1.2 spec (Section 6.2.3, p.454), the Average > TRB Length (avg_trb_len) for control endpoints should be set to 8. Maybe add here "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 > > Currently, during the Address Device Command, EP0's avg_trb_len remains 0, > which may cause some xHCI hardware to reject the Input Context, resulting > in device enumeration failures. In extreme cases, using a zero avg_trb_len > in calculations may lead to division-by-zero errors and unexpected system > crashes. Would be good to specify here which exact hardware requires avg_trb_len to be set before the 'Address Device Command'. This way we can later create a quirk for it in case it turns out other existing controllers can't handle it. So far it seems other hosts can handle it well, and quirks may not be needed at all. Thanks to Michał for testing. Thanks Mathias > > This patch sets avg_trb_len to 8 for EP0 in > xhci_setup_addressable_virt_dev(), ensuring compliance with the spec > and improving compatibility across various host controller implementations. I'd skip the 'compliance with spec..' part as spec is a bit unclear on this issue. Thanks Mathias
Hi Michał and Mathias, Thank you for the thoughtful feedback and review. > Only the xHC internal firmware could crash or misbehave from that. Yes, precisely. The potential crash or division-by-zero would happen within the internal firmware of certain host controllers, rather than in the Linux kernel itself. I'll make this clearer in the commit description as well. > The rest of ep0 tx_info is zero, so this could be = instead of |=. Agreed, I'll update the next revision of this patch to use '=' instead of '|=' for clarity and correctness. --- > I'd skip the 'compliance with spec..' part as spec is a bit unclear on this issue. Sure, I will modify the commit message accordingly. As noted, there is a subtle contradiction between two sections of the xHCI 1.2 specification: - 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. - Section 6.2.3 "Endpoint Context" (p.453) specifies that the Average TRB Length field shall be greater than ‘0’, and further notes (p.454): “Software shall set Average TRB Length to ‘8’ for control endpoints.” Strictly setting all fields to 0 during initialization conflicts with the explicit recommendation that avg_trb_len should be set to 8 for control endpoints. In practice, setting avg_trb_len = 0 is meaningless for the hardware/firmware, as it defeats the purpose of the field, which is used for transfer calculations and validation. Motivation / Real-world context: I am developing a custom Virtual xHC hardware platform that strictly follows the xHCI specification and its recommendations. During validation, we found that enumeration fails and a parameter error (TRB Completion Code = 5) is reported if avg_trb_len for EP0 is not set to 8 as recommended by Section 6.2.3. This behavior aligns with the spec's intent and highlights the importance of setting a meaningful, non-zero value for avg_trb_len, even in virtualized or emulated environments. Therefore, this patch is intended to better align Linux xHCI host controller driver behavior with the recommendation in Section 6.2.3, and to improve robustness and interoperability with both current and future xHCI implementations—real or virtual—that may enforce spec recommendations more strictly. Thanks again for your feedback. I'll prepare and submit a v4 patch shortly, incorporating your suggestions. Let me know if you have further questions or suggestions. Best regards, Jay Chen On Mon, May 19, 2025 at 9:14 PM Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > > On 16.5.2025 6.39, Jay Chen wrote: > > According to the xHCI 1.2 spec (Section 6.2.3, p.454), the Average > > TRB Length (avg_trb_len) for control endpoints should be set to 8. > > Maybe add here "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 > > > Currently, during the Address Device Command, EP0's avg_trb_len remains 0, > > which may cause some xHCI hardware to reject the Input Context, resulting > > in device enumeration failures. In extreme cases, using a zero avg_trb_len > > in calculations may lead to division-by-zero errors and unexpected system > > crashes. > > Would be good to specify here which exact hardware requires avg_trb_len to be > set before the 'Address Device Command'. This way we can later create a > quirk for it in case it turns out other existing controllers can't handle it. > > So far it seems other hosts can handle it well, and quirks may not be needed > at all. Thanks to Michał for testing. > > Thanks > Mathias > > > > > This patch sets avg_trb_len to 8 for EP0 in > > xhci_setup_addressable_virt_dev(), ensuring compliance with the spec > > and improving compatibility across various host controller implementations. > > I'd skip the 'compliance with spec..' part as spec is a bit unclear on this > issue. > > Thanks > Mathias >
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index d698095fc88d..fed9e9d1990c 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1166,6 +1166,8 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud ep0_ctx->deq = cpu_to_le64(dev->eps[0].ring->first_seg->dma | dev->eps[0].ring->cycle_state); + ep0_ctx->tx_info |= cpu_to_le32(EP_AVG_TRB_LENGTH(8)); + trace_xhci_setup_addressable_virt_device(dev); /* Steps 7 and 8 were done in xhci_alloc_virt_device() */
According to the xHCI 1.2 spec (Section 6.2.3, p.454), the Average TRB Length (avg_trb_len) for control endpoints should be set to 8. Currently, during the Address Device Command, EP0's avg_trb_len remains 0, which may cause some xHCI hardware to reject the Input Context, resulting in device enumeration failures. In extreme cases, using a zero avg_trb_len in calculations may lead to division-by-zero errors and unexpected system crashes. This patch sets avg_trb_len to 8 for EP0 in xhci_setup_addressable_virt_dev(), ensuring compliance with the spec and improving compatibility across various host controller implementations. v3: - Corrected author name in commit metadata and added changelog. v2: - Fixed malformed patch formatting issue pointed out by maintainer. Link: https://bugzilla.kernel.org/show_bug.cgi?id=220033 Signed-off-by: Jay Chen <shawn2000100@gmail.com> --- drivers/usb/host/xhci-mem.c | 2 ++ 1 file changed, 2 insertions(+)