When submitting transfers there is a race condition between adding the pending

transfer to the transfer list and scheduling the transfer descriptors on the
controller by switching the endpoint tail. Since we only check that the endpoint
head is equal to the endpoint tail to conclude that there are no active
transfers, we need to ensure that we don't accidently do that check before the
transfer descriptors are scheduled. Otherwise we could happen to processed a not
yet started transfer and finishing it before an actual transfer has taken place.
This would then lead to 0 byte transfers and toggle mismatches. To fix this we
now protect the transfer addition and tail switching as well as the check in the
finisher with a per-endpoint mutex. Note that we allocate the lock on the heap
and only store the pointer in the endpoint structure as this one is allocated
from the precious physical memory pool. Could fix #4067.
Also switched the Jamfile to UsePrivateKernelHeaders (for the MutexLocker) and
removed BeOS compatibility.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@33129 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/busses/usb/Jamfile

@@ -1,14 +1,9 @@
 SubDir HAIKU_TOP src add-ons kernel busses usb ;
 
-SetSubDirSupportedPlatformsBeOSCompatible ;
-
 SubDirC++Flags -fno-rtti ;
 
-UsePrivateHeaders [ FDirName kernel ] ; 
+UsePrivateKernelHeaders ; 
 UseHeaders [ FDirName $(HAIKU_TOP) src add-ons kernel bus_managers usb ] ;
-if $(TARGET_PLATFORM) != haiku {
-	UsePublicHeaders [ FDirName drivers ] ;
-}
 
 KernelAddon <usb>uhci :
 	uhci.cpp

src/add-ons/kernel/busses/usb/ohci.cpp

@@ -12,6 +12,7 @@
 #include <PCI.h>
 #include <USB3.h>
 #include <KernelExport.h>
+#include <util/AutoLock.h>
 
 #include "ohci.h"
 
@@ -886,9 +887,11 @@ OHCI::_FinishTransfers()
 		while (transfer) {
 			bool transferDone = false;
 			ohci_general_td *descriptor = transfer->first_descriptor;
+			ohci_endpoint_descriptor *endpoint = transfer->endpoint;
 			status_t callbackStatus = B_OK;
 
-			ohci_endpoint_descriptor *endpoint = transfer->endpoint;
+			MutexLocker endpointLocker(endpoint->lock);
+
 			if ((endpoint->head_physical_descriptor & OHCI_ENDPOINT_HEAD_MASK)
 				!= endpoint->tail_physical_descriptor) {
 				// there are still active transfers on this endpoint, we need
@@ -900,6 +903,8 @@ OHCI::_FinishTransfers()
 				continue;
 			}
 
+			endpointLocker.Unlock();
+
 			while (descriptor && !transfer->canceled) {
 				uint32 status = OHCI_TD_GET_CONDITION_CODE(descriptor->flags);
 				if (status == OHCI_TD_CONDITION_NOT_ACCESSED) {
@@ -1148,6 +1153,8 @@ OHCI::_SubmitRequest(Transfer *transfer)
 	// Add to the transfer list
 	ohci_endpoint_descriptor *endpoint
 		= (ohci_endpoint_descriptor *)transfer->TransferPipe()->ControllerCookie();
+
+	MutexLocker endpointLocker(endpoint->lock);
 	result = _AddPendingTransfer(transfer, endpoint, setupDescriptor,
 		dataDescriptor, statusDescriptor, directionIn);
 	if (result < B_OK) {
@@ -1158,6 +1165,7 @@ OHCI::_SubmitRequest(Transfer *transfer)
 
 	// Add the descriptor chain to the endpoint
 	_SwitchEndpointTail(endpoint, setupDescriptor, statusDescriptor);
+	endpointLocker.Unlock();
 
 	// Tell the controller to process the control list
 	endpoint->flags &= ~OHCI_ENDPOINT_SKIP;
@@ -1199,6 +1207,8 @@ OHCI::_SubmitTransfer(Transfer *transfer)
 	// Add to the transfer list
 	ohci_endpoint_descriptor *endpoint
 		= (ohci_endpoint_descriptor *)pipe->ControllerCookie();
+
+	MutexLocker endpointLocker(endpoint->lock);
 	result = _AddPendingTransfer(transfer, endpoint, firstDescriptor,
 		firstDescriptor, lastDescriptor, directionIn);
 	if (result < B_OK) {
@@ -1209,8 +1219,9 @@ OHCI::_SubmitTransfer(Transfer *transfer)
 
 	// Add the descriptor chain to the endpoint
 	_SwitchEndpointTail(endpoint, firstDescriptor, lastDescriptor);
-	endpoint->flags &= ~OHCI_ENDPOINT_SKIP;
+	endpointLocker.Unlock();
 
+	endpoint->flags &= ~OHCI_ENDPOINT_SKIP;
 	if (pipe->Type() & USB_OBJECT_BULK_PIPE) {
 		// Tell the controller to process the bulk list
 		_WriteReg(OHCI_COMMAND_STATUS, OHCI_BULK_LIST_FILLED);
@@ -1306,13 +1317,22 @@ OHCI::_AllocateEndpoint()
 	ohci_endpoint_descriptor *endpoint;
 	void *physicalAddress;
 
+	mutex *lock = (mutex *)malloc(sizeof(mutex));
+	if (lock == NULL) {
+		TRACE_ERROR("no memory to allocate endpoint lock\n");
+		return NULL;
+	}
+
 	// Allocate memory chunk
 	if (fStack->AllocateChunk((void **)&endpoint, &physicalAddress,
 		sizeof(ohci_endpoint_descriptor)) < B_OK) {
 		TRACE_ERROR("failed to allocate endpoint descriptor\n");
+		free(lock);
 		return NULL;
 	}
 
+	mutex_init(lock, "ohci endpoint lock");
+
 	endpoint->flags = OHCI_ENDPOINT_SKIP;
 	endpoint->physical_address = (addr_t)physicalAddress;
 	endpoint->head_physical_descriptor = 0;
@@ -1320,6 +1340,7 @@ OHCI::_AllocateEndpoint()
 	endpoint->tail_physical_descriptor = 0;
 	endpoint->next_logical_endpoint = NULL;
 	endpoint->next_physical_endpoint = 0;
+	endpoint->lock = lock;
 	return endpoint;
 }
 
@@ -1330,6 +1351,9 @@ OHCI::_FreeEndpoint(ohci_endpoint_descriptor *endpoint)
 	if (!endpoint)
 		return;
 
+	mutex_destroy(endpoint->lock);
+	free(endpoint->lock);
+
 	fStack->FreeChunk((void *)endpoint, (void *)endpoint->physical_address,
 		sizeof(ohci_endpoint_descriptor));
 }

src/add-ons/kernel/busses/usb/ohci_hardware.h

@@ -304,6 +304,7 @@ typedef struct {
 	addr_t	physical_address;			// Physical pointer to this address
 	void	*tail_logical_descriptor;	// Queue tail logical pointer
 	void	*next_logical_endpoint;		// Logical pointer to the next endpoint
+	mutex	*lock;						// Protects tail changes and checks
 } ohci_endpoint_descriptor;
 
 #define	OHCI_ENDPOINT_ADDRESS_MASK				0x0000007f