XHCI: Rewrite transfer-complete handling code.

* Don't set the IOC bit on the link TRB in LinkDescriptorForPipe. We don't
   want to know about this' completion, only about the other transfers
   completion and statuses. This should halve the interrupt rate.
 * Check if this is an Event Data TRB, and return an error if it is.
   (I haven't managed to trigger this code, but it is theoretically possible.)
 * Rewrite loops for clarity and consistency.
 * Use the correct offset when checking for the TRB.
   - Don't rely on the trb_count to tell us whether the TRB is in the TD,
     but just check the address based on MAX_TRBS_PER_TD.
   - Previously, as the link TRB would trigger an interrupt, we could rely
     on that to determine when the transfer finished. But that of course
     did not tell us the correct status, as the link TRB is techically in a
     different TD, as it isn't linked to the previous TRBs. Now we always use
     "count - 1", which will be the final TRB in the TD, properly speaking.
 * Print errors when we fail to find the TRB for any reason.

Reading multiple GB and abusing "stat" on a usb_disk following this commit
only managed to stall my usb_hid attached mouse once in multiple rounds of
testing, which seems a marked improvement; previously only a few hundred MB
and not that much abuse of "stat" were needed to actually trigger the stall.
So it seems this improves the stall situation considerably.
This commit is contained in:
Augustin Cavalier
2019-02-23 17:08:10 -05:00
parent 6a00da689e
commit 65ceb4c931
+43 -31
View File
@@ -751,10 +751,10 @@ XHCI::SubmitNormalRequest(Transfer *transfer)
} }
if (last->trb_count > 0) { if (last->trb_count > 0) {
last->trbs[last->trb_count - 1].dwtrb3
|= B_HOST_TO_LENDIAN_INT32(TRB_3_IOC_BIT);
last->trbs[last->trb_count - 1].dwtrb3 last->trbs[last->trb_count - 1].dwtrb3
&= B_HOST_TO_LENDIAN_INT32(~TRB_3_CHAIN_BIT); &= B_HOST_TO_LENDIAN_INT32(~TRB_3_CHAIN_BIT);
last->trbs[last->trb_count - 1].dwtrb3
|= B_HOST_TO_LENDIAN_INT32(TRB_3_IOC_BIT);
} }
if (!directionIn) { if (!directionIn) {
@@ -1604,7 +1604,7 @@ XHCI::_LinkDescriptorForPipe(xhci_td *descriptor, xhci_endpoint *endpoint)
last->trbs[last->trb_count].qwtrb0 = addr; last->trbs[last->trb_count].qwtrb0 = addr;
last->trbs[last->trb_count].dwtrb2 = TRB_2_IRQ(0); last->trbs[last->trb_count].dwtrb2 = TRB_2_IRQ(0);
last->trbs[last->trb_count].dwtrb3 = B_HOST_TO_LENDIAN_INT32( last->trbs[last->trb_count].dwtrb3 = B_HOST_TO_LENDIAN_INT32(
TRB_3_TYPE(TRB_TYPE_LINK) | TRB_3_IOC_BIT | TRB_3_CYCLE_BIT); TRB_3_TYPE(TRB_TYPE_LINK) | TRB_3_CYCLE_BIT);
endpoint->trbs[next].qwtrb0 = 0; endpoint->trbs[next].qwtrb0 = 0;
endpoint->trbs[next].dwtrb2 = 0; endpoint->trbs[next].dwtrb2 = 0;
@@ -2076,48 +2076,60 @@ void
XHCI::HandleTransferComplete(xhci_trb* trb) XHCI::HandleTransferComplete(xhci_trb* trb)
{ {
TRACE("HandleTransferComplete trb %p\n", trb); TRACE("HandleTransferComplete trb %p\n", trb);
addr_t source = trb->qwtrb0;
uint8 completionCode = TRB_2_COMP_CODE_GET(trb->dwtrb2);
uint32 remainder = TRB_2_REM_GET(trb->dwtrb2);
uint8 endpointNumber uint8 endpointNumber
= TRB_3_ENDPOINT_GET(B_LENDIAN_TO_HOST_INT32(trb->dwtrb3)); = TRB_3_ENDPOINT_GET(B_LENDIAN_TO_HOST_INT32(trb->dwtrb3));
uint8 slot = TRB_3_SLOT_GET(B_LENDIAN_TO_HOST_INT32(trb->dwtrb3)); uint8 slot = TRB_3_SLOT_GET(B_LENDIAN_TO_HOST_INT32(trb->dwtrb3));
uint8 type = TRB_3_TYPE_GET(B_LENDIAN_TO_HOST_INT32(trb->dwtrb3));
if (slot > fSlotCount) if (slot > fSlotCount)
TRACE_ERROR("invalid slot\n"); TRACE_ERROR("invalid slot\n");
if (endpointNumber == 0 || endpointNumber >= XHCI_MAX_ENDPOINTS) if (endpointNumber == 0 || endpointNumber >= XHCI_MAX_ENDPOINTS)
TRACE_ERROR("invalid endpoint\n"); TRACE_ERROR("invalid endpoint\n");
if (type == TRB_TYPE_EVENT_DATA) {
// TODO: Implement these. (Do we trigger any at present?)
TRACE_ERROR("event data TRBs are not handled yet!\n");
return;
}
addr_t source = trb->qwtrb0;
uint8 completionCode = TRB_2_COMP_CODE_GET(trb->dwtrb2);
uint32 remainder = TRB_2_REM_GET(trb->dwtrb2);
xhci_device *device = &fDevices[slot]; xhci_device *device = &fDevices[slot];
xhci_endpoint *endpoint = &device->endpoints[endpointNumber - 1]; xhci_endpoint *endpoint = &device->endpoints[endpointNumber - 1];
xhci_td *td = endpoint->td_head;
for (; td != NULL; td = td->next) { for (xhci_td *td = endpoint->td_head; td != NULL; td = td->next) {
xhci_td *td_chain = td; for (xhci_td *td_chain = td; td_chain != NULL;
for (; td_chain != NULL; td_chain = td_chain->next_chain) { td_chain = td_chain->next_chain) {
int64 offset = source - td_chain->this_phy; int64 offset = (source - td_chain->this_phy) / sizeof(xhci_trb);
TRACE("HandleTransferComplete td %p offset %" B_PRId64 " %" if (offset < 0 || offset >= XHCI_MAX_TRBS_PER_TD)
B_PRIxADDR "\n", td_chain, offset, source); continue;
offset = offset / sizeof(xhci_trb) + 1;
if (offset <= td_chain->trb_count && offset >= 1) { TRACE("HandleTransferComplete td %p trb %" B_PRId64 " found\n"
TRACE("HandleTransferComplete td %p trb %" B_PRId64 " found " td_chain, offset);
"\n", td_chain, offset);
// is it the last trb? // The TRB at offset trb_count will be the link TRB, which we do not
if (offset == td_chain->trb_count) { // care about (and should not generate an interrupt at all.)
_UnlinkDescriptorForPipe(td, endpoint); // We really care about the properly last TRB, at index "count - 1".
td->trb_completion_code = completionCode; if (offset == td_chain->trb_count - 1) {
td->trb_left = remainder; _UnlinkDescriptorForPipe(td, endpoint);
// add descriptor to finished list td->trb_completion_code = completionCode;
Lock(); td->trb_left = remainder;
td->next = fFinishedHead; // add descriptor to finished list
fFinishedHead = td; Lock();
Unlock(); td->next = fFinishedHead;
release_sem(fFinishTransfersSem); fFinishedHead = td;
TRACE("HandleTransferComplete td %p\n", td); Unlock();
} release_sem(fFinishTransfersSem);
return; TRACE("HandleTransferComplete td %p done\n", td);
} else {
TRACE_ERROR("TRB %" B_PRIxADDR " was found, but it wasn't the "
"last in the TD!\n", source);
} }
return;
} }
} }
TRACE_ERROR("TRB %" B_PRIxADDR " was not found in the endpoint!\n", source);
} }