From 9cd8943940e9e72ad51025968f290503667f60de Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Sun, 24 Feb 2019 14:49:10 -0500 Subject: [PATCH] XHCI: Rework locking in HandleTransferComplete. We need to hold the endpoint lock while reading the TD list on the endpoint, as otherwise we have no guarantee that the pointers will not be modified while we are looking at them. Since this is the only consumer of _UnlinkDescriptorForPipe, just make that function assume a lock, and then do all locking within HandleTransferComplete. --- src/add-ons/kernel/busses/usb/xhci.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp b/src/add-ons/kernel/busses/usb/xhci.cpp index 9cf7aa4d1d..ce9f4f4fff 100644 --- a/src/add-ons/kernel/busses/usb/xhci.cpp +++ b/src/add-ons/kernel/busses/usb/xhci.cpp @@ -1631,7 +1631,8 @@ status_t XHCI::_UnlinkDescriptorForPipe(xhci_td *descriptor, xhci_endpoint *endpoint) { TRACE("_UnlinkDescriptorForPipe\n"); - MutexLocker endpointLocker(endpoint->lock); + // We presume that the caller has already locked or owns the endpoint. + endpoint->used--; if (descriptor == endpoint->td_head) { endpoint->td_head = descriptor->next; @@ -2092,11 +2093,13 @@ XHCI::HandleTransferComplete(xhci_trb* trb) return; } + xhci_device *device = &fDevices[slot]; + xhci_endpoint *endpoint = &device->endpoints[endpointNumber - 1]; + MutexLocker endpointLocker(endpoint->lock); + 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_endpoint *endpoint = &device->endpoints[endpointNumber - 1]; for (xhci_td *td = endpoint->td_head; td != NULL; td = td->next) { for (xhci_td *td_chain = td; td_chain != NULL; @@ -2105,7 +2108,7 @@ XHCI::HandleTransferComplete(xhci_trb* trb) if (offset < 0 || offset >= XHCI_MAX_TRBS_PER_TD) continue; - TRACE("HandleTransferComplete td %p trb %" B_PRId64 " found\n" + TRACE("HandleTransferComplete td %p trb %" B_PRId64 " found\n", td_chain, offset); // The TRB at offset trb_count will be the link TRB, which we do not @@ -2113,6 +2116,8 @@ XHCI::HandleTransferComplete(xhci_trb* trb) // We really care about the properly last TRB, at index "count - 1". if (offset == td_chain->trb_count - 1) { _UnlinkDescriptorForPipe(td, endpoint); + endpointLocker.Unlock(); + td->trb_completion_code = completionCode; td->trb_left = remainder; // add descriptor to finished list @@ -2377,7 +2382,6 @@ XHCI::CompleteEvents() if (j != k) break; - switch (event) { case TRB_TYPE_COMMAND_COMPLETION: HandleCmdComplete(&fEventRing[i]);