xhci: dbc: Fix STALL transfer event handling

Don't flush all pending DbC data requests when an endpoint halts.

An endpoint may halt and xHC DbC triggers a STALL error event if there's
an issue with a bulk data transfer. The transfer should restart once xHC
DbC receives a ClearFeature(ENDPOINT_HALT) request from the host.

Once xHC DbC restarts it will start from the TRB pointed to by dequeue
field in the endpoint context, which might be the same TRB we got the
STALL event for. Turn the TRB to a no-op in this case to make sure xHC
DbC doesn't reuse and tries to retransmit this same TRB after we already
handled it, and gave its corresponding data request back.

Other STALL events might be completely bogus.
Lukasz Bartosik discovered that xHC DbC might issue spurious STALL events
if hosts sends a ClearFeature(ENDPOINT_HALT) request to non-halted
endpoints even without any active bulk transfers.

Assume STALL event is spurious if it reports 0 bytes transferred, and
the endpoint stopped on the STALLED TRB.
Don't give back the data request corresponding to the TRB in this case.

The halted status is per endpoint. Track it with a per endpoint flag
instead of the driver invented DbC wide DS_STALLED state.
DbC remains in DbC-Configured state even if endpoints halt. There is no
Stalled state in the DbC Port state Machine (xhci section 7.6.6)

Reported-by: Łukasz Bartosik <ukaszb@chromium.org>
Closes: https://lore.kernel.org/linux-usb/20240725074857.623299-1-ukaszb@chromium.org/
Tested-by: Łukasz Bartosik <ukaszb@chromium.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20240905143300.1959279-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Mathias Nyman and committed by

Greg Kroah-Hartman 2 years ago 9044ad57 9313d139

+83 -52

2 changed files

expand all

drivers

usb

host

xhci-dbgcap.c

xhci-dbgcap.h

+82 -51

drivers/usb/host/xhci-dbgcap.c

··· 173 173 spin_lock(&dbc->lock); 174 174 } 175 175 176 - static void xhci_dbc_flush_single_request(struct dbc_request *req) 176 + static void trb_to_noop(union xhci_trb *trb) 177 177 { 178 - union xhci_trb *trb = req->trb; 179 - 180 178 trb->generic.field[0] = 0; 181 179 trb->generic.field[1] = 0; 182 180 trb->generic.field[2] = 0; 183 181 trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); 184 182 trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP)); 183 + } 185 184 185 + static void xhci_dbc_flush_single_request(struct dbc_request *req) 186 + { 187 + trb_to_noop(req->trb); 186 188 xhci_dbc_giveback(req, -ESHUTDOWN); 187 189 } 188 190 ··· 651 649 case DS_DISABLED: 652 650 return; 653 651 case DS_CONFIGURED: 654 - case DS_STALLED: 655 652 if (dbc->driver->disconnect) 656 653 dbc->driver->disconnect(dbc); 657 654 break; ··· 668 667 669 668 xhci_dbc_mem_cleanup(dbc); 670 669 pm_runtime_put_sync(dbc->dev); /* note, was self.controller */ 670 + } 671 + 672 + static void 673 + handle_ep_halt_changes(struct xhci_dbc *dbc, struct dbc_ep *dep, bool halted) 674 + { 675 + if (halted) { 676 + dev_info(dbc->dev, "DbC Endpoint halted\n"); 677 + dep->halted = 1; 678 + 679 + } else if (dep->halted) { 680 + dev_info(dbc->dev, "DbC Endpoint halt cleared\n"); 681 + dep->halted = 0; 682 + 683 + if (!list_empty(&dep->list_pending)) 684 + writel(DBC_DOOR_BELL_TARGET(dep->direction), 685 + &dbc->regs->doorbell); 686 + } 671 687 } 672 688 673 689 static void ··· 715 697 struct xhci_ring *ring; 716 698 int ep_id; 717 699 int status; 700 + struct xhci_ep_ctx *ep_ctx; 718 701 u32 comp_code; 719 702 size_t remain_length; 720 703 struct dbc_request *req = NULL, *r; ··· 725 706 ep_id = TRB_TO_EP_ID(le32_to_cpu(event->generic.field[3])); 726 707 dep = (ep_id == EPID_OUT) ? 727 708 get_out_ep(dbc) : get_in_ep(dbc); 709 + ep_ctx = (ep_id == EPID_OUT) ? 710 + dbc_bulkout_ctx(dbc) : dbc_bulkin_ctx(dbc); 728 711 ring = dep->ring; 712 + 713 + /* Match the pending request: */ 714 + list_for_each_entry(r, &dep->list_pending, list_pending) { 715 + if (r->trb_dma == event->trans_event.buffer) { 716 + req = r; 717 + break; 718 + } 719 + if (r->status == -COMP_STALL_ERROR) { 720 + dev_warn(dbc->dev, "Give back stale stalled req\n"); 721 + ring->num_trbs_free++; 722 + xhci_dbc_giveback(r, 0); 723 + } 724 + } 725 + 726 + if (!req) { 727 + dev_warn(dbc->dev, "no matched request\n"); 728 + return; 729 + } 730 + 731 + trace_xhci_dbc_handle_transfer(ring, &req->trb->generic); 729 732 730 733 switch (comp_code) { 731 734 case COMP_SUCCESS: ··· 759 718 case COMP_TRB_ERROR: 760 719 case COMP_BABBLE_DETECTED_ERROR: 761 720 case COMP_USB_TRANSACTION_ERROR: 762 - case COMP_STALL_ERROR: 763 721 dev_warn(dbc->dev, "tx error %d detected\n", comp_code); 764 722 status = -comp_code; 765 723 break; 724 + case COMP_STALL_ERROR: 725 + dev_warn(dbc->dev, "Stall error at bulk TRB %llx, remaining %zu, ep deq %llx\n", 726 + event->trans_event.buffer, remain_length, ep_ctx->deq); 727 + status = 0; 728 + dep->halted = 1; 729 + 730 + /* 731 + * xHC DbC may trigger a STALL bulk xfer event when host sends a 732 + * ClearFeature(ENDPOINT_HALT) request even if there wasn't an 733 + * active bulk transfer. 734 + * 735 + * Don't give back this transfer request as hardware will later 736 + * start processing TRBs starting from this 'STALLED' TRB, 737 + * causing TRBs and requests to be out of sync. 738 + * 739 + * If STALL event shows some bytes were transferred then assume 740 + * it's an actual transfer issue and give back the request. 741 + * In this case mark the TRB as No-Op to avoid hw from using the 742 + * TRB again. 743 + */ 744 + 745 + if ((ep_ctx->deq & ~TRB_CYCLE) == event->trans_event.buffer) { 746 + dev_dbg(dbc->dev, "Ep stopped on Stalled TRB\n"); 747 + if (remain_length == req->length) { 748 + dev_dbg(dbc->dev, "Spurious stall event, keep req\n"); 749 + req->status = -COMP_STALL_ERROR; 750 + req->actual = 0; 751 + return; 752 + } 753 + dev_dbg(dbc->dev, "Give back stalled req, but turn TRB to No-op\n"); 754 + trb_to_noop(req->trb); 755 + } 756 + break; 757 + 766 758 default: 767 759 dev_err(dbc->dev, "unknown tx error %d\n", comp_code); 768 760 status = -comp_code; 769 761 break; 770 762 } 771 - 772 - /* Match the pending request: */ 773 - list_for_each_entry(r, &dep->list_pending, list_pending) { 774 - if (r->trb_dma == event->trans_event.buffer) { 775 - req = r; 776 - break; 777 - } 778 - } 779 - 780 - if (!req) { 781 - dev_warn(dbc->dev, "no matched request\n"); 782 - return; 783 - } 784 - 785 - trace_xhci_dbc_handle_transfer(ring, &req->trb->generic); 786 763 787 764 ring->num_trbs_free++; 788 765 req->actual = req->length - remain_length; ··· 821 762 static enum evtreturn xhci_dbc_do_handle_events(struct xhci_dbc *dbc) 822 763 { 823 764 dma_addr_t deq; 824 - struct dbc_ep *dep; 825 765 union xhci_trb *evt; 826 766 u32 ctrl, portsc; 827 767 bool update_erdp = false; ··· 872 814 return EVT_DISC; 873 815 } 874 816 875 - /* Handle endpoint stall event: */ 817 + /* Check and handle changes in endpoint halt status */ 876 818 ctrl = readl(&dbc->regs->control); 877 - if ((ctrl & DBC_CTRL_HALT_IN_TR) || 878 - (ctrl & DBC_CTRL_HALT_OUT_TR)) { 879 - dev_info(dbc->dev, "DbC Endpoint stall\n"); 880 - dbc->state = DS_STALLED; 881 - 882 - if (ctrl & DBC_CTRL_HALT_IN_TR) { 883 - dep = get_in_ep(dbc); 884 - xhci_dbc_flush_endpoint_requests(dep); 885 - } 886 - 887 - if (ctrl & DBC_CTRL_HALT_OUT_TR) { 888 - dep = get_out_ep(dbc); 889 - xhci_dbc_flush_endpoint_requests(dep); 890 - } 891 - 892 - return EVT_DONE; 893 - } 819 + handle_ep_halt_changes(dbc, get_in_ep(dbc), ctrl & DBC_CTRL_HALT_IN_TR); 820 + handle_ep_halt_changes(dbc, get_out_ep(dbc), ctrl & DBC_CTRL_HALT_OUT_TR); 894 821 895 822 /* Clear DbC run change bit: */ 896 823 if (ctrl & DBC_CTRL_DBC_RUN_CHANGE) { 897 824 writel(ctrl, &dbc->regs->control); 898 825 ctrl = readl(&dbc->regs->control); 899 826 } 900 - 901 827 break; 902 - case DS_STALLED: 903 - ctrl = readl(&dbc->regs->control); 904 - if (!(ctrl & DBC_CTRL_HALT_IN_TR) && 905 - !(ctrl & DBC_CTRL_HALT_OUT_TR) && 906 - (ctrl & DBC_CTRL_DBC_RUN)) { 907 - dbc->state = DS_CONFIGURED; 908 - break; 909 - } 910 - 911 - return EVT_DONE; 912 828 default: 913 829 dev_err(dbc->dev, "Unknown DbC state %d\n", dbc->state); 914 830 break; ··· 971 939 [DS_ENABLED] = "enabled", 972 940 [DS_CONNECTED] = "connected", 973 941 [DS_CONFIGURED] = "configured", 974 - [DS_STALLED] = "stalled", 975 942 }; 976 943 977 944 static ssize_t dbc_show(struct device *dev,

+1 -1

drivers/usb/host/xhci-dbgcap.h

··· 81 81 DS_ENABLED, 82 82 DS_CONNECTED, 83 83 DS_CONFIGURED, 84 - DS_STALLED, 85 84 DS_MAX 86 85 }; 87 86 ··· 89 90 struct list_head list_pending; 90 91 struct xhci_ring *ring; 91 92 unsigned int direction:1; 93 + unsigned int halted:1; 92 94 }; 93 95 94 96 #define DBC_QUEUE_SIZE 16

Configure Feed

Configure Feed