lkml.org 
[lkml]   [2018]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v4 1/3] usb: host: Implement workaround for Erratum A-007463
From
Date
On 19.12.2017 12:16, yinbo.zhu@nxp.com wrote:
> From: yinbo.zhu <yinbo.zhu@nxp.com>
>
> When a transaction error (defined in Section 4.10.2.3, "USB
> Transaction Error" of the xHCI Specification) occurs on the
> USB, the host controller reports this through a transfer
> event with the completion code "USB Transaction Error". When
> this happens, the endpoint is placed in the Halted state. In
> response, software must issue a Reset Endpoint command to
> transition the endpoint to the Stopped state. In order to
> restart the transfer, the driver can perform either of the
> following:
> a) Ring the doorbell again, which restarts the transfer from
> where it stopped, or
> b) Issue a Set TR (Transfer Ring) Dequeue Pointer command
> for the endpoint to start the transfer form a different
> Transfer Ring pointer Consider the following
> scenario:
> 1. The xHCI driver prepares a control transfer read to one
> of the device's control endpoints;
> 2. During the IN data stage, a transaction error occurs on
> the USB, causing a transfer event with the completion
> code "USB Transaction Error";
> 3. The driver issues a Reset Endpoint command;
> 4. The driver rings the doorbell of the control endpoint to
> resume the transfer. In this scenario, the controller
> may reverse the direction of the data stage from IN to OUT.

The xhci driver should not ring the doorbell without first issuing a
Set TR Dequeue Pointer command in the USB Transaction error case.

code flow:

handle_tx_event()
case COMP_USB_TRANSACTION_ERROR:
status = -EPROTO;
process_ctrl_td()
switch (trb_comp_code)
default:
if (!xhci_requires_manual_halt_cleanup() // FALSE
break;
/* else fall through */
case COMP_STALL_ERROR:
finish_td()
if (xhci_requires_manual_halt_cleanup()) // TRUE
xhci_cleanup_halted_endpoint(...,EP_HARD_RESET)
xhci_queue_reset_ep()
xhci_cleanup_stalled_ring()
xhci_find_new_dequeue_state()
xhci_queue_new_dequeue_state()

If you can see the driver ringing the doorbell before the
Set TR Dequeue Pointer command then there is some race going
on in the driver

> Instead of sending an ACK to the endpoint to poll for read
> data, it sends a Data Packet (DP) to the endpoint. It
> fetches the data from the data stage Transfer Request
> Block (TRB) that is being resumed, even though the data
> buffer is setup to receive data and not transmit it.
> NOTE
> This issue occurs only if the transaction error happens
> during an IN data stage. There is no issue if the transaction
> error happens during an OUT data stage.

Have you been able to trigger this issue?

>
> Impact: When this issue occurs, the device likely responds in
> one of the following ways:
> a) The device responds with a STALL because the data stage
> has unexpectedly changed directions. The controller then
> generates a Stall Error transfer event, to which software
> must issue a Reset Endpoint command followed by a Set TR
> Dequeue Pointer command pointing to a new Setup TRB to clear
> the STALL condition.
> b) The device does not respond to the inverted data stage and
> the transaction times out. The controller generates another
> USB Transaction Error transfer event, to which software
> likely performs a USB Reset to the device because it is
> unresponsive. It is not expected that any of these recovery
> steps will cause instability in the system because this
> recovery is part of a standard xHCI driver and could happen
> regardless of the defect. Another possible system-level
> impact is that the controller attempts to read from the
> memory location pointed at by the Data Stage TRB or a Normal
> TRB chained to it. associated with this TRB is intended to be
> written by the controller, but the controller reads from it
> instead. Normally, this does not cause a problem. However, if
> the system has some type of memory protection where this
> unexpected read is treated as a bus error,
> a problem. However, if the system has some type of memory
> it may cause the system to become unstable or to crash.
>
> Workaround: If a USB Transaction Error occurs during the IN
> data phase of a control transfer, the driver must use the
> Set TR Dequeue Pointer command to either restart the data
> Phase or restart the entire control transfer from the
> Setup phase.

The Set TR Dequeue pointer command should already be the default way
the xhci driver handles this now.

>
> Configs Affected:
> LS1021-20-22A-R1.0, LS1021-20-22A-R2.007463
>
> Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
> ---


> @@ -1951,14 +1953,30 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
> if (trb_comp_code == COMP_STALL_ERROR ||
> xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
> trb_comp_code)) {
> - /* Issue a reset endpoint command to clear the host side
> - * halt, followed by a set dequeue command to move the
> - * dequeue pointer past the TD.
> - * The class driver clears the device side halt later.
> + /*erratum A-007463:
> + *After transaction error, controller switches control transfer
> + *data stage from IN to OUT direction.
> */
> - xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
> + remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
> + if (remaining && xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
> + trb_comp_code) &&
> + (xhci->quirks & XHCI_REVERSE_IN_OUT)) {
> + memset(&deq_state, 0, sizeof(deq_state));
> + xhci_find_new_dequeue_state(xhci, slot_id,
> + ep_index, td->urb->stream_id, td, &deq_state);
> + xhci_queue_new_dequeue_state(xhci, slot_id, ep_index,
> + &deq_state);
> + xhci_ring_cmd_db(xhci);

Even if all these changes should be unnecessary the endpoint is still halted
here, and we would need a reset endpoint command here as well, right?

Has this new codepath been tested?

> + } else {
> + /* Issue a reset endpoint command to clear the host side
> + * halt, followed by a set dequeue command to move the
> + * dequeue pointer past the TD.
> + * The class driver clears the device side halt later.
> + */
> + xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
> ep_ring->stream_id, td, ep_trb,
> EP_HARD_RESET);

-Mathias

\
 
 \ /
  Last update: 2018-01-04 14:36    [W:1.071 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site